Re: [PATCH 5/8] drm/i915/gen9: minimum scanlines for Y tile is not always 4

["Zanoni, Paulo R" <paulo.r.zanoni@intel.com>]

Em Qua, 2016-09-07 às 19:03 -0400, Lyude escreveu:
> On Tue, 2016-09-06 at 21:52 -0300, Paulo Zanoni wrote:
> > 
> > During watermarks calculations, this value is used in 3 different
> > places. Only one of them was not using a hardcoded 4. Move the code
> > up
> > so everybody can benefit from the actual value.
> > 
> > This should only help on situations with Y tiling + 90/270 rotation
> > +
> > 1 or 2 bpp or NV12.
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 56 +++++++++++++++++++++++------
> > ------------
> >  1 file changed, 32 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index f8ac928..5a23a91 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3499,7 +3499,8 @@ static uint32_t skl_wm_method1(uint32_t
> > pixel_rate, uint8_t cpp, uint32_t latenc
> >  
> >  static uint32_t skl_wm_method2(uint32_t pixel_rate, uint32_t
> > pipe_htotal,
> >  			       uint32_t horiz_pixels, uint8_t cpp,
> > -			       uint64_t tiling, uint32_t latency)
> > +			       uint64_t tiling, uint32_t latency,
> > +			       uint32_t y_min_scanlines)
> >  {
> >  	uint32_t ret;
> >  	uint32_t plane_bytes_per_line, plane_blocks_per_line;
> > @@ -3512,9 +3513,9 @@ static uint32_t skl_wm_method2(uint32_t
> > pixel_rate, uint32_t pipe_htotal,
> >  
> >  	if (tiling == I915_FORMAT_MOD_Y_TILED ||
> >  	    tiling == I915_FORMAT_MOD_Yf_TILED) {
> > -		plane_bytes_per_line *= 4;
> > +		plane_bytes_per_line *= y_min_scanlines;
> >  		plane_blocks_per_line =
> > DIV_ROUND_UP(plane_bytes_per_line, 512);
> > -		plane_blocks_per_line /= 4;
> > +		plane_blocks_per_line /= y_min_scanlines;
> >  	} else if (tiling == DRM_FORMAT_MOD_NONE) {
> >  		plane_blocks_per_line =
> > DIV_ROUND_UP(plane_bytes_per_line, 512) + 1;
> >  	} else {
> > @@ -3571,6 +3572,7 @@ static int skl_compute_plane_wm(const struct
> > drm_i915_private *dev_priv,
> >  	uint8_t cpp;
> >  	uint32_t width = 0, height = 0;
> >  	uint32_t plane_pixel_rate;
> > +	uint32_t y_min_scanlines;
> >  
> >  	if (latency == 0 || !cstate->base.active || !intel_pstate-
> > > 
> > > base.visible) {
> >  		*enabled = false;
> > @@ -3586,38 +3588,44 @@ static int skl_compute_plane_wm(const
> > struct
> > drm_i915_private *dev_priv,
> >  	cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> >  	plane_pixel_rate = skl_adjusted_plane_pixel_rate(cstate,
> > intel_pstate);
> >  
> > +	if (intel_rotation_90_or_270(pstate->rotation)) {
> > +		int cpp = (fb->pixel_format == DRM_FORMAT_NV12) ?
> > +			drm_format_plane_cpp(fb->pixel_format, 1)
> > :
> > +			drm_format_plane_cpp(fb->pixel_format, 0);
> > +
> > +		switch (cpp) {
> > +		case 1:
> > +			y_min_scanlines = 16;
> > +			break;
> > +		case 2:
> > +			y_min_scanlines = 8;
> > +			break;
> > +		default:
> > +			WARN(1, "Unsupported pixel depth for
> > rotation");
> 
> This looks like it's leftover from the code that you moved around,
> but
> we should be erroring out here and returning -EINVAL here so the
> modeset actually fails.

Good catch. I'll do this as a separate patch since it's a separate fix.

> 
> > 
> > +		case 4:
> > +			y_min_scanlines = 4;
> > +			break;
> > +		}
> > +	} else {
> > +		y_min_scanlines = 4;
> > +	}
> > +
> >  	method1 = skl_wm_method1(plane_pixel_rate, cpp, latency);
> >  	method2 = skl_wm_method2(plane_pixel_rate,
> >  				 cstate-
> > > 
> > > base.adjusted_mode.crtc_htotal,
> >  				 width,
> >  				 cpp,
> >  				 fb->modifier[0],
> > -				 latency);
> > +				 latency,
> > +				 y_min_scanlines);
> >  
> >  	plane_bytes_per_line = width * cpp;
> >  	plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line,
> > 512);
> >  
> >  	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
> >  	    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
> > -		uint32_t min_scanlines = 4;
> > -		uint32_t y_tile_minimum;
> > -		if (intel_rotation_90_or_270(pstate->rotation)) {
> > -			int cpp = (fb->pixel_format ==
> > DRM_FORMAT_NV12) ?
> > -				drm_format_plane_cpp(fb-
> > > 
> > > pixel_format, 1) :
> > -				drm_format_plane_cpp(fb-
> > > 
> > > pixel_format, 0);
> > -
> > -			switch (cpp) {
> > -			case 1:
> > -				min_scanlines = 16;
> > -				break;
> > -			case 2:
> > -				min_scanlines = 8;
> > -				break;
> > -			case 8:
> > -				WARN(1, "Unsupported pixel depth
> > for
> > rotation");
> > -			}
> > -		}
> > -		y_tile_minimum = plane_blocks_per_line *
> > min_scanlines;
> > +		uint32_t y_tile_minimum = plane_blocks_per_line *
> > +					  y_min_scanlines;
> >  		selected_result = max(method2, y_tile_minimum);
> >  	} else {
> >  		if ((ddb_allocation / plane_blocks_per_line) >= 1)
> > @@ -3632,7 +3640,7 @@ static int skl_compute_plane_wm(const struct
> > drm_i915_private *dev_priv,
> >  	if (level >= 1 && level <= 7) {
> >  		if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
> >  		    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED)
> > -			res_lines += 4;
> > +			res_lines += y_min_scanlines;
> >  		else
> >  			res_blocks++;
> >  	}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx