Re: [PATCH 1/3] drm/i915: Use the the correct pixel rate to compute wm line time

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

On Wed, Oct 01, 2025 at 10:27:44AM +0000, Govindapillai, Vinod wrote:
> On Fri, 2025-09-19 at 21:08 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The line time used for the watermark calculations is supposed to
> > based on the plane's adjusted pixel rate, not the pipe's adjusted
> > pixel rate. The current code will give incorrect answers if plane
> > downscaling is used.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/skl_watermark.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
> > b/drivers/gpu/drm/i915/display/skl_watermark.c
> > index d74cbb43ae6f..bdd005c6cc2d 100644
> > --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> > @@ -1637,18 +1637,16 @@ skl_wm_method2(u32 pixel_rate, u32 pipe_htotal, u32 latency,
> >  }
> >  
> >  static uint_fixed_16_16_t
> > -intel_get_linetime_us(const struct intel_crtc_state *crtc_state)
> > +intel_get_linetime_us(const struct intel_crtc_state *crtc_state,
> > +		      int pixel_rate)
> >  {
> >  	struct intel_display *display = to_intel_display(crtc_state);
> > -	u32 pixel_rate;
> >  	u32 crtc_htotal;
> >  	uint_fixed_16_16_t linetime_us;
> >  
> >  	if (!crtc_state->hw.active)
> >  		return u32_to_fixed16(0);
> >  
> > -	pixel_rate = crtc_state->pixel_rate;
> > -
> >  	if (drm_WARN_ON(display->drm, pixel_rate == 0))
> >  		return u32_to_fixed16(0);
> >  
> > @@ -1743,7 +1741,8 @@ skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
> >  	wp->y_tile_minimum = mul_u32_fixed16(wp->y_min_scanlines,
> >  					     wp->plane_blocks_per_line);
> >  
> > -	wp->linetime_us = fixed16_to_u32_round_up(intel_get_linetime_us(crtc_state));
> > +	wp->linetime_us = fixed16_to_u32_round_up(intel_get_linetime_us(crtc_state,
> > +									plane_pixel_rate));
> 
> Hi Ville,
> 
> As per the bspec 49325 the starting point is,
> 
> Adjusted pipe pixel rate = pixel rate for the screen resolution
> if (pipe scale enabled)
> 	adjusted pipe pixel rate = adjusted pipe pixel rate * pipe down scale amoun
> 
> adjusted plane pixel rate = adjusted pipe pixel rate
> if (plane scale enabled)
> 	adjusted plane pixel rate = adjusted plane pixel rate * plane down scale 
> 
> and
> line time microseconds = pipe horizontal total pixels/adjusted plane pixel rate MHz
> 
> Our Method1, Method2 and line time calculations are based on plane_pixel_rate vs. adjusted plane
> pixel rate in bspec. So I wonder if we differ from the bspec in these wm calculations?

plane_pixel_rate is what the spec calls "adjusted plane pixel rate"

-- 
Ville Syrjälä
Intel