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

["Sharma, Shashank" <shashank.sharma@intel.com>]

Regards

Shashank


On 8/3/2016 11:14 PM, Sean Paul wrote:
> On Wed, Aug 3, 2016 at 6:56 AM, Shashank Sharma
> <shashank.sharma@intel.com> wrote:
>> Current DRM layer functions dont parse aspect ratio information
> s/dont/don't/
Got it.
>
>> while converting a user mode->kernel mode or viceversa. This
> s/viceversa/vice versa/
Got it.
>> causes modeset to pick mode with wrong aspect ratio, eventually
>> cauing failures in HDMI compliance test cases, due to wrong VIC.
> s/cauing/causing/
Ok.
> Sorry for the spelling nits. I originally just found this one, but
> since I was nitpicking one, I figured I should pick at them all.
Thanks for pointing them out. Will fix this.
>> This patch adds aspect ratio information in DRM's mode conversion
>> and mode comparision functions, to make sure kernel picks mode
>> with right aspect ratio (as per the VIC).
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Lin, Jia <lin.a.jia@intel.com>
>> Signed-off-by: Akashdeep Sharma <akashdeep.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/drm_modes.c | 31 +++++++++++++++++++++++++++++++
>>   1 file changed, 31 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
>> index fc5040a..e6029e0 100644
>> --- a/drivers/gpu/drm/drm_modes.c
>> +++ b/drivers/gpu/drm/drm_modes.c
>> @@ -969,6 +969,7 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1,
>>              mode1->vsync_end == mode2->vsync_end &&
>>              mode1->vtotal == mode2->vtotal &&
>>              mode1->vscan == mode2->vscan &&
>> +           mode1->picture_aspect_ratio == mode2->picture_aspect_ratio &&
>>              (mode1->flags & ~DRM_MODE_FLAG_3D_MASK) ==
>>               (mode2->flags & ~DRM_MODE_FLAG_3D_MASK))
>>                  return true;
>> @@ -1471,6 +1472,22 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out,
>>          out->vrefresh = in->vrefresh;
>>          out->flags = in->flags;
>>          out->type = in->type;
>> +       out->flags &= ~DRM_MODE_FLAG_PARMASK;
>> +
>> +       switch (in->picture_aspect_ratio) {
>> +       case HDMI_PICTURE_ASPECT_4_3:
>> +               out->flags |= DRM_MODE_FLAG_PAR4_3;
>> +               break;
>> +       case HDMI_PICTURE_ASPECT_16_9:
>> +               out->flags |= DRM_MODE_FLAG_PAR16_9;
>> +               break;
>> +       case HDMI_PICTURE_ASPECT_NONE:
>> +       case HDMI_PICTURE_ASPECT_RESERVED:
>> +       default:
> No need to specify HDMI_PICTURE_ASPECT_NONE &
> HDMI_PICTURE_ASPECT_RESERVED if you use default.
I agree on ASPECT_NONE, but I think we should keep ASPECT_RESERVED, to 
maintain the convention, do you think so ?
>> +               out->flags |= DRM_MODE_FLAG_PARNONE;
>> +               break;
>> +       }
>> +
>>          strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
>>          out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
>>   }
>> @@ -1516,6 +1533,20 @@ int drm_mode_convert_umode(struct drm_display_mode *out,
>>          strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
>>          out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
>>
>> +       /* Clearing picture aspect ratio bits from out flags */
>> +       out->flags &= ~DRM_MODE_FLAG_PARMASK;
>> +
>> +       switch (in->flags & DRM_MODE_FLAG_PARMASK) {
>> +       case DRM_MODE_FLAG_PAR4_3:
>> +               out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_4_3;
>> +               break;
>> +       case DRM_MODE_FLAG_PAR16_9:
>> +               out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_16_9;
>> +               break;
>> +       default:
>> +               out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> For safety,
>
> break;
Sure, Can be done.

Regards
Shashank
>
>> +       }
>> +
>>          out->status = drm_mode_validate_basic(out);
>>          if (out->status != MODE_OK)
>>                  goto out;
>> --
>> 1.9.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx