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);
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);
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
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
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);
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
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);
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
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
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
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
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
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
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
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
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
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
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
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
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
- 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
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
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
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
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
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
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);
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Jesse Barnes -
Thierry Reding -
Vandana Kannan