On Tue, 2024-11-05 at 09:44 -0300, Gustavo Sousa wrote:
> Quoting Luca Coelho (2024-11-01 09:24:08-03:00)
> > On Mon, 2024-10-21 at 19:27 -0300, Gustavo Sousa wrote:
> > > Bspec says that disabling dynamic DC states require taking the DMC
> > > wakelock to cause an DC exit before writing to DC_STATE_EN. Implement
> > > that.
> > >
> > > In fact, testing on PTL revealed we end up failing to exit DC5/6 without
> > > this step.
> > >
> > > Bspec: 71583
> > > Signed-off-by: Gustavo Sousa <gustavo.sousa@xxxxxxxxx>
> > > ---
> > > .../drm/i915/display/intel_display_power_well.c | 10 +++++++---
> > > drivers/gpu/drm/i915/display/intel_dmc_wl.c | 14 ++++++++++++--
> > > drivers/gpu/drm/i915/display/intel_dmc_wl.h | 2 ++
> > > 3 files changed, 21 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> > > index adaf7cf3a33b..e8946ce86aaa 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> > > @@ -987,10 +987,14 @@ void gen9_disable_dc_states(struct intel_display *display)
> > > return;
> > > }
> > >
> > > - gen9_set_dc_state(display, DC_STATE_DISABLE);
> > > -
> > > - if (!HAS_DISPLAY(display))
> > > + if (HAS_DISPLAY(display)) {
> > > + intel_dmc_wl_get_noreg(display);
> > > + gen9_set_dc_state(display, DC_STATE_DISABLE);
> > > + intel_dmc_wl_put_noreg(display);
> > > + } else {
> > > + gen9_set_dc_state(display, DC_STATE_DISABLE);
> > > return;
> > > + }
> >
> > I think intel_dmc_get/put() already protect indirectly on
> > HAS_DISPLAY(), doesn't it? If that's the case, then the if here is
> > unnecessary.
>
> Actually, intel_dmc_wl_init() gets called only when HAS_DISPLAY() is
> true, so I think using intel_dmc_wl_{get,put}_noreg() for the
> !HAS_DISPLAY() case would not be right, at least not with the current
> state of the code.
Okay, fair enough.
--
Cheers,
Luca.
