From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753165AbdEPPwH (ORCPT ); Tue, 16 May 2017 11:52:07 -0400 Received: from bhuna.collabora.co.uk ([46.235.227.227]:36039 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751009AbdEPPwE (ORCPT ); Tue, 16 May 2017 11:52:04 -0400 Subject: Re: [PATCH v1] drm: Add DRM_ROTATE_ and DRM_REFLECT_ defines to UAPI To: Emil Velikov References: <20170514172637.28937-1-robert.foss@collabora.com> Cc: ML dri-devel , Tomeu Vizoso , Daniel Vetter , =?UTF-8?Q?Kristian_H=c3=b8gsberg?= , "Linux-Kernel@Vger. Kernel. Org" From: Robert Foss Message-ID: <4f271759-074a-1cae-dcd9-20be7a3dbeca@collabora.com> Date: Tue, 16 May 2017 11:51:58 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017-05-16 06:58 AM, Emil Velikov wrote: > On 15 May 2017 at 18:13, Robert Foss wrote: >> >> >> On 2017-05-15 09:23 AM, Emil Velikov wrote: >>> >>> Hi Rob, >>> >>> On 14 May 2017 at 18:26, Robert Foss 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 >>>> --- >>>> 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 >>>> #include >>>> #include >>>> +#include >>>> #include >>>> #include >>>> >>>> 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_ 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. Done. Rob. > > -Emil >