[Intel-gfx] [PATCH] drm/i915: Disable DC5 before going to DC9

[Intel-gfx] [PATCH] drm/i915: Disable DC5 before going to DC9

[Rodrigo Vivi]

According to BSPec:
	Sequence to Allow DC9:
		1. Follow Sequence to Disallow DC5.

which is:
	Sequence to Disallow DC5 and DC6
		Set DC_STATE_EN Dynamic DC State Enable = "Disable".

I understand that we haven't had any issue so far. But since
DC9 is a software thing, it is better to disable DC5 before
to avoid any conflict. And respect the spec to avoid potential
future issues.

Cc: Imre Deak <imre.deak@intel.com>
Cc: Anshuman Gupta <anshuman.gupta@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_power.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 6a5695008f7c..b3cf5182044f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -883,6 +883,9 @@ static void bxt_enable_dc9(struct drm_i915_private *dev_priv)
 {
 	assert_can_enable_dc9(dev_priv);
 
+	/* Disable DC5 before enabling DC9 */
+	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
+
 	drm_dbg_kms(&dev_priv->drm, "Enabling DC9\n");
 	/*
 	 * Power sequencer reset is not needed on
-- 
2.34.1

Re: [Intel-gfx] [PATCH] drm/i915: Disable DC5 before going to DC9

[Srivatsa, Anusha]

> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Sent: Wednesday, April 20, 2022 12:09 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Deak, Imre
> <imre.deak@intel.com>; Gupta, Anshuman <anshuman.gupta@intel.com>;
> Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Subject: [PATCH] drm/i915: Disable DC5 before going to DC9
> 
> According to BSPec:
> 	Sequence to Allow DC9:
> 		1. Follow Sequence to Disallow DC5.
> 
> which is:
> 	Sequence to Disallow DC5 and DC6
> 		Set DC_STATE_EN Dynamic DC State Enable = "Disable".
> 
> I understand that we haven't had any issue so far. But since
> DC9 is a software thing, it is better to disable DC5 before to avoid any conflict.
> And respect the spec to avoid potential future issues.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_power.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 6a5695008f7c..b3cf5182044f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -883,6 +883,9 @@ static void bxt_enable_dc9(struct drm_i915_private
> *dev_priv)  {
>  	assert_can_enable_dc9(dev_priv);

^ We are already checking if DC5 is enabled. If enabled then don't enable DC9. SO the change should be- if driver cannot enableDC9 because DC5 is enabled then go ahead and disable DC5.

Anusha 
> +	/* Disable DC5 before enabling DC9 */
> +	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> +
>  	drm_dbg_kms(&dev_priv->drm, "Enabling DC9\n");
>  	/*
>  	 * Power sequencer reset is not needed on
> --
> 2.34.1

Re: [Intel-gfx] [PATCH] drm/i915: Disable DC5 before going to DC9

[Imre Deak]

On Wed, Apr 20, 2022 at 03:09:21PM -0400, Rodrigo Vivi wrote:
> According to BSPec:
> 	Sequence to Allow DC9:
> 		1. Follow Sequence to Disallow DC5.
> 
> which is:
> 	Sequence to Disallow DC5 and DC6
> 		Set DC_STATE_EN Dynamic DC State Enable = "Disable".
> 
> I understand that we haven't had any issue so far. But since
> DC9 is a software thing, it is better to disable DC5 before
> to avoid any conflict. And respect the spec to avoid potential
> future issues.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_power.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 6a5695008f7c..b3cf5182044f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -883,6 +883,9 @@ static void bxt_enable_dc9(struct drm_i915_private *dev_priv)
>  {
>  	assert_can_enable_dc9(dev_priv);
>  
> +	/* Disable DC5 before enabling DC9 */
> +	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);

For DC9 DMC should be disabled already, along with DC states and other
dependencies like power well 1, etc. That happens in
bxt/icl_display_core_uninit().

> +
>  	drm_dbg_kms(&dev_priv->drm, "Enabling DC9\n");
>  	/*
>  	 * Power sequencer reset is not needed on
> -- 
> 2.34.1
> 

Re: [Intel-gfx] [PATCH] drm/i915: Disable DC5 before going to DC9

[Rodrigo Vivi]

On Wed, Apr 20, 2022 at 10:21:19PM +0300, Imre Deak wrote:
> On Wed, Apr 20, 2022 at 03:09:21PM -0400, Rodrigo Vivi wrote:
> > According to BSPec:
> > 	Sequence to Allow DC9:
> > 		1. Follow Sequence to Disallow DC5.
> > 
> > which is:
> > 	Sequence to Disallow DC5 and DC6
> > 		Set DC_STATE_EN Dynamic DC State Enable = "Disable".
> > 
> > I understand that we haven't had any issue so far. But since
> > DC9 is a software thing, it is better to disable DC5 before
> > to avoid any conflict. And respect the spec to avoid potential
> > future issues.
> > 
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display_power.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 6a5695008f7c..b3cf5182044f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -883,6 +883,9 @@ static void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> >  {
> >  	assert_can_enable_dc9(dev_priv);
> >  
> > +	/* Disable DC5 before enabling DC9 */
> > +	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> 
> For DC9 DMC should be disabled already, along with DC states and other
> dependencies like power well 1, etc. That happens in
> bxt/icl_display_core_uninit().

but that wasn't on the suspend_late path... but Anusha is right and
probably the check in the assert is already enough for now... if we
notice that assert is coming we do something else...

I was willing to avoid indirection and the risk of changing something
and this dc5 disable getting forgotten, but with the assert we are
probably good...

let's just ignore this patch...

> 
> > +
> >  	drm_dbg_kms(&dev_priv->drm, "Enabling DC9\n");
> >  	/*
> >  	 * Power sequencer reset is not needed on
> > -- 
> > 2.34.1
> > 

[Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Disable DC5 before going to DC9

[Patchwork]

== Series Details ==

Series: drm/i915: Disable DC5 before going to DC9
URL   : https://patchwork.freedesktop.org/series/102881/
State : failure

== Summary ==

Error: patch https://patchwork.freedesktop.org/api/1.0/series/102881/revisions/1/mbox/ not applied
Applying: drm/i915: Disable DC5 before going to DC9
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/display/intel_display_power.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/display/intel_display_power.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/display/intel_display_power.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 drm/i915: Disable DC5 before going to DC9
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".