Re: [Freedreno] [PATCH v3] drm: Add DRM_MODE_ROTATE_ and DRM_MODE_REFLECT_ to UAPI

[Robert Foss]

On 2017-05-18 05:49 AM, Ville Syrjälä wrote:

On Wed, May 17, 2017 at 09:39:11PM -0400, Robert Foss wrote:

+/*
+ * DRM_MODE_REFLECT_
+ *
+ * Signals that the contents of a drm plane has been reflected in
+ * the  axis.


Still vague.


Ack, I'll add some verbiage.



Also you didn't respond to my comment about the use of past tense.


Fixed in v4.


Rob.




+ *
+ * This define is provided as a convenience, looking up the property id
+ * using the name->prop id lookup is the preferred method.
+ */
+#define DRM_MODE_REFLECT_X  (1<<4)
+#define DRM_MODE_REFLECT_Y  (1<<5)
+
+/*
+ * DRM_MODE_REFLECT_MASK
+ *
+ * Bitmask used to look for drm plane reflections.
+ */
+#define DRM_MODE_REFLECT_MASK (\
+   DRM_MODE_REFLECT_X | \
+   DRM_MODE_REFLECT_Y)
+
+
  struct drm_mode_modeinfo {
__u32 clock;
__u16 hdisplay;
--
2.11.0.453.g787f75f05



___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH v3] drm: Add DRM_MODE_ROTATE_ and DRM_MODE_REFLECT_ to UAPI

[Liviu Dudau]

On Wed, May 17, 2017 at 09:39:11PM -0400, Robert Foss wrote:
> Add DRM_MODE_ROTATE_ and DRM_MODE_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 
> ---
> Changes since v2:
>  - Changed define prefix from DRM_MODE_PROP_ to DRM_MODE_
>  - Fix compilation errors
>  - Changed comment formatting
>  - Deduplicated comment lines
>  - Clarified DRM_MODE_PROP_REFLECT_ comment
> 
> Changes since v1:
>  - Moved defines from drm.h to drm_mode.h
>  - Changed define prefix from DRM_ to DRM_MODE_PROP_ 
>  - Updated uses of the defines to the new prefix
>  - Removed include from drm_rect.c
>  - Stopped using the BIT() macro 
> 
>  drivers/gpu/drm/arm/malidp_drv.h|  2 +-
>  drivers/gpu/drm/arm/malidp_planes.c | 18 -
>  drivers/gpu/drm/armada/armada_overlay.c |  2 +-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 20 +-
>  drivers/gpu/drm/drm_atomic.c|  2 +-
>  drivers/gpu/drm/drm_atomic_helper.c |  2 +-
>  drivers/gpu/drm/drm_blend.c | 43 ++---
>  drivers/gpu/drm/drm_fb_helper.c |  4 +-
>  drivers/gpu/drm/drm_plane_helper.c  |  2 +-
>  drivers/gpu/drm/drm_rect.c  | 36 +-
>  drivers/gpu/drm/i915/i915_debugfs.c | 14 +++
>  drivers/gpu/drm/i915/intel_atomic_plane.c   |  6 +--
>  drivers/gpu/drm/i915/intel_display.c| 50 
> -
>  drivers/gpu/drm/i915/intel_fbc.c|  2 +-
>  drivers/gpu/drm/i915/intel_fbdev.c  |  2 +-
>  drivers/gpu/drm/i915/intel_sprite.c | 20 +-
>  drivers/gpu/drm/imx/ipuv3-plane.c   |  2 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   | 30 +++
>  drivers/gpu/drm/nouveau/nv50_display.c  |  2 +-
>  drivers/gpu/drm/omapdrm/omap_drv.c  |  4 +-
>  drivers/gpu/drm/omapdrm/omap_fb.c   | 18 -
>  drivers/gpu/drm/omapdrm/omap_plane.c| 16 
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c |  4 +-
>  include/drm/drm_blend.h | 21 +--
>  include/uapi/drm/drm_mode.h | 49 +++-
>  25 files changed, 201 insertions(+), 170 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_drv.h 
> b/drivers/gpu/drm/arm/malidp_drv.h
> index 040311ffcaec..2e2033140efc 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.h
> +++ b/drivers/gpu/drm/arm/malidp_drv.h
> @@ -65,6 +65,6 @@ void malidp_de_planes_destroy(struct drm_device *drm);
>  int malidp_crtc_init(struct drm_device *drm);
>  
>  /* often used combination of rotational bits */
> -#define MALIDP_ROTATED_MASK  (DRM_ROTATE_90 | DRM_ROTATE_270)
> +#define MALIDP_ROTATED_MASK  (DRM_MODE_ROTATE_90 | DRM_MODE_ROTATE_270)
>  
>  #endif  /* __MALIDP_DRV_H__ */
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c 
> b/drivers/gpu/drm/arm/malidp_planes.c
> index 814fda23cead..063a8d2b0be3 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -80,7 +80,7 @@ static void malidp_plane_reset(struct drm_plane *plane)
>   state = kzalloc(sizeof(*state), GFP_KERNEL);
>   if (state) {
>   state->base.plane = plane;
> - state->base.rotation = DRM_ROTATE_0;
> + state->base.rotation = DRM_MODE_ROTATE_0;
>   plane->state = >base;
>   }
>  }
> @@ -221,7 +221,7 @@ static int malidp_de_plane_check(struct drm_plane *plane,
>   return ret;
>  
>   /* packed RGB888 / BGR888 can't be rotated or flipped */
> - if (state->rotation != DRM_ROTATE_0 &&
> + if (state->rotation != DRM_MODE_ROTATE_0 &&
>   (fb->format->format == DRM_FORMAT_RGB888 ||
>fb->format->format == DRM_FORMAT_BGR888))
>   return -EINVAL;
> @@ -315,12 +315,12 @@ static void malidp_de_plane_update(struct drm_plane 
> *plane,
>   val &= ~LAYER_ROT_MASK;
>  
>   /* setup the rotation and axis flip bits */
> - if (plane->state->rotation & DRM_ROTATE_MASK)
> - val |= ilog2(plane->state->rotation & DRM_ROTATE_MASK) <<
> + if (plane->state->rotation & DRM_MODE_ROTATE_MASK)
> + val |= ilog2(plane->state->rotation & DRM_MODE_ROTATE_MASK) <<
>  LAYER_ROT_OFFSET;
> - if (plane->state->rotation & DRM_REFLECT_X)
> + if (plane->state->rotation & DRM_MODE_REFLECT_X)
>   val |= LAYER_H_FLIP;
> - if (plane->state->rotation & DRM_REFLECT_Y)
> + if (plane->state->rotation & DRM_MODE_REFLECT_Y)
> 

Re: [Freedreno] [PATCH v3] drm: Add DRM_MODE_ROTATE_ and DRM_MODE_REFLECT_ to UAPI

[Ville Syrjälä]

On Wed, May 17, 2017 at 09:39:11PM -0400, Robert Foss wrote:
> +/*
> + * DRM_MODE_REFLECT_
> + *
> + * Signals that the contents of a drm plane has been reflected in
> + * the  axis.

Still vague.

Also you didn't respond to my comment about the use of past tense.

> + *
> + * This define is provided as a convenience, looking up the property id
> + * using the name->prop id lookup is the preferred method.
> + */
> +#define DRM_MODE_REFLECT_X  (1<<4)
> +#define DRM_MODE_REFLECT_Y  (1<<5)
> +
> +/*
> + * DRM_MODE_REFLECT_MASK
> + *
> + * Bitmask used to look for drm plane reflections.
> + */
> +#define DRM_MODE_REFLECT_MASK (\
> + DRM_MODE_REFLECT_X | \
> + DRM_MODE_REFLECT_Y)
> +
> +
>  struct drm_mode_modeinfo {
>   __u32 clock;
>   __u16 hdisplay;
> -- 
> 2.11.0.453.g787f75f05

-- 
Ville Syrjälä
Intel OTC
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH v3] drm: Add DRM_MODE_ROTATE_ and DRM_MODE_REFLECT_ to UAPI

[Emil Velikov]

Hi Rob,

On 18 May 2017 at 02:39, Robert Foss  wrote:
> Add DRM_MODE_ROTATE_ and DRM_MODE_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 
> ---
> Changes since v2:
>  - Changed define prefix from DRM_MODE_PROP_ to DRM_MODE_
>  - Fix compilation errors
>  - Changed comment formatting
>  - Deduplicated comment lines
>  - Clarified DRM_MODE_PROP_REFLECT_ comment
>
> Changes since v1:
>  - Moved defines from drm.h to drm_mode.h
>  - Changed define prefix from DRM_ to DRM_MODE_PROP_
>  - Updated uses of the defines to the new prefix
>  - Removed include from drm_rect.c
>  - Stopped using the BIT() macro
>
Reviewed-by: Emil Velikov 

-Emil
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno