[PATCH 2/3] drm/atomic: Pimp the debugs for scaling fails

From: Ville Syrjala <ville.syrjala@linux.intel.com>
To: dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Subject: [PATCH 2/3] drm/atomic: Pimp the debugs for scaling fails
Date: Thu, 19 Sep 2019 17:19:03 +0300	[thread overview]
Message-ID: <20190919141904.15840-2-ville.syrjala@linux.intel.com> (raw)
In-Reply-To: <20190919141904.15840-1-ville.syrjala@linux.intel.com>

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

When the calculate scaling factors are out of range let's
print out the calculated value and the min/max. Should make
life a bit less confusing when decoding failure logs.

I decided to print them in decimal rather than hex. Not sure
which people prefer but maybe this is less confusing to the
casual reader at least.

Also write out the magic 15625 constant we use in
the binary->decimal conversion as '1000000 >> (16-10)'
to make it more clear how it relates to the final '>> 10'.

Suggested-by: Sean Paul <sean@poorly.run>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 19 ++++++++++++++++---
 include/drm/drm_rect.h              | 29 ++++++++++++++++++++++-------
 2 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index c60db3bf2a83..9de39da54c48 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -760,6 +760,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
 					bool can_position,
 					bool can_update_disabled)
 {
+	struct drm_plane *plane = plane_state->plane;
 	struct drm_framebuffer *fb = plane_state->fb;
 	struct drm_rect *src = &plane_state->src;
 	struct drm_rect *dst = &plane_state->dst;
@@ -792,12 +793,24 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
 
 	/* Check scaling */
 	ret = drm_rect_calc_hscale(src, dst, min_scale, max_scale, &hscale);
-	ret |= drm_rect_calc_vscale(src, dst, min_scale, max_scale, &vscale);
 	if (ret) {
-		DRM_DEBUG_KMS("Invalid scaling of plane\n");
+		DRM_DEBUG_KMS("[PLANE:%d:%s] Invalid horizontal scaling factor: "
+			      DRM_FP_FMT " (limits: " DRM_FP_FMT " - " DRM_FP_FMT ")\n",
+			      plane->base.id, plane->name, DRM_FP_ARG(hscale),
+			      DRM_FP_ARG(min_scale), DRM_FP_ARG(max_scale));
 		drm_rect_debug_print("src: ", &plane_state->src, true);
 		drm_rect_debug_print("dst: ", &plane_state->dst, false);
-		return -ERANGE;
+		return ret;
+	}
+	ret = drm_rect_calc_vscale(src, dst, min_scale, max_scale, &vscale);
+	if (ret) {
+		DRM_DEBUG_KMS("[PLANE:%d:%s] Invalid vertical scaling factor: "
+			      DRM_FP_FMT " (limits: " DRM_FP_FMT " - " DRM_FP_FMT ")\n",
+			      plane->base.id, plane->name, DRM_FP_ARG(vscale),
+			      DRM_FP_ARG(min_scale), DRM_FP_ARG(max_scale));
+		drm_rect_debug_print("src: ", &plane_state->src, true);
+		drm_rect_debug_print("dst: ", &plane_state->dst, false);
+		return ret;
 	}
 
 	if (crtc_state->enable)
diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
index 34a88ba9ca7b..f700ef39f328 100644
--- a/include/drm/drm_rect.h
+++ b/include/drm/drm_rect.h
@@ -53,21 +53,36 @@ struct drm_rect {
 #define DRM_RECT_ARG(r) drm_rect_width(r), drm_rect_height(r), (r)->x1, (r)->y1
 
 /**
- * DRM_RECT_FP_FMT - printf string for &struct drm_rect in 16.16 fixed point
+ * DRM_FP_FMT - printf string for 16.16 binary fixed point
+ *
+ * Format a 16.16 binary fixed point number as .6 decimal fixed point.
+ */
+#define DRM_FP_FMT "%d.%06u"
+/**
+ * DRM_FP_ARG - printf arguments for 16.16 binary fixed point
+ * @f: 16.16 binary fixed point number
+ *
+ * This is useful for e.g. printing plane scaling factors, which are in 16.16
+ * binary fixed point.
+ */
+#define DRM_FP_ARG(f) (f) >> 16, (((f) & 0xffff) * (1000000 >> (16 - 10))) >> 10
+
+/**
+ * DRM_RECT_FP_FMT - printf string for &struct drm_rect in 16.16 binary fixed point
+ *
+ * Format a 16.16 binary fixed point rectangle as .6 decimal fixed point.
  */
 #define DRM_RECT_FP_FMT "%d.%06ux%d.%06u%+d.%06u%+d.%06u"
 /**
- * DRM_RECT_FP_ARG - printf arguments for &struct drm_rect in 16.16 fixed point
+ * DRM_RECT_FP_ARG - printf arguments for &struct drm_rect in 16.16 binary fixed point
  * @r: rectangle struct
  *
  * This is useful for e.g. printing plane source rectangles, which are in 16.16
- * fixed point.
+ * binary fixed point.
  */
 #define DRM_RECT_FP_ARG(r) \
-		drm_rect_width(r) >> 16, ((drm_rect_width(r) & 0xffff) * 15625) >> 10, \
-		drm_rect_height(r) >> 16, ((drm_rect_height(r) & 0xffff) * 15625) >> 10, \
-		(r)->x1 >> 16, (((r)->x1 & 0xffff) * 15625) >> 10, \
-		(r)->y1 >> 16, (((r)->y1 & 0xffff) * 15625) >> 10
+	DRM_FP_ARG(drm_rect_width(r)), DRM_FP_ARG(drm_rect_height(r)), \
+	DRM_FP_ARG((r)->x1), DRM_FP_ARG((r)->y1)
 
 /**
  * drm_rect_adjust_size - adjust the size of the rectangle
-- 
2.21.0

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