[PATCH 0/4] drm/i915: Sprite clipping with related utility funcs to drm core

[PATCH 0/4] drm/i915: Sprite clipping with related utility funcs to drm core

[ville.syrjala]

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.

[1] https://gitorious.org/vsyrjala/glplane

[PATCH 1/4] drm: Add struct drm_region and assorted utility functions

[ville.syrjala]

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
-- 
1.7.12.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/4] drm: Add struct drm_region and assorted utility functions

[Chris Wilson]

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

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [Intel-gfx] [PATCH 1/4] drm: Add struct drm_region and assorted utility functions

[Ville Syrjälä]

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.

-- 
Ville Syrjälä
Intel OTC

[PATCH 2/4] drm: Add drm_calc_{hscale, vscale}() utility functions

[ville.syrjala]

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
-- 
1.7.12.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/4] drm: Add drm_region_debug()

[ville.syrjala]

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
-- 
1.7.12.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/4] drm/i915: Implement proper clipping for video sprites

[ville.syrjala]

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;
 }
 
-- 
1.7.12.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx