Re: [PATCH 02/14] drm/i915/cdclk: Pass CDCLK in MHz to pcode on DG2

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

On Wed, Jun 10, 2026 at 08:31:48PM +0300, Jani Nikula wrote:
> On Wed, 10 Jun 2026, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We are currently trying to pass the CDCLK in kHz to the pcode
> > on DG2, while the pcode expects a value in MHz units. Adjust
> > the units appropriately.
> 
> How is it working? :o

I don't think DG2 pcode does all that much a with the information.
Eg. AFAIK it doesn't actually adjust any voltages due to this stuff.
I think it's more for some internal power usage estimates or something,
but dunno what that really means in practice.

> 
> Fixes: ?
> 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cdclk.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index 9ca56bab281f..9718062d8d6c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -2703,8 +2703,10 @@ static void intel_cdclk_pcode_pre_notify(struct intel_atomic_state *state)
> >  	 * if CDCLK is decreasing or not changing, set bits 25:16 to current CDCLK,
> >  	 * which basically means we choose the maximum of old and new CDCLK, if we know both
> >  	 */
> > -	if (change_cdclk)
> > +	if (change_cdclk) {
> >  		cdclk = max(new_cdclk_state->actual.cdclk, old_cdclk_state->actual.cdclk);
> > +		cdclk = DIV_ROUND_UP(cdclk, 1000);
> > +	}
> 
> I'd consider s/cdclk/cdclk_mhz/g here and in intel_pcode_notify() to
> emphasize it's not kHz.
> 
> >  	/*
> >  	 * According to "Sequence For Pipe Count Change",
> > @@ -2740,8 +2742,10 @@ static void intel_cdclk_pcode_post_notify(struct intel_atomic_state *state)
> >  	 * According to "Sequence After Frequency Change",
> >  	 * set bits 25:16 to current CDCLK
> >  	 */
> > -	if (update_cdclk)
> > +	if (update_cdclk) {
> >  		cdclk = new_cdclk_state->actual.cdclk;
> > +		cdclk = DIV_ROUND_UP(cdclk, 1000);
> > +	}
> 
> Ditto.
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> >  
> >  	/*
> >  	 * According to "Sequence For Pipe Count Change",
> 
> -- 
> Jani Nikula, Intel

-- 
Ville Syrjälä
Intel