[PATCH v2 0/4] drm/rect: Bugfixes and selftests

[PATCH v2 0/4] drm/rect: Bugfixes and selftests

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

My earlier fixes for drm_rect + div-by-zero fix + some
selftests that Daniel requested.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Benjamin Gaignard <benjamin.gaignard@st.com>
Cc: Daniel Vetter <daniel@ffwll.ch>

Ville Syrjälä (4):
  drm/rect: Avoid division by zero
  drm/rect: Keep the scaled clip bounded
  drm/rect: Keep the clipped dst rectangle in place
  drm/selftests: Add drm_rect selftests

 drivers/gpu/drm/drm_rect.c                    |  36 +--
 drivers/gpu/drm/selftests/Makefile            |   3 +-
 .../gpu/drm/selftests/drm_modeset_selftests.h |   4 +
 .../drm/selftests/test-drm_modeset_common.h   |   7 +
 drivers/gpu/drm/selftests/test-drm_rect.c     | 220 ++++++++++++++++++
 include/drm/drm_rect.h                        |   2 +
 6 files changed, 257 insertions(+), 15 deletions(-)
 create mode 100644 drivers/gpu/drm/selftests/test-drm_rect.c

-- 
2.23.0

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

[PATCH v2 1/4] drm/rect: Avoid division by zero

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Check for zero width/height destination rectangle in
drm_rect_clip_scaled() to avoid a division by zero.

Cc: stable@vger.kernel.org
Fixes: f96bdf564f3e ("drm/rect: Handle rounding errors in drm_rect_clip_scaled, v3.")
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Benjamin Gaignard <benjamin.gaignard@st.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Testcase: igt/kms_selftest/drm_rect_clip_scaled_div_by_zero
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_rect.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
index b8363aaa9032..818738e83d06 100644
--- a/drivers/gpu/drm/drm_rect.c
+++ b/drivers/gpu/drm/drm_rect.c
@@ -54,7 +54,12 @@ EXPORT_SYMBOL(drm_rect_intersect);
 
 static u32 clip_scaled(u32 src, u32 dst, u32 clip)
 {
-	u64 tmp = mul_u32_u32(src, dst - clip);
+	u64 tmp;
+
+	if (dst == 0)
+		return 0;
+
+	tmp = mul_u32_u32(src, dst - clip);
 
 	/*
 	 * Round toward 1.0 when clipping so that we don't accidentally
-- 
2.23.0

[PATCH v2 1/4] drm/rect: Avoid division by zero

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Check for zero width/height destination rectangle in
drm_rect_clip_scaled() to avoid a division by zero.

Cc: stable@vger.kernel.org
Fixes: f96bdf564f3e ("drm/rect: Handle rounding errors in drm_rect_clip_scaled, v3.")
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Benjamin Gaignard <benjamin.gaignard@st.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Testcase: igt/kms_selftest/drm_rect_clip_scaled_div_by_zero
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_rect.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
index b8363aaa9032..818738e83d06 100644
--- a/drivers/gpu/drm/drm_rect.c
+++ b/drivers/gpu/drm/drm_rect.c
@@ -54,7 +54,12 @@ EXPORT_SYMBOL(drm_rect_intersect);
 
 static u32 clip_scaled(u32 src, u32 dst, u32 clip)
 {
-	u64 tmp = mul_u32_u32(src, dst - clip);
+	u64 tmp;
+
+	if (dst == 0)
+		return 0;
+
+	tmp = mul_u32_u32(src, dst - clip);
 
 	/*
 	 * Round toward 1.0 when clipping so that we don't accidentally
-- 
2.23.0

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

[PATCH v2 2/4] drm/rect: Keep the scaled clip bounded

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Limit the scaled clip to only clip at most dst_w/h pixels.
This avoids the problem with clip_scaled() not being able
to return negative values. Since new_src_w/h is now properly
bounded we can remove the clamp()s.

Cc: Benjamin Gaignard <benjamin.gaignard@st.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Testcase: igt/kms_selftest/drm_rect_clip_scaled_signed_vs_unsigned
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_rect.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
index 818738e83d06..a9c7f90836f3 100644
--- a/drivers/gpu/drm/drm_rect.c
+++ b/drivers/gpu/drm/drm_rect.c
@@ -59,6 +59,9 @@ static u32 clip_scaled(u32 src, u32 dst, u32 clip)
 	if (dst == 0)
 		return 0;
 
+	/* Only clip what we have. Keeps the result bounded. */
+	clip = min(clip, dst);
+
 	tmp = mul_u32_u32(src, dst - clip);
 
 	/*
@@ -94,7 +97,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
 		u32 new_src_w = clip_scaled(drm_rect_width(src),
 					    drm_rect_width(dst), diff);
 
-		src->x1 = clamp_t(int64_t, src->x2 - new_src_w, INT_MIN, INT_MAX);
+		src->x1 = src->x2 - new_src_w;
 		dst->x1 = clip->x1;
 	}
 	diff = clip->y1 - dst->y1;
@@ -102,7 +105,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
 		u32 new_src_h = clip_scaled(drm_rect_height(src),
 					    drm_rect_height(dst), diff);
 
-		src->y1 = clamp_t(int64_t, src->y2 - new_src_h, INT_MIN, INT_MAX);
+		src->y1 = src->y2 - new_src_h;
 		dst->y1 = clip->y1;
 	}
 	diff = dst->x2 - clip->x2;
@@ -110,7 +113,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
 		u32 new_src_w = clip_scaled(drm_rect_width(src),
 					    drm_rect_width(dst), diff);
 
-		src->x2 = clamp_t(int64_t, src->x1 + new_src_w, INT_MIN, INT_MAX);
+		src->x2 = src->x1 + new_src_w;
 		dst->x2 = clip->x2;
 	}
 	diff = dst->y2 - clip->y2;
@@ -118,7 +121,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
 		u32 new_src_h = clip_scaled(drm_rect_height(src),
 					    drm_rect_height(dst), diff);
 
-		src->y2 = clamp_t(int64_t, src->y1 + new_src_h, INT_MIN, INT_MAX);
+		src->y2 = src->y1 + new_src_h;
 		dst->y2 = clip->y2;
 	}
 
-- 
2.23.0

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

[PATCH v2 3/4] drm/rect: Keep the clipped dst rectangle in place

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Now that we've constrained the clipped source rectangle such
that it can't have negative dimensions doing the same for the
dst rectangle seems appropriate. Should at least result in
the clipped src and dst rectangles being a bit more consistent
with each other.

Cc: Benjamin Gaignard <benjamin.gaignard@st.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_rect.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
index a9c7f90836f3..1e1e2101007a 100644
--- a/drivers/gpu/drm/drm_rect.c
+++ b/drivers/gpu/drm/drm_rect.c
@@ -52,7 +52,7 @@ bool drm_rect_intersect(struct drm_rect *r1, const struct drm_rect *r2)
 }
 EXPORT_SYMBOL(drm_rect_intersect);
 
-static u32 clip_scaled(u32 src, u32 dst, u32 clip)
+static u32 clip_scaled(int src, int dst, int *clip)
 {
 	u64 tmp;
 
@@ -60,9 +60,9 @@ static u32 clip_scaled(u32 src, u32 dst, u32 clip)
 		return 0;
 
 	/* Only clip what we have. Keeps the result bounded. */
-	clip = min(clip, dst);
+	*clip = min(*clip, dst);
 
-	tmp = mul_u32_u32(src, dst - clip);
+	tmp = mul_u32_u32(src, dst - *clip);
 
 	/*
 	 * Round toward 1.0 when clipping so that we don't accidentally
@@ -95,34 +95,34 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
 	diff = clip->x1 - dst->x1;
 	if (diff > 0) {
 		u32 new_src_w = clip_scaled(drm_rect_width(src),
-					    drm_rect_width(dst), diff);
+					    drm_rect_width(dst), &diff);
 
 		src->x1 = src->x2 - new_src_w;
-		dst->x1 = clip->x1;
+		dst->x1 += diff;
 	}
 	diff = clip->y1 - dst->y1;
 	if (diff > 0) {
 		u32 new_src_h = clip_scaled(drm_rect_height(src),
-					    drm_rect_height(dst), diff);
+					    drm_rect_height(dst), &diff);
 
 		src->y1 = src->y2 - new_src_h;
-		dst->y1 = clip->y1;
+		dst->y1 += diff;
 	}
 	diff = dst->x2 - clip->x2;
 	if (diff > 0) {
 		u32 new_src_w = clip_scaled(drm_rect_width(src),
-					    drm_rect_width(dst), diff);
+					    drm_rect_width(dst), &diff);
 
 		src->x2 = src->x1 + new_src_w;
-		dst->x2 = clip->x2;
+		dst->x2 -= diff;
 	}
 	diff = dst->y2 - clip->y2;
 	if (diff > 0) {
 		u32 new_src_h = clip_scaled(drm_rect_height(src),
-					    drm_rect_height(dst), diff);
+					    drm_rect_height(dst), &diff);
 
 		src->y2 = src->y1 + new_src_h;
-		dst->y2 = clip->y2;
+		dst->y2 -= diff;
 	}
 
 	return drm_rect_visible(dst);
-- 
2.23.0

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

[PATCH v2 3/4] drm/rect: Keep the clipped dst rectangle in place

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Now that we've constrained the clipped source rectangle such
that it can't have negative dimensions doing the same for the
dst rectangle seems appropriate. Should at least result in
the clipped src and dst rectangles being a bit more consistent
with each other.

Cc: Benjamin Gaignard <benjamin.gaignard@st.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_rect.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
index a9c7f90836f3..1e1e2101007a 100644
--- a/drivers/gpu/drm/drm_rect.c
+++ b/drivers/gpu/drm/drm_rect.c
@@ -52,7 +52,7 @@ bool drm_rect_intersect(struct drm_rect *r1, const struct drm_rect *r2)
 }
 EXPORT_SYMBOL(drm_rect_intersect);
 
-static u32 clip_scaled(u32 src, u32 dst, u32 clip)
+static u32 clip_scaled(int src, int dst, int *clip)
 {
 	u64 tmp;
 
@@ -60,9 +60,9 @@ static u32 clip_scaled(u32 src, u32 dst, u32 clip)
 		return 0;
 
 	/* Only clip what we have. Keeps the result bounded. */
-	clip = min(clip, dst);
+	*clip = min(*clip, dst);
 
-	tmp = mul_u32_u32(src, dst - clip);
+	tmp = mul_u32_u32(src, dst - *clip);
 
 	/*
 	 * Round toward 1.0 when clipping so that we don't accidentally
@@ -95,34 +95,34 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
 	diff = clip->x1 - dst->x1;
 	if (diff > 0) {
 		u32 new_src_w = clip_scaled(drm_rect_width(src),
-					    drm_rect_width(dst), diff);
+					    drm_rect_width(dst), &diff);
 
 		src->x1 = src->x2 - new_src_w;
-		dst->x1 = clip->x1;
+		dst->x1 += diff;
 	}
 	diff = clip->y1 - dst->y1;
 	if (diff > 0) {
 		u32 new_src_h = clip_scaled(drm_rect_height(src),
-					    drm_rect_height(dst), diff);
+					    drm_rect_height(dst), &diff);
 
 		src->y1 = src->y2 - new_src_h;
-		dst->y1 = clip->y1;
+		dst->y1 += diff;
 	}
 	diff = dst->x2 - clip->x2;
 	if (diff > 0) {
 		u32 new_src_w = clip_scaled(drm_rect_width(src),
-					    drm_rect_width(dst), diff);
+					    drm_rect_width(dst), &diff);
 
 		src->x2 = src->x1 + new_src_w;
-		dst->x2 = clip->x2;
+		dst->x2 -= diff;
 	}
 	diff = dst->y2 - clip->y2;
 	if (diff > 0) {
 		u32 new_src_h = clip_scaled(drm_rect_height(src),
-					    drm_rect_height(dst), diff);
+					    drm_rect_height(dst), &diff);
 
 		src->y2 = src->y1 + new_src_h;
-		dst->y2 = clip->y2;
+		dst->y2 -= diff;
 	}
 
 	return drm_rect_visible(dst);
-- 
2.23.0

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

[PATCH v2 4/4] drm/selftests: Add drm_rect selftests

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add selftests for drm_rect. A few basic ones for clipped and unclipped
cases, and a few special ones for specific bugs we had in the code.

I'm too lazy to think of more corner cases to check at this time.
Maybe later.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/selftests/Makefile            |   3 +-
 .../gpu/drm/selftests/drm_modeset_selftests.h |   4 +
 .../drm/selftests/test-drm_modeset_common.h   |   7 +
 drivers/gpu/drm/selftests/test-drm_rect.c     | 223 ++++++++++++++++++
 include/drm/drm_rect.h                        |   2 +
 5 files changed, 238 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/selftests/test-drm_rect.c

diff --git a/drivers/gpu/drm/selftests/Makefile b/drivers/gpu/drm/selftests/Makefile
index d2137342b371..0856e4b12f70 100644
--- a/drivers/gpu/drm/selftests/Makefile
+++ b/drivers/gpu/drm/selftests/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 test-drm_modeset-y := test-drm_modeset_common.o test-drm_plane_helper.o \
                       test-drm_format.o test-drm_framebuffer.o \
-		      test-drm_damage_helper.o test-drm_dp_mst_helper.o
+		      test-drm_damage_helper.o test-drm_dp_mst_helper.o \
+		      test-drm_rect.o
 
 obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-drm_modeset.o test-drm_cmdline_parser.o
diff --git a/drivers/gpu/drm/selftests/drm_modeset_selftests.h b/drivers/gpu/drm/selftests/drm_modeset_selftests.h
index 1898de0b4a4d..782e285ca383 100644
--- a/drivers/gpu/drm/selftests/drm_modeset_selftests.h
+++ b/drivers/gpu/drm/selftests/drm_modeset_selftests.h
@@ -6,6 +6,10 @@
  *
  * Tests are executed in order by igt/drm_selftests_helper
  */
+selftest(drm_rect_clip_scaled_div_by_zero, igt_drm_rect_clip_scaled_div_by_zero)
+selftest(drm_rect_clip_scaled_not_clipped, igt_drm_rect_clip_scaled_not_clipped)
+selftest(drm_rect_clip_scaled_clipped, igt_drm_rect_clip_scaled_clipped)
+selftest(drm_rect_clip_scaled_signed_vs_unsigned, igt_drm_rect_clip_scaled_signed_vs_unsigned)
 selftest(check_plane_state, igt_check_plane_state)
 selftest(check_drm_format_block_width, igt_check_drm_format_block_width)
 selftest(check_drm_format_block_height, igt_check_drm_format_block_height)
diff --git a/drivers/gpu/drm/selftests/test-drm_modeset_common.h b/drivers/gpu/drm/selftests/test-drm_modeset_common.h
index 0fcb8bbc6a1b..cfb51d8da2bc 100644
--- a/drivers/gpu/drm/selftests/test-drm_modeset_common.h
+++ b/drivers/gpu/drm/selftests/test-drm_modeset_common.h
@@ -3,6 +3,9 @@
 #ifndef __TEST_DRM_MODESET_COMMON_H__
 #define __TEST_DRM_MODESET_COMMON_H__
 
+#include <linux/errno.h>
+#include <linux/printk.h>
+
 #define FAIL(test, msg, ...) \
 	do { \
 		if (test) { \
@@ -13,6 +16,10 @@
 
 #define FAIL_ON(x) FAIL((x), "%s", "FAIL_ON(" __stringify(x) ")\n")
 
+int igt_drm_rect_clip_scaled_div_by_zero(void *ignored);
+int igt_drm_rect_clip_scaled_not_clipped(void *ignored);
+int igt_drm_rect_clip_scaled_clipped(void *ignored);
+int igt_drm_rect_clip_scaled_signed_vs_unsigned(void *ignored);
 int igt_check_plane_state(void *ignored);
 int igt_check_drm_format_block_width(void *ignored);
 int igt_check_drm_format_block_height(void *ignored);
diff --git a/drivers/gpu/drm/selftests/test-drm_rect.c b/drivers/gpu/drm/selftests/test-drm_rect.c
new file mode 100644
index 000000000000..3a5ff38321f4
--- /dev/null
+++ b/drivers/gpu/drm/selftests/test-drm_rect.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test cases for the drm_rect functions
+ */
+
+#define pr_fmt(fmt) "drm_rect: " fmt
+
+#include <linux/limits.h>
+
+#include <drm/drm_rect.h>
+
+#include "test-drm_modeset_common.h"
+
+int igt_drm_rect_clip_scaled_div_by_zero(void *ignored)
+{
+	struct drm_rect src, dst, clip;
+	bool visible;
+
+	/*
+	 * Make sure we don't divide by zero when dst
+	 * width/height is zero and dst and clip do not intersect.
+	 */
+	drm_rect_init(&src, 0, 0, 0, 0);
+	drm_rect_init(&dst, 0, 0, 0, 0);
+	drm_rect_init(&clip, 1, 1, 1, 1);
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+	FAIL(visible, "Destination not be visible\n");
+	FAIL(drm_rect_visible(&src), "Source should not be visible\n");
+
+	drm_rect_init(&src, 0, 0, 0, 0);
+	drm_rect_init(&dst, 3, 3, 0, 0);
+	drm_rect_init(&clip, 1, 1, 1, 1);
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+	FAIL(visible, "Destination not be visible\n");
+	FAIL(drm_rect_visible(&src), "Source should not be visible\n");
+
+	return 0;
+}
+
+int igt_drm_rect_clip_scaled_not_clipped(void *ignored)
+{
+	struct drm_rect src, dst, clip;
+	bool visible;
+
+	/* 1:1 scaling */
+	drm_rect_init(&src, 0, 0, 1 << 16, 1 << 16);
+	drm_rect_init(&dst, 0, 0, 1, 1);
+	drm_rect_init(&clip, 0, 0, 1, 1);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 0 || src.x2 != 1 << 16 ||
+	     src.y1 != 0 || src.y2 != 1 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 0 || dst.x2 != 1 ||
+	     dst.y1 != 0 || dst.y2 != 1,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	/* 2:1 scaling */
+	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
+	drm_rect_init(&dst, 0, 0, 1, 1);
+	drm_rect_init(&clip, 0, 0, 1, 1);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 0 || src.x2 != 2 << 16 ||
+	     src.y1 != 0 || src.y2 != 2 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 0 || dst.x2 != 1 ||
+	     dst.y1 != 0 || dst.y2 != 1,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	/* 1:2 scaling */
+	drm_rect_init(&src, 0, 0, 1 << 16, 1 << 16);
+	drm_rect_init(&dst, 0, 0, 2, 2);
+	drm_rect_init(&clip, 0, 0, 2, 2);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 0 || src.x2 != 1 << 16 ||
+	     src.y1 != 0 || src.y2 != 1 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 0 || dst.x2 != 2 ||
+	     dst.y1 != 0 || dst.y2 != 2,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	return 0;
+}
+
+int igt_drm_rect_clip_scaled_clipped(void *ignored)
+{
+	struct drm_rect src, dst, clip;
+	bool visible;
+
+	/* 1:1 scaling top/left clip */
+	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
+	drm_rect_init(&dst, 0, 0, 2, 2);
+	drm_rect_init(&clip, 0, 0, 1, 1);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 0 || src.x2 != 1 << 16 ||
+	     src.y1 != 0 || src.y2 != 1 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 0 || dst.x2 != 1 ||
+	     dst.y1 != 0 || dst.y2 != 1,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	/* 1:1 scaling bottom/right clip */
+	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
+	drm_rect_init(&dst, 0, 0, 2, 2);
+	drm_rect_init(&clip, 1, 1, 1, 1);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 1 << 16 || src.x2 != 2 << 16 ||
+	     src.y1 != 1 << 16 || src.y2 != 2 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 1 || dst.x2 != 2 ||
+	     dst.y1 != 1 || dst.y2 != 2,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	/* 2:1 scaling top/left clip */
+	drm_rect_init(&src, 0, 0, 4 << 16, 4 << 16);
+	drm_rect_init(&dst, 0, 0, 2, 2);
+	drm_rect_init(&clip, 0, 0, 1, 1);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 0 || src.x2 != 2 << 16 ||
+	     src.y1 != 0 || src.y2 != 2 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 0 || dst.x2 != 1 ||
+	     dst.y1 != 0 || dst.y2 != 1,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	/* 2:1 scaling bottom/right clip */
+	drm_rect_init(&src, 0, 0, 4 << 16, 4 << 16);
+	drm_rect_init(&dst, 0, 0, 2, 2);
+	drm_rect_init(&clip, 1, 1, 1, 1);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 2 << 16 || src.x2 != 4 << 16 ||
+	     src.y1 != 2 << 16 || src.y2 != 4 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 1 || dst.x2 != 2 ||
+	     dst.y1 != 1 || dst.y2 != 2,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	/* 1:2 scaling top/left clip */
+	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
+	drm_rect_init(&dst, 0, 0, 4, 4);
+	drm_rect_init(&clip, 0, 0, 2, 2);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 0 || src.x2 != 1 << 16 ||
+	     src.y1 != 0 || src.y2 != 1 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 0 || dst.x2 != 2 ||
+	     dst.y1 != 0 || dst.y2 != 2,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	/* 1:2 scaling bottom/right clip */
+	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
+	drm_rect_init(&dst, 0, 0, 4, 4);
+	drm_rect_init(&clip, 2, 2, 2, 2);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 1 << 16 || src.x2 != 2 << 16 ||
+	     src.y1 != 1 << 16 || src.y2 != 2 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 2 || dst.x2 != 4 ||
+	     dst.y1 != 2 || dst.y2 != 4,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	return 0;
+}
+
+int igt_drm_rect_clip_scaled_signed_vs_unsigned(void *ignored)
+{
+	struct drm_rect src, dst, clip;
+	bool visible;
+
+	/*
+	 * 'clip.x2 - dst.x1 >= dst width' could result a negative
+	 * src rectangle width which is no longer expected by the
+	 * code as it's using unsigned types. This could lead to
+	 * the clipped source rectangle appering visible when it
+	 * should have been fully clipped. Make sure both rectangles
+	 * end up invisible.
+	 */
+	drm_rect_init(&src, 0, 0, INT_MAX, INT_MAX);
+	drm_rect_init(&dst, 0, 0, 2, 2);
+	drm_rect_init(&clip, 3, 3, 1, 1);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(visible, "Destination should not be visible\n");
+	FAIL(drm_rect_visible(&src), "Source should not be visible\n");
+
+	return 0;
+}
diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
index cd0106135b6a..57a3be9e53e4 100644
--- a/include/drm/drm_rect.h
+++ b/include/drm/drm_rect.h
@@ -24,6 +24,8 @@
 #ifndef DRM_RECT_H
 #define DRM_RECT_H
 
+#include <linux/types.h>
+
 /**
  * DOC: rect utils
  *
-- 
2.23.0

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

[PATCH v2 4/4] drm/selftests: Add drm_rect selftests

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add selftests for drm_rect. A few basic ones for clipped and unclipped
cases, and a few special ones for specific bugs we had in the code.

I'm too lazy to think of more corner cases to check at this time.
Maybe later.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/selftests/Makefile            |   3 +-
 .../gpu/drm/selftests/drm_modeset_selftests.h |   4 +
 .../drm/selftests/test-drm_modeset_common.h   |   7 +
 drivers/gpu/drm/selftests/test-drm_rect.c     | 223 ++++++++++++++++++
 include/drm/drm_rect.h                        |   2 +
 5 files changed, 238 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/selftests/test-drm_rect.c

diff --git a/drivers/gpu/drm/selftests/Makefile b/drivers/gpu/drm/selftests/Makefile
index d2137342b371..0856e4b12f70 100644
--- a/drivers/gpu/drm/selftests/Makefile
+++ b/drivers/gpu/drm/selftests/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 test-drm_modeset-y := test-drm_modeset_common.o test-drm_plane_helper.o \
                       test-drm_format.o test-drm_framebuffer.o \
-		      test-drm_damage_helper.o test-drm_dp_mst_helper.o
+		      test-drm_damage_helper.o test-drm_dp_mst_helper.o \
+		      test-drm_rect.o
 
 obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-drm_modeset.o test-drm_cmdline_parser.o
diff --git a/drivers/gpu/drm/selftests/drm_modeset_selftests.h b/drivers/gpu/drm/selftests/drm_modeset_selftests.h
index 1898de0b4a4d..782e285ca383 100644
--- a/drivers/gpu/drm/selftests/drm_modeset_selftests.h
+++ b/drivers/gpu/drm/selftests/drm_modeset_selftests.h
@@ -6,6 +6,10 @@
  *
  * Tests are executed in order by igt/drm_selftests_helper
  */
+selftest(drm_rect_clip_scaled_div_by_zero, igt_drm_rect_clip_scaled_div_by_zero)
+selftest(drm_rect_clip_scaled_not_clipped, igt_drm_rect_clip_scaled_not_clipped)
+selftest(drm_rect_clip_scaled_clipped, igt_drm_rect_clip_scaled_clipped)
+selftest(drm_rect_clip_scaled_signed_vs_unsigned, igt_drm_rect_clip_scaled_signed_vs_unsigned)
 selftest(check_plane_state, igt_check_plane_state)
 selftest(check_drm_format_block_width, igt_check_drm_format_block_width)
 selftest(check_drm_format_block_height, igt_check_drm_format_block_height)
diff --git a/drivers/gpu/drm/selftests/test-drm_modeset_common.h b/drivers/gpu/drm/selftests/test-drm_modeset_common.h
index 0fcb8bbc6a1b..cfb51d8da2bc 100644
--- a/drivers/gpu/drm/selftests/test-drm_modeset_common.h
+++ b/drivers/gpu/drm/selftests/test-drm_modeset_common.h
@@ -3,6 +3,9 @@
 #ifndef __TEST_DRM_MODESET_COMMON_H__
 #define __TEST_DRM_MODESET_COMMON_H__
 
+#include <linux/errno.h>
+#include <linux/printk.h>
+
 #define FAIL(test, msg, ...) \
 	do { \
 		if (test) { \
@@ -13,6 +16,10 @@
 
 #define FAIL_ON(x) FAIL((x), "%s", "FAIL_ON(" __stringify(x) ")\n")
 
+int igt_drm_rect_clip_scaled_div_by_zero(void *ignored);
+int igt_drm_rect_clip_scaled_not_clipped(void *ignored);
+int igt_drm_rect_clip_scaled_clipped(void *ignored);
+int igt_drm_rect_clip_scaled_signed_vs_unsigned(void *ignored);
 int igt_check_plane_state(void *ignored);
 int igt_check_drm_format_block_width(void *ignored);
 int igt_check_drm_format_block_height(void *ignored);
diff --git a/drivers/gpu/drm/selftests/test-drm_rect.c b/drivers/gpu/drm/selftests/test-drm_rect.c
new file mode 100644
index 000000000000..3a5ff38321f4
--- /dev/null
+++ b/drivers/gpu/drm/selftests/test-drm_rect.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test cases for the drm_rect functions
+ */
+
+#define pr_fmt(fmt) "drm_rect: " fmt
+
+#include <linux/limits.h>
+
+#include <drm/drm_rect.h>
+
+#include "test-drm_modeset_common.h"
+
+int igt_drm_rect_clip_scaled_div_by_zero(void *ignored)
+{
+	struct drm_rect src, dst, clip;
+	bool visible;
+
+	/*
+	 * Make sure we don't divide by zero when dst
+	 * width/height is zero and dst and clip do not intersect.
+	 */
+	drm_rect_init(&src, 0, 0, 0, 0);
+	drm_rect_init(&dst, 0, 0, 0, 0);
+	drm_rect_init(&clip, 1, 1, 1, 1);
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+	FAIL(visible, "Destination not be visible\n");
+	FAIL(drm_rect_visible(&src), "Source should not be visible\n");
+
+	drm_rect_init(&src, 0, 0, 0, 0);
+	drm_rect_init(&dst, 3, 3, 0, 0);
+	drm_rect_init(&clip, 1, 1, 1, 1);
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+	FAIL(visible, "Destination not be visible\n");
+	FAIL(drm_rect_visible(&src), "Source should not be visible\n");
+
+	return 0;
+}
+
+int igt_drm_rect_clip_scaled_not_clipped(void *ignored)
+{
+	struct drm_rect src, dst, clip;
+	bool visible;
+
+	/* 1:1 scaling */
+	drm_rect_init(&src, 0, 0, 1 << 16, 1 << 16);
+	drm_rect_init(&dst, 0, 0, 1, 1);
+	drm_rect_init(&clip, 0, 0, 1, 1);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 0 || src.x2 != 1 << 16 ||
+	     src.y1 != 0 || src.y2 != 1 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 0 || dst.x2 != 1 ||
+	     dst.y1 != 0 || dst.y2 != 1,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	/* 2:1 scaling */
+	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
+	drm_rect_init(&dst, 0, 0, 1, 1);
+	drm_rect_init(&clip, 0, 0, 1, 1);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 0 || src.x2 != 2 << 16 ||
+	     src.y1 != 0 || src.y2 != 2 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 0 || dst.x2 != 1 ||
+	     dst.y1 != 0 || dst.y2 != 1,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	/* 1:2 scaling */
+	drm_rect_init(&src, 0, 0, 1 << 16, 1 << 16);
+	drm_rect_init(&dst, 0, 0, 2, 2);
+	drm_rect_init(&clip, 0, 0, 2, 2);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 0 || src.x2 != 1 << 16 ||
+	     src.y1 != 0 || src.y2 != 1 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 0 || dst.x2 != 2 ||
+	     dst.y1 != 0 || dst.y2 != 2,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	return 0;
+}
+
+int igt_drm_rect_clip_scaled_clipped(void *ignored)
+{
+	struct drm_rect src, dst, clip;
+	bool visible;
+
+	/* 1:1 scaling top/left clip */
+	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
+	drm_rect_init(&dst, 0, 0, 2, 2);
+	drm_rect_init(&clip, 0, 0, 1, 1);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 0 || src.x2 != 1 << 16 ||
+	     src.y1 != 0 || src.y2 != 1 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 0 || dst.x2 != 1 ||
+	     dst.y1 != 0 || dst.y2 != 1,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	/* 1:1 scaling bottom/right clip */
+	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
+	drm_rect_init(&dst, 0, 0, 2, 2);
+	drm_rect_init(&clip, 1, 1, 1, 1);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 1 << 16 || src.x2 != 2 << 16 ||
+	     src.y1 != 1 << 16 || src.y2 != 2 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 1 || dst.x2 != 2 ||
+	     dst.y1 != 1 || dst.y2 != 2,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	/* 2:1 scaling top/left clip */
+	drm_rect_init(&src, 0, 0, 4 << 16, 4 << 16);
+	drm_rect_init(&dst, 0, 0, 2, 2);
+	drm_rect_init(&clip, 0, 0, 1, 1);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 0 || src.x2 != 2 << 16 ||
+	     src.y1 != 0 || src.y2 != 2 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 0 || dst.x2 != 1 ||
+	     dst.y1 != 0 || dst.y2 != 1,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	/* 2:1 scaling bottom/right clip */
+	drm_rect_init(&src, 0, 0, 4 << 16, 4 << 16);
+	drm_rect_init(&dst, 0, 0, 2, 2);
+	drm_rect_init(&clip, 1, 1, 1, 1);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 2 << 16 || src.x2 != 4 << 16 ||
+	     src.y1 != 2 << 16 || src.y2 != 4 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 1 || dst.x2 != 2 ||
+	     dst.y1 != 1 || dst.y2 != 2,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	/* 1:2 scaling top/left clip */
+	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
+	drm_rect_init(&dst, 0, 0, 4, 4);
+	drm_rect_init(&clip, 0, 0, 2, 2);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 0 || src.x2 != 1 << 16 ||
+	     src.y1 != 0 || src.y2 != 1 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 0 || dst.x2 != 2 ||
+	     dst.y1 != 0 || dst.y2 != 2,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	/* 1:2 scaling bottom/right clip */
+	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
+	drm_rect_init(&dst, 0, 0, 4, 4);
+	drm_rect_init(&clip, 2, 2, 2, 2);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(src.x1 != 1 << 16 || src.x2 != 2 << 16 ||
+	     src.y1 != 1 << 16 || src.y2 != 2 << 16,
+	     "Source badly clipped\n");
+	FAIL(dst.x1 != 2 || dst.x2 != 4 ||
+	     dst.y1 != 2 || dst.y2 != 4,
+	     "Destination badly clipped\n");
+	FAIL(!visible, "Destination should be visible\n");
+	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
+
+	return 0;
+}
+
+int igt_drm_rect_clip_scaled_signed_vs_unsigned(void *ignored)
+{
+	struct drm_rect src, dst, clip;
+	bool visible;
+
+	/*
+	 * 'clip.x2 - dst.x1 >= dst width' could result a negative
+	 * src rectangle width which is no longer expected by the
+	 * code as it's using unsigned types. This could lead to
+	 * the clipped source rectangle appering visible when it
+	 * should have been fully clipped. Make sure both rectangles
+	 * end up invisible.
+	 */
+	drm_rect_init(&src, 0, 0, INT_MAX, INT_MAX);
+	drm_rect_init(&dst, 0, 0, 2, 2);
+	drm_rect_init(&clip, 3, 3, 1, 1);
+
+	visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+	FAIL(visible, "Destination should not be visible\n");
+	FAIL(drm_rect_visible(&src), "Source should not be visible\n");
+
+	return 0;
+}
diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
index cd0106135b6a..57a3be9e53e4 100644
--- a/include/drm/drm_rect.h
+++ b/include/drm/drm_rect.h
@@ -24,6 +24,8 @@
 #ifndef DRM_RECT_H
 #define DRM_RECT_H
 
+#include <linux/types.h>
+
 /**
  * DOC: rect utils
  *
-- 
2.23.0

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

Re: [PATCH v2 1/4] drm/rect: Avoid division by zero

[Daniel Vetter]

On Fri, Nov 22, 2019 at 07:56:20PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Check for zero width/height destination rectangle in
> drm_rect_clip_scaled() to avoid a division by zero.
> 
> Cc: stable@vger.kernel.org
> Fixes: f96bdf564f3e ("drm/rect: Handle rounding errors in drm_rect_clip_scaled, v3.")
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Benjamin Gaignard <benjamin.gaignard@st.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Testcase: igt/kms_selftest/drm_rect_clip_scaled_div_by_zero
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Clamping src to 0 if dst is 0 makes sense to me.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_rect.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
> index b8363aaa9032..818738e83d06 100644
> --- a/drivers/gpu/drm/drm_rect.c
> +++ b/drivers/gpu/drm/drm_rect.c
> @@ -54,7 +54,12 @@ EXPORT_SYMBOL(drm_rect_intersect);
>  
>  static u32 clip_scaled(u32 src, u32 dst, u32 clip)
>  {
> -	u64 tmp = mul_u32_u32(src, dst - clip);
> +	u64 tmp;
> +
> +	if (dst == 0)
> +		return 0;
> +
> +	tmp = mul_u32_u32(src, dst - clip);
>  
>  	/*
>  	 * Round toward 1.0 when clipping so that we don't accidentally
> -- 
> 2.23.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 1/4] drm/rect: Avoid division by zero

[Daniel Vetter]

On Fri, Nov 22, 2019 at 07:56:20PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Check for zero width/height destination rectangle in
> drm_rect_clip_scaled() to avoid a division by zero.
> 
> Cc: stable@vger.kernel.org
> Fixes: f96bdf564f3e ("drm/rect: Handle rounding errors in drm_rect_clip_scaled, v3.")
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Benjamin Gaignard <benjamin.gaignard@st.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Testcase: igt/kms_selftest/drm_rect_clip_scaled_div_by_zero
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Clamping src to 0 if dst is 0 makes sense to me.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_rect.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
> index b8363aaa9032..818738e83d06 100644
> --- a/drivers/gpu/drm/drm_rect.c
> +++ b/drivers/gpu/drm/drm_rect.c
> @@ -54,7 +54,12 @@ EXPORT_SYMBOL(drm_rect_intersect);
>  
>  static u32 clip_scaled(u32 src, u32 dst, u32 clip)
>  {
> -	u64 tmp = mul_u32_u32(src, dst - clip);
> +	u64 tmp;
> +
> +	if (dst == 0)
> +		return 0;
> +
> +	tmp = mul_u32_u32(src, dst - clip);
>  
>  	/*
>  	 * Round toward 1.0 when clipping so that we don't accidentally
> -- 
> 2.23.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/4] drm/rect: Keep the scaled clip bounded

[Daniel Vetter]

On Fri, Nov 22, 2019 at 07:56:21PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Limit the scaled clip to only clip at most dst_w/h pixels.
> This avoids the problem with clip_scaled() not being able
> to return negative values. Since new_src_w/h is now properly
> bounded we can remove the clamp()s.
> 
> Cc: Benjamin Gaignard <benjamin.gaignard@st.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Testcase: igt/kms_selftest/drm_rect_clip_scaled_signed_vs_unsigned
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_rect.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
> index 818738e83d06..a9c7f90836f3 100644
> --- a/drivers/gpu/drm/drm_rect.c
> +++ b/drivers/gpu/drm/drm_rect.c
> @@ -59,6 +59,9 @@ static u32 clip_scaled(u32 src, u32 dst, u32 clip)
>  	if (dst == 0)
>  		return 0;
>  
> +	/* Only clip what we have. Keeps the result bounded. */
> +	clip = min(clip, dst);
> +
>  	tmp = mul_u32_u32(src, dst - clip);
>  
>  	/*
> @@ -94,7 +97,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,

btw I just noticed that the kerneldoc for drm_rect_clip_scaled() wasn't
updated after Maarten's patch :-/ It still talks about hscale and vscale.
I'm going to type a patch to fix that.

On your patch here: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  		u32 new_src_w = clip_scaled(drm_rect_width(src),
>  					    drm_rect_width(dst), diff);
>  
> -		src->x1 = clamp_t(int64_t, src->x2 - new_src_w, INT_MIN, INT_MAX);
> +		src->x1 = src->x2 - new_src_w;
>  		dst->x1 = clip->x1;
>  	}
>  	diff = clip->y1 - dst->y1;
> @@ -102,7 +105,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
>  		u32 new_src_h = clip_scaled(drm_rect_height(src),
>  					    drm_rect_height(dst), diff);
>  
> -		src->y1 = clamp_t(int64_t, src->y2 - new_src_h, INT_MIN, INT_MAX);
> +		src->y1 = src->y2 - new_src_h;
>  		dst->y1 = clip->y1;
>  	}
>  	diff = dst->x2 - clip->x2;
> @@ -110,7 +113,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
>  		u32 new_src_w = clip_scaled(drm_rect_width(src),
>  					    drm_rect_width(dst), diff);
>  
> -		src->x2 = clamp_t(int64_t, src->x1 + new_src_w, INT_MIN, INT_MAX);
> +		src->x2 = src->x1 + new_src_w;
>  		dst->x2 = clip->x2;
>  	}
>  	diff = dst->y2 - clip->y2;
> @@ -118,7 +121,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
>  		u32 new_src_h = clip_scaled(drm_rect_height(src),
>  					    drm_rect_height(dst), diff);
>  
> -		src->y2 = clamp_t(int64_t, src->y1 + new_src_h, INT_MIN, INT_MAX);
> +		src->y2 = src->y1 + new_src_h;
>  		dst->y2 = clip->y2;
>  	}
>  
> -- 
> 2.23.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/4] drm/rect: Keep the scaled clip bounded

[Daniel Vetter]

On Fri, Nov 22, 2019 at 07:56:21PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Limit the scaled clip to only clip at most dst_w/h pixels.
> This avoids the problem with clip_scaled() not being able
> to return negative values. Since new_src_w/h is now properly
> bounded we can remove the clamp()s.
> 
> Cc: Benjamin Gaignard <benjamin.gaignard@st.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Testcase: igt/kms_selftest/drm_rect_clip_scaled_signed_vs_unsigned
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_rect.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
> index 818738e83d06..a9c7f90836f3 100644
> --- a/drivers/gpu/drm/drm_rect.c
> +++ b/drivers/gpu/drm/drm_rect.c
> @@ -59,6 +59,9 @@ static u32 clip_scaled(u32 src, u32 dst, u32 clip)
>  	if (dst == 0)
>  		return 0;
>  
> +	/* Only clip what we have. Keeps the result bounded. */
> +	clip = min(clip, dst);
> +
>  	tmp = mul_u32_u32(src, dst - clip);
>  
>  	/*
> @@ -94,7 +97,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,

btw I just noticed that the kerneldoc for drm_rect_clip_scaled() wasn't
updated after Maarten's patch :-/ It still talks about hscale and vscale.
I'm going to type a patch to fix that.

On your patch here: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  		u32 new_src_w = clip_scaled(drm_rect_width(src),
>  					    drm_rect_width(dst), diff);
>  
> -		src->x1 = clamp_t(int64_t, src->x2 - new_src_w, INT_MIN, INT_MAX);
> +		src->x1 = src->x2 - new_src_w;
>  		dst->x1 = clip->x1;
>  	}
>  	diff = clip->y1 - dst->y1;
> @@ -102,7 +105,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
>  		u32 new_src_h = clip_scaled(drm_rect_height(src),
>  					    drm_rect_height(dst), diff);
>  
> -		src->y1 = clamp_t(int64_t, src->y2 - new_src_h, INT_MIN, INT_MAX);
> +		src->y1 = src->y2 - new_src_h;
>  		dst->y1 = clip->y1;
>  	}
>  	diff = dst->x2 - clip->x2;
> @@ -110,7 +113,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
>  		u32 new_src_w = clip_scaled(drm_rect_width(src),
>  					    drm_rect_width(dst), diff);
>  
> -		src->x2 = clamp_t(int64_t, src->x1 + new_src_w, INT_MIN, INT_MAX);
> +		src->x2 = src->x1 + new_src_w;
>  		dst->x2 = clip->x2;
>  	}
>  	diff = dst->y2 - clip->y2;
> @@ -118,7 +121,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
>  		u32 new_src_h = clip_scaled(drm_rect_height(src),
>  					    drm_rect_height(dst), diff);
>  
> -		src->y2 = clamp_t(int64_t, src->y1 + new_src_h, INT_MIN, INT_MAX);
> +		src->y2 = src->y1 + new_src_h;
>  		dst->y2 = clip->y2;
>  	}
>  
> -- 
> 2.23.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/4] drm/rect: Keep the clipped dst rectangle in place

[Daniel Vetter]

On Fri, Nov 22, 2019 at 07:56:22PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Now that we've constrained the clipped source rectangle such
> that it can't have negative dimensions doing the same for the
> dst rectangle seems appropriate. Should at least result in
> the clipped src and dst rectangles being a bit more consistent
> with each other.
> 
> Cc: Benjamin Gaignard <benjamin.gaignard@st.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_rect.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
> index a9c7f90836f3..1e1e2101007a 100644
> --- a/drivers/gpu/drm/drm_rect.c
> +++ b/drivers/gpu/drm/drm_rect.c
> @@ -52,7 +52,7 @@ bool drm_rect_intersect(struct drm_rect *r1, const struct drm_rect *r2)
>  }
>  EXPORT_SYMBOL(drm_rect_intersect);
>  
> -static u32 clip_scaled(u32 src, u32 dst, u32 clip)
> +static u32 clip_scaled(int src, int dst, int *clip)

Mild scare about this one here, but I think we've clamped everything
sufficiently now that silly stuff can't happen anymore. And everything in
here seems to cast to sufficiently big types (and we shouldn't have
negative values here I hope). I'll see how realistic that assumption is
when looking at the selftests. For this:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  {
>  	u64 tmp;
>  
> @@ -60,9 +60,9 @@ static u32 clip_scaled(u32 src, u32 dst, u32 clip)
>  		return 0;
>  
>  	/* Only clip what we have. Keeps the result bounded. */
> -	clip = min(clip, dst);
> +	*clip = min(*clip, dst);
>  
> -	tmp = mul_u32_u32(src, dst - clip);
> +	tmp = mul_u32_u32(src, dst - *clip);
>  
>  	/*
>  	 * Round toward 1.0 when clipping so that we don't accidentally
> @@ -95,34 +95,34 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
>  	diff = clip->x1 - dst->x1;
>  	if (diff > 0) {
>  		u32 new_src_w = clip_scaled(drm_rect_width(src),
> -					    drm_rect_width(dst), diff);
> +					    drm_rect_width(dst), &diff);
>  
>  		src->x1 = src->x2 - new_src_w;
> -		dst->x1 = clip->x1;
> +		dst->x1 += diff;
>  	}
>  	diff = clip->y1 - dst->y1;
>  	if (diff > 0) {
>  		u32 new_src_h = clip_scaled(drm_rect_height(src),
> -					    drm_rect_height(dst), diff);
> +					    drm_rect_height(dst), &diff);
>  
>  		src->y1 = src->y2 - new_src_h;
> -		dst->y1 = clip->y1;
> +		dst->y1 += diff;
>  	}
>  	diff = dst->x2 - clip->x2;
>  	if (diff > 0) {
>  		u32 new_src_w = clip_scaled(drm_rect_width(src),
> -					    drm_rect_width(dst), diff);
> +					    drm_rect_width(dst), &diff);
>  
>  		src->x2 = src->x1 + new_src_w;
> -		dst->x2 = clip->x2;
> +		dst->x2 -= diff;
>  	}
>  	diff = dst->y2 - clip->y2;
>  	if (diff > 0) {
>  		u32 new_src_h = clip_scaled(drm_rect_height(src),
> -					    drm_rect_height(dst), diff);
> +					    drm_rect_height(dst), &diff);
>  
>  		src->y2 = src->y1 + new_src_h;
> -		dst->y2 = clip->y2;
> +		dst->y2 -= diff;
>  	}
>  
>  	return drm_rect_visible(dst);
> -- 
> 2.23.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/4] drm/selftests: Add drm_rect selftests

[Daniel Vetter]

On Fri, Nov 22, 2019 at 07:56:23PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add selftests for drm_rect. A few basic ones for clipped and unclipped
> cases, and a few special ones for specific bugs we had in the code.
> 
> I'm too lazy to think of more corner cases to check at this time.
> Maybe later.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/selftests/Makefile            |   3 +-
>  .../gpu/drm/selftests/drm_modeset_selftests.h |   4 +
>  .../drm/selftests/test-drm_modeset_common.h   |   7 +
>  drivers/gpu/drm/selftests/test-drm_rect.c     | 223 ++++++++++++++++++
>  include/drm/drm_rect.h                        |   2 +
>  5 files changed, 238 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/selftests/test-drm_rect.c
> 
> diff --git a/drivers/gpu/drm/selftests/Makefile b/drivers/gpu/drm/selftests/Makefile
> index d2137342b371..0856e4b12f70 100644
> --- a/drivers/gpu/drm/selftests/Makefile
> +++ b/drivers/gpu/drm/selftests/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  test-drm_modeset-y := test-drm_modeset_common.o test-drm_plane_helper.o \
>                        test-drm_format.o test-drm_framebuffer.o \
> -		      test-drm_damage_helper.o test-drm_dp_mst_helper.o
> +		      test-drm_damage_helper.o test-drm_dp_mst_helper.o \
> +		      test-drm_rect.o
>  
>  obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-drm_modeset.o test-drm_cmdline_parser.o
> diff --git a/drivers/gpu/drm/selftests/drm_modeset_selftests.h b/drivers/gpu/drm/selftests/drm_modeset_selftests.h
> index 1898de0b4a4d..782e285ca383 100644
> --- a/drivers/gpu/drm/selftests/drm_modeset_selftests.h
> +++ b/drivers/gpu/drm/selftests/drm_modeset_selftests.h
> @@ -6,6 +6,10 @@
>   *
>   * Tests are executed in order by igt/drm_selftests_helper
>   */
> +selftest(drm_rect_clip_scaled_div_by_zero, igt_drm_rect_clip_scaled_div_by_zero)
> +selftest(drm_rect_clip_scaled_not_clipped, igt_drm_rect_clip_scaled_not_clipped)
> +selftest(drm_rect_clip_scaled_clipped, igt_drm_rect_clip_scaled_clipped)
> +selftest(drm_rect_clip_scaled_signed_vs_unsigned, igt_drm_rect_clip_scaled_signed_vs_unsigned)
>  selftest(check_plane_state, igt_check_plane_state)
>  selftest(check_drm_format_block_width, igt_check_drm_format_block_width)
>  selftest(check_drm_format_block_height, igt_check_drm_format_block_height)
> diff --git a/drivers/gpu/drm/selftests/test-drm_modeset_common.h b/drivers/gpu/drm/selftests/test-drm_modeset_common.h
> index 0fcb8bbc6a1b..cfb51d8da2bc 100644
> --- a/drivers/gpu/drm/selftests/test-drm_modeset_common.h
> +++ b/drivers/gpu/drm/selftests/test-drm_modeset_common.h
> @@ -3,6 +3,9 @@
>  #ifndef __TEST_DRM_MODESET_COMMON_H__
>  #define __TEST_DRM_MODESET_COMMON_H__
>  
> +#include <linux/errno.h>
> +#include <linux/printk.h>
> +
>  #define FAIL(test, msg, ...) \
>  	do { \
>  		if (test) { \
> @@ -13,6 +16,10 @@
>  
>  #define FAIL_ON(x) FAIL((x), "%s", "FAIL_ON(" __stringify(x) ")\n")
>  
> +int igt_drm_rect_clip_scaled_div_by_zero(void *ignored);
> +int igt_drm_rect_clip_scaled_not_clipped(void *ignored);
> +int igt_drm_rect_clip_scaled_clipped(void *ignored);
> +int igt_drm_rect_clip_scaled_signed_vs_unsigned(void *ignored);
>  int igt_check_plane_state(void *ignored);
>  int igt_check_drm_format_block_width(void *ignored);
>  int igt_check_drm_format_block_height(void *ignored);
> diff --git a/drivers/gpu/drm/selftests/test-drm_rect.c b/drivers/gpu/drm/selftests/test-drm_rect.c
> new file mode 100644
> index 000000000000..3a5ff38321f4
> --- /dev/null
> +++ b/drivers/gpu/drm/selftests/test-drm_rect.c
> @@ -0,0 +1,223 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Test cases for the drm_rect functions
> + */
> +
> +#define pr_fmt(fmt) "drm_rect: " fmt
> +
> +#include <linux/limits.h>
> +
> +#include <drm/drm_rect.h>
> +
> +#include "test-drm_modeset_common.h"
> +
> +int igt_drm_rect_clip_scaled_div_by_zero(void *ignored)
> +{
> +	struct drm_rect src, dst, clip;
> +	bool visible;
> +
> +	/*
> +	 * Make sure we don't divide by zero when dst
> +	 * width/height is zero and dst and clip do not intersect.
> +	 */
> +	drm_rect_init(&src, 0, 0, 0, 0);
> +	drm_rect_init(&dst, 0, 0, 0, 0);
> +	drm_rect_init(&clip, 1, 1, 1, 1);
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +	FAIL(visible, "Destination not be visible\n");
			          ^ "should" missing

> +	FAIL(drm_rect_visible(&src), "Source should not be visible\n");
> +
> +	drm_rect_init(&src, 0, 0, 0, 0);
> +	drm_rect_init(&dst, 3, 3, 0, 0);
> +	drm_rect_init(&clip, 1, 1, 1, 1);
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +	FAIL(visible, "Destination not be visible\n");
			          ^ "should" missing

> +	return 0;
> +}
> +
> +int igt_drm_rect_clip_scaled_not_clipped(void *ignored)
> +{
> +	struct drm_rect src, dst, clip;
> +	bool visible;
> +
> +	/* 1:1 scaling */
> +	drm_rect_init(&src, 0, 0, 1 << 16, 1 << 16);
> +	drm_rect_init(&dst, 0, 0, 1, 1);
> +	drm_rect_init(&clip, 0, 0, 1, 1);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 0 || src.x2 != 1 << 16 ||
> +	     src.y1 != 0 || src.y2 != 1 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 0 || dst.x2 != 1 ||
> +	     dst.y1 != 0 || dst.y2 != 1,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
> +
> +	/* 2:1 scaling */
> +	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
> +	drm_rect_init(&dst, 0, 0, 1, 1);
> +	drm_rect_init(&clip, 0, 0, 1, 1);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 0 || src.x2 != 2 << 16 ||
> +	     src.y1 != 0 || src.y2 != 2 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 0 || dst.x2 != 1 ||
> +	     dst.y1 != 0 || dst.y2 != 1,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
> +
> +	/* 1:2 scaling */
> +	drm_rect_init(&src, 0, 0, 1 << 16, 1 << 16);
> +	drm_rect_init(&dst, 0, 0, 2, 2);
> +	drm_rect_init(&clip, 0, 0, 2, 2);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 0 || src.x2 != 1 << 16 ||
> +	     src.y1 != 0 || src.y2 != 1 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 0 || dst.x2 != 2 ||
> +	     dst.y1 != 0 || dst.y2 != 2,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");

I wonder whether an INT_MAX dst, with src = 0,0,1,1 could be interesting
here too. Less practically relevant, it's more about making sure we don't
overflow anywhere in silly ways.

> +
> +	return 0;
> +}
> +
> +int igt_drm_rect_clip_scaled_clipped(void *ignored)
> +{
> +	struct drm_rect src, dst, clip;
> +	bool visible;
> +
> +	/* 1:1 scaling top/left clip */
> +	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
> +	drm_rect_init(&dst, 0, 0, 2, 2);
> +	drm_rect_init(&clip, 0, 0, 1, 1);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 0 || src.x2 != 1 << 16 ||
> +	     src.y1 != 0 || src.y2 != 1 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 0 || dst.x2 != 1 ||
> +	     dst.y1 != 0 || dst.y2 != 1,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
> +
> +	/* 1:1 scaling bottom/right clip */
> +	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
> +	drm_rect_init(&dst, 0, 0, 2, 2);
> +	drm_rect_init(&clip, 1, 1, 1, 1);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 1 << 16 || src.x2 != 2 << 16 ||
> +	     src.y1 != 1 << 16 || src.y2 != 2 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 1 || dst.x2 != 2 ||
> +	     dst.y1 != 1 || dst.y2 != 2,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
> +
> +	/* 2:1 scaling top/left clip */
> +	drm_rect_init(&src, 0, 0, 4 << 16, 4 << 16);
> +	drm_rect_init(&dst, 0, 0, 2, 2);
> +	drm_rect_init(&clip, 0, 0, 1, 1);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 0 || src.x2 != 2 << 16 ||
> +	     src.y1 != 0 || src.y2 != 2 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 0 || dst.x2 != 1 ||
> +	     dst.y1 != 0 || dst.y2 != 1,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
> +
> +	/* 2:1 scaling bottom/right clip */
> +	drm_rect_init(&src, 0, 0, 4 << 16, 4 << 16);
> +	drm_rect_init(&dst, 0, 0, 2, 2);
> +	drm_rect_init(&clip, 1, 1, 1, 1);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 2 << 16 || src.x2 != 4 << 16 ||
> +	     src.y1 != 2 << 16 || src.y2 != 4 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 1 || dst.x2 != 2 ||
> +	     dst.y1 != 1 || dst.y2 != 2,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
> +
> +	/* 1:2 scaling top/left clip */
> +	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
> +	drm_rect_init(&dst, 0, 0, 4, 4);
> +	drm_rect_init(&clip, 0, 0, 2, 2);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 0 || src.x2 != 1 << 16 ||
> +	     src.y1 != 0 || src.y2 != 1 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 0 || dst.x2 != 2 ||
> +	     dst.y1 != 0 || dst.y2 != 2,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
> +
> +	/* 1:2 scaling bottom/right clip */
> +	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
> +	drm_rect_init(&dst, 0, 0, 4, 4);
> +	drm_rect_init(&clip, 2, 2, 2, 2);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 1 << 16 || src.x2 != 2 << 16 ||
> +	     src.y1 != 1 << 16 || src.y2 != 2 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 2 || dst.x2 != 4 ||
> +	     dst.y1 != 2 || dst.y2 != 4,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");

Not sure whether dst/src origin != 0,0 would be interesting, but I think
this is a solid start.

Thanks a lot for typing these.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +
> +	return 0;
> +}
> +
> +int igt_drm_rect_clip_scaled_signed_vs_unsigned(void *ignored)
> +{
> +	struct drm_rect src, dst, clip;
> +	bool visible;
> +
> +	/*
> +	 * 'clip.x2 - dst.x1 >= dst width' could result a negative
> +	 * src rectangle width which is no longer expected by the
> +	 * code as it's using unsigned types. This could lead to
> +	 * the clipped source rectangle appering visible when it
> +	 * should have been fully clipped. Make sure both rectangles
> +	 * end up invisible.
> +	 */
> +	drm_rect_init(&src, 0, 0, INT_MAX, INT_MAX);
> +	drm_rect_init(&dst, 0, 0, 2, 2);
> +	drm_rect_init(&clip, 3, 3, 1, 1);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(visible, "Destination should not be visible\n");
> +	FAIL(drm_rect_visible(&src), "Source should not be visible\n");
> +
> +	return 0;
> +}
> diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
> index cd0106135b6a..57a3be9e53e4 100644
> --- a/include/drm/drm_rect.h
> +++ b/include/drm/drm_rect.h
> @@ -24,6 +24,8 @@
>  #ifndef DRM_RECT_H
>  #define DRM_RECT_H
>  
> +#include <linux/types.h>
> +
>  /**
>   * DOC: rect utils
>   *
> -- 
> 2.23.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 4/4] drm/selftests: Add drm_rect selftests

[Daniel Vetter]

On Fri, Nov 22, 2019 at 07:56:23PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add selftests for drm_rect. A few basic ones for clipped and unclipped
> cases, and a few special ones for specific bugs we had in the code.
> 
> I'm too lazy to think of more corner cases to check at this time.
> Maybe later.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/selftests/Makefile            |   3 +-
>  .../gpu/drm/selftests/drm_modeset_selftests.h |   4 +
>  .../drm/selftests/test-drm_modeset_common.h   |   7 +
>  drivers/gpu/drm/selftests/test-drm_rect.c     | 223 ++++++++++++++++++
>  include/drm/drm_rect.h                        |   2 +
>  5 files changed, 238 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/selftests/test-drm_rect.c
> 
> diff --git a/drivers/gpu/drm/selftests/Makefile b/drivers/gpu/drm/selftests/Makefile
> index d2137342b371..0856e4b12f70 100644
> --- a/drivers/gpu/drm/selftests/Makefile
> +++ b/drivers/gpu/drm/selftests/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  test-drm_modeset-y := test-drm_modeset_common.o test-drm_plane_helper.o \
>                        test-drm_format.o test-drm_framebuffer.o \
> -		      test-drm_damage_helper.o test-drm_dp_mst_helper.o
> +		      test-drm_damage_helper.o test-drm_dp_mst_helper.o \
> +		      test-drm_rect.o
>  
>  obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-drm_modeset.o test-drm_cmdline_parser.o
> diff --git a/drivers/gpu/drm/selftests/drm_modeset_selftests.h b/drivers/gpu/drm/selftests/drm_modeset_selftests.h
> index 1898de0b4a4d..782e285ca383 100644
> --- a/drivers/gpu/drm/selftests/drm_modeset_selftests.h
> +++ b/drivers/gpu/drm/selftests/drm_modeset_selftests.h
> @@ -6,6 +6,10 @@
>   *
>   * Tests are executed in order by igt/drm_selftests_helper
>   */
> +selftest(drm_rect_clip_scaled_div_by_zero, igt_drm_rect_clip_scaled_div_by_zero)
> +selftest(drm_rect_clip_scaled_not_clipped, igt_drm_rect_clip_scaled_not_clipped)
> +selftest(drm_rect_clip_scaled_clipped, igt_drm_rect_clip_scaled_clipped)
> +selftest(drm_rect_clip_scaled_signed_vs_unsigned, igt_drm_rect_clip_scaled_signed_vs_unsigned)
>  selftest(check_plane_state, igt_check_plane_state)
>  selftest(check_drm_format_block_width, igt_check_drm_format_block_width)
>  selftest(check_drm_format_block_height, igt_check_drm_format_block_height)
> diff --git a/drivers/gpu/drm/selftests/test-drm_modeset_common.h b/drivers/gpu/drm/selftests/test-drm_modeset_common.h
> index 0fcb8bbc6a1b..cfb51d8da2bc 100644
> --- a/drivers/gpu/drm/selftests/test-drm_modeset_common.h
> +++ b/drivers/gpu/drm/selftests/test-drm_modeset_common.h
> @@ -3,6 +3,9 @@
>  #ifndef __TEST_DRM_MODESET_COMMON_H__
>  #define __TEST_DRM_MODESET_COMMON_H__
>  
> +#include <linux/errno.h>
> +#include <linux/printk.h>
> +
>  #define FAIL(test, msg, ...) \
>  	do { \
>  		if (test) { \
> @@ -13,6 +16,10 @@
>  
>  #define FAIL_ON(x) FAIL((x), "%s", "FAIL_ON(" __stringify(x) ")\n")
>  
> +int igt_drm_rect_clip_scaled_div_by_zero(void *ignored);
> +int igt_drm_rect_clip_scaled_not_clipped(void *ignored);
> +int igt_drm_rect_clip_scaled_clipped(void *ignored);
> +int igt_drm_rect_clip_scaled_signed_vs_unsigned(void *ignored);
>  int igt_check_plane_state(void *ignored);
>  int igt_check_drm_format_block_width(void *ignored);
>  int igt_check_drm_format_block_height(void *ignored);
> diff --git a/drivers/gpu/drm/selftests/test-drm_rect.c b/drivers/gpu/drm/selftests/test-drm_rect.c
> new file mode 100644
> index 000000000000..3a5ff38321f4
> --- /dev/null
> +++ b/drivers/gpu/drm/selftests/test-drm_rect.c
> @@ -0,0 +1,223 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Test cases for the drm_rect functions
> + */
> +
> +#define pr_fmt(fmt) "drm_rect: " fmt
> +
> +#include <linux/limits.h>
> +
> +#include <drm/drm_rect.h>
> +
> +#include "test-drm_modeset_common.h"
> +
> +int igt_drm_rect_clip_scaled_div_by_zero(void *ignored)
> +{
> +	struct drm_rect src, dst, clip;
> +	bool visible;
> +
> +	/*
> +	 * Make sure we don't divide by zero when dst
> +	 * width/height is zero and dst and clip do not intersect.
> +	 */
> +	drm_rect_init(&src, 0, 0, 0, 0);
> +	drm_rect_init(&dst, 0, 0, 0, 0);
> +	drm_rect_init(&clip, 1, 1, 1, 1);
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +	FAIL(visible, "Destination not be visible\n");
			          ^ "should" missing

> +	FAIL(drm_rect_visible(&src), "Source should not be visible\n");
> +
> +	drm_rect_init(&src, 0, 0, 0, 0);
> +	drm_rect_init(&dst, 3, 3, 0, 0);
> +	drm_rect_init(&clip, 1, 1, 1, 1);
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +	FAIL(visible, "Destination not be visible\n");
			          ^ "should" missing

> +	return 0;
> +}
> +
> +int igt_drm_rect_clip_scaled_not_clipped(void *ignored)
> +{
> +	struct drm_rect src, dst, clip;
> +	bool visible;
> +
> +	/* 1:1 scaling */
> +	drm_rect_init(&src, 0, 0, 1 << 16, 1 << 16);
> +	drm_rect_init(&dst, 0, 0, 1, 1);
> +	drm_rect_init(&clip, 0, 0, 1, 1);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 0 || src.x2 != 1 << 16 ||
> +	     src.y1 != 0 || src.y2 != 1 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 0 || dst.x2 != 1 ||
> +	     dst.y1 != 0 || dst.y2 != 1,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
> +
> +	/* 2:1 scaling */
> +	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
> +	drm_rect_init(&dst, 0, 0, 1, 1);
> +	drm_rect_init(&clip, 0, 0, 1, 1);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 0 || src.x2 != 2 << 16 ||
> +	     src.y1 != 0 || src.y2 != 2 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 0 || dst.x2 != 1 ||
> +	     dst.y1 != 0 || dst.y2 != 1,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
> +
> +	/* 1:2 scaling */
> +	drm_rect_init(&src, 0, 0, 1 << 16, 1 << 16);
> +	drm_rect_init(&dst, 0, 0, 2, 2);
> +	drm_rect_init(&clip, 0, 0, 2, 2);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 0 || src.x2 != 1 << 16 ||
> +	     src.y1 != 0 || src.y2 != 1 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 0 || dst.x2 != 2 ||
> +	     dst.y1 != 0 || dst.y2 != 2,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");

I wonder whether an INT_MAX dst, with src = 0,0,1,1 could be interesting
here too. Less practically relevant, it's more about making sure we don't
overflow anywhere in silly ways.

> +
> +	return 0;
> +}
> +
> +int igt_drm_rect_clip_scaled_clipped(void *ignored)
> +{
> +	struct drm_rect src, dst, clip;
> +	bool visible;
> +
> +	/* 1:1 scaling top/left clip */
> +	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
> +	drm_rect_init(&dst, 0, 0, 2, 2);
> +	drm_rect_init(&clip, 0, 0, 1, 1);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 0 || src.x2 != 1 << 16 ||
> +	     src.y1 != 0 || src.y2 != 1 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 0 || dst.x2 != 1 ||
> +	     dst.y1 != 0 || dst.y2 != 1,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
> +
> +	/* 1:1 scaling bottom/right clip */
> +	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
> +	drm_rect_init(&dst, 0, 0, 2, 2);
> +	drm_rect_init(&clip, 1, 1, 1, 1);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 1 << 16 || src.x2 != 2 << 16 ||
> +	     src.y1 != 1 << 16 || src.y2 != 2 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 1 || dst.x2 != 2 ||
> +	     dst.y1 != 1 || dst.y2 != 2,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
> +
> +	/* 2:1 scaling top/left clip */
> +	drm_rect_init(&src, 0, 0, 4 << 16, 4 << 16);
> +	drm_rect_init(&dst, 0, 0, 2, 2);
> +	drm_rect_init(&clip, 0, 0, 1, 1);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 0 || src.x2 != 2 << 16 ||
> +	     src.y1 != 0 || src.y2 != 2 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 0 || dst.x2 != 1 ||
> +	     dst.y1 != 0 || dst.y2 != 1,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
> +
> +	/* 2:1 scaling bottom/right clip */
> +	drm_rect_init(&src, 0, 0, 4 << 16, 4 << 16);
> +	drm_rect_init(&dst, 0, 0, 2, 2);
> +	drm_rect_init(&clip, 1, 1, 1, 1);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 2 << 16 || src.x2 != 4 << 16 ||
> +	     src.y1 != 2 << 16 || src.y2 != 4 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 1 || dst.x2 != 2 ||
> +	     dst.y1 != 1 || dst.y2 != 2,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
> +
> +	/* 1:2 scaling top/left clip */
> +	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
> +	drm_rect_init(&dst, 0, 0, 4, 4);
> +	drm_rect_init(&clip, 0, 0, 2, 2);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 0 || src.x2 != 1 << 16 ||
> +	     src.y1 != 0 || src.y2 != 1 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 0 || dst.x2 != 2 ||
> +	     dst.y1 != 0 || dst.y2 != 2,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");
> +
> +	/* 1:2 scaling bottom/right clip */
> +	drm_rect_init(&src, 0, 0, 2 << 16, 2 << 16);
> +	drm_rect_init(&dst, 0, 0, 4, 4);
> +	drm_rect_init(&clip, 2, 2, 2, 2);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(src.x1 != 1 << 16 || src.x2 != 2 << 16 ||
> +	     src.y1 != 1 << 16 || src.y2 != 2 << 16,
> +	     "Source badly clipped\n");
> +	FAIL(dst.x1 != 2 || dst.x2 != 4 ||
> +	     dst.y1 != 2 || dst.y2 != 4,
> +	     "Destination badly clipped\n");
> +	FAIL(!visible, "Destination should be visible\n");
> +	FAIL(!drm_rect_visible(&src), "Source should be visible\n");

Not sure whether dst/src origin != 0,0 would be interesting, but I think
this is a solid start.

Thanks a lot for typing these.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +
> +	return 0;
> +}
> +
> +int igt_drm_rect_clip_scaled_signed_vs_unsigned(void *ignored)
> +{
> +	struct drm_rect src, dst, clip;
> +	bool visible;
> +
> +	/*
> +	 * 'clip.x2 - dst.x1 >= dst width' could result a negative
> +	 * src rectangle width which is no longer expected by the
> +	 * code as it's using unsigned types. This could lead to
> +	 * the clipped source rectangle appering visible when it
> +	 * should have been fully clipped. Make sure both rectangles
> +	 * end up invisible.
> +	 */
> +	drm_rect_init(&src, 0, 0, INT_MAX, INT_MAX);
> +	drm_rect_init(&dst, 0, 0, 2, 2);
> +	drm_rect_init(&clip, 3, 3, 1, 1);
> +
> +	visible = drm_rect_clip_scaled(&src, &dst, &clip);
> +
> +	FAIL(visible, "Destination should not be visible\n");
> +	FAIL(drm_rect_visible(&src), "Source should not be visible\n");
> +
> +	return 0;
> +}
> diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
> index cd0106135b6a..57a3be9e53e4 100644
> --- a/include/drm/drm_rect.h
> +++ b/include/drm/drm_rect.h
> @@ -24,6 +24,8 @@
>  #ifndef DRM_RECT_H
>  #define DRM_RECT_H
>  
> +#include <linux/types.h>
> +
>  /**
>   * DOC: rect utils
>   *
> -- 
> 2.23.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/4] drm/rect: Bugfixes and selftests

[Benjamin GAIGNARD]

On 11/22/19 6:56 PM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> My earlier fixes for drm_rect + div-by-zero fix + some
> selftests that Daniel requested.
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Benjamin Gaignard <benjamin.gaignard@st.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>

Thanks to have handle this.

Reviewed-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>
> Ville Syrjälä (4):
>    drm/rect: Avoid division by zero
>    drm/rect: Keep the scaled clip bounded
>    drm/rect: Keep the clipped dst rectangle in place
>    drm/selftests: Add drm_rect selftests
>
>   drivers/gpu/drm/drm_rect.c                    |  36 +--
>   drivers/gpu/drm/selftests/Makefile            |   3 +-
>   .../gpu/drm/selftests/drm_modeset_selftests.h |   4 +
>   .../drm/selftests/test-drm_modeset_common.h   |   7 +
>   drivers/gpu/drm/selftests/test-drm_rect.c     | 220 ++++++++++++++++++
>   include/drm/drm_rect.h                        |   2 +
>   6 files changed, 257 insertions(+), 15 deletions(-)
>   create mode 100644 drivers/gpu/drm/selftests/test-drm_rect.c
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 0/4] drm/rect: Bugfixes and selftests

[Benjamin GAIGNARD]

On 11/22/19 6:56 PM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> My earlier fixes for drm_rect + div-by-zero fix + some
> selftests that Daniel requested.
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Benjamin Gaignard <benjamin.gaignard@st.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>

Thanks to have handle this.

Reviewed-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>
> Ville Syrjälä (4):
>    drm/rect: Avoid division by zero
>    drm/rect: Keep the scaled clip bounded
>    drm/rect: Keep the clipped dst rectangle in place
>    drm/selftests: Add drm_rect selftests
>
>   drivers/gpu/drm/drm_rect.c                    |  36 +--
>   drivers/gpu/drm/selftests/Makefile            |   3 +-
>   .../gpu/drm/selftests/drm_modeset_selftests.h |   4 +
>   .../drm/selftests/test-drm_modeset_common.h   |   7 +
>   drivers/gpu/drm/selftests/test-drm_rect.c     | 220 ++++++++++++++++++
>   include/drm/drm_rect.h                        |   2 +
>   6 files changed, 257 insertions(+), 15 deletions(-)
>   create mode 100644 drivers/gpu/drm/selftests/test-drm_rect.c
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel