[v2,1/4] drm/crtc: Add property for aspect ratio

Commit Message

[e-mail] May 26, 2014, 10:04 a.m. UTC

Added a property to enable user space to set aspect ratio.
This patch contains declaration of the property and code to create the
property.

v2: Thierry's review comments.
	- Made aspect ratio enum generic instead of HDMI/CEA specfic
	- Removed usage of temporary aspect_ratio variable

Signed-off-by: Vandana Kannan <[e-mail]>
Cc: Thierry Reding <[e-mail]>
Cc: Daniel Vetter <[e-mail]>
---
 drivers/gpu/drm/drm_crtc.c  | 27 +++++++++++++++++++++++++++
 include/drm/drm_crtc.h      |  2 ++
 include/uapi/drm/drm_mode.h |  5 +++++
 3 files changed, 34 insertions(+)

Comments

Thierry Reding June 5, 2014, 6:40 a.m. UTC | #1

On Mon, May 26, 2014 at 03:34:43PM +0530, Vandana Kannan wrote:
[...]
> @@ -1344,6 +1350,27 @@ int drm_mode_create_scaling_mode_property(struct drm_device *dev)
>  EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
>  
>  /**
> + * drm_mode_create_aspect_ratio_property - create aspect ratio property
> + * @dev: DRM device
> + *
> + * Called by a driver the first time it's needed, must be attached to desired
> + * connectors.
> + */
> +int drm_mode_create_aspect_ratio_property(struct drm_device *dev)
> +{
> +	if (dev->mode_config.aspect_ratio_property)
> +		return 0;
> +
> +	dev->mode_config.aspect_ratio_property =
> +		drm_property_create_enum(dev, 0, "aspect ratio",
> +				drm_aspect_ratio_enum_list,
> +				    ARRAY_SIZE(drm_aspect_ratio_enum_list));

Indentation here is funny. Given the long names involved here it's
difficult to make this look good, but at least the last parameter here
could be aligned with the second to last.

> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 5c1c31c..1149617 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -801,6 +801,7 @@ struct drm_mode_config {
>  
>  	/* Optional properties */
>  	struct drm_property *scaling_mode_property;
> +	struct drm_property *aspect_ratio_property;
>  	struct drm_property *dirty_info_property;

This seems rather randomly inserted into the list. I think either the
list should be sorted alphabetically or you should append new entries.
But that's somewhat bikesheddy, so feel free to ignore.

With my first comment addressed:

Reviewed-by: Thierry Reding <[e-mail]>

Patch

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 37a3e07..3085c34 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -139,6 +139,12 @@  static const struct drm_prop_enum_list drm_scaling_mode_enum_list[] =
 	{ DRM_MODE_SCALE_ASPECT, "Full aspect" },
 };
 
+static const struct drm_prop_enum_list drm_aspect_ratio_enum_list[] = {
+	{ DRM_MODE_PICTURE_ASPECT_NONE, "Automatic" },
+	{ DRM_MODE_PICTURE_ASPECT_4_3, "4:3" },
+	{ DRM_MODE_PICTURE_ASPECT_16_9, "16:9" },
+};
+
 /*
  * Non-global properties, but "required" for certain connectors.
  */
@@ -1344,6 +1350,27 @@  int drm_mode_create_scaling_mode_property(struct drm_device *dev)
 EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
 
 /**
+ * drm_mode_create_aspect_ratio_property - create aspect ratio property
+ * @dev: DRM device
+ *
+ * Called by a driver the first time it's needed, must be attached to desired
+ * connectors.
+ */
+int drm_mode_create_aspect_ratio_property(struct drm_device *dev)
+{
+	if (dev->mode_config.aspect_ratio_property)
+		return 0;
+
+	dev->mode_config.aspect_ratio_property =
+		drm_property_create_enum(dev, 0, "aspect ratio",
+				drm_aspect_ratio_enum_list,
+				    ARRAY_SIZE(drm_aspect_ratio_enum_list));
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property);
+
+/**
  * drm_mode_create_dirty_property - create dirty property
  * @dev: DRM device
  *
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 5c1c31c..1149617 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -801,6 +801,7 @@  struct drm_mode_config {
 
 	/* Optional properties */
 	struct drm_property *scaling_mode_property;
+	struct drm_property *aspect_ratio_property;
 	struct drm_property *dirty_info_property;
 
 	/* dumb ioctl parameters */
@@ -971,6 +972,7 @@  extern int drm_mode_create_dvi_i_properties(struct drm_device *dev);
 extern int drm_mode_create_tv_properties(struct drm_device *dev, int num_formats,
 				     char *formats[]);
 extern int drm_mode_create_scaling_mode_property(struct drm_device *dev);
+extern int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
 extern int drm_mode_create_dirty_info_property(struct drm_device *dev);
 extern const char *drm_get_encoder_name(const struct drm_encoder *encoder);
 
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index f104c26..943b377 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -88,6 +88,11 @@ 
 #define DRM_MODE_SCALE_CENTER		2 /* Centered, no scaling */
 #define DRM_MODE_SCALE_ASPECT		3 /* Full screen, preserve aspect */
 
+/* Picture aspect ratio options */
+#define DRM_MODE_PICTURE_ASPECT_NONE	0
+#define DRM_MODE_PICTURE_ASPECT_4_3	1
+#define DRM_MODE_PICTURE_ASPECT_16_9	2
+
 /* Dithering mode options */
 #define DRM_MODE_DITHERING_OFF	0
 #define DRM_MODE_DITHERING_ON	1