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

[Imre Deak]

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 
> Cc: Damien Lespiau 
> Cc: Imre Deak 
> Cc: Sunil Kamath 
> Signed-off-by: Animesh Manna 
> Signed-off-by: Vathsala Nagaraju 
> ---
>  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;
>  }


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

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

[Animesh Manna]

On 8/4/2015 5:03 PM, Animesh Manna wrote:



On 8/4/2015 4:50 PM, Sunil Kamath wrote:

On Monday 03 August 2015 09:55 PM, Animesh Manna wrote:

As csr firmware is taking care of loading the firmware,
so no need for driver to load again.

Cc: Daniel Vetter daniel.vet...@intel.com
Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-by: Vathsala Nagaraju vathsala.nagar...@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);


Same comment as before.
The context save and restore program is reset on cold boot, warm 
reset, PCI function level reset, and hibernate/suspend.


Need valid reason to do this change. If reading DC5/DC6 counters is a 
concern, lets use this as just debug patch.


Dont hurry on this patch.
Need to close on the above opens.

- Sunil


Agree, I want to add this patch as part of this patch series, already 
started communication with firmware team.
Waiting for suggestion, will followup and proceed further based on 
suggestion.



Regards,
Animesh


Firmware team confirmed that one time firmware loading during driver loading is 
sufficient, no need
to load firmware in csr-address-space every suspend (dc6 entry) - resume (dc6 
exit) flow, dmc will
take care of it.

- Animesh


return 0;
  }






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

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

[Sunil Kamath]

On Monday 03 August 2015 09:55 PM, Animesh Manna wrote:

As csr firmware is taking care of loading the firmware,
so no need for driver to load again.

Cc: Daniel Vetter daniel.vet...@intel.com
Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-by: Vathsala Nagaraju vathsala.nagar...@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);


Same comment as before.
The context save and restore program is reset on cold boot, warm reset, 
PCI function level reset, and hibernate/suspend.


Need valid reason to do this change. If reading DC5/DC6 counters is a 
concern, lets use this as just debug patch.


Dont hurry on this patch.
Need to close on the above opens.

- Sunil
  
  	return 0;

  }


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

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

[Animesh Manna]

On 8/4/2015 4:50 PM, Sunil Kamath wrote:

On Monday 03 August 2015 09:55 PM, Animesh Manna wrote:

As csr firmware is taking care of loading the firmware,
so no need for driver to load again.

Cc: Daniel Vetter daniel.vet...@intel.com
Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-by: Vathsala Nagaraju vathsala.nagar...@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);


Same comment as before.
The context save and restore program is reset on cold boot, warm 
reset, PCI function level reset, and hibernate/suspend.


Need valid reason to do this change. If reading DC5/DC6 counters is a 
concern, lets use this as just debug patch.


Dont hurry on this patch.
Need to close on the above opens.

- Sunil


Agree, I want to add this patch as part of this patch series, already started 
communication with firmware team.
Waiting for suggestion, will followup and proceed further based on suggestion.


Regards,
Animesh

return 0;
  }




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

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

[Animesh Manna]

As csr firmware is taking care of loading the firmware,
so no need for driver to load again.

Cc: Daniel Vetter daniel.vet...@intel.com
Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
Signed-off-by: Vathsala Nagaraju vathsala.nagar...@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);
 
return 0;
 }
-- 
2.0.2

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