Re: [PATCH 10/12] drm/i915: Compute display surface offset in the plane check hook for SKL+

[Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>]

Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>

On Tuesday 03 May 2016 09:09 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> SKL has nasty limitations with the display surface offsets:
> * source x offset + width must be less than the stride for X tiled
>    surfaces or the display engine falls over
> * the surface offset requires lots of alignment (256K or 1M)
>
> These facts mean that we can't just pick any suitably aligned tile
> boundary as the offset and expect the resulting x offset to be useable.
> The solution is to start with the closest boundary as before, but then
> keep searhing backwards until we find one that works, or don't. This
> means we must be prepared to fail, hence the whole surface offset
> calculation needs to be moved to the .check_plane() hook from the
> .update_plane() hook.
>
> While at it we can check that the source width/height don't exceed
> maximum plane size limits.
>
> We'll store the results of the computation in the plane state to make
> it easy for the .update_plane() hook to do its thing.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 164 +++++++++++++++++++++++++++++------
>   drivers/gpu/drm/i915/intel_drv.h     |   5 ++
>   drivers/gpu/drm/i915/intel_sprite.c  |  33 +++----
>   3 files changed, 151 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ad7c48757ba6..b3d7d312e57c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2873,6 +2873,120 @@ valid_fb:
>   	obj->frontbuffer_bits |= to_intel_plane(primary)->frontbuffer_bit;
>   }
>   
> +static int skl_max_plane_width(const struct drm_framebuffer *fb, int plane,
> +			       unsigned int rotation)
> +{
> +	int cpp = drm_format_plane_cpp(fb->pixel_format, plane);
> +
> +	switch (fb->modifier[plane]) {
> +	case DRM_FORMAT_MOD_NONE:
> +	case I915_FORMAT_MOD_X_TILED:
> +		switch (cpp) {
> +		case 8:
> +			return 4096;
> +		case 4:
> +		case 2:
> +		case 1:
> +			return 8192;
> +		default:
> +			MISSING_CASE(cpp);
> +			break;
> +		}
> +		break;
> +	case I915_FORMAT_MOD_Y_TILED:
> +	case I915_FORMAT_MOD_Yf_TILED:
> +		switch (cpp) {
> +		case 8:
> +			return 2048;
> +		case 4:
> +			return 4096;
> +		case 2:
> +		case 1:
> +			return 8192;
> +		default:
> +			MISSING_CASE(cpp);
> +			break;
> +		}
> +		break;
> +	default:
> +		MISSING_CASE(fb->modifier[plane]);
> +	}
> +
> +	return 2048;
> +}
> +
> +static int skl_check_main_surface(struct intel_plane_state *plane_state)
> +{
> +	const struct drm_i915_private *dev_priv = to_i915(plane_state->base.plane->dev);
> +	const struct drm_framebuffer *fb = plane_state->base.fb;
> +	unsigned int rotation = plane_state->base.rotation;
> +	int x = plane_state->src.x1 >> 16;
> +	int y = plane_state->src.y1 >> 16;
> +	int w = drm_rect_width(&plane_state->src) >> 16;
> +	int h = drm_rect_height(&plane_state->src) >> 16;
> +	int max_width = skl_max_plane_width(fb, 0, rotation);
> +	int max_height = 4096;
> +	u32 alignment, offset;
> +
> +	if (w > max_width || h > max_height) {
> +		DRM_DEBUG_KMS("requested Y/RGB source size %dx%d too big (limit %dx%d)\n",
> +			      w, h, max_width, max_height);
> +		return -EINVAL;
> +	}
> +
> +	intel_add_fb_offsets(&x, &y, plane_state, 0);
> +	offset = intel_compute_tile_offset(&x, &y, plane_state, 0);
> +
> +	alignment = intel_surf_alignment(dev_priv, fb->modifier[0]);
> +
> +	/*
> +	 * When using an X-tiled surface, the plane blows up
> +	 * if the x offset + width exceed the stride.
> +	 *
> +	 * TODO: linear and Y-tiled seem fine, Yf untested,
> +	 */
> +	if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED) {
> +		int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> +
> +		for (;;) {
> +			if ((x + w) * cpp <= fb->pitches[0])
> +				break;
> +
> +			if (offset == 0) {
> +				DRM_DEBUG_KMS("Unable to find suitable display surface offset\n");
> +				return -EINVAL;
> +			}
> +
> +			offset = intel_adjust_tile_offset(&x, &y, plane_state, 0,
> +							  offset, offset - alignment);
> +		}
> +	}
> +
> +	plane_state->main.offset = offset;
> +	plane_state->main.x = x;
> +	plane_state->main.y = y;
> +
> +	return 0;
> +}
> +
> +int skl_check_plane_surface(struct intel_plane_state *plane_state)
> +{
> +	const struct drm_framebuffer *fb = plane_state->base.fb;
> +	unsigned int rotation = plane_state->base.rotation;
> +	int ret;
> +
> +	/* Rotate src coordinates to match rotated GTT view */
> +	if (intel_rotation_90_or_270(rotation))
> +		drm_rect_rotate(&plane_state->src,
> +				fb->width, fb->height, BIT(DRM_ROTATE_270));
> +
> +	ret = skl_check_main_surface(plane_state);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
>   static void i9xx_update_primary_plane(struct drm_plane *primary,
>   				      const struct intel_crtc_state *crtc_state,
>   				      const struct intel_plane_state *plane_state)
> @@ -3252,10 +3366,10 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
>   	u32 plane_ctl;
>   	unsigned int rotation = plane_state->base.rotation;
>   	u32 stride = skl_plane_stride(fb, 0, rotation);
> -	u32 surf_addr;
> +	u32 surf_addr = plane_state->main.offset;
>   	int scaler_id = plane_state->scaler_id;
> -	int src_x = plane_state->src.x1 >> 16;
> -	int src_y = plane_state->src.y1 >> 16;
> +	int src_x = plane_state->main.x;
> +	int src_y = plane_state->main.y;
>   	int src_w = drm_rect_width(&plane_state->src) >> 16;
>   	int src_h = drm_rect_height(&plane_state->src) >> 16;
>   	int dst_x = plane_state->dst.x1;
> @@ -3272,26 +3386,6 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
>   	plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
>   	plane_ctl |= skl_plane_ctl_rotation(rotation);
>   
> -	if (intel_rotation_90_or_270(rotation)) {
> -		struct drm_rect r = {
> -			.x1 = src_x,
> -			.x2 = src_x + src_w,
> -			.y1 = src_y,
> -			.y2 = src_y + src_h,
> -		};
> -
> -		/* Rotate src coordinates to match rotated GTT view */
> -		drm_rect_rotate(&r, fb->width, fb->height, BIT(DRM_ROTATE_270));
> -
> -		src_x = r.x1;
> -		src_y = r.y1;
> -		src_w = drm_rect_width(&r);
> -		src_h = drm_rect_height(&r);
> -	}
> -
> -	intel_add_fb_offsets(&src_x, &src_y, plane_state, 0);
> -	surf_addr = intel_compute_tile_offset(&src_x, &src_y, plane_state, 0);
> -
>   	/* Sizes are 0 based */
>   	src_w--;
>   	src_h--;
> @@ -14170,6 +14264,7 @@ intel_check_primary_plane(struct drm_plane *plane,
>   	int min_scale = DRM_PLANE_HELPER_NO_SCALING;
>   	int max_scale = DRM_PLANE_HELPER_NO_SCALING;
>   	bool can_position = false;
> +	int ret;
>   
>   	if (INTEL_INFO(plane->dev)->gen >= 9) {
>   		/* use scaler when colorkey is not required */
> @@ -14180,11 +14275,24 @@ intel_check_primary_plane(struct drm_plane *plane,
>   		can_position = true;
>   	}
>   
> -	return drm_plane_helper_check_update(plane, crtc, fb, &state->src,
> -					     &state->dst, &state->clip,
> -					     min_scale, max_scale,
> -					     can_position, true,
> -					     &state->visible);
> +	ret = drm_plane_helper_check_update(plane, crtc, fb, &state->src,
> +					    &state->dst, &state->clip,
> +					    min_scale, max_scale,
> +					    can_position, true,
> +					    &state->visible);
> +	if (ret)
> +		return ret;
> +
> +	if (!fb)
> +		return 0;
> +
> +	if (INTEL_INFO(plane->dev)->gen >= 9) {
> +		ret = skl_check_plane_surface(state);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
>   }
>   
>   static void intel_begin_crtc_commit(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 75e4d59f1ab3..fd4571d01304 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -318,6 +318,10 @@ struct intel_plane_state {
>   	struct drm_rect src;
>   	struct drm_rect dst;
>   	struct drm_rect clip;
> +	struct {
> +		u32 offset;
> +		int x, y;
> +	} main;
>   	bool visible;
>   
>   	/*
> @@ -1288,6 +1292,7 @@ u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
>   u32 skl_plane_ctl_rotation(unsigned int rotation);
>   u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
>   		     unsigned int rotation);
> +int skl_check_plane_surface(struct intel_plane_state *plane_state);
>   
>   /* intel_csr.c */
>   void intel_csr_ucode_init(struct drm_i915_private *);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index ed46a84d97f7..02e840134ada 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -190,15 +190,15 @@ skl_update_plane(struct drm_plane *drm_plane,
>   	const int plane = intel_plane->plane + 1;
>   	u32 plane_ctl;
>   	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> -	u32 surf_addr;
> +	u32 surf_addr = plane_state->main.offset;
>   	unsigned int rotation = plane_state->base.rotation;
>   	u32 stride = skl_plane_stride(fb, 0, rotation);
>   	int crtc_x = plane_state->dst.x1;
>   	int crtc_y = plane_state->dst.y1;
>   	uint32_t crtc_w = drm_rect_width(&plane_state->dst);
>   	uint32_t crtc_h = drm_rect_height(&plane_state->dst);
> -	uint32_t x = plane_state->src.x1 >> 16;
> -	uint32_t y = plane_state->src.y1 >> 16;
> +	uint32_t x = plane_state->main.x;
> +	uint32_t y = plane_state->main.y;
>   	uint32_t src_w = drm_rect_width(&plane_state->src) >> 16;
>   	uint32_t src_h = drm_rect_height(&plane_state->src) >> 16;
>   	const struct intel_scaler *scaler =
> @@ -224,26 +224,6 @@ skl_update_plane(struct drm_plane *drm_plane,
>   	else if (key->flags & I915_SET_COLORKEY_SOURCE)
>   		plane_ctl |= PLANE_CTL_KEY_ENABLE_SOURCE;
>   
> -	if (intel_rotation_90_or_270(rotation)) {
> -		struct drm_rect r = {
> -			.x1 = x,
> -			.x2 = x + src_w,
> -			.y1 = y,
> -			.y2 = y + src_h,
> -		};
> -
> -		/* Rotate src coordinates to match rotated GTT view */
> -		drm_rect_rotate(&r, fb->width, fb->height, BIT(DRM_ROTATE_270));
> -
> -		x = r.x1;
> -		y = r.y1;
> -		src_w = drm_rect_width(&r);
> -		src_h = drm_rect_height(&r);
> -	}
> -
> -	intel_add_fb_offsets(&x, &y, plane_state, 0);
> -	surf_addr = intel_compute_tile_offset(&x, &y, plane_state, 0);
> -
>   	/* Sizes are 0 based */
>   	src_w--;
>   	src_h--;
> @@ -755,6 +735,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
>   	int hscale, vscale;
>   	int max_scale, min_scale;
>   	bool can_scale;
> +	int ret;
>   
>   	if (!fb) {
>   		state->visible = false;
> @@ -909,6 +890,12 @@ intel_check_sprite_plane(struct drm_plane *plane,
>   	dst->y1 = crtc_y;
>   	dst->y2 = crtc_y + crtc_h;
>   
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		ret = skl_check_plane_surface(state);
> +		if (ret)
> +			return ret;
> +	}
> +
>   	return 0;
>   }
>   

-- 
regards,
Sivakumar Thulasimani

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx