Re: [PATCH v4 02/37] drm/blend: Get a rotation name from it's bitfield
From: Louis Chauvet
Date: Thu Apr 23 2026 - 10:29:45 EST
On 4/23/26 12:35, Ville Syrjälä wrote:
On Thu, Apr 23, 2026 at 10:47:38AM +0200, Louis Chauvet wrote:
On 4/22/26 18:52, Ville Syrjälä wrote:
On Wed, Apr 22, 2026 at 06:47:59PM +0200, Louis Chauvet wrote:
Having the rotation/reflection name from its value can be useful for
debugging purpose. Extract the rotation property table and implement
drm_get_rotation_name.
Reviewed-by: José Expósito <jose.exposito@xxxxxxxxxx>
Reviewed-by: Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx>
Signed-off-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
---
drivers/gpu/drm/drm_blend.c | 35 ++++++++++++++++++++++++++---------
include/drm/drm_blend.h | 2 ++
2 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 1f3af27d2418..11d8e13caea3 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -256,6 +256,31 @@ int drm_plane_create_alpha_property(struct drm_plane *plane)
}
EXPORT_SYMBOL(drm_plane_create_alpha_property);
+static const struct drm_prop_enum_list rotation_props[] = {
+ { __builtin_ffs(DRM_MODE_ROTATE_0) - 1, "rotate-0" },
+ { __builtin_ffs(DRM_MODE_ROTATE_90) - 1, "rotate-90" },
+ { __builtin_ffs(DRM_MODE_ROTATE_180) - 1, "rotate-180" },
+ { __builtin_ffs(DRM_MODE_ROTATE_270) - 1, "rotate-270" },
+ { __builtin_ffs(DRM_MODE_REFLECT_X) - 1, "reflect-x" },
+ { __builtin_ffs(DRM_MODE_REFLECT_Y) - 1, "reflect-y" },
+};
+
+/**
+ * drm_get_rotation_name - Return the name of a rotation
+ * @rotation: The rotation mask (DRM_MODE_ROTATE_* | DRM_MODE_REFLECT_*)
+ *
+ * Returns: the name of the rotation type (unknown) if rotation is not
+ * a known rotation/reflection
+ */
+const char *drm_get_rotation_name(unsigned int rotation)
+{
+ if (rotation < ARRAY_SIZE(rotation_props))
+ return rotation_props[rotation].name;
The value is a bitmask. This does not work.
That true, the documentation is not clear.
Take a look at patch 15 [1] for the usage. Is it better if I change the
documentation to:
drm_get_rotation_name - Returns the name of a rotation/reflection
bitmask (with only one bit set).
@rotation: Bitmask with a single bit set.
Name of the rotation/reflection, or "(unknown)" if invalid.
[1]:https://lore.kernel.org/all/20260422-vkms-all-config-v4-15-dbb52e9aadc3@xxxxxxxxxxx/
For that kind of local thing I don't think it should be called
drm_get_rotation_name(). Also your docs seem to disagree with the
implementation.
I didn't remembered well, yes, this implementation takes the *index* of the bitmask. So documentation is:
drm_get_rotation_name - Returns the name of a rotation/reflection
@rotation: Bit index of the requested rotation (0 = DRM_MODE_ROTATE_0, 1 = DRM_MODE_ROTATE_90, ...)
Returns: Name of the rotation/reflection, or "(unknown)" if invalid.
But this does sound like a useful thing to have, eg. in
drm_atomic_plane_print_state().
I agree this can be a common implementation. Do you think the following implementation can be the "common implementation"?
static const char* drm_get_rotation_name(int rotation_index) {
return rotation_props[rotation_index].name
}
void drm_get_rotation_str(int rotation, char *buf, int buflen) {
u32 offset = 0;
while (offset < ARRAY_SIZE(rotation_props))
if (1 << offset && rotation)
snprintf(buf,buflen,"%s ",drm_get_rotation_name(offset));
}
An other solution can be format macro (inspiration from IGT, I don't know if this is accepted in the kernel):
#define DRM_ROTATION_FMT "%s %s %s %s %s %s"
#define DRM_ROTATION_FMT_ARGS(rot) \
rot & 0x1 << 0 ? drm_get_rotation_name(0) : "", \
rot & 0x1 << 1 ? drm_get_rotation_name(1) : "", \
rot & 0x1 << 2 ? drm_get_rotation_name(2) : "", \
rot & 0x1 << 3 ? drm_get_rotation_name(3) : "", \
rot & 0x1 << 4 ? drm_get_rotation_name(4) : "", \
rot & 0x1 << 5 ? drm_get_rotation_name(5) : ""
We already have at least one implementation of this in
i915/display/intel_display_debugfs.c, although with a bit more
terse format (and with somewhat weird "FLIP" terminology) to
make it not eat so much screen real estate.
I think it is better to have a consistent naming and use the enumeration name everywhere, but if you think intel is better, I can switch.
Thanks,
Louis Chauvet