[Intel-gfx] [PATCH v3 4/4] drm/i915: Implement proper clipping for video sprites
From: Ville Syrjälä ville.syrj...@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().
v3: Adapt to drm_region-drm_rect rename. Fix misaligned crtc_w for
packed YUV formats when scaling isn't supported.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_sprite.c | 191 +++-
1 file changed, 145 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index c7d25c5..72b2ce4 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,28 @@ 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 = {
+ .x1 = src_x,
+ .x2 = src_x + src_w,
+ .y1 = src_y,
+ .y2 = src_y + src_h,
+ };
+ struct drm_rect dst = {
+ .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 +652,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,53 +676,111 @@ 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
Re: [Intel-gfx] [PATCH v3 4/4] drm/i915: Implement proper clipping for video sprites
On Tue, Apr 16, 2013 at 01:47:22PM +0300, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@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().
v3: Adapt to drm_region-drm_rect rename. Fix misaligned crtc_w for
packed YUV formats when scaling isn't supported.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
Skipping to the end, use of drm_rect looks good.
The one ugly that stood out is:
/*
* 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;
which would be
disable_primary = drm_rect_equals(dst, clip);
BUG_ON(disable_primary !visible);
with a little bit of massaging of crtc/dst.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 4/4] drm/i915: Implement proper clipping for video sprites
On Tue, Apr 16, 2013 at 02:37:24PM +0100, Chris Wilson wrote:
On Tue, Apr 16, 2013 at 01:47:22PM +0300, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@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().
v3: Adapt to drm_region-drm_rect rename. Fix misaligned crtc_w for
packed YUV formats when scaling isn't supported.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
Skipping to the end, use of drm_rect looks good.
The one ugly that stood out is:
/*
* 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;
which would be
disable_primary = drm_rect_equals(dst, clip);
BUG_ON(disable_primary !visible);
with a little bit of massaging of crtc/dst.
Yeah, looks nicer. I'll add drm_rect_equals() to our repertoire.
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 4/4] drm/i915: Implement proper clipping for video sprites
From: Ville Syrjälä ville.syrj...@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().
v3: Adapt to drm_region-drm_rect rename. Fix misaligned crtc_w for
packed YUV formats when scaling isn't supported.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_sprite.c | 191 +++-
1 file changed, 145 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 414d325..62e09d1 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
@@ -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,9 +447,28 @@ 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 = {
+ .x1 = src_x,
+ .x2 = src_x + src_w,
+ .y1 = src_y,
+ .y2 = src_y + src_h,
+ };
+ struct drm_rect dst = {
+ .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;
@@ -450,19 +484,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) {
@@ -470,53 +508,111 @@ 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
