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

Sivakumar Thulasimani
2016-05-09T10:25:32Z

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