[Intel-gfx] [PATCH 01/15] drm/i915: Allocate min dbuf blocks per bspec

[Ville Syrjälä]

On Wed, Aug 19, 2015 at 06:02:22PM -0700, Chandra Konduru wrote:
> Properly allocate min blocks per hw requirements.
> 
> Signed-off-by: Chandra Konduru <chandra.konduru at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c |   39 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index fff0c22..da3046f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2959,6 +2959,41 @@ skl_get_total_relative_data_rate(struct intel_crtc *intel_crtc,
>  	return total_data_rate;
>  }
>  
> +static uint16_t
> +skl_dbuf_min_alloc(const struct intel_plane_wm_parameters *p, int y_plane)

bool y_plane

In general I dislike the fact that the code treats Y as the special case
instead of CbCr. The oppostire would be far more natural IMO, and,
I believe, would result in less checks in the code all around. Althouh
I think in general we should just pass around the format and plane index.

> +{
> +	uint16_t min_alloc;
> +
> +	/* For packed formats, no y-plane, return 0 */
> +	if (y_plane && !p->y_bytes_per_pixel)
> +		return 0;
> +
> +

Extra line.

> +	if (p->tiling == I915_FORMAT_MOD_Y_TILED ||
> +	    p->tiling == I915_FORMAT_MOD_Yf_TILED) {
> +		uint32_t min_scanlines = 8;
> +		uint8_t bytes_per_pixel =
> +			y_plane ? p->y_bytes_per_pixel : p->bytes_per_pixel;
> +
> +		switch (bytes_per_pixel) {
> +		case 1:
> +			min_scanlines = 32;
> +			break;
> +		case 2:
> +			min_scanlines = 16;
> +			break;
> +		case 8:
> +			WARN(1, "Unsupported pixel depth for rotation");
> +		}

Could be just 32/cpp.

> +		min_alloc = DIV_ROUND_UP((4 * p->horiz_pixels/(y_plane ? 1 : 2) *
> +			bytes_per_pixel), 512) * min_scanlines/4 + 3;

Another case that could be simplified by removing the y_plane special
casing. In fact just passing in the format and plane index in we could get:

cpp = drm_format_plane_cpp(format, plane);
width = width / drm_format_horiz_subsampling(format);
min_scanlines = 32 / cpp;
min_alloc = DIV_ROUND_UP(4 * width * cpp, 512) * min_scanlines / 4 + 3;


We could even move the width/subsampling (and height too) thing into a
common helper in a drm header, eg.:

static inline int drm_format_plane_width(format, plane, width)
{
	if (plane)
		return width / drm_format_plane_horiz_subsampling(format);
	else
		return width;
}
static inline int drm_format_plane_height(format, plane, height);
{
	if (plane)
		return height / drm_format_plane_vert_subsampling(format);
	else
		return height;
}

> +	} else {
> +		min_alloc = 8;
> +	}
> +
> +	return min_alloc;
> +}
> +
>  static void
>  skl_allocate_pipe_ddb(struct drm_crtc *crtc,
>  		      const struct intel_wm_config *config,
> @@ -2999,9 +3034,9 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
>  		if (!p->enabled)
>  			continue;
>  
> -		minimum[plane] = 8;
> +		minimum[plane] = skl_dbuf_min_alloc(p, 0);    /* uv-plane/packed */
>  		alloc_size -= minimum[plane];
> -		y_minimum[plane] = p->y_bytes_per_pixel ? 8 : 0;
> +		y_minimum[plane] = skl_dbuf_min_alloc(p, 1);  /* y-plane */

false/true instead of 0/1.

With the bool things changed this does what it says so:
Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

>  		alloc_size -= y_minimum[plane];
>  	}
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC