[Intel-gfx] [PATCH 04/11] drm: parse ycbcr420 vcb block

[Sharma, Shashank]

Regards

Shashank


On 4/8/2017 11:13 PM, Emil Velikov wrote:
> Hi Shashank,
>
> On 7 April 2017 at 17:39, Shashank Sharma <shashank.sharma at intel.com> wrote:
>
>> +       u64 hdmi_420_cap_map = connector->display_info.hdmi.ycbcr420_vcb_map;
>>
>>          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 such CEA modes in VDB, which can
>> +                        * support ycbcr420 sampling output also.
>> +                        * For example, if the bit 0 in bitmap is set,
>> +                        * first mode in VDB can support ycbcr420 output too.
>> +                        */
>> +                       if (hdmi_420_cap_map & (1 << i))
> Since map is u64 you really want to use something like (1ull << i)
> here. Otherwise you'll get a 32 bit value, regardless of i.
Thanks Emil, this was a good catch.
>
>> +       for (count = 0; count < map_len; count++)
>> +               map = (db[2 + count] << 8 * count) | map;
>> +
> The above issue is applicable here as well. With a small nitpick the
> whole thing will read
>
> map |= (u64)db[2 + count] << (8 * count);
Agree, thanks.
>
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -136,6 +136,7 @@ struct drm_scdc {
>>   struct drm_hdmi_info {
>>          /** @scdc: sink's scdc support and capabilities */
>>          struct drm_scdc scdc;
>> +       u64 ycbcr420_vcb_map;
> As pointed by the kbuild robot - you really want to document the field.
Agree, will handle this in next patch set.
> Regards,
> Emil