[Intel-gfx] [PATCH 1/2] drm/i915: Extract intel_adjusted_rate()

[Ville Syrjälä]

On Thu, Apr 01, 2021 at 05:32:20PM +0300, Ville Syrjälä wrote:
> On Thu, Apr 01, 2021 at 03:43:37PM +0300, Jani Nikula wrote:
> > On Tue, 30 Mar 2021, Ville Syrjala <ville.syrjala at linux.intel.com> wrote:
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > >
> > > Extract a small helper to calculate the downscaling
> > > adjusted pixel rate/data rate/etc.
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > ---
> > >  .../gpu/drm/i915/display/intel_atomic_plane.c | 27 +++++++++++++------
> > >  1 file changed, 19 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > index c3f2962aa1eb..3f830b70b0c1 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > @@ -133,25 +133,36 @@ intel_plane_destroy_state(struct drm_plane *plane,
> > >  	kfree(plane_state);
> > >  }
> > >  
> > > -unsigned int intel_plane_pixel_rate(const struct intel_crtc_state *crtc_state,
> > > -				    const struct intel_plane_state *plane_state)
> > > +static unsigned int intel_adjusted_rate(const struct drm_rect *src,
> > > +					const struct drm_rect *dst,
> > > +					unsigned int rate)
> > >  {
> > >  	unsigned int src_w, src_h, dst_w, dst_h;
> > > -	unsigned int pixel_rate = crtc_state->pixel_rate;
> > >  
> > > -	src_w = drm_rect_width(&plane_state->uapi.src) >> 16;
> > > -	src_h = drm_rect_height(&plane_state->uapi.src) >> 16;
> > > -	dst_w = drm_rect_width(&plane_state->uapi.dst);
> > > -	dst_h = drm_rect_height(&plane_state->uapi.dst);
> > > +	src_w = drm_rect_width(src) >> 16;
> > > +	src_h = drm_rect_height(src) >> 16;
> > > +	dst_w = drm_rect_width(dst);
> > > +	dst_h = drm_rect_height(dst);
> > >  
> > >  	/* Downscaling limits the maximum pixel rate */
> > >  	dst_w = min(src_w, dst_w);
> > >  	dst_h = min(src_h, dst_h);
> > >  
> > > -	return DIV_ROUND_UP_ULL(mul_u32_u32(pixel_rate, src_w * src_h),
> > > +	return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h),
> > >  				dst_w * dst_h);
> > >  }
> > >  
> > > +unsigned int intel_plane_pixel_rate(const struct intel_crtc_state *crtc_state,
> > > +				    const struct intel_plane_state *plane_state)
> > > +{
> > > +	if (!plane_state->uapi.visible)
> > 
> > Potential functional change not covered in the commit message? Makes
> > sense, but the rabbit hole is too deep to find out if this could
> > actually make a difference.
> 
> This is fine. If the plane isn't visible then it's not
> generating any pixels anyway. I think I either had some other
> patches originally that wanted this, or I just wanted to make
> this safe to call at any point without checking for plane
> visibility in the caller. But IIRC I dropped those other
> patches and so this might not be necessary anymore. I'll double
> check and either drop this or amend the commit msg a bit.

Actually the one case where this might matter a bit is a
fully offscreen cursor plane. The watermark claculations
consider the cursor visible in that case (to avoid redundant
watermark updates when the cursor pops in and out of visibility
due to going offscreen). So in fact we want it to have a
non-zero pixel rate or else the watermarks will just come out
as zero.
 
Fortunately intel_check_cursor() does populate the plane 
src/dst rectangles correctly even in that case. Normally
!visible implies that the src/dst rectangles don't
contain data we can use.

So I'll drop the check and toss in a comment somewhere to
remind us of the facts for the next time...

> 
> > 
> > If mentioned in the commit message,
> > 
> > Reviewed-by: Jani Nikula <jani.nikula at intel.com>
> > 
> > 
> > > +		return 0;
> > > +
> > > +	return intel_adjusted_rate(&plane_state->uapi.src,
> > > +				   &plane_state->uapi.dst,
> > > +				   crtc_state->pixel_rate);
> > > +}
> > > +
> > >  unsigned int intel_plane_data_rate(const struct intel_crtc_state *crtc_state,
> > >  				   const struct intel_plane_state *plane_state)
> > >  {
> > 
> > -- 
> > Jani Nikula, Intel Open Source Graphics Center
> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel