All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/rect: Add midpoint to scale calculation functions
@ 2018-04-24 11:36 Maarten Lankhorst
  2018-04-24 11:36 ` [PATCH 2/2] drm/i915: Do not adjust scale when out of bounds Maarten Lankhorst
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Maarten Lankhorst @ 2018-04-24 11:36 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Vidya Srinivas

When calculating limits we want to be as pessimistic as possible,
so we have to explicitly say whether we want to round up or down
to accurately calculate whether we are below min_scale or above
max_scale.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c |  4 +--
 drivers/gpu/drm/drm_rect.c          | 39 +++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_sprite.c |  8 +++---
 include/drm/drm_rect.h              |  8 +++---
 4 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 9cb2209f6fc8..7643202bfcf7 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -754,8 +754,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
 	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
 
 	/* Check scaling */
-	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
-	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
+	hscale = drm_rect_calc_hscale(src, dst, min_scale, 1<<16, max_scale);
+	vscale = drm_rect_calc_vscale(src, dst, min_scale, 1<<16, max_scale);
 	if (hscale < 0 || vscale < 0) {
 		DRM_DEBUG_KMS("Invalid scaling of plane\n");
 		drm_rect_debug_print("src: ", &plane_state->src, true);
diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
index 9817c1445ba9..2093474c268c 100644
--- a/drivers/gpu/drm/drm_rect.c
+++ b/drivers/gpu/drm/drm_rect.c
@@ -96,7 +96,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
 }
 EXPORT_SYMBOL(drm_rect_clip_scaled);
 
-static int drm_calc_scale(int src, int dst)
+static int drm_calc_scale(int src, int dst, int scale_center)
 {
 	int scale = 0;
 
@@ -106,7 +106,10 @@ static int drm_calc_scale(int src, int dst)
 	if (dst == 0)
 		return 0;
 
-	scale = src / dst;
+	if (DIV_ROUND_UP(dst, scale_center) > src)
+		return DIV_ROUND_UP(src, dst);
+	else
+		scale = src / dst;
 
 	return scale;
 }
@@ -116,21 +119,25 @@ static int drm_calc_scale(int src, int dst)
  * @src: source window rectangle
  * @dst: destination window rectangle
  * @min_hscale: minimum allowed horizontal scaling factor
+ * @mid_hscale: mid point, below this point round down scaling, above round up.
  * @max_hscale: maximum allowed horizontal scaling factor
  *
  * Calculate the horizontal scaling factor as
  * (@src width) / (@dst width).
  *
+ * If the scale is below @mid_hscale we round down, if above up. This will
+ * calculate the hscale with the most pessimistic limit calculation.
+ *
  * RETURNS:
  * The horizontal scaling factor, or errno of out of limits.
  */
 int drm_rect_calc_hscale(const struct drm_rect *src,
 			 const struct drm_rect *dst,
-			 int min_hscale, int max_hscale)
+			 int min_hscale, int mid_hscale, int max_hscale)
 {
 	int src_w = drm_rect_width(src);
 	int dst_w = drm_rect_width(dst);
-	int hscale = drm_calc_scale(src_w, dst_w);
+	int hscale = drm_calc_scale(src_w, dst_w, mid_hscale);
 
 	if (hscale < 0 || dst_w == 0)
 		return hscale;
@@ -147,21 +154,25 @@ EXPORT_SYMBOL(drm_rect_calc_hscale);
  * @src: source window rectangle
  * @dst: destination window rectangle
  * @min_vscale: minimum allowed vertical scaling factor
+ * @mid_vscale: mid point, below this point round down scaling, above round up.
  * @max_vscale: maximum allowed vertical scaling factor
  *
  * Calculate the vertical scaling factor as
  * (@src height) / (@dst height).
  *
+ * If the scale is below @mid_vscale we round down, if above up. This will
+ * calculate the vscale with the most pessimistic limit calculation.
+ *
  * RETURNS:
  * The vertical scaling factor, or errno of out of limits.
  */
 int drm_rect_calc_vscale(const struct drm_rect *src,
 			 const struct drm_rect *dst,
-			 int min_vscale, int max_vscale)
+			 int min_vscale, int mid_vscale, int max_vscale)
 {
 	int src_h = drm_rect_height(src);
 	int dst_h = drm_rect_height(dst);
-	int vscale = drm_calc_scale(src_h, dst_h);
+	int vscale = drm_calc_scale(src_h, dst_h, mid_vscale);
 
 	if (vscale < 0 || dst_h == 0)
 		return vscale;
@@ -178,6 +189,7 @@ EXPORT_SYMBOL(drm_rect_calc_vscale);
  * @src: source window rectangle
  * @dst: destination window rectangle
  * @min_hscale: minimum allowed horizontal scaling factor
+ * @mid_hscale: mid point, below this point round down scaling, above round up.
  * @max_hscale: maximum allowed horizontal scaling factor
  *
  * Calculate the horizontal scaling factor as
@@ -189,16 +201,19 @@ EXPORT_SYMBOL(drm_rect_calc_vscale);
  * If the calculated scaling factor is above @max_vscale,
  * decrease the height of rectangle @src to compensate.
  *
+ * If the scale is below @mid_hscale we round down, if above up. This will
+ * calculate the hscale with the most pessimistic limit calculation.
+ *
  * RETURNS:
  * The horizontal scaling factor.
  */
 int drm_rect_calc_hscale_relaxed(struct drm_rect *src,
 				 struct drm_rect *dst,
-				 int min_hscale, int max_hscale)
+				 int min_hscale, int mid_hscale, int max_hscale)
 {
 	int src_w = drm_rect_width(src);
 	int dst_w = drm_rect_width(dst);
-	int hscale = drm_calc_scale(src_w, dst_w);
+	int hscale = drm_calc_scale(src_w, dst_w, mid_hscale);
 
 	if (hscale < 0 || dst_w == 0)
 		return hscale;
@@ -228,6 +243,7 @@ EXPORT_SYMBOL(drm_rect_calc_hscale_relaxed);
  * @src: source window rectangle
  * @dst: destination window rectangle
  * @min_vscale: minimum allowed vertical scaling factor
+ * @mid_vscale: mid point, below this point round down scaling, above round up.
  * @max_vscale: maximum allowed vertical scaling factor
  *
  * Calculate the vertical scaling factor as
@@ -239,16 +255,19 @@ EXPORT_SYMBOL(drm_rect_calc_hscale_relaxed);
  * If the calculated scaling factor is above @max_vscale,
  * decrease the height of rectangle @src to compensate.
  *
+ * If the scale is below @mid_vscale we round down, if above up. This will
+ * calculate the vscale with the most pessimistic limit calculation.
+ *
  * RETURNS:
  * The vertical scaling factor.
  */
 int drm_rect_calc_vscale_relaxed(struct drm_rect *src,
 				 struct drm_rect *dst,
-				 int min_vscale, int max_vscale)
+				 int min_vscale, int mid_vscale, int max_vscale)
 {
 	int src_h = drm_rect_height(src);
 	int dst_h = drm_rect_height(dst);
-	int vscale = drm_calc_scale(src_h, dst_h);
+	int vscale = drm_calc_scale(src_h, mid_vscale, dst_h);
 
 	if (vscale < 0 || dst_h == 0)
 		return vscale;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index aa1dfaa692b9..f8f7c66fc3ac 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -998,10 +998,10 @@ intel_check_sprite_plane(struct intel_plane *plane,
 	drm_rect_rotate(src, fb->width << 16, fb->height << 16,
 			state->base.rotation);
 
-	hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale);
+	hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, 1 << 16, max_scale);
 	BUG_ON(hscale < 0);
 
-	vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale);
+	vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, 1 << 16, max_scale);
 	BUG_ON(vscale < 0);
 
 	if (crtc_state->base.enable)
@@ -1017,7 +1017,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
 
 	if (state->base.visible) {
 		/* check again in case clipping clamped the results */
-		hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
+		hscale = drm_rect_calc_hscale(src, dst, min_scale, 1 << 16, max_scale);
 		if (hscale < 0) {
 			DRM_DEBUG_KMS("Horizontal scaling factor out of limits\n");
 			drm_rect_debug_print("src: ", src, true);
@@ -1026,7 +1026,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
 			return hscale;
 		}
 
-		vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
+		vscale = drm_rect_calc_vscale(src, dst, min_scale, 1 << 16, max_scale);
 		if (vscale < 0) {
 			DRM_DEBUG_KMS("Vertical scaling factor out of limits\n");
 			drm_rect_debug_print("src: ", src, true);
diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
index 44bc122b9ee0..dbb8631a9288 100644
--- a/include/drm/drm_rect.h
+++ b/include/drm/drm_rect.h
@@ -179,16 +179,16 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
 			  int hscale, int vscale);
 int drm_rect_calc_hscale(const struct drm_rect *src,
 			 const struct drm_rect *dst,
-			 int min_hscale, int max_hscale);
+			 int min_hscale, int mid_hscale, int max_hscale);
 int drm_rect_calc_vscale(const struct drm_rect *src,
 			 const struct drm_rect *dst,
-			 int min_vscale, int max_vscale);
+			 int min_vscale, int mid_vscale, int max_vscale);
 int drm_rect_calc_hscale_relaxed(struct drm_rect *src,
 				 struct drm_rect *dst,
-				 int min_hscale, int max_hscale);
+				 int min_hscale, int mid_hscale, int max_hscale);
 int drm_rect_calc_vscale_relaxed(struct drm_rect *src,
 				 struct drm_rect *dst,
-				 int min_vscale, int max_vscale);
+				 int min_vscale, int mid_vscale, int max_vscale);
 void drm_rect_debug_print(const char *prefix,
 			  const struct drm_rect *r, bool fixed_point);
 void drm_rect_rotate(struct drm_rect *r,
-- 
2.17.0

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] drm/i915: Do not adjust scale when out of bounds.
  2018-04-24 11:36 [PATCH 1/2] drm/rect: Add midpoint to scale calculation functions Maarten Lankhorst
@ 2018-04-24 11:36 ` Maarten Lankhorst
  2018-04-24 15:34   ` Ville Syrjälä
  2018-04-24 12:21 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/rect: Add midpoint to scale calculation functions Patchwork
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Maarten Lankhorst @ 2018-04-24 11:36 UTC (permalink / raw)
  To: intel-gfx, dri-devel

With the previous patch drm_atomic_helper_check_plane_state correctly
calculates clipping and the xf86-video-intel ddx is fixed to fall back
to GPU correctly when SetPlane fails, we can remove the hack where
we try to pan/zoom when out of min/max scaling range. This was already
poor behavior where the screen didn't show what was requested, and now
instead we reject it outright. This simplifies check_sprite_plane a lot.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 142 +++++++---------------------
 1 file changed, 36 insertions(+), 106 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index f8f7c66fc3ac..d4003d24883b 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -936,22 +936,12 @@ intel_check_sprite_plane(struct intel_plane *plane,
 	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_framebuffer *fb = state->base.fb;
-	int crtc_x, crtc_y;
-	unsigned int crtc_w, crtc_h;
-	uint32_t src_x, src_y, src_w, src_h;
-	struct drm_rect *src = &state->base.src;
-	struct drm_rect *dst = &state->base.dst;
-	struct drm_rect clip = {};
 	int max_stride = INTEL_GEN(dev_priv) >= 9 ? 32768 : 16384;
-	int hscale, vscale;
 	int max_scale, min_scale;
 	bool can_scale;
 	int ret;
 	uint32_t pixel_format = 0;
 
-	*src = drm_plane_state_src(&state->base);
-	*dst = drm_plane_state_dest(&state->base);
-
 	if (!fb) {
 		state->base.visible = false;
 		return 0;
@@ -990,64 +980,19 @@ intel_check_sprite_plane(struct intel_plane *plane,
 		min_scale = plane->can_scale ? 1 : (1 << 16);
 	}
 
-	/*
-	 * 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.
-	 */
-	drm_rect_rotate(src, fb->width << 16, fb->height << 16,
-			state->base.rotation);
-
-	hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, 1 << 16, max_scale);
-	BUG_ON(hscale < 0);
-
-	vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, 1 << 16, max_scale);
-	BUG_ON(vscale < 0);
-
-	if (crtc_state->base.enable)
-		drm_mode_get_hv_timing(&crtc_state->base.mode,
-				       &clip.x2, &clip.y2);
-
-	state->base.visible = drm_rect_clip_scaled(src, dst, &clip, hscale, vscale);
-
-	crtc_x = dst->x1;
-	crtc_y = dst->y1;
-	crtc_w = drm_rect_width(dst);
-	crtc_h = drm_rect_height(dst);
+	ret = drm_atomic_helper_check_plane_state(&state->base,
+						  &crtc_state->base,
+						  min_scale, max_scale,
+						  true, true);
+	if (ret)
+		return ret;
 
 	if (state->base.visible) {
-		/* check again in case clipping clamped the results */
-		hscale = drm_rect_calc_hscale(src, dst, min_scale, 1 << 16, max_scale);
-		if (hscale < 0) {
-			DRM_DEBUG_KMS("Horizontal scaling factor out of limits\n");
-			drm_rect_debug_print("src: ", src, true);
-			drm_rect_debug_print("dst: ", dst, false);
-
-			return hscale;
-		}
-
-		vscale = drm_rect_calc_vscale(src, dst, min_scale, 1 << 16, max_scale);
-		if (vscale < 0) {
-			DRM_DEBUG_KMS("Vertical scaling factor out of limits\n");
-			drm_rect_debug_print("src: ", src, true);
-			drm_rect_debug_print("dst: ", dst, false);
-
-			return vscale;
-		}
-
-		/* Make the source viewport size an exact multiple of the scaling factors. */
-		drm_rect_adjust_size(src,
-				     drm_rect_width(dst) * hscale - drm_rect_width(src),
-				     drm_rect_height(dst) * vscale - drm_rect_height(src));
-
-		drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
-				    state->base.rotation);
-
-		/* sanity check to make sure the src viewport wasn't enlarged */
-		WARN_ON(src->x1 < (int) state->base.src_x ||
-			src->y1 < (int) state->base.src_y ||
-			src->x2 > (int) state->base.src_x + state->base.src_w ||
-			src->y2 > (int) state->base.src_y + state->base.src_h);
+		struct drm_rect *src = &state->base.src;
+		struct drm_rect *dst = &state->base.dst;
+		unsigned int crtc_w = drm_rect_width(dst);
+		unsigned int crtc_h = drm_rect_width(dst);
+		uint32_t src_x, src_y, src_w, src_h;
 
 		/*
 		 * Hardware doesn't handle subpixel coordinates.
@@ -1060,58 +1005,43 @@ intel_check_sprite_plane(struct intel_plane *plane,
 		src_y = src->y1 >> 16;
 		src_h = drm_rect_height(src) >> 16;
 
-		if (intel_format_is_yuv(fb->format->format)) {
-			src_x &= ~1;
-			src_w &= ~1;
-
-			/*
-			 * Must keep src and dst the
-			 * same if we can't scale.
-			 */
-			if (!can_scale)
-				crtc_w &= ~1;
+		src->x1 = src_x << 16;
+		src->x2 = (src_x + src_w) << 16;
+		src->y1 = src_y << 16;
+		src->y2 = (src_y + src_h) << 16;
 
-			if (crtc_w == 0)
-				state->base.visible = false;
+		if (intel_format_is_yuv(fb->format->format) &&
+		    (src_x % 2 || src_w % 2)) {
+			DRM_DEBUG_KMS("src x/w (%u, %u) must be a multiple of 2 for YUV planes\n",
+				      src_x, src_w);
+			return -EINVAL;
 		}
-	}
 
-	/* Check size restrictions when scaling */
-	if (state->base.visible && (src_w != crtc_w || src_h != crtc_h)) {
-		unsigned int width_bytes;
-		int cpp = fb->format->cpp[0];
+		/* Check size restrictions when scaling */
+		if (src_w != crtc_w || src_h != crtc_h) {
+			unsigned int width_bytes;
+			int cpp = fb->format->cpp[0];
 
-		WARN_ON(!can_scale);
+			WARN_ON(!can_scale);
 
-		/* FIXME interlacing min height is 6 */
+			/* FIXME interlacing min height is 6 */
 
-		if (crtc_w < 3 || crtc_h < 3)
-			state->base.visible = false;
+			if (crtc_w < 3 || crtc_h < 3)
+				state->base.visible = false;
 
-		if (src_w < 3 || src_h < 3)
-			state->base.visible = false;
+			if (src_w < 3 || src_h < 3)
+				state->base.visible = false;
 
-		width_bytes = ((src_x * cpp) & 63) + src_w * cpp;
+			width_bytes = ((src_x * cpp) & 63) + src_w * cpp;
 
-		if (INTEL_GEN(dev_priv) < 9 && (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 (INTEL_GEN(dev_priv) < 9 && (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 (state->base.visible) {
-		src->x1 = src_x << 16;
-		src->x2 = (src_x + src_w) << 16;
-		src->y1 = src_y << 16;
-		src->y2 = (src_y + src_h) << 16;
-	}
-
-	dst->x1 = crtc_x;
-	dst->x2 = crtc_x + crtc_w;
-	dst->y1 = crtc_y;
-	dst->y2 = crtc_y + crtc_h;
-
 	if (INTEL_GEN(dev_priv) >= 9) {
 		ret = skl_check_plane_surface(crtc_state, state);
 		if (ret)
-- 
2.17.0

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/rect: Add midpoint to scale calculation functions
  2018-04-24 11:36 [PATCH 1/2] drm/rect: Add midpoint to scale calculation functions Maarten Lankhorst
  2018-04-24 11:36 ` [PATCH 2/2] drm/i915: Do not adjust scale when out of bounds Maarten Lankhorst
@ 2018-04-24 12:21 ` Patchwork
  2018-04-24 12:37 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-04-24 12:21 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/rect: Add midpoint to scale calculation functions
URL   : https://patchwork.freedesktop.org/series/42186/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
aca1294901eb drm/rect: Add midpoint to scale calculation functions
-:23: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#23: FILE: drivers/gpu/drm/drm_atomic_helper.c:757:
+	hscale = drm_rect_calc_hscale(src, dst, min_scale, 1<<16, max_scale);
 	                                                    ^

-:24: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#24: FILE: drivers/gpu/drm/drm_atomic_helper.c:758:
+	vscale = drm_rect_calc_vscale(src, dst, min_scale, 1<<16, max_scale);
 	                                                    ^

-:48: WARNING:UNNECESSARY_ELSE: else is not generally useful after a break or return
#48: FILE: drivers/gpu/drm/drm_rect.c:111:
+		return DIV_ROUND_UP(src, dst);
+	else

total: 0 errors, 1 warnings, 2 checks, 187 lines checked
97d164bdcbbe drm/i915: Do not adjust scale when out of bounds.
-:180: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#180: FILE: drivers/gpu/drm/i915/intel_sprite.c:1038:
+			if (INTEL_GEN(dev_priv) < 9 && (src_w > 2048 || src_h > 2048 ||
+			    width_bytes > 4096 || fb->pitches[0] > 4096)) {

total: 0 errors, 0 warnings, 1 checks, 180 lines checked

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/rect: Add midpoint to scale calculation functions
  2018-04-24 11:36 [PATCH 1/2] drm/rect: Add midpoint to scale calculation functions Maarten Lankhorst
  2018-04-24 11:36 ` [PATCH 2/2] drm/i915: Do not adjust scale when out of bounds Maarten Lankhorst
  2018-04-24 12:21 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/rect: Add midpoint to scale calculation functions Patchwork
@ 2018-04-24 12:37 ` Patchwork
  2018-04-24 15:05 ` ✗ Fi.CI.IGT: failure " Patchwork
  2018-04-24 15:21 ` [PATCH 1/2] " Ville Syrjälä
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-04-24 12:37 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/rect: Add midpoint to scale calculation functions
URL   : https://patchwork.freedesktop.org/series/42186/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4084 -> Patchwork_8785 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42186/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_8785 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@debugfs_test@read_all_entries:
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-cnl-psr:         PASS -> DMESG-WARN (fdo#104951)

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
      fi-skl-6700k2:      FAIL (fdo#103191) -> PASS

    
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951


== Participating hosts (36 -> 33) ==

  Missing    (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4084 -> Patchwork_8785

  CI_DRM_4084: 45349bde096d69db4d81b55ce2f4b4a48eec4834 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4446: e5e8dafc991ee922ec159491c680caff0cfe9235 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8785: 97d164bdcbbe3b28305f58a8c0d2e164b47be7c3 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4446: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit


== Linux commits ==

97d164bdcbbe drm/i915: Do not adjust scale when out of bounds.
aca1294901eb drm/rect: Add midpoint to scale calculation functions

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8785/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* ✗ Fi.CI.IGT: failure for series starting with [1/2] drm/rect: Add midpoint to scale calculation functions
  2018-04-24 11:36 [PATCH 1/2] drm/rect: Add midpoint to scale calculation functions Maarten Lankhorst
                   ` (2 preceding siblings ...)
  2018-04-24 12:37 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-04-24 15:05 ` Patchwork
  2018-04-24 15:21 ` [PATCH 1/2] " Ville Syrjälä
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-04-24 15:05 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/rect: Add midpoint to scale calculation functions
URL   : https://patchwork.freedesktop.org/series/42186/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4084_full -> Patchwork_8785_full =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_8785_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8785_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42186/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8785_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_atomic@plane_overlay_legacy:
      shard-hsw:          PASS -> DMESG-WARN +21

    igt@kms_rmfb@close-fd:
      shard-hsw:          PASS -> DMESG-FAIL +7

    
    ==== Warnings ====

    igt@gem_exec_schedule@deep-vebox:
      shard-kbl:          SKIP -> PASS

    igt@gem_mocs_settings@mocs-rc6-blt:
      shard-kbl:          PASS -> SKIP +1

    igt@kms_atomic_transition@plane-use-after-nonblocking-unbind:
      shard-hsw:          PASS -> SKIP +8

    igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-untiled:
      shard-glk:          PASS -> SKIP +67

    igt@kms_mmap_write_crc:
      shard-glk:          SKIP -> PASS +108

    igt@pm_rc6_residency@rc6-accuracy:
      shard-snb:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_8785_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_schedule@pi-ringfull-blt:
      shard-kbl:          NOTRUN -> FAIL (fdo#103158)

    igt@gem_exec_store@cachelines-bsd:
      shard-hsw:          PASS -> FAIL (fdo#100007)

    igt@kms_flip@blocking-absolute-wf_vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#106134)

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#102887, fdo#105363)

    igt@kms_flip@plain-flip-fb-recreate-interruptible:
      shard-glk:          SKIP -> FAIL (fdo#100368)
      shard-hsw:          PASS -> FAIL (fdo#100368)

    igt@kms_flip@wf_vblank-ts-check-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@kms_setmode@basic:
      shard-hsw:          PASS -> FAIL (fdo#99912)

    igt@perf@polling:
      shard-hsw:          PASS -> FAIL (fdo#102252)

    
    ==== Possible fixes ====

    igt@kms_flip@2x-plain-flip-ts-check-interruptible:
      shard-hsw:          FAIL (fdo#100368) -> PASS +1

    igt@kms_flip@flip-vs-dpms-off-vs-modeset:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    igt@kms_flip@modeset-vs-vblank-race:
      shard-hsw:          FAIL (fdo#103060) -> PASS

    igt@kms_setmode@basic:
      shard-glk:          FAIL (fdo#99912) -> PASS

    
  fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#106134 https://bugs.freedesktop.org/show_bug.cgi?id=106134
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 5) ==

  Missing    (1): shard-glkb 


== Build changes ==

    * Linux: CI_DRM_4084 -> Patchwork_8785

  CI_DRM_4084: 45349bde096d69db4d81b55ce2f4b4a48eec4834 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4446: e5e8dafc991ee922ec159491c680caff0cfe9235 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8785: 97d164bdcbbe3b28305f58a8c0d2e164b47be7c3 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4446: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8785/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] drm/rect: Add midpoint to scale calculation functions
  2018-04-24 15:21 ` [PATCH 1/2] " Ville Syrjälä
@ 2018-04-24 15:11   ` Maarten Lankhorst
  2018-04-24 15:51     ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: Maarten Lankhorst @ 2018-04-24 15:11 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Vidya Srinivas, dri-devel

Op 24-04-18 om 17:21 schreef Ville Syrjälä:
> On Tue, Apr 24, 2018 at 01:36:32PM +0200, Maarten Lankhorst wrote:
>> When calculating limits we want to be as pessimistic as possible,
>> so we have to explicitly say whether we want to round up or down
>> to accurately calculate whether we are below min_scale or above
>> max_scale.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/drm_atomic_helper.c |  4 +--
>>  drivers/gpu/drm/drm_rect.c          | 39 +++++++++++++++++++++--------
>>  drivers/gpu/drm/i915/intel_sprite.c |  8 +++---
>>  include/drm/drm_rect.h              |  8 +++---
>>  4 files changed, 39 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index 9cb2209f6fc8..7643202bfcf7 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -754,8 +754,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
>>  	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
>>  
>>  	/* Check scaling */
>> -	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
>> -	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
>> +	hscale = drm_rect_calc_hscale(src, dst, min_scale, 1<<16, max_scale);
>> +	vscale = drm_rect_calc_vscale(src, dst, min_scale, 1<<16, max_scale);
>>  	if (hscale < 0 || vscale < 0) {
>>  		DRM_DEBUG_KMS("Invalid scaling of plane\n");
>>  		drm_rect_debug_print("src: ", &plane_state->src, true);
>> diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
>> index 9817c1445ba9..2093474c268c 100644
>> --- a/drivers/gpu/drm/drm_rect.c
>> +++ b/drivers/gpu/drm/drm_rect.c
>> @@ -96,7 +96,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
>>  }
>>  EXPORT_SYMBOL(drm_rect_clip_scaled);
>>  
>> -static int drm_calc_scale(int src, int dst)
>> +static int drm_calc_scale(int src, int dst, int scale_center)
>>  {
>>  	int scale = 0;
>>  
>> @@ -106,7 +106,10 @@ static int drm_calc_scale(int src, int dst)
>>  	if (dst == 0)
>>  		return 0;
>>  
>> -	scale = src / dst;
>> +	if (DIV_ROUND_UP(dst, scale_center) > src)
> That doesn't look right to me.
>
> How about just 'if (src > dst << 16)' ?
>
> I guess if we want to keep this code independent of the number of
> fractional bits we could pass the 16 in from the caller(s). Not sure
> there's much point in that though.
pass a bool round_up to the calc_scale functions?
>> +		return DIV_ROUND_UP(src, dst);
>> +	else
>> +		scale = src / dst;
>>  
>>  	return scale;
>>  }
>> @@ -116,21 +119,25 @@ static int drm_calc_scale(int src, int dst)
>>   * @src: source window rectangle
>>   * @dst: destination window rectangle
>>   * @min_hscale: minimum allowed horizontal scaling factor
>> + * @mid_hscale: mid point, below this point round down scaling, above round up.
>>   * @max_hscale: maximum allowed horizontal scaling factor
>>   *
>>   * Calculate the horizontal scaling factor as
>>   * (@src width) / (@dst width).
>>   *
>> + * If the scale is below @mid_hscale we round down, if above up. This will
>> + * calculate the hscale with the most pessimistic limit calculation.
>> + *
>>   * RETURNS:
>>   * The horizontal scaling factor, or errno of out of limits.
>>   */
>>  int drm_rect_calc_hscale(const struct drm_rect *src,
>>  			 const struct drm_rect *dst,
>> -			 int min_hscale, int max_hscale)
>> +			 int min_hscale, int mid_hscale, int max_hscale)
>>  {
>>  	int src_w = drm_rect_width(src);
>>  	int dst_w = drm_rect_width(dst);
>> -	int hscale = drm_calc_scale(src_w, dst_w);
>> +	int hscale = drm_calc_scale(src_w, dst_w, mid_hscale);
>>  
>>  	if (hscale < 0 || dst_w == 0)
>>  		return hscale;
>> @@ -147,21 +154,25 @@ EXPORT_SYMBOL(drm_rect_calc_hscale);
>>   * @src: source window rectangle
>>   * @dst: destination window rectangle
>>   * @min_vscale: minimum allowed vertical scaling factor
>> + * @mid_vscale: mid point, below this point round down scaling, above round up.
>>   * @max_vscale: maximum allowed vertical scaling factor
>>   *
>>   * Calculate the vertical scaling factor as
>>   * (@src height) / (@dst height).
>>   *
>> + * If the scale is below @mid_vscale we round down, if above up. This will
>> + * calculate the vscale with the most pessimistic limit calculation.
>> + *
>>   * RETURNS:
>>   * The vertical scaling factor, or errno of out of limits.
>>   */
>>  int drm_rect_calc_vscale(const struct drm_rect *src,
>>  			 const struct drm_rect *dst,
>> -			 int min_vscale, int max_vscale)
>> +			 int min_vscale, int mid_vscale, int max_vscale)
>>  {
>>  	int src_h = drm_rect_height(src);
>>  	int dst_h = drm_rect_height(dst);
>> -	int vscale = drm_calc_scale(src_h, dst_h);
>> +	int vscale = drm_calc_scale(src_h, dst_h, mid_vscale);
>>  
>>  	if (vscale < 0 || dst_h == 0)
>>  		return vscale;
>> @@ -178,6 +189,7 @@ EXPORT_SYMBOL(drm_rect_calc_vscale);
>>   * @src: source window rectangle
>>   * @dst: destination window rectangle
>>   * @min_hscale: minimum allowed horizontal scaling factor
>> + * @mid_hscale: mid point, below this point round down scaling, above round up.
>>   * @max_hscale: maximum allowed horizontal scaling factor
>>   *
>>   * Calculate the horizontal scaling factor as
>> @@ -189,16 +201,19 @@ EXPORT_SYMBOL(drm_rect_calc_vscale);
>>   * If the calculated scaling factor is above @max_vscale,
>>   * decrease the height of rectangle @src to compensate.
>>   *
>> + * If the scale is below @mid_hscale we round down, if above up. This will
>> + * calculate the hscale with the most pessimistic limit calculation.
>> + *
>>   * RETURNS:
>>   * The horizontal scaling factor.
>>   */
>>  int drm_rect_calc_hscale_relaxed(struct drm_rect *src,
>>  				 struct drm_rect *dst,
>> -				 int min_hscale, int max_hscale)
>> +				 int min_hscale, int mid_hscale, int max_hscale)
>>  {
>>  	int src_w = drm_rect_width(src);
>>  	int dst_w = drm_rect_width(dst);
>> -	int hscale = drm_calc_scale(src_w, dst_w);
>> +	int hscale = drm_calc_scale(src_w, dst_w, mid_hscale);
>>  
>>  	if (hscale < 0 || dst_w == 0)
>>  		return hscale;
>> @@ -228,6 +243,7 @@ EXPORT_SYMBOL(drm_rect_calc_hscale_relaxed);
>>   * @src: source window rectangle
>>   * @dst: destination window rectangle
>>   * @min_vscale: minimum allowed vertical scaling factor
>> + * @mid_vscale: mid point, below this point round down scaling, above round up.
>>   * @max_vscale: maximum allowed vertical scaling factor
>>   *
>>   * Calculate the vertical scaling factor as
>> @@ -239,16 +255,19 @@ EXPORT_SYMBOL(drm_rect_calc_hscale_relaxed);
>>   * If the calculated scaling factor is above @max_vscale,
>>   * decrease the height of rectangle @src to compensate.
>>   *
>> + * If the scale is below @mid_vscale we round down, if above up. This will
>> + * calculate the vscale with the most pessimistic limit calculation.
>> + *
>>   * RETURNS:
>>   * The vertical scaling factor.
>>   */
>>  int drm_rect_calc_vscale_relaxed(struct drm_rect *src,
>>  				 struct drm_rect *dst,
>> -				 int min_vscale, int max_vscale)
>> +				 int min_vscale, int mid_vscale, int max_vscale)
>>  {
>>  	int src_h = drm_rect_height(src);
>>  	int dst_h = drm_rect_height(dst);
>> -	int vscale = drm_calc_scale(src_h, dst_h);
>> +	int vscale = drm_calc_scale(src_h, mid_vscale, dst_h);
>>  
>>  	if (vscale < 0 || dst_h == 0)
>>  		return vscale;
>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
>> index aa1dfaa692b9..f8f7c66fc3ac 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -998,10 +998,10 @@ intel_check_sprite_plane(struct intel_plane *plane,
>>  	drm_rect_rotate(src, fb->width << 16, fb->height << 16,
>>  			state->base.rotation);
>>  
>> -	hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale);
>> +	hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, 1 << 16, max_scale);
>>  	BUG_ON(hscale < 0);
>>  
>> -	vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale);
>> +	vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, 1 << 16, max_scale);
>>  	BUG_ON(vscale < 0);
>>  
>>  	if (crtc_state->base.enable)
>> @@ -1017,7 +1017,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
>>  
>>  	if (state->base.visible) {
>>  		/* check again in case clipping clamped the results */
>> -		hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
>> +		hscale = drm_rect_calc_hscale(src, dst, min_scale, 1 << 16, max_scale);
>>  		if (hscale < 0) {
>>  			DRM_DEBUG_KMS("Horizontal scaling factor out of limits\n");
>>  			drm_rect_debug_print("src: ", src, true);
>> @@ -1026,7 +1026,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
>>  			return hscale;
>>  		}
>>  
>> -		vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
>> +		vscale = drm_rect_calc_vscale(src, dst, min_scale, 1 << 16, max_scale);
>>  		if (vscale < 0) {
>>  			DRM_DEBUG_KMS("Vertical scaling factor out of limits\n");
>>  			drm_rect_debug_print("src: ", src, true);
>> diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
>> index 44bc122b9ee0..dbb8631a9288 100644
>> --- a/include/drm/drm_rect.h
>> +++ b/include/drm/drm_rect.h
>> @@ -179,16 +179,16 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
>>  			  int hscale, int vscale);
>>  int drm_rect_calc_hscale(const struct drm_rect *src,
>>  			 const struct drm_rect *dst,
>> -			 int min_hscale, int max_hscale);
>> +			 int min_hscale, int mid_hscale, int max_hscale);
>>  int drm_rect_calc_vscale(const struct drm_rect *src,
>>  			 const struct drm_rect *dst,
>> -			 int min_vscale, int max_vscale);
>> +			 int min_vscale, int mid_vscale, int max_vscale);
>>  int drm_rect_calc_hscale_relaxed(struct drm_rect *src,
>>  				 struct drm_rect *dst,
>> -				 int min_hscale, int max_hscale);
>> +				 int min_hscale, int mid_hscale, int max_hscale);
>>  int drm_rect_calc_vscale_relaxed(struct drm_rect *src,
>>  				 struct drm_rect *dst,
>> -				 int min_vscale, int max_vscale);
>> +				 int min_vscale, int mid_vscale, int max_vscale);
>>  void drm_rect_debug_print(const char *prefix,
>>  			  const struct drm_rect *r, bool fixed_point);
>>  void drm_rect_rotate(struct drm_rect *r,
>> -- 
>> 2.17.0


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] drm/rect: Add midpoint to scale calculation functions
  2018-04-24 11:36 [PATCH 1/2] drm/rect: Add midpoint to scale calculation functions Maarten Lankhorst
                   ` (3 preceding siblings ...)
  2018-04-24 15:05 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2018-04-24 15:21 ` Ville Syrjälä
  2018-04-24 15:11   ` Maarten Lankhorst
  4 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2018-04-24 15:21 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx, dri-devel

On Tue, Apr 24, 2018 at 01:36:32PM +0200, Maarten Lankhorst wrote:
> When calculating limits we want to be as pessimistic as possible,
> so we have to explicitly say whether we want to round up or down
> to accurately calculate whether we are below min_scale or above
> max_scale.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c |  4 +--
>  drivers/gpu/drm/drm_rect.c          | 39 +++++++++++++++++++++--------
>  drivers/gpu/drm/i915/intel_sprite.c |  8 +++---
>  include/drm/drm_rect.h              |  8 +++---
>  4 files changed, 39 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 9cb2209f6fc8..7643202bfcf7 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -754,8 +754,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
>  	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
>  
>  	/* Check scaling */
> -	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> -	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> +	hscale = drm_rect_calc_hscale(src, dst, min_scale, 1<<16, max_scale);
> +	vscale = drm_rect_calc_vscale(src, dst, min_scale, 1<<16, max_scale);
>  	if (hscale < 0 || vscale < 0) {
>  		DRM_DEBUG_KMS("Invalid scaling of plane\n");
>  		drm_rect_debug_print("src: ", &plane_state->src, true);
> diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
> index 9817c1445ba9..2093474c268c 100644
> --- a/drivers/gpu/drm/drm_rect.c
> +++ b/drivers/gpu/drm/drm_rect.c
> @@ -96,7 +96,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
>  }
>  EXPORT_SYMBOL(drm_rect_clip_scaled);
>  
> -static int drm_calc_scale(int src, int dst)
> +static int drm_calc_scale(int src, int dst, int scale_center)
>  {
>  	int scale = 0;
>  
> @@ -106,7 +106,10 @@ static int drm_calc_scale(int src, int dst)
>  	if (dst == 0)
>  		return 0;
>  
> -	scale = src / dst;
> +	if (DIV_ROUND_UP(dst, scale_center) > src)

That doesn't look right to me.

How about just 'if (src > dst << 16)' ?

I guess if we want to keep this code independent of the number of
fractional bits we could pass the 16 in from the caller(s). Not sure
there's much point in that though.

> +		return DIV_ROUND_UP(src, dst);
> +	else
> +		scale = src / dst;
>  
>  	return scale;
>  }
> @@ -116,21 +119,25 @@ static int drm_calc_scale(int src, int dst)
>   * @src: source window rectangle
>   * @dst: destination window rectangle
>   * @min_hscale: minimum allowed horizontal scaling factor
> + * @mid_hscale: mid point, below this point round down scaling, above round up.
>   * @max_hscale: maximum allowed horizontal scaling factor
>   *
>   * Calculate the horizontal scaling factor as
>   * (@src width) / (@dst width).
>   *
> + * If the scale is below @mid_hscale we round down, if above up. This will
> + * calculate the hscale with the most pessimistic limit calculation.
> + *
>   * RETURNS:
>   * The horizontal scaling factor, or errno of out of limits.
>   */
>  int drm_rect_calc_hscale(const struct drm_rect *src,
>  			 const struct drm_rect *dst,
> -			 int min_hscale, int max_hscale)
> +			 int min_hscale, int mid_hscale, int max_hscale)
>  {
>  	int src_w = drm_rect_width(src);
>  	int dst_w = drm_rect_width(dst);
> -	int hscale = drm_calc_scale(src_w, dst_w);
> +	int hscale = drm_calc_scale(src_w, dst_w, mid_hscale);
>  
>  	if (hscale < 0 || dst_w == 0)
>  		return hscale;
> @@ -147,21 +154,25 @@ EXPORT_SYMBOL(drm_rect_calc_hscale);
>   * @src: source window rectangle
>   * @dst: destination window rectangle
>   * @min_vscale: minimum allowed vertical scaling factor
> + * @mid_vscale: mid point, below this point round down scaling, above round up.
>   * @max_vscale: maximum allowed vertical scaling factor
>   *
>   * Calculate the vertical scaling factor as
>   * (@src height) / (@dst height).
>   *
> + * If the scale is below @mid_vscale we round down, if above up. This will
> + * calculate the vscale with the most pessimistic limit calculation.
> + *
>   * RETURNS:
>   * The vertical scaling factor, or errno of out of limits.
>   */
>  int drm_rect_calc_vscale(const struct drm_rect *src,
>  			 const struct drm_rect *dst,
> -			 int min_vscale, int max_vscale)
> +			 int min_vscale, int mid_vscale, int max_vscale)
>  {
>  	int src_h = drm_rect_height(src);
>  	int dst_h = drm_rect_height(dst);
> -	int vscale = drm_calc_scale(src_h, dst_h);
> +	int vscale = drm_calc_scale(src_h, dst_h, mid_vscale);
>  
>  	if (vscale < 0 || dst_h == 0)
>  		return vscale;
> @@ -178,6 +189,7 @@ EXPORT_SYMBOL(drm_rect_calc_vscale);
>   * @src: source window rectangle
>   * @dst: destination window rectangle
>   * @min_hscale: minimum allowed horizontal scaling factor
> + * @mid_hscale: mid point, below this point round down scaling, above round up.
>   * @max_hscale: maximum allowed horizontal scaling factor
>   *
>   * Calculate the horizontal scaling factor as
> @@ -189,16 +201,19 @@ EXPORT_SYMBOL(drm_rect_calc_vscale);
>   * If the calculated scaling factor is above @max_vscale,
>   * decrease the height of rectangle @src to compensate.
>   *
> + * If the scale is below @mid_hscale we round down, if above up. This will
> + * calculate the hscale with the most pessimistic limit calculation.
> + *
>   * RETURNS:
>   * The horizontal scaling factor.
>   */
>  int drm_rect_calc_hscale_relaxed(struct drm_rect *src,
>  				 struct drm_rect *dst,
> -				 int min_hscale, int max_hscale)
> +				 int min_hscale, int mid_hscale, int max_hscale)
>  {
>  	int src_w = drm_rect_width(src);
>  	int dst_w = drm_rect_width(dst);
> -	int hscale = drm_calc_scale(src_w, dst_w);
> +	int hscale = drm_calc_scale(src_w, dst_w, mid_hscale);
>  
>  	if (hscale < 0 || dst_w == 0)
>  		return hscale;
> @@ -228,6 +243,7 @@ EXPORT_SYMBOL(drm_rect_calc_hscale_relaxed);
>   * @src: source window rectangle
>   * @dst: destination window rectangle
>   * @min_vscale: minimum allowed vertical scaling factor
> + * @mid_vscale: mid point, below this point round down scaling, above round up.
>   * @max_vscale: maximum allowed vertical scaling factor
>   *
>   * Calculate the vertical scaling factor as
> @@ -239,16 +255,19 @@ EXPORT_SYMBOL(drm_rect_calc_hscale_relaxed);
>   * If the calculated scaling factor is above @max_vscale,
>   * decrease the height of rectangle @src to compensate.
>   *
> + * If the scale is below @mid_vscale we round down, if above up. This will
> + * calculate the vscale with the most pessimistic limit calculation.
> + *
>   * RETURNS:
>   * The vertical scaling factor.
>   */
>  int drm_rect_calc_vscale_relaxed(struct drm_rect *src,
>  				 struct drm_rect *dst,
> -				 int min_vscale, int max_vscale)
> +				 int min_vscale, int mid_vscale, int max_vscale)
>  {
>  	int src_h = drm_rect_height(src);
>  	int dst_h = drm_rect_height(dst);
> -	int vscale = drm_calc_scale(src_h, dst_h);
> +	int vscale = drm_calc_scale(src_h, mid_vscale, dst_h);
>  
>  	if (vscale < 0 || dst_h == 0)
>  		return vscale;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index aa1dfaa692b9..f8f7c66fc3ac 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -998,10 +998,10 @@ intel_check_sprite_plane(struct intel_plane *plane,
>  	drm_rect_rotate(src, fb->width << 16, fb->height << 16,
>  			state->base.rotation);
>  
> -	hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale);
> +	hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, 1 << 16, max_scale);
>  	BUG_ON(hscale < 0);
>  
> -	vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale);
> +	vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, 1 << 16, max_scale);
>  	BUG_ON(vscale < 0);
>  
>  	if (crtc_state->base.enable)
> @@ -1017,7 +1017,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
>  
>  	if (state->base.visible) {
>  		/* check again in case clipping clamped the results */
> -		hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> +		hscale = drm_rect_calc_hscale(src, dst, min_scale, 1 << 16, max_scale);
>  		if (hscale < 0) {
>  			DRM_DEBUG_KMS("Horizontal scaling factor out of limits\n");
>  			drm_rect_debug_print("src: ", src, true);
> @@ -1026,7 +1026,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
>  			return hscale;
>  		}
>  
> -		vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> +		vscale = drm_rect_calc_vscale(src, dst, min_scale, 1 << 16, max_scale);
>  		if (vscale < 0) {
>  			DRM_DEBUG_KMS("Vertical scaling factor out of limits\n");
>  			drm_rect_debug_print("src: ", src, true);
> diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
> index 44bc122b9ee0..dbb8631a9288 100644
> --- a/include/drm/drm_rect.h
> +++ b/include/drm/drm_rect.h
> @@ -179,16 +179,16 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
>  			  int hscale, int vscale);
>  int drm_rect_calc_hscale(const struct drm_rect *src,
>  			 const struct drm_rect *dst,
> -			 int min_hscale, int max_hscale);
> +			 int min_hscale, int mid_hscale, int max_hscale);
>  int drm_rect_calc_vscale(const struct drm_rect *src,
>  			 const struct drm_rect *dst,
> -			 int min_vscale, int max_vscale);
> +			 int min_vscale, int mid_vscale, int max_vscale);
>  int drm_rect_calc_hscale_relaxed(struct drm_rect *src,
>  				 struct drm_rect *dst,
> -				 int min_hscale, int max_hscale);
> +				 int min_hscale, int mid_hscale, int max_hscale);
>  int drm_rect_calc_vscale_relaxed(struct drm_rect *src,
>  				 struct drm_rect *dst,
> -				 int min_vscale, int max_vscale);
> +				 int min_vscale, int mid_vscale, int max_vscale);
>  void drm_rect_debug_print(const char *prefix,
>  			  const struct drm_rect *r, bool fixed_point);
>  void drm_rect_rotate(struct drm_rect *r,
> -- 
> 2.17.0

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] drm/i915: Do not adjust scale when out of bounds.
  2018-04-24 11:36 ` [PATCH 2/2] drm/i915: Do not adjust scale when out of bounds Maarten Lankhorst
@ 2018-04-24 15:34   ` Ville Syrjälä
  0 siblings, 0 replies; 9+ messages in thread
From: Ville Syrjälä @ 2018-04-24 15:34 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx, dri-devel

On Tue, Apr 24, 2018 at 01:36:33PM +0200, Maarten Lankhorst wrote:
> With the previous patch drm_atomic_helper_check_plane_state correctly
> calculates clipping and the xf86-video-intel ddx is fixed to fall back
> to GPU correctly when SetPlane fails, we can remove the hack where
> we try to pan/zoom when out of min/max scaling range. This was already
> poor behavior where the screen didn't show what was requested, and now
> instead we reject it outright. This simplifies check_sprite_plane a lot.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 142 +++++++---------------------
>  1 file changed, 36 insertions(+), 106 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index f8f7c66fc3ac..d4003d24883b 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -936,22 +936,12 @@ intel_check_sprite_plane(struct intel_plane *plane,
>  	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_framebuffer *fb = state->base.fb;
> -	int crtc_x, crtc_y;
> -	unsigned int crtc_w, crtc_h;
> -	uint32_t src_x, src_y, src_w, src_h;
> -	struct drm_rect *src = &state->base.src;
> -	struct drm_rect *dst = &state->base.dst;
> -	struct drm_rect clip = {};
>  	int max_stride = INTEL_GEN(dev_priv) >= 9 ? 32768 : 16384;
> -	int hscale, vscale;
>  	int max_scale, min_scale;
>  	bool can_scale;
>  	int ret;
>  	uint32_t pixel_format = 0;
>  
> -	*src = drm_plane_state_src(&state->base);
> -	*dst = drm_plane_state_dest(&state->base);
> -
>  	if (!fb) {
>  		state->base.visible = false;
>  		return 0;
> @@ -990,64 +980,19 @@ intel_check_sprite_plane(struct intel_plane *plane,
>  		min_scale = plane->can_scale ? 1 : (1 << 16);
>  	}
>  
> -	/*
> -	 * 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.
> -	 */
> -	drm_rect_rotate(src, fb->width << 16, fb->height << 16,
> -			state->base.rotation);
> -
> -	hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, 1 << 16, max_scale);
> -	BUG_ON(hscale < 0);
> -
> -	vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, 1 << 16, max_scale);
> -	BUG_ON(vscale < 0);
> -
> -	if (crtc_state->base.enable)
> -		drm_mode_get_hv_timing(&crtc_state->base.mode,
> -				       &clip.x2, &clip.y2);
> -
> -	state->base.visible = drm_rect_clip_scaled(src, dst, &clip, hscale, vscale);
> -
> -	crtc_x = dst->x1;
> -	crtc_y = dst->y1;
> -	crtc_w = drm_rect_width(dst);
> -	crtc_h = drm_rect_height(dst);
> +	ret = drm_atomic_helper_check_plane_state(&state->base,
> +						  &crtc_state->base,
> +						  min_scale, max_scale,
> +						  true, true);
> +	if (ret)
> +		return ret;
>  
>  	if (state->base.visible) {
> -		/* check again in case clipping clamped the results */
> -		hscale = drm_rect_calc_hscale(src, dst, min_scale, 1 << 16, max_scale);
> -		if (hscale < 0) {
> -			DRM_DEBUG_KMS("Horizontal scaling factor out of limits\n");
> -			drm_rect_debug_print("src: ", src, true);
> -			drm_rect_debug_print("dst: ", dst, false);
> -
> -			return hscale;
> -		}
> -
> -		vscale = drm_rect_calc_vscale(src, dst, min_scale, 1 << 16, max_scale);
> -		if (vscale < 0) {
> -			DRM_DEBUG_KMS("Vertical scaling factor out of limits\n");
> -			drm_rect_debug_print("src: ", src, true);
> -			drm_rect_debug_print("dst: ", dst, false);
> -
> -			return vscale;
> -		}
> -
> -		/* Make the source viewport size an exact multiple of the scaling factors. */
> -		drm_rect_adjust_size(src,
> -				     drm_rect_width(dst) * hscale - drm_rect_width(src),
> -				     drm_rect_height(dst) * vscale - drm_rect_height(src));
> -
> -		drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
> -				    state->base.rotation);
> -
> -		/* sanity check to make sure the src viewport wasn't enlarged */
> -		WARN_ON(src->x1 < (int) state->base.src_x ||
> -			src->y1 < (int) state->base.src_y ||
> -			src->x2 > (int) state->base.src_x + state->base.src_w ||
> -			src->y2 > (int) state->base.src_y + state->base.src_h);
> +		struct drm_rect *src = &state->base.src;
> +		struct drm_rect *dst = &state->base.dst;
> +		unsigned int crtc_w = drm_rect_width(dst);
> +		unsigned int crtc_h = drm_rect_width(dst);
> +		uint32_t src_x, src_y, src_w, src_h;
>  
>  		/*
>  		 * Hardware doesn't handle subpixel coordinates.
> @@ -1060,58 +1005,43 @@ intel_check_sprite_plane(struct intel_plane *plane,
>  		src_y = src->y1 >> 16;
>  		src_h = drm_rect_height(src) >> 16;
>  
> -		if (intel_format_is_yuv(fb->format->format)) {
> -			src_x &= ~1;
> -			src_w &= ~1;
> -
> -			/*
> -			 * Must keep src and dst the
> -			 * same if we can't scale.
> -			 */
> -			if (!can_scale)
> -				crtc_w &= ~1;
> +		src->x1 = src_x << 16;
> +		src->x2 = (src_x + src_w) << 16;
> +		src->y1 = src_y << 16;
> +		src->y2 = (src_y + src_h) << 16;
>  
> -			if (crtc_w == 0)
> -				state->base.visible = false;

Hmm. This makes me wonder what happens if we end up with src_w/h < 1.0
and crtc_w/h > 0. The >>16 will make src_w/h zero, (and I'm not 100%
convinced it couldn't be 0 already due to the scaled clipping and the
rounded up hscale/vscale). So we probably still need some kind of check
for visible(dst) vs. visible(src) after we have the final src coordinates.

> +		if (intel_format_is_yuv(fb->format->format) &&
> +		    (src_x % 2 || src_w % 2)) {
> +			DRM_DEBUG_KMS("src x/w (%u, %u) must be a multiple of 2 for YUV planes\n",
> +				      src_x, src_w);
> +			return -EINVAL;
>  		}
> -	}
>  
> -	/* Check size restrictions when scaling */
> -	if (state->base.visible && (src_w != crtc_w || src_h != crtc_h)) {
> -		unsigned int width_bytes;
> -		int cpp = fb->format->cpp[0];
> +		/* Check size restrictions when scaling */
> +		if (src_w != crtc_w || src_h != crtc_h) {
> +			unsigned int width_bytes;
> +			int cpp = fb->format->cpp[0];
>  
> -		WARN_ON(!can_scale);
> +			WARN_ON(!can_scale);
>  
> -		/* FIXME interlacing min height is 6 */
> +			/* FIXME interlacing min height is 6 */
>  
> -		if (crtc_w < 3 || crtc_h < 3)
> -			state->base.visible = false;
> +			if (crtc_w < 3 || crtc_h < 3)
> +				state->base.visible = false;
>  
> -		if (src_w < 3 || src_h < 3)
> -			state->base.visible = false;
> +			if (src_w < 3 || src_h < 3)
> +				state->base.visible = false;

I guess we should turn these into errors as well. Could be done as a
separate patch though.

>  
> -		width_bytes = ((src_x * cpp) & 63) + src_w * cpp;
> +			width_bytes = ((src_x * cpp) & 63) + src_w * cpp;
>  
> -		if (INTEL_GEN(dev_priv) < 9 && (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 (INTEL_GEN(dev_priv) < 9 && (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 (state->base.visible) {
> -		src->x1 = src_x << 16;
> -		src->x2 = (src_x + src_w) << 16;
> -		src->y1 = src_y << 16;
> -		src->y2 = (src_y + src_h) << 16;
> -	}
> -
> -	dst->x1 = crtc_x;
> -	dst->x2 = crtc_x + crtc_w;
> -	dst->y1 = crtc_y;
> -	dst->y2 = crtc_y + crtc_h;
> -
>  	if (INTEL_GEN(dev_priv) >= 9) {
>  		ret = skl_check_plane_surface(crtc_state, state);
>  		if (ret)
> -- 
> 2.17.0

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] drm/rect: Add midpoint to scale calculation functions
  2018-04-24 15:11   ` Maarten Lankhorst
@ 2018-04-24 15:51     ` Ville Syrjälä
  0 siblings, 0 replies; 9+ messages in thread
From: Ville Syrjälä @ 2018-04-24 15:51 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx, dri-devel

On Tue, Apr 24, 2018 at 05:11:37PM +0200, Maarten Lankhorst wrote:
> Op 24-04-18 om 17:21 schreef Ville Syrjälä:
> > On Tue, Apr 24, 2018 at 01:36:32PM +0200, Maarten Lankhorst wrote:
> >> When calculating limits we want to be as pessimistic as possible,
> >> so we have to explicitly say whether we want to round up or down
> >> to accurately calculate whether we are below min_scale or above
> >> max_scale.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/drm_atomic_helper.c |  4 +--
> >>  drivers/gpu/drm/drm_rect.c          | 39 +++++++++++++++++++++--------
> >>  drivers/gpu/drm/i915/intel_sprite.c |  8 +++---
> >>  include/drm/drm_rect.h              |  8 +++---
> >>  4 files changed, 39 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> >> index 9cb2209f6fc8..7643202bfcf7 100644
> >> --- a/drivers/gpu/drm/drm_atomic_helper.c
> >> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> >> @@ -754,8 +754,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> >>  	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
> >>  
> >>  	/* Check scaling */
> >> -	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> >> -	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> >> +	hscale = drm_rect_calc_hscale(src, dst, min_scale, 1<<16, max_scale);
> >> +	vscale = drm_rect_calc_vscale(src, dst, min_scale, 1<<16, max_scale);
> >>  	if (hscale < 0 || vscale < 0) {
> >>  		DRM_DEBUG_KMS("Invalid scaling of plane\n");
> >>  		drm_rect_debug_print("src: ", &plane_state->src, true);
> >> diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
> >> index 9817c1445ba9..2093474c268c 100644
> >> --- a/drivers/gpu/drm/drm_rect.c
> >> +++ b/drivers/gpu/drm/drm_rect.c
> >> @@ -96,7 +96,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
> >>  }
> >>  EXPORT_SYMBOL(drm_rect_clip_scaled);
> >>  
> >> -static int drm_calc_scale(int src, int dst)
> >> +static int drm_calc_scale(int src, int dst, int scale_center)
> >>  {
> >>  	int scale = 0;
> >>  
> >> @@ -106,7 +106,10 @@ static int drm_calc_scale(int src, int dst)
> >>  	if (dst == 0)
> >>  		return 0;
> >>  
> >> -	scale = src / dst;
> >> +	if (DIV_ROUND_UP(dst, scale_center) > src)
> > That doesn't look right to me.
> >
> > How about just 'if (src > dst << 16)' ?
> >
> > I guess if we want to keep this code independent of the number of
> > fractional bits we could pass the 16 in from the caller(s). Not sure
> > there's much point in that though.
> pass a bool round_up to the calc_scale functions?

That would mean duplicating the "which way we round?" logic in all
the callers, which doesn't seem ideal. I'd probably just go with the
hardcoded 16 fractional bits assumption. We can always come up
with something else later if the need arises.

> >> +		return DIV_ROUND_UP(src, dst);
> >> +	else
> >> +		scale = src / dst;
> >>  
> >>  	return scale;
> >>  }
> >> @@ -116,21 +119,25 @@ static int drm_calc_scale(int src, int dst)
> >>   * @src: source window rectangle
> >>   * @dst: destination window rectangle
> >>   * @min_hscale: minimum allowed horizontal scaling factor
> >> + * @mid_hscale: mid point, below this point round down scaling, above round up.
> >>   * @max_hscale: maximum allowed horizontal scaling factor
> >>   *
> >>   * Calculate the horizontal scaling factor as
> >>   * (@src width) / (@dst width).
> >>   *
> >> + * If the scale is below @mid_hscale we round down, if above up. This will
> >> + * calculate the hscale with the most pessimistic limit calculation.
> >> + *
> >>   * RETURNS:
> >>   * The horizontal scaling factor, or errno of out of limits.
> >>   */
> >>  int drm_rect_calc_hscale(const struct drm_rect *src,
> >>  			 const struct drm_rect *dst,
> >> -			 int min_hscale, int max_hscale)
> >> +			 int min_hscale, int mid_hscale, int max_hscale)
> >>  {
> >>  	int src_w = drm_rect_width(src);
> >>  	int dst_w = drm_rect_width(dst);
> >> -	int hscale = drm_calc_scale(src_w, dst_w);
> >> +	int hscale = drm_calc_scale(src_w, dst_w, mid_hscale);
> >>  
> >>  	if (hscale < 0 || dst_w == 0)
> >>  		return hscale;
> >> @@ -147,21 +154,25 @@ EXPORT_SYMBOL(drm_rect_calc_hscale);
> >>   * @src: source window rectangle
> >>   * @dst: destination window rectangle
> >>   * @min_vscale: minimum allowed vertical scaling factor
> >> + * @mid_vscale: mid point, below this point round down scaling, above round up.
> >>   * @max_vscale: maximum allowed vertical scaling factor
> >>   *
> >>   * Calculate the vertical scaling factor as
> >>   * (@src height) / (@dst height).
> >>   *
> >> + * If the scale is below @mid_vscale we round down, if above up. This will
> >> + * calculate the vscale with the most pessimistic limit calculation.
> >> + *
> >>   * RETURNS:
> >>   * The vertical scaling factor, or errno of out of limits.
> >>   */
> >>  int drm_rect_calc_vscale(const struct drm_rect *src,
> >>  			 const struct drm_rect *dst,
> >> -			 int min_vscale, int max_vscale)
> >> +			 int min_vscale, int mid_vscale, int max_vscale)
> >>  {
> >>  	int src_h = drm_rect_height(src);
> >>  	int dst_h = drm_rect_height(dst);
> >> -	int vscale = drm_calc_scale(src_h, dst_h);
> >> +	int vscale = drm_calc_scale(src_h, dst_h, mid_vscale);
> >>  
> >>  	if (vscale < 0 || dst_h == 0)
> >>  		return vscale;
> >> @@ -178,6 +189,7 @@ EXPORT_SYMBOL(drm_rect_calc_vscale);
> >>   * @src: source window rectangle
> >>   * @dst: destination window rectangle
> >>   * @min_hscale: minimum allowed horizontal scaling factor
> >> + * @mid_hscale: mid point, below this point round down scaling, above round up.
> >>   * @max_hscale: maximum allowed horizontal scaling factor
> >>   *
> >>   * Calculate the horizontal scaling factor as
> >> @@ -189,16 +201,19 @@ EXPORT_SYMBOL(drm_rect_calc_vscale);
> >>   * If the calculated scaling factor is above @max_vscale,
> >>   * decrease the height of rectangle @src to compensate.
> >>   *
> >> + * If the scale is below @mid_hscale we round down, if above up. This will
> >> + * calculate the hscale with the most pessimistic limit calculation.
> >> + *
> >>   * RETURNS:
> >>   * The horizontal scaling factor.
> >>   */
> >>  int drm_rect_calc_hscale_relaxed(struct drm_rect *src,
> >>  				 struct drm_rect *dst,
> >> -				 int min_hscale, int max_hscale)
> >> +				 int min_hscale, int mid_hscale, int max_hscale)
> >>  {
> >>  	int src_w = drm_rect_width(src);
> >>  	int dst_w = drm_rect_width(dst);
> >> -	int hscale = drm_calc_scale(src_w, dst_w);
> >> +	int hscale = drm_calc_scale(src_w, dst_w, mid_hscale);
> >>  
> >>  	if (hscale < 0 || dst_w == 0)
> >>  		return hscale;
> >> @@ -228,6 +243,7 @@ EXPORT_SYMBOL(drm_rect_calc_hscale_relaxed);
> >>   * @src: source window rectangle
> >>   * @dst: destination window rectangle
> >>   * @min_vscale: minimum allowed vertical scaling factor
> >> + * @mid_vscale: mid point, below this point round down scaling, above round up.
> >>   * @max_vscale: maximum allowed vertical scaling factor
> >>   *
> >>   * Calculate the vertical scaling factor as
> >> @@ -239,16 +255,19 @@ EXPORT_SYMBOL(drm_rect_calc_hscale_relaxed);
> >>   * If the calculated scaling factor is above @max_vscale,
> >>   * decrease the height of rectangle @src to compensate.
> >>   *
> >> + * If the scale is below @mid_vscale we round down, if above up. This will
> >> + * calculate the vscale with the most pessimistic limit calculation.
> >> + *
> >>   * RETURNS:
> >>   * The vertical scaling factor.
> >>   */
> >>  int drm_rect_calc_vscale_relaxed(struct drm_rect *src,
> >>  				 struct drm_rect *dst,
> >> -				 int min_vscale, int max_vscale)
> >> +				 int min_vscale, int mid_vscale, int max_vscale)
> >>  {
> >>  	int src_h = drm_rect_height(src);
> >>  	int dst_h = drm_rect_height(dst);
> >> -	int vscale = drm_calc_scale(src_h, dst_h);
> >> +	int vscale = drm_calc_scale(src_h, mid_vscale, dst_h);
> >>  
> >>  	if (vscale < 0 || dst_h == 0)
> >>  		return vscale;
> >> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> >> index aa1dfaa692b9..f8f7c66fc3ac 100644
> >> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >> @@ -998,10 +998,10 @@ intel_check_sprite_plane(struct intel_plane *plane,
> >>  	drm_rect_rotate(src, fb->width << 16, fb->height << 16,
> >>  			state->base.rotation);
> >>  
> >> -	hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale);
> >> +	hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, 1 << 16, max_scale);
> >>  	BUG_ON(hscale < 0);
> >>  
> >> -	vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale);
> >> +	vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, 1 << 16, max_scale);
> >>  	BUG_ON(vscale < 0);
> >>  
> >>  	if (crtc_state->base.enable)
> >> @@ -1017,7 +1017,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
> >>  
> >>  	if (state->base.visible) {
> >>  		/* check again in case clipping clamped the results */
> >> -		hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> >> +		hscale = drm_rect_calc_hscale(src, dst, min_scale, 1 << 16, max_scale);
> >>  		if (hscale < 0) {
> >>  			DRM_DEBUG_KMS("Horizontal scaling factor out of limits\n");
> >>  			drm_rect_debug_print("src: ", src, true);
> >> @@ -1026,7 +1026,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
> >>  			return hscale;
> >>  		}
> >>  
> >> -		vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> >> +		vscale = drm_rect_calc_vscale(src, dst, min_scale, 1 << 16, max_scale);
> >>  		if (vscale < 0) {
> >>  			DRM_DEBUG_KMS("Vertical scaling factor out of limits\n");
> >>  			drm_rect_debug_print("src: ", src, true);
> >> diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
> >> index 44bc122b9ee0..dbb8631a9288 100644
> >> --- a/include/drm/drm_rect.h
> >> +++ b/include/drm/drm_rect.h
> >> @@ -179,16 +179,16 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
> >>  			  int hscale, int vscale);
> >>  int drm_rect_calc_hscale(const struct drm_rect *src,
> >>  			 const struct drm_rect *dst,
> >> -			 int min_hscale, int max_hscale);
> >> +			 int min_hscale, int mid_hscale, int max_hscale);
> >>  int drm_rect_calc_vscale(const struct drm_rect *src,
> >>  			 const struct drm_rect *dst,
> >> -			 int min_vscale, int max_vscale);
> >> +			 int min_vscale, int mid_vscale, int max_vscale);
> >>  int drm_rect_calc_hscale_relaxed(struct drm_rect *src,
> >>  				 struct drm_rect *dst,
> >> -				 int min_hscale, int max_hscale);
> >> +				 int min_hscale, int mid_hscale, int max_hscale);
> >>  int drm_rect_calc_vscale_relaxed(struct drm_rect *src,
> >>  				 struct drm_rect *dst,
> >> -				 int min_vscale, int max_vscale);
> >> +				 int min_vscale, int mid_vscale, int max_vscale);
> >>  void drm_rect_debug_print(const char *prefix,
> >>  			  const struct drm_rect *r, bool fixed_point);
> >>  void drm_rect_rotate(struct drm_rect *r,
> >> -- 
> >> 2.17.0
> 

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-04-24 15:51 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-24 11:36 [PATCH 1/2] drm/rect: Add midpoint to scale calculation functions Maarten Lankhorst
2018-04-24 11:36 ` [PATCH 2/2] drm/i915: Do not adjust scale when out of bounds Maarten Lankhorst
2018-04-24 15:34   ` Ville Syrjälä
2018-04-24 12:21 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/rect: Add midpoint to scale calculation functions Patchwork
2018-04-24 12:37 ` ✓ Fi.CI.BAT: success " Patchwork
2018-04-24 15:05 ` ✗ Fi.CI.IGT: failure " Patchwork
2018-04-24 15:21 ` [PATCH 1/2] " Ville Syrjälä
2018-04-24 15:11   ` Maarten Lankhorst
2018-04-24 15:51     ` Ville Syrjälä

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.