From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [Intel-gfx] [PATCH v4 1/4] drm/crtc: Add property for aspect ratio Date: Tue, 15 Jul 2014 08:48:42 +0200 Message-ID: <20140715064842.GM15237@phenom.ffwll.local> References: <20140610111557.GA10797@ulmo> <1402463808-31446-1-git-send-email-vandana.kannan@intel.com> <20140714065144.GE2081@ulmo> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wg0-f44.google.com (mail-wg0-f44.google.com [74.125.82.44]) by gabe.freedesktop.org (Postfix) with ESMTP id 1CD4A6E4EE for ; Mon, 14 Jul 2014 23:48:32 -0700 (PDT) Received: by mail-wg0-f44.google.com with SMTP id m15so4981937wgh.3 for ; Mon, 14 Jul 2014 23:48:31 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140714065144.GE2081@ulmo> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Thierry Reding Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org 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 > > Cc: Thierry Reding > > --- > > 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 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