On Mon, 23 Oct 2017 14:15:06 +0200, Chris Wilson
<chris@xxxxxxxxxxxxxxxxxx> wrote:
Quoting Michal Wajdeczko (2017-10-23 13:04:49)
On Sun, 22 Oct 2017 12:15:50 +0200, Chris Wilson
<chris@xxxxxxxxxxxxxxxxxx> wrote:
> Just show the firmware URL on the first failure, not every failure.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_csr.c | 3 +--
> drivers/gpu/drm/i915/intel_uc_fw.c | 13 +++++++++++--
> drivers/gpu/drm/i915/intel_uc_fw.h | 5 ++---
> 3 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_csr.c
> b/drivers/gpu/drm/i915/intel_csr.c
> index da9de47562b8..12a857acdb9b 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -427,8 +427,7 @@ static void csr_load_work_fn(struct work_struct
> *work)
> "Failed to load DMC firmware %s."
> " Disabling runtime power management.\n",
> csr->fw_path);
> - dev_notice(dev_priv->drm.dev, "DMC firmware homepage:
%s",
> - INTEL_UC_FIRMWARE_URL);
> + intel_uc_fw_show_url(dev_priv);
> }
> release_firmware(fw);
> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c
> b/drivers/gpu/drm/i915/intel_uc_fw.c
> index 973888e94cba..4ee90fda326b 100644
> --- a/drivers/gpu/drm/i915/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_uc_fw.c
> @@ -28,6 +28,16 @@
> #include "intel_uc_fw.h"
> #include "i915_drv.h"
> +/* Home of GuC, HuC and DMC firmwares */
> +#define INTEL_UC_FIRMWARE_URL
> "https://01.org/linuxgraphics/downloads/firmware";
> +
> +void intel_uc_fw_show_url(struct drm_i915_private *i915)
> +{
> + dev_info_once(i915->drm.dev,
> + "Firmware can be downloaded from %s\n",
> + INTEL_UC_FIRMWARE_URL);
> +}
> +
> /**
> * intel_uc_fw_fetch - fetch uC firmware
> *
> @@ -189,8 +199,7 @@ void intel_uc_fw_fetch(struct drm_i915_private
> *dev_priv,
> DRM_WARN("%s: Failed to fetch firmware %s (error %d)\n",
> intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
> - DRM_INFO("%s: Firmware can be downloaded from %s\n",
> - intel_uc_fw_type_repr(uc_fw->type),
INTEL_UC_FIRMWARE_URL);
> + intel_uc_fw_show_url(dev_priv);
Hmm, intel_uc_fw_fetch() should be called only once per specific uC
and only as part of i915_driver_load() - so there shall be no repeated
fetch failures for the same uC.
Also your changed message is little too generic (in old one we had uC
type, which may be helpful for the user).
I had the same url multiple times; at different log levels, that was
ugly. The url we give is the base for all firmwares, which one failed is
given in the previous line (including expected version) so unless you
plan on generating the url for the specific firmware, it is repeating
itself.
Hmm, we can use query url (and this will work even without immediate
support
from 01.org site, user will just see homepage). Then your log will look
like:
[ 4.961005] i915 0000:00:02.0: Direct firmware load for
i915/bxt_dmc_ver1_07.bin failed with error -2
[ 4.961035] i915 0000:00:02.0: DMC firmware can be downloaded from:
https://01.org/linuxgraphics/downloads/firmware?name=i915/bxt_dmc_ver1_07.bin
[ 4.983194] i915 0000:00:02.0: Direct firmware load for
i915/bxt_huc_ver01_07_1398.bin failed with error -2
[ 4.983224] i915 0000:00:02.0: HuC firmware can be downloaded from:
https://01.org/linuxgraphics/downloads/firmware?name=i915/bxt_huc_ver01_07_1398.bin
And in case of independent
fetch failures (lets say there is no DMC and HuC fw), we may wrongly
suggest that given url is for fw that failed first (DMC?), as messages
from second failures (HuC) will not have such hint - user will have to
grep whole dmesg to guess that earlier url is applicable there too.
Correct, but such warnings are clustered.
And they are clustered accidentally as DMC fw is loaded from separate
csr.work.
[ 4.961005] i915 0000:00:02.0: Direct firmware load for
i915/bxt_dmc_ver1_07.bin failed with error -2
[ 4.961028] i915 0000:00:02.0: Failed to load DMC firmware
i915/bxt_dmc_ver1_07.bin. Disabling runtime power management.
[ 4.961035] i915 0000:00:02.0: DMC firmware homepage:
https://01.org/linuxgraphics/downloads/firmware
[ 4.983194] i915 0000:00:02.0: Direct firmware load for
i915/bxt_huc_ver01_07_1398.bin failed with error -2
[ 4.983218] [drm] HuC: Failed to fetch firmware
i915/bxt_huc_ver01_07_1398.bin (error -2)
[ 4.983224] [drm] HuC: Firmware can be downloaded from
https://01.org/linuxgraphics/downloads/firmware
So can we limit this patch only to s/DRM_INFO/dev_notice ?
This is info, not notice.
Correct.
It's not a significant condition as that's the
"ok, load failed, but we can survive with little change" before and this
is just the information where to find the firmware (outside of their
distro packing linux-firmwares which presumably was out of date).
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
