Re: [PATCH v2 06/15] drm/i915/pfit: Extract i965_pfit_get_config()

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

On Thu, Nov 12, 2026 at 10:57:03AM +0100, Michał Grzelak wrote:
> On Tue, Oct 20, 2026 at 09:38:42PM GMT, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Split i965_pfit_get_config() out from i9xx_pfit_get_config() to
> > unconfuse things a bit.
> > 
> > Note that we stop reading PFIT_PGM_RATIOS into
> > crtc_state->gmch_pfit.pgm_ratios since wen ever use the
> > progammed ratios on i965+. The .compute_config() path doesn't
> > populate crtc_state->gmch_pfit.pgm_ratios either.
> 
> I got lost a bit here. Maybe:
> s/wen ever/whenever we/
> s/progammed/programmed/
> s/i965+. The/i965+ the/

Its should have been:
 Note that we stop reading PFIT_PGM_RATIOS into
 crtc_state->gmch_pfit.pgm_ratios since we never use the
 programmed ratios on i965+. The .compute_config() path doesn't
 populate crtc_state->gmch_pfit.pgm_ratios either.

>
> 
> But that's just suggestion, not a blocker:
> Reviewed-by: Michał Grzelak <michal.grzelak@linux.intel.com>
> 
> BR,
> Michał
> 
> > 
> > v2: Highlight the crtc_state->gmch_pfit.pgm_ratios change (Jani)
> > 
> > Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_pfit.c | 58 +++++++++++++++--------
> >  1 file changed, 38 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_pfit.c b/drivers/gpu/drm/i915/display/intel_pfit.c
> > index 972218346362..82804159d0a1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_pfit.c
> > +++ b/drivers/gpu/drm/i915/display/intel_pfit.c
> > @@ -720,38 +720,51 @@ void i9xx_pfit_disable(const struct intel_crtc_state *old_crtc_state)
> >  	intel_de_write(display, PFIT_CONTROL(display), 0);
> >  }
> >  
> > -static bool i9xx_has_pfit(struct intel_display *display)
> > +static void i965_pfit_get_config(struct intel_crtc_state *crtc_state)
> >  {
> > +	struct intel_display *display = to_intel_display(crtc_state);
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > +	enum pipe pipe;
> > +	u32 tmp;
> > +
> > +	tmp = intel_de_read(display, PFIT_CONTROL(display));
> > +	if (!(tmp & PFIT_ENABLE))
> > +		return;
> > +
> > +	/* Check whether the pfit is attached to our pipe. */
> > +	pipe = REG_FIELD_GET(PFIT_PIPE_MASK, tmp);
> > +	if (pipe != crtc->pipe)
> > +		return;
> > +
> > +	crtc_state->gmch_pfit.control = tmp;
> > +}
> > +
> > +static bool i9xx_has_pfit(struct intel_crtc *crtc)
> > +{
> > +	struct intel_display *display = to_intel_display(crtc);
> > +
> > +	if (crtc->pipe != PIPE_B)
> > +		return false;
> > +
> >  	if (display->platform.i830)
> >  		return false;
> >  
> > -	return DISPLAY_VER(display) >= 4 ||
> > -		display->platform.pineview || display->platform.mobile;
> > +	return display->platform.pineview || display->platform.mobile;
> >  }
> >  
> >  static void i9xx_pfit_get_config(struct intel_crtc_state *crtc_state)
> >  {
> >  	struct intel_display *display = to_intel_display(crtc_state);
> >  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > -	enum pipe pipe;
> >  	u32 tmp;
> >  
> > -	if (!i9xx_has_pfit(display))
> > +	if (!i9xx_has_pfit(crtc))
> >  		return;
> >  
> >  	tmp = intel_de_read(display, PFIT_CONTROL(display));
> >  	if (!(tmp & PFIT_ENABLE))
> >  		return;
> >  
> > -	/* Check whether the pfit is attached to our pipe. */
> > -	if (DISPLAY_VER(display) >= 4)
> > -		pipe = REG_FIELD_GET(PFIT_PIPE_MASK, tmp);
> > -	else
> > -		pipe = PIPE_B;
> > -
> > -	if (pipe != crtc->pipe)
> > -		return;
> > -
> >  	crtc_state->gmch_pfit.control = tmp;
> >  	crtc_state->gmch_pfit.pgm_ratios =
> >  		intel_de_read(display, PFIT_PGM_RATIOS(display));
> > @@ -761,10 +774,15 @@ void intel_pfit_get_config(struct intel_crtc_state *crtc_state)
> >  {
> >  	struct intel_display *display = to_intel_display(crtc_state);
> >  
> > -	if (HAS_GMCH(display))
> > -		i9xx_pfit_get_config(crtc_state);
> > -	else if (DISPLAY_VER(display) >= 9)
> > -		skl_scaler_get_config(crtc_state);
> > -	else
> > -		ilk_pfit_get_config(crtc_state);
> > +	if (HAS_GMCH(display)) {
> > +		if (DISPLAY_VER(display) >= 4)
> > +			i965_pfit_get_config(crtc_state);
> > +		else
> > +			i9xx_pfit_get_config(crtc_state);
> > +	} else {
> > +		if (DISPLAY_VER(display) >= 9)
> > +			skl_scaler_get_config(crtc_state);
> > +		else
> > +			ilk_pfit_get_config(crtc_state);
> > +	}
> >  }
> > -- 
> > 2.55.0
> > 
> > 

-- 
Ville Syrjälä
Intel