From: Ville Syrjälä ville.syrjala@linux.intel.com
Properly clip the source when the destination gets clipped by the pipe dimensions.
Sadly the video sprite hardware is rather limited so it can't do proper sub-pixel postitioning. Resort to truncating the source coordinates to (macro)pixel boundary.
The scaling checks are done using the strict drm_region functions. Which means that an error is returned when the min/max scaling ratios are exceeded.
Also do some additional checking against various hardware limits.
v2: Truncate src coords instead of rounding to avoid increasing src viewport size, and adapt to changes in drm_calc_{h,v}scale(). v3: Adapt to drm_region->drm_rect rename. Fix misaligned crtc_w for packed YUV formats when scaling isn't supported. v4: Use stricter scaling checks, use drm_rect_equals() v5: If sprite is below min size, make it invisible instead returning an error. Use WARN_ON() instead if BUG_ON(), and add one to sanity check the src viewport size. v6: Add comments to remind about src and dst coordinate types
Reviewed-by: Chris Wilson chris@chris-wilson.co.uk Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/intel_sprite.c | 206 +++++++++++++++++++++++++++--------- 1 file changed, 154 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index c7d25c5..044cc263 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -32,6 +32,7 @@ #include <drm/drmP.h> #include <drm/drm_crtc.h> #include <drm/drm_fourcc.h> +#include <drm/drm_rect.h> #include "intel_drv.h" #include <drm/i915_drm.h> #include "i915_drv.h" @@ -583,6 +584,20 @@ ilk_get_colorkey(struct drm_plane *plane, struct drm_intel_sprite_colorkey *key) key->flags = I915_SET_COLORKEY_NONE; }
+static bool +format_is_yuv(uint32_t format) +{ + switch (format) { + case DRM_FORMAT_YUYV: + case DRM_FORMAT_UYVY: + case DRM_FORMAT_VYUY: + case DRM_FORMAT_YVYU: + return true; + default: + return false; + } +} + static int intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, struct drm_framebuffer *fb, int crtc_x, int crtc_y, @@ -600,9 +615,29 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv, pipe); int ret = 0; - int x = src_x >> 16, y = src_y >> 16; - int primary_w = crtc->mode.hdisplay, primary_h = crtc->mode.vdisplay; bool disable_primary = false; + bool visible; + int hscale, vscale; + int max_scale, min_scale; + int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); + struct drm_rect src = { + /* sample coordinates in 16.16 fixed point */ + .x1 = src_x, + .x2 = src_x + src_w, + .y1 = src_y, + .y2 = src_y + src_h, + }; + struct drm_rect dst = { + /* integer pixels */ + .x1 = crtc_x, + .x2 = crtc_x + crtc_w, + .y1 = crtc_y, + .y2 = crtc_y + crtc_h, + }; + const struct drm_rect clip = { + .x2 = crtc->mode.hdisplay, + .y2 = crtc->mode.vdisplay, + };
intel_fb = to_intel_framebuffer(fb); obj = intel_fb->obj; @@ -618,19 +653,23 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, intel_plane->src_w = src_w; intel_plane->src_h = src_h;
- src_w = src_w >> 16; - src_h = src_h >> 16; - /* Pipe must be running... */ - if (!(I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_ENABLE)) - return -EINVAL; - - if (crtc_x >= primary_w || crtc_y >= primary_h) + if (!(I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_ENABLE)) { + DRM_DEBUG_KMS("Pipe disabled\n"); return -EINVAL; + }
/* Don't modify another pipe's plane */ - if (intel_plane->pipe != intel_crtc->pipe) + if (intel_plane->pipe != intel_crtc->pipe) { + DRM_DEBUG_KMS("Wrong plane <-> crtc mapping\n"); return -EINVAL; + } + + /* FIXME check all gen limits */ + if (fb->width < 3 || fb->height < 3 || fb->pitches[0] > 16384) { + DRM_DEBUG_KMS("Unsuitable framebuffer for plane\n"); + return -EINVAL; + }
/* Sprite planes can be linear or x-tiled surfaces */ switch (obj->tiling_mode) { @@ -638,55 +677,115 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, case I915_TILING_X: break; default: + DRM_DEBUG_KMS("Unsupported tiling mode\n"); return -EINVAL; }
- /* - * Clamp the width & height into the visible area. Note we don't - * try to scale the source if part of the visible region is offscreen. - * The caller must handle that by adjusting source offset and size. - */ - if ((crtc_x < 0) && ((crtc_x + crtc_w) > 0)) { - crtc_w += crtc_x; - crtc_x = 0; + max_scale = intel_plane->max_downscale << 16; + min_scale = intel_plane->can_scale ? 1 : (1 << 16); + + hscale = drm_rect_calc_hscale(&src, &dst, min_scale, max_scale); + if (hscale < 0) { + DRM_DEBUG_KMS("Horizontal scaling factor out of limits\n"); + drm_rect_debug_print(&src, true); + drm_rect_debug_print(&dst, false); + + return hscale; } - if ((crtc_x + crtc_w) <= 0) /* Nothing to display */ - goto out; - if ((crtc_x + crtc_w) > primary_w) - crtc_w = primary_w - crtc_x; - - if ((crtc_y < 0) && ((crtc_y + crtc_h) > 0)) { - crtc_h += crtc_y; - crtc_y = 0; + + vscale = drm_rect_calc_vscale(&src, &dst, min_scale, max_scale); + if (vscale < 0) { + DRM_DEBUG_KMS("Vertical scaling factor out of limits\n"); + drm_rect_debug_print(&src, true); + drm_rect_debug_print(&dst, false); + + return vscale; } - if ((crtc_y + crtc_h) <= 0) /* Nothing to display */ - goto out; - if (crtc_y + crtc_h > primary_h) - crtc_h = primary_h - crtc_y; - - if (!crtc_w || !crtc_h) /* Again, nothing to display */ - goto out; - - /* - * We may not have a scaler, eg. HSW does not have it any more - */ - if (!intel_plane->can_scale && (crtc_w != src_w || crtc_h != src_h)) - return -EINVAL; - - /* - * We can take a larger source and scale it down, but - * only so much... 16x is the max on SNB. - */ - if (((src_w * src_h) / (crtc_w * crtc_h)) > intel_plane->max_downscale) - return -EINVAL; + + visible = drm_rect_clip_scaled(&src, &dst, &clip, hscale, vscale); + + crtc_x = dst.x1; + crtc_y = dst.y1; + crtc_w = drm_rect_width(&dst); + crtc_h = drm_rect_height(&dst); + + if (visible) { + /* Make the source viewport size an exact multiple of the scaling factors. */ + drm_rect_adjust_size(&src, + drm_rect_width(&dst) * hscale - drm_rect_width(&src), + drm_rect_height(&dst) * vscale - drm_rect_height(&src)); + + /* sanity check to make sure the src viewport wasn't enlarged */ + WARN_ON(src.x1 < (int) src_x || + src.y1 < (int) src_y || + src.x2 > (int) (src_x + src_w) || + src.y2 > (int) (src_y + src_h)); + + /* + * Hardware doesn't handle subpixel coordinates. + * Adjust to (macro)pixel boundary, but be careful not to + * increase the source viewport size, because that could + * push the downscaling factor out of bounds. + * + * FIXME Should we be really strict and reject the + * config if it results in non (macro)pixel aligned + * coords? + */ + src_x = src.x1 >> 16; + src_w = drm_rect_width(&src) >> 16; + src_y = src.y1 >> 16; + src_h = drm_rect_height(&src) >> 16; + + if (format_is_yuv(fb->pixel_format)) { + src_x &= ~1; + src_w &= ~1; + + /* + * Must keep src and dst the + * same if we can't scale. + */ + if (!intel_plane->can_scale) + crtc_w &= ~1; + + if (crtc_w == 0) + visible = false; + } + } + + /* Check size restrictions when scaling */ + if (visible && (src_w != crtc_w || src_h != crtc_h)) { + unsigned int width_bytes; + + WARN_ON(!intel_plane->can_scale); + + /* FIXME interlacing min height is 6 */ + + if (crtc_w < 3 || crtc_h < 3) + visible = false; + + if (src_w < 3 || src_h < 3) + visible = false; + + width_bytes = ((src_x * pixel_size) & 63) + src_w * pixel_size; + + if (src_w > 2048 || src_h > 2048 || + width_bytes > 4096 || fb->pitches[0] > 4096) { + DRM_DEBUG_KMS("Source dimensions exceed hardware limits\n"); + return -EINVAL; + } + } + + dst.x1 = crtc_x; + dst.x2 = crtc_x + crtc_w; + dst.y1 = crtc_y; + dst.y2 = crtc_y + crtc_h;
/* * If the sprite is completely covering the primary plane, * we can disable the primary and save power. */ - if ((crtc_x == 0) && (crtc_y == 0) && - (crtc_w == primary_w) && (crtc_h == primary_h)) - disable_primary = true; + disable_primary = drm_rect_equals(&dst, &clip); + WARN_ON(disable_primary && !visible);
mutex_lock(&dev->struct_mutex);
@@ -708,8 +807,12 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, if (!disable_primary) intel_enable_primary(crtc);
- intel_plane->update_plane(plane, fb, obj, crtc_x, crtc_y, - crtc_w, crtc_h, x, y, src_w, src_h); + if (visible) + intel_plane->update_plane(plane, fb, obj, + crtc_x, crtc_y, crtc_w, crtc_h, + src_x, src_y, src_w, src_h); + else + intel_plane->disable_plane(plane);
if (disable_primary) intel_disable_primary(crtc); @@ -732,7 +835,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
out_unlock: mutex_unlock(&dev->struct_mutex); -out: return ret; }