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

Emil Velikov emil.l.velikov at gmail.com
Wed Oct 19 11:29:49 UTC 2016


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


More information about the Intel-gfx-trybot mailing list