[Intel-gfx] [PATCH 02/15] drm/i915/skl+: refactore WM calculation for NV12

Maarten Lankhorst maarten.lankhorst at linux.intel.com
Thu Jan 4 16:19:33 UTC 2018


Op 07-01-18 om 10:59 schreef Vidya Srinivas:
> From: Mahesh Kumar <mahesh1.kumar at intel.com>
>
> Current code calculates DDB for planar formats in such a way that we
> store DDB of plane-0 in plane 1 & vice-versa.
> In order to make this clean this patch refactors WM/DDB calculation for
> NV12 planar formats.
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |   2 +-
>  drivers/gpu/drm/i915/intel_drv.h |   1 +
>  drivers/gpu/drm/i915/intel_pm.c  | 120 +++++++++++++++++++--------------------
>  3 files changed, 62 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 56047f8..962717d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1434,7 +1434,7 @@ static inline bool skl_ddb_entry_equal(const struct skl_ddb_entry *e1,
>  
>  struct skl_ddb_allocation {
>  	struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES]; /* packed/uv */
^Should this be changed to packed/y?
> -	struct skl_ddb_entry y_plane[I915_MAX_PIPES][I915_MAX_PLANES];
> +	struct skl_ddb_entry uv_plane[I915_MAX_PIPES][I915_MAX_PLANES];
>  };
>  
>  struct skl_ddb_values {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9c8aef9..5f5e070 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -513,6 +513,7 @@ struct intel_pipe_wm {
>  struct skl_plane_wm {
>  	struct skl_wm_level wm[8];
>  	struct skl_wm_level trans_wm;
> +	bool is_nv12;
>  };
>  
>  struct skl_pipe_wm {
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e2598cf..15edb9a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4004,9 +4004,9 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
>  static unsigned int
>  skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
>  			     const struct drm_plane_state *pstate,
> -			     int y)
> +			     const int plane)
>  {
> -	struct intel_plane *plane = to_intel_plane(pstate->plane);
> +	struct intel_plane *intel_plane = to_intel_plane(pstate->plane);
>  	struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
>  	uint32_t data_rate;
>  	uint32_t width = 0, height = 0;
> @@ -4020,9 +4020,9 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
>  	fb = pstate->fb;
>  	format = fb->format->format;
>  
> -	if (plane->id == PLANE_CURSOR)
> +	if (intel_plane->id == PLANE_CURSOR)
>  		return 0;
> -	if (y && format != DRM_FORMAT_NV12)
> +	if (plane == 1 && format != DRM_FORMAT_NV12)
>  		return 0;
>  
>  	/*
> @@ -4033,19 +4033,14 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
>  	width = drm_rect_width(&intel_pstate->base.src) >> 16;
>  	height = drm_rect_height(&intel_pstate->base.src) >> 16;
>  
> -	/* for planar format */
> -	if (format == DRM_FORMAT_NV12) {
> -		if (y)  /* y-plane data rate */
> -			data_rate = width * height *
> -				fb->format->cpp[0];
> -		else    /* uv-plane data rate */
> -			data_rate = (width / 2) * (height / 2) *
> -				fb->format->cpp[1];
> -	} else {
> -		/* for packed formats */
> -		data_rate = width * height * fb->format->cpp[0];
> +	/* UV plane does 1/2 pixel sub-sampling */
> +	if (plane == 1 && format == DRM_FORMAT_NV12) {
> +		width /= 2;
> +		height /= 2;
>  	}
>  
> +	data_rate = width * height * fb->format->cpp[plane];
> +
>  	down_scale_amount = skl_plane_downscale_amount(cstate, intel_pstate);
>  
>  	return mul_round_up_u32_fixed16(data_rate, down_scale_amount);
> @@ -4058,8 +4053,8 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
>   */
>  static unsigned int
>  skl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate,
> -				 unsigned *plane_data_rate,
> -				 unsigned *plane_y_data_rate)
> +				 unsigned int *plane_data_rate,
> +				 unsigned int *uv_plane_data_rate)
>  {
>  	struct drm_crtc_state *cstate = &intel_cstate->base;
>  	struct drm_atomic_state *state = cstate->state;
> @@ -4075,17 +4070,16 @@ skl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate,
>  		enum plane_id plane_id = to_intel_plane(plane)->id;
>  		unsigned int rate;
>  
> -		/* packed/uv */
packed/y?
>  		rate = skl_plane_relative_data_rate(intel_cstate,
>  						    pstate, 0);
>  		plane_data_rate[plane_id] = rate;
>  
>  		total_data_rate += rate;
>  
> -		/* y-plane */
> +		/* uv-plane */
>  		rate = skl_plane_relative_data_rate(intel_cstate,
>  						    pstate, 1);
> -		plane_y_data_rate[plane_id] = rate;
> +		uv_plane_data_rate[plane_id] = rate;
>  
>  		total_data_rate += rate;
>  	}
> @@ -4094,8 +4088,7 @@ skl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate,
>  }
>  
>  static uint16_t
> -skl_ddb_min_alloc(const struct drm_plane_state *pstate,
> -		  const int y)
> +skl_ddb_min_alloc(const struct drm_plane_state *pstate, const int plane)
>  {
>  	struct drm_framebuffer *fb = pstate->fb;
>  	struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
> @@ -4106,8 +4099,8 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
>  	if (WARN_ON(!fb))
>  		return 0;
>  
> -	/* For packed formats, no y-plane, return 0 */
> -	if (y && fb->format->format != DRM_FORMAT_NV12)
> +	/* For packed formats, and uv-plane, return 0 */
> +	if (plane == 1 && fb->format->format != DRM_FORMAT_NV12)
>  		return 0;
>  
>  	/* For Non Y-tile return 8-blocks */
> @@ -4126,15 +4119,12 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
>  	src_h = drm_rect_height(&intel_pstate->base.src) >> 16;
>  
>  	/* Halve UV plane width and height for NV12 */
> -	if (fb->format->format == DRM_FORMAT_NV12 && !y) {
> +	if (plane == 1) {
>  		src_w /= 2;
>  		src_h /= 2;
>  	}
>  
> -	if (fb->format->format == DRM_FORMAT_NV12 && !y)
> -		plane_bpp = fb->format->cpp[1];
> -	else
> -		plane_bpp = fb->format->cpp[0];
> +	plane_bpp = fb->format->cpp[plane];
>  
>  	if (drm_rotation_90_or_270(pstate->rotation)) {
>  		switch (plane_bpp) {
> @@ -4162,7 +4152,7 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
>  
>  static void
>  skl_ddb_calc_min(const struct intel_crtc_state *cstate, int num_active,
> -		 uint16_t *minimum, uint16_t *y_minimum)
> +		 uint16_t *minimum, uint16_t *uv_minimum)
>  {
>  	const struct drm_plane_state *pstate;
>  	struct drm_plane *plane;
> @@ -4177,7 +4167,7 @@ skl_ddb_calc_min(const struct intel_crtc_state *cstate, int num_active,
>  			continue;
>  
>  		minimum[plane_id] = skl_ddb_min_alloc(pstate, 0);
> -		y_minimum[plane_id] = skl_ddb_min_alloc(pstate, 1);
> +		uv_minimum[plane_id] = skl_ddb_min_alloc(pstate, 1);
>  	}
>  
>  	minimum[PLANE_CURSOR] = skl_cursor_allocation(num_active);
> @@ -4195,17 +4185,17 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  	struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb;
>  	uint16_t alloc_size, start;
>  	uint16_t minimum[I915_MAX_PLANES] = {};
> -	uint16_t y_minimum[I915_MAX_PLANES] = {};
> +	uint16_t uv_minimum[I915_MAX_PLANES] = {};
>  	unsigned int total_data_rate;
>  	enum plane_id plane_id;
>  	int num_active;
> -	unsigned plane_data_rate[I915_MAX_PLANES] = {};
> -	unsigned plane_y_data_rate[I915_MAX_PLANES] = {};
> +	unsigned int plane_data_rate[I915_MAX_PLANES] = {};
> +	unsigned int uv_plane_data_rate[I915_MAX_PLANES] = {};
>  	uint16_t total_min_blocks = 0;
>  
>  	/* Clear the partitioning for disabled planes. */
>  	memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
> -	memset(ddb->y_plane[pipe], 0, sizeof(ddb->y_plane[pipe]));
> +	memset(ddb->uv_plane[pipe], 0, sizeof(ddb->uv_plane[pipe]));
>  
>  	if (WARN_ON(!state))
>  		return 0;
> @@ -4220,7 +4210,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  	if (alloc_size == 0)
>  		return 0;
>  
> -	skl_ddb_calc_min(cstate, num_active, minimum, y_minimum);
> +	skl_ddb_calc_min(cstate, num_active, minimum, uv_minimum);
>  
>  	/*
>  	 * 1. Allocate the mininum required blocks for each active plane
> @@ -4230,7 +4220,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  
>  	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
>  		total_min_blocks += minimum[plane_id];
> -		total_min_blocks += y_minimum[plane_id];
> +		total_min_blocks += uv_minimum[plane_id];
>  	}
>  
>  	if (total_min_blocks > alloc_size) {
> @@ -4252,14 +4242,14 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  	 */
>  	total_data_rate = skl_get_total_relative_data_rate(cstate,
>  							   plane_data_rate,
> -							   plane_y_data_rate);
> +							   uv_plane_data_rate);
>  	if (total_data_rate == 0)
>  		return 0;
>  
>  	start = alloc->start;
>  	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
> -		unsigned int data_rate, y_data_rate;
> -		uint16_t plane_blocks, y_plane_blocks = 0;
> +		unsigned int data_rate, uv_data_rate;
> +		uint16_t plane_blocks, uv_plane_blocks;
>  
>  		if (plane_id == PLANE_CURSOR)
>  			continue;
> @@ -4283,21 +4273,20 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  
>  		start += plane_blocks;
>  
> -		/*
> -		 * allocation for y_plane part of planar format:
> -		 */
> -		y_data_rate = plane_y_data_rate[plane_id];
> +		/* Allocate DDB for UV plane for planar format/NV12 */
> +		uv_data_rate = uv_plane_data_rate[plane_id];
>  
> -		y_plane_blocks = y_minimum[plane_id];
> -		y_plane_blocks += div_u64((uint64_t)alloc_size * y_data_rate,
> +		uv_plane_blocks = uv_minimum[plane_id];
> +		uv_plane_blocks += div_u64((uint64_t)alloc_size * uv_data_rate,
>  					total_data_rate);
>  
> -		if (y_data_rate) {
> -			ddb->y_plane[pipe][plane_id].start = start;
> -			ddb->y_plane[pipe][plane_id].end = start + y_plane_blocks;
> +		if (uv_data_rate) {
> +			ddb->uv_plane[pipe][plane_id].start = start;
> +			ddb->uv_plane[pipe][plane_id].end = start +
> +								uv_plane_blocks;
>  		}
>  
> -		start += y_plane_blocks;
> +		start += uv_plane_blocks;
>  	}
>  
>  	return 0;
> @@ -4425,8 +4414,7 @@ skl_compute_plane_wm_params(const struct drm_i915_private *dev_priv,
>  		wp->width = drm_rect_width(&intel_pstate->base.src) >> 16;
>  	}
>  
> -	wp->cpp = (fb->format->format == DRM_FORMAT_NV12) ? fb->format->cpp[1] :
> -							    fb->format->cpp[0];
> +	wp->cpp = fb->format->cpp[0];
>  	wp->plane_pixel_rate = skl_adjusted_plane_pixel_rate(cstate,
>  							     intel_pstate);
>  
> @@ -4618,6 +4606,9 @@ skl_compute_wm_levels(const struct drm_i915_private *dev_priv,
>  			return ret;
>  	}
>  
> +	if (intel_pstate->base.fb->format->format == DRM_FORMAT_NV12)
> +		wm->is_nv12 = true;
> +
>  	return 0;
>  }
>  
> @@ -4788,10 +4779,19 @@ static void skl_write_plane_wm(struct intel_crtc *intel_crtc,
>  	skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id),
>  			   &wm->trans_wm);
>  
> -	skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id),
> -			    &ddb->plane[pipe][plane_id]);
> -	skl_ddb_entry_write(dev_priv, PLANE_NV12_BUF_CFG(pipe, plane_id),
> -			    &ddb->y_plane[pipe][plane_id]);
> +	if (wm->is_nv12) {
> +		skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id),
> +				&ddb->uv_plane[pipe][plane_id]);
> +		skl_ddb_entry_write(dev_priv,
> +				    PLANE_NV12_BUF_CFG(pipe, plane_id),
> +				    &ddb->plane[pipe][plane_id]);
> +	} else {
> +		skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id),
> +				&ddb->plane[pipe][plane_id]);
> +		skl_ddb_entry_write(dev_priv,
> +				    PLANE_NV12_BUF_CFG(pipe, plane_id),
> +				    &ddb->uv_plane[pipe][plane_id]);

It's probably better to always write 0 here to avoid confusion. :)
Writing something else here is probably a bad idea..


> +	}
>  }
>  
>  static void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
> @@ -4906,8 +4906,8 @@ skl_ddb_add_affected_planes(struct intel_crtc_state *cstate)
>  
>  		if (skl_ddb_entry_equal(&cur_ddb->plane[pipe][plane_id],
>  					&new_ddb->plane[pipe][plane_id]) &&
> -		    skl_ddb_entry_equal(&cur_ddb->y_plane[pipe][plane_id],
> -					&new_ddb->y_plane[pipe][plane_id]))
> +		    skl_ddb_entry_equal(&cur_ddb->uv_plane[pipe][plane_id],
> +					&new_ddb->uv_plane[pipe][plane_id]))
>  			continue;
>  
>  		plane_state = drm_atomic_get_plane_state(state, plane);
> @@ -5001,8 +5001,8 @@ skl_copy_wm_for_pipe(struct skl_ddb_values *dst,
>  		     struct skl_ddb_values *src,
>  		     enum pipe pipe)
>  {
> -	memcpy(dst->ddb.y_plane[pipe], src->ddb.y_plane[pipe],
> -	       sizeof(dst->ddb.y_plane[pipe]));
> +	memcpy(dst->ddb.uv_plane[pipe], src->ddb.uv_plane[pipe],
> +	       sizeof(dst->ddb.uv_plane[pipe]));
>  	memcpy(dst->ddb.plane[pipe], src->ddb.plane[pipe],
>  	       sizeof(dst->ddb.plane[pipe]));
>  }

With that fixed for this and patch 1:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>



More information about the Intel-gfx mailing list