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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 19 Oct 2026, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Mon, 19 Oct 2026, Ville Syrjala <ville.syrjala@linux.intel.com> 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.
>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_pfit.c | 66 ++++++++++++++---------
>>  1 file changed, 42 insertions(+), 24 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,39 +720,52 @@ 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)
>> -{
>> -	if (display->platform.i830)
>> -		return false;
>> -
>> -	return DISPLAY_VER(display) >= 4 ||
>> -		display->platform.pineview || display->platform.mobile;
>> -}
>> -
>> -static void i9xx_pfit_get_config(struct intel_crtc_state *crtc_state)
>> +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;
>>  
>> -	if (!i9xx_has_pfit(display))
>> -		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;
>> -
>> +	pipe = REG_FIELD_GET(PFIT_PIPE_MASK, tmp);
>>  	if (pipe != crtc->pipe)
>>  		return;
>>  
>>  	crtc_state->gmch_pfit.control = tmp;
>
> This drops PFIT_PGM_RATIOS read.

Judging by the next patch, this is intentional. Should be mentioned in
the commit message, and why it's okay we're still writing the register
also for i965.

BR,
Jani.

>
> With that fixed,
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
>
>> +}
>> +
>> +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->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);
>> +	u32 tmp;
>> +
>> +	if (!i9xx_has_pfit(crtc))
>> +		return;
>> +
>> +	tmp = intel_de_read(display, PFIT_CONTROL(display));
>> +	if (!(tmp & PFIT_ENABLE))
>> +		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);
>> +	}
>>  }

-- 
Jani Nikula, Intel