linux-kernel - Re: [PATCH v1] drm: Add DRM_ROTATE_ and DRM_REFLECT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACvgo51nztw-uQg3YrsXz=HfCKctZ3my3Y578FWJjW+1jOvjXw@mail.gmail.com>
Date:   Tue, 16 May 2017 11:58:53 +0100
From:   Emil Velikov <emil.l.velikov@...il.com>
To:     Robert Foss <robert.foss@...labora.com>
Cc:     ML dri-devel <dri-devel@...ts.freedesktop.org>,
        Tomeu Vizoso <tomeu.vizoso@...labora.com>,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        Kristian Høgsberg <hoegsberg@...il.com>,
        "Linux-Kernel@...r. Kernel. Org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1] drm: Add DRM_ROTATE_ and DRM_REFLECT_ defines to UAPI

On 15 May 2017 at 18:13, Robert Foss <robert.foss@...labora.com> wrote:
>
>
> On 2017-05-15 09:23 AM, Emil Velikov wrote:
>>
>> Hi Rob,
>>
>> On 14 May 2017 at 18:26, Robert Foss <robert.foss@...labora.com> 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@...labora.com>
>>> ---
>>>  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?
>
Feel free to give them a DRM_MODE_ prefix or something else - say
DRM_MODE_PROP_.

AFAICT drm_mode.h deals with KMS specifics and the prop_id mentioned
in the documentation is already there, so it would make sense to add
it there.

-Emil