Regards
Shashank
On 8/4/2016 7:46 PM, Jose Abreu wrote:
Hi Sharma,
On 03-08-2016 16:47, Sharma, Shashank wrote:
Hello Joes,
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):
Thanks for addressing these patches. You are welcome to review the series.
Will it be possible for you, to comment in-line with the patch code, it's easier that way, and kind of conventional too.
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.
Please note that we can run the aspect ratio test cases (7-27 and similar) for CEA modes only. For the modes coming from DTDs and VSDBs can be with or without aspect ratio.
But the suggestion to initialize all the drm_modes with ASPECT_RATIO_NONE/DEFAULT is a good one, and it might help for these modes too.
I think Daniel also had similar suggestion last time, in a different context.
In our compliance equipment the modes are coming from DTD and
from the video datablock but the kernel is preferring the DTD
modes so we found a way of determining the picture aspect ratio
from the DTD section. We do not populate the field with
ASPECT_RATIO_NONE/DEFAULT, we populate it given the ratio of
width_mm and height_mm that is sent in the DTD (of course if
these values are zero we set aspect ratio to none). I think it
could be a nice addition to the EDID parsing.
I agree, I will come up with another patch, which does either of this:
- initialize all the DRM_MODE with aspect ratio default while creating
the new mode itself
- initialize all the DRM_MODES with aspect ratio default while parsing
the modes.
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.
I think while parsing the aspect ratio from libdrm to userspace (X), it's getting lost, and we have to fix your Xserver implementation.
We had added similar support in our HWComposer, and I guess it would be required for X and Wayland too, coz finally these guys issue the modeset.
So May be X server is not handling these flags, ignoring these flags, and sending the flagless modeset back to libdrm.
Do you mean Xserver, the X driver that I am using (which is
modesetting), the xrandr or all of them? I guess that if they
don't touch the flags field and return the mode exactly the same
as it was probed to DRM then it will work as expected.
I agree, in fact, if the userspace is not even touching the flag field,
we should get the aspect ratio information intact.
But if we are populating the aspect ratio information while reading the
modes from EDID, and passing the right flags to
userspace, but still we see the modeset doesn't contain the right flags,
means userspace is clearing the flags or modifying
the flags. So we should check:
- While creating a user_mode from kernel_mode, are we populating the
aspect ratio flags (If you have my patches, this shoud work)
- These modes are passed to userspace via a get_modes or get_connector
IOCTLs, so we should check these IOCTLS
- Once user space sends a modeset, does it set the flags properly ?
- while creating a kernel_mode from user_mode, during modeset, are we
preserving these flags ? (if you have my patches, this should work)
Regards
Shashank
Best regards,
Jose Miguel Abreu
Regards
Shashank
-----Original Message-----
From: Jose Abreu [mailto:Jose.Abreu@xxxxxxxxxxxx]
Sent: Wednesday, August 3, 2016 6:38 PM
To: Daniel Vetter <daniel@xxxxxxxx>; Sharma, Shashank <shashank.sharma@xxxxxxxxx>
Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; Vetter, Daniel <daniel.vetter@xxxxxxxxx>; Carlos Palminha <CARLOS.PALMINHA@xxxxxxxxxxxx>
Subject: Re: [PATCH 0/4]: Picture aspect ratio support in DRM layer
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Best regards,
Jose Miguel Abreu
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
