Re: [DMC_REDESIGN_V2 07/14] drm/i915/gen9: Simplify csr loading failure printing.

[Imre Deak <imre.deak@intel.com>]

On ke, 2015-08-26 at 16:58 +0530, Animesh Manna wrote:
> From: Daniel Vetter <daniel.vetter@intel.com>
> 
> If we really want to we can be more verbose here, but we really don't
> need an entire function for this.
> 
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Sunil Kamath <sunil.kamath@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c  | 20 --------------------
>  drivers/gpu/drm/i915/i915_drv.h  |  1 -
>  drivers/gpu/drm/i915/intel_csr.c |  8 +++-----
>  3 files changed, 3 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1ddf3a9..3cdd319 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -537,26 +537,6 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
>  	return true;
>  }
>  
> -void i915_firmware_load_error_print(const char *fw_path, int err)
> -{
> -	DRM_ERROR("failed to load firmware %s (%d)\n", fw_path, err);
> -
> -	/*
> -	 * If the reason is not known assume -ENOENT since that's the most
> -	 * usual failure mode.
> -	 */
> -	if (!err)
> -		err = -ENOENT;
> -
> -	if (!(IS_BUILTIN(CONFIG_DRM_I915) && err == -ENOENT))
> -		return;
> -
> -	DRM_ERROR(
> -	  "The driver is built-in, so to load the firmware you need to\n"
> -	  "include it either in the kernel (see CONFIG_EXTRA_FIRMWARE) or\n"
> -	  "in your initrd/initramfs image.\n");
> -}
> -

The point here was to clarify the reason why the loading failed, since
that caused quite a confusion. It was a separate function since the same
could've been called from the GuC loader too. I think the error message
would be still useful.

>  static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9f01c72..6241b49 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2654,7 +2654,6 @@ extern unsigned long i915_mch_val(struct drm_i915_private *dev_priv);
>  extern unsigned long i915_gfx_val(struct drm_i915_private *dev_priv);
>  extern void i915_update_gfx_val(struct drm_i915_private *dev_priv);
>  int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on);
> -void i915_firmware_load_error_print(const char *fw_path, int err);
>  
>  /* intel_hotplug.c */
>  void intel_hpd_irq_handler(struct drm_device *dev, u32 pin_mask, u32 long_mask);
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 00e9fbf..9d4b37b 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -237,10 +237,8 @@ static void finish_csr_load(const struct firmware *fw, void *context)
>  	uint32_t *dmc_payload;
>  	bool fw_loaded = false;
>  
> -	if (!fw) {
> -		i915_firmware_load_error_print(csr->fw_path, 0);
> +	if (!fw)
>  		goto out;
> -	}
>  
>  	if ((stepping == -ENODATA) || (substepping == -ENODATA)) {
>  		DRM_ERROR("Unknown stepping info, firmware loading failed\n");
> @@ -340,6 +338,8 @@ static void finish_csr_load(const struct firmware *fw, void *context)
>  out:
>  	if (fw_loaded || IS_BROXTON(dev))
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
> +	else
> +		DRM_ERROR("Failed to load DMC firmware, disabling rpm\n");
>  
>  	release_firmware(fw);
>  }
> @@ -380,8 +380,6 @@ void intel_csr_ucode_init(struct drm_device *dev)
>  				      &dev_priv->dev->pdev->dev,
>  				      GFP_KERNEL, dev_priv,
>  				      finish_csr_load);
> -	if (ret)
> -		i915_firmware_load_error_print(csr->fw_path, ret);
>  }
>  
>  /**


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx