Re: [PATCH 03/11] drm: parse ycbcr 420 vdb block

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Regards

Shashank


On 5/8/2017 10:39 PM, Ville Syrjälä wrote:
On Mon, May 08, 2017 at 10:11:53PM +0530, Sharma, Shashank wrote:
Regards

Shashank


On 5/8/2017 9:54 PM, Ville Syrjälä wrote:
On Fri, Apr 07, 2017 at 07:39:20PM +0300, Shashank Sharma wrote:
From: Jose Abreu <jose.abreu@xxxxxxxxxxxx>

HDMI 2.0 spec adds support for ycbcr420 subsampled output.
CEA-861-F adds two new blocks in EDID, to provide information about
sink's support for ycbcr420 output.

These new blocks are:
- ycbcr420 video data (vdb) block: video modes which can be supported
    only in ycbcr420 output mode.
- ycbcr420 video capability data (vcb) block: video modes which can be
    support in ycbcr420 output mode also (along with RGB, YCBCR 444/422 etc)

This patch adds parsing and handling of ycbcr420-vdb in the DRM
layer.

This patch is a modified version of Jose's RFC patch:
https://patchwork.kernel.org/patch/9492327/
so the authorship is maintained.

Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx>
Signed-off-by: Jose Abreu <joabreu@xxxxxxxxxxxx>
Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
---
   drivers/gpu/drm/drm_edid.c  | 54 +++++++++++++++++++++++++++++++++++++++++++--
   drivers/gpu/drm/drm_modes.c | 10 +++++++--
   include/drm/drm_connector.h |  1 +
   include/uapi/drm/drm_mode.h |  6 +++++
   4 files changed, 67 insertions(+), 4 deletions(-)
<snip>
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 4eeda12..cef76b2 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -199,6 +199,7 @@ struct drm_display_info {
   #define DRM_COLOR_FORMAT_RGB444		(1<<0)
   #define DRM_COLOR_FORMAT_YCRCB444	(1<<1)
   #define DRM_COLOR_FORMAT_YCRCB422	(1<<2)
+#define DRM_COLOR_FORMAT_YCRCB420	(1<<2)
/**
   	 * @color_formats: HDMI Color formats, selects between RGB and YCrCb
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 8c67fc0..1e74d8e 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -84,6 +84,12 @@ extern "C" {
   #define  DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH	(6<<14)
   #define  DRM_MODE_FLAG_3D_TOP_AND_BOTTOM	(7<<14)
   #define  DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF	(8<<14)
+/*
+ * HDMI 2.0
+ */
+#define DRM_MODE_FLAG_420_MASK			(0x03<<23)
+#define  DRM_MODE_FLAG_420			(1<<23)
+#define  DRM_MODE_FLAG_420_ONLY			(1<<24)
Adding those would again break the uabi. We can't add new mode flags
without some kind of client cap.
But I think we agreed that new user
space visible mode flags aren't needed, and instad we can keep it all
internal?
Yep you are right, we had decided to keep it internal, and this whole
patch series is implemented in such a way only, to control everything
through the HDMI output property itself.
But may be I slightly misunderstood that we shouldn't add the flags bits
all together, and I added this flag to differentiate between YCBCR420
and notmal modes.
Can you please suggest me on:
- how to differentiate a YCBCR420 mode with normal mode ? I still need
to add a flag, but not expose it into uapi layer.
I guess we can just tack on a few new bools to the end of
drm_display_mode. And then when we get the mode passed in by the user
we'll have to check whether that mode matches any CEA mode and
then look up the correct YCbCr 4:2:0 mode for it.
seems good to me, I can add a is_ycbcr420 flag, and we need not to bother about converting it to drm_mode_modeinfo as we are keeping it internal.

Hmm. Actually, that probably means that it isn't sufficient to
actually store this information on the modes we have on the connector's
mode list, because that list has been filtered and so may not actually
have all the modes that were declared in the EDID.
I dint get this point, Why do you think its not sufficient ? Do we need to care about the modes which are getting rejected from the driver ? I guess they cant be applied anyways. Do you think we will miss some of the YCBCR modes due to mode filtering ?
So I'm thinking we
should perhaps make the bitmap parsed from the Y402CMDB index the
full CEA mode list. That we can just lookup the matching VIC for
the user provided mode and check whether the bit for that VIC
indicates 4:2:0 support. And maybe we can handle Y420VDB in exactly
the same way (ie. just a second bitmap). That would have the additional
nice feature that the maximum length of those bitmaps is well defined
(at most 256 VICs).

We can do this, but do we really need 2 bitmaps ? A YCBCR420 support is same whether its coming from VCB or VDB, we just need a ORing of these supports. Even in the current implementation, I have been using only the YCBCR420_MASK to identify support.

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux