Re: [PATCH 0/4]: Picture aspect ratio support in DRM layer

[Jose Abreu <Jose.Abreu@synopsys.com>]

Hi,


On 03-08-2016 12:48, Daniel Vetter wrote:
> On Wed, Aug 03, 2016 at 04:26:24PM +0530, Shashank Sharma wrote:
>> This patch series adds 4 patches.
>> - The first two patches add aspect ratio support in DRM layes
>> - Next two patches add new aspect ratios defined in CEA-861-F
>>   supported for HDMI 2.0 4k modes.
>>
>> Adding aspect ratio support in DRM layer:
>> - The CEA videmodes contain aspect ratio information, which we
>>   parse when we read the modes from EDID. But while transforming
>>   user_mode to kernel_mode or viceversa, DRM layer lose this
>>   information.
>> - HDMI compliance testing for CEA modes, expects the AVI info frames
>>   to contain exact VIC no for the 'video mode under test'. Now CEA
>>   modes have different VIC for same modes but different aspect ratio
>>   for example:
>> 	VIC 2 = 720x480@60 4:3 
>> 	VIC 3 = 720x480@60 16:9
>>   In this way, lack of aspect ratio information, can cause wrong VIC
>>   no in AVI IF, causing HDMI complaince test to fail.
>> - This patch set adds code, which embeds the aspect ratio information
>>   also in DRM video mode flags, and uses it while comparing two modes. 
>>
>> Adding new aspect ratios for HDMI 2.0
>> - CEA-861-F defines two new aspect ratios, to be used for 4k HDMI 2.0
>>   modes.
>> 	- 64:27
>> 	- 256:135
>> Last two patches in the series, adds code to handle these new
>> aspect ratios.
>>
>> Shashank Sharma (4):
>>   drm: add picture aspect ratio flags
>>   drm: Add aspect ratio parsing in DRM layer
>>   video: Add new aspect ratios for HDMI 2.0
>>   drm: Add and handle new aspect ratios in DRM layer
> Patch series seems to have 0 changelogs anywhere, but I'm pretty sure I've
> seen this before. Please try again and state what changed and why you are
> resubmitting this.
> -Daniel

I've also seen this before and I am using them in order to pass
HDMI compliance. Without these patches the compliance fails.
Still, I've made some changes which I can submit. I've some
comments to you (Shashank):

First, you add an if condition in
drm_mode_equal_no_clocks_no_stereo() (patch 2) which
unconditionally compares the aspect ratio. But I think that you
have to take into account that some modes handed by the user to
the DRM layer do not initialize this field so I think the best
solution would be to compare the aspect ratios only when the
field is populated (i.e. picture_aspect_ratio !=
HDMI_PICTURE_ASPECT_NONE).

Second, you are expecting that the picture aspect field is
correctly set in the HDMI parsing but, at least in the test
equipment that I am using, this field is not set by the DRM layer
because the mode is coming in the detailed timings section which
does not include a picture aspect field. In my implementation I
add a function which given the mode width and height (fields
->width_mm and ->height_mm of mode) computes the aspect ratio and
populates the field.

Third, I am facing some difficulties when using Xserver and
Xrandr. Using libdrm's modetest application everything works ok
but with xrandr the aspect ratio gets lost between the link DRM
-> Xserver -> DRM. I set the aspect ratio in the flags field when
passing the mode to user level (just like you do in patch 2) but
then when the mode is returned and delivered to the DRM driver
the picture aspect is not present. I think this is due to how
Xserver or xrandr sets the mode but I am not sure.

@Daniel, can you give some comments regarding this?

>>  drivers/gpu/drm/drm_modes.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>>  drivers/video/hdmi.c        |  4 ++++
>>  include/linux/hdmi.h        |  2 ++
>>  include/uapi/drm/drm_mode.h | 24 +++++++++++++++++++-----
>>  4 files changed, 68 insertions(+), 5 deletions(-)
>>
>> -- 
>> 1.9.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Best regards,
Jose Miguel Abreu
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel