diff mbox

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

Message ID 1361482503-3268-2-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Feb. 21, 2013, 9:35 p.m. UTC
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

Comments

Chris Wilson March 6, 2013, 10:56 a.m. UTC | #1
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
Ville Syrjälä March 6, 2013, 11:10 a.m. UTC | #2
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.
diff mbox

Patch

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