Re: [SKL-DMC-BUGFIX 5/5] drm/i915/skl: Removed csr firmware load in resume path

Imre Deak
2015-10-12T14:02:59Z

On ma, 2015-08-03 at 21:55 +0530, Animesh Manna wrote:
> As csr firmware is taking care of loading the firmware,
> so no need for driver to load again.

I'd think that it's some other (PUnit, PCode) firmware that would take
care of reprogramming the CSR(DMC) firmware, rather than the CSR
firmware itself.

> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Sunil Kamath <sunil.kamath@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e1d0102..02019e9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1066,13 +1066,10 @@ static int bxt_resume_prepare(struct drm_i915_private *dev_priv)
>
> static int skl_resume_prepare(struct drm_i915_private *dev_priv)
> {
> - struct drm_device *dev = dev_priv->dev;
> -
> if (intel_csr_load_status_get(dev_priv) == FW_LOADED)
> skl_disable_dc6(dev_priv);
>
> skl_init_cdclk(dev_priv);
> - intel_csr_load_program(dev);

This also means that we don't reprogram the firmware during S3 and S4
resume. I can see how this would work during S3 resume, but I can't see
how it would work during S4 resume, especially if the kernel loading the
hibernation image doesn't load the firmware itself (possible if it
doesn't have the i915 driver). If you confirmed that we don't need to
reprogram the firmware ever, please add this to the commit message
mentioning explicitly the D3, S0ix, S3, S4 states.

>
> return 0;
> }