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

[Sharma, Shashank]

Regards

Shashank


On 8/3/2016 11:14 PM, Sean Paul wrote:
> On Wed, Aug 3, 2016 at 6:56 AM, Shashank Sharma
>  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 
>> Signed-off-by: Lin, Jia 
>> Signed-off-by: Akashdeep Sharma 
>> ---
>>   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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Shashank Sharma]

Current DRM layer functions dont parse aspect ratio information
while converting a user mode->kernel mode or viceversa. This
causes modeset to pick mode with wrong aspect ratio, eventually
cauing failures in HDMI compliance test cases, due to wrong VIC.

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 
Signed-off-by: Lin, Jia 
Signed-off-by: Akashdeep Sharma 
---
 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:
+   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;
+   }
+
out->status = drm_mode_validate_basic(out);
if (out->status != MODE_OK)
goto out;
-- 
1.9.1

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

[Sean Paul]

On Wed, Aug 3, 2016 at 6:56 AM, Shashank Sharma
 wrote:
> Current DRM layer functions dont parse aspect ratio information

s/dont/don't/

> while converting a user mode->kernel mode or viceversa. This

s/viceversa/vice versa/

> causes modeset to pick mode with wrong aspect ratio, eventually
> cauing failures in HDMI compliance test cases, due to wrong VIC.

s/cauing/causing/

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.

>
> 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 
> Signed-off-by: Lin, Jia 
> Signed-off-by: Akashdeep Sharma 
> ---
>  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.

> +   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;

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