* [PATCH v6 01/12] drm/i915: Rewrite fb rotation GTT handling
2016-08-10 9:23 [PATCH v5 00/12] drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5) ville.syrjala
@ 2016-08-10 9:23 ` ville.syrjala
2016-08-10 9:49 ` Daniel Vetter
2016-08-10 9:23 ` [PATCH v3 02/12] drm/i915: Don't pass pitch to intel_compute_page_offset() ville.syrjala
` (11 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: ville.syrjala @ 2016-08-10 9:23 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Redo the fb rotation handling in order to:
- eliminate the NV12 special casing
- handle fb->offsets[] properly
- make the rotation handling easier for the plane code
To achieve these goals we reduce intel_rotation_info to only contain
(for each plane) the rotated view width,height,stride in tile units,
and the page offset into the object where the plane starts. Each plane
is handled exactly the same way, no special casing for NV12 or other
formats. We then store the computed rotation_info under
intel_framebuffer so that we don't have to recompute it again.
To handle fb->offsets[] we treat them as a linear offsets and convert
them to x/y offsets from the start of the relevant GTT mapping (either
normal or rotated). We store the x/y offsets under intel_framebuffer,
and for some extra convenience we also store the rotated pitch (ie.
tile aligned plane height). So for each plane we have the normal
x/y offsets, rotated x/y offsets, and the rotated pitch. The normal
pitch is available already in fb->pitches[].
While we're gathering up all that extra information, we can also easily
compute the storage requirements for the framebuffer, so that we can
check that the object is big enough to hold it.
When it comes time to deal with the plane source coordinates, we first
rotate the clipped src coordinates to match the relevant GTT view
orientation, then add to them the fb x/y offsets. Next we compute
the aligned surface page offset, and as a result we're left with some
residual x/y offsets. Finally, if required by the hardware, we convert
the remaining x/y offsets into a linear offset.
For gen2/3 we simply skip computing the final page offset, and just
convert the src+fb x/y offsets directly into a linear offset since
that's what the hardware wants.
After this all platforms, incluing SKL+, compute these things in exactly
the same way (excluding alignemnt differences).
v2: Use BIT(DRM_ROTATE_270) instead of ROTATE_270 when rotating
plane src coordinates
Drop some spurious changes that got left behind during
development
v3: Split out more changes to prep patches (Daniel)
s/intel_fb->plane[].foo.bar/intel_fb->foo[].bar/ for brevity
Rename intel_surf_gtt_offset to intel_fb_gtt_offset
Kill the pointless 'plane' parameter from intel_fb_gtt_offset()
v4: Fix alignment vs. alignment-1 when calling
_intel_compute_tile_offset() from intel_fill_fb_info()
Pass the pitch in tiles in
stad of pixels to intel_adjust_tile_offset() from intel_fill_fb_info()
Pass the full width/height of the rotated area to
drm_rect_rotate() for clarity
Use u32 for more offsets
v5: Preserve the upper_32_bits()/lower_32_bits() handling for the
fb ggtt offset (Sivakumar)
v6: Rebase due to drm_plane_state src/dst rects
Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 51 ++---
drivers/gpu/drm/i915/i915_gem_gtt.h | 5 +-
drivers/gpu/drm/i915/intel_display.c | 368 ++++++++++++++++++++++++-----------
drivers/gpu/drm/i915/intel_drv.h | 19 +-
drivers/gpu/drm/i915/intel_sprite.c | 97 ++++-----
5 files changed, 331 insertions(+), 209 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 18c7c9644761..d876501694c6 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3449,18 +3449,16 @@ rotate_pages(const dma_addr_t *in, unsigned int offset,
}
static struct sg_table *
-intel_rotate_fb_obj_pages(struct intel_rotation_info *rot_info,
+intel_rotate_fb_obj_pages(const struct intel_rotation_info *rot_info,
struct drm_i915_gem_object *obj)
{
const size_t n_pages = obj->base.size / PAGE_SIZE;
- unsigned int size_pages = rot_info->plane[0].width * rot_info->plane[0].height;
- unsigned int size_pages_uv;
+ unsigned int size = intel_rotation_info_size(rot_info);
struct sgt_iter sgt_iter;
dma_addr_t dma_addr;
unsigned long i;
dma_addr_t *page_addr_list;
struct sg_table *st;
- unsigned int uv_start_page;
struct scatterlist *sg;
int ret = -ENOMEM;
@@ -3471,18 +3469,12 @@ intel_rotate_fb_obj_pages(struct intel_rotation_info *rot_info,
if (!page_addr_list)
return ERR_PTR(ret);
- /* Account for UV plane with NV12. */
- if (rot_info->pixel_format == DRM_FORMAT_NV12)
- size_pages_uv = rot_info->plane[1].width * rot_info->plane[1].height;
- else
- size_pages_uv = 0;
-
/* Allocate target SG list. */
st = kmalloc(sizeof(*st), GFP_KERNEL);
if (!st)
goto err_st_alloc;
- ret = sg_alloc_table(st, size_pages + size_pages_uv, GFP_KERNEL);
+ ret = sg_alloc_table(st, size, GFP_KERNEL);
if (ret)
goto err_sg_alloc;
@@ -3495,32 +3487,14 @@ intel_rotate_fb_obj_pages(struct intel_rotation_info *rot_info,
st->nents = 0;
sg = st->sgl;
- /* Rotate the pages. */
- sg = rotate_pages(page_addr_list, 0,
- rot_info->plane[0].width, rot_info->plane[0].height,
- rot_info->plane[0].width,
- st, sg);
-
- /* Append the UV plane if NV12. */
- if (rot_info->pixel_format == DRM_FORMAT_NV12) {
- uv_start_page = size_pages;
-
- /* Check for tile-row un-alignment. */
- if (offset_in_page(rot_info->uv_offset))
- uv_start_page--;
-
- rot_info->uv_start_page = uv_start_page;
-
- sg = rotate_pages(page_addr_list, rot_info->uv_start_page,
- rot_info->plane[1].width, rot_info->plane[1].height,
- rot_info->plane[1].width,
- st, sg);
+ for (i = 0 ; i < ARRAY_SIZE(rot_info->plane); i++) {
+ sg = rotate_pages(page_addr_list, rot_info->plane[i].offset,
+ rot_info->plane[i].width, rot_info->plane[i].height,
+ rot_info->plane[i].stride, st, sg);
}
- DRM_DEBUG_KMS("Created rotated page mapping for object size %zu (%ux%u tiles, %u pages (%u plane 0)).\n",
- obj->base.size, rot_info->plane[0].width,
- rot_info->plane[0].height, size_pages + size_pages_uv,
- size_pages);
+ DRM_DEBUG_KMS("Created rotated page mapping for object size %zu (%ux%u tiles, %u pages)\n",
+ obj->base.size, rot_info->plane[0].width, rot_info->plane[0].height, size);
drm_free_large(page_addr_list);
@@ -3531,10 +3505,9 @@ err_sg_alloc:
err_st_alloc:
drm_free_large(page_addr_list);
- DRM_DEBUG_KMS("Failed to create rotated mapping for object size %zu! (%d) (%ux%u tiles, %u pages (%u plane 0))\n",
- obj->base.size, ret, rot_info->plane[0].width,
- rot_info->plane[0].height, size_pages + size_pages_uv,
- size_pages);
+ DRM_DEBUG_KMS("Failed to create rotated mapping for object size %zu! (%ux%u tiles, %u pages)\n",
+ obj->base.size, rot_info->plane[0].width, rot_info->plane[0].height, size);
+
return ERR_PTR(ret);
}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index cc56206a1600..56e64a5355e8 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -139,12 +139,9 @@ enum i915_ggtt_view_type {
};
struct intel_rotation_info {
- unsigned int uv_offset;
- uint32_t pixel_format;
- unsigned int uv_start_page;
struct {
/* tiles */
- unsigned int width, height;
+ unsigned int width, height, stride, offset;
} plane[2];
};
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8e71dc200880..f16a59ea7c18 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2147,33 +2147,6 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view,
}
}
-static void
-intel_fill_fb_info(struct drm_i915_private *dev_priv,
- struct drm_framebuffer *fb)
-{
- struct intel_rotation_info *info = &to_intel_framebuffer(fb)->rot_info;
- unsigned int tile_size, tile_width, tile_height, cpp;
-
- tile_size = intel_tile_size(dev_priv);
-
- cpp = drm_format_plane_cpp(fb->pixel_format, 0);
- intel_tile_dims(dev_priv, &tile_width, &tile_height,
- fb->modifier[0], cpp);
-
- info->plane[0].width = DIV_ROUND_UP(fb->pitches[0], tile_width * cpp);
- info->plane[0].height = DIV_ROUND_UP(fb->height, tile_height);
-
- if (info->pixel_format == DRM_FORMAT_NV12) {
- cpp = drm_format_plane_cpp(fb->pixel_format, 1);
- intel_tile_dims(dev_priv, &tile_width, &tile_height,
- fb->modifier[1], cpp);
-
- info->uv_offset = fb->offsets[1];
- info->plane[1].width = DIV_ROUND_UP(fb->pitches[1], tile_width * cpp);
- info->plane[1].height = DIV_ROUND_UP(fb->height / 2, tile_height);
- }
-}
-
static unsigned int intel_linear_alignment(const struct drm_i915_private *dev_priv)
{
if (INTEL_INFO(dev_priv)->gen >= 9)
@@ -2295,6 +2268,42 @@ void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
}
/*
+ * Convert the x/y offsets into a linear offset.
+ * Only valid with 0/180 degree rotation, which is fine since linear
+ * offset is only used with linear buffers on pre-hsw and tiled buffers
+ * with gen2/3, and 90/270 degree rotations isn't supported on any of them.
+ */
+u32 intel_fb_xy_to_linear(int x, int y,
+ const struct drm_framebuffer *fb, int plane)
+{
+ unsigned int cpp = drm_format_plane_cpp(fb->pixel_format, plane);
+ unsigned int pitch = fb->pitches[plane];
+
+ return y * pitch + x * cpp;
+}
+
+/*
+ * Add the x/y offsets derived from fb->offsets[] to the user
+ * specified plane src x/y offsets. The resulting x/y offsets
+ * specify the start of scanout from the beginning of the gtt mapping.
+ */
+void intel_add_fb_offsets(int *x, int *y,
+ const struct drm_framebuffer *fb, int plane,
+ unsigned int rotation)
+
+{
+ const struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+
+ if (intel_rotation_90_or_270(rotation)) {
+ *x += intel_fb->rotated[plane].x;
+ *y += intel_fb->rotated[plane].y;
+ } else {
+ *x += intel_fb->normal[plane].x;
+ *y += intel_fb->normal[plane].y;
+ }
+}
+
+/*
* Adjust the tile offset by moving the difference into
* the x/y offsets.
*
@@ -2330,18 +2339,24 @@ static u32 intel_adjust_tile_offset(int *x, int *y,
* In the 90/270 rotated case, x and y are assumed
* to be already rotated to match the rotated GTT view, and
* pitch is the tile_height aligned framebuffer height.
+ *
+ * This function is used when computing the derived information
+ * under intel_framebuffer, so using any of that information
+ * here is not allowed. Anything under drm_framebuffer can be
+ * used. This is why the user has to pass in the pitch since it
+ * is specified in the rotated orientation.
*/
-u32 intel_compute_tile_offset(int *x, int *y,
- const struct drm_framebuffer *fb, int plane,
- unsigned int pitch,
- unsigned int rotation)
+static u32 _intel_compute_tile_offset(const struct drm_i915_private *dev_priv,
+ int *x, int *y,
+ const struct drm_framebuffer *fb, int plane,
+ unsigned int pitch,
+ unsigned int rotation,
+ u32 alignment)
{
- const struct drm_i915_private *dev_priv = to_i915(fb->dev);
uint64_t fb_modifier = fb->modifier[plane];
unsigned int cpp = drm_format_plane_cpp(fb->pixel_format, plane);
- u32 offset, offset_aligned, alignment;
+ u32 offset, offset_aligned;
- alignment = intel_surf_alignment(dev_priv, fb_modifier);
if (alignment)
alignment--;
@@ -2383,6 +2398,141 @@ u32 intel_compute_tile_offset(int *x, int *y,
return offset_aligned;
}
+u32 intel_compute_tile_offset(int *x, int *y,
+ const struct drm_framebuffer *fb, int plane,
+ unsigned int pitch,
+ unsigned int rotation)
+{
+ const struct drm_i915_private *dev_priv = to_i915(fb->dev);
+ u32 alignment = intel_surf_alignment(dev_priv, fb->modifier[plane]);
+
+ return _intel_compute_tile_offset(dev_priv, x, y, fb, plane, pitch,
+ rotation, alignment);
+}
+
+/* Convert the fb->offset[] linear offset into x/y offsets */
+static void intel_fb_offset_to_xy(int *x, int *y,
+ const struct drm_framebuffer *fb, int plane)
+{
+ unsigned int cpp = drm_format_plane_cpp(fb->pixel_format, plane);
+ unsigned int pitch = fb->pitches[plane];
+ u32 linear_offset = fb->offsets[plane];
+
+ *y = linear_offset / pitch;
+ *x = linear_offset % pitch / cpp;
+}
+
+static int
+intel_fill_fb_info(struct drm_i915_private *dev_priv,
+ struct drm_framebuffer *fb)
+{
+ struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+ struct intel_rotation_info *rot_info = &intel_fb->rot_info;
+ u32 gtt_offset_rotated = 0;
+ unsigned int max_size = 0;
+ uint32_t format = fb->pixel_format;
+ int i, num_planes = drm_format_num_planes(format);
+ unsigned int tile_size = intel_tile_size(dev_priv);
+
+ for (i = 0; i < num_planes; i++) {
+ unsigned int width, height;
+ unsigned int cpp, size;
+ u32 offset;
+ int x, y;
+
+ cpp = drm_format_plane_cpp(format, i);
+ width = drm_format_plane_width(fb->width, format, i);
+ height = drm_format_plane_height(fb->height, format, i);
+
+ intel_fb_offset_to_xy(&x, &y, fb, i);
+
+ /*
+ * First pixel of the framebuffer from
+ * the start of the normal gtt mapping.
+ */
+ intel_fb->normal[i].x = x;
+ intel_fb->normal[i].y = y;
+
+ offset = _intel_compute_tile_offset(dev_priv, &x, &y,
+ fb, 0, fb->pitches[i],
+ BIT(DRM_ROTATE_0), tile_size);
+ offset /= tile_size;
+
+ if (fb->modifier[i] != DRM_FORMAT_MOD_NONE) {
+ unsigned int tile_width, tile_height;
+ unsigned int pitch_tiles;
+ struct drm_rect r;
+
+ intel_tile_dims(dev_priv, &tile_width, &tile_height,
+ fb->modifier[i], cpp);
+
+ rot_info->plane[i].offset = offset;
+ rot_info->plane[i].stride = DIV_ROUND_UP(fb->pitches[i], tile_width * cpp);
+ rot_info->plane[i].width = DIV_ROUND_UP(x + width, tile_width);
+ rot_info->plane[i].height = DIV_ROUND_UP(y + height, tile_height);
+
+ intel_fb->rotated[i].pitch =
+ rot_info->plane[i].height * tile_height;
+
+ /* how many tiles does this plane need */
+ size = rot_info->plane[i].stride * rot_info->plane[i].height;
+ /*
+ * If the plane isn't horizontally tile aligned,
+ * we need one more tile.
+ */
+ if (x != 0)
+ size++;
+
+ /* rotate the x/y offsets to match the GTT view */
+ r.x1 = x;
+ r.y1 = y;
+ r.x2 = x + width;
+ r.y2 = y + height;
+ drm_rect_rotate(&r,
+ rot_info->plane[i].width * tile_width,
+ rot_info->plane[i].height * tile_height,
+ BIT(DRM_ROTATE_270));
+ x = r.x1;
+ y = r.y1;
+
+ /* rotate the tile dimensions to match the GTT view */
+ pitch_tiles = intel_fb->rotated[i].pitch / tile_height;
+ swap(tile_width, tile_height);
+
+ /*
+ * We only keep the x/y offsets, so push all of the
+ * gtt offset into the x/y offsets.
+ */
+ intel_adjust_tile_offset(&x, &y, tile_size,
+ tile_width, tile_height, pitch_tiles,
+ gtt_offset_rotated * tile_size, 0);
+
+ gtt_offset_rotated += rot_info->plane[i].width * rot_info->plane[i].height;
+
+ /*
+ * First pixel of the framebuffer from
+ * the start of the rotated gtt mapping.
+ */
+ intel_fb->rotated[i].x = x;
+ intel_fb->rotated[i].y = y;
+ } else {
+ size = DIV_ROUND_UP((y + height) * fb->pitches[i] +
+ x * cpp, tile_size);
+ }
+
+ /* how many tiles in total needed in the bo */
+ max_size = max(max_size, offset + size);
+ }
+
+ if (max_size * tile_size > to_intel_framebuffer(fb)->obj->base.size) {
+ DRM_DEBUG("fb too big for bo (need %u bytes, have %zu bytes)\n",
+ max_size * tile_size, to_intel_framebuffer(fb)->obj->base.size);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int i9xx_format_to_fourcc(int format)
{
switch (format) {
@@ -2618,7 +2768,6 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
u32 dspcntr;
i915_reg_t reg = DSPCNTR(plane);
unsigned int rotation = plane_state->base.rotation;
- int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
int x = plane_state->base.src.x1 >> 16;
int y = plane_state->base.src.y1 >> 16;
@@ -2677,30 +2826,25 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
if (IS_G4X(dev))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
- linear_offset = y * fb->pitches[0] + x * cpp;
+ intel_add_fb_offsets(&x, &y, fb, 0, rotation);
- if (INTEL_INFO(dev)->gen >= 4) {
+ if (INTEL_INFO(dev)->gen >= 4)
intel_crtc->dspaddr_offset =
intel_compute_tile_offset(&x, &y, fb, 0,
fb->pitches[0], rotation);
- linear_offset -= intel_crtc->dspaddr_offset;
- } else {
- intel_crtc->dspaddr_offset = linear_offset;
- }
if (rotation == DRM_ROTATE_180) {
dspcntr |= DISPPLANE_ROTATE_180;
x += (crtc_state->pipe_src_w - 1);
y += (crtc_state->pipe_src_h - 1);
-
- /* Finding the last pixel of the last line of the display
- data and adding to linear_offset*/
- linear_offset +=
- (crtc_state->pipe_src_h - 1) * fb->pitches[0] +
- (crtc_state->pipe_src_w - 1) * cpp;
}
+ linear_offset = intel_fb_xy_to_linear(x, y, fb, 0);
+
+ if (INTEL_INFO(dev)->gen < 4)
+ intel_crtc->dspaddr_offset = linear_offset;
+
intel_crtc->adjusted_x = x;
intel_crtc->adjusted_y = y;
@@ -2709,7 +2853,8 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
if (INTEL_INFO(dev)->gen >= 4) {
I915_WRITE(DSPSURF(plane),
- i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset);
+ intel_fb_gtt_offset(fb, rotation) +
+ intel_crtc->dspaddr_offset);
I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
I915_WRITE(DSPLINOFF(plane), linear_offset);
} else
@@ -2747,7 +2892,6 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
u32 dspcntr;
i915_reg_t reg = DSPCNTR(plane);
unsigned int rotation = plane_state->base.rotation;
- int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
int x = plane_state->base.src.x1 >> 16;
int y = plane_state->base.src.y1 >> 16;
@@ -2786,26 +2930,23 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
if (!IS_HASWELL(dev) && !IS_BROADWELL(dev))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
- linear_offset = y * fb->pitches[0] + x * cpp;
+ intel_add_fb_offsets(&x, &y, fb, 0, rotation);
+
intel_crtc->dspaddr_offset =
intel_compute_tile_offset(&x, &y, fb, 0,
fb->pitches[0], rotation);
- linear_offset -= intel_crtc->dspaddr_offset;
+
if (rotation == DRM_ROTATE_180) {
dspcntr |= DISPPLANE_ROTATE_180;
if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
x += (crtc_state->pipe_src_w - 1);
y += (crtc_state->pipe_src_h - 1);
-
- /* Finding the last pixel of the last line of the display
- data and adding to linear_offset*/
- linear_offset +=
- (crtc_state->pipe_src_h - 1) * fb->pitches[0] +
- (crtc_state->pipe_src_w - 1) * cpp;
}
}
+ linear_offset = intel_fb_xy_to_linear(x, y, fb, 0);
+
intel_crtc->adjusted_x = x;
intel_crtc->adjusted_y = y;
@@ -2813,7 +2954,8 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
I915_WRITE(DSPSURF(plane),
- i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset);
+ intel_fb_gtt_offset(fb, rotation) +
+ intel_crtc->dspaddr_offset);
if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
I915_WRITE(DSPOFFSET(plane), (y << 16) | x);
} else {
@@ -2835,28 +2977,16 @@ u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv,
}
}
-u32 intel_plane_obj_offset(struct intel_plane *intel_plane,
- struct drm_i915_gem_object *obj,
- unsigned int plane)
+u32 intel_fb_gtt_offset(struct drm_framebuffer *fb,
+ unsigned int rotation)
{
+ struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct i915_ggtt_view view;
- struct i915_vma *vma;
u64 offset;
- intel_fill_fb_ggtt_view(&view, intel_plane->base.state->fb,
- intel_plane->base.state->rotation);
-
- vma = i915_gem_obj_to_ggtt_view(obj, &view);
- if (WARN(!vma, "ggtt vma for display object not found! (view=%u)\n",
- view.type))
- return -1;
-
- offset = vma->node.start;
+ intel_fill_fb_ggtt_view(&view, fb, rotation);
- if (plane == 1) {
- offset += vma->ggtt_view.params.rotated.uv_start_page *
- PAGE_SIZE;
- }
+ offset = i915_gem_obj_ggtt_offset_view(obj, &view);
WARN_ON(upper_32_bits(offset));
@@ -2979,12 +3109,9 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_framebuffer *fb = plane_state->base.fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
int pipe = intel_crtc->pipe;
u32 plane_ctl, stride_div, stride;
- u32 tile_height, plane_offset, plane_size;
unsigned int rotation = plane_state->base.rotation;
- int x_offset, y_offset;
u32 surf_addr;
int scaler_id = plane_state->scaler_id;
int src_x = plane_state->base.src.x1 >> 16;
@@ -3005,36 +3132,49 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
plane_ctl |= skl_plane_ctl_rotation(rotation);
- stride_div = intel_fb_stride_alignment(dev_priv, fb->modifier[0],
- fb->pixel_format);
- surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj, 0);
-
- WARN_ON(drm_rect_width(&plane_state->base.src) == 0);
-
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,
+ };
int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
+ struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+
+ /* Rotate src coordinates to match rotated GTT view */
+ drm_rect_rotate(&r, fb->width, fb->height, BIT(DRM_ROTATE_270));
- /* stride = Surface height in tiles */
- tile_height = intel_tile_height(dev_priv, fb->modifier[0], cpp);
- stride = DIV_ROUND_UP(fb->height, tile_height);
- x_offset = stride * tile_height - src_y - src_h;
- y_offset = src_x;
- plane_size = (src_w - 1) << 16 | (src_h - 1);
+ src_x = r.x1;
+ src_y = r.y1;
+ src_w = drm_rect_width(&r);
+ src_h = drm_rect_height(&r);
+
+ stride_div = intel_tile_height(dev_priv, fb->modifier[0], cpp);
+ stride = intel_fb->rotated[0].pitch;
} else {
- stride = fb->pitches[0] / stride_div;
- x_offset = src_x;
- y_offset = src_y;
- plane_size = (src_h - 1) << 16 | (src_w - 1);
+ stride_div = intel_fb_stride_alignment(dev_priv, fb->modifier[0],
+ fb->pixel_format);
+ stride = fb->pitches[0];
}
- plane_offset = y_offset << 16 | x_offset;
- intel_crtc->adjusted_x = x_offset;
- intel_crtc->adjusted_y = y_offset;
+ intel_add_fb_offsets(&src_x, &src_y, fb, 0, rotation);
+ surf_addr = intel_compute_tile_offset(&src_x, &src_y, fb, 0,
+ stride, rotation);
+
+ /* Sizes are 0 based */
+ src_w--;
+ src_h--;
+ dst_w--;
+ dst_h--;
+
+ intel_crtc->adjusted_x = src_x;
+ intel_crtc->adjusted_y = src_y;
I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
- I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset);
- I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
- I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
+ I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x);
+ I915_WRITE(PLANE_STRIDE(pipe, 0), stride / stride_div);
+ I915_WRITE(PLANE_SIZE(pipe, 0), (src_h << 16) | src_w);
if (scaler_id >= 0) {
uint32_t ps_ctrl = 0;
@@ -3051,7 +3191,8 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
I915_WRITE(PLANE_POS(pipe, 0), (dst_y << 16) | dst_x);
}
- I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
+ I915_WRITE(PLANE_SURF(pipe, 0),
+ intel_fb_gtt_offset(fb, rotation) + surf_addr);
POSTING_READ(PLANE_SURF(pipe, 0));
}
@@ -11462,7 +11603,7 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
struct drm_i915_private *dev_priv = to_i915(dev);
struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
const enum pipe pipe = intel_crtc->pipe;
- u32 ctl, stride, tile_height;
+ u32 ctl, stride;
ctl = I915_READ(PLANE_CTL(pipe, 0));
ctl &= ~PLANE_CTL_TILED_MASK;
@@ -11487,9 +11628,11 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
* linear buffers or in number of tiles for tiled buffers.
*/
if (intel_rotation_90_or_270(rotation)) {
- /* stride = Surface height in tiles */
- tile_height = intel_tile_height(dev_priv, fb->modifier[0], 0);
- stride = DIV_ROUND_UP(fb->height, tile_height);
+ int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
+ struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+
+ stride = intel_fb->rotated[0].pitch /
+ intel_tile_height(dev_priv, fb->modifier[0], cpp);
} else {
stride = fb->pitches[0] /
intel_fb_stride_alignment(dev_priv, fb->modifier[0],
@@ -11769,8 +11912,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
if (ret)
goto cleanup_pending;
- work->gtt_offset = intel_plane_obj_offset(to_intel_plane(primary),
- obj, 0);
+ work->gtt_offset = intel_fb_gtt_offset(fb, primary->state->rotation);
work->gtt_offset += intel_crtc->dspaddr_offset;
work->rotation = crtc->primary->state->rotation;
@@ -14988,7 +15130,6 @@ static int intel_framebuffer_init(struct drm_device *dev,
struct drm_i915_gem_object *obj)
{
struct drm_i915_private *dev_priv = to_i915(dev);
- unsigned int aligned_height;
int ret;
u32 pitch_limit, stride_alignment;
@@ -15114,17 +15255,12 @@ static int intel_framebuffer_init(struct drm_device *dev,
if (mode_cmd->offsets[0] != 0)
return -EINVAL;
- aligned_height = intel_fb_align_height(dev, mode_cmd->height,
- mode_cmd->pixel_format,
- mode_cmd->modifier[0]);
- /* FIXME drm helper for size checks (especially planar formats)? */
- if (obj->base.size < aligned_height * mode_cmd->pitches[0])
- return -EINVAL;
-
drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
intel_fb->obj = obj;
- intel_fill_fb_info(dev_priv, &intel_fb->base);
+ ret = intel_fill_fb_info(dev_priv, &intel_fb->base);
+ if (ret)
+ return ret;
ret = drm_framebuffer_init(dev, &intel_fb->base, &intel_fb_funcs);
if (ret) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c29a429cbc45..a81d2b8bd740 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -178,6 +178,16 @@ struct intel_framebuffer {
struct drm_framebuffer base;
struct drm_i915_gem_object *obj;
struct intel_rotation_info rot_info;
+
+ /* for each plane in the normal GTT view */
+ struct {
+ unsigned int x, y;
+ } normal[2];
+ /* for each plane in the rotated GTT view */
+ struct {
+ unsigned int x, y;
+ unsigned int pitch; /* pixels */
+ } rotated[2];
};
struct intel_fbdev {
@@ -1153,6 +1163,11 @@ int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
const char *name, u32 reg, int ref_freq);
extern const struct drm_plane_funcs intel_plane_funcs;
void intel_init_display_hooks(struct drm_i915_private *dev_priv);
+unsigned int intel_fb_xy_to_linear(int x, int y,
+ const struct drm_framebuffer *fb, int plane);
+void intel_add_fb_offsets(int *x, int *y,
+ const struct drm_framebuffer *fb, int plane,
+ unsigned int rotation);
unsigned int intel_rotation_info_size(const struct intel_rotation_info *rot_info);
bool intel_has_pending_fb_unpin(struct drm_device *dev);
void intel_mark_busy(struct drm_i915_private *dev_priv);
@@ -1322,9 +1337,7 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
-u32 intel_plane_obj_offset(struct intel_plane *intel_plane,
- struct drm_i915_gem_object *obj,
- unsigned int plane);
+u32 intel_fb_gtt_offset(struct drm_framebuffer *fb, unsigned int rotation);
u32 skl_plane_ctl_format(uint32_t pixel_format);
u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index cbdca7e4d307..ef4d0a6e23af 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -203,15 +203,13 @@ skl_update_plane(struct drm_plane *drm_plane,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_plane *intel_plane = to_intel_plane(drm_plane);
struct drm_framebuffer *fb = plane_state->base.fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
const int pipe = intel_plane->pipe;
const int plane = intel_plane->plane + 1;
u32 plane_ctl, stride_div, stride;
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
u32 surf_addr;
- u32 tile_height, plane_offset, plane_size;
unsigned int rotation = plane_state->base.rotation;
- int x_offset, y_offset;
int crtc_x = plane_state->base.dst.x1;
int crtc_y = plane_state->base.dst.y1;
uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
@@ -230,15 +228,6 @@ skl_update_plane(struct drm_plane *drm_plane,
plane_ctl |= skl_plane_ctl_rotation(rotation);
- stride_div = intel_fb_stride_alignment(dev_priv, fb->modifier[0],
- fb->pixel_format);
-
- /* Sizes are 0 based */
- src_w--;
- src_h--;
- crtc_w--;
- crtc_h--;
-
if (key->flags) {
I915_WRITE(PLANE_KEYVAL(pipe, plane), key->min_value);
I915_WRITE(PLANE_KEYMAX(pipe, plane), key->max_value);
@@ -250,28 +239,44 @@ skl_update_plane(struct drm_plane *drm_plane,
else if (key->flags & I915_SET_COLORKEY_SOURCE)
plane_ctl |= PLANE_CTL_KEY_ENABLE_SOURCE;
- surf_addr = intel_plane_obj_offset(intel_plane, obj, 0);
-
if (intel_rotation_90_or_270(rotation)) {
- int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
-
- /* stride: Surface height in tiles */
- tile_height = intel_tile_height(dev_priv, fb->modifier[0], cpp);
- stride = DIV_ROUND_UP(fb->height, tile_height);
- plane_size = (src_w << 16) | src_h;
- x_offset = stride * tile_height - y - (src_h + 1);
- y_offset = x;
+ struct drm_rect r = {
+ .x1 = x,
+ .x2 = x + src_w,
+ .y1 = y,
+ .y2 = y + src_h,
+ };
+ unsigned int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
+
+ /* 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);
+
+ stride_div = intel_tile_height(dev_priv, fb->modifier[0], cpp);
+ stride = intel_fb->rotated[0].pitch;
} else {
- stride = fb->pitches[0] / stride_div;
- plane_size = (src_h << 16) | src_w;
- x_offset = x;
- y_offset = y;
+ stride_div = intel_fb_stride_alignment(dev_priv, fb->modifier[0],
+ fb->pixel_format);
+ stride = fb->pitches[0];
}
- plane_offset = y_offset << 16 | x_offset;
- I915_WRITE(PLANE_OFFSET(pipe, plane), plane_offset);
- I915_WRITE(PLANE_STRIDE(pipe, plane), stride);
- I915_WRITE(PLANE_SIZE(pipe, plane), plane_size);
+ intel_add_fb_offsets(&x, &y, fb, 0, rotation);
+ surf_addr = intel_compute_tile_offset(&x, &y, fb, 0,
+ stride, rotation);
+
+ /* Sizes are 0 based */
+ src_w--;
+ src_h--;
+ crtc_w--;
+ crtc_h--;
+
+ I915_WRITE(PLANE_OFFSET(pipe, plane), (y << 16) | x);
+ I915_WRITE(PLANE_STRIDE(pipe, plane), stride / stride_div);
+ I915_WRITE(PLANE_SIZE(pipe, plane), (src_h << 16) | src_w);
/* program plane scaler */
if (plane_state->scaler_id >= 0) {
@@ -296,7 +301,8 @@ skl_update_plane(struct drm_plane *drm_plane,
}
I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl);
- I915_WRITE(PLANE_SURF(pipe, plane), surf_addr);
+ I915_WRITE(PLANE_SURF(pipe, plane),
+ intel_fb_gtt_offset(fb, rotation) + surf_addr);
POSTING_READ(PLANE_SURF(pipe, plane));
}
@@ -369,7 +375,6 @@ vlv_update_plane(struct drm_plane *dplane,
u32 sprctl;
u32 sprsurf_offset, linear_offset;
unsigned int rotation = dplane->state->rotation;
- int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
int crtc_x = plane_state->base.dst.x1;
int crtc_y = plane_state->base.dst.y1;
@@ -440,19 +445,19 @@ vlv_update_plane(struct drm_plane *dplane,
crtc_w--;
crtc_h--;
- linear_offset = y * fb->pitches[0] + x * cpp;
+ intel_add_fb_offsets(&x, &y, fb, 0, rotation);
sprsurf_offset = intel_compute_tile_offset(&x, &y, fb, 0,
fb->pitches[0], rotation);
- linear_offset -= sprsurf_offset;
if (rotation == DRM_ROTATE_180) {
sprctl |= SP_ROTATE_180;
x += src_w;
y += src_h;
- linear_offset += src_h * fb->pitches[0] + src_w * cpp;
}
+ linear_offset = intel_fb_xy_to_linear(x, y, fb, 0);
+
if (key->flags) {
I915_WRITE(SPKEYMINVAL(pipe, plane), key->min_value);
I915_WRITE(SPKEYMAXVAL(pipe, plane), key->max_value);
@@ -477,8 +482,8 @@ vlv_update_plane(struct drm_plane *dplane,
I915_WRITE(SPSIZE(pipe, plane), (crtc_h << 16) | crtc_w);
I915_WRITE(SPCNTR(pipe, plane), sprctl);
- I915_WRITE(SPSURF(pipe, plane), i915_gem_obj_ggtt_offset(obj) +
- sprsurf_offset);
+ I915_WRITE(SPSURF(pipe, plane),
+ intel_fb_gtt_offset(fb, rotation) + sprsurf_offset);
POSTING_READ(SPSURF(pipe, plane));
}
@@ -511,7 +516,6 @@ ivb_update_plane(struct drm_plane *plane,
u32 sprctl, sprscale = 0;
u32 sprsurf_offset, linear_offset;
unsigned int rotation = plane_state->base.rotation;
- int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
int crtc_x = plane_state->base.dst.x1;
int crtc_y = plane_state->base.dst.y1;
@@ -573,10 +577,9 @@ ivb_update_plane(struct drm_plane *plane,
if (crtc_w != src_w || crtc_h != src_h)
sprscale = SPRITE_SCALE_ENABLE | (src_w << 16) | src_h;
- linear_offset = y * fb->pitches[0] + x * cpp;
+ intel_add_fb_offsets(&x, &y, fb, 0, rotation);
sprsurf_offset = intel_compute_tile_offset(&x, &y, fb, 0,
fb->pitches[0], rotation);
- linear_offset -= sprsurf_offset;
if (rotation == DRM_ROTATE_180) {
sprctl |= SPRITE_ROTATE_180;
@@ -585,10 +588,11 @@ ivb_update_plane(struct drm_plane *plane,
if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
x += src_w;
y += src_h;
- linear_offset += src_h * fb->pitches[0] + src_w * cpp;
}
}
+ linear_offset = intel_fb_xy_to_linear(x, y, fb, 0);
+
if (key->flags) {
I915_WRITE(SPRKEYVAL(pipe), key->min_value);
I915_WRITE(SPRKEYMAX(pipe), key->max_value);
@@ -617,7 +621,7 @@ ivb_update_plane(struct drm_plane *plane,
I915_WRITE(SPRSCALE(pipe), sprscale);
I915_WRITE(SPRCTL(pipe), sprctl);
I915_WRITE(SPRSURF(pipe),
- i915_gem_obj_ggtt_offset(obj) + sprsurf_offset);
+ intel_fb_gtt_offset(fb, rotation) + sprsurf_offset);
POSTING_READ(SPRSURF(pipe));
}
@@ -652,7 +656,6 @@ ilk_update_plane(struct drm_plane *plane,
u32 dvscntr, dvsscale;
u32 dvssurf_offset, linear_offset;
unsigned int rotation = plane_state->base.rotation;
- int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
int crtc_x = plane_state->base.dst.x1;
int crtc_y = plane_state->base.dst.y1;
@@ -710,19 +713,19 @@ ilk_update_plane(struct drm_plane *plane,
if (crtc_w != src_w || crtc_h != src_h)
dvsscale = DVS_SCALE_ENABLE | (src_w << 16) | src_h;
- linear_offset = y * fb->pitches[0] + x * cpp;
+ intel_add_fb_offsets(&x, &y, fb, 0, rotation);
dvssurf_offset = intel_compute_tile_offset(&x, &y, fb, 0,
fb->pitches[0], rotation);
- linear_offset -= dvssurf_offset;
if (rotation == DRM_ROTATE_180) {
dvscntr |= DVS_ROTATE_180;
x += src_w;
y += src_h;
- linear_offset += src_h * fb->pitches[0] + src_w * cpp;
}
+ linear_offset = intel_fb_xy_to_linear(x, y, fb, 0);
+
if (key->flags) {
I915_WRITE(DVSKEYVAL(pipe), key->min_value);
I915_WRITE(DVSKEYMAX(pipe), key->max_value);
@@ -746,7 +749,7 @@ ilk_update_plane(struct drm_plane *plane,
I915_WRITE(DVSSCALE(pipe), dvsscale);
I915_WRITE(DVSCNTR(pipe), dvscntr);
I915_WRITE(DVSSURF(pipe),
- i915_gem_obj_ggtt_offset(obj) + dvssurf_offset);
+ intel_fb_gtt_offset(fb, rotation) + dvssurf_offset);
POSTING_READ(DVSSURF(pipe));
}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v6 01/12] drm/i915: Rewrite fb rotation GTT handling
2016-08-10 9:23 ` [PATCH v6 01/12] drm/i915: Rewrite fb rotation GTT handling ville.syrjala
@ 2016-08-10 9:49 ` Daniel Vetter
0 siblings, 0 replies; 27+ messages in thread
From: Daniel Vetter @ 2016-08-10 9:49 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Wed, Aug 10, 2016 at 12:23:10PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Redo the fb rotation handling in order to:
> - eliminate the NV12 special casing
> - handle fb->offsets[] properly
> - make the rotation handling easier for the plane code
>
> To achieve these goals we reduce intel_rotation_info to only contain
> (for each plane) the rotated view width,height,stride in tile units,
> and the page offset into the object where the plane starts. Each plane
> is handled exactly the same way, no special casing for NV12 or other
> formats. We then store the computed rotation_info under
> intel_framebuffer so that we don't have to recompute it again.
>
> To handle fb->offsets[] we treat them as a linear offsets and convert
> them to x/y offsets from the start of the relevant GTT mapping (either
> normal or rotated). We store the x/y offsets under intel_framebuffer,
> and for some extra convenience we also store the rotated pitch (ie.
> tile aligned plane height). So for each plane we have the normal
> x/y offsets, rotated x/y offsets, and the rotated pitch. The normal
> pitch is available already in fb->pitches[].
>
> While we're gathering up all that extra information, we can also easily
> compute the storage requirements for the framebuffer, so that we can
> check that the object is big enough to hold it.
>
> When it comes time to deal with the plane source coordinates, we first
> rotate the clipped src coordinates to match the relevant GTT view
> orientation, then add to them the fb x/y offsets. Next we compute
> the aligned surface page offset, and as a result we're left with some
> residual x/y offsets. Finally, if required by the hardware, we convert
> the remaining x/y offsets into a linear offset.
>
> For gen2/3 we simply skip computing the final page offset, and just
> convert the src+fb x/y offsets directly into a linear offset since
> that's what the hardware wants.
>
> After this all platforms, incluing SKL+, compute these things in exactly
> the same way (excluding alignemnt differences).
>
> v2: Use BIT(DRM_ROTATE_270) instead of ROTATE_270 when rotating
> plane src coordinates
> Drop some spurious changes that got left behind during
> development
> v3: Split out more changes to prep patches (Daniel)
> s/intel_fb->plane[].foo.bar/intel_fb->foo[].bar/ for brevity
> Rename intel_surf_gtt_offset to intel_fb_gtt_offset
> Kill the pointless 'plane' parameter from intel_fb_gtt_offset()
> v4: Fix alignment vs. alignment-1 when calling
> _intel_compute_tile_offset() from intel_fill_fb_info()
> Pass the pitch in tiles in
> stad of pixels to intel_adjust_tile_offset() from intel_fill_fb_info()
> Pass the full width/height of the rotated area to
> drm_rect_rotate() for clarity
> Use u32 for more offsets
> v5: Preserve the upper_32_bits()/lower_32_bits() handling for the
> fb ggtt offset (Sivakumar)
> v6: Rebase due to drm_plane_state src/dst rects
>
> Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Not feeling like checking the entire math again in full detail, but makes
sense, and I did check the math on an earlier version.
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 51 ++---
> drivers/gpu/drm/i915/i915_gem_gtt.h | 5 +-
> drivers/gpu/drm/i915/intel_display.c | 368 ++++++++++++++++++++++++-----------
> drivers/gpu/drm/i915/intel_drv.h | 19 +-
> drivers/gpu/drm/i915/intel_sprite.c | 97 ++++-----
> 5 files changed, 331 insertions(+), 209 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 18c7c9644761..d876501694c6 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3449,18 +3449,16 @@ rotate_pages(const dma_addr_t *in, unsigned int offset,
> }
>
> static struct sg_table *
> -intel_rotate_fb_obj_pages(struct intel_rotation_info *rot_info,
> +intel_rotate_fb_obj_pages(const struct intel_rotation_info *rot_info,
> struct drm_i915_gem_object *obj)
> {
> const size_t n_pages = obj->base.size / PAGE_SIZE;
> - unsigned int size_pages = rot_info->plane[0].width * rot_info->plane[0].height;
> - unsigned int size_pages_uv;
> + unsigned int size = intel_rotation_info_size(rot_info);
> struct sgt_iter sgt_iter;
> dma_addr_t dma_addr;
> unsigned long i;
> dma_addr_t *page_addr_list;
> struct sg_table *st;
> - unsigned int uv_start_page;
> struct scatterlist *sg;
> int ret = -ENOMEM;
>
> @@ -3471,18 +3469,12 @@ intel_rotate_fb_obj_pages(struct intel_rotation_info *rot_info,
> if (!page_addr_list)
> return ERR_PTR(ret);
>
> - /* Account for UV plane with NV12. */
> - if (rot_info->pixel_format == DRM_FORMAT_NV12)
> - size_pages_uv = rot_info->plane[1].width * rot_info->plane[1].height;
> - else
> - size_pages_uv = 0;
> -
> /* Allocate target SG list. */
> st = kmalloc(sizeof(*st), GFP_KERNEL);
> if (!st)
> goto err_st_alloc;
>
> - ret = sg_alloc_table(st, size_pages + size_pages_uv, GFP_KERNEL);
> + ret = sg_alloc_table(st, size, GFP_KERNEL);
> if (ret)
> goto err_sg_alloc;
>
> @@ -3495,32 +3487,14 @@ intel_rotate_fb_obj_pages(struct intel_rotation_info *rot_info,
> st->nents = 0;
> sg = st->sgl;
>
> - /* Rotate the pages. */
> - sg = rotate_pages(page_addr_list, 0,
> - rot_info->plane[0].width, rot_info->plane[0].height,
> - rot_info->plane[0].width,
> - st, sg);
> -
> - /* Append the UV plane if NV12. */
> - if (rot_info->pixel_format == DRM_FORMAT_NV12) {
> - uv_start_page = size_pages;
> -
> - /* Check for tile-row un-alignment. */
> - if (offset_in_page(rot_info->uv_offset))
> - uv_start_page--;
> -
> - rot_info->uv_start_page = uv_start_page;
> -
> - sg = rotate_pages(page_addr_list, rot_info->uv_start_page,
> - rot_info->plane[1].width, rot_info->plane[1].height,
> - rot_info->plane[1].width,
> - st, sg);
> + for (i = 0 ; i < ARRAY_SIZE(rot_info->plane); i++) {
> + sg = rotate_pages(page_addr_list, rot_info->plane[i].offset,
> + rot_info->plane[i].width, rot_info->plane[i].height,
> + rot_info->plane[i].stride, st, sg);
> }
>
> - DRM_DEBUG_KMS("Created rotated page mapping for object size %zu (%ux%u tiles, %u pages (%u plane 0)).\n",
> - obj->base.size, rot_info->plane[0].width,
> - rot_info->plane[0].height, size_pages + size_pages_uv,
> - size_pages);
> + DRM_DEBUG_KMS("Created rotated page mapping for object size %zu (%ux%u tiles, %u pages)\n",
> + obj->base.size, rot_info->plane[0].width, rot_info->plane[0].height, size);
>
> drm_free_large(page_addr_list);
>
> @@ -3531,10 +3505,9 @@ err_sg_alloc:
> err_st_alloc:
> drm_free_large(page_addr_list);
>
> - DRM_DEBUG_KMS("Failed to create rotated mapping for object size %zu! (%d) (%ux%u tiles, %u pages (%u plane 0))\n",
> - obj->base.size, ret, rot_info->plane[0].width,
> - rot_info->plane[0].height, size_pages + size_pages_uv,
> - size_pages);
> + DRM_DEBUG_KMS("Failed to create rotated mapping for object size %zu! (%ux%u tiles, %u pages)\n",
> + obj->base.size, rot_info->plane[0].width, rot_info->plane[0].height, size);
> +
> return ERR_PTR(ret);
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index cc56206a1600..56e64a5355e8 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -139,12 +139,9 @@ enum i915_ggtt_view_type {
> };
>
> struct intel_rotation_info {
> - unsigned int uv_offset;
> - uint32_t pixel_format;
> - unsigned int uv_start_page;
> struct {
> /* tiles */
> - unsigned int width, height;
> + unsigned int width, height, stride, offset;
> } plane[2];
> };
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8e71dc200880..f16a59ea7c18 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2147,33 +2147,6 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view,
> }
> }
>
> -static void
> -intel_fill_fb_info(struct drm_i915_private *dev_priv,
> - struct drm_framebuffer *fb)
> -{
> - struct intel_rotation_info *info = &to_intel_framebuffer(fb)->rot_info;
> - unsigned int tile_size, tile_width, tile_height, cpp;
> -
> - tile_size = intel_tile_size(dev_priv);
> -
> - cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> - intel_tile_dims(dev_priv, &tile_width, &tile_height,
> - fb->modifier[0], cpp);
> -
> - info->plane[0].width = DIV_ROUND_UP(fb->pitches[0], tile_width * cpp);
> - info->plane[0].height = DIV_ROUND_UP(fb->height, tile_height);
> -
> - if (info->pixel_format == DRM_FORMAT_NV12) {
> - cpp = drm_format_plane_cpp(fb->pixel_format, 1);
> - intel_tile_dims(dev_priv, &tile_width, &tile_height,
> - fb->modifier[1], cpp);
> -
> - info->uv_offset = fb->offsets[1];
> - info->plane[1].width = DIV_ROUND_UP(fb->pitches[1], tile_width * cpp);
> - info->plane[1].height = DIV_ROUND_UP(fb->height / 2, tile_height);
> - }
> -}
> -
> static unsigned int intel_linear_alignment(const struct drm_i915_private *dev_priv)
> {
> if (INTEL_INFO(dev_priv)->gen >= 9)
> @@ -2295,6 +2268,42 @@ void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
> }
>
> /*
> + * Convert the x/y offsets into a linear offset.
> + * Only valid with 0/180 degree rotation, which is fine since linear
> + * offset is only used with linear buffers on pre-hsw and tiled buffers
> + * with gen2/3, and 90/270 degree rotations isn't supported on any of them.
> + */
> +u32 intel_fb_xy_to_linear(int x, int y,
> + const struct drm_framebuffer *fb, int plane)
> +{
> + unsigned int cpp = drm_format_plane_cpp(fb->pixel_format, plane);
> + unsigned int pitch = fb->pitches[plane];
> +
> + return y * pitch + x * cpp;
> +}
> +
> +/*
> + * Add the x/y offsets derived from fb->offsets[] to the user
> + * specified plane src x/y offsets. The resulting x/y offsets
> + * specify the start of scanout from the beginning of the gtt mapping.
> + */
> +void intel_add_fb_offsets(int *x, int *y,
> + const struct drm_framebuffer *fb, int plane,
> + unsigned int rotation)
> +
> +{
> + const struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
> +
> + if (intel_rotation_90_or_270(rotation)) {
> + *x += intel_fb->rotated[plane].x;
> + *y += intel_fb->rotated[plane].y;
> + } else {
> + *x += intel_fb->normal[plane].x;
> + *y += intel_fb->normal[plane].y;
> + }
> +}
> +
> +/*
> * Adjust the tile offset by moving the difference into
> * the x/y offsets.
> *
> @@ -2330,18 +2339,24 @@ static u32 intel_adjust_tile_offset(int *x, int *y,
> * In the 90/270 rotated case, x and y are assumed
> * to be already rotated to match the rotated GTT view, and
> * pitch is the tile_height aligned framebuffer height.
> + *
> + * This function is used when computing the derived information
> + * under intel_framebuffer, so using any of that information
> + * here is not allowed. Anything under drm_framebuffer can be
> + * used. This is why the user has to pass in the pitch since it
> + * is specified in the rotated orientation.
> */
> -u32 intel_compute_tile_offset(int *x, int *y,
> - const struct drm_framebuffer *fb, int plane,
> - unsigned int pitch,
> - unsigned int rotation)
> +static u32 _intel_compute_tile_offset(const struct drm_i915_private *dev_priv,
> + int *x, int *y,
> + const struct drm_framebuffer *fb, int plane,
> + unsigned int pitch,
> + unsigned int rotation,
> + u32 alignment)
> {
> - const struct drm_i915_private *dev_priv = to_i915(fb->dev);
> uint64_t fb_modifier = fb->modifier[plane];
> unsigned int cpp = drm_format_plane_cpp(fb->pixel_format, plane);
> - u32 offset, offset_aligned, alignment;
> + u32 offset, offset_aligned;
>
> - alignment = intel_surf_alignment(dev_priv, fb_modifier);
> if (alignment)
> alignment--;
>
> @@ -2383,6 +2398,141 @@ u32 intel_compute_tile_offset(int *x, int *y,
> return offset_aligned;
> }
>
> +u32 intel_compute_tile_offset(int *x, int *y,
> + const struct drm_framebuffer *fb, int plane,
> + unsigned int pitch,
> + unsigned int rotation)
> +{
> + const struct drm_i915_private *dev_priv = to_i915(fb->dev);
> + u32 alignment = intel_surf_alignment(dev_priv, fb->modifier[plane]);
> +
> + return _intel_compute_tile_offset(dev_priv, x, y, fb, plane, pitch,
> + rotation, alignment);
> +}
> +
> +/* Convert the fb->offset[] linear offset into x/y offsets */
> +static void intel_fb_offset_to_xy(int *x, int *y,
> + const struct drm_framebuffer *fb, int plane)
> +{
> + unsigned int cpp = drm_format_plane_cpp(fb->pixel_format, plane);
> + unsigned int pitch = fb->pitches[plane];
> + u32 linear_offset = fb->offsets[plane];
> +
> + *y = linear_offset / pitch;
> + *x = linear_offset % pitch / cpp;
> +}
> +
> +static int
> +intel_fill_fb_info(struct drm_i915_private *dev_priv,
> + struct drm_framebuffer *fb)
> +{
> + struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
> + struct intel_rotation_info *rot_info = &intel_fb->rot_info;
> + u32 gtt_offset_rotated = 0;
> + unsigned int max_size = 0;
> + uint32_t format = fb->pixel_format;
> + int i, num_planes = drm_format_num_planes(format);
> + unsigned int tile_size = intel_tile_size(dev_priv);
> +
> + for (i = 0; i < num_planes; i++) {
> + unsigned int width, height;
> + unsigned int cpp, size;
> + u32 offset;
> + int x, y;
> +
> + cpp = drm_format_plane_cpp(format, i);
> + width = drm_format_plane_width(fb->width, format, i);
> + height = drm_format_plane_height(fb->height, format, i);
> +
> + intel_fb_offset_to_xy(&x, &y, fb, i);
> +
> + /*
> + * First pixel of the framebuffer from
> + * the start of the normal gtt mapping.
> + */
> + intel_fb->normal[i].x = x;
> + intel_fb->normal[i].y = y;
> +
> + offset = _intel_compute_tile_offset(dev_priv, &x, &y,
> + fb, 0, fb->pitches[i],
> + BIT(DRM_ROTATE_0), tile_size);
> + offset /= tile_size;
> +
> + if (fb->modifier[i] != DRM_FORMAT_MOD_NONE) {
> + unsigned int tile_width, tile_height;
> + unsigned int pitch_tiles;
> + struct drm_rect r;
> +
> + intel_tile_dims(dev_priv, &tile_width, &tile_height,
> + fb->modifier[i], cpp);
> +
> + rot_info->plane[i].offset = offset;
> + rot_info->plane[i].stride = DIV_ROUND_UP(fb->pitches[i], tile_width * cpp);
> + rot_info->plane[i].width = DIV_ROUND_UP(x + width, tile_width);
> + rot_info->plane[i].height = DIV_ROUND_UP(y + height, tile_height);
> +
> + intel_fb->rotated[i].pitch =
> + rot_info->plane[i].height * tile_height;
> +
> + /* how many tiles does this plane need */
> + size = rot_info->plane[i].stride * rot_info->plane[i].height;
> + /*
> + * If the plane isn't horizontally tile aligned,
> + * we need one more tile.
> + */
> + if (x != 0)
> + size++;
> +
> + /* rotate the x/y offsets to match the GTT view */
> + r.x1 = x;
> + r.y1 = y;
> + r.x2 = x + width;
> + r.y2 = y + height;
> + drm_rect_rotate(&r,
> + rot_info->plane[i].width * tile_width,
> + rot_info->plane[i].height * tile_height,
> + BIT(DRM_ROTATE_270));
> + x = r.x1;
> + y = r.y1;
> +
> + /* rotate the tile dimensions to match the GTT view */
> + pitch_tiles = intel_fb->rotated[i].pitch / tile_height;
> + swap(tile_width, tile_height);
> +
> + /*
> + * We only keep the x/y offsets, so push all of the
> + * gtt offset into the x/y offsets.
> + */
> + intel_adjust_tile_offset(&x, &y, tile_size,
> + tile_width, tile_height, pitch_tiles,
> + gtt_offset_rotated * tile_size, 0);
> +
> + gtt_offset_rotated += rot_info->plane[i].width * rot_info->plane[i].height;
> +
> + /*
> + * First pixel of the framebuffer from
> + * the start of the rotated gtt mapping.
> + */
> + intel_fb->rotated[i].x = x;
> + intel_fb->rotated[i].y = y;
> + } else {
> + size = DIV_ROUND_UP((y + height) * fb->pitches[i] +
> + x * cpp, tile_size);
> + }
> +
> + /* how many tiles in total needed in the bo */
> + max_size = max(max_size, offset + size);
> + }
> +
> + if (max_size * tile_size > to_intel_framebuffer(fb)->obj->base.size) {
> + DRM_DEBUG("fb too big for bo (need %u bytes, have %zu bytes)\n",
> + max_size * tile_size, to_intel_framebuffer(fb)->obj->base.size);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static int i9xx_format_to_fourcc(int format)
> {
> switch (format) {
> @@ -2618,7 +2768,6 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
> u32 dspcntr;
> i915_reg_t reg = DSPCNTR(plane);
> unsigned int rotation = plane_state->base.rotation;
> - int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> int x = plane_state->base.src.x1 >> 16;
> int y = plane_state->base.src.y1 >> 16;
>
> @@ -2677,30 +2826,25 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
> if (IS_G4X(dev))
> dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
>
> - linear_offset = y * fb->pitches[0] + x * cpp;
> + intel_add_fb_offsets(&x, &y, fb, 0, rotation);
>
> - if (INTEL_INFO(dev)->gen >= 4) {
> + if (INTEL_INFO(dev)->gen >= 4)
> intel_crtc->dspaddr_offset =
> intel_compute_tile_offset(&x, &y, fb, 0,
> fb->pitches[0], rotation);
> - linear_offset -= intel_crtc->dspaddr_offset;
> - } else {
> - intel_crtc->dspaddr_offset = linear_offset;
> - }
>
> if (rotation == DRM_ROTATE_180) {
> dspcntr |= DISPPLANE_ROTATE_180;
>
> x += (crtc_state->pipe_src_w - 1);
> y += (crtc_state->pipe_src_h - 1);
> -
> - /* Finding the last pixel of the last line of the display
> - data and adding to linear_offset*/
> - linear_offset +=
> - (crtc_state->pipe_src_h - 1) * fb->pitches[0] +
> - (crtc_state->pipe_src_w - 1) * cpp;
> }
>
> + linear_offset = intel_fb_xy_to_linear(x, y, fb, 0);
> +
> + if (INTEL_INFO(dev)->gen < 4)
> + intel_crtc->dspaddr_offset = linear_offset;
> +
> intel_crtc->adjusted_x = x;
> intel_crtc->adjusted_y = y;
>
> @@ -2709,7 +2853,8 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
> I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
> if (INTEL_INFO(dev)->gen >= 4) {
> I915_WRITE(DSPSURF(plane),
> - i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset);
> + intel_fb_gtt_offset(fb, rotation) +
> + intel_crtc->dspaddr_offset);
> I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
> I915_WRITE(DSPLINOFF(plane), linear_offset);
> } else
> @@ -2747,7 +2892,6 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
> u32 dspcntr;
> i915_reg_t reg = DSPCNTR(plane);
> unsigned int rotation = plane_state->base.rotation;
> - int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> int x = plane_state->base.src.x1 >> 16;
> int y = plane_state->base.src.y1 >> 16;
>
> @@ -2786,26 +2930,23 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
> if (!IS_HASWELL(dev) && !IS_BROADWELL(dev))
> dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
>
> - linear_offset = y * fb->pitches[0] + x * cpp;
> + intel_add_fb_offsets(&x, &y, fb, 0, rotation);
> +
> intel_crtc->dspaddr_offset =
> intel_compute_tile_offset(&x, &y, fb, 0,
> fb->pitches[0], rotation);
> - linear_offset -= intel_crtc->dspaddr_offset;
> +
> if (rotation == DRM_ROTATE_180) {
> dspcntr |= DISPPLANE_ROTATE_180;
>
> if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
> x += (crtc_state->pipe_src_w - 1);
> y += (crtc_state->pipe_src_h - 1);
> -
> - /* Finding the last pixel of the last line of the display
> - data and adding to linear_offset*/
> - linear_offset +=
> - (crtc_state->pipe_src_h - 1) * fb->pitches[0] +
> - (crtc_state->pipe_src_w - 1) * cpp;
> }
> }
>
> + linear_offset = intel_fb_xy_to_linear(x, y, fb, 0);
> +
> intel_crtc->adjusted_x = x;
> intel_crtc->adjusted_y = y;
>
> @@ -2813,7 +2954,8 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
>
> I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
> I915_WRITE(DSPSURF(plane),
> - i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset);
> + intel_fb_gtt_offset(fb, rotation) +
> + intel_crtc->dspaddr_offset);
> if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> I915_WRITE(DSPOFFSET(plane), (y << 16) | x);
> } else {
> @@ -2835,28 +2977,16 @@ u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv,
> }
> }
>
> -u32 intel_plane_obj_offset(struct intel_plane *intel_plane,
> - struct drm_i915_gem_object *obj,
> - unsigned int plane)
> +u32 intel_fb_gtt_offset(struct drm_framebuffer *fb,
> + unsigned int rotation)
> {
> + struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> struct i915_ggtt_view view;
> - struct i915_vma *vma;
> u64 offset;
>
> - intel_fill_fb_ggtt_view(&view, intel_plane->base.state->fb,
> - intel_plane->base.state->rotation);
> -
> - vma = i915_gem_obj_to_ggtt_view(obj, &view);
> - if (WARN(!vma, "ggtt vma for display object not found! (view=%u)\n",
> - view.type))
> - return -1;
> -
> - offset = vma->node.start;
> + intel_fill_fb_ggtt_view(&view, fb, rotation);
>
> - if (plane == 1) {
> - offset += vma->ggtt_view.params.rotated.uv_start_page *
> - PAGE_SIZE;
> - }
> + offset = i915_gem_obj_ggtt_offset_view(obj, &view);
>
> WARN_ON(upper_32_bits(offset));
>
> @@ -2979,12 +3109,9 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> struct drm_framebuffer *fb = plane_state->base.fb;
> - struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> int pipe = intel_crtc->pipe;
> u32 plane_ctl, stride_div, stride;
> - u32 tile_height, plane_offset, plane_size;
> unsigned int rotation = plane_state->base.rotation;
> - int x_offset, y_offset;
> u32 surf_addr;
> int scaler_id = plane_state->scaler_id;
> int src_x = plane_state->base.src.x1 >> 16;
> @@ -3005,36 +3132,49 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
> plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
> plane_ctl |= skl_plane_ctl_rotation(rotation);
>
> - stride_div = intel_fb_stride_alignment(dev_priv, fb->modifier[0],
> - fb->pixel_format);
> - surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj, 0);
> -
> - WARN_ON(drm_rect_width(&plane_state->base.src) == 0);
> -
> 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,
> + };
> int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> + struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
> +
> + /* Rotate src coordinates to match rotated GTT view */
> + drm_rect_rotate(&r, fb->width, fb->height, BIT(DRM_ROTATE_270));
>
> - /* stride = Surface height in tiles */
> - tile_height = intel_tile_height(dev_priv, fb->modifier[0], cpp);
> - stride = DIV_ROUND_UP(fb->height, tile_height);
> - x_offset = stride * tile_height - src_y - src_h;
> - y_offset = src_x;
> - plane_size = (src_w - 1) << 16 | (src_h - 1);
> + src_x = r.x1;
> + src_y = r.y1;
> + src_w = drm_rect_width(&r);
> + src_h = drm_rect_height(&r);
> +
> + stride_div = intel_tile_height(dev_priv, fb->modifier[0], cpp);
> + stride = intel_fb->rotated[0].pitch;
> } else {
> - stride = fb->pitches[0] / stride_div;
> - x_offset = src_x;
> - y_offset = src_y;
> - plane_size = (src_h - 1) << 16 | (src_w - 1);
> + stride_div = intel_fb_stride_alignment(dev_priv, fb->modifier[0],
> + fb->pixel_format);
> + stride = fb->pitches[0];
> }
> - plane_offset = y_offset << 16 | x_offset;
>
> - intel_crtc->adjusted_x = x_offset;
> - intel_crtc->adjusted_y = y_offset;
> + intel_add_fb_offsets(&src_x, &src_y, fb, 0, rotation);
> + surf_addr = intel_compute_tile_offset(&src_x, &src_y, fb, 0,
> + stride, rotation);
> +
> + /* Sizes are 0 based */
> + src_w--;
> + src_h--;
> + dst_w--;
> + dst_h--;
> +
> + intel_crtc->adjusted_x = src_x;
> + intel_crtc->adjusted_y = src_y;
>
> I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> - I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset);
> - I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
> - I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
> + I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x);
> + I915_WRITE(PLANE_STRIDE(pipe, 0), stride / stride_div);
> + I915_WRITE(PLANE_SIZE(pipe, 0), (src_h << 16) | src_w);
>
> if (scaler_id >= 0) {
> uint32_t ps_ctrl = 0;
> @@ -3051,7 +3191,8 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
> I915_WRITE(PLANE_POS(pipe, 0), (dst_y << 16) | dst_x);
> }
>
> - I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
> + I915_WRITE(PLANE_SURF(pipe, 0),
> + intel_fb_gtt_offset(fb, rotation) + surf_addr);
>
> POSTING_READ(PLANE_SURF(pipe, 0));
> }
> @@ -11462,7 +11603,7 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
> const enum pipe pipe = intel_crtc->pipe;
> - u32 ctl, stride, tile_height;
> + u32 ctl, stride;
>
> ctl = I915_READ(PLANE_CTL(pipe, 0));
> ctl &= ~PLANE_CTL_TILED_MASK;
> @@ -11487,9 +11628,11 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
> * linear buffers or in number of tiles for tiled buffers.
> */
> if (intel_rotation_90_or_270(rotation)) {
> - /* stride = Surface height in tiles */
> - tile_height = intel_tile_height(dev_priv, fb->modifier[0], 0);
> - stride = DIV_ROUND_UP(fb->height, tile_height);
> + int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> + struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
> +
> + stride = intel_fb->rotated[0].pitch /
> + intel_tile_height(dev_priv, fb->modifier[0], cpp);
> } else {
> stride = fb->pitches[0] /
> intel_fb_stride_alignment(dev_priv, fb->modifier[0],
> @@ -11769,8 +11912,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> if (ret)
> goto cleanup_pending;
>
> - work->gtt_offset = intel_plane_obj_offset(to_intel_plane(primary),
> - obj, 0);
> + work->gtt_offset = intel_fb_gtt_offset(fb, primary->state->rotation);
> work->gtt_offset += intel_crtc->dspaddr_offset;
> work->rotation = crtc->primary->state->rotation;
>
> @@ -14988,7 +15130,6 @@ static int intel_framebuffer_init(struct drm_device *dev,
> struct drm_i915_gem_object *obj)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> - unsigned int aligned_height;
> int ret;
> u32 pitch_limit, stride_alignment;
>
> @@ -15114,17 +15255,12 @@ static int intel_framebuffer_init(struct drm_device *dev,
> if (mode_cmd->offsets[0] != 0)
> return -EINVAL;
>
> - aligned_height = intel_fb_align_height(dev, mode_cmd->height,
> - mode_cmd->pixel_format,
> - mode_cmd->modifier[0]);
> - /* FIXME drm helper for size checks (especially planar formats)? */
> - if (obj->base.size < aligned_height * mode_cmd->pitches[0])
> - return -EINVAL;
> -
> drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
> intel_fb->obj = obj;
>
> - intel_fill_fb_info(dev_priv, &intel_fb->base);
> + ret = intel_fill_fb_info(dev_priv, &intel_fb->base);
> + if (ret)
> + return ret;
>
> ret = drm_framebuffer_init(dev, &intel_fb->base, &intel_fb_funcs);
> if (ret) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c29a429cbc45..a81d2b8bd740 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -178,6 +178,16 @@ struct intel_framebuffer {
> struct drm_framebuffer base;
> struct drm_i915_gem_object *obj;
> struct intel_rotation_info rot_info;
> +
> + /* for each plane in the normal GTT view */
> + struct {
> + unsigned int x, y;
> + } normal[2];
> + /* for each plane in the rotated GTT view */
> + struct {
> + unsigned int x, y;
> + unsigned int pitch; /* pixels */
> + } rotated[2];
> };
>
> struct intel_fbdev {
> @@ -1153,6 +1163,11 @@ int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
> const char *name, u32 reg, int ref_freq);
> extern const struct drm_plane_funcs intel_plane_funcs;
> void intel_init_display_hooks(struct drm_i915_private *dev_priv);
> +unsigned int intel_fb_xy_to_linear(int x, int y,
> + const struct drm_framebuffer *fb, int plane);
> +void intel_add_fb_offsets(int *x, int *y,
> + const struct drm_framebuffer *fb, int plane,
> + unsigned int rotation);
> unsigned int intel_rotation_info_size(const struct intel_rotation_info *rot_info);
> bool intel_has_pending_fb_unpin(struct drm_device *dev);
> void intel_mark_busy(struct drm_i915_private *dev_priv);
> @@ -1322,9 +1337,7 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
> int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
> int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
>
> -u32 intel_plane_obj_offset(struct intel_plane *intel_plane,
> - struct drm_i915_gem_object *obj,
> - unsigned int plane);
> +u32 intel_fb_gtt_offset(struct drm_framebuffer *fb, unsigned int rotation);
>
> u32 skl_plane_ctl_format(uint32_t pixel_format);
> u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index cbdca7e4d307..ef4d0a6e23af 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -203,15 +203,13 @@ skl_update_plane(struct drm_plane *drm_plane,
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_plane *intel_plane = to_intel_plane(drm_plane);
> struct drm_framebuffer *fb = plane_state->base.fb;
> - struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> + struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
> const int pipe = intel_plane->pipe;
> const int plane = intel_plane->plane + 1;
> u32 plane_ctl, stride_div, stride;
> const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> u32 surf_addr;
> - u32 tile_height, plane_offset, plane_size;
> unsigned int rotation = plane_state->base.rotation;
> - int x_offset, y_offset;
> int crtc_x = plane_state->base.dst.x1;
> int crtc_y = plane_state->base.dst.y1;
> uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
> @@ -230,15 +228,6 @@ skl_update_plane(struct drm_plane *drm_plane,
>
> plane_ctl |= skl_plane_ctl_rotation(rotation);
>
> - stride_div = intel_fb_stride_alignment(dev_priv, fb->modifier[0],
> - fb->pixel_format);
> -
> - /* Sizes are 0 based */
> - src_w--;
> - src_h--;
> - crtc_w--;
> - crtc_h--;
> -
> if (key->flags) {
> I915_WRITE(PLANE_KEYVAL(pipe, plane), key->min_value);
> I915_WRITE(PLANE_KEYMAX(pipe, plane), key->max_value);
> @@ -250,28 +239,44 @@ skl_update_plane(struct drm_plane *drm_plane,
> else if (key->flags & I915_SET_COLORKEY_SOURCE)
> plane_ctl |= PLANE_CTL_KEY_ENABLE_SOURCE;
>
> - surf_addr = intel_plane_obj_offset(intel_plane, obj, 0);
> -
> if (intel_rotation_90_or_270(rotation)) {
> - int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> -
> - /* stride: Surface height in tiles */
> - tile_height = intel_tile_height(dev_priv, fb->modifier[0], cpp);
> - stride = DIV_ROUND_UP(fb->height, tile_height);
> - plane_size = (src_w << 16) | src_h;
> - x_offset = stride * tile_height - y - (src_h + 1);
> - y_offset = x;
> + struct drm_rect r = {
> + .x1 = x,
> + .x2 = x + src_w,
> + .y1 = y,
> + .y2 = y + src_h,
> + };
> + unsigned int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> +
> + /* 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);
> +
> + stride_div = intel_tile_height(dev_priv, fb->modifier[0], cpp);
> + stride = intel_fb->rotated[0].pitch;
> } else {
> - stride = fb->pitches[0] / stride_div;
> - plane_size = (src_h << 16) | src_w;
> - x_offset = x;
> - y_offset = y;
> + stride_div = intel_fb_stride_alignment(dev_priv, fb->modifier[0],
> + fb->pixel_format);
> + stride = fb->pitches[0];
> }
> - plane_offset = y_offset << 16 | x_offset;
>
> - I915_WRITE(PLANE_OFFSET(pipe, plane), plane_offset);
> - I915_WRITE(PLANE_STRIDE(pipe, plane), stride);
> - I915_WRITE(PLANE_SIZE(pipe, plane), plane_size);
> + intel_add_fb_offsets(&x, &y, fb, 0, rotation);
> + surf_addr = intel_compute_tile_offset(&x, &y, fb, 0,
> + stride, rotation);
> +
> + /* Sizes are 0 based */
> + src_w--;
> + src_h--;
> + crtc_w--;
> + crtc_h--;
> +
> + I915_WRITE(PLANE_OFFSET(pipe, plane), (y << 16) | x);
> + I915_WRITE(PLANE_STRIDE(pipe, plane), stride / stride_div);
> + I915_WRITE(PLANE_SIZE(pipe, plane), (src_h << 16) | src_w);
>
> /* program plane scaler */
> if (plane_state->scaler_id >= 0) {
> @@ -296,7 +301,8 @@ skl_update_plane(struct drm_plane *drm_plane,
> }
>
> I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl);
> - I915_WRITE(PLANE_SURF(pipe, plane), surf_addr);
> + I915_WRITE(PLANE_SURF(pipe, plane),
> + intel_fb_gtt_offset(fb, rotation) + surf_addr);
> POSTING_READ(PLANE_SURF(pipe, plane));
> }
>
> @@ -369,7 +375,6 @@ vlv_update_plane(struct drm_plane *dplane,
> u32 sprctl;
> u32 sprsurf_offset, linear_offset;
> unsigned int rotation = dplane->state->rotation;
> - int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> int crtc_x = plane_state->base.dst.x1;
> int crtc_y = plane_state->base.dst.y1;
> @@ -440,19 +445,19 @@ vlv_update_plane(struct drm_plane *dplane,
> crtc_w--;
> crtc_h--;
>
> - linear_offset = y * fb->pitches[0] + x * cpp;
> + intel_add_fb_offsets(&x, &y, fb, 0, rotation);
> sprsurf_offset = intel_compute_tile_offset(&x, &y, fb, 0,
> fb->pitches[0], rotation);
> - linear_offset -= sprsurf_offset;
>
> if (rotation == DRM_ROTATE_180) {
> sprctl |= SP_ROTATE_180;
>
> x += src_w;
> y += src_h;
> - linear_offset += src_h * fb->pitches[0] + src_w * cpp;
> }
>
> + linear_offset = intel_fb_xy_to_linear(x, y, fb, 0);
> +
> if (key->flags) {
> I915_WRITE(SPKEYMINVAL(pipe, plane), key->min_value);
> I915_WRITE(SPKEYMAXVAL(pipe, plane), key->max_value);
> @@ -477,8 +482,8 @@ vlv_update_plane(struct drm_plane *dplane,
>
> I915_WRITE(SPSIZE(pipe, plane), (crtc_h << 16) | crtc_w);
> I915_WRITE(SPCNTR(pipe, plane), sprctl);
> - I915_WRITE(SPSURF(pipe, plane), i915_gem_obj_ggtt_offset(obj) +
> - sprsurf_offset);
> + I915_WRITE(SPSURF(pipe, plane),
> + intel_fb_gtt_offset(fb, rotation) + sprsurf_offset);
> POSTING_READ(SPSURF(pipe, plane));
> }
>
> @@ -511,7 +516,6 @@ ivb_update_plane(struct drm_plane *plane,
> u32 sprctl, sprscale = 0;
> u32 sprsurf_offset, linear_offset;
> unsigned int rotation = plane_state->base.rotation;
> - int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> int crtc_x = plane_state->base.dst.x1;
> int crtc_y = plane_state->base.dst.y1;
> @@ -573,10 +577,9 @@ ivb_update_plane(struct drm_plane *plane,
> if (crtc_w != src_w || crtc_h != src_h)
> sprscale = SPRITE_SCALE_ENABLE | (src_w << 16) | src_h;
>
> - linear_offset = y * fb->pitches[0] + x * cpp;
> + intel_add_fb_offsets(&x, &y, fb, 0, rotation);
> sprsurf_offset = intel_compute_tile_offset(&x, &y, fb, 0,
> fb->pitches[0], rotation);
> - linear_offset -= sprsurf_offset;
>
> if (rotation == DRM_ROTATE_180) {
> sprctl |= SPRITE_ROTATE_180;
> @@ -585,10 +588,11 @@ ivb_update_plane(struct drm_plane *plane,
> if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
> x += src_w;
> y += src_h;
> - linear_offset += src_h * fb->pitches[0] + src_w * cpp;
> }
> }
>
> + linear_offset = intel_fb_xy_to_linear(x, y, fb, 0);
> +
> if (key->flags) {
> I915_WRITE(SPRKEYVAL(pipe), key->min_value);
> I915_WRITE(SPRKEYMAX(pipe), key->max_value);
> @@ -617,7 +621,7 @@ ivb_update_plane(struct drm_plane *plane,
> I915_WRITE(SPRSCALE(pipe), sprscale);
> I915_WRITE(SPRCTL(pipe), sprctl);
> I915_WRITE(SPRSURF(pipe),
> - i915_gem_obj_ggtt_offset(obj) + sprsurf_offset);
> + intel_fb_gtt_offset(fb, rotation) + sprsurf_offset);
> POSTING_READ(SPRSURF(pipe));
> }
>
> @@ -652,7 +656,6 @@ ilk_update_plane(struct drm_plane *plane,
> u32 dvscntr, dvsscale;
> u32 dvssurf_offset, linear_offset;
> unsigned int rotation = plane_state->base.rotation;
> - int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> int crtc_x = plane_state->base.dst.x1;
> int crtc_y = plane_state->base.dst.y1;
> @@ -710,19 +713,19 @@ ilk_update_plane(struct drm_plane *plane,
> if (crtc_w != src_w || crtc_h != src_h)
> dvsscale = DVS_SCALE_ENABLE | (src_w << 16) | src_h;
>
> - linear_offset = y * fb->pitches[0] + x * cpp;
> + intel_add_fb_offsets(&x, &y, fb, 0, rotation);
> dvssurf_offset = intel_compute_tile_offset(&x, &y, fb, 0,
> fb->pitches[0], rotation);
> - linear_offset -= dvssurf_offset;
>
> if (rotation == DRM_ROTATE_180) {
> dvscntr |= DVS_ROTATE_180;
>
> x += src_w;
> y += src_h;
> - linear_offset += src_h * fb->pitches[0] + src_w * cpp;
> }
>
> + linear_offset = intel_fb_xy_to_linear(x, y, fb, 0);
> +
> if (key->flags) {
> I915_WRITE(DVSKEYVAL(pipe), key->min_value);
> I915_WRITE(DVSKEYMAX(pipe), key->max_value);
> @@ -746,7 +749,7 @@ ilk_update_plane(struct drm_plane *plane,
> I915_WRITE(DVSSCALE(pipe), dvsscale);
> I915_WRITE(DVSCNTR(pipe), dvscntr);
> I915_WRITE(DVSSURF(pipe),
> - i915_gem_obj_ggtt_offset(obj) + dvssurf_offset);
> + intel_fb_gtt_offset(fb, rotation) + dvssurf_offset);
> POSTING_READ(DVSSURF(pipe));
> }
>
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3 02/12] drm/i915: Don't pass pitch to intel_compute_page_offset()
2016-08-10 9:23 [PATCH v5 00/12] drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5) ville.syrjala
2016-08-10 9:23 ` [PATCH v6 01/12] drm/i915: Rewrite fb rotation GTT handling ville.syrjala
@ 2016-08-10 9:23 ` ville.syrjala
2016-08-10 9:23 ` [PATCH v2 03/12] drm/i915: Move SKL hw stride calculation into a helper ville.syrjala
` (10 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: ville.syrjala @ 2016-08-10 9:23 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
intel_compute_page_offset() can dig up the correct pitch from the fb
itself, no need for the caller to pass it in.
A bit of extra care is needed for the lower level
_intel_compute_page_offset() since that one gets called before the
rotated pitch under intel_fb is populated. Note that we don't actually
call it with anything but DRM_ROTATE_0 there so we wouldn't actually
look up the rotated pitch there, but still, leave the pitch as something
the caller has to pass to _intel_compute_page_offset() as an
indicator that something is a bit special.
This leaves 'stride_div' in the skl plane update hooks as a mostly useless
variable so just get rid of it.
v2: Add a note why stride_div got nuked
v3: Extract intel_fb_pitch() since it can be useful later
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> (v2)
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 34 ++++++++++++++++++++--------------
drivers/gpu/drm/i915/intel_drv.h | 1 -
drivers/gpu/drm/i915/intel_sprite.c | 26 +++++++++++---------------
3 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f16a59ea7c18..392ee6966f06 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2267,6 +2267,15 @@ void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
i915_gem_object_unpin_from_display_plane(obj, &view);
}
+static int intel_fb_pitch(const struct drm_framebuffer *fb, int plane,
+ unsigned int rotation)
+{
+ if (intel_rotation_90_or_270(rotation))
+ return to_intel_framebuffer(fb)->rotated[plane].pitch;
+ else
+ return fb->pitches[plane];
+}
+
/*
* Convert the x/y offsets into a linear offset.
* Only valid with 0/180 degree rotation, which is fine since linear
@@ -2400,11 +2409,11 @@ static u32 _intel_compute_tile_offset(const struct drm_i915_private *dev_priv,
u32 intel_compute_tile_offset(int *x, int *y,
const struct drm_framebuffer *fb, int plane,
- unsigned int pitch,
unsigned int rotation)
{
const struct drm_i915_private *dev_priv = to_i915(fb->dev);
u32 alignment = intel_surf_alignment(dev_priv, fb->modifier[plane]);
+ int pitch = intel_fb_pitch(fb, plane, rotation);
return _intel_compute_tile_offset(dev_priv, x, y, fb, plane, pitch,
rotation, alignment);
@@ -2830,8 +2839,7 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
if (INTEL_INFO(dev)->gen >= 4)
intel_crtc->dspaddr_offset =
- intel_compute_tile_offset(&x, &y, fb, 0,
- fb->pitches[0], rotation);
+ intel_compute_tile_offset(&x, &y, fb, 0, rotation);
if (rotation == DRM_ROTATE_180) {
dspcntr |= DISPPLANE_ROTATE_180;
@@ -2933,8 +2941,7 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
intel_add_fb_offsets(&x, &y, fb, 0, rotation);
intel_crtc->dspaddr_offset =
- intel_compute_tile_offset(&x, &y, fb, 0,
- fb->pitches[0], rotation);
+ intel_compute_tile_offset(&x, &y, fb, 0, rotation);
if (rotation == DRM_ROTATE_180) {
dspcntr |= DISPPLANE_ROTATE_180;
@@ -3110,7 +3117,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_framebuffer *fb = plane_state->base.fb;
int pipe = intel_crtc->pipe;
- u32 plane_ctl, stride_div, stride;
+ u32 plane_ctl, stride;
unsigned int rotation = plane_state->base.rotation;
u32 surf_addr;
int scaler_id = plane_state->scaler_id;
@@ -3150,17 +3157,16 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
src_w = drm_rect_width(&r);
src_h = drm_rect_height(&r);
- stride_div = intel_tile_height(dev_priv, fb->modifier[0], cpp);
- stride = intel_fb->rotated[0].pitch;
+ stride = intel_fb->rotated[0].pitch /
+ intel_tile_height(dev_priv, fb->modifier[0], cpp);
} else {
- stride_div = intel_fb_stride_alignment(dev_priv, fb->modifier[0],
- fb->pixel_format);
- stride = fb->pitches[0];
+ stride = fb->pitches[0] /
+ intel_fb_stride_alignment(dev_priv, fb->modifier[0],
+ fb->pixel_format);
}
intel_add_fb_offsets(&src_x, &src_y, fb, 0, rotation);
- surf_addr = intel_compute_tile_offset(&src_x, &src_y, fb, 0,
- stride, rotation);
+ surf_addr = intel_compute_tile_offset(&src_x, &src_y, fb, 0, rotation);
/* Sizes are 0 based */
src_w--;
@@ -3173,7 +3179,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x);
- I915_WRITE(PLANE_STRIDE(pipe, 0), stride / stride_div);
+ I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
I915_WRITE(PLANE_SIZE(pipe, 0), (src_h << 16) | src_w);
if (scaler_id >= 0) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a81d2b8bd740..381194d04299 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1293,7 +1293,6 @@ void assert_pipe(struct drm_i915_private *dev_priv, enum pipe pipe, bool state);
#define assert_pipe_disabled(d, p) assert_pipe(d, p, false)
u32 intel_compute_tile_offset(int *x, int *y,
const struct drm_framebuffer *fb, int plane,
- unsigned int pitch,
unsigned int rotation);
void intel_prepare_reset(struct drm_i915_private *dev_priv);
void intel_finish_reset(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index ef4d0a6e23af..e2b575877cdd 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -206,7 +206,7 @@ skl_update_plane(struct drm_plane *drm_plane,
struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
const int pipe = intel_plane->pipe;
const int plane = intel_plane->plane + 1;
- u32 plane_ctl, stride_div, stride;
+ u32 plane_ctl, stride;
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
u32 surf_addr;
unsigned int rotation = plane_state->base.rotation;
@@ -256,17 +256,16 @@ skl_update_plane(struct drm_plane *drm_plane,
src_w = drm_rect_width(&r);
src_h = drm_rect_height(&r);
- stride_div = intel_tile_height(dev_priv, fb->modifier[0], cpp);
- stride = intel_fb->rotated[0].pitch;
+ stride = intel_fb->rotated[0].pitch /
+ intel_tile_height(dev_priv, fb->modifier[0], cpp);
} else {
- stride_div = intel_fb_stride_alignment(dev_priv, fb->modifier[0],
- fb->pixel_format);
- stride = fb->pitches[0];
+ stride = fb->pitches[0] /
+ intel_fb_stride_alignment(dev_priv, fb->modifier[0],
+ fb->pixel_format);
}
intel_add_fb_offsets(&x, &y, fb, 0, rotation);
- surf_addr = intel_compute_tile_offset(&x, &y, fb, 0,
- stride, rotation);
+ surf_addr = intel_compute_tile_offset(&x, &y, fb, 0, rotation);
/* Sizes are 0 based */
src_w--;
@@ -275,7 +274,7 @@ skl_update_plane(struct drm_plane *drm_plane,
crtc_h--;
I915_WRITE(PLANE_OFFSET(pipe, plane), (y << 16) | x);
- I915_WRITE(PLANE_STRIDE(pipe, plane), stride / stride_div);
+ I915_WRITE(PLANE_STRIDE(pipe, plane), stride);
I915_WRITE(PLANE_SIZE(pipe, plane), (src_h << 16) | src_w);
/* program plane scaler */
@@ -446,8 +445,7 @@ vlv_update_plane(struct drm_plane *dplane,
crtc_h--;
intel_add_fb_offsets(&x, &y, fb, 0, rotation);
- sprsurf_offset = intel_compute_tile_offset(&x, &y, fb, 0,
- fb->pitches[0], rotation);
+ sprsurf_offset = intel_compute_tile_offset(&x, &y, fb, 0, rotation);
if (rotation == DRM_ROTATE_180) {
sprctl |= SP_ROTATE_180;
@@ -578,8 +576,7 @@ ivb_update_plane(struct drm_plane *plane,
sprscale = SPRITE_SCALE_ENABLE | (src_w << 16) | src_h;
intel_add_fb_offsets(&x, &y, fb, 0, rotation);
- sprsurf_offset = intel_compute_tile_offset(&x, &y, fb, 0,
- fb->pitches[0], rotation);
+ sprsurf_offset = intel_compute_tile_offset(&x, &y, fb, 0, rotation);
if (rotation == DRM_ROTATE_180) {
sprctl |= SPRITE_ROTATE_180;
@@ -714,8 +711,7 @@ ilk_update_plane(struct drm_plane *plane,
dvsscale = DVS_SCALE_ENABLE | (src_w << 16) | src_h;
intel_add_fb_offsets(&x, &y, fb, 0, rotation);
- dvssurf_offset = intel_compute_tile_offset(&x, &y, fb, 0,
- fb->pitches[0], rotation);
+ dvssurf_offset = intel_compute_tile_offset(&x, &y, fb, 0, rotation);
if (rotation == DRM_ROTATE_180) {
dvscntr |= DVS_ROTATE_180;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 03/12] drm/i915: Move SKL hw stride calculation into a helper
2016-08-10 9:23 [PATCH v5 00/12] drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5) ville.syrjala
2016-08-10 9:23 ` [PATCH v6 01/12] drm/i915: Rewrite fb rotation GTT handling ville.syrjala
2016-08-10 9:23 ` [PATCH v3 02/12] drm/i915: Don't pass pitch to intel_compute_page_offset() ville.syrjala
@ 2016-08-10 9:23 ` ville.syrjala
2016-08-10 9:23 ` [PATCH 04/12] drm/i915: Pass around plane_state instead of fb+rotation ville.syrjala
` (9 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: ville.syrjala @ 2016-08-10 9:23 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
We repeat the SKL stride register value calculations a several places.
Move it into a small helper function.
v2: Rebase due to drm_plane_state src/dst rects
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 52 +++++++++++++++++-------------------
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_sprite.c | 12 ++-------
3 files changed, 29 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 392ee6966f06..e018e144f4b3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3027,6 +3027,28 @@ static void skl_detach_scalers(struct intel_crtc *intel_crtc)
}
}
+u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
+ unsigned int rotation)
+{
+ const struct drm_i915_private *dev_priv = to_i915(fb->dev);
+ u32 stride = intel_fb_pitch(fb, plane, rotation);
+
+ /*
+ * The stride is either expressed as a multiple of 64 bytes chunks for
+ * linear buffers or in number of tiles for tiled buffers.
+ */
+ if (intel_rotation_90_or_270(rotation)) {
+ int cpp = drm_format_plane_cpp(fb->pixel_format, plane);
+
+ stride /= intel_tile_height(dev_priv, fb->modifier[0], cpp);
+ } else {
+ stride /= intel_fb_stride_alignment(dev_priv, fb->modifier[0],
+ fb->pixel_format);
+ }
+
+ return stride;
+}
+
u32 skl_plane_ctl_format(uint32_t pixel_format)
{
switch (pixel_format) {
@@ -3117,8 +3139,9 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_framebuffer *fb = plane_state->base.fb;
int pipe = intel_crtc->pipe;
- u32 plane_ctl, stride;
+ u32 plane_ctl;
unsigned int rotation = plane_state->base.rotation;
+ u32 stride = skl_plane_stride(fb, 0, rotation);
u32 surf_addr;
int scaler_id = plane_state->scaler_id;
int src_x = plane_state->base.src.x1 >> 16;
@@ -3146,8 +3169,6 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
.y1 = src_y,
.y2 = src_y + src_h,
};
- int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
- struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
/* Rotate src coordinates to match rotated GTT view */
drm_rect_rotate(&r, fb->width, fb->height, BIT(DRM_ROTATE_270));
@@ -3156,13 +3177,6 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
src_y = r.y1;
src_w = drm_rect_width(&r);
src_h = drm_rect_height(&r);
-
- stride = intel_fb->rotated[0].pitch /
- intel_tile_height(dev_priv, fb->modifier[0], cpp);
- } else {
- stride = fb->pitches[0] /
- intel_fb_stride_alignment(dev_priv, fb->modifier[0],
- fb->pixel_format);
}
intel_add_fb_offsets(&src_x, &src_y, fb, 0, rotation);
@@ -11609,7 +11623,7 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
struct drm_i915_private *dev_priv = to_i915(dev);
struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
const enum pipe pipe = intel_crtc->pipe;
- u32 ctl, stride;
+ u32 ctl, stride = skl_plane_stride(fb, 0, rotation);
ctl = I915_READ(PLANE_CTL(pipe, 0));
ctl &= ~PLANE_CTL_TILED_MASK;
@@ -11630,22 +11644,6 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
}
/*
- * The stride is either expressed as a multiple of 64 bytes chunks for
- * linear buffers or in number of tiles for tiled buffers.
- */
- if (intel_rotation_90_or_270(rotation)) {
- int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
- struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
-
- stride = intel_fb->rotated[0].pitch /
- intel_tile_height(dev_priv, fb->modifier[0], cpp);
- } else {
- stride = fb->pitches[0] /
- intel_fb_stride_alignment(dev_priv, fb->modifier[0],
- fb->pixel_format);
- }
-
- /*
* Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
* PLANE_SURF updates, the update is then guaranteed to be atomic.
*/
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 381194d04299..0a5b285f3c9d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1341,6 +1341,8 @@ u32 intel_fb_gtt_offset(struct drm_framebuffer *fb, unsigned int rotation);
u32 skl_plane_ctl_format(uint32_t pixel_format);
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);
/* 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 e2b575877cdd..86b5acaf8bb9 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -203,13 +203,13 @@ skl_update_plane(struct drm_plane *drm_plane,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_plane *intel_plane = to_intel_plane(drm_plane);
struct drm_framebuffer *fb = plane_state->base.fb;
- struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
const int pipe = intel_plane->pipe;
const int plane = intel_plane->plane + 1;
- u32 plane_ctl, stride;
+ u32 plane_ctl;
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
u32 surf_addr;
unsigned int rotation = plane_state->base.rotation;
+ u32 stride = skl_plane_stride(fb, 0, rotation);
int crtc_x = plane_state->base.dst.x1;
int crtc_y = plane_state->base.dst.y1;
uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
@@ -246,7 +246,6 @@ skl_update_plane(struct drm_plane *drm_plane,
.y1 = y,
.y2 = y + src_h,
};
- unsigned int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
/* Rotate src coordinates to match rotated GTT view */
drm_rect_rotate(&r, fb->width, fb->height, BIT(DRM_ROTATE_270));
@@ -255,13 +254,6 @@ skl_update_plane(struct drm_plane *drm_plane,
y = r.y1;
src_w = drm_rect_width(&r);
src_h = drm_rect_height(&r);
-
- stride = intel_fb->rotated[0].pitch /
- intel_tile_height(dev_priv, fb->modifier[0], cpp);
- } else {
- stride = fb->pitches[0] /
- intel_fb_stride_alignment(dev_priv, fb->modifier[0],
- fb->pixel_format);
}
intel_add_fb_offsets(&x, &y, fb, 0, rotation);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 04/12] drm/i915: Pass around plane_state instead of fb+rotation
2016-08-10 9:23 [PATCH v5 00/12] drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5) ville.syrjala
` (2 preceding siblings ...)
2016-08-10 9:23 ` [PATCH v2 03/12] drm/i915: Move SKL hw stride calculation into a helper ville.syrjala
@ 2016-08-10 9:23 ` ville.syrjala
2016-08-10 9:23 ` [PATCH v4 05/12] drm/i915: Use fb modifiers for display tiling decisions ville.syrjala
` (8 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: ville.syrjala @ 2016-08-10 9:23 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
intel_compute_tile_offset() and intel_add_fb_offsets() get passed the fb
and the rotation. As both of those come from the plane state we can just
pass that in instead.
For extra consitency pass the plane state to intel_fb_xy_to_linear() as
well even though it only really needs the fb.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 35 ++++++++++++++++++++---------------
drivers/gpu/drm/i915/intel_drv.h | 9 ++++-----
drivers/gpu/drm/i915/intel_sprite.c | 22 +++++++++++-----------
3 files changed, 35 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e018e144f4b3..8122e4768a48 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2283,8 +2283,10 @@ static int intel_fb_pitch(const struct drm_framebuffer *fb, int plane,
* with gen2/3, and 90/270 degree rotations isn't supported on any of them.
*/
u32 intel_fb_xy_to_linear(int x, int y,
- const struct drm_framebuffer *fb, int plane)
+ const struct intel_plane_state *state,
+ int plane)
{
+ const struct drm_framebuffer *fb = state->base.fb;
unsigned int cpp = drm_format_plane_cpp(fb->pixel_format, plane);
unsigned int pitch = fb->pitches[plane];
@@ -2297,11 +2299,12 @@ u32 intel_fb_xy_to_linear(int x, int y,
* specify the start of scanout from the beginning of the gtt mapping.
*/
void intel_add_fb_offsets(int *x, int *y,
- const struct drm_framebuffer *fb, int plane,
- unsigned int rotation)
+ const struct intel_plane_state *state,
+ int plane)
{
- const struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+ const struct intel_framebuffer *intel_fb = to_intel_framebuffer(state->base.fb);
+ unsigned int rotation = state->base.rotation;
if (intel_rotation_90_or_270(rotation)) {
*x += intel_fb->rotated[plane].x;
@@ -2408,10 +2411,12 @@ static u32 _intel_compute_tile_offset(const struct drm_i915_private *dev_priv,
}
u32 intel_compute_tile_offset(int *x, int *y,
- const struct drm_framebuffer *fb, int plane,
- unsigned int rotation)
+ const struct intel_plane_state *state,
+ int plane)
{
- const struct drm_i915_private *dev_priv = to_i915(fb->dev);
+ const struct drm_i915_private *dev_priv = to_i915(state->base.plane->dev);
+ const struct drm_framebuffer *fb = state->base.fb;
+ unsigned int rotation = state->base.rotation;
u32 alignment = intel_surf_alignment(dev_priv, fb->modifier[plane]);
int pitch = intel_fb_pitch(fb, plane, rotation);
@@ -2835,11 +2840,11 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
if (IS_G4X(dev))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
- intel_add_fb_offsets(&x, &y, fb, 0, rotation);
+ intel_add_fb_offsets(&x, &y, plane_state, 0);
if (INTEL_INFO(dev)->gen >= 4)
intel_crtc->dspaddr_offset =
- intel_compute_tile_offset(&x, &y, fb, 0, rotation);
+ intel_compute_tile_offset(&x, &y, plane_state, 0);
if (rotation == DRM_ROTATE_180) {
dspcntr |= DISPPLANE_ROTATE_180;
@@ -2848,7 +2853,7 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
y += (crtc_state->pipe_src_h - 1);
}
- linear_offset = intel_fb_xy_to_linear(x, y, fb, 0);
+ linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
if (INTEL_INFO(dev)->gen < 4)
intel_crtc->dspaddr_offset = linear_offset;
@@ -2938,10 +2943,10 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
if (!IS_HASWELL(dev) && !IS_BROADWELL(dev))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
- intel_add_fb_offsets(&x, &y, fb, 0, rotation);
+ intel_add_fb_offsets(&x, &y, plane_state, 0);
intel_crtc->dspaddr_offset =
- intel_compute_tile_offset(&x, &y, fb, 0, rotation);
+ intel_compute_tile_offset(&x, &y, plane_state, 0);
if (rotation == DRM_ROTATE_180) {
dspcntr |= DISPPLANE_ROTATE_180;
@@ -2952,7 +2957,7 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
}
}
- linear_offset = intel_fb_xy_to_linear(x, y, fb, 0);
+ linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
intel_crtc->adjusted_x = x;
intel_crtc->adjusted_y = y;
@@ -3179,8 +3184,8 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
src_h = drm_rect_height(&r);
}
- intel_add_fb_offsets(&src_x, &src_y, fb, 0, rotation);
- surf_addr = intel_compute_tile_offset(&src_x, &src_y, fb, 0, rotation);
+ 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--;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0a5b285f3c9d..3abdc817e99d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1164,10 +1164,10 @@ int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
extern const struct drm_plane_funcs intel_plane_funcs;
void intel_init_display_hooks(struct drm_i915_private *dev_priv);
unsigned int intel_fb_xy_to_linear(int x, int y,
- const struct drm_framebuffer *fb, int plane);
+ const struct intel_plane_state *state,
+ int plane);
void intel_add_fb_offsets(int *x, int *y,
- const struct drm_framebuffer *fb, int plane,
- unsigned int rotation);
+ const struct intel_plane_state *state, int plane);
unsigned int intel_rotation_info_size(const struct intel_rotation_info *rot_info);
bool intel_has_pending_fb_unpin(struct drm_device *dev);
void intel_mark_busy(struct drm_i915_private *dev_priv);
@@ -1292,8 +1292,7 @@ void assert_pipe(struct drm_i915_private *dev_priv, enum pipe pipe, bool state);
#define assert_pipe_enabled(d, p) assert_pipe(d, p, true)
#define assert_pipe_disabled(d, p) assert_pipe(d, p, false)
u32 intel_compute_tile_offset(int *x, int *y,
- const struct drm_framebuffer *fb, int plane,
- unsigned int rotation);
+ const struct intel_plane_state *state, int plane);
void intel_prepare_reset(struct drm_i915_private *dev_priv);
void intel_finish_reset(struct drm_i915_private *dev_priv);
void hsw_enable_pc8(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 86b5acaf8bb9..0e7d26003a8d 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -256,8 +256,8 @@ skl_update_plane(struct drm_plane *drm_plane,
src_h = drm_rect_height(&r);
}
- intel_add_fb_offsets(&x, &y, fb, 0, rotation);
- surf_addr = intel_compute_tile_offset(&x, &y, fb, 0, rotation);
+ 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--;
@@ -436,8 +436,8 @@ vlv_update_plane(struct drm_plane *dplane,
crtc_w--;
crtc_h--;
- intel_add_fb_offsets(&x, &y, fb, 0, rotation);
- sprsurf_offset = intel_compute_tile_offset(&x, &y, fb, 0, rotation);
+ intel_add_fb_offsets(&x, &y, plane_state, 0);
+ sprsurf_offset = intel_compute_tile_offset(&x, &y, plane_state, 0);
if (rotation == DRM_ROTATE_180) {
sprctl |= SP_ROTATE_180;
@@ -446,7 +446,7 @@ vlv_update_plane(struct drm_plane *dplane,
y += src_h;
}
- linear_offset = intel_fb_xy_to_linear(x, y, fb, 0);
+ linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
if (key->flags) {
I915_WRITE(SPKEYMINVAL(pipe, plane), key->min_value);
@@ -567,8 +567,8 @@ ivb_update_plane(struct drm_plane *plane,
if (crtc_w != src_w || crtc_h != src_h)
sprscale = SPRITE_SCALE_ENABLE | (src_w << 16) | src_h;
- intel_add_fb_offsets(&x, &y, fb, 0, rotation);
- sprsurf_offset = intel_compute_tile_offset(&x, &y, fb, 0, rotation);
+ intel_add_fb_offsets(&x, &y, plane_state, 0);
+ sprsurf_offset = intel_compute_tile_offset(&x, &y, plane_state, 0);
if (rotation == DRM_ROTATE_180) {
sprctl |= SPRITE_ROTATE_180;
@@ -580,7 +580,7 @@ ivb_update_plane(struct drm_plane *plane,
}
}
- linear_offset = intel_fb_xy_to_linear(x, y, fb, 0);
+ linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
if (key->flags) {
I915_WRITE(SPRKEYVAL(pipe), key->min_value);
@@ -702,8 +702,8 @@ ilk_update_plane(struct drm_plane *plane,
if (crtc_w != src_w || crtc_h != src_h)
dvsscale = DVS_SCALE_ENABLE | (src_w << 16) | src_h;
- intel_add_fb_offsets(&x, &y, fb, 0, rotation);
- dvssurf_offset = intel_compute_tile_offset(&x, &y, fb, 0, rotation);
+ intel_add_fb_offsets(&x, &y, plane_state, 0);
+ dvssurf_offset = intel_compute_tile_offset(&x, &y, plane_state, 0);
if (rotation == DRM_ROTATE_180) {
dvscntr |= DVS_ROTATE_180;
@@ -712,7 +712,7 @@ ilk_update_plane(struct drm_plane *plane,
y += src_h;
}
- linear_offset = intel_fb_xy_to_linear(x, y, fb, 0);
+ linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
if (key->flags) {
I915_WRITE(DVSKEYVAL(pipe), key->min_value);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v4 05/12] drm/i915: Use fb modifiers for display tiling decisions
2016-08-10 9:23 [PATCH v5 00/12] drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5) ville.syrjala
` (3 preceding siblings ...)
2016-08-10 9:23 ` [PATCH 04/12] drm/i915: Pass around plane_state instead of fb+rotation ville.syrjala
@ 2016-08-10 9:23 ` ville.syrjala
2016-08-10 9:23 ` [PATCH v3 06/12] drm/i915: Adjust obj tiling vs. fb modifier rules ville.syrjala
` (7 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: ville.syrjala @ 2016-08-10 9:23 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Soon the fence tiling mode may not always match the fb modifier
even for X tiled buffers. So let's use the fb modifier
consistently for all display tiling decisions.
v2: Rebased due to s/ring/engine/
v3: Rebased due to s/engine/ring/ O_o
v4: Rebase due to i915_gem_object_get_tiling() & co.
Reviewed-by: Matthew Auld <matthew.auld@intel.com> (v1)
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++++++++------------
drivers/gpu/drm/i915/intel_sprite.c | 15 ++++++---------
2 files changed, 29 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8122e4768a48..d8855ba969be 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2436,6 +2436,18 @@ static void intel_fb_offset_to_xy(int *x, int *y,
*x = linear_offset % pitch / cpp;
}
+static unsigned int intel_fb_modifier_to_tiling(uint64_t fb_modifier)
+{
+ switch (fb_modifier) {
+ case I915_FORMAT_MOD_X_TILED:
+ return I915_TILING_X;
+ case I915_FORMAT_MOD_Y_TILED:
+ return I915_TILING_Y;
+ default:
+ return I915_TILING_NONE;
+ }
+}
+
static int
intel_fill_fb_info(struct drm_i915_private *dev_priv,
struct drm_framebuffer *fb)
@@ -2834,7 +2846,8 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
BUG();
}
- if (INTEL_INFO(dev)->gen >= 4 && i915_gem_object_is_tiled(obj))
+ if (INTEL_GEN(dev_priv) >= 4 &&
+ fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
dspcntr |= DISPPLANE_TILED;
if (IS_G4X(dev))
@@ -2899,7 +2912,6 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_framebuffer *fb = plane_state->base.fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
int plane = intel_crtc->plane;
u32 linear_offset;
u32 dspcntr;
@@ -2937,7 +2949,7 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
BUG();
}
- if (i915_gem_object_is_tiled(obj))
+ if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
dspcntr |= DISPPLANE_TILED;
if (!IS_HASWELL(dev) && !IS_BROADWELL(dev))
@@ -11444,7 +11456,7 @@ static int intel_gen4_queue_flip(struct drm_device *dev,
MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
intel_ring_emit(ring, fb->pitches[0]);
intel_ring_emit(ring, intel_crtc->flip_work->gtt_offset |
- i915_gem_object_get_tiling(obj));
+ intel_fb_modifier_to_tiling(fb->modifier[0]));
/* XXX Enabling the panel-fitter across page-flip is so far
* untested on non-native modes, so ignore it for now.
@@ -11476,7 +11488,8 @@ static int intel_gen6_queue_flip(struct drm_device *dev,
intel_ring_emit(ring, MI_DISPLAY_FLIP |
MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
- intel_ring_emit(ring, fb->pitches[0] | i915_gem_object_get_tiling(obj));
+ intel_ring_emit(ring, fb->pitches[0] |
+ intel_fb_modifier_to_tiling(fb->modifier[0]));
intel_ring_emit(ring, intel_crtc->flip_work->gtt_offset);
/* Contrary to the suggestions in the documentation,
@@ -11579,7 +11592,8 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
}
intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane_bit);
- intel_ring_emit(ring, fb->pitches[0] | i915_gem_object_get_tiling(obj));
+ intel_ring_emit(ring, fb->pitches[0] |
+ intel_fb_modifier_to_tiling(fb->modifier[0]));
intel_ring_emit(ring, intel_crtc->flip_work->gtt_offset);
intel_ring_emit(ring, (MI_NOOP));
@@ -11664,15 +11678,13 @@ static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc,
{
struct drm_device *dev = intel_crtc->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_framebuffer *intel_fb =
- to_intel_framebuffer(intel_crtc->base.primary->fb);
- struct drm_i915_gem_object *obj = intel_fb->obj;
+ struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
i915_reg_t reg = DSPCNTR(intel_crtc->plane);
u32 dspcntr;
dspcntr = I915_READ(reg);
- if (i915_gem_object_is_tiled(obj))
+ if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
dspcntr |= DISPPLANE_TILED;
else
dspcntr &= ~DISPPLANE_TILED;
@@ -11900,8 +11912,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
engine = &dev_priv->engine[BCS];
- if (i915_gem_object_get_tiling(obj) !=
- i915_gem_object_get_tiling(intel_fb_obj(work->old_fb)))
+ if (fb->modifier[0] != old_fb->modifier[0])
/* vlv: DISPLAY_FLIP fails to change tiling */
engine = NULL;
} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 0e7d26003a8d..4d5ba488a9f4 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -360,7 +360,6 @@ vlv_update_plane(struct drm_plane *dplane,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_plane *intel_plane = to_intel_plane(dplane);
struct drm_framebuffer *fb = plane_state->base.fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
int pipe = intel_plane->pipe;
int plane = intel_plane->plane;
u32 sprctl;
@@ -427,7 +426,7 @@ vlv_update_plane(struct drm_plane *dplane,
*/
sprctl |= SP_GAMMA_ENABLE;
- if (i915_gem_object_is_tiled(obj))
+ if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
sprctl |= SP_TILED;
/* Sizes are 0 based */
@@ -463,7 +462,7 @@ vlv_update_plane(struct drm_plane *dplane,
I915_WRITE(SPSTRIDE(pipe, plane), fb->pitches[0]);
I915_WRITE(SPPOS(pipe, plane), (crtc_y << 16) | crtc_x);
- if (i915_gem_object_is_tiled(obj))
+ if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
I915_WRITE(SPTILEOFF(pipe, plane), (y << 16) | x);
else
I915_WRITE(SPLINOFF(pipe, plane), linear_offset);
@@ -501,7 +500,6 @@ ivb_update_plane(struct drm_plane *plane,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_framebuffer *fb = plane_state->base.fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
enum pipe pipe = intel_plane->pipe;
u32 sprctl, sprscale = 0;
u32 sprsurf_offset, linear_offset;
@@ -547,7 +545,7 @@ ivb_update_plane(struct drm_plane *plane,
*/
sprctl |= SPRITE_GAMMA_ENABLE;
- if (i915_gem_object_is_tiled(obj))
+ if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
sprctl |= SPRITE_TILED;
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
@@ -600,7 +598,7 @@ ivb_update_plane(struct drm_plane *plane,
* register */
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
I915_WRITE(SPROFFSET(pipe), (y << 16) | x);
- else if (i915_gem_object_is_tiled(obj))
+ else if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
I915_WRITE(SPRTILEOFF(pipe), (y << 16) | x);
else
I915_WRITE(SPRLINOFF(pipe), linear_offset);
@@ -640,7 +638,6 @@ ilk_update_plane(struct drm_plane *plane,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_framebuffer *fb = plane_state->base.fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
int pipe = intel_plane->pipe;
u32 dvscntr, dvsscale;
u32 dvssurf_offset, linear_offset;
@@ -686,7 +683,7 @@ ilk_update_plane(struct drm_plane *plane,
*/
dvscntr |= DVS_GAMMA_ENABLE;
- if (i915_gem_object_is_tiled(obj))
+ if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
dvscntr |= DVS_TILED;
if (IS_GEN6(dev))
@@ -728,7 +725,7 @@ ilk_update_plane(struct drm_plane *plane,
I915_WRITE(DVSSTRIDE(pipe), fb->pitches[0]);
I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x);
- if (i915_gem_object_is_tiled(obj))
+ if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
I915_WRITE(DVSTILEOFF(pipe), (y << 16) | x);
else
I915_WRITE(DVSLINOFF(pipe), linear_offset);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v3 06/12] drm/i915: Adjust obj tiling vs. fb modifier rules
2016-08-10 9:23 [PATCH v5 00/12] drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5) ville.syrjala
` (4 preceding siblings ...)
2016-08-10 9:23 ` [PATCH v4 05/12] drm/i915: Use fb modifiers for display tiling decisions ville.syrjala
@ 2016-08-10 9:23 ` ville.syrjala
2016-08-10 9:43 ` Daniel Vetter
2016-08-10 9:23 ` [PATCH v2 07/12] drm/i915: Limit fb x offset due to fences ville.syrjala
` (6 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: ville.syrjala @ 2016-08-10 9:23 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Currently we require the object to be X tiled if the fb is X
tiled. The argument is supposedly FBC GTT tracking. But
actually that no longer holds water since FBC supports
Y tiling as well on SKL+.
A better rule IMO is to require that if there is a fence, the
fb modifier match the object tiling mode. But if the object is linear,
we can allow the fb modifier to be anything. The idea being that
if the user set the tiling mode on the object, presumably the intention
is to actually use the fence for CPU access. But if the tiling mode is
not set, the user has no intention of using a fence (and can't actually
since we disallow tiling mode changes when there are framebuffers
associated with the object).
On gen2/3 we must keep to the rule that the object and fb
must be either both linear or both X tiled. No mixing allowed
since the display engine itself will use the fence if it's present.
v2: Fix typos
v3: Rebase due to i915_gem_object_get_tiling() & co.
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d8855ba969be..ad9f8b303fbc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15150,23 +15150,26 @@ static int intel_framebuffer_init(struct drm_device *dev,
struct drm_i915_gem_object *obj)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ unsigned int tiling = i915_gem_object_get_tiling(obj);
int ret;
u32 pitch_limit, stride_alignment;
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
- /* Enforce that fb modifier and tiling mode match, but only for
- * X-tiled. This is needed for FBC. */
- if (!!(i915_gem_object_get_tiling(obj) == I915_TILING_X) !=
- !!(mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED)) {
+ /*
+ * If there's a fence, enforce that
+ * the fb modifier and tiling mode match.
+ */
+ if (tiling != I915_TILING_NONE &&
+ tiling != intel_fb_modifier_to_tiling(mode_cmd->modifier[0])) {
DRM_DEBUG("tiling_mode doesn't match fb modifier\n");
return -EINVAL;
}
} else {
- if (i915_gem_object_get_tiling(obj) == I915_TILING_X)
+ if (tiling == I915_TILING_X) {
mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED;
- else if (i915_gem_object_get_tiling(obj) == I915_TILING_Y) {
+ } else if (tiling == I915_TILING_Y) {
DRM_DEBUG("No Y tiling for legacy addfb\n");
return -EINVAL;
}
@@ -15190,6 +15193,16 @@ static int intel_framebuffer_init(struct drm_device *dev,
return -EINVAL;
}
+ /*
+ * gen2/3 display engine uses the fence if present,
+ * so the tiling mode must match the fb modifier exactly.
+ */
+ if (INTEL_INFO(dev_priv)->gen < 4 &&
+ tiling != intel_fb_modifier_to_tiling(mode_cmd->modifier[0])) {
+ DRM_DEBUG("tiling_mode must match fb modifier exactly on gen2/3\n");
+ return -EINVAL;
+ }
+
stride_alignment = intel_fb_stride_alignment(dev_priv,
mode_cmd->modifier[0],
mode_cmd->pixel_format);
@@ -15209,7 +15222,11 @@ static int intel_framebuffer_init(struct drm_device *dev,
return -EINVAL;
}
- if (mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED &&
+ /*
+ * If there's a fence, enforce that
+ * the fb pitch and fence stride match.
+ */
+ if (tiling != I915_TILING_NONE &&
mode_cmd->pitches[0] != i915_gem_object_get_stride(obj)) {
DRM_DEBUG("pitch (%d) must match tiling stride (%d)\n",
mode_cmd->pitches[0],
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v3 06/12] drm/i915: Adjust obj tiling vs. fb modifier rules
2016-08-10 9:23 ` [PATCH v3 06/12] drm/i915: Adjust obj tiling vs. fb modifier rules ville.syrjala
@ 2016-08-10 9:43 ` Daniel Vetter
0 siblings, 0 replies; 27+ messages in thread
From: Daniel Vetter @ 2016-08-10 9:43 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Wed, Aug 10, 2016 at 12:23:15PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently we require the object to be X tiled if the fb is X
> tiled. The argument is supposedly FBC GTT tracking. But
> actually that no longer holds water since FBC supports
> Y tiling as well on SKL+.
>
> A better rule IMO is to require that if there is a fence, the
> fb modifier match the object tiling mode. But if the object is linear,
> we can allow the fb modifier to be anything. The idea being that
> if the user set the tiling mode on the object, presumably the intention
> is to actually use the fence for CPU access. But if the tiling mode is
> not set, the user has no intention of using a fence (and can't actually
> since we disallow tiling mode changes when there are framebuffers
> associated with the object).
>
> On gen2/3 we must keep to the rule that the object and fb
> must be either both linear or both X tiled. No mixing allowed
> since the display engine itself will use the fence if it's present.
>
> v2: Fix typos
> v3: Rebase due to i915_gem_object_get_tiling() & co.
>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
So the reason why I didn't like this is that it makes the fbc checking
more tricky. Someone might think that checking for X-tiling on just the
drm_framebuffer is good enough and then we'd have some potentially funny
bugs. But the fbc code still only looks at the gem bo tiling, so all is
still fine.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++++++++++++++-------
> 1 file changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d8855ba969be..ad9f8b303fbc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15150,23 +15150,26 @@ static int intel_framebuffer_init(struct drm_device *dev,
> struct drm_i915_gem_object *obj)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + unsigned int tiling = i915_gem_object_get_tiling(obj);
> int ret;
> u32 pitch_limit, stride_alignment;
>
> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>
> if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
> - /* Enforce that fb modifier and tiling mode match, but only for
> - * X-tiled. This is needed for FBC. */
> - if (!!(i915_gem_object_get_tiling(obj) == I915_TILING_X) !=
> - !!(mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED)) {
> + /*
> + * If there's a fence, enforce that
> + * the fb modifier and tiling mode match.
> + */
> + if (tiling != I915_TILING_NONE &&
> + tiling != intel_fb_modifier_to_tiling(mode_cmd->modifier[0])) {
> DRM_DEBUG("tiling_mode doesn't match fb modifier\n");
> return -EINVAL;
> }
> } else {
> - if (i915_gem_object_get_tiling(obj) == I915_TILING_X)
> + if (tiling == I915_TILING_X) {
> mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED;
> - else if (i915_gem_object_get_tiling(obj) == I915_TILING_Y) {
> + } else if (tiling == I915_TILING_Y) {
> DRM_DEBUG("No Y tiling for legacy addfb\n");
> return -EINVAL;
> }
> @@ -15190,6 +15193,16 @@ static int intel_framebuffer_init(struct drm_device *dev,
> return -EINVAL;
> }
>
> + /*
> + * gen2/3 display engine uses the fence if present,
> + * so the tiling mode must match the fb modifier exactly.
> + */
> + if (INTEL_INFO(dev_priv)->gen < 4 &&
> + tiling != intel_fb_modifier_to_tiling(mode_cmd->modifier[0])) {
> + DRM_DEBUG("tiling_mode must match fb modifier exactly on gen2/3\n");
> + return -EINVAL;
> + }
> +
> stride_alignment = intel_fb_stride_alignment(dev_priv,
> mode_cmd->modifier[0],
> mode_cmd->pixel_format);
> @@ -15209,7 +15222,11 @@ static int intel_framebuffer_init(struct drm_device *dev,
> return -EINVAL;
> }
>
> - if (mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED &&
> + /*
> + * If there's a fence, enforce that
> + * the fb pitch and fence stride match.
> + */
> + if (tiling != I915_TILING_NONE &&
> mode_cmd->pitches[0] != i915_gem_object_get_stride(obj)) {
> DRM_DEBUG("pitch (%d) must match tiling stride (%d)\n",
> mode_cmd->pitches[0],
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 07/12] drm/i915: Limit fb x offset due to fences
2016-08-10 9:23 [PATCH v5 00/12] drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5) ville.syrjala
` (5 preceding siblings ...)
2016-08-10 9:23 ` [PATCH v3 06/12] drm/i915: Adjust obj tiling vs. fb modifier rules ville.syrjala
@ 2016-08-10 9:23 ` ville.syrjala
2016-08-10 9:23 ` [PATCH 08/12] drm/i915: Allow calling intel_adjust_tile_offset() multiple times ville.syrjala
` (5 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: ville.syrjala @ 2016-08-10 9:23 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
If there's a fence on the object it will be aligned to the start
of the object, and hence CPU rendering to any fb that straddles
the fence edge will come out wrong due to lines wrapping at the
wrong place.
We have no API to manage fences on a sub-object level, so we can't
really fix this in any way. Additonally gen2/3 fences are rather
coarse grained so adjusting the offset migth not even be possible.
Avoid these problems by requiring the fb layout to agree with the
fence layout (if present).
v2: Rebase due to i915_gem_object_get_tiling() & co.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ad9f8b303fbc..f4b45625703e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2473,6 +2473,22 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
intel_fb_offset_to_xy(&x, &y, fb, i);
/*
+ * The fence (if used) is aligned to the start of the object
+ * so having the framebuffer wrap around across the edge of the
+ * fenced region doesn't really work. We have no API to configure
+ * the fence start offset within the object (nor could we probably
+ * on gen2/3). So it's just easier if we just require that the
+ * fb layout agrees with the fence layout. We already check that the
+ * fb stride matches the fence stride elsewhere.
+ */
+ if (i915_gem_object_is_tiled(intel_fb->obj) &&
+ (x + width) * cpp > fb->pitches[i]) {
+ DRM_DEBUG("bad fb plane %d offset: 0x%x\n",
+ i, fb->offsets[i]);
+ return -EINVAL;
+ }
+
+ /*
* First pixel of the framebuffer from
* the start of the normal gtt mapping.
*/
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 08/12] drm/i915: Allow calling intel_adjust_tile_offset() multiple times
2016-08-10 9:23 [PATCH v5 00/12] drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5) ville.syrjala
` (6 preceding siblings ...)
2016-08-10 9:23 ` [PATCH v2 07/12] drm/i915: Limit fb x offset due to fences ville.syrjala
@ 2016-08-10 9:23 ` ville.syrjala
2016-08-10 9:46 ` Chris Wilson
2016-08-10 9:48 ` Daniel Vetter
2016-08-10 9:23 ` [PATCH 09/12] drm/i915: Make intel_adjust_tile_offset() work for linear buffers ville.syrjala
` (4 subsequent siblings)
12 siblings, 2 replies; 27+ messages in thread
From: ville.syrjala @ 2016-08-10 9:23 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Minimize the resulting X coordinate after intel_adjust_tile_offset() is
done with it's offset adjustment. This allows calling
intel_adjust_tile_offset() multiple times in case we need to adjust
the offset several times.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f4b45625703e..40c809593ae9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2330,6 +2330,7 @@ static u32 intel_adjust_tile_offset(int *x, int *y,
u32 old_offset,
u32 new_offset)
{
+ unsigned int pitch_pixels = pitch_tiles * tile_width;
unsigned int tiles;
WARN_ON(old_offset & (tile_size - 1));
@@ -2341,6 +2342,10 @@ static u32 intel_adjust_tile_offset(int *x, int *y,
*y += tiles / pitch_tiles * tile_height;
*x += tiles % pitch_tiles * tile_width;
+ /* minimize x in case it got needlessly big */
+ *y += *x / pitch_pixels * tile_height;
+ *x %= pitch_pixels;
+
return new_offset;
}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 08/12] drm/i915: Allow calling intel_adjust_tile_offset() multiple times
2016-08-10 9:23 ` [PATCH 08/12] drm/i915: Allow calling intel_adjust_tile_offset() multiple times ville.syrjala
@ 2016-08-10 9:46 ` Chris Wilson
2016-08-10 9:48 ` Daniel Vetter
1 sibling, 0 replies; 27+ messages in thread
From: Chris Wilson @ 2016-08-10 9:46 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Wed, Aug 10, 2016 at 12:23:17PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Minimize the resulting X coordinate after intel_adjust_tile_offset() is
> done with it's offset adjustment. This allows calling
> intel_adjust_tile_offset() multiple times in case we need to adjust
> the offset several times.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f4b45625703e..40c809593ae9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2330,6 +2330,7 @@ static u32 intel_adjust_tile_offset(int *x, int *y,
> u32 old_offset,
> u32 new_offset)
> {
> + unsigned int pitch_pixels = pitch_tiles * tile_width;
> unsigned int tiles;
>
> WARN_ON(old_offset & (tile_size - 1));
> @@ -2341,6 +2342,10 @@ static u32 intel_adjust_tile_offset(int *x, int *y,
> *y += tiles / pitch_tiles * tile_height;
> *x += tiles % pitch_tiles * tile_width;
>
> + /* minimize x in case it got needlessly big */
> + *y += *x / pitch_pixels * tile_height;
> + *x %= pitch_pixels;
Multiples of pitch_pixels (entire tile rows) are converted from an X offset
to a Y offset (in tile steps).
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 08/12] drm/i915: Allow calling intel_adjust_tile_offset() multiple times
2016-08-10 9:23 ` [PATCH 08/12] drm/i915: Allow calling intel_adjust_tile_offset() multiple times ville.syrjala
2016-08-10 9:46 ` Chris Wilson
@ 2016-08-10 9:48 ` Daniel Vetter
2016-08-10 11:17 ` Ville Syrjälä
1 sibling, 1 reply; 27+ messages in thread
From: Daniel Vetter @ 2016-08-10 9:48 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Wed, Aug 10, 2016 at 12:23:17PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Minimize the resulting X coordinate after intel_adjust_tile_offset() is
> done with it's offset adjustment. This allows calling
> intel_adjust_tile_offset() multiple times in case we need to adjust
> the offset several times.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Do we check anywhere that y doesn't eventually overflow? Anyway, this
looks good.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_display.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f4b45625703e..40c809593ae9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2330,6 +2330,7 @@ static u32 intel_adjust_tile_offset(int *x, int *y,
> u32 old_offset,
> u32 new_offset)
> {
> + unsigned int pitch_pixels = pitch_tiles * tile_width;
> unsigned int tiles;
>
> WARN_ON(old_offset & (tile_size - 1));
> @@ -2341,6 +2342,10 @@ static u32 intel_adjust_tile_offset(int *x, int *y,
> *y += tiles / pitch_tiles * tile_height;
> *x += tiles % pitch_tiles * tile_width;
>
> + /* minimize x in case it got needlessly big */
> + *y += *x / pitch_pixels * tile_height;
> + *x %= pitch_pixels;
> +
> return new_offset;
> }
>
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 08/12] drm/i915: Allow calling intel_adjust_tile_offset() multiple times
2016-08-10 9:48 ` Daniel Vetter
@ 2016-08-10 11:17 ` Ville Syrjälä
0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2016-08-10 11:17 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Wed, Aug 10, 2016 at 11:48:56AM +0200, Daniel Vetter wrote:
> On Wed, Aug 10, 2016 at 12:23:17PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Minimize the resulting X coordinate after intel_adjust_tile_offset() is
> > done with it's offset adjustment. This allows calling
> > intel_adjust_tile_offset() multiple times in case we need to adjust
> > the offset several times.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Do we check anywhere that y doesn't eventually overflow? Anyway, this
> looks good.
Hmm. I was thinking it's limited by the fb size, but only the plane src
coordinate part is limited like that. The part coming from fb->offsets[]
is extra, so I guess the total might end up too big even for 32 bits.
And the hardware for sure has even lower number of bits available for
the x/y offsets, so we should definitely add some extra checks for
this.
I have a feeling I had some checks like this at some point, but either I
imagined it, or I mistakenly threw them out, or they're in some random
branch I've misplaced.
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index f4b45625703e..40c809593ae9 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2330,6 +2330,7 @@ static u32 intel_adjust_tile_offset(int *x, int *y,
> > u32 old_offset,
> > u32 new_offset)
> > {
> > + unsigned int pitch_pixels = pitch_tiles * tile_width;
> > unsigned int tiles;
> >
> > WARN_ON(old_offset & (tile_size - 1));
> > @@ -2341,6 +2342,10 @@ static u32 intel_adjust_tile_offset(int *x, int *y,
> > *y += tiles / pitch_tiles * tile_height;
> > *x += tiles % pitch_tiles * tile_width;
> >
> > + /* minimize x in case it got needlessly big */
> > + *y += *x / pitch_pixels * tile_height;
> > + *x %= pitch_pixels;
> > +
> > return new_offset;
> > }
> >
> > --
> > 2.7.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 09/12] drm/i915: Make intel_adjust_tile_offset() work for linear buffers
2016-08-10 9:23 [PATCH v5 00/12] drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5) ville.syrjala
` (7 preceding siblings ...)
2016-08-10 9:23 ` [PATCH 08/12] drm/i915: Allow calling intel_adjust_tile_offset() multiple times ville.syrjala
@ 2016-08-10 9:23 ` ville.syrjala
2016-08-10 9:23 ` [PATCH v2 10/12] drm/i915: Compute display surface offset in the plane check hook for SKL+ ville.syrjala
` (3 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: ville.syrjala @ 2016-08-10 9:23 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
To make life less surprising we can make intel_adjust_tile_offset()
deal with linear buffers as well. Currently it doesn't seem like there's
a real need for this since only X tiling and NV12 (which would always
be tiled currently) should need it. But I've used it for some debug
hacks already so seems like a reasonable thing to have.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 73 ++++++++++++++++++++++++++++--------
1 file changed, 57 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 40c809593ae9..8174c43dc8f8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2316,19 +2316,16 @@ void intel_add_fb_offsets(int *x, int *y,
}
/*
- * Adjust the tile offset by moving the difference into
- * the x/y offsets.
- *
* Input tile dimensions and pitch must already be
* rotated to match x and y, and in pixel units.
*/
-static u32 intel_adjust_tile_offset(int *x, int *y,
- unsigned int tile_width,
- unsigned int tile_height,
- unsigned int tile_size,
- unsigned int pitch_tiles,
- u32 old_offset,
- u32 new_offset)
+static u32 _intel_adjust_tile_offset(int *x, int *y,
+ unsigned int tile_width,
+ unsigned int tile_height,
+ unsigned int tile_size,
+ unsigned int pitch_tiles,
+ u32 old_offset,
+ u32 new_offset)
{
unsigned int pitch_pixels = pitch_tiles * tile_width;
unsigned int tiles;
@@ -2350,6 +2347,50 @@ static u32 intel_adjust_tile_offset(int *x, int *y,
}
/*
+ * Adjust the tile offset by moving the difference into
+ * the x/y offsets.
+ */
+static u32 intel_adjust_tile_offset(int *x, int *y,
+ const struct intel_plane_state *state, int plane,
+ u32 old_offset, u32 new_offset)
+{
+ const struct drm_i915_private *dev_priv = to_i915(state->base.plane->dev);
+ const struct drm_framebuffer *fb = state->base.fb;
+ unsigned int cpp = drm_format_plane_cpp(fb->pixel_format, plane);
+ unsigned int rotation = state->base.rotation;
+ unsigned int pitch = intel_fb_pitch(fb, plane, rotation);
+
+ WARN_ON(new_offset > old_offset);
+
+ if (fb->modifier[plane] != DRM_FORMAT_MOD_NONE) {
+ unsigned int tile_size, tile_width, tile_height;
+ unsigned int pitch_tiles;
+
+ tile_size = intel_tile_size(dev_priv);
+ intel_tile_dims(dev_priv, &tile_width, &tile_height,
+ fb->modifier[plane], cpp);
+
+ if (intel_rotation_90_or_270(rotation)) {
+ pitch_tiles = pitch / tile_height;
+ swap(tile_width, tile_height);
+ } else {
+ pitch_tiles = pitch / (tile_width * cpp);
+ }
+
+ _intel_adjust_tile_offset(x, y, tile_width, tile_height,
+ tile_size, pitch_tiles,
+ old_offset, new_offset);
+ } else {
+ old_offset += *y * pitch + *x * cpp;
+
+ *y = (old_offset - new_offset) / pitch;
+ *x = ((old_offset - new_offset) - *y * pitch) / cpp;
+ }
+
+ return new_offset;
+}
+
+/*
* Computes the linear offset to the base tile and adjusts
* x, y. bytes per pixel is assumed to be a power-of-two.
*
@@ -2401,9 +2442,9 @@ static u32 _intel_compute_tile_offset(const struct drm_i915_private *dev_priv,
offset = (tile_rows * pitch_tiles + tiles) * tile_size;
offset_aligned = offset & ~alignment;
- intel_adjust_tile_offset(x, y, tile_width, tile_height,
- tile_size, pitch_tiles,
- offset, offset_aligned);
+ _intel_adjust_tile_offset(x, y, tile_width, tile_height,
+ tile_size, pitch_tiles,
+ offset, offset_aligned);
} else {
offset = *y * pitch + *x * cpp;
offset_aligned = offset & ~alignment;
@@ -2550,9 +2591,9 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
* We only keep the x/y offsets, so push all of the
* gtt offset into the x/y offsets.
*/
- intel_adjust_tile_offset(&x, &y, tile_size,
- tile_width, tile_height, pitch_tiles,
- gtt_offset_rotated * tile_size, 0);
+ _intel_adjust_tile_offset(&x, &y, tile_size,
+ tile_width, tile_height, pitch_tiles,
+ gtt_offset_rotated * tile_size, 0);
gtt_offset_rotated += rot_info->plane[i].width * rot_info->plane[i].height;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 10/12] drm/i915: Compute display surface offset in the plane check hook for SKL+
2016-08-10 9:23 [PATCH v5 00/12] drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5) ville.syrjala
` (8 preceding siblings ...)
2016-08-10 9:23 ` [PATCH 09/12] drm/i915: Make intel_adjust_tile_offset() work for linear buffers ville.syrjala
@ 2016-08-10 9:23 ` ville.syrjala
2016-08-10 9:23 ` [PATCH v2 11/12] drm/i915: Deal with NV12 CbCr plane AUX surface on SKL+ ville.syrjala
` (2 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: ville.syrjala @ 2016-08-10 9:23 UTC (permalink / raw)
To: intel-gfx
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 searching 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.
v2: Replace for+break loop with while loop
Rebase due to drm_plane_state src/dst rects
Rebase due to plane_check_state()
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 162 +++++++++++++++++++++++++++++------
drivers/gpu/drm/i915/intel_drv.h | 6 ++
drivers/gpu/drm/i915/intel_sprite.c | 33 +++----
3 files changed, 150 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8174c43dc8f8..b719c07599fd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2842,6 +2842,117 @@ valid_fb:
&obj->frontbuffer_bits);
}
+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->base.src.x1 >> 16;
+ int y = plane_state->base.src.y1 >> 16;
+ int w = drm_rect_width(&plane_state->base.src) >> 16;
+ int h = drm_rect_height(&plane_state->base.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);
+
+ while ((x + w) * cpp > fb->pitches[0]) {
+ 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->base.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)
@@ -3221,10 +3332,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->base.src.x1 >> 16;
- int src_y = plane_state->base.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->base.src) >> 16;
int src_h = drm_rect_height(&plane_state->base.src) >> 16;
int dst_x = plane_state->base.dst.x1;
@@ -3241,26 +3352,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--;
@@ -14410,12 +14501,14 @@ intel_check_primary_plane(struct drm_plane *plane,
struct intel_crtc_state *crtc_state,
struct intel_plane_state *state)
{
+ struct drm_i915_private *dev_priv = to_i915(plane->dev);
struct drm_crtc *crtc = state->base.crtc;
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) {
+ if (INTEL_GEN(dev_priv) >= 9) {
/* use scaler when colorkey is not required */
if (state->ckey.flags == I915_SET_COLORKEY_NONE) {
min_scale = 1;
@@ -14424,10 +14517,23 @@ intel_check_primary_plane(struct drm_plane *plane,
can_position = true;
}
- return drm_plane_helper_check_state(&state->base,
- &state->clip,
- min_scale, max_scale,
- can_position, true);
+ ret = drm_plane_helper_check_state(&state->base,
+ &state->clip,
+ min_scale, max_scale,
+ can_position, true);
+ if (ret)
+ return ret;
+
+ if (!state->base.fb)
+ return 0;
+
+ if (INTEL_GEN(dev_priv) >= 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 3abdc817e99d..2fde13833fcb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -350,6 +350,11 @@ struct intel_plane_state {
struct drm_plane_state base;
struct drm_rect clip;
+ struct {
+ u32 offset;
+ int x, y;
+ } main;
+
/*
* scaler_id
* = -1 : not using a scaler
@@ -1342,6 +1347,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 4d5ba488a9f4..366900dcde34 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -207,15 +207,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->base.dst.x1;
int crtc_y = plane_state->base.dst.y1;
uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
- uint32_t x = plane_state->base.src.x1 >> 16;
- uint32_t y = plane_state->base.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->base.src) >> 16;
uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
@@ -239,26 +239,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--;
@@ -773,6 +753,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
int hscale, vscale;
int max_scale, min_scale;
bool can_scale;
+ int ret;
src->x1 = state->base.src_x;
src->y1 = state->base.src_y;
@@ -937,6 +918,12 @@ intel_check_sprite_plane(struct drm_plane *plane,
dst->y1 = crtc_y;
dst->y2 = crtc_y + crtc_h;
+ if (INTEL_GEN(dev) >= 9) {
+ ret = skl_check_plane_surface(state);
+ if (ret)
+ return ret;
+ }
+
return 0;
}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 11/12] drm/i915: Deal with NV12 CbCr plane AUX surface on SKL+
2016-08-10 9:23 [PATCH v5 00/12] drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5) ville.syrjala
` (9 preceding siblings ...)
2016-08-10 9:23 ` [PATCH v2 10/12] drm/i915: Compute display surface offset in the plane check hook for SKL+ ville.syrjala
@ 2016-08-10 9:23 ` ville.syrjala
2016-08-10 10:03 ` Daniel Vetter
2016-08-10 9:23 ` [PATCH v2 12/12] drm/i915: Make sure fb offset is (macro)pixel aligned ville.syrjala
2016-08-10 11:46 ` ✗ Ro.CI.BAT: failure for drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5) Patchwork
12 siblings, 1 reply; 27+ messages in thread
From: ville.syrjala @ 2016-08-10 9:23 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
With NV12 we have two color planes to deal with so we must compute the
surface and x/y offsets for the second plane as well.
What makes this a bit nasty is that the hardware expects the surface
offset to be specified as a distance from the main surface offset.
What's worse, the distance must be non-negative (no neat wraparound or
anything). So we must make sure that the main surface offset is always
less or equal to the AUX surface offset. We do that by computing the AUX
offset first and the main surface offset second. If the main surface
offset ends up being above the AUX offset, we just push it down as far
as is required while still maintaining the required alignment etc.
Fortunately the AUX offset only reuqires 4K alignment, so we don't need
to do any of the backwards searching for an acceptable offset that we
must do for the main surface. And X tiled + NV12 isn't a supported
combination anyway.
Note that this just computes aux surface offsets, we do not yet program
them into the actual hardware registers, and hence we can't yet expose
NV12.
v2: Rebase due to drm_plane_state src/dst rects
s/TODO.../something else/ in the commit message/ (Daniel)
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 62 ++++++++++++++++++++++++++++++++++--
drivers/gpu/drm/i915/intel_drv.h | 4 +++
2 files changed, 64 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b719c07599fd..f8487bcdb271 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2463,8 +2463,14 @@ u32 intel_compute_tile_offset(int *x, int *y,
const struct drm_i915_private *dev_priv = to_i915(state->base.plane->dev);
const struct drm_framebuffer *fb = state->base.fb;
unsigned int rotation = state->base.rotation;
- u32 alignment = intel_surf_alignment(dev_priv, fb->modifier[plane]);
int pitch = intel_fb_pitch(fb, plane, rotation);
+ u32 alignment;
+
+ /* AUX_DIST needs only 4K alignment */
+ if (fb->pixel_format == DRM_FORMAT_NV12 && plane == 1)
+ alignment = 4096;
+ else
+ alignment = intel_surf_alignment(dev_priv, fb->modifier[plane]);
return _intel_compute_tile_offset(dev_priv, x, y, fb, plane, pitch,
rotation, alignment);
@@ -2895,7 +2901,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
int h = drm_rect_height(&plane_state->base.src) >> 16;
int max_width = skl_max_plane_width(fb, 0, rotation);
int max_height = 4096;
- u32 alignment, offset;
+ u32 alignment, offset, aux_offset = plane_state->aux.offset;
if (w > max_width || h > max_height) {
DRM_DEBUG_KMS("requested Y/RGB source size %dx%d too big (limit %dx%d)\n",
@@ -2909,6 +2915,15 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
alignment = intel_surf_alignment(dev_priv, fb->modifier[0]);
/*
+ * AUX surface offset is specified as the distance from the
+ * main surface offset, and it must be non-negative. Make
+ * sure that is what we will get.
+ */
+ if (offset > aux_offset)
+ offset = intel_adjust_tile_offset(&x, &y, plane_state, 0,
+ offset, aux_offset & ~(alignment - 1));
+
+ /*
* When using an X-tiled surface, the plane blows up
* if the x offset + width exceed the stride.
*
@@ -2935,6 +2950,35 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
return 0;
}
+static int skl_check_nv12_aux_surface(struct intel_plane_state *plane_state)
+{
+ const struct drm_framebuffer *fb = plane_state->base.fb;
+ unsigned int rotation = plane_state->base.rotation;
+ int max_width = skl_max_plane_width(fb, 1, rotation);
+ int max_height = 4096;
+ int x = plane_state->base.src.x1 >> 17;
+ int y = plane_state->base.src.y1 >> 17;
+ int w = drm_rect_width(&plane_state->base.src) >> 17;
+ int h = drm_rect_height(&plane_state->base.src) >> 17;
+ u32 offset;
+
+ intel_add_fb_offsets(&x, &y, plane_state, 1);
+ offset = intel_compute_tile_offset(&x, &y, plane_state, 1);
+
+ /* FIXME not quite sure how/if these apply to the chroma plane */
+ if (w > max_width || h > max_height) {
+ DRM_DEBUG_KMS("CbCr source size %dx%d too big (limit %dx%d)\n",
+ w, h, max_width, max_height);
+ return -EINVAL;
+ }
+
+ plane_state->aux.offset = offset;
+ plane_state->aux.x = x;
+ plane_state->aux.y = y;
+
+ return 0;
+}
+
int skl_check_plane_surface(struct intel_plane_state *plane_state)
{
const struct drm_framebuffer *fb = plane_state->base.fb;
@@ -2946,6 +2990,20 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state)
drm_rect_rotate(&plane_state->base.src,
fb->width, fb->height, BIT(DRM_ROTATE_270));
+ /*
+ * Handle the AUX surface first since
+ * the main surface setup depends on it.
+ */
+ if (fb->pixel_format == DRM_FORMAT_NV12) {
+ ret = skl_check_nv12_aux_surface(plane_state);
+ if (ret)
+ return ret;
+ } else {
+ plane_state->aux.offset = ~0xfff;
+ plane_state->aux.x = 0;
+ plane_state->aux.y = 0;
+ }
+
ret = skl_check_main_surface(plane_state);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2fde13833fcb..df8e7514c08c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -354,6 +354,10 @@ struct intel_plane_state {
u32 offset;
int x, y;
} main;
+ struct {
+ u32 offset;
+ int x, y;
+ } aux;
/*
* scaler_id
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v2 11/12] drm/i915: Deal with NV12 CbCr plane AUX surface on SKL+
2016-08-10 9:23 ` [PATCH v2 11/12] drm/i915: Deal with NV12 CbCr plane AUX surface on SKL+ ville.syrjala
@ 2016-08-10 10:03 ` Daniel Vetter
2016-08-10 11:26 ` Ville Syrjälä
0 siblings, 1 reply; 27+ messages in thread
From: Daniel Vetter @ 2016-08-10 10:03 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Wed, Aug 10, 2016 at 12:23:20PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> With NV12 we have two color planes to deal with so we must compute the
> surface and x/y offsets for the second plane as well.
>
> What makes this a bit nasty is that the hardware expects the surface
> offset to be specified as a distance from the main surface offset.
> What's worse, the distance must be non-negative (no neat wraparound or
> anything). So we must make sure that the main surface offset is always
> less or equal to the AUX surface offset. We do that by computing the AUX
> offset first and the main surface offset second. If the main surface
> offset ends up being above the AUX offset, we just push it down as far
> as is required while still maintaining the required alignment etc.
>
> Fortunately the AUX offset only reuqires 4K alignment, so we don't need
> to do any of the backwards searching for an acceptable offset that we
> must do for the main surface. And X tiled + NV12 isn't a supported
> combination anyway.
>
> Note that this just computes aux surface offsets, we do not yet program
> them into the actual hardware registers, and hence we can't yet expose
> NV12.
>
> v2: Rebase due to drm_plane_state src/dst rects
> s/TODO.../something else/ in the commit message/ (Daniel)
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 62 ++++++++++++++++++++++++++++++++++--
> drivers/gpu/drm/i915/intel_drv.h | 4 +++
> 2 files changed, 64 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b719c07599fd..f8487bcdb271 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2463,8 +2463,14 @@ u32 intel_compute_tile_offset(int *x, int *y,
> const struct drm_i915_private *dev_priv = to_i915(state->base.plane->dev);
> const struct drm_framebuffer *fb = state->base.fb;
> unsigned int rotation = state->base.rotation;
> - u32 alignment = intel_surf_alignment(dev_priv, fb->modifier[plane]);
> int pitch = intel_fb_pitch(fb, plane, rotation);
> + u32 alignment;
> +
> + /* AUX_DIST needs only 4K alignment */
> + if (fb->pixel_format == DRM_FORMAT_NV12 && plane == 1)
> + alignment = 4096;
> + else
> + alignment = intel_surf_alignment(dev_priv, fb->modifier[plane]);
>
> return _intel_compute_tile_offset(dev_priv, x, y, fb, plane, pitch,
> rotation, alignment);
> @@ -2895,7 +2901,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> int h = drm_rect_height(&plane_state->base.src) >> 16;
> int max_width = skl_max_plane_width(fb, 0, rotation);
> int max_height = 4096;
> - u32 alignment, offset;
> + u32 alignment, offset, aux_offset = plane_state->aux.offset;
>
> if (w > max_width || h > max_height) {
> DRM_DEBUG_KMS("requested Y/RGB source size %dx%d too big (limit %dx%d)\n",
> @@ -2909,6 +2915,15 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> alignment = intel_surf_alignment(dev_priv, fb->modifier[0]);
>
> /*
> + * AUX surface offset is specified as the distance from the
> + * main surface offset, and it must be non-negative. Make
> + * sure that is what we will get.
> + */
> + if (offset > aux_offset)
> + offset = intel_adjust_tile_offset(&x, &y, plane_state, 0,
> + offset, aux_offset & ~(alignment - 1));
Note to self: Need to make sure that multiplanar fbs all reference the
same underlying bo. But that code isn't yet added to
intel_framebuffer_init().
> +
> + /*
> * When using an X-tiled surface, the plane blows up
> * if the x offset + width exceed the stride.
> *
> @@ -2935,6 +2950,35 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> return 0;
> }
>
> +static int skl_check_nv12_aux_surface(struct intel_plane_state *plane_state)
> +{
> + const struct drm_framebuffer *fb = plane_state->base.fb;
> + unsigned int rotation = plane_state->base.rotation;
> + int max_width = skl_max_plane_width(fb, 1, rotation);
> + int max_height = 4096;
> + int x = plane_state->base.src.x1 >> 17;
> + int y = plane_state->base.src.y1 >> 17;
> + int w = drm_rect_width(&plane_state->base.src) >> 17;
> + int h = drm_rect_height(&plane_state->base.src) >> 17;
> + u32 offset;
> +
> + intel_add_fb_offsets(&x, &y, plane_state, 1);
> + offset = intel_compute_tile_offset(&x, &y, plane_state, 1);
> +
> + /* FIXME not quite sure how/if these apply to the chroma plane */
> + if (w > max_width || h > max_height) {
> + DRM_DEBUG_KMS("CbCr source size %dx%d too big (limit %dx%d)\n",
> + w, h, max_width, max_height);
> + return -EINVAL;
> + }
We also need to check pitch of the 2nd plane, but I guess that's done at
fb_init time, not at atomic_check time. Looks all reasonable as-is.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> +
> + plane_state->aux.offset = offset;
> + plane_state->aux.x = x;
> + plane_state->aux.y = y;
> +
> + return 0;
> +}
> +
> int skl_check_plane_surface(struct intel_plane_state *plane_state)
> {
> const struct drm_framebuffer *fb = plane_state->base.fb;
> @@ -2946,6 +2990,20 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state)
> drm_rect_rotate(&plane_state->base.src,
> fb->width, fb->height, BIT(DRM_ROTATE_270));
>
> + /*
> + * Handle the AUX surface first since
> + * the main surface setup depends on it.
> + */
> + if (fb->pixel_format == DRM_FORMAT_NV12) {
> + ret = skl_check_nv12_aux_surface(plane_state);
> + if (ret)
> + return ret;
> + } else {
> + plane_state->aux.offset = ~0xfff;
> + plane_state->aux.x = 0;
> + plane_state->aux.y = 0;
> + }
> +
> ret = skl_check_main_surface(plane_state);
> if (ret)
> return ret;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 2fde13833fcb..df8e7514c08c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -354,6 +354,10 @@ struct intel_plane_state {
> u32 offset;
> int x, y;
> } main;
> + struct {
> + u32 offset;
> + int x, y;
> + } aux;
>
> /*
> * scaler_id
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 11/12] drm/i915: Deal with NV12 CbCr plane AUX surface on SKL+
2016-08-10 10:03 ` Daniel Vetter
@ 2016-08-10 11:26 ` Ville Syrjälä
0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2016-08-10 11:26 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Wed, Aug 10, 2016 at 12:03:08PM +0200, Daniel Vetter wrote:
> On Wed, Aug 10, 2016 at 12:23:20PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > With NV12 we have two color planes to deal with so we must compute the
> > surface and x/y offsets for the second plane as well.
> >
> > What makes this a bit nasty is that the hardware expects the surface
> > offset to be specified as a distance from the main surface offset.
> > What's worse, the distance must be non-negative (no neat wraparound or
> > anything). So we must make sure that the main surface offset is always
> > less or equal to the AUX surface offset. We do that by computing the AUX
> > offset first and the main surface offset second. If the main surface
> > offset ends up being above the AUX offset, we just push it down as far
> > as is required while still maintaining the required alignment etc.
> >
> > Fortunately the AUX offset only reuqires 4K alignment, so we don't need
> > to do any of the backwards searching for an acceptable offset that we
> > must do for the main surface. And X tiled + NV12 isn't a supported
> > combination anyway.
> >
> > Note that this just computes aux surface offsets, we do not yet program
> > them into the actual hardware registers, and hence we can't yet expose
> > NV12.
> >
> > v2: Rebase due to drm_plane_state src/dst rects
> > s/TODO.../something else/ in the commit message/ (Daniel)
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 62 ++++++++++++++++++++++++++++++++++--
> > drivers/gpu/drm/i915/intel_drv.h | 4 +++
> > 2 files changed, 64 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index b719c07599fd..f8487bcdb271 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2463,8 +2463,14 @@ u32 intel_compute_tile_offset(int *x, int *y,
> > const struct drm_i915_private *dev_priv = to_i915(state->base.plane->dev);
> > const struct drm_framebuffer *fb = state->base.fb;
> > unsigned int rotation = state->base.rotation;
> > - u32 alignment = intel_surf_alignment(dev_priv, fb->modifier[plane]);
> > int pitch = intel_fb_pitch(fb, plane, rotation);
> > + u32 alignment;
> > +
> > + /* AUX_DIST needs only 4K alignment */
> > + if (fb->pixel_format == DRM_FORMAT_NV12 && plane == 1)
> > + alignment = 4096;
> > + else
> > + alignment = intel_surf_alignment(dev_priv, fb->modifier[plane]);
> >
> > return _intel_compute_tile_offset(dev_priv, x, y, fb, plane, pitch,
> > rotation, alignment);
> > @@ -2895,7 +2901,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> > int h = drm_rect_height(&plane_state->base.src) >> 16;
> > int max_width = skl_max_plane_width(fb, 0, rotation);
> > int max_height = 4096;
> > - u32 alignment, offset;
> > + u32 alignment, offset, aux_offset = plane_state->aux.offset;
> >
> > if (w > max_width || h > max_height) {
> > DRM_DEBUG_KMS("requested Y/RGB source size %dx%d too big (limit %dx%d)\n",
> > @@ -2909,6 +2915,15 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> > alignment = intel_surf_alignment(dev_priv, fb->modifier[0]);
> >
> > /*
> > + * AUX surface offset is specified as the distance from the
> > + * main surface offset, and it must be non-negative. Make
> > + * sure that is what we will get.
> > + */
> > + if (offset > aux_offset)
> > + offset = intel_adjust_tile_offset(&x, &y, plane_state, 0,
> > + offset, aux_offset & ~(alignment - 1));
>
> Note to self: Need to make sure that multiplanar fbs all reference the
> same underlying bo. But that code isn't yet added to
> intel_framebuffer_init().
Yep. It would be a lot more work to support different handles for the
planes, so I figure we shouldn't bother. Frankly I don't understand
why the hw doesn't just take two independent offsets, since it would
require exactly the same number of bits in the registers anyway. But
yeah, we have the hw we have, so we must live with it.
>
> > +
> > + /*
> > * When using an X-tiled surface, the plane blows up
> > * if the x offset + width exceed the stride.
> > *
> > @@ -2935,6 +2950,35 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> > return 0;
> > }
> >
> > +static int skl_check_nv12_aux_surface(struct intel_plane_state *plane_state)
> > +{
> > + const struct drm_framebuffer *fb = plane_state->base.fb;
> > + unsigned int rotation = plane_state->base.rotation;
> > + int max_width = skl_max_plane_width(fb, 1, rotation);
> > + int max_height = 4096;
> > + int x = plane_state->base.src.x1 >> 17;
> > + int y = plane_state->base.src.y1 >> 17;
> > + int w = drm_rect_width(&plane_state->base.src) >> 17;
> > + int h = drm_rect_height(&plane_state->base.src) >> 17;
> > + u32 offset;
> > +
> > + intel_add_fb_offsets(&x, &y, plane_state, 1);
> > + offset = intel_compute_tile_offset(&x, &y, plane_state, 1);
> > +
> > + /* FIXME not quite sure how/if these apply to the chroma plane */
> > + if (w > max_width || h > max_height) {
> > + DRM_DEBUG_KMS("CbCr source size %dx%d too big (limit %dx%d)\n",
> > + w, h, max_width, max_height);
> > + return -EINVAL;
> > + }
>
> We also need to check pitch of the 2nd plane, but I guess that's done at
> fb_init time, not at atomic_check time. Looks all reasonable as-is.
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
>
> > +
> > + plane_state->aux.offset = offset;
> > + plane_state->aux.x = x;
> > + plane_state->aux.y = y;
> > +
> > + return 0;
> > +}
> > +
> > int skl_check_plane_surface(struct intel_plane_state *plane_state)
> > {
> > const struct drm_framebuffer *fb = plane_state->base.fb;
> > @@ -2946,6 +2990,20 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state)
> > drm_rect_rotate(&plane_state->base.src,
> > fb->width, fb->height, BIT(DRM_ROTATE_270));
> >
> > + /*
> > + * Handle the AUX surface first since
> > + * the main surface setup depends on it.
> > + */
> > + if (fb->pixel_format == DRM_FORMAT_NV12) {
> > + ret = skl_check_nv12_aux_surface(plane_state);
> > + if (ret)
> > + return ret;
> > + } else {
> > + plane_state->aux.offset = ~0xfff;
> > + plane_state->aux.x = 0;
> > + plane_state->aux.y = 0;
> > + }
> > +
> > ret = skl_check_main_surface(plane_state);
> > if (ret)
> > return ret;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 2fde13833fcb..df8e7514c08c 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -354,6 +354,10 @@ struct intel_plane_state {
> > u32 offset;
> > int x, y;
> > } main;
> > + struct {
> > + u32 offset;
> > + int x, y;
> > + } aux;
> >
> > /*
> > * scaler_id
> > --
> > 2.7.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 12/12] drm/i915: Make sure fb offset is (macro)pixel aligned
2016-08-10 9:23 [PATCH v5 00/12] drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5) ville.syrjala
` (10 preceding siblings ...)
2016-08-10 9:23 ` [PATCH v2 11/12] drm/i915: Deal with NV12 CbCr plane AUX surface on SKL+ ville.syrjala
@ 2016-08-10 9:23 ` ville.syrjala
2016-08-10 10:04 ` Daniel Vetter
2016-08-10 11:46 ` ✗ Ro.CI.BAT: failure for drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5) Patchwork
12 siblings, 1 reply; 27+ messages in thread
From: ville.syrjala @ 2016-08-10 9:23 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
We convert the fb->offsets[] into x/y offsets, so they must be
(macro)pixel aligned. Check for this, and if things look good
allow fb->offsets[] != 0 when creating fbs since we now handle
them correctly.
v2: Move to last place in the series and improve the commit message
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> (v1)
---
drivers/gpu/drm/i915/intel_display.c | 37 +++++++++++++++++++++++++++++++++---
1 file changed, 34 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f8487bcdb271..d3de56f0e764 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15370,6 +15370,37 @@ u32 intel_fb_pitch_limit(struct drm_device *dev, uint64_t fb_modifier,
}
}
+static int intel_fb_check_offsets(const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+ uint32_t format = mode_cmd->pixel_format;
+ int num_planes = drm_format_num_planes(format);
+ int i;
+
+ for (i = 0; i < num_planes; i++) {
+ unsigned int cpp;
+
+ switch (format) {
+ case DRM_FORMAT_YUYV:
+ case DRM_FORMAT_UYVY:
+ case DRM_FORMAT_YVYU:
+ case DRM_FORMAT_VYUY:
+ cpp = 4;
+ break;
+ default:
+ cpp = drm_format_plane_cpp(format, i);
+ break;
+ }
+
+ if (mode_cmd->offsets[i] % cpp) {
+ DRM_DEBUG("fb plane %d offset 0x%08x not (macro)pixel aligned\n",
+ i, mode_cmd->offsets[i]);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
static int intel_framebuffer_init(struct drm_device *dev,
struct intel_framebuffer *intel_fb,
struct drm_mode_fb_cmd2 *mode_cmd,
@@ -15514,9 +15545,9 @@ static int intel_framebuffer_init(struct drm_device *dev,
return -EINVAL;
}
- /* FIXME need to adjust LINOFF/TILEOFF accordingly. */
- if (mode_cmd->offsets[0] != 0)
- return -EINVAL;
+ ret = intel_fb_check_offsets(mode_cmd);
+ if (ret)
+ return ret;
drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
intel_fb->obj = obj;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v2 12/12] drm/i915: Make sure fb offset is (macro)pixel aligned
2016-08-10 9:23 ` [PATCH v2 12/12] drm/i915: Make sure fb offset is (macro)pixel aligned ville.syrjala
@ 2016-08-10 10:04 ` Daniel Vetter
2016-08-10 11:02 ` Ville Syrjälä
0 siblings, 1 reply; 27+ messages in thread
From: Daniel Vetter @ 2016-08-10 10:04 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Wed, Aug 10, 2016 at 12:23:21PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We convert the fb->offsets[] into x/y offsets, so they must be
> (macro)pixel aligned. Check for this, and if things look good
> allow fb->offsets[] != 0 when creating fbs since we now handle
> them correctly.
>
> v2: Move to last place in the series and improve the commit message
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> (v1)
> ---
> drivers/gpu/drm/i915/intel_display.c | 37 +++++++++++++++++++++++++++++++++---
> 1 file changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f8487bcdb271..d3de56f0e764 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15370,6 +15370,37 @@ u32 intel_fb_pitch_limit(struct drm_device *dev, uint64_t fb_modifier,
> }
> }
>
> +static int intel_fb_check_offsets(const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> + uint32_t format = mode_cmd->pixel_format;
> + int num_planes = drm_format_num_planes(format);
> + int i;
> +
> + for (i = 0; i < num_planes; i++) {
> + unsigned int cpp;
> +
> + switch (format) {
> + case DRM_FORMAT_YUYV:
> + case DRM_FORMAT_UYVY:
> + case DRM_FORMAT_YVYU:
> + case DRM_FORMAT_VYUY:
> + cpp = 4;
> + break;
> + default:
> + cpp = drm_format_plane_cpp(format, i);
I didn't ask for a helper in drm_fourcc.c for macropixel blocksize? Shame
on me! Anyway, looks all good and I didn't spot anything else amiss while
re-reading the series.
Another wishlist item: Extracting intel_framebuffer.c might be a good
idea.
-Daniel
> + break;
> + }
> +
> + if (mode_cmd->offsets[i] % cpp) {
> + DRM_DEBUG("fb plane %d offset 0x%08x not (macro)pixel aligned\n",
> + i, mode_cmd->offsets[i]);
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int intel_framebuffer_init(struct drm_device *dev,
> struct intel_framebuffer *intel_fb,
> struct drm_mode_fb_cmd2 *mode_cmd,
> @@ -15514,9 +15545,9 @@ static int intel_framebuffer_init(struct drm_device *dev,
> return -EINVAL;
> }
>
> - /* FIXME need to adjust LINOFF/TILEOFF accordingly. */
> - if (mode_cmd->offsets[0] != 0)
> - return -EINVAL;
> + ret = intel_fb_check_offsets(mode_cmd);
> + if (ret)
> + return ret;
>
> drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
> intel_fb->obj = obj;
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 12/12] drm/i915: Make sure fb offset is (macro)pixel aligned
2016-08-10 10:04 ` Daniel Vetter
@ 2016-08-10 11:02 ` Ville Syrjälä
2016-08-10 11:33 ` Ville Syrjälä
0 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjälä @ 2016-08-10 11:02 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Wed, Aug 10, 2016 at 12:04:32PM +0200, Daniel Vetter wrote:
> On Wed, Aug 10, 2016 at 12:23:21PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We convert the fb->offsets[] into x/y offsets, so they must be
> > (macro)pixel aligned. Check for this, and if things look good
> > allow fb->offsets[] != 0 when creating fbs since we now handle
> > them correctly.
> >
> > v2: Move to last place in the series and improve the commit message
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> (v1)
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 37 +++++++++++++++++++++++++++++++++---
> > 1 file changed, 34 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index f8487bcdb271..d3de56f0e764 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -15370,6 +15370,37 @@ u32 intel_fb_pitch_limit(struct drm_device *dev, uint64_t fb_modifier,
> > }
> > }
> >
> > +static int intel_fb_check_offsets(const struct drm_mode_fb_cmd2 *mode_cmd)
> > +{
> > + uint32_t format = mode_cmd->pixel_format;
> > + int num_planes = drm_format_num_planes(format);
> > + int i;
> > +
> > + for (i = 0; i < num_planes; i++) {
> > + unsigned int cpp;
> > +
> > + switch (format) {
> > + case DRM_FORMAT_YUYV:
> > + case DRM_FORMAT_UYVY:
> > + case DRM_FORMAT_YVYU:
> > + case DRM_FORMAT_VYUY:
> > + cpp = 4;
> > + break;
> > + default:
> > + cpp = drm_format_plane_cpp(format, i);
>
> I didn't ask for a helper in drm_fourcc.c for macropixel blocksize? Shame
> on me!
You did, long ago. I just couldn't be bothered to go and see what we
need to do there. Now that I do, it doesn't look like we've gained
any new formats that would need special treatment, so I suppose I
could just reuse this guy as is.
> Anyway, looks all good and I didn't spot anything else amiss while
> re-reading the series.
>
> Another wishlist item: Extracting intel_framebuffer.c might be a good
> idea.
> -Daniel
>
> > + break;
> > + }
> > +
> > + if (mode_cmd->offsets[i] % cpp) {
> > + DRM_DEBUG("fb plane %d offset 0x%08x not (macro)pixel aligned\n",
> > + i, mode_cmd->offsets[i]);
> > + return -EINVAL;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int intel_framebuffer_init(struct drm_device *dev,
> > struct intel_framebuffer *intel_fb,
> > struct drm_mode_fb_cmd2 *mode_cmd,
> > @@ -15514,9 +15545,9 @@ static int intel_framebuffer_init(struct drm_device *dev,
> > return -EINVAL;
> > }
> >
> > - /* FIXME need to adjust LINOFF/TILEOFF accordingly. */
> > - if (mode_cmd->offsets[0] != 0)
> > - return -EINVAL;
> > + ret = intel_fb_check_offsets(mode_cmd);
> > + if (ret)
> > + return ret;
> >
> > drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
> > intel_fb->obj = obj;
> > --
> > 2.7.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 12/12] drm/i915: Make sure fb offset is (macro)pixel aligned
2016-08-10 11:02 ` Ville Syrjälä
@ 2016-08-10 11:33 ` Ville Syrjälä
2016-08-11 16:37 ` Ville Syrjälä
0 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjälä @ 2016-08-10 11:33 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Wed, Aug 10, 2016 at 02:02:43PM +0300, Ville Syrjälä wrote:
> On Wed, Aug 10, 2016 at 12:04:32PM +0200, Daniel Vetter wrote:
> > On Wed, Aug 10, 2016 at 12:23:21PM +0300, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > We convert the fb->offsets[] into x/y offsets, so they must be
> > > (macro)pixel aligned. Check for this, and if things look good
> > > allow fb->offsets[] != 0 when creating fbs since we now handle
> > > them correctly.
> > >
> > > v2: Move to last place in the series and improve the commit message
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> (v1)
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 37 +++++++++++++++++++++++++++++++++---
> > > 1 file changed, 34 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index f8487bcdb271..d3de56f0e764 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -15370,6 +15370,37 @@ u32 intel_fb_pitch_limit(struct drm_device *dev, uint64_t fb_modifier,
> > > }
> > > }
> > >
> > > +static int intel_fb_check_offsets(const struct drm_mode_fb_cmd2 *mode_cmd)
> > > +{
> > > + uint32_t format = mode_cmd->pixel_format;
> > > + int num_planes = drm_format_num_planes(format);
> > > + int i;
> > > +
> > > + for (i = 0; i < num_planes; i++) {
> > > + unsigned int cpp;
> > > +
> > > + switch (format) {
> > > + case DRM_FORMAT_YUYV:
> > > + case DRM_FORMAT_UYVY:
> > > + case DRM_FORMAT_YVYU:
> > > + case DRM_FORMAT_VYUY:
> > > + cpp = 4;
> > > + break;
> > > + default:
> > > + cpp = drm_format_plane_cpp(format, i);
> >
> > I didn't ask for a helper in drm_fourcc.c for macropixel blocksize? Shame
> > on me!
>
> You did, long ago. I just couldn't be bothered to go and see what we
> need to do there. Now that I do, it doesn't look like we've gained
> any new formats that would need special treatment, so I suppose I
> could just reuse this guy as is.
Any ideas for the name? Anything I can come up with sounds stupid.
The _cpp() precedent makes me want to do _cpm(), but that looks
totally stupid. _macropixel_size() perhaps? Doesn't match the _cpp()
Maybe rename the _cpp() to _pixel_size() to match? Dunno. Ideas?
>
> > Anyway, looks all good and I didn't spot anything else amiss while
> > re-reading the series.
> >
> > Another wishlist item: Extracting intel_framebuffer.c might be a good
> > idea.
> > -Daniel
> >
> > > + break;
> > > + }
> > > +
> > > + if (mode_cmd->offsets[i] % cpp) {
> > > + DRM_DEBUG("fb plane %d offset 0x%08x not (macro)pixel aligned\n",
> > > + i, mode_cmd->offsets[i]);
> > > + return -EINVAL;
> > > + }
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static int intel_framebuffer_init(struct drm_device *dev,
> > > struct intel_framebuffer *intel_fb,
> > > struct drm_mode_fb_cmd2 *mode_cmd,
> > > @@ -15514,9 +15545,9 @@ static int intel_framebuffer_init(struct drm_device *dev,
> > > return -EINVAL;
> > > }
> > >
> > > - /* FIXME need to adjust LINOFF/TILEOFF accordingly. */
> > > - if (mode_cmd->offsets[0] != 0)
> > > - return -EINVAL;
> > > + ret = intel_fb_check_offsets(mode_cmd);
> > > + if (ret)
> > > + return ret;
> > >
> > > drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
> > > intel_fb->obj = obj;
> > > --
> > > 2.7.4
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 12/12] drm/i915: Make sure fb offset is (macro)pixel aligned
2016-08-10 11:33 ` Ville Syrjälä
@ 2016-08-11 16:37 ` Ville Syrjälä
0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2016-08-11 16:37 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Wed, Aug 10, 2016 at 02:33:53PM +0300, Ville Syrjälä wrote:
> On Wed, Aug 10, 2016 at 02:02:43PM +0300, Ville Syrjälä wrote:
> > On Wed, Aug 10, 2016 at 12:04:32PM +0200, Daniel Vetter wrote:
> > > On Wed, Aug 10, 2016 at 12:23:21PM +0300, ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > >
> > > > We convert the fb->offsets[] into x/y offsets, so they must be
> > > > (macro)pixel aligned. Check for this, and if things look good
> > > > allow fb->offsets[] != 0 when creating fbs since we now handle
> > > > them correctly.
> > > >
> > > > v2: Move to last place in the series and improve the commit message
> > > >
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> (v1)
> > > > ---
> > > > drivers/gpu/drm/i915/intel_display.c | 37 +++++++++++++++++++++++++++++++++---
> > > > 1 file changed, 34 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > index f8487bcdb271..d3de56f0e764 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -15370,6 +15370,37 @@ u32 intel_fb_pitch_limit(struct drm_device *dev, uint64_t fb_modifier,
> > > > }
> > > > }
> > > >
> > > > +static int intel_fb_check_offsets(const struct drm_mode_fb_cmd2 *mode_cmd)
> > > > +{
> > > > + uint32_t format = mode_cmd->pixel_format;
> > > > + int num_planes = drm_format_num_planes(format);
> > > > + int i;
> > > > +
> > > > + for (i = 0; i < num_planes; i++) {
> > > > + unsigned int cpp;
> > > > +
> > > > + switch (format) {
> > > > + case DRM_FORMAT_YUYV:
> > > > + case DRM_FORMAT_UYVY:
> > > > + case DRM_FORMAT_YVYU:
> > > > + case DRM_FORMAT_VYUY:
> > > > + cpp = 4;
> > > > + break;
> > > > + default:
> > > > + cpp = drm_format_plane_cpp(format, i);
> > >
> > > I didn't ask for a helper in drm_fourcc.c for macropixel blocksize? Shame
> > > on me!
> >
> > You did, long ago. I just couldn't be bothered to go and see what we
> > need to do there. Now that I do, it doesn't look like we've gained
> > any new formats that would need special treatment, so I suppose I
> > could just reuse this guy as is.
>
> Any ideas for the name? Anything I can come up with sounds stupid.
> The _cpp() precedent makes me want to do _cpm(), but that looks
> totally stupid. _macropixel_size() perhaps? Doesn't match the _cpp()
> Maybe rename the _cpp() to _pixel_size() to match? Dunno. Ideas?
I've pushed everything but this last patch to dinq. Thanks for the
reviews.
There are two open questions with this patch: first one is about
moving the helper to the core, and the second one has to do with
tests. Namely, I can't recall where I put the ones I had written,
so I need to try and dig them up.
>
> >
> > > Anyway, looks all good and I didn't spot anything else amiss while
> > > re-reading the series.
> > >
> > > Another wishlist item: Extracting intel_framebuffer.c might be a good
> > > idea.
> > > -Daniel
> > >
> > > > + break;
> > > > + }
> > > > +
> > > > + if (mode_cmd->offsets[i] % cpp) {
> > > > + DRM_DEBUG("fb plane %d offset 0x%08x not (macro)pixel aligned\n",
> > > > + i, mode_cmd->offsets[i]);
> > > > + return -EINVAL;
> > > > + }
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > static int intel_framebuffer_init(struct drm_device *dev,
> > > > struct intel_framebuffer *intel_fb,
> > > > struct drm_mode_fb_cmd2 *mode_cmd,
> > > > @@ -15514,9 +15545,9 @@ static int intel_framebuffer_init(struct drm_device *dev,
> > > > return -EINVAL;
> > > > }
> > > >
> > > > - /* FIXME need to adjust LINOFF/TILEOFF accordingly. */
> > > > - if (mode_cmd->offsets[0] != 0)
> > > > - return -EINVAL;
> > > > + ret = intel_fb_check_offsets(mode_cmd);
> > > > + if (ret)
> > > > + return ret;
> > > >
> > > > drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
> > > intel_fb->obj = obj;
> > > > --
> > > > 2.7.4
> > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* ✗ Ro.CI.BAT: failure for drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5)
2016-08-10 9:23 [PATCH v5 00/12] drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5) ville.syrjala
` (11 preceding siblings ...)
2016-08-10 9:23 ` [PATCH v2 12/12] drm/i915: Make sure fb offset is (macro)pixel aligned ville.syrjala
@ 2016-08-10 11:46 ` Patchwork
2016-08-10 12:40 ` Ville Syrjälä
12 siblings, 1 reply; 27+ messages in thread
From: Patchwork @ 2016-08-10 11:46 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5)
URL : https://patchwork.freedesktop.org/series/10895/
State : failure
== Summary ==
Series 10895v1 drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5)
http://patchwork.freedesktop.org/api/1.0/series/10895/revisions/1/mbox
Test kms_cursor_legacy:
Subgroup basic-flip-vs-cursor-legacy:
fail -> PASS (ro-byt-n2820)
pass -> FAIL (ro-skl3-i5-6260u)
Subgroup basic-flip-vs-cursor-varying-size:
dmesg-fail -> PASS (fi-skl-i7-6700k)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
skip -> DMESG-WARN (ro-bdw-i5-5250u)
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS (ro-bdw-i7-5600u)
skip -> DMESG-WARN (ro-bdw-i5-5250u)
Subgroup suspend-read-crc-pipe-c:
dmesg-warn -> PASS (ro-bdw-i7-5600u)
fi-hsw-i7-4770k total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22
fi-kbl-qkkr total:244 pass:185 dwarn:30 dfail:0 fail:2 skip:27
fi-skl-i5-6260u total:244 pass:224 dwarn:4 dfail:0 fail:2 skip:14
fi-skl-i7-6700k total:244 pass:209 dwarn:4 dfail:1 fail:2 skip:28
fi-snb-i7-2600 total:244 pass:202 dwarn:0 dfail:0 fail:0 skip:42
ro-bdw-i5-5250u total:240 pass:219 dwarn:3 dfail:0 fail:1 skip:17
ro-bdw-i7-5557U total:240 pass:220 dwarn:1 dfail:0 fail:0 skip:19
ro-bdw-i7-5600u total:240 pass:207 dwarn:0 dfail:0 fail:1 skip:32
ro-bsw-n3050 total:240 pass:194 dwarn:0 dfail:0 fail:4 skip:42
ro-byt-n2820 total:240 pass:197 dwarn:0 dfail:0 fail:3 skip:40
ro-hsw-i7-4770r total:240 pass:214 dwarn:0 dfail:0 fail:0 skip:26
ro-ilk1-i5-650 total:235 pass:173 dwarn:0 dfail:0 fail:2 skip:60
ro-ivb-i7-3770 total:240 pass:205 dwarn:0 dfail:0 fail:0 skip:35
ro-ivb2-i7-3770 total:240 pass:209 dwarn:0 dfail:0 fail:0 skip:31
ro-skl3-i5-6260u total:240 pass:222 dwarn:0 dfail:0 fail:4 skip:14
Results at /archive/results/CI_IGT_test/RO_Patchwork_1810/
fa68dcb drm-intel-nightly: 2016y-08m-10d-09h-40m-37s UTC integration manifest
be4616b drm/i915: Make sure fb offset is (macro)pixel aligned
28528e8 drm/i915: Deal with NV12 CbCr plane AUX surface on SKL+
2372a31 drm/i915: Compute display surface offset in the plane check hook for SKL+
aa4b3f5 drm/i915: Make intel_adjust_tile_offset() work for linear buffers
d58f257 drm/i915: Allow calling intel_adjust_tile_offset() multiple times
a4b2620 drm/i915: Limit fb x offset due to fences
0c714bb drm/i915: Adjust obj tiling vs. fb modifier rules
65407a6 drm/i915: Use fb modifiers for display tiling decisions
d9470d4 drm/i915: Pass around plane_state instead of fb+rotation
68a2d0d drm/i915: Move SKL hw stride calculation into a helper
150ced5 drm/i915: Don't pass pitch to intel_compute_page_offset()
3d3bd95 drm/i915: Rewrite fb rotation GTT handling
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: ✗ Ro.CI.BAT: failure for drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5)
2016-08-10 11:46 ` ✗ Ro.CI.BAT: failure for drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5) Patchwork
@ 2016-08-10 12:40 ` Ville Syrjälä
0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2016-08-10 12:40 UTC (permalink / raw)
To: intel-gfx
On Wed, Aug 10, 2016 at 11:46:25AM -0000, Patchwork wrote:
> == Series Details ==
>
> Series: drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5)
> URL : https://patchwork.freedesktop.org/series/10895/
> State : failure
>
> == Summary ==
>
> Series 10895v1 drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v5)
> http://patchwork.freedesktop.org/api/1.0/series/10895/revisions/1/mbox
>
> Test kms_cursor_legacy:
> Subgroup basic-flip-vs-cursor-legacy:
> fail -> PASS (ro-byt-n2820)
> pass -> FAIL (ro-skl3-i5-6260u)
(kms_cursor_legacy:9283) DEBUG: Test requirement passed: target > 1
(kms_cursor_legacy:9283) DEBUG: Using a target of 64 cursor updates per half-vblank
(kms_cursor_legacy:9283) CRITICAL: Test assertion failure function basic_flip_vs_cursor, file kms_cursor_legacy.c:514:
(kms_cursor_legacy:9283) CRITICAL: Failed assertion: get_vblank(display->drm_fd, pipe, 0) == vblank_start
(kms_cursor_legacy:9283) CRITICAL: error: 11754 != 11753
https://bugs.freedesktop.org/show_bug.cgi?id=97188
> Subgroup basic-flip-vs-cursor-varying-size:
> dmesg-fail -> PASS (fi-skl-i7-6700k)
> Test kms_pipe_crc_basic:
> Subgroup suspend-read-crc-pipe-a:
> skip -> DMESG-WARN (ro-bdw-i5-5250u)
[ 449.058915] [drm:intel_dp_link_training_clock_recovery [i915]] *ERROR* failed to enable link training
[ 449.143730] [drm:intel_dp_start_link_train [i915]] *ERROR* failed to start channel equalization
https://bugs.freedesktop.org/show_bug.cgi?id=96913
> Subgroup suspend-read-crc-pipe-b:
> dmesg-warn -> PASS (ro-bdw-i7-5600u)
> skip -> DMESG-WARN (ro-bdw-i5-5250u)
same
> Subgroup suspend-read-crc-pipe-c:
> dmesg-warn -> PASS (ro-bdw-i7-5600u)
>
> fi-hsw-i7-4770k total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22
> fi-kbl-qkkr total:244 pass:185 dwarn:30 dfail:0 fail:2 skip:27
> fi-skl-i5-6260u total:244 pass:224 dwarn:4 dfail:0 fail:2 skip:14
> fi-skl-i7-6700k total:244 pass:209 dwarn:4 dfail:1 fail:2 skip:28
> fi-snb-i7-2600 total:244 pass:202 dwarn:0 dfail:0 fail:0 skip:42
> ro-bdw-i5-5250u total:240 pass:219 dwarn:3 dfail:0 fail:1 skip:17
> ro-bdw-i7-5557U total:240 pass:220 dwarn:1 dfail:0 fail:0 skip:19
> ro-bdw-i7-5600u total:240 pass:207 dwarn:0 dfail:0 fail:1 skip:32
> ro-bsw-n3050 total:240 pass:194 dwarn:0 dfail:0 fail:4 skip:42
> ro-byt-n2820 total:240 pass:197 dwarn:0 dfail:0 fail:3 skip:40
> ro-hsw-i7-4770r total:240 pass:214 dwarn:0 dfail:0 fail:0 skip:26
> ro-ilk1-i5-650 total:235 pass:173 dwarn:0 dfail:0 fail:2 skip:60
> ro-ivb-i7-3770 total:240 pass:205 dwarn:0 dfail:0 fail:0 skip:35
> ro-ivb2-i7-3770 total:240 pass:209 dwarn:0 dfail:0 fail:0 skip:31
> ro-skl3-i5-6260u total:240 pass:222 dwarn:0 dfail:0 fail:4 skip:14
>
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1810/
>
> fa68dcb drm-intel-nightly: 2016y-08m-10d-09h-40m-37s UTC integration manifest
> be4616b drm/i915: Make sure fb offset is (macro)pixel aligned
> 28528e8 drm/i915: Deal with NV12 CbCr plane AUX surface on SKL+
> 2372a31 drm/i915: Compute display surface offset in the plane check hook for SKL+
> aa4b3f5 drm/i915: Make intel_adjust_tile_offset() work for linear buffers
> d58f257 drm/i915: Allow calling intel_adjust_tile_offset() multiple times
> a4b2620 drm/i915: Limit fb x offset due to fences
> 0c714bb drm/i915: Adjust obj tiling vs. fb modifier rules
> 65407a6 drm/i915: Use fb modifiers for display tiling decisions
> d9470d4 drm/i915: Pass around plane_state instead of fb+rotation
> 68a2d0d drm/i915: Move SKL hw stride calculation into a helper
> 150ced5 drm/i915: Don't pass pitch to intel_compute_page_offset()
> 3d3bd95 drm/i915: Rewrite fb rotation GTT handling
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread