[PATCH 04/13] drm/i915/dmc_wl: Get wakelock when disabling dynamic DC states

[Luca Coelho]

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 at intel.com>
> > > ---
> > >  .../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.