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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx> 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

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux