Re: [PATCH 15/15] drm/i915/pfit: Allow downscaling on i966+

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

On Mon, Oct 19, 2026 at 02:47:56PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] drm/i915/pfit: off-by-one max_scale in i9xx_pfit_check_scaling()
> --
> 
> commit c5496f644982a254aa09057b7c02f1bea3cb5b18
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> drm/i915/pfit: Allow downscaling on i966+
> 
> This commit updates i9xx_pfit_check_scaling() to raise max_scale to 0x20000 on
> DISPLAY_VER(display) >= 4 platforms to permit panel fitter downscaling.
> 
> > diff --git a/drivers/gpu/drm/i915/display/intel_pfit.c b/drivers/gpu/drm/i915/display/intel_pfit.c
> > index 6eb457047bf4c..2b6706c7af534 100644
> > --- a/drivers/gpu/drm/i915/display/intel_pfit.c
> > +++ b/drivers/gpu/drm/i915/display/intel_pfit.c
> > @@ -563,7 +563,10 @@ static int i9xx_pfit_check_scaling(const struct intel_crtc_state *crtc_state)
> >  	int hscale, vscale, max_scale;
> >  	struct drm_rect src;
> >  
> > -	max_scale = 0x10000;
> > +	if (DISPLAY_VER(display) >= 4)
> > +		max_scale = 0x20000;
> > +	else
> > +		max_scale = 0x10000;
> >  
> >  	drm_rect_init(&src, 0, 0, pipe_src_w << 16, pipe_src_h << 16);
> >  
> 
> [Severity: Medium]
> Should max_scale be 0x20000 - 1 (or 0x1fff0) rather than 0x20000 here?

I guess I'll just go with 0x20000-1 for now. I suppose that isn't
100% correct because it doesn't match the hardware precision, but
then again neither does the rounding done by drm_rect_{h,v}scale().
The same issue actually exist in the ILK+ pfit and SKL+ scaler
code as well, though in that case the hw precision is .15. I think
what we should do is perform the check using the hardware precision
and rounding, but that check will have to be done after clipping
and all this stuff right now runs before clipping. Currently we
don't have clipping for pfit but I have a branch somewhere that
tries to add that to properly handle the pfit+joiner combo.
For SKL+ plane scaling clipping is already there of course.

> 
> In i9xx_pfit_check_scaling(), max_scale is passed to drm_rect_calc_hscale()
> and drm_rect_calc_vscale(), which treat max_hscale and max_vscale as inclusive
> upper bounds and only reject with -ERANGE when scale > max_scale. Setting
> max_scale = 0x20000 allows an exact 2.0x downscale factor (for example, when
> scaling a 1600x1200 pipe_src to an 800x600 dst).
> 
> However, in drivers/gpu/drm/i915/display/intel_pfit_regs.h, the i965+ scaling
> ratio fields in PFIT_PGM_RATIOS are 13-bit 1.12 fixed-point values:
> 
> #define   PFIT_VERT_SCALE_MASK_965	REG_GENMASK(28, 16) /* 965+ */
> #define   PFIT_VERT_SCALE_965(x)	REG_FIELD_PREP(PFIT_VERT_SCALE_MASK_965, (x))
> #define   PFIT_HORIZ_SCALE_MASK_965	REG_GENMASK(12, 0) /* 965+ */
> #define   PFIT_HORIZ_SCALE_965(x)	REG_FIELD_PREP(PFIT_HORIZ_SCALE_MASK_965, (x))
> 
> These 13-bit fields have a maximum representable value of 0x1fff (< 2.0). For
> an exact 2.0x downscale, panel_fitter_scaling() computes 0x2000 (BIT(13)),
> which overflows the 13-bit register field and causes a hardware state readout
> mismatch in i965_pfit_get_config() -> i9xx_compute_pfit_dst() on
> crtc_state->pfit.dst and pixel_rate:
> 
> drivers/gpu/drm/i915/display/intel_pfit.c:i965_pfit_get_config() {
> 	...
> 	case PFIT_SCALING_AUTO:
> 	case PFIT_SCALING_PROGRAMMED:
> 		hscale = REG_FIELD_GET(PFIT_HORIZ_SCALE_MASK_965, ratios);
> 		vscale = REG_FIELD_GET(PFIT_VERT_SCALE_MASK_965, ratios);
> 		break;
> 	}
> 
> 	i9xx_compute_pfit_dst(crtc_state, hscale, vscale);
> 	...
> }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261019135309.23578-1-ville.syrjala@linux.intel.com?part=15

-- 
Ville Syrjälä
Intel