[PATCH 1/3] drm/crtc: Add property for aspect ratio

[PATCH 1/3] drm/crtc: Add property for aspect ratio

[Vandana Kannan]

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

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_crtc.c | 31 +++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h     |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 37a3e07..84d359e 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[] = {
+	{ HDMI_PICTURE_ASPECT_NONE, "Automatic" },
+	{ HDMI_PICTURE_ASPECT_4_3, "4:3" },
+	{ HDMI_PICTURE_ASPECT_16_9, "16:9" },
+};
+
 /*
  * Non-global properties, but "required" for certain connectors.
  */
@@ -1344,6 +1350,31 @@ 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)
+{
+	struct drm_property *aspect_ratio;
+
+	if (dev->mode_config.aspect_ratio_property)
+		return 0;
+
+	aspect_ratio =
+		drm_property_create_enum(dev, 0, "aspect ratio",
+				drm_aspect_ratio_enum_list,
+				    ARRAY_SIZE(drm_aspect_ratio_enum_list));
+
+	dev->mode_config.aspect_ratio_property = aspect_ratio;
+
+	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);
 
-- 
1.9.3

[PATCH 2/3] drm/edid: Check for user aspect ratio input

[Vandana Kannan]

In case user has specified an input for aspect ratio through the property,
then the user space value for PAR would take preference over the value from
CEA mode list.

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_edid.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 7a4fd2e..05db619 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3657,8 +3657,13 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 
 	frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
 
-	/* Populate picture aspect ratio from CEA mode list */
-	if (frame->video_code > 0)
+	/* Populate picture aspect ratio from either CEA mode list or
+	 *  user input
+	*/
+	if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 ||
+		mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9)
+		frame->picture_aspect = mode->picture_aspect_ratio;
+	else if (frame->video_code > 0)
 		frame->picture_aspect = drm_get_cea_aspect_ratio(
 						frame->video_code);
 
-- 
1.9.3

Re: [PATCH 2/3] drm/edid: Check for user aspect ratio input

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1447 bytes --]

On Thu, May 22, 2014 at 04:50:49PM +0530, Vandana Kannan wrote:
> In case user has specified an input for aspect ratio through the property,
> then the user space value for PAR would take preference over the value from
> CEA mode list.
> 
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/drm_edid.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 7a4fd2e..05db619 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3657,8 +3657,13 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>  
>  	frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
>  
> -	/* Populate picture aspect ratio from CEA mode list */
> -	if (frame->video_code > 0)
> +	/* Populate picture aspect ratio from either CEA mode list or
> +	 *  user input
> +	*/

This comment is mangled, it should look like this:

	/*
	 * Populate...
	 */

And perhaps to clarify that user input takes precedence over CEA you
could list it first in the comment, like so for example:

	/*
	 * Populate picture aspect ratio from either user input (if specified)
	 * or from the CEA mode.
	 */

Also can you please resend patch 3/3 to dri-devel@lists.freedesktop.org
as well so we can see how this is used in a driver?

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/edid: Check for user aspect ratio input

[Vandana Kannan]

On May-22-2014 5:12 PM, Thierry Reding wrote:
> On Thu, May 22, 2014 at 04:50:49PM +0530, Vandana Kannan wrote:
>> In case user has specified an input for aspect ratio through the property,
>> then the user space value for PAR would take preference over the value from
>> CEA mode list.
>>
>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>> Cc: dri-devel@lists.freedesktop.org
>> ---
>>  drivers/gpu/drm/drm_edid.c | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 7a4fd2e..05db619 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -3657,8 +3657,13 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>>  
>>  	frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
>>  
>> -	/* Populate picture aspect ratio from CEA mode list */
>> -	if (frame->video_code > 0)
>> +	/* Populate picture aspect ratio from either CEA mode list or
>> +	 *  user input
>> +	*/
> 
> This comment is mangled, it should look like this:
> 
> 	/*
> 	 * Populate...
> 	 */
> 
> And perhaps to clarify that user input takes precedence over CEA you
> could list it first in the comment, like so for example:
> 
> 	/*
> 	 * Populate picture aspect ratio from either user input (if specified)
> 	 * or from the CEA mode.
> 	 */
>
Sure, I will modify this comment

> Also can you please resend patch 3/3 to dri-devel@lists.freedesktop.org
> as well so we can see how this is used in a driver?
> 
> Thierry
> 
I have resent this patch including dri-devel@lists.freedesktop.org

Thanks,
Vandana

[PATCH v2 2/4] drm/edid: Check for user aspect ratio input

[Vandana Kannan]

In case user has specified an input for aspect ratio through the property,
then the user space value for PAR would take preference over the value from
CEA mode list.

v2: Thierry's review comments.
	- Modified the comment "Populate..." as per review comments

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

---
 drivers/gpu/drm/drm_edid.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 7a4fd2e..2628dd1 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3657,8 +3657,13 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 
 	frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
 
-	/* Populate picture aspect ratio from CEA mode list */
-	if (frame->video_code > 0)
+	/* Populate picture aspect ratio from either user input (if specified)
+	 * or from the CEA mode list
+	 */
+	if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 ||
+		mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9)
+		frame->picture_aspect = mode->picture_aspect_ratio;
+	else if (frame->video_code > 0)
 		frame->picture_aspect = drm_get_cea_aspect_ratio(
 						frame->video_code);
 
-- 
1.9.3

Re: [PATCH v2 2/4] drm/edid: Check for user aspect ratio input

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1657 bytes --]

On Mon, May 26, 2014 at 03:37:42PM +0530, Vandana Kannan wrote:
> In case user has specified an input for aspect ratio through the property,
> then the user space value for PAR would take preference over the value from
> CEA mode list.
> 
> v2: Thierry's review comments.
> 	- Modified the comment "Populate..." as per review comments
> 
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> ---
>  drivers/gpu/drm/drm_edid.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 7a4fd2e..2628dd1 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3657,8 +3657,13 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>  
>  	frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
>  
> -	/* Populate picture aspect ratio from CEA mode list */
> -	if (frame->video_code > 0)
> +	/* Populate picture aspect ratio from either user input (if specified)
> +	 * or from the CEA mode list
> +	 */

Block comments should be of this form:

	/*
	 * Populate ...
	 * ... mode list.
	 */

> +	if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 ||
> +		mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9)
> +		frame->picture_aspect = mode->picture_aspect_ratio;
> +	else if (frame->video_code > 0)
>  		frame->picture_aspect = drm_get_cea_aspect_ratio(
>  						frame->video_code);

With the above fixed, this is:

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 2/4] drm/edid: Check for user aspect ratio input

[Vandana Kannan]

In case user has specified an input for aspect ratio through the property,
then the user space value for PAR would take preference over the value from
CEA mode list.

v2: Thierry's review comments.
	- Modified the comment "Populate..." as per review comments

v3: Thierry's review comments.
	- Modified the comment to block comment format.

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
---
 drivers/gpu/drm/drm_edid.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 7a4fd2e..e76c58c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3657,8 +3657,14 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 
 	frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
 
-	/* Populate picture aspect ratio from CEA mode list */
-	if (frame->video_code > 0)
+	/*
+	 * Populate picture aspect ratio from either
+	 * user input (if specified) or from the CEA mode list.
+	 */
+	if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 ||
+		mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9)
+		frame->picture_aspect = mode->picture_aspect_ratio;
+	else if (frame->video_code > 0)
 		frame->picture_aspect = drm_get_cea_aspect_ratio(
 						frame->video_code);
 
-- 
1.9.3

Re: [PATCH v3 2/4] drm/edid: Check for user aspect ratio input

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1395 bytes --]

On Thu, Jun 05, 2014 at 02:45:29PM +0530, Vandana Kannan wrote:
> In case user has specified an input for aspect ratio through the property,
> then the user space value for PAR would take preference over the value from
> CEA mode list.
> 
> v2: Thierry's review comments.
> 	- Modified the comment "Populate..." as per review comments
> 
> v3: Thierry's review comments.
> 	- Modified the comment to block comment format.
> 
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 7a4fd2e..e76c58c 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3657,8 +3657,14 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>  
>  	frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
>  
> -	/* Populate picture aspect ratio from CEA mode list */
> -	if (frame->video_code > 0)
> +	/*
> +	 * Populate picture aspect ratio from either
> +	 * user input (if specified) or from the CEA mode list.
> +	 */

This still looks slightly odd. Why not use the full available width on
the first line of the comment?

But either way:

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: Add aspect ratio property for HDMI

[Vandana Kannan]

Create and attach the drm property to set aspect ratio. If there is no user
specified value, then PAR_NONE/Automatic option is set by default. User can
select aspect ratio 4:3 or 16:9. The aspect ratio selected by user would
come into effect with a mode set.

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  5 +++++
 drivers/gpu/drm/i915/intel_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c | 31 +++++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 13495a4..8dc5f59 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2691,6 +2691,11 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val);
 #define INTEL_BROADCAST_RGB_FULL 1
 #define INTEL_BROADCAST_RGB_LIMITED 2
 
+/* Aspect ratio property */
+#define INTEL_ASPECT_RATIO_AUTO 0
+#define INTEL_ASPECT_RATIO_4_3  1
+#define INTEL_ASPECT_RATIO_16_9 2
+
 static inline uint32_t i915_vgacntrl_reg(struct drm_device *dev)
 {
 	if (HAS_PCH_SPLIT(dev))
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 287b89e..f9f19b6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -488,6 +488,7 @@ struct intel_hdmi {
 	bool has_audio;
 	enum hdmi_force_audio force_audio;
 	bool rgb_quant_range_selectable;
+	enum hdmi_picture_aspect aspect_ratio;
 	void (*write_infoframe)(struct drm_encoder *encoder,
 				enum hdmi_infoframe_type type,
 				const void *frame, ssize_t len);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 171d0dd..2c6aa76 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -367,6 +367,9 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
 	union hdmi_infoframe frame;
 	int ret;
 
+	/* Set user selected PAR to incoming mode's member */
+	adjusted_mode->picture_aspect_ratio = intel_hdmi->aspect_ratio;
+
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
 						       adjusted_mode);
 	if (ret < 0) {
@@ -1124,6 +1127,23 @@ intel_hdmi_set_property(struct drm_connector *connector,
 		goto done;
 	}
 
+	if (property == connector->dev->mode_config.aspect_ratio_property) {
+		switch (val) {
+		case INTEL_ASPECT_RATIO_AUTO:
+			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+			break;
+		case INTEL_ASPECT_RATIO_4_3:
+			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_4_3;
+			break;
+		case INTEL_ASPECT_RATIO_16_9:
+			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_16_9;
+			break;
+		default:
+			return -EINVAL;
+		}
+		goto done;
+	}
+
 	return -EINVAL;
 
 done:
@@ -1416,11 +1436,22 @@ static const struct drm_encoder_funcs intel_hdmi_enc_funcs = {
 };
 
 static void
+intel_attach_aspect_ratio_property(struct drm_connector *connector)
+{
+	drm_mode_create_aspect_ratio_property(connector->dev);
+	drm_object_attach_property(&connector->base,
+			connector->dev->mode_config.aspect_ratio_property,
+			HDMI_PICTURE_ASPECT_NONE);
+}
+
+static void
 intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *connector)
 {
 	intel_attach_force_audio_property(connector);
 	intel_attach_broadcast_rgb_property(connector);
 	intel_hdmi->color_range_auto = true;
+	intel_attach_aspect_ratio_property(connector);
+	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
 }
 
 void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
-- 
1.9.3

Re: [PATCH 3/3] drm/i915: Add aspect ratio property for HDMI

[Vandana Kannan]

Adding dri-devel..

On May-22-2014 4:50 PM, Kannan, Vandana wrote:
> Create and attach the drm property to set aspect ratio. If there is no user
> specified value, then PAR_NONE/Automatic option is set by default. User can
> select aspect ratio 4:3 or 16:9. The aspect ratio selected by user would
> come into effect with a mode set.
> 
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  5 +++++
>  drivers/gpu/drm/i915/intel_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_hdmi.c | 31 +++++++++++++++++++++++++++++++
>  3 files changed, 37 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 13495a4..8dc5f59 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2691,6 +2691,11 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val);
>  #define INTEL_BROADCAST_RGB_FULL 1
>  #define INTEL_BROADCAST_RGB_LIMITED 2
>  
> +/* Aspect ratio property */
> +#define INTEL_ASPECT_RATIO_AUTO 0
> +#define INTEL_ASPECT_RATIO_4_3  1
> +#define INTEL_ASPECT_RATIO_16_9 2
> +
>  static inline uint32_t i915_vgacntrl_reg(struct drm_device *dev)
>  {
>  	if (HAS_PCH_SPLIT(dev))
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 287b89e..f9f19b6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -488,6 +488,7 @@ struct intel_hdmi {
>  	bool has_audio;
>  	enum hdmi_force_audio force_audio;
>  	bool rgb_quant_range_selectable;
> +	enum hdmi_picture_aspect aspect_ratio;
>  	void (*write_infoframe)(struct drm_encoder *encoder,
>  				enum hdmi_infoframe_type type,
>  				const void *frame, ssize_t len);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 171d0dd..2c6aa76 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -367,6 +367,9 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
>  	union hdmi_infoframe frame;
>  	int ret;
>  
> +	/* Set user selected PAR to incoming mode's member */
> +	adjusted_mode->picture_aspect_ratio = intel_hdmi->aspect_ratio;
> +
>  	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
>  						       adjusted_mode);
>  	if (ret < 0) {
> @@ -1124,6 +1127,23 @@ intel_hdmi_set_property(struct drm_connector *connector,
>  		goto done;
>  	}
>  
> +	if (property == connector->dev->mode_config.aspect_ratio_property) {
> +		switch (val) {
> +		case INTEL_ASPECT_RATIO_AUTO:
> +			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> +			break;
> +		case INTEL_ASPECT_RATIO_4_3:
> +			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_4_3;
> +			break;
> +		case INTEL_ASPECT_RATIO_16_9:
> +			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_16_9;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		goto done;
> +	}
> +
>  	return -EINVAL;
>  
>  done:
> @@ -1416,11 +1436,22 @@ static const struct drm_encoder_funcs intel_hdmi_enc_funcs = {
>  };
>  
>  static void
> +intel_attach_aspect_ratio_property(struct drm_connector *connector)
> +{
> +	drm_mode_create_aspect_ratio_property(connector->dev);
> +	drm_object_attach_property(&connector->base,
> +			connector->dev->mode_config.aspect_ratio_property,
> +			HDMI_PICTURE_ASPECT_NONE);
> +}
> +
> +static void
>  intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *connector)
>  {
>  	intel_attach_force_audio_property(connector);
>  	intel_attach_broadcast_rgb_property(connector);
>  	intel_hdmi->color_range_auto = true;
> +	intel_attach_aspect_ratio_property(connector);
> +	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>  }
>  
>  void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> 

[PATCH v2 3/4] drm/i915: Add aspect ratio property for HDMI

[Vandana Kannan]

Create and attach the drm property to set aspect ratio. If there is no user
specified value, then PAR_NONE/Automatic option is set by default. User can
select aspect ratio 4:3 or 16:9. The aspect ratio selected by user would
come into effect with a mode set.

v2: Modifications made based on changes to aspect ratio enum list.

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

---
 drivers/gpu/drm/i915/intel_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c | 31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 287b89e..f9f19b6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -488,6 +488,7 @@ struct intel_hdmi {
 	bool has_audio;
 	enum hdmi_force_audio force_audio;
 	bool rgb_quant_range_selectable;
+	enum hdmi_picture_aspect aspect_ratio;
 	void (*write_infoframe)(struct drm_encoder *encoder,
 				enum hdmi_infoframe_type type,
 				const void *frame, ssize_t len);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 171d0dd..34ab689 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -367,6 +367,9 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
 	union hdmi_infoframe frame;
 	int ret;
 
+	/* Set user selected PAR to incoming mode's member */
+	adjusted_mode->picture_aspect_ratio = intel_hdmi->aspect_ratio;
+
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
 						       adjusted_mode);
 	if (ret < 0) {
@@ -1124,6 +1127,23 @@ intel_hdmi_set_property(struct drm_connector *connector,
 		goto done;
 	}
 
+	if (property == connector->dev->mode_config.aspect_ratio_property) {
+		switch (val) {
+		case DRM_MODE_PICTURE_ASPECT_NONE:
+			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+			break;
+		case DRM_MODE_PICTURE_ASPECT_4_3:
+			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_4_3;
+			break;
+		case DRM_MODE_PICTURE_ASPECT_16_9:
+			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_16_9;
+			break;
+		default:
+			return -EINVAL;
+		}
+		goto done;
+	}
+
 	return -EINVAL;
 
 done:
@@ -1416,11 +1436,22 @@ static const struct drm_encoder_funcs intel_hdmi_enc_funcs = {
 };
 
 static void
+intel_attach_aspect_ratio_property(struct drm_connector *connector)
+{
+	drm_mode_create_aspect_ratio_property(connector->dev);
+	drm_object_attach_property(&connector->base,
+			connector->dev->mode_config.aspect_ratio_property,
+			DRM_MODE_PICTURE_ASPECT_NONE);
+}
+
+static void
 intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *connector)
 {
 	intel_attach_force_audio_property(connector);
 	intel_attach_broadcast_rgb_property(connector);
 	intel_hdmi->color_range_auto = true;
+	intel_attach_aspect_ratio_property(connector);
+	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
 }
 
 void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
-- 
1.9.3

[PATCH v3 3/4] drm/i915: Add aspect ratio property for HDMI

[Vandana Kannan]

Create and attach the drm property to set aspect ratio. If there is no user
specified value, then PAR_NONE/Automatic option is set by default. User can
select aspect ratio 4:3 or 16:9. The aspect ratio selected by user would
come into effect with a mode set.

v2: Modified switch case to include aspect ratio enum changes

v3: Modified the patch according the change in the earlier patch to return
errno in case property creation fails. With this change, property will be
attached only if creation is successful

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

---
 drivers/gpu/drm/i915/intel_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c | 31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 287b89e..f9f19b6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -488,6 +488,7 @@ struct intel_hdmi {
 	bool has_audio;
 	enum hdmi_force_audio force_audio;
 	bool rgb_quant_range_selectable;
+	enum hdmi_picture_aspect aspect_ratio;
 	void (*write_infoframe)(struct drm_encoder *encoder,
 				enum hdmi_infoframe_type type,
 				const void *frame, ssize_t len);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 171d0dd..056409c 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -367,6 +367,9 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
 	union hdmi_infoframe frame;
 	int ret;
 
+	/* Set user selected PAR to incoming mode's member */
+	adjusted_mode->picture_aspect_ratio = intel_hdmi->aspect_ratio;
+
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
 						       adjusted_mode);
 	if (ret < 0) {
@@ -1124,6 +1127,23 @@ intel_hdmi_set_property(struct drm_connector *connector,
 		goto done;
 	}
 
+	if (property == connector->dev->mode_config.aspect_ratio_property) {
+		switch (val) {
+		case DRM_MODE_PICTURE_ASPECT_NONE:
+			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+			break;
+		case DRM_MODE_PICTURE_ASPECT_4_3:
+			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_4_3;
+			break;
+		case DRM_MODE_PICTURE_ASPECT_16_9:
+			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_16_9;
+			break;
+		default:
+			return -EINVAL;
+		}
+		goto done;
+	}
+
 	return -EINVAL;
 
 done:
@@ -1416,11 +1436,22 @@ static const struct drm_encoder_funcs intel_hdmi_enc_funcs = {
 };
 
 static void
+intel_attach_aspect_ratio_property(struct drm_connector *connector)
+{
+	if (!drm_mode_create_aspect_ratio_property(connector->dev))
+		drm_object_attach_property(&connector->base,
+			connector->dev->mode_config.aspect_ratio_property,
+			DRM_MODE_PICTURE_ASPECT_NONE);
+}
+
+static void
 intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *connector)
 {
 	intel_attach_force_audio_property(connector);
 	intel_attach_broadcast_rgb_property(connector);
 	intel_hdmi->color_range_auto = true;
+	intel_attach_aspect_ratio_property(connector);
+	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
 }
 
 void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
-- 
1.9.3

Re: [PATCH 1/3] drm/crtc: Add property for aspect ratio

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2391 bytes --]

On Thu, May 22, 2014 at 04:50:48PM +0530, Vandana Kannan wrote:
> Added a property to enable user space to set aspect ratio.
> This patch contains declaration of the property and code to create the
> property.
> 
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/drm_crtc.c | 31 +++++++++++++++++++++++++++++++
>  include/drm/drm_crtc.h     |  2 ++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 37a3e07..84d359e 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[] = {
> +	{ HDMI_PICTURE_ASPECT_NONE, "Automatic" },
> +	{ HDMI_PICTURE_ASPECT_4_3, "4:3" },
> +	{ HDMI_PICTURE_ASPECT_16_9, "16:9" },
> +};

This seems like it should be either an HDMI specific property, since it
uses values defined by HDMI/CEA. Alternatively we could introduce some
new generic enumeration and translate that to the HDMI/CEA equivalent in
the AVI infoframe helpers.

Doing so would allow us to add aspect ratios different from what HDMI or
CEA define.

>  /*
>   * Non-global properties, but "required" for certain connectors.
>   */
> @@ -1344,6 +1350,31 @@ 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)
> +{
> +	struct drm_property *aspect_ratio;
> +
> +	if (dev->mode_config.aspect_ratio_property)
> +		return 0;
> +
> +	aspect_ratio =
> +		drm_property_create_enum(dev, 0, "aspect ratio",
> +				drm_aspect_ratio_enum_list,
> +				    ARRAY_SIZE(drm_aspect_ratio_enum_list));
> +
> +	dev->mode_config.aspect_ratio_property = aspect_ratio;

I don't think you need the temporary aspect_ratio variable here. Can't
you directly assign the new property to .aspect_ratio_property?

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/crtc: Add property for aspect ratio

[Vandana Kannan]

On May-22-2014 5:08 PM, Thierry Reding wrote:
> On Thu, May 22, 2014 at 04:50:48PM +0530, Vandana Kannan wrote:
>> Added a property to enable user space to set aspect ratio.
>> This patch contains declaration of the property and code to create the
>> property.
>>
>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>> Cc: dri-devel@lists.freedesktop.org
>> ---
>>  drivers/gpu/drm/drm_crtc.c | 31 +++++++++++++++++++++++++++++++
>>  include/drm/drm_crtc.h     |  2 ++
>>  2 files changed, 33 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index 37a3e07..84d359e 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[] = {
>> +	{ HDMI_PICTURE_ASPECT_NONE, "Automatic" },
>> +	{ HDMI_PICTURE_ASPECT_4_3, "4:3" },
>> +	{ HDMI_PICTURE_ASPECT_16_9, "16:9" },
>> +};
> 
> This seems like it should be either an HDMI specific property, since it
> uses values defined by HDMI/CEA. Alternatively we could introduce some
> new generic enumeration and translate that to the HDMI/CEA equivalent in
> the AVI infoframe helpers.
> 
> Doing so would allow us to add aspect ratios different from what HDMI or
> CEA define.
> 
>>  /*
>>   * Non-global properties, but "required" for certain connectors.
>>   */
>> @@ -1344,6 +1350,31 @@ 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)
>> +{
>> +	struct drm_property *aspect_ratio;
>> +
>> +	if (dev->mode_config.aspect_ratio_property)
>> +		return 0;
>> +
>> +	aspect_ratio =
>> +		drm_property_create_enum(dev, 0, "aspect ratio",
>> +				drm_aspect_ratio_enum_list,
>> +				    ARRAY_SIZE(drm_aspect_ratio_enum_list));
>> +
>> +	dev->mode_config.aspect_ratio_property = aspect_ratio;
> 
> I don't think you need the temporary aspect_ratio variable here. Can't
> you directly assign the new property to .aspect_ratio_property?
> 
> Thierry
> 
Thanks for your inputs.
I will make the following changes and resend the patch..
- Make the enum generic and translate that to the HDMI/CEA equivalent
for AVI IF.
- Remove the temporary aspect_ratio variable.

Thanks,
Vandana

Re: [Intel-gfx] [PATCH 1/3] drm/crtc: Add property for aspect ratio

[Daniel Vetter]

On Thu, May 22, 2014 at 04:50:48PM +0530, Vandana Kannan wrote:
> Added a property to enable user space to set aspect ratio.
> This patch contains declaration of the property and code to create the
> property.
> 
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Cc: dri-devel@lists.freedesktop.org

Documentation update is missing. Also for such patch series I recommend to
post the entire patch series to dri-devel and intel-gfx. Otherwise people
on dri-devel don't see how the new code is used and so can't really review
it properly.
-Daniel

> ---
>  drivers/gpu/drm/drm_crtc.c | 31 +++++++++++++++++++++++++++++++
>  include/drm/drm_crtc.h     |  2 ++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 37a3e07..84d359e 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[] = {
> +	{ HDMI_PICTURE_ASPECT_NONE, "Automatic" },
> +	{ HDMI_PICTURE_ASPECT_4_3, "4:3" },
> +	{ HDMI_PICTURE_ASPECT_16_9, "16:9" },
> +};
> +
>  /*
>   * Non-global properties, but "required" for certain connectors.
>   */
> @@ -1344,6 +1350,31 @@ 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)
> +{
> +	struct drm_property *aspect_ratio;
> +
> +	if (dev->mode_config.aspect_ratio_property)
> +		return 0;
> +
> +	aspect_ratio =
> +		drm_property_create_enum(dev, 0, "aspect ratio",
> +				drm_aspect_ratio_enum_list,
> +				    ARRAY_SIZE(drm_aspect_ratio_enum_list));
> +
> +	dev->mode_config.aspect_ratio_property = aspect_ratio;
> +
> +	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);
>  
> -- 
> 1.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 1/3] drm/crtc: Add property for aspect ratio

[Vandana Kannan]

On May-22-2014 5:46 PM, Daniel Vetter wrote:
> On Thu, May 22, 2014 at 04:50:48PM +0530, Vandana Kannan wrote:
>> Added a property to enable user space to set aspect ratio.
>> This patch contains declaration of the property and code to create the
>> property.
>>
>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>> Cc: dri-devel@lists.freedesktop.org
> 
> Documentation update is missing. Also for such patch series I recommend to
> post the entire patch series to dri-devel and intel-gfx. Otherwise people
> on dri-devel don't see how the new code is used and so can't really review
> it properly.
> -Daniel
> 
Thanks for your inputs.
I will send the Documentation change along with the rest of the patches
(when I resend them).
Resent patch 3 adding dri-devel..

Thanks,
Vandana
>> ---
>>  drivers/gpu/drm/drm_crtc.c | 31 +++++++++++++++++++++++++++++++
>>  include/drm/drm_crtc.h     |  2 ++
>>  2 files changed, 33 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index 37a3e07..84d359e 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[] = {
>> +	{ HDMI_PICTURE_ASPECT_NONE, "Automatic" },
>> +	{ HDMI_PICTURE_ASPECT_4_3, "4:3" },
>> +	{ HDMI_PICTURE_ASPECT_16_9, "16:9" },
>> +};
>> +
>>  /*
>>   * Non-global properties, but "required" for certain connectors.
>>   */
>> @@ -1344,6 +1350,31 @@ 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)
>> +{
>> +	struct drm_property *aspect_ratio;
>> +
>> +	if (dev->mode_config.aspect_ratio_property)
>> +		return 0;
>> +
>> +	aspect_ratio =
>> +		drm_property_create_enum(dev, 0, "aspect ratio",
>> +				drm_aspect_ratio_enum_list,
>> +				    ARRAY_SIZE(drm_aspect_ratio_enum_list));
>> +
>> +	dev->mode_config.aspect_ratio_property = aspect_ratio;
>> +
>> +	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);
>>  
>> -- 
>> 1.9.3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

Re: [Intel-gfx] [PATCH 1/3] drm/crtc: Add property for aspect ratio

[Vandana Kannan]

On May-23-2014 4:18 PM, Vandana Kannan wrote:
> On May-22-2014 5:46 PM, Daniel Vetter wrote:
>> On Thu, May 22, 2014 at 04:50:48PM +0530, Vandana Kannan wrote:
>>> Added a property to enable user space to set aspect ratio.
>>> This patch contains declaration of the property and code to create the
>>> property.
>>>
>>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>>> Cc: dri-devel@lists.freedesktop.org
>>
>> Documentation update is missing. Also for such patch series I recommend to
>> post the entire patch series to dri-devel and intel-gfx. Otherwise people
>> on dri-devel don't see how the new code is used and so can't really review
>> it properly.
>> -Daniel
>>
> Thanks for your inputs.
> I will send the Documentation change along with the rest of the patches
> (when I resend them).
> Resent patch 3 adding dri-devel..
> 
> Thanks,
> Vandana

Hi Daniel,
For the Documentation update, should HTML table format be used in
drm.tmpl or is there some other method?
-Vandana
>>> ---
>>>  drivers/gpu/drm/drm_crtc.c | 31 +++++++++++++++++++++++++++++++
>>>  include/drm/drm_crtc.h     |  2 ++
>>>  2 files changed, 33 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>>> index 37a3e07..84d359e 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[] = {
>>> +	{ HDMI_PICTURE_ASPECT_NONE, "Automatic" },
>>> +	{ HDMI_PICTURE_ASPECT_4_3, "4:3" },
>>> +	{ HDMI_PICTURE_ASPECT_16_9, "16:9" },
>>> +};
>>> +
>>>  /*
>>>   * Non-global properties, but "required" for certain connectors.
>>>   */
>>> @@ -1344,6 +1350,31 @@ 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)
>>> +{
>>> +	struct drm_property *aspect_ratio;
>>> +
>>> +	if (dev->mode_config.aspect_ratio_property)
>>> +		return 0;
>>> +
>>> +	aspect_ratio =
>>> +		drm_property_create_enum(dev, 0, "aspect ratio",
>>> +				drm_aspect_ratio_enum_list,
>>> +				    ARRAY_SIZE(drm_aspect_ratio_enum_list));
>>> +
>>> +	dev->mode_config.aspect_ratio_property = aspect_ratio;
>>> +
>>> +	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);
>>>  
>>> -- 
>>> 1.9.3
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

Re: [Intel-gfx] [PATCH 1/3] drm/crtc: Add property for aspect ratio

[Daniel Vetter]

On Mon, May 26, 2014 at 11:00:41AM +0530, Vandana Kannan wrote:
> On May-23-2014 4:18 PM, Vandana Kannan wrote:
> > On May-22-2014 5:46 PM, Daniel Vetter wrote:
> >> On Thu, May 22, 2014 at 04:50:48PM +0530, Vandana Kannan wrote:
> >>> Added a property to enable user space to set aspect ratio.
> >>> This patch contains declaration of the property and code to create the
> >>> property.
> >>>
> >>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> >>> Cc: dri-devel@lists.freedesktop.org
> >>
> >> Documentation update is missing. Also for such patch series I recommend to
> >> post the entire patch series to dri-devel and intel-gfx. Otherwise people
> >> on dri-devel don't see how the new code is used and so can't really review
> >> it properly.
> >> -Daniel
> >>
> > Thanks for your inputs.
> > I will send the Documentation change along with the rest of the patches
> > (when I resend them).
> > Resent patch 3 adding dri-devel..
> > 
> > Thanks,
> > Vandana
> 
> Hi Daniel,
> For the Documentation update, should HTML table format be used in
> drm.tmpl or is there some other method?

Currently we only have the html table.
-Daniel

> -Vandana
> >>> ---
> >>>  drivers/gpu/drm/drm_crtc.c | 31 +++++++++++++++++++++++++++++++
> >>>  include/drm/drm_crtc.h     |  2 ++
> >>>  2 files changed, 33 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> >>> index 37a3e07..84d359e 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[] = {
> >>> +	{ HDMI_PICTURE_ASPECT_NONE, "Automatic" },
> >>> +	{ HDMI_PICTURE_ASPECT_4_3, "4:3" },
> >>> +	{ HDMI_PICTURE_ASPECT_16_9, "16:9" },
> >>> +};
> >>> +
> >>>  /*
> >>>   * Non-global properties, but "required" for certain connectors.
> >>>   */
> >>> @@ -1344,6 +1350,31 @@ 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)
> >>> +{
> >>> +	struct drm_property *aspect_ratio;
> >>> +
> >>> +	if (dev->mode_config.aspect_ratio_property)
> >>> +		return 0;
> >>> +
> >>> +	aspect_ratio =
> >>> +		drm_property_create_enum(dev, 0, "aspect ratio",
> >>> +				drm_aspect_ratio_enum_list,
> >>> +				    ARRAY_SIZE(drm_aspect_ratio_enum_list));
> >>> +
> >>> +	dev->mode_config.aspect_ratio_property = aspect_ratio;
> >>> +
> >>> +	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);
> >>>  
> >>> -- 
> >>> 1.9.3
> >>>
> >>> _______________________________________________
> >>> Intel-gfx mailing list
> >>> Intel-gfx@lists.freedesktop.org
> >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >>
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

[Vandana Kannan]

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 <vandana.kannan@intel.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 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(+)

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
-- 
1.9.3

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

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1654 bytes --]

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 <treding@nvidia.com>

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 1/4] drm/crtc: Add property for aspect ratio

[Vandana Kannan]

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

v3: Thierry's review comments.
	- Fixed indentation drm_mode_create_aspect_ratio_property()

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
---
 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(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 37a3e07..dcbc033 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
-- 
1.9.3

Re: [PATCH v3 1/4] drm/crtc: Add property for aspect ratio

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 898 bytes --]

On Thu, Jun 05, 2014 at 02:40:08PM +0530, Vandana Kannan wrote:
[...]
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
[...]
>  /**
> + * 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;

Sorry for not noticing this before: what if drm_propert_create_enum()
fails? Should that return an error? This function will currently
silently ignore failure to create the property.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 1/4] drm/crtc: Add property for aspect ratio

[Vandana Kannan]

On Jun-05-2014 2:58 PM, Thierry Reding wrote:
> On Thu, Jun 05, 2014 at 02:40:08PM +0530, Vandana Kannan wrote:
> [...]
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> [...]
>>  /**
>> + * 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;
> 
> Sorry for not noticing this before: what if drm_propert_create_enum()
> fails? Should that return an error? This function will currently
> silently ignore failure to create the property.
> 
> Thierry
> 
Yes..
I can
1. modify this to return the property (which would be NULL if create
fails) and have a NULL check at the calling end or
2. have a NULL check for the property at the calling end keeping the
existing implementation or
3. return a non-zero value in case of failure.

Please let me know your inputs on this..
- Vandana

Re: [PATCH v3 1/4] drm/crtc: Add property for aspect ratio

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1504 bytes --]

On Tue, Jun 10, 2014 at 02:00:37PM +0530, Vandana Kannan wrote:
> On Jun-05-2014 2:58 PM, Thierry Reding wrote:
> > On Thu, Jun 05, 2014 at 02:40:08PM +0530, Vandana Kannan wrote:
> > [...]
> >> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > [...]
> >>  /**
> >> + * 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;
> > 
> > Sorry for not noticing this before: what if drm_propert_create_enum()
> > fails? Should that return an error? This function will currently
> > silently ignore failure to create the property.
> > 
> > Thierry
> > 
> Yes..
> I can
> 1. modify this to return the property (which would be NULL if create
> fails) and have a NULL check at the calling end or
> 2. have a NULL check for the property at the calling end keeping the
> existing implementation or
> 3. return a non-zero value in case of failure.

I prefer 3. -ENOMEM sounds like a good candidate here.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v4 1/4] drm/crtc: Add property for aspect ratio

[Vandana Kannan]

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

v3: Thierry's review comments.
	- Fixed indentation

v4: Thierry's review comments.
	- Return ENOMEM when property creation fails

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
---
 drivers/gpu/drm/drm_crtc.c  | 33 +++++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h      |  2 ++
 include/uapi/drm/drm_mode.h |  5 +++++
 3 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 37a3e07..a745df3 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,33 @@ 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.
+ *
+ * Returns:
+ * Zero on success, errno on failure.
+ */
+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));
+
+	if (dev->mode_config.aspect_ratio_property == NULL)
+		return -ENOMEM;
+
+	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
-- 
1.9.3

Re: [PATCH v4 1/4] drm/crtc: Add property for aspect ratio

[Vandana Kannan]

Hi Thierry/Daniel,

Please help review this patch series on aspect ratio and let me know
your inputs..

1. http://lists.freedesktop.org/archives/dri-devel/2014-June/061576.html
- All review comments (from Thierry) addressed.
2. http://lists.freedesktop.org/archives/dri-devel/2014-June/061217.html
- R-b from Thierry
3. http://lists.freedesktop.org/archives/dri-devel/2014-June/061577.html
4. http://lists.freedesktop.org/archives/dri-devel/2014-June/061592.html
- R-b from Sagar

Thanks,
Vandana

On Jun-11-2014 10:46 AM, Kannan, Vandana wrote:
> 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
> 
> v3: Thierry's review comments.
> 	- Fixed indentation
> 
> v4: Thierry's review comments.
> 	- Return ENOMEM when property creation fails
> 
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> ---
>  drivers/gpu/drm/drm_crtc.c  | 33 +++++++++++++++++++++++++++++++++
>  include/drm/drm_crtc.h      |  2 ++
>  include/uapi/drm/drm_mode.h |  5 +++++
>  3 files changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 37a3e07..a745df3 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,33 @@ 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.
> + *
> + * Returns:
> + * Zero on success, errno on failure.
> + */
> +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));
> +
> +	if (dev->mode_config.aspect_ratio_property == NULL)
> +		return -ENOMEM;
> +
> +	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
> 

Re: [PATCH v4 1/4] drm/crtc: Add property for aspect ratio

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2158 bytes --]

On Wed, Jun 11, 2014 at 10:46:48AM +0530, Vandana Kannan wrote:
> 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
> 
> v3: Thierry's review comments.
> 	- Fixed indentation
> 
> v4: Thierry's review comments.
> 	- Return ENOMEM when property creation fails
> 
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> ---
>  drivers/gpu/drm/drm_crtc.c  | 33 +++++++++++++++++++++++++++++++++
>  include/drm/drm_crtc.h      |  2 ++
>  include/uapi/drm/drm_mode.h |  5 +++++
>  3 files changed, 40 insertions(+)

One nit below...

> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 37a3e07..a745df3 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,33 @@ 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.
> + *
> + * Returns:

According to Documentation/kernel-doc-nano-HOWTO.txt this section should
be named "Return:". But it seems that at least in DRM "Returns:" is used
much more often (89:31), so with or without this addressed:

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4 1/4] drm/crtc: Add property for aspect ratio

[Daniel Vetter]

On Mon, Jul 14, 2014 at 08:51:46AM +0200, Thierry Reding wrote:
> On Wed, Jun 11, 2014 at 10:46:48AM +0530, Vandana Kannan wrote:
> > 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
> > 
> > v3: Thierry's review comments.
> > 	- Fixed indentation
> > 
> > v4: Thierry's review comments.
> > 	- Return ENOMEM when property creation fails
> > 
> > Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > ---
> >  drivers/gpu/drm/drm_crtc.c  | 33 +++++++++++++++++++++++++++++++++
> >  include/drm/drm_crtc.h      |  2 ++
> >  include/uapi/drm/drm_mode.h |  5 +++++
> >  3 files changed, 40 insertions(+)
> 
> One nit below...
> 
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index 37a3e07..a745df3 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,33 @@ 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.
> > + *
> > + * Returns:
> 
> According to Documentation/kernel-doc-nano-HOWTO.txt this section should
> be named "Return:". But it seems that at least in DRM "Returns:" is used
> much more often (89:31), so with or without this addressed:
> 
> Reviewed-by: Thierry Reding <treding@nvidia.com>

I've pulled all 4 patches. Please double-check that I've picked up the
right ones since the series is a bit spread out.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH v4 1/4] drm/crtc: Add property for aspect ratio

[Vandana Kannan]

On Jul-15-2014 12:18 PM, Daniel Vetter wrote:
[...]

> 
> I've pulled all 4 patches. Please double-check that I've picked up the
> right ones since the series is a bit spread out.
> 
> Thanks, Daniel
> 

Hi Daniel,
I checked the 4 patches in -next-queued. They are the correct version.
Thanks,
Vandana

Re: [Intel-gfx] [PATCH 1/3] drm/crtc: Add property for aspect ratio

[Jesse Barnes]

Daniel, looks like this series has some r-bs; iirc this fixed some Asus
HDMI monitors too (and who knows how many TVs).

Jesse

On Thu, 22 May 2014 16:50:48 +0530
Vandana Kannan <vandana.kannan@intel.com> wrote:

> Added a property to enable user space to set aspect ratio.
> This patch contains declaration of the property and code to create the
> property.
> 
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/drm_crtc.c | 31 +++++++++++++++++++++++++++++++
>  include/drm/drm_crtc.h     |  2 ++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 37a3e07..84d359e 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[] = {
> +	{ HDMI_PICTURE_ASPECT_NONE, "Automatic" },
> +	{ HDMI_PICTURE_ASPECT_4_3, "4:3" },
> +	{ HDMI_PICTURE_ASPECT_16_9, "16:9" },
> +};
> +
>  /*
>   * Non-global properties, but "required" for certain connectors.
>   */
> @@ -1344,6 +1350,31 @@ 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)
> +{
> +	struct drm_property *aspect_ratio;
> +
> +	if (dev->mode_config.aspect_ratio_property)
> +		return 0;
> +
> +	aspect_ratio =
> +		drm_property_create_enum(dev, 0, "aspect ratio",
> +				drm_aspect_ratio_enum_list,
> +				    ARRAY_SIZE(drm_aspect_ratio_enum_list));
> +
> +	dev->mode_config.aspect_ratio_property = aspect_ratio;
> +
> +	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);
>  


-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 1/3] drm/crtc: Add property for aspect ratio

[Vandana Kannan]

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

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_crtc.c | 31 +++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h     |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d8b7099..6cd34ad 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[] = {
+	{ HDMI_PICTURE_ASPECT_NONE, "Automatic" },
+	{ HDMI_PICTURE_ASPECT_4_3, "4:3" },
+	{ HDMI_PICTURE_ASPECT_16_9, "16:9" },
+};
+
 /*
  * Non-global properties, but "required" for certain connectors.
  */
@@ -1334,6 +1340,31 @@ 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)
+{
+	struct drm_property *aspect_ratio;
+
+	if (dev->mode_config.aspect_ratio_property)
+		return 0;
+
+	aspect_ratio =
+		drm_property_create_enum(dev, 0, "aspect ratio",
+				drm_aspect_ratio_enum_list,
+				    ARRAY_SIZE(drm_aspect_ratio_enum_list));
+
+	dev->mode_config.aspect_ratio_property = aspect_ratio;
+
+	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 c061bb3..99bb6ed 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -797,6 +797,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 */
@@ -966,6 +967,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);
 
-- 
1.9.1

[PATCH 3/3] drm/i915: Add aspect ratio property for HDMI

[Vandana Kannan]

Create and attach the drm property to set aspect ratio. If there is no user
specified value, then PAR_NONE/Automatic option is set by default. User can
select aspect ratio 4:3 or 16:9. The aspect ratio selected by user would
come into effect with a mode set.

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  5 +++++
 drivers/gpu/drm/i915/intel_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c | 31 +++++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 55addaa..ae5525d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2583,6 +2583,11 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val);
 #define INTEL_BROADCAST_RGB_FULL 1
 #define INTEL_BROADCAST_RGB_LIMITED 2
 
+/* Aspect ratio property */
+#define INTEL_ASPECT_RATIO_AUTO 0
+#define INTEL_ASPECT_RATIO_4_3  1
+#define INTEL_ASPECT_RATIO_16_9 2
+
 static inline uint32_t i915_vgacntrl_reg(struct drm_device *dev)
 {
 	if (HAS_PCH_SPLIT(dev))
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 42762b7..256ea24 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -473,6 +473,7 @@ struct intel_hdmi {
 	bool has_audio;
 	enum hdmi_force_audio force_audio;
 	bool rgb_quant_range_selectable;
+	enum hdmi_picture_aspect aspect_ratio;
 	void (*write_infoframe)(struct drm_encoder *encoder,
 				enum hdmi_infoframe_type type,
 				const void *frame, ssize_t len);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index b0413e1..5fd2f20 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -367,6 +367,9 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
 	union hdmi_infoframe frame;
 	int ret;
 
+	/* Set user selected PAR to incoming mode's member */
+	adjusted_mode->picture_aspect_ratio = intel_hdmi->aspect_ratio;
+
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
 						       adjusted_mode);
 	if (ret < 0) {
@@ -1094,6 +1097,23 @@ intel_hdmi_set_property(struct drm_connector *connector,
 		goto done;
 	}
 
+	if (property == connector->dev->mode_config.aspect_ratio_property) {
+		switch (val) {
+		case INTEL_ASPECT_RATIO_AUTO:
+			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+			break;
+		case INTEL_ASPECT_RATIO_4_3:
+			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_4_3;
+			break;
+		case INTEL_ASPECT_RATIO_16_9:
+			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_16_9;
+			break;
+		default:
+			return -EINVAL;
+		}
+		goto done;
+	}
+
 	return -EINVAL;
 
 done:
@@ -1223,11 +1243,22 @@ static const struct drm_encoder_funcs intel_hdmi_enc_funcs = {
 };
 
 static void
+intel_attach_aspect_ratio_property(struct drm_connector *connector)
+{
+	drm_mode_create_aspect_ratio_property(connector->dev);
+	drm_object_attach_property(&connector->base,
+			connector->dev->mode_config.aspect_ratio_property,
+			HDMI_PICTURE_ASPECT_NONE);
+}
+
+static void
 intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *connector)
 {
 	intel_attach_force_audio_property(connector);
 	intel_attach_broadcast_rgb_property(connector);
 	intel_hdmi->color_range_auto = true;
+	intel_attach_aspect_ratio_property(connector);
+	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
 }
 
 void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
-- 
1.9.1

[PATCH 1/3] drm/crtc: Add property for aspect ratio

[Vandana Kannan]

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

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_crtc.c | 31 +++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h     |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d8b7099..6cd34ad 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[] = {
+	{ HDMI_PICTURE_ASPECT_NONE, "Automatic" },
+	{ HDMI_PICTURE_ASPECT_4_3, "4:3" },
+	{ HDMI_PICTURE_ASPECT_16_9, "16:9" },
+};
+
 /*
  * Non-global properties, but "required" for certain connectors.
  */
@@ -1334,6 +1340,31 @@ 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)
+{
+	struct drm_property *aspect_ratio;
+
+	if (dev->mode_config.aspect_ratio_property)
+		return 0;
+
+	aspect_ratio =
+		drm_property_create_enum(dev, 0, "aspect ratio",
+				drm_aspect_ratio_enum_list,
+				    ARRAY_SIZE(drm_aspect_ratio_enum_list));
+
+	dev->mode_config.aspect_ratio_property = aspect_ratio;
+
+	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 c061bb3..99bb6ed 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -797,6 +797,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 */
@@ -966,6 +967,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);
 
-- 
1.9.1

[PATCH 3/3] drm/i915: Add aspect ratio property for HDMI

[Vandana Kannan]

Create and attach the drm property to set aspect ratio. If there is no user
specified value, then PAR_NONE/Automatic option is set by default. User can
select aspect ratio 4:3 or 16:9. The aspect ratio selected by user would
come into effect with a mode set.

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  5 +++++
 drivers/gpu/drm/i915/intel_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c | 31 +++++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 55addaa..ae5525d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2583,6 +2583,11 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val);
 #define INTEL_BROADCAST_RGB_FULL 1
 #define INTEL_BROADCAST_RGB_LIMITED 2
 
+/* Aspect ratio property */
+#define INTEL_ASPECT_RATIO_AUTO 0
+#define INTEL_ASPECT_RATIO_4_3  1
+#define INTEL_ASPECT_RATIO_16_9 2
+
 static inline uint32_t i915_vgacntrl_reg(struct drm_device *dev)
 {
 	if (HAS_PCH_SPLIT(dev))
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 42762b7..256ea24 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -473,6 +473,7 @@ struct intel_hdmi {
 	bool has_audio;
 	enum hdmi_force_audio force_audio;
 	bool rgb_quant_range_selectable;
+	enum hdmi_picture_aspect aspect_ratio;
 	void (*write_infoframe)(struct drm_encoder *encoder,
 				enum hdmi_infoframe_type type,
 				const void *frame, ssize_t len);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index b0413e1..5fd2f20 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -367,6 +367,9 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
 	union hdmi_infoframe frame;
 	int ret;
 
+	/* Set user selected PAR to incoming mode's member */
+	adjusted_mode->picture_aspect_ratio = intel_hdmi->aspect_ratio;
+
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
 						       adjusted_mode);
 	if (ret < 0) {
@@ -1094,6 +1097,23 @@ intel_hdmi_set_property(struct drm_connector *connector,
 		goto done;
 	}
 
+	if (property == connector->dev->mode_config.aspect_ratio_property) {
+		switch (val) {
+		case INTEL_ASPECT_RATIO_AUTO:
+			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+			break;
+		case INTEL_ASPECT_RATIO_4_3:
+			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_4_3;
+			break;
+		case INTEL_ASPECT_RATIO_16_9:
+			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_16_9;
+			break;
+		default:
+			return -EINVAL;
+		}
+		goto done;
+	}
+
 	return -EINVAL;
 
 done:
@@ -1223,11 +1243,22 @@ static const struct drm_encoder_funcs intel_hdmi_enc_funcs = {
 };
 
 static void
+intel_attach_aspect_ratio_property(struct drm_connector *connector)
+{
+	drm_mode_create_aspect_ratio_property(connector->dev);
+	drm_object_attach_property(&connector->base,
+			connector->dev->mode_config.aspect_ratio_property,
+			HDMI_PICTURE_ASPECT_NONE);
+}
+
+static void
 intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *connector)
 {
 	intel_attach_force_audio_property(connector);
 	intel_attach_broadcast_rgb_property(connector);
 	intel_hdmi->color_range_auto = true;
+	intel_attach_aspect_ratio_property(connector);
+	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
 }
 
 void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
-- 
1.9.1