Re: [PATCH 04/11] drm: parse ycbcr420 vcb block

[Emil Velikov <emil.l.velikov@gmail.com>]

Hi Shashank,

On 7 April 2017 at 17:39, Shashank Sharma <shashank.sharma@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.


> +       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);


> --- 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.

Regards,
Emil
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx