From: Ville Syrjälä <ville.syrj...@linux.intel.com> The display engine stride limits are getting in our way. On SKL+ we are limited to 8k pixels, which is easily exceeded with three 4k displays. To overcome this limitation we can remap the pages in the GTT to provide the display engine with a view of memory with a smaller stride.
The code is mostly already there as We already play tricks with the plane surface address and x/y offsets. A few caveats apply: * linear buffers need the fb stride to be page aligned, as otherwise the remapped lines wouldn't start at the same spot * compressed buffers can't be remapped due to the new ccs hash mode causing the virtual address of the pages to affect the interpretation of the compressed data. IIRC the old hash was limited to the low 12 bits so if we were using that mode we could remap. As it stands we just refuse to remapp with compressed fbs. * no remapping gen2/3 as we'd need a fence for the remapped vma, which we currently don't have. Need to deal with the fence POT requirements, and do something about the gen2 gtt page size vs tile size difference v2: Rebase due to is_ccs_modifier() Fix up the skl+ stride_mult mess memset() the gtt_view because otherwise we could leave junk in plane[1] when going from 2 plane to 1 plane format v3: intel_check_plane_stride() was split out v4: Drop the aligned viewport stuff, it was meant for ccs which can't be remapped anyway v5: Introduce intel_plane_can_remap() Reorder the code so that plane_state->view gets filled even for invisible planes, otherwise we'd keep using stale values and could explode during remapping. The new logic never remaps invisible planes since we don't have a viewport, and instead pins the full fb instead v6: Fix plane src coord checks after remapping by moving plane_state->base.src to the final plane x/y offsets. Allow intel_plane_check_stride() to fail even with remapping (can happen at least with a linear 64bpp fb with a 4k plane and a suitably inconvenient src coordinates). Improve aux plane FIXME (Daniel) Move some code shuffling into a separate patch (Daniel) Testcase: igt/kms_big_fb Cc: Daniel Vetter <dan...@ffwll.ch> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 354 ++++++++++++++++++++++----- drivers/gpu/drm/i915/intel_display.h | 2 + drivers/gpu/drm/i915/intel_sprite.c | 34 ++- 3 files changed, 323 insertions(+), 67 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 321cb6d2bc76..94faac9e3666 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1915,7 +1915,7 @@ intel_tile_width_bytes(const struct drm_framebuffer *fb, int color_plane) switch (fb->modifier) { case DRM_FORMAT_MOD_LINEAR: - return cpp; + return intel_tile_size(dev_priv); case I915_FORMAT_MOD_X_TILED: if (IS_GEN(dev_priv, 2)) return 128; @@ -1958,11 +1958,8 @@ intel_tile_width_bytes(const struct drm_framebuffer *fb, int color_plane) static unsigned int intel_tile_height(const struct drm_framebuffer *fb, int color_plane) { - if (fb->modifier == DRM_FORMAT_MOD_LINEAR) - return 1; - else - return intel_tile_size(to_i915(fb->dev)) / - intel_tile_width_bytes(fb, color_plane); + return intel_tile_size(to_i915(fb->dev)) / + intel_tile_width_bytes(fb, color_plane); } /* Return the tile dimensions in pixel units */ @@ -2220,16 +2217,8 @@ void intel_add_fb_offsets(int *x, int *y, int color_plane) { - const struct intel_framebuffer *intel_fb = to_intel_framebuffer(state->base.fb); - unsigned int rotation = state->base.rotation; - - if (drm_rotation_90_or_270(rotation)) { - *x += intel_fb->rotated[color_plane].x; - *y += intel_fb->rotated[color_plane].y; - } else { - *x += intel_fb->normal[color_plane].x; - *y += intel_fb->normal[color_plane].y; - } + *x += state->color_plane[color_plane].x; + *y += state->color_plane[color_plane].y; } static u32 intel_adjust_tile_offset(int *x, int *y, @@ -2510,8 +2499,8 @@ bool is_ccs_modifier(u64 modifier) } static -u32 intel_fb_max_stride(struct drm_i915_private *dev_priv, - u32 pixel_format, u64 modifier) +u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv, + u32 pixel_format, u64 modifier) { struct intel_crtc *crtc; struct intel_plane *plane; @@ -2527,13 +2516,102 @@ u32 intel_fb_max_stride(struct drm_i915_private *dev_priv, DRM_MODE_ROTATE_0); } +static +u32 intel_fb_max_stride(struct drm_i915_private *dev_priv, + u32 pixel_format, u64 modifier) +{ + return intel_plane_fb_max_stride(dev_priv, pixel_format, modifier); +} + static u32 intel_fb_stride_alignment(const struct drm_framebuffer *fb, int color_plane) { - if (fb->modifier == DRM_FORMAT_MOD_LINEAR) - return 64; - else + struct drm_i915_private *dev_priv = to_i915(fb->dev); + + if (fb->modifier == DRM_FORMAT_MOD_LINEAR) { + u32 max_stride = intel_plane_fb_max_stride(dev_priv, + fb->format->format, + fb->modifier); + + /* + * To make remapping with linear generally feasible + * we need the stride to be page aligned. + */ + if (fb->pitches[color_plane] > max_stride) + return intel_tile_size(dev_priv); + else + return 64; + } else { return intel_tile_width_bytes(fb, color_plane); + } +} + +bool intel_plane_can_remap(const struct intel_plane_state *plane_state) +{ + struct intel_plane *plane = to_intel_plane(plane_state->base.plane); + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); + const struct drm_framebuffer *fb = plane_state->base.fb; + int i; + + /* We don't want to deal with remapping with cursors */ + if (plane->id == PLANE_CURSOR) + return false; + + /* + * The display engine limits already match/exceed the + * render engine limits, so not much point in remapping. + * Would also need to deal with the fence POT alignment + * and gen2 2KiB GTT tile size. + */ + if (INTEL_GEN(dev_priv) < 4) + return false; + + /* + * The new CCS hash mode isn't compatible with remapping as + * the virtual address of the pages affects the compressed data. + */ + if (is_ccs_modifier(fb->modifier)) + return false; + + /* Linear needs a page aligned stride for remapping */ + if (fb->modifier == DRM_FORMAT_MOD_LINEAR) { + unsigned int alignment = intel_tile_size(dev_priv) - 1; + + for (i = 0; i < fb->format->num_planes; i++) { + if (fb->pitches[i] & alignment) + return false; + } + } + + return true; +} + +static bool intel_plane_needs_remap(const struct intel_plane_state *plane_state) +{ + struct intel_plane *plane = to_intel_plane(plane_state->base.plane); + const struct drm_framebuffer *fb = plane_state->base.fb; + unsigned int rotation = plane_state->base.rotation; + u32 stride, max_stride; + + /* + * No remapping for invisible planes since we don't have + * an actual source viewport to remap. + */ + if (!plane_state->base.visible) + return false; + + if (!intel_plane_can_remap(plane_state)) + return false; + + /* + * FIXME: aux plane limits on gen9+ are + * unclear in Bspec, for now no checking. + */ + stride = intel_fb_pitch(fb, 0, rotation); + max_stride = plane->max_stride(plane, fb->format->format, + fb->modifier, rotation); + + return stride > max_stride; } static int @@ -2701,6 +2779,168 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv, return 0; } +static void +intel_plane_remap_gtt(struct intel_plane_state *plane_state) +{ + struct drm_i915_private *dev_priv = + to_i915(plane_state->base.plane->dev); + struct drm_framebuffer *fb = plane_state->base.fb; + struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); + struct intel_rotation_info *info = &plane_state->view.rotated; + unsigned int rotation = plane_state->base.rotation; + int i, num_planes = fb->format->num_planes; + unsigned int tile_size = intel_tile_size(dev_priv); + unsigned int src_x, src_y; + unsigned int src_w, src_h; + u32 gtt_offset = 0; + + memset(&plane_state->view, 0, sizeof(plane_state->view)); + plane_state->view.type = drm_rotation_90_or_270(rotation) ? + I915_GGTT_VIEW_ROTATED : I915_GGTT_VIEW_REMAPPED; + + src_x = plane_state->base.src.x1 >> 16; + src_y = plane_state->base.src.y1 >> 16; + src_w = drm_rect_width(&plane_state->base.src) >> 16; + src_h = drm_rect_height(&plane_state->base.src) >> 16; + + WARN_ON(is_ccs_modifier(fb->modifier)); + + /* Make src coordinates relative to the viewport */ + drm_rect_translate(&plane_state->base.src, + -(src_x << 16), -(src_y << 16)); + + /* Rotate src coordinates to match rotated GTT view */ + if (drm_rotation_90_or_270(rotation)) + drm_rect_rotate(&plane_state->base.src, + src_w << 16, src_h << 16, + DRM_MODE_ROTATE_270); + + for (i = 0; i < num_planes; i++) { + unsigned int hsub = i ? fb->format->hsub : 1; + unsigned int vsub = i ? fb->format->vsub : 1; + unsigned int cpp = fb->format->cpp[i]; + unsigned int tile_width, tile_height; + unsigned int width, height; + unsigned int pitch_tiles; + unsigned int x, y; + u32 offset; + + intel_tile_dims(fb, i, &tile_width, &tile_height); + + x = src_x / hsub; + y = src_y / vsub; + width = src_w / hsub; + height = src_h / vsub; + + /* + * First pixel of the src viewport from the + * start of the normal gtt mapping. + */ + x += intel_fb->normal[i].x; + y += intel_fb->normal[i].y; + + offset = intel_compute_aligned_offset(dev_priv, &x, &y, + fb, i, fb->pitches[i], + DRM_MODE_ROTATE_0, tile_size); + offset /= tile_size; + + info->plane[i].offset = offset; + info->plane[i].stride = DIV_ROUND_UP(fb->pitches[i], + tile_width * cpp); + info->plane[i].width = DIV_ROUND_UP(x + width, tile_width); + info->plane[i].height = DIV_ROUND_UP(y + height, tile_height); + + if (drm_rotation_90_or_270(rotation)) { + struct drm_rect r; + + /* 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, + info->plane[i].width * tile_width, + info->plane[i].height * tile_height, + DRM_MODE_ROTATE_270); + x = r.x1; + y = r.y1; + + pitch_tiles = info->plane[i].height; + plane_state->color_plane[i].stride = pitch_tiles * tile_height; + + /* rotate the tile dimensions to match the GTT view */ + swap(tile_width, tile_height); + } else { + pitch_tiles = info->plane[i].width; + plane_state->color_plane[i].stride = pitch_tiles * tile_width * cpp; + } + + /* + * 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_width, tile_height, + tile_size, pitch_tiles, + gtt_offset * tile_size, 0); + + gtt_offset += info->plane[i].width * info->plane[i].height; + + plane_state->color_plane[i].offset = 0; + plane_state->color_plane[i].x = x; + plane_state->color_plane[i].y = y; + } +} + +static int +intel_plane_compute_gtt(struct intel_plane_state *plane_state) +{ + const struct intel_framebuffer *fb = + to_intel_framebuffer(plane_state->base.fb); + unsigned int rotation = plane_state->base.rotation; + int i, num_planes; + + if (!fb) + return 0; + + num_planes = fb->base.format->num_planes; + + if (intel_plane_needs_remap(plane_state)) { + intel_plane_remap_gtt(plane_state); + + /* + * Sometimes even remapping can't overcome + * the stride limitations :( Can happen with + * big plane sizes and suitably misaligned + * offsets. + */ + return intel_plane_check_stride(plane_state); + } + + intel_fill_fb_ggtt_view(&plane_state->view, &fb->base, rotation); + + for (i = 0; i < num_planes; i++) { + plane_state->color_plane[i].stride = intel_fb_pitch(&fb->base, i, rotation); + plane_state->color_plane[i].offset = 0; + + if (drm_rotation_90_or_270(rotation)) { + plane_state->color_plane[i].x = fb->rotated[i].x; + plane_state->color_plane[i].y = fb->rotated[i].y; + } else { + plane_state->color_plane[i].x = fb->normal[i].x; + plane_state->color_plane[i].y = fb->normal[i].y; + } + } + + /* Rotate src coordinates to match rotated GTT view */ + if (drm_rotation_90_or_270(rotation)) + drm_rect_rotate(&plane_state->base.src, + fb->base.width << 16, fb->base.height << 16, + DRM_MODE_ROTATE_270); + + return intel_plane_check_stride(plane_state); +} + static int i9xx_format_to_fourcc(int format) { switch (format) { @@ -3199,6 +3439,14 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) plane_state->color_plane[0].x = x; plane_state->color_plane[0].y = y; + /* + * Put the final coordinates back so that the src + * coordinate checks will see the right values. + */ + drm_rect_translate(&plane_state->base.src, + (x << 16) - plane_state->base.src.x1, + (y << 16) - plane_state->base.src.y1); + return 0; } @@ -3255,26 +3503,15 @@ static int skl_check_ccs_aux_surface(struct intel_plane_state *plane_state) 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; - intel_fill_fb_ggtt_view(&plane_state->view, fb, rotation); - plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0, rotation); - plane_state->color_plane[1].stride = intel_fb_pitch(fb, 1, rotation); - - ret = intel_plane_check_stride(plane_state); + ret = intel_plane_compute_gtt(plane_state); if (ret) return ret; if (!plane_state->base.visible) return 0; - /* Rotate src coordinates to match rotated GTT view */ - if (drm_rotation_90_or_270(rotation)) - drm_rect_rotate(&plane_state->base.src, - fb->width << 16, fb->height << 16, - DRM_MODE_ROTATE_270); - /* * Handle the AUX surface first since * the main surface setup depends on it. @@ -3404,20 +3641,20 @@ int i9xx_check_plane_surface(struct intel_plane_state *plane_state) { 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 src_x = plane_state->base.src.x1 >> 16; - int src_y = plane_state->base.src.y1 >> 16; + int src_x, src_y; u32 offset; int ret; - intel_fill_fb_ggtt_view(&plane_state->view, fb, rotation); - plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0, rotation); - - ret = intel_plane_check_stride(plane_state); + ret = intel_plane_compute_gtt(plane_state); if (ret) return ret; + if (!plane_state->base.visible) + return 0; + + src_x = plane_state->base.src.x1 >> 16; + src_y = plane_state->base.src.y1 >> 16; + intel_add_fb_offsets(&src_x, &src_y, plane_state, 0); if (INTEL_GEN(dev_priv) >= 4) @@ -3426,8 +3663,17 @@ int i9xx_check_plane_surface(struct intel_plane_state *plane_state) else offset = 0; + /* + * Put the final coordinates back so that the src + * coordinate checks will see the right values. + */ + drm_rect_translate(&plane_state->base.src, + (src_x << 16) - plane_state->base.src.x1, + (src_y << 16) - plane_state->base.src.y1); + /* HSW/BDW do this automagically in hardware */ if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv)) { + unsigned int rotation = plane_state->base.rotation; int src_w = drm_rect_width(&plane_state->base.src) >> 16; int src_h = drm_rect_height(&plane_state->base.src) >> 16; @@ -3464,6 +3710,10 @@ i9xx_plane_check(struct intel_crtc_state *crtc_state, if (ret) return ret; + ret = i9xx_check_plane_surface(plane_state); + if (ret) + return ret; + if (!plane_state->base.visible) return 0; @@ -3471,10 +3721,6 @@ i9xx_plane_check(struct intel_crtc_state *crtc_state, if (ret) return ret; - ret = i9xx_check_plane_surface(plane_state); - if (ret) - return ret; - plane_state->ctl = i9xx_plane_ctl(crtc_state, plane_state); return 0; @@ -10001,19 +10247,17 @@ static bool intel_cursor_size_ok(const struct intel_plane_state *plane_state) static int intel_cursor_check_surface(struct intel_plane_state *plane_state) { - const struct drm_framebuffer *fb = plane_state->base.fb; - unsigned int rotation = plane_state->base.rotation; int src_x, src_y; u32 offset; int ret; - intel_fill_fb_ggtt_view(&plane_state->view, fb, rotation); - plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0, rotation); - - ret = intel_plane_check_stride(plane_state); + ret = intel_plane_compute_gtt(plane_state); if (ret) return ret; + if (!plane_state->base.visible) + return 0; + src_x = plane_state->base.src_x >> 16; src_y = plane_state->base.src_y >> 16; @@ -10050,6 +10294,10 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state, if (ret) return ret; + ret = intel_cursor_check_surface(plane_state); + if (ret) + return ret; + if (!plane_state->base.visible) return 0; @@ -10057,10 +10305,6 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state, if (ret) return ret; - ret = intel_cursor_check_surface(plane_state); - if (ret) - return ret; - return 0; } diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h index 1b6f5a71184d..500eec90928d 100644 --- a/drivers/gpu/drm/i915/intel_display.h +++ b/drivers/gpu/drm/i915/intel_display.h @@ -29,6 +29,7 @@ #include <drm/i915_drm.h> struct drm_i915_private; +struct intel_plane_state; enum i915_gpio { GPIOA, @@ -435,5 +436,6 @@ void intel_link_compute_m_n(u16 bpp, int nlanes, bool constant_n); bool is_ccs_modifier(u64 modifier); void lpt_disable_clkout_dp(struct drm_i915_private *dev_priv); +bool intel_plane_can_remap(const struct intel_plane_state *plane_state); #endif diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 2913e89280d7..91b461be70be 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -256,6 +256,16 @@ int intel_plane_check_stride(const struct intel_plane_state *plane_state) unsigned int rotation = plane_state->base.rotation; u32 stride, max_stride; + /* + * We ignore stride for all invisible planes that + * can be remapped. Otherwise we could end up + * with a false positive when the remapping didn't + * kick in due the plane being invisible. + */ + if (intel_plane_can_remap(plane_state) && + !plane_state->base.visible) + return 0; + /* FIXME other color planes? */ stride = plane_state->color_plane[0].stride; max_stride = plane->max_stride(plane, fb->format->format, @@ -1417,6 +1427,10 @@ g4x_sprite_check(struct intel_crtc_state *crtc_state, if (ret) return ret; + ret = i9xx_check_plane_surface(plane_state); + if (ret) + return ret; + if (!plane_state->base.visible) return 0; @@ -1428,10 +1442,6 @@ g4x_sprite_check(struct intel_crtc_state *crtc_state, if (ret) return ret; - ret = i9xx_check_plane_surface(plane_state); - if (ret) - return ret; - if (INTEL_GEN(dev_priv) >= 7) plane_state->ctl = ivb_sprite_ctl(crtc_state, plane_state); else @@ -1475,6 +1485,10 @@ vlv_sprite_check(struct intel_crtc_state *crtc_state, if (ret) return ret; + ret = i9xx_check_plane_surface(plane_state); + if (ret) + return ret; + if (!plane_state->base.visible) return 0; @@ -1482,10 +1496,6 @@ vlv_sprite_check(struct intel_crtc_state *crtc_state, if (ret) return ret; - ret = i9xx_check_plane_surface(plane_state); - if (ret) - return ret; - plane_state->ctl = vlv_sprite_ctl(crtc_state, plane_state); return 0; @@ -1639,6 +1649,10 @@ static int skl_plane_check(struct intel_crtc_state *crtc_state, if (ret) return ret; + ret = skl_check_plane_surface(plane_state); + if (ret) + return ret; + if (!plane_state->base.visible) return 0; @@ -1654,10 +1668,6 @@ static int skl_plane_check(struct intel_crtc_state *crtc_state, if (ret) return ret; - ret = skl_check_plane_surface(plane_state); - if (ret) - return ret; - /* HW only has 8 bits pixel precision, disable plane if invisible */ if (!(plane_state->base.alpha >> 8)) plane_state->base.visible = false; -- 2.21.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx