[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