From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932620AbdEORNc (ORCPT ); Mon, 15 May 2017 13:13:32 -0400 Received: from bhuna.collabora.co.uk ([46.235.227.227]:60919 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932080AbdEORNb (ORCPT ); Mon, 15 May 2017 13:13:31 -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: Date: Mon, 15 May 2017 13:13:26 -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-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? > >> @@ -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< > -Emil > Done. Rob.