On Tue, May 09, 2017 at 02:04:55PM +0530, Sharma, Shashank wrote: > 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 ? Yes, that was my worry. In general I don't think connector->modes should ever be used for mode validation or state computation. Eg. if fbdev handled the previus hotplug connector->modes will have been filtered based on the size of the fb_helper framebuffer, and then a new master might just blindly come in and blindly set a new mode without doing a full connector probe. > 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. We'll need to know if the mode is one of the "4:2:0 only" modes so that we'll automagically pick 4:2:0 whenever the user asks for this mode. And we'll need to know whether the mode is one of the "4:2:0 too" modes when the user asks for 4:2:0 for explicitly. The latter case of course needs a new property for that purpose. > Even in the current implementation, I have been using only the > YCBCR420_MASK to identify support. > > - Shashank -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
