Re: [PATCH v2 06/20] drm/i915/cdclk: Extract dg2_power_well_count()

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Sep 24, 2025 at 12:05:56PM +0300, Jani Nikula wrote:
> On Wed, 24 Sep 2025, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Extract the code to determine the DG2 pipe power well count
> > into a small helper. I'll have other uses for this later.
> >
> > TODO: need to move this power well stuff out from the cdclk code...
> >
> > v2: Don't lose the early return from intel_cdclk_pcode_pre_notify()
> >     (kernel test robot)
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cdclk.c | 33 +++++++++++++---------
> >  1 file changed, 19 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index 05d9f488895e..f190cfb85a34 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -2591,6 +2591,12 @@ static void intel_set_cdclk(struct intel_display *display,
> >  	}
> >  }
> >  
> > +static bool dg2_power_well_count(struct intel_display *display,
> > +				 const struct intel_cdclk_state *cdclk_state)
> > +{
> > +	return display->platform.dg2 ? hweight8(cdclk_state->active_pipes) : 0;
> > +}
> > +
> >  static void intel_cdclk_pcode_pre_notify(struct intel_atomic_state *state)
> >  {
> >  	struct intel_display *display = to_intel_display(state);
> > @@ -2603,16 +2609,16 @@ static void intel_cdclk_pcode_pre_notify(struct intel_atomic_state *state)
> >  
> >  	if (!intel_cdclk_changed(&old_cdclk_state->actual,
> >  				 &new_cdclk_state->actual) &&
> > -				 new_cdclk_state->active_pipes ==
> > -				 old_cdclk_state->active_pipes)
> > +	    dg2_power_well_count(display, old_cdclk_state) ==
> > +	    dg2_power_well_count(display, old_cdclk_state))
> 
> Both have old_cdclk_state, making this always true.

doh

> 
> Side note, perhaps the whole function should be renamed
> dg2_cdclk_pcode_pre_notify(), because the additional dg2 check in
> dg2_power_well_count() felt weird until I realized this is all dg2 only.

Yeah, the whole thing should really be moved out from the cdclk
code completely. This is essentially a variant of pmdemand,
just through the pcode mailbox instead of using dedicated
pmdemand registers (which internally will get sent over
to pcode anyway). But the actual pmdemand code is also borked
in various ways, so that too will need a lot of work :/

I can cook up a patch to rename these in the meantime.

-- 
Ville Syrjälä
Intel