Re: [PATCH v1] drm: Add DRM_ROTATE_ and DRM_REFLECT_ defines to UAPI

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2017-05-15 09:23 AM, Emil Velikov wrote:
Hi Rob,

On 14 May 2017 at 18:26, Robert Foss <robert.foss@xxxxxxxxxxxxx> wrote:
Add DRM_ROTATE_ and DRM_REFLECT_ defines to the UAPI as a convenience.

Ideally the DRM_ROTATE_ and DRM_REFLECT_ property ids are looked up
through the atomic API, but realizing that userspace is likely to take
shortcuts and assume that the enum values are what is sent over the
wire.

As a result these defines are provided purely as a convenience to
userspace applications.

Signed-off-by: Robert Foss <robert.foss@xxxxxxxxxxxxx>
---
 drivers/gpu/drm/drm_rect.c |  1 +
 include/drm/drm_blend.h    | 18 ------------
 include/uapi/drm/drm.h     | 73 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
index bc5575960ebc..bdb27434bb10 100644
--- a/drivers/gpu/drm/drm_rect.c
+++ b/drivers/gpu/drm/drm_rect.c
@@ -24,6 +24,7 @@
 #include <linux/errno.h>
 #include <linux/export.h>
 #include <linux/kernel.h>
+#include <drm/drm.h>
 #include <drm/drmP.h>
 #include <drm/drm_rect.h>

diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
index 13221cf9b3eb..d149a63b893b 100644
--- a/include/drm/drm_blend.h
+++ b/include/drm/drm_blend.h
@@ -29,24 +29,6 @@
 struct drm_device;
 struct drm_atomic_state;

Since the defines are used here, move the above include to this file?

Done.


-/*
- * Rotation property bits. DRM_ROTATE_<degrees> rotates the image by the
- * specified amount in degrees in counter clockwise direction. DRM_REFLECT_X and
- * DRM_REFLECT_Y reflects the image along the specified axis prior to rotation
- *
- * WARNING: These defines are UABI since they're exposed in the rotation
- * property.
- */
-#define DRM_ROTATE_0   BIT(0)
-#define DRM_ROTATE_90  BIT(1)
-#define DRM_ROTATE_180 BIT(2)
-#define DRM_ROTATE_270 BIT(3)
-#define DRM_ROTATE_MASK (DRM_ROTATE_0   | DRM_ROTATE_90 | \
-                        DRM_ROTATE_180 | DRM_ROTATE_270)
-#define DRM_REFLECT_X  BIT(4)
-#define DRM_REFLECT_Y  BIT(5)
-#define DRM_REFLECT_MASK (DRM_REFLECT_X | DRM_REFLECT_Y)
-
 static inline bool drm_rotation_90_or_270(unsigned int rotation)
 {
        return rotation & (DRM_ROTATE_90 | DRM_ROTATE_270);
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 42d9f64ce416..d7140b0091bc 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h

drm_mode.h might be a better fit.

About this, I don't disagree, but other defines in drm_mode.h seem to be prefixed with DRM_MODE_ which this isn't which is why I didn't put it there.

Knowing this, do you still prefer these defines living in drm_mode.h?


@@ -697,6 +697,79 @@ struct drm_prime_handle {
        __s32 fd;
 };

+/** DRM_ROTATE_0
+ *
+ * Signals that a drm plane has been rotated 0 degrees.
+ *
+ * This define is provided as a convenience, looking up the property id
+ * using the name->prop id lookup is the preferred method.
Comments look quite good, small question:
Is it plane only? Haven't looked at the code, but gut feeling says no.

+ */
+#define DRM_ROTATE_0           BIT(0)
+
The UAPI headers do not use BIT(). Please use (1<<x) instead.

-Emil


Done.


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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux