Re: [PATCH v3 6/8] drm/i915: Overcome display engine stride limits via GTT remapping

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting Ville Syrjala (2018-09-25 20:37:12)
> +static bool intel_plane_needs_remap(const struct intel_plane_state *plane_state)
> +{
> +       struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
> +       struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> +       const struct drm_framebuffer *fb = plane_state->base.fb;
> +       unsigned int rotation = plane_state->base.rotation;
> +       u32 stride, max_stride;
> +
> +       /* We don't want to deal with remapping with cursors */
> +       if (plane->id == PLANE_CURSOR)
> +               return false;
> +
> +       /* No fence for the remapped vma */

Should work now (with a minor tweak to plane_can_fence), right?

> +       if (INTEL_GEN(dev_priv) < 4)
> +               return false;
> +
> +       /* New CCS hash mode makes remapping impossible */
> +       if (is_ccs_modifier(fb->modifier))
> +               return false;
> +
> +       /* FIXME other color planes? */
> +       stride = intel_fb_pitch(fb, 0, rotation);
> +       max_stride = plane->max_stride(plane, fb->format->format,
> +                                      fb->modifier, rotation);
> +
> +       return stride > max_stride;
> +}
> +
>  static int
>  intel_fill_fb_info(struct drm_i915_private *dev_priv,
>                    struct drm_framebuffer *fb)
> @@ -2676,6 +2741,182 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
>         return 0;
>  }
>  
> +static void
> +intel_plane_remap_gtt(struct intel_plane_state *plane_state)
> +{
> +       struct drm_i915_private *dev_priv =
> +               to_i915(plane_state->base.plane->dev);
> +       struct drm_framebuffer *fb = plane_state->base.fb;
> +       struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
> +       struct intel_rotation_info *info = &plane_state->view.rotated;
> +       unsigned int rotation = plane_state->base.rotation;
> +       int i, num_planes = fb->format->num_planes;
> +       unsigned int tile_size = intel_tile_size(dev_priv);
> +       unsigned int tile_width, tile_height;
> +       unsigned int aligned_x, aligned_y;
> +       unsigned int aligned_w, aligned_h;
> +       unsigned int src_x, src_y;
> +       unsigned int src_w, src_h;
> +       unsigned int x, y;
> +       u32 gtt_offset = 0;
> +
> +       memset(&plane_state->view, 0, sizeof(plane_state->view));
> +       plane_state->view.type = drm_rotation_90_or_270(rotation) ?
> +               I915_GGTT_VIEW_ROTATED : I915_GGTT_VIEW_REMAPPED;
> +
> +       src_x = plane_state->base.src.x1 >> 16;
> +       src_y = plane_state->base.src.y1 >> 16;
> +       src_w = drm_rect_width(&plane_state->base.src) >> 16;
> +       src_h = drm_rect_height(&plane_state->base.src) >> 16;
> +
> +       WARN_ON(is_ccs_modifier(fb->modifier));
> +
> +       /* Align our viewport start to tile boundary */
> +       intel_tile_dims(fb, 0, &tile_width, &tile_height);
> +
> +       x = src_x & (tile_width - 1);
> +       y = src_y & (tile_height - 1);
> +
> +       aligned_x = src_x - x;
> +       aligned_y = src_y - y;
> +
> +       aligned_w = x + src_w;
> +       aligned_h = y + src_h;
> +
> +       /* Make src coordinates relative to the aligned viewport */
> +       drm_rect_translate(&plane_state->base.src,
> +                          -(aligned_x << 16), -(aligned_y << 16));
> +
> +       /* Rotate src coordinates to match rotated GTT view */
> +       if (drm_rotation_90_or_270(rotation))
> +               drm_rect_rotate(&plane_state->base.src,
> +                               aligned_w << 16, aligned_h << 16,
> +                               DRM_MODE_ROTATE_270);
> +
> +       for (i = 0; i < num_planes; i++) {
> +               unsigned int hsub = i ? fb->format->hsub : 1;
> +               unsigned int vsub = i ? fb->format->vsub : 1;
> +               unsigned int cpp = fb->format->cpp[i];
> +               unsigned int width, height;
> +               unsigned int pitch_tiles;
> +               unsigned int x, y;
> +               u32 offset;
> +
> +               intel_tile_dims(fb, i, &tile_width, &tile_height);
> +
> +               x = aligned_x / hsub;
> +               y = aligned_y / vsub;
> +               width = aligned_w / hsub;

aligned_w here seems to be just x2.

Did I miss aligned_w = ALIGN(aligned_w, tile_width) - aligned_x?

> +               height = aligned_h / vsub;
> +
> +               /*
> +                * First pixel of the aligned src viewport
> +                * from the start of the normal gtt mapping.
> +                */
> +               x += intel_fb->normal[i].x;
> +               y += intel_fb->normal[i].y;
> +
> +               offset = intel_compute_aligned_offset(dev_priv, &x, &y,
> +                                                     fb, i, fb->pitches[i],
> +                                                     DRM_MODE_ROTATE_0, tile_size);
> +               offset /= tile_size;
> +
> +               info->plane[i].offset = offset;
> +               info->plane[i].stride = DIV_ROUND_UP(fb->pitches[i],
> +                                                    tile_width * cpp);
> +               info->plane[i].width = DIV_ROUND_UP(x + width, tile_width);
> +               info->plane[i].height = DIV_ROUND_UP(y + height, tile_height);

I thought .width here would be in pages? Ok tile_width != fence_stride
and does produce pages just fine.

And shouldn't width here be adjusted for x? Similarly for height?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx