This series adds a bunch of scaling and clipping related utility stuff to drm core, and then implementes real clipping for intel sprite planes.
Most of this stuff was in my drm_atomic branch already for quite a while, but I did do some minor changes here and there.
My glplane test app [1] can now be compiled to use the "legacy" (aka current) drm plane APIs instead of the atomic page flip/modeset APIs. Without the atomic page flip support the test is rather slow due the synchronous vblank waits, and it's also rather ugly (naturally since atomic page flips are needed to compose pretty pictures). But anyways, since the test can throw the plane around and off the screen in various ways, I think it's a decent test case for this code.
From: Ville Syrjälä ville.syrjala@linux.intel.com
struct drm_region represents a two dimensional region. The utility functions are there to help driver writers.
v2: Moved the region stuff into its own file, made the smaller funcs static inline, used 64bit maths in the scaled clipping function to avoid overflows (instead it will saturate to INT_MIN or INT_MAX).
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/Makefile | 3 +- drivers/gpu/drm/drm_region.c | 95 +++++++++++++++++++++++++++++++ include/drm/drm_region.h | 132 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 229 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/drm_region.c create mode 100644 include/drm/drm_region.h
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index b6b43cb..2c0bceb 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -12,7 +12,8 @@ drm-y := drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \ drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \ drm_crtc.o drm_modes.o drm_edid.o \ drm_info.o drm_debugfs.o drm_encoder_slave.o \ - drm_trace_points.o drm_global.o drm_prime.o + drm_trace_points.o drm_global.o drm_prime.o \ + drm_region.o
drm-$(CONFIG_COMPAT) += drm_ioc32.o drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o diff --git a/drivers/gpu/drm/drm_region.c b/drivers/gpu/drm/drm_region.c new file mode 100644 index 0000000..41a3929 --- /dev/null +++ b/drivers/gpu/drm/drm_region.c @@ -0,0 +1,95 @@ +/* + * Copyright (C) 2011-2013 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +#include <linux/errno.h> +#include <linux/export.h> +#include <linux/kernel.h> +#include <drm/drm_region.h> + +/** + * drm_region_clip - clip one region by another region + * @r: region to be clipped + * @clip: clip region + * + * Clip region @r by region @clip. + * + * RETURNS: + * @true if the region is still visible after being clipped, + * @false otherwise. + */ +bool drm_region_clip(struct drm_region *r, const struct drm_region *clip) +{ + r->x1 = max(r->x1, clip->x1); + r->y1 = max(r->y1, clip->y1); + r->x2 = min(r->x2, clip->x2); + r->y2 = min(r->y2, clip->y2); + + return drm_region_visible(r); +} +EXPORT_SYMBOL(drm_region_clip); + +/** + * drm_region_clip_scaled - perform a scaled clip operation + * @src: source window region + * @dst: destination window region + * @clip: clip region + * @hscale: horizontal scaling factor + * @vscale: vertical scaling factor + * + * Clip region @dst by region @clip. Clip region @src by the same + * amounts multiplied by @hscale and @vscale. + * + * RETUTRNS: + * @true if region @dst is still visible after being clipped, + * @false otherwise + */ +bool drm_region_clip_scaled(struct drm_region *src, struct drm_region *dst, + const struct drm_region *clip, + int hscale, int vscale) +{ + int diff; + + diff = clip->x1 - dst->x1; + if (diff > 0) { + int64_t tmp = src->x1 + (int64_t) diff * hscale; + src->x1 = clamp_t(int64_t, tmp, INT_MIN, INT_MAX); + } + diff = clip->y1 - dst->y1; + if (diff > 0) { + int64_t tmp = src->y1 + (int64_t) diff * vscale; + src->y1 = clamp_t(int64_t, tmp, INT_MIN, INT_MAX); + } + diff = dst->x2 - clip->x2; + if (diff > 0) { + int64_t tmp = src->x2 - (int64_t) diff * hscale; + src->x2 = clamp_t(int64_t, tmp, INT_MIN, INT_MAX); + } + diff = dst->y2 - clip->y2; + if (diff > 0) { + int64_t tmp = src->y2 - (int64_t) diff * vscale; + src->y2 = clamp_t(int64_t, tmp, INT_MIN, INT_MAX); + } + + return drm_region_clip(dst, clip); +} +EXPORT_SYMBOL(drm_region_clip_scaled); diff --git a/include/drm/drm_region.h b/include/drm/drm_region.h new file mode 100644 index 0000000..81bed21 --- /dev/null +++ b/include/drm/drm_region.h @@ -0,0 +1,132 @@ +/* + * Copyright (C) 2011-2013 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +#ifndef DRM_REGION_H +#define DRM_REGION_H + +/** + * drm_region - two dimensional region + * @x1: horizontal starting coordinate (inclusive) + * @x2: horizontal ending coordinate (exclusive) + * @y1: vertical starting coordinate (inclusive) + * @y2: vertical ending coordinate (exclusive) + */ +struct drm_region { + int x1, y1, x2, y2; +}; + +/** + * drm_region_adjust_size - adjust the size of the region + * @r: region to be adjusted + * @x: horizontal adjustment + * @y: vertical adjustment + * + * Change the size of region @r by @x in the horizontal direction, + * and by @y in the vertical direction, while keeping the center + * of @r stationary. + * + * Positive @x and @y increase the size, negative values decrease it. + */ +static inline void drm_region_adjust_size(struct drm_region *r, int x, int y) +{ + r->x1 -= x >> 1; + r->y1 -= y >> 1; + r->x2 += (x + 1) >> 1; + r->y2 += (y + 1) >> 1; +} + +/** + * drm_region_translate - translate the region + * @r: region to be tranlated + * @x: horizontal translation + * @y: vertical translation + * + * Move region @r by @x in the horizontal direction, + * and by @y in the vertical direction. + */ +static inline void drm_region_translate(struct drm_region *r, int x, int y) +{ + r->x1 += x; + r->y1 += y; + r->x2 += x; + r->y2 += y; +} + +/** + * drm_region_subsample - subsample a region + * @r: region to be subsampled + * @hsub: horizontal subsampling factor + * @vsub: vertical subsampling factor + * + * Divide the coordinates of region @r by @hsub and @vsub. + */ +static inline void drm_region_subsample(struct drm_region *r, int hsub, int vsub) +{ + r->x1 /= hsub; + r->y1 /= vsub; + r->x2 /= hsub; + r->y2 /= vsub; +} + +/** + * drm_region_width - determine the region width + * @r: region whose width is returned + * + * RETURNS: + * The width of the region. + */ +static inline int drm_region_width(const struct drm_region *r) +{ + return r->x2 - r->x1; +} + +/** + * drm_region_height - determine the region height + * @r: region whose height is returned + * + * RETURNS: + * The height of the region. + */ +static inline int drm_region_height(const struct drm_region *r) +{ + return r->y2 - r->y1; +} + +/** + * drm_region_visible - determine if the the region is visible + * @r: region whose visibility is returned + * + * RETURNS: + * @true if the region is visible, @false otherwise. + */ +static inline bool drm_region_visible(const struct drm_region *r) +{ + return drm_region_width(r) > 0 && drm_region_height(r) > 0; +} + +bool drm_region_clip(struct drm_region *r, const struct drm_region *clip); +bool drm_region_clip_scaled(struct drm_region *src, struct drm_region *dst, + const struct drm_region *clip, + int hscale, int vscale); + +#endif
On Thu, Feb 21, 2013 at 11:35:00PM +0200, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
struct drm_region represents a two dimensional region. The utility functions are there to help driver writers.
v2: Moved the region stuff into its own file, made the smaller funcs static inline, used 64bit maths in the scaled clipping function to avoid overflows (instead it will saturate to INT_MIN or INT_MAX).
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
My criticisms here are mainly that the function names are different than what I am used to for a region API. Namely drm_region_clip, drm_region_subsample would more conventionally be drm_region_intersect, drm_region_downscale. And that a region to me is an ordered list of rectangles, whereas here you just have a single rectangle. -Chris
On Wed, Mar 06, 2013 at 10:56:05AM +0000, Chris Wilson wrote:
On Thu, Feb 21, 2013 at 11:35:00PM +0200, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
struct drm_region represents a two dimensional region. The utility functions are there to help driver writers.
v2: Moved the region stuff into its own file, made the smaller funcs static inline, used 64bit maths in the scaled clipping function to avoid overflows (instead it will saturate to INT_MIN or INT_MAX).
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
My criticisms here are mainly that the function names are different than what I am used to for a region API. Namely drm_region_clip, drm_region_subsample would more conventionally be drm_region_intersect, drm_region_downscale. And that a region to me is an ordered list of rectangles, whereas here you just have a single rectangle.
I'm fine with renaming stuff. Ideally the names should be so obvious that people never have to look at the documentation.
From: Ville Syrjälä ville.syrjala@linux.intel.com
These functions calculcate the scaling factor based on the source and destination regions.
There are two version of the functions, the strict ones that will return an error if the min/max scaling factor is exceeded, and the relaxed versions that will adjust the src/dst regions in order to keep the scaling factor withing the limits.
v2: Return error instead of adjusting regions, refactor common parts into one function, and split into strict and relaxed versions.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_region.c | 173 +++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_region.h | 8 ++ 2 files changed, 181 insertions(+)
diff --git a/drivers/gpu/drm/drm_region.c b/drivers/gpu/drm/drm_region.c index 41a3929..c694424 100644 --- a/drivers/gpu/drm/drm_region.c +++ b/drivers/gpu/drm/drm_region.c @@ -93,3 +93,176 @@ bool drm_region_clip_scaled(struct drm_region *src, struct drm_region *dst, return drm_region_clip(dst, clip); } EXPORT_SYMBOL(drm_region_clip_scaled); + +static int drm_calc_scale(int src, int dst) +{ + int scale = 0; + + if (src < 0 || dst < 0) + return -EINVAL; + + if (dst == 0) + return 0; + + scale = src / dst; + + return scale; +} + +/** + * drm_calc_hscale - calculate the horizontal scaling factor + * @src: source window region + * @dst: destination window region + * @min_hscale: minimum allowed horizontal scaling factor + * @max_hscale: maximum allowed horizontal scaling factor + * + * Calculate the horizontal scaling factor as + * (@src width) / (@dst width). + * + * RETURNS: + * The horizontal scaling factor, or errno of out of limits. + */ +int drm_calc_hscale(struct drm_region *src, struct drm_region *dst, + int min_hscale, int max_hscale) +{ + int src_w = drm_region_width(src); + int dst_w = drm_region_width(dst); + int hscale = drm_calc_scale(src_w, dst_w); + + if (hscale < 0 || dst_w == 0) + return hscale; + + if (hscale < min_hscale || hscale > max_hscale) + return -ERANGE; + + return hscale; +} +EXPORT_SYMBOL(drm_calc_hscale); + +/** + * drm_calc_vscale - calculate the vertical scaling factor + * @src: source window region + * @dst: destination window region + * @min_vscale: minimum allowed vertical scaling factor + * @max_vscale: maximum allowed vertical scaling factor + * + * Calculate the vertical scaling factor as + * (@src height) / (@dst height). + * + * RETURNS: + * The vertical scaling factor, or errno of out of limits. + */ +int drm_calc_vscale(struct drm_region *src, struct drm_region *dst, + int min_vscale, int max_vscale) +{ + int src_h = drm_region_height(src); + int dst_h = drm_region_height(dst); + int vscale = drm_calc_scale(src_h, dst_h); + + if (vscale < 0 || dst_h == 0) + return vscale; + + if (vscale < min_vscale || vscale > max_vscale) + return -ERANGE; + + return vscale; +} +EXPORT_SYMBOL(drm_calc_vscale); + +/** + * drm_calc_hscale_relaxed - calculate the horizontal scaling factor + * @src: source window region + * @dst: destination window region + * @min_hscale: minimum allowed horizontal scaling factor + * @max_hscale: maximum allowed horizontal scaling factor + * + * Calculate the horizontal scaling factor as + * (@src width) / (@dst width). + * + * If the calculated scaling factor is below @min_vscale, + * decrease the height of region @dst to compensate. + * + * If the calculcated scaling factor is above @max_vscale, + * decrease the height of region @src to compensate. + * + * RETURNS: + * The horizontal scaling factor. + */ +int drm_calc_hscale_relaxed(struct drm_region *src, struct drm_region *dst, + int min_hscale, int max_hscale) +{ + int src_w = drm_region_width(src); + int dst_w = drm_region_width(dst); + int hscale = drm_calc_scale(src_w, dst_w); + + if (hscale < 0 || dst_w == 0) + return hscale; + + if (hscale < min_hscale) { + int max_dst_w = src_w / min_hscale; + + drm_region_adjust_size(dst, max_dst_w - dst_w, 0); + + return min_hscale; + } + + if (hscale > max_hscale) { + int max_src_w = dst_w * max_hscale; + + drm_region_adjust_size(src, max_src_w - src_w, 0); + + return max_hscale; + } + + return hscale; +} +EXPORT_SYMBOL(drm_calc_hscale_relaxed); + +/** + * drm_calc_vscale_relaxed - calculate the vertical scaling factor + * @src: source window region + * @dst: destination window region + * @min_vscale: minimum allowed vertical scaling factor + * @max_vscale: maximum allowed vertical scaling factor + * + * Calculate the vertical scaling factor as + * (@src height) / (@dst height). + * + * If the calculated scaling factor is below @min_vscale, + * decrease the height of region @dst to compensate. + * + * If the calculcated scaling factor is above @max_vscale, + * decrease the height of region @src to compensate. + * + * RETURNS: + * The vertical scaling factor. + */ +int drm_calc_vscale_relaxed(struct drm_region *src, struct drm_region *dst, + int min_vscale, int max_vscale) +{ + int src_h = drm_region_height(src); + int dst_h = drm_region_height(dst); + int vscale = drm_calc_scale(src_h, dst_h); + + if (vscale < 0 || dst_h == 0) + return vscale; + + if (vscale < min_vscale) { + int max_dst_h = src_h / min_vscale; + + drm_region_adjust_size(dst, 0, max_dst_h - dst_h); + + return min_vscale; + } + + if (vscale > max_vscale) { + int max_src_h = dst_h * max_vscale; + + drm_region_adjust_size(src, 0, max_src_h - src_h); + + return max_vscale; + } + + return vscale; +} +EXPORT_SYMBOL(drm_calc_vscale_relaxed); diff --git a/include/drm/drm_region.h b/include/drm/drm_region.h index 81bed21..c12d953 100644 --- a/include/drm/drm_region.h +++ b/include/drm/drm_region.h @@ -128,5 +128,13 @@ bool drm_region_clip(struct drm_region *r, const struct drm_region *clip); bool drm_region_clip_scaled(struct drm_region *src, struct drm_region *dst, const struct drm_region *clip, int hscale, int vscale); +int drm_calc_hscale(struct drm_region *src, struct drm_region *dst, + int min_hscale, int max_hscale); +int drm_calc_vscale(struct drm_region *src, struct drm_region *dst, + int min_vscale, int max_vscale); +int drm_calc_hscale_relaxed(struct drm_region *src, struct drm_region *dst, + int min_hscale, int max_hscale); +int drm_calc_vscale_relaxed(struct drm_region *src, struct drm_region *dst, + int min_vscale, int max_vscale);
#endif
From: Ville Syrjälä ville.syrjala@linux.intel.com
Add a debug function to print the region in a human readable format.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_region.c | 22 ++++++++++++++++++++++ include/drm/drm_region.h | 1 + 2 files changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/drm_region.c b/drivers/gpu/drm/drm_region.c index c694424..82e1043 100644 --- a/drivers/gpu/drm/drm_region.c +++ b/drivers/gpu/drm/drm_region.c @@ -24,6 +24,7 @@ #include <linux/errno.h> #include <linux/export.h> #include <linux/kernel.h> +#include <drm/drmP.h> #include <drm/drm_region.h>
/** @@ -266,3 +267,24 @@ int drm_calc_vscale_relaxed(struct drm_region *src, struct drm_region *dst, return vscale; } EXPORT_SYMBOL(drm_calc_vscale_relaxed); + +/** + * drm_region_debug - print the region information + * @r: region to print + * @fixed_point: is the region in 16.16 fixed point format + */ +void drm_region_debug(const struct drm_region *r, bool fixed_point) +{ + int w = drm_region_width(r); + int h = drm_region_height(r); + + if (fixed_point) + DRM_DEBUG_KMS("%d.%06ux%d.%06u+%d.%06u+%d.%06u\n", + w >> 16, ((w & 0xffff) * 15625) >> 10, + h >> 16, ((h & 0xffff) * 15625) >> 10, + r->x1 >> 16, ((r->x1 & 0xffff) * 15625) >> 10, + r->y1 >> 16, ((r->y1 & 0xffff) * 15625) >> 10); + else + DRM_DEBUG_KMS("%ux%u+%d+%d\n", w, h, r->x1, r->y1); +} +EXPORT_SYMBOL(drm_region_debug); diff --git a/include/drm/drm_region.h b/include/drm/drm_region.h index c12d953..10591f5 100644 --- a/include/drm/drm_region.h +++ b/include/drm/drm_region.h @@ -136,5 +136,6 @@ int drm_calc_hscale_relaxed(struct drm_region *src, struct drm_region *dst, int min_hscale, int max_hscale); int drm_calc_vscale_relaxed(struct drm_region *src, struct drm_region *dst, int min_vscale, int max_vscale); +void drm_region_debug(const struct drm_region *r, bool fixed_point);
#endif
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 relaxed drm_region functions. That means that the src/dst regions are reduced in size in order to keep the scaling factor within the limits.
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().
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/intel_sprite.c | 184 +++++++++++++++++++++++++++--------- 1 file changed, 138 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index d086e48..57001c4 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_region.h> #include "intel_drv.h" #include <drm/i915_drm.h> #include "i915_drv.h" @@ -415,6 +416,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, @@ -432,28 +447,51 @@ 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_region src = { + .x1 = src_x, + .x2 = src_x + src_w, + .y1 = src_y, + .y2 = src_y + src_h, + }; + struct drm_region dst = { + .x1 = crtc_x, + .x2 = crtc_x + crtc_w, + .y1 = crtc_y, + .y2 = crtc_y + crtc_h, + }; + const struct drm_region clip = { + .x2 = crtc->mode.hdisplay, + .y2 = crtc->mode.vdisplay, + };
intel_fb = to_intel_framebuffer(fb); obj = intel_fb->obj;
old_obj = intel_plane->obj;
- 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) { @@ -461,53 +499,104 @@ 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. + * FIXME the following code does a bunch of fuzzy adjustments to the + * coordinates and sizes. We probably need some way to decide whether + * more strict checking should be done instead. */ - 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_calc_hscale_relaxed(&src, &dst, min_scale, max_scale); + BUG_ON(hscale < 0); + + vscale = drm_calc_vscale_relaxed(&src, &dst, min_scale, max_scale); + BUG_ON(vscale < 0); + + visible = drm_region_clip_scaled(&src, &dst, &clip, hscale, vscale); + + if (visible) { + /* check again in case clipping clamped the results */ + hscale = drm_calc_hscale(&src, &dst, min_scale, max_scale); + if (hscale < 0) { + DRM_DEBUG_KMS("Horizontal scaling factor out of limits\n"); + drm_region_debug(&src, true); + drm_region_debug(&dst, false); + return hscale; + } + + vscale = drm_calc_vscale(&src, &dst, min_scale, max_scale); + if (vscale < 0) { + DRM_DEBUG_KMS("Vertical scaling factor out of limits\n"); + drm_region_debug(&src, true); + drm_region_debug(&dst, false); + return vscale; + } + + /* Make the source viewport size an exact multiple of the scaling factors. */ + drm_region_adjust_size(&src, + drm_region_width(&dst) * hscale - drm_region_width(&src), + drm_region_height(&dst) * vscale - drm_region_height(&src)); + + crtc_x = dst.x1; + crtc_y = dst.y1; + crtc_w = drm_region_width(&dst); + crtc_h = drm_region_height(&dst); + + /* + * 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. + */ + src_x = src.x1 >> 16; + src_w = drm_region_width(&src) >> 16; + src_y = src.y1 >> 16; + src_h = drm_region_height(&src) >> 16; + + if (format_is_yuv(fb->pixel_format)) { + src_x &= ~1; + src_w &= ~1; + } } - 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; + + /* Account for minimum size when scaling */ + if (visible && (src_w != crtc_w || src_h != crtc_h)) { + BUG_ON(!intel_plane->can_scale); + + /* FIXME interlacing min height is 6 */ + /* FIXME return an error instead? */ + + if (crtc_w < 3 || crtc_h < 3) + visible = false; + + if (src_w < 3 || src_h < 3) + visible = false; } - 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; + /* Check size restrictions when scaling */ + if (visible && (src_w != crtc_w || src_h != crtc_h)) { + unsigned int width_bytes = ((src_x * pixel_size) & 63) + src_w * pixel_size;
- /* - * 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; + BUG_ON(!intel_plane->can_scale);
- /* - * 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; + 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; + } + }
/* * If the sprite is completely covering the primary plane, * we can disable the primary and save power. */ - if ((crtc_x == 0) && (crtc_y == 0) && + if (visible && + (crtc_x == 0) && (crtc_y == 0) && (crtc_w == primary_w) && (crtc_h == primary_h)) disable_primary = true;
@@ -526,11 +615,15 @@ 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);
- if (disable_primary) - intel_disable_primary(crtc); + if (disable_primary) + intel_disable_primary(crtc); + } else + intel_plane->disable_plane(plane);
/* Unpin old obj after new one is active to avoid ugliness */ if (old_obj) { @@ -550,7 +643,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
out_unlock: mutex_unlock(&dev->struct_mutex); -out: return ret; }
dri-devel@lists.freedesktop.org
-
Chris Wilson -
Ville Syrjälä -
ville.syrjala＠linux.intel.com