| Message ID | 20220420190921.419345-1-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Disable DC5 before going to DC9 | expand |
> -----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
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 >
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 > >
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
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(+)