[PATCH v4 2/4] drm: Add aspect ratio parsing in DRM layer

[Sharma, Shashank]

Hello Emil, it’s a possible case. 
Thanks for this suggestion. 

I was preparing a patch for aspect_ratio property handling. 
I will send a follow up patch on this in the same series. 

Regards
Shashank
-----Original Message-----
From: Emil Velikov [mailto:emil.l.velikov at gmail.com] 
Sent: Wednesday, October 19, 2016 5:00 PM
To: Sharma, Shashank <shashank.sharma at intel.com>
Cc: Intel-gfx-trybot at lists.freedesktop.org; Daniel Vetter <daniel.vetter at ffwll.ch>
Subject: Re: [PATCH v4 2/4] drm: Add aspect ratio parsing in DRM layer

Hi Shashank,

I believe I may have spotted a small defect. Please don't read too much into it.

On 19 October 2016 at 11:56, Shashank Sharma <shashank.sharma at intel.com> wrote:

> +       switch (in->flags & DRM_MODE_FLAG_PIC_AR_MASK) {
> +       case DRM_MODE_FLAG_PIC_AR_4_3:
> +               out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_4_3;
> +               break;
> +       case DRM_MODE_FLAG_PIC_AR_16_9:
> +               out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_16_9;
> +               break;
> +       default:
Since flags is a bit mask, the user can set multiple aspect ratios
(DRM_MODE_FLAG_PIC_AR_16_9|DRM_MODE_FLAG_PIC_AR_4_3 or alike) in which case we'll silently ignore the mistake and set NONE.
Maybe it's worth doing s/default/DRM_MODE_FLAG_PIC_AR_NONE/ here and having the default case that returns -EINVAL or alike.

Just food for thought.
Emil