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@xxxxxxxxx> >> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx >> --- >> 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel