[PATCH v3 04/14] drm/edid: parse YCBCR 420 videomodes from EDID

Sharma, Shashank shashank.sharma at intel.com
Thu Jun 15 17:02:20 UTC 2017


> Yes, we should check for features, not for some standard version that
> implements some/all of those features. But not convinced we need any
> featur flags for the other things you listed. Aren't we already doing
> all those things anyway?

Yep, I realized you will come back with this point, while typing these 
examples :-).
Ok then, I will add a flag which sounds more like ycbcr_420_supported or 
so.

Regards
Shashank
On 6/15/2017 10:29 PM, Ville Syrjälä wrote:
> On Thu, Jun 15, 2017 at 10:18:40PM +0530, Sharma, Shashank wrote:
>> Regards
>>
>> Shashank
>>
>>
>> On 6/15/2017 9:42 PM, Ville Syrjälä wrote:
>>> On Thu, Jun 15, 2017 at 09:05:10PM +0530, Sharma, Shashank wrote:
>>>> Regards
>>>>
>>>> Shashank
>>>>
>>>>
>>>> On 6/15/2017 8:13 PM, Ville Syrjälä wrote:
>>>>> On Wed, Jun 14, 2017 at 11:17:35PM +0530, Shashank Sharma wrote:
>>>>>> HDMI 2.0 spec adds support for YCBCR420 sub-sampled output.
>>>>>> CEA-861-F adds two new blocks in EDID's CEA extension blocks,
>>>>>> to provide information about sink's YCBCR420 output capabilities.
>>>>>>
>>>>>> These blocks are:
>>>>>>
>>>>>> - YCBCR420vdb(YCBCR 420 video data block):
>>>>>> This block contains VICs of video modes, which can be sopported only
>>>>>> in YCBCR420 output mode (Not in RGB/YCBCR444/422. Its like a normal
>>>>>> SVD block, valid for YCBCR420 modes only.
>>>>>>
>>>>>> - YCBCR420cmdb(YCBCR 420 capability map data block):
>>>>>> This block gives information about video modes which can support
>>>>>> YCBCR420 output mode also (along with RGB,YCBCR444/422 etc) This
>>>>>> block contains a bitmap index of normal svd videomodes, which can
>>>>>> support YCBCR420 output too.
>>>>>> So if bit 0 from first vcb byte is set, first video mode in the svd
>>>>>> list can support YCBCR420 output too. Bit 1 means second video mode
>>>>>> from svd list can support YCBCR420 output too, and so on.
>>>>>>
>>>>>> This patch adds two bitmaps in display's hdmi_info structure, one each
>>>>>> for VCB and VDB modes. If the source is HDMI 2.0 capable, this patch
>>>>>> adds:
>>>>>> - VDB modes (YCBCR 420 only modes) in connector's mode list, also makes
>>>>>>      an entry in the vdb_bitmap per vic.
>>>>>> - VCB modes (YCBCR 420 also modes) only entry in the vcb_bitmap.
>>>>>>
>>>>>> Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
>>>>>> Cc: Jose Abreu <joabreu at synopsys.com>
>>>>>> Cc: Emil Velikov <emil.l.velikov at gmail.com>
>>>>>>
>>>>>> V2: Addressed
>>>>>>        Review comments from Emil:
>>>>>>        - Use 1ULL<<i instead of 1<<i to make sure the output is 64bit.
>>>>>>        - Use the suggested method for updating dbmap.
>>>>>>        - Add documentation for YCBCR420_vcb_map to fix kbuild warning.
>>>>>>
>>>>>>        Review comments from Ville:
>>>>>>        - Do not expose the YCBCR420 flags in uabi layer, keep it internal.
>>>>>>        - Save a map of YCBCR420 modes for future reference.
>>>>>>        - Check db length before trying to parse extended tag.
>>>>>>        - Add a warning if there are > 64 modes in capability map block.
>>>>>>        - Use y420cmdb in function names and macros while dealing with vcb
>>>>>>          to be aligned with spec.
>>>>>>        - Move the display information parsing block ahead of mode parsing
>>>>>>          blocks.
>>>>>>
>>>>>> V3: Addressed design/review comments from Ville
>>>>>>        - Do not add flags in video modes, else we have to expose them to user
>>>>>>        - There should not be a UABI change, and kernel should detect the
>>>>>>          choice of the output based on type of mode, and the bitmaps.
>>>>>>        - Use standard bitops from kernel bitmap header, instead of calculating
>>>>>>          bit positions manually.
>>>>>>
>>>>>> Signed-off-by: Shashank Sharma <shashank.sharma at intel.com>
>>>>>> ---
>>>>>>     drivers/gpu/drm/drm_edid.c  | 193 ++++++++++++++++++++++++++++++++++++++++++--
>>>>>>     include/drm/drm_connector.h |  23 ++++++
>>>>>>     2 files changed, 211 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>>>>>> index 6fd8a98..4953f87 100644
>>>>>> --- a/drivers/gpu/drm/drm_edid.c
>>>>>> +++ b/drivers/gpu/drm/drm_edid.c
>>>>>> @@ -2781,7 +2781,9 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
>>>>>>     #define VIDEO_BLOCK     0x02
>>>>>>     #define VENDOR_BLOCK    0x03
>>>>>>     #define SPEAKER_BLOCK	0x04
>>>>>> -#define VIDEO_CAPABILITY_BLOCK	0x07
>>>>>> +#define VIDEO_CAPABILITY_BLOCK 0x07
>>>>>> +#define VIDEO_DATA_BLOCK_420	0x0E
>>>>>> +#define VIDEO_CAP_BLOCK_Y420CMDB 0x0F
>>>>>>     #define EDID_BASIC_AUDIO	(1 << 6)
>>>>>>     #define EDID_CEA_YCRCB444	(1 << 5)
>>>>>>     #define EDID_CEA_YCRCB422	(1 << 4)
>>>>>> @@ -3143,15 +3145,103 @@ drm_display_mode_from_vic_index(struct drm_connector *connector,
>>>>>>     	return newmode;
>>>>>>     }
>>>>>>     
>>>>>> +/*
>>>>>> + * do_ycbcr_420_vdb_modes - Parse YCBCR 420 only modes
>>>>>> + * @connector: connector corresponding to the HDMI sink
>>>>>> + * @svds: start of the data block of CEA YCBCR 420 VDB
>>>>>> + * @len: length of the CEA YCBCR 420 VDB
>>>>>> + *
>>>>>> + * CEA-861-F has added a new video block called YCBCR 420 Video
>>>>>> + * Data Block (ycbcr 420 vdb). This block contains modes which
>>>>>> + * support YCBCR 420 HDMI output (only YCBCR 420). This function
>>>>>> + * parses the block and adds these modes into connector's mode list.
>>>>> Seems a bit verbose. Maybe something like:
>>>>> "Parse the CEA-861-F YCbCr 4:2:0 Video Data Block (Y420VDB)
>>>>> which contains modes which only support YCbCr 4:2:0 output."
>>>> Got it.
>>>>>> + */
>>>>>> +static int do_ycbcr_420_vdb_modes(struct drm_connector *connector,
>>>>>> +				   const u8 *svds, u8 svds_len)
>>>>> Probably we could s/ycbcr_420_vdb/y420vdb/ all over to keep things
>>>>> shorter and match the terminology in the spec. Same for y420cmdb.
>>>> Got it.
>>>>>> +{
>>>>>> +	int modes = 0, i;
>>>>>> +	struct drm_device *dev = connector->dev;
>>>>>> +	struct drm_display_mode *newmode;
>>>>> This variable can be moved into the loop scope.
>>>> Ok.
>>>>>> +	struct drm_display_info *info = &connector->display_info;
>>>>>> +	struct drm_hdmi_info *hdmi = &info->hdmi;
>>>>>> +
>>>>>> +	for (i = 0; i < svds_len; i++) {
>>>>>> +		u8 vic = svds[i] & 0x7f;
>>>>> What's the 0x7f here? Native bit stuff? I'm thinkign we probably want
>>>>> some kind of svd_to_vic() function to make sure everyone deals with
>>>>> this stuff correctly.
>>>> Current VICs are 1 < VIC < 108, so seven bits required to show a VIC.
>>>> This is inspired from
>>>> drm_mode_from_cea_vic_index().
>>>>>> +
>>>>>> +		newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]);
>>>>>> +		if (!newmode)
>>>>>> +			break;
>>>>>> +		/*
>>>>>> +		 * ycbcr420_vdb_modes is a fix position 128 bit bitmap.
>>>>>> +		 * Every bit here represents a VIC, from CEA-861-F list.
>>>>>> +		 * So if bit[n] is set, it indicates vic[n+1] supports
>>>>>> +		 * YCBCR420 output. Bit 0 is dummy, as VICs start at 1.
>>>>>> +		 * ycbcr420_vcb_modes[0] = |VIC=64 |VIC=63 |.....|VIC=2 |VIC=1 |
>>>>>> +		 * ycbcr420_vcb_modes[1] = |VIC=128|VIC=127|.....|VIC=66|VIC=65|
>>>>>> +		 */
>>>>> I think people can figure out how the standard bitmaps work without
>>>>> having to explain it with such detail. If you want to elaborate on the
>>>>> contents of the bitmap, then I think the comment should be next to
>>>>> where the bitmap is defined.
>>>> There is already similar explanation :), I will probably remove this.
>>>>>> +		bitmap_set(hdmi->ycbcr420_vdb_modes, vic, 1);
>>>>> __set_bit()
>>>> Any harm on bitmap_set ? I guess this is specially for bitmaps ...
>>>>>> +		drm_mode_probed_add(connector, newmode);
>>>>>> +		modes++;
>>>>>> +	}
>>>>>> +
>>>>>> +	if (modes > 0)
>>>>>> +		info->color_formats |= DRM_COLOR_FORMAT_YCRCB420;
>>>>>> +	return modes;
>>>>>> +}
>>>>>> +
>>>>>> +/*
>>>>>> + * drm_add_vcb_modes - Add a YCBCR 420 mode into bitmap
>>>>>> + * @connector: connector corresponding to the HDMI sink
>>>>>> + * @vic: CEA vic for the video mode to be added in the map
>>>>>> + *
>>>>>> + * Makes an entry for a videomode in the YCBCR 420 bitmap
>>>>>> + */
>>>>>> +static void
>>>>>> +drm_add_vcb_modes(struct drm_connector *connector, u8 vic)
>>>>> The "vcb" term doesn't seem to be in the spec. Should be "cmdb" maybe?
>>>>>
>>>>> Or maybe we want ycbcr420_vdb_modes and ycbcr420_y420vdb_modes just to
>>>>> make it clear to which VDB they relate with.
>>>> cmdb seems appropriate, which is aligned to the spec too, will do the
>>>> change.
>>>>>> +{
>>>>>> +	struct drm_hdmi_info *hdmi = &connector->display_info.hdmi;
>>>>>> +
>>>>>> +	/* VICs are 1 to 127(107 defined till CEA-861-F) */
>>>>>> +	vic &= 127;
>>>>>> +
>>>>>> +	/*
>>>>>> +	 * ycbcr420_vcb_modes is a fix position 128 bit bitmap.
>>>>>> +	 * Every bit here represents a VIC, from CEA-861-F list.
>>>>>> +	 * So if bit[n] is set, it indicates vic[n+1] supports
>>>>>> +	 * YCBCR420 output. Bit 0 is dummy, as VICs start at 1.
>>>>>> +	 * ycbcr420_vcb_modes[0] = |VIC=64 |VIC=63 |.....|VIC=2 |VIC=1 |
>>>>>> +	 * ycbcr420_vcb_modes[1] = |VIC=128|VIC=127|.....|VIC=66|VIC=65|
>>>>>> +	 * Difference between a vcb_mode and vdb_mode is that a vcb_mode
>>>>>> +	 * can support other HDMI outputs like RGB/YCBCR444/422 also
>>>>>> +	 * along with YCBCR 240, whereas a vdb_mode can only support YCBCR
>>>>>> +	 * 420 HDMI output.
>>>>>> +	 */
>>>>>> +	bitmap_set(hdmi->ycbcr420_vcb_modes, vic, 1);
>>>>>> +}
>>>>>> +
>>>>>>     static int
>>>>>>     do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len)
>>>>>>     {
>>>>>>     	int i, modes = 0;
>>>>>> +	struct drm_hdmi_info *hdmi = &connector->display_info.hdmi;
>>>>>>     
>>>>>>     	for (i = 0; i < len; i++) {
>>>>>>     		struct drm_display_mode *mode;
>>>>>>     		mode = drm_display_mode_from_vic_index(connector, db, len, i);
>>>>>>     		if (mode) {
>>>>>> +			/*
>>>>>> +			 * YCBCR420 capability block contains a bitmap which
>>>>>> +			 * gives the index of CEA modes from CEA VDB, which
>>>>>> +			 * can support YCBCR 420 sampling output also (apart
>>>>>> +			 * from RGB/YCBCR444 etc).
>>>>>> +			 * For example, if the bit 0 in bitmap is set,
>>>>>> +			 * first mode in VDB can support YCBCR420 output too.
>>>>>> +			 * Add YCBCR420 modes only if sink is HDMI 2.0 capable.
>>>>>> +			 */
>>>>>> +			if (connector->is_hdmi2_src &&
>>>>> Hmm. I wonder if need this here at all. I guess we do need something for
>>>>> the "420 only" modes, but not sure if it should be here or if we should
>>>>> reject them only during mode validation. The latter would be nice
>>>>> because it would then leave a message into the log that the mode was
>>>>> present but was rejected.
>>>> Seems like a good idea, the only benefit of doing this is, this is in
>>>> DRM layer, and will protect other drivers
>>>> from accidentally adding 420_vcb modes. Mode valid would be internal to
>>>> driver, and applicable in I915 layer.
>>> No, we should put into the probe helpers standard mode validation stuff.
>>> No driver specifics needed apart from setting the support_ycbcr420 or
>>> whatever flag.
>> Ok, got it.
>>>> So your wish is my command :)
>>>>> I'm thinking we should probably call this
>>>>> flag ycbcr420_allowed or something like that to match the other
>>>>> foo_allowed flags we have in the connector for mode validation.
>>>> I don't like that honestly. I am hoping that this flag would be used
>>>> further, to identify a HDMI 2.0 src over HDMI 1.4
>>> And what happens when when you have an otherwise HDMI 2.0 capable
>>> source that can't output YCbCr 4:2:0?
>> This makes equation difficult, but in this way, we might need flags for
>> all sub-features like
>> 4k at 60, scrambling, scdc, scdc_rr, ycbcr420. May be, I will add a comment
> Yes, we should check for features, not for some standard version that
> implements some/all of those features. But not convinced we need any
> featur flags for the other things you listed. Aren't we already doing
> all those things anyway?
>
>> that if you
>> enable this flag, this is going to happen or something ?
>>>> source, beyond YCBCR420 feature, and at that time, it might be more
>>>> suitable to have hdmi_2_src. Does it make sense ?
>>>>> So I think this could perhaps just be an uncoditional
>>>>> if (test_bit(i, ...))
>>>>> 	__set_bit(vic, ...);
>>>>>
>>>>>> +				test_bit(i, &hdmi->ycbcr420_vcb_map))
>>>>>> +				drm_add_vcb_modes(connector, db[i]);
>>>>>> +
>>>>>>     			drm_mode_probed_add(connector, mode);
>>>>>>     			modes++;
>>>>>>     		}
>>>>>> @@ -3427,6 +3517,12 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len,
>>>>>>     }
>>>>>>     
>>>>>>     static int
>>>>>> +cea_db_extended_tag(const u8 *db)
>>>>>> +{
>>>>>> +	return db[1];
>>>>>> +}
>>>>> Could you clean up the current extended tag usage as a separate
>>>>> prep patch? Would be nice to avoid the confusion of calling the
>>>>> "Use extended tag" tag VIDEO_CAPABILITY_BLOCK.
>>>> Sure, adding in my to-do list.
>>>>>> +
>>>>>> +static int
>>>>>>     cea_db_payload_len(const u8 *db)
>>>>>>     {
>>>>>>     	return db[0] & 0x1f;
>>>>>> @@ -3487,9 +3583,81 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db)
>>>>>>     	return oui == HDMI_FORUM_IEEE_OUI;
>>>>>>     }
>>>>>>     
>>>>>> +static bool cea_db_is_ycbcr_420_cmdb(const u8 *db)
>>>>>> +{
>>>>>> +	u8 len = cea_db_payload_len(db);
>>>>> 'len' seems like a fairly pointless variable since it's used exactly
>>>>> once.
>>>> Agree,
>>>>>> +
>>>>>> +	if (cea_db_tag(db) != VIDEO_CAPABILITY_BLOCK)
>>>>>> +		return false;
>>>>>> +
>>>>>> +	if (!len)
>>>>>> +		return false;
>>>>>> +
>>>>>> +	if (cea_db_extended_tag(db) != VIDEO_CAP_BLOCK_Y420CMDB)
>>>>>> +		return false;
>>>>>> +
>>>>>> +	return true;
>>>>>> +}
>>>>>> +
>>>>>> +static bool cea_db_is_ycbcr_420_vdb(const u8 *db)
>>>>>> +{
>>>>>> +	if (cea_db_tag(db) != VIDEO_CAPABILITY_BLOCK)
>>>>>> +		return false;
>>>>>> +
>>>>> Length check missing.
>>>> Oops, got it.
>>>>>> +	if (cea_db_extended_tag(db) != VIDEO_DATA_BLOCK_420)
>>>>>> +		return false;
>>>>>> +
>>>>>> +	return true;
>>>>>> +}
>>>>>> +
>>>>>>     #define for_each_cea_db(cea, i, start, end) \
>>>>>>     	for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1)
>>>>>>     
>>>>>> +static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector,
>>>>>> +				     const u8 *db)
>>>>>> +{
>>>>>> +	struct drm_display_info *info = &connector->display_info;
>>>>>> +	struct drm_hdmi_info *hdmi = &info->hdmi;
>>>>>> +	u8 map_len = cea_db_payload_len(db) - 1;
>>>>>> +	u8 count;
>>>>>> +	u64 map = 0;
>>>>>> +
>>>>>> +	if (!db)
>>>>>> +		return;
>>>>> Can't happen.
>>>> I assumed you were convince on my previous (lame) excuse of corrupt db :)
>>>> Will remove this check.
>>>>>> +
>>>>>> +	if (map_len == 0) {
>>>>>> +		/* All CEA modes support ycbcr420 sampling also.*/
>>>>>> +		hdmi->ycbcr420_vcb_map = U64_MAX;
>>>>>> +		info->color_formats |= DRM_COLOR_FORMAT_YCRCB420;
>>>>>> +		return;
>>>>>> +	}
>>>>>> +
>>>>>> +	/*
>>>>>> +	 * This map indicates which of the existing CEA block modes
>>>>>> +	 * from VDB can support YCBCR420 output too. So if bit=0 is
>>>>>> +	 * set, first mode from VDB can support YCBCR420 output too.
>>>>>> +	 * We will parse and keep this map, before parsing VDB itself
>>>>>> +	 * to avoid going through the same block again and again.
>>>>>> +	 *
>>>>>> +	 * Spec is not clear about max possible size of this block.
>>>>>> +	 * Clamping max bitmap block size at 8 bytes. Every byte can
>>>>>> +	 * address 8 CEA modes, in this way this map can address
>>>>>> +	 * 8*8 = first 64 SVDs.
>>>>>> +	 */
>>>>>> +	if (WARN_ON_ONCE(map_len > 8))
>>>>>> +		map_len = 8;
>>>>>> +
>>>>>> +	for (count = 0; count < map_len; count++)
>>>>>> +		map |= (u64)db[2 + count] << (8 * count);
>>>>>> +
>>>>>> +	if (map) {
>>>>>> +		DRM_DEBUG_KMS("Sink supports ycbcr 420\n");
>>>>> If we want to print something about the sinks color capabilities I think
>>>>> it should be in central place instead of being sprinkled all over the
>>>>> place.
>>>> Yes, seems like a good idea. I will remove this, and may be add a
>>>> separate patch to dump
>>>> sink capabilities.
>>>>>> +		info->color_formats |= DRM_COLOR_FORMAT_YCRCB420;
>>>>>> +	}
>>>>>> +
>>>>>> +	hdmi->ycbcr420_vcb_map = map;
>>>>>> +}
>>>>>> +
>>>>>>     static int
>>>>>>     add_cea_modes(struct drm_connector *connector, struct edid *edid)
>>>>>>     {
>>>>>> @@ -3512,10 +3680,17 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid)
>>>>>>     				video = db + 1;
>>>>>>     				video_len = dbl;
>>>>>>     				modes += do_cea_modes(connector, video, dbl);
>>>>>> -			}
>>>>>> -			else if (cea_db_is_hdmi_vsdb(db)) {
>>>>>> +			} else if (cea_db_is_hdmi_vsdb(db)) {
>>>>>>     				hdmi = db;
>>>>>>     				hdmi_len = dbl;
>>>>>> +			} else if (cea_db_is_ycbcr_420_vdb(db) &&
>>>>>> +					connector->is_hdmi2_src) {
>>>>> Broken indentation in a bunch of places.
>>>> Damn, I thought I am improving :(
>>>>>> +				const u8 *vdb420 = &db[2];
>>>>>> +
>>>>>> +				/* Add 4:2:0(only) modes present in EDID */
>>>>>> +				modes += do_ycbcr_420_vdb_modes(connector,
>>>>>> +								vdb420,
>>>>>> +								dbl - 1);
>>>>>>     			}
>>>>>>     		}
>>>>>>     	}
>>>>>> @@ -4196,6 +4371,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>>>>>>     			drm_parse_hdmi_vsdb_video(connector, db);
>>>>>>     		if (cea_db_is_hdmi_forum_vsdb(db))
>>>>>>     			drm_parse_hdmi_forum_vsdb(connector, db);
>>>>>> +		if (cea_db_is_ycbcr_420_cmdb(db))
>>>>>> +			drm_parse_y420cmdb_bitmap(connector, db);
>>>>>>     	}
>>>>>>     }
>>>>>>     
>>>>>> @@ -4430,6 +4607,13 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
>>>>>>     	quirks = edid_get_quirks(edid);
>>>>>>     
>>>>>>     	/*
>>>>>> +	 * CEA-861-F adds ycbcr capability map block, for HDMI 2.0 sinks.
>>>>>> +	 * To avoid multiple parsing of same block, lets get the sink info
>>>>>> +	 * before parsing CEA modes.
>>>>>> +	 */
>>>>>> +	drm_add_display_info(connector, edid);
>>>>>> +
>>>>>> +	/*
>>>>>>     	 * EDID spec says modes should be preferred in this order:
>>>>>>     	 * - preferred detailed mode
>>>>>>     	 * - other detailed modes from base block
>>>>>> @@ -4450,14 +4634,13 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
>>>>>>     	num_modes += add_cea_modes(connector, edid);
>>>>>>     	num_modes += add_alternate_cea_modes(connector, edid);
>>>>>>     	num_modes += add_displayid_detailed_modes(connector, edid);
>>>>>> +
>>>>>>     	if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)
>>>>>>     		num_modes += add_inferred_modes(connector, edid);
>>>>>>     
>>>>>>     	if (quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75))
>>>>>>     		edid_fixup_preferred(connector, quirks);
>>>>>>     
>>>>>> -	drm_add_display_info(connector, edid);
>>>>>> -
>>>>> I think this could be a separate patch, just in case it causes a
>>>>> regression.
>>>> Got it.
>>>>>>     	if (quirks & EDID_QUIRK_FORCE_6BPC)
>>>>>>     		connector->display_info.bpc = 6;
>>>>>>     
>>>>>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>>>>>> index 390319c..5a47c33 100644
>>>>>> --- a/include/drm/drm_connector.h
>>>>>> +++ b/include/drm/drm_connector.h
>>>>>> @@ -137,6 +137,28 @@ struct drm_scdc {
>>>>>>     struct drm_hdmi_info {
>>>>>>     	/** @scdc: sink's scdc support and capabilities */
>>>>>>     	struct drm_scdc scdc;
>>>>>> +
>>>>>> +	/**
>>>>>> +	 * @ycbcr420_vdb_modes: bitmap of modes which can support ycbcr420
>>>>>> +	 * output only (not normal RGB/YCBCR444/422 outputs). There are total
>>>>>> +	 * 107 VICs defined by CEA-861-F spec, so the size is 128 bits to map
>>>>>> +	 * upto 128 VICs;
>>>>>> +	 */
>>>>>> +	unsigned long ycbcr420_vdb_modes[BITS_TO_LONGS(128)];
>>>>>> +
>>>>>> +	/**
>>>>>> +	 * @ycbcr420_vcb_modes: bitmap of modes which can support ycbcr420
>>>>>> +	 * output also, along with normal HDMI outputs. There are total 107
>>>>>> +	 * VICs defined by CEA-861-F spec, so the size is 128 bits to map upto
>>>>>> +	 * 128 VICs;
>>>>>> +	 */
>>>>>> +	unsigned long ycbcr420_vcb_modes[BITS_TO_LONGS(128)];
>>>>>> +
>>>>>> +	/** @ycbcr420_vcb_map: bitmap of SVD index, to extraxt vcb modes */
>>>>>> +	unsigned long ycbcr420_vcb_map;
>>>>>> +
>>>>>> +	/** @ycbcr420_dc_modes: bitmap of deep color support index */
>>>>>> +	u8 ycbcr420_dc_modes;
>>>>> unused
>>>> Its used. We are parsing 420 deep color info and checking the it in
>>>> hdmi_12bpc_possible()
>>> Not in this patch.
>> Yes, actually that's in next patch (parse ycbcr_420_deep_color). I will
>> move it in next patch.
>>
>> - Shashank
>>>> Shashank
>>>>>>     };
>>>>>>     
>>>>>>     /**
>>>>>> @@ -200,6 +222,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<<3)
>>>>>>     
>>>>>>     	/**
>>>>>>     	 * @color_formats: HDMI Color formats, selects between RGB and YCrCb
>>>>>> -- 
>>>>>> 2.7.4



More information about the dri-devel mailing list