public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm: parse hf-vsdb
@ 2016-11-29 14:54 Shashank Sharma
  2016-11-29 18:23 ` ✗ Fi.CI.BAT: failure for " Patchwork
  2016-12-05 16:59 ` [PATCH] " Thierry Reding
  0 siblings, 2 replies; 4+ messages in thread
From: Shashank Sharma @ 2016-11-29 14:54 UTC (permalink / raw)
  To: dri-devel, intel-gfx; +Cc: airlied, daniel.vetter

HDMI 2.0 / CEA-861-F specs define a new CEA extension data block,
called hdmi-forum vendor specific data block (HF-VSDB). This block
contains information about sink's support for HDMI 2.0 compliant
features. These features are:
    - Deep color YUV 420 support and BPC
    - 3D flags for
        - OSD Displarity
        - Dual view signaling
        - independent view signaling
    - SCDC support
    - Max TMDS char rate
    - Scrambling support

This patch adds a parser function for this block, and add flags to
indicate support for new features, in drm_display_info structure.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/drm_edid.c  | 73 +++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_connector.h | 48 +++++++++++++++++++++++++++++
 include/drm/drm_edid.h      |  5 ++++
 include/linux/hdmi.h        |  1 +
 4 files changed, 127 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 336be31..b18bfe0 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3223,6 +3223,27 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure,
 	return 0;
 }
 
+static bool cea_db_is_hf_vsdb(const u8 *db)
+{
+	u8 len;
+	int hfvsdb_id;
+
+	if (cea_db_tag(db) != VENDOR_BLOCK)
+		return false;
+
+	len = cea_db_payload_len(db);
+	if (len < 7 || len > 31)
+		return false;
+
+	/* version should be 1 */
+	if (db[4]  != 1)
+		return false;
+
+	hfvsdb_id = db[1] | (db[2] << 8) | (db[3] << 16);
+
+	return hfvsdb_id == HDMI_IEEE_OUI_HFVSDB;
+}
+
 static bool cea_db_is_hdmi_vsdb(const u8 *db)
 {
 	int hdmi_id;
@@ -3767,6 +3788,56 @@ bool drm_rgb_quant_range_selectable(struct edid *edid)
 }
 EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
 
+static void drm_parse_yuv420_deep_color_info(struct drm_connector *connector,
+						const u8 *db)
+{
+	struct drm_display_info *info = &connector->display_info;
+
+	if (db[7] & DRM_EDID_YUV420_DC_48)
+		info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_48;
+	if (db[7] & DRM_EDID_YUV420_DC_36)
+		info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_36;
+	if (db[7] & DRM_EDID_YUV420_DC_30)
+		info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_30;
+
+	if (!info->edid_yuv420_dc_modes) {
+		DRM_DEBUG("%s: No YUV 420 deep color support in sink.\n",
+			  connector->name);
+		return;
+	}
+}
+
+static void
+drm_parse_hf_vsdb(struct drm_connector *connector, const u8 *db)
+{
+	struct drm_display_info *info = &connector->display_info;
+
+	if (db[5]) {
+		/*
+		 * If the sink supplies max tmds char rate in db,
+		 * the actual max tmds rate = db[5] * 5Mhz.
+		 */
+		info->max_tmds_clock = db[5] * 5000;
+		DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
+		info->max_tmds_clock);
+	}
+
+	if (db[6] & DRM_HFVSDB_SCDC_SUPPORT)
+		info->scdc_supported = true;
+	if (db[6] & DRM_HFVSDB_SCDC_RR_CAP)
+		info->scdc_rr_cap = true;
+	if (db[6] & DRM_HFVSDB_SCRAMBLING)
+		info->scrambling = true;
+	if (db[6] & DRM_HFVSDB_INDEPENDENT_VIEW)
+		info->independent_view_3d = true;
+	if (db[6] & DRM_HFVSDB_DUAL_VIEW)
+		info->dual_view_3d = true;
+	if (db[6] & DRM_HFVSDB_3D_OSD)
+		info->osd_disparity_3d = true;
+
+	drm_parse_yuv420_deep_color_info(connector, db);
+}
+
 static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
 					   const u8 *hdmi)
 {
@@ -3881,6 +3952,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
 
 		if (cea_db_is_hdmi_vsdb(db))
 			drm_parse_hdmi_vsdb_video(connector, db);
+		if (cea_db_is_hf_vsdb(db))
+			drm_parse_hf_vsdb(connector, db);
 	}
 }
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 34f9741..d011dd5 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -167,6 +167,46 @@ struct drm_display_info {
 	 */
 	u32 bus_flags;
 
+#define DRM_HFVSDB_SCDC_SUPPORT	(1<<7)
+#define DRM_HFVSDB_SCDC_RR_CAP		(1<<6)
+#define DRM_HFVSDB_SCRAMBLING		(1<<3)
+#define DRM_HFVSDB_INDEPENDENT_VIEW	(1<<2)
+#define DRM_HFVSDB_DUAL_VIEW		(1<<1)
+#define DRM_HFVSDB_3D_OSD		(1<<0)
+
+	/**
+	 * @scdc_supported: Sink supports SCDC functionality.
+	 */
+	bool scdc_supported;
+
+	/**
+	 * @scdc_rr_cap: Sink has SCDC read request capability.
+	 */
+	bool scdc_rr_cap;
+
+	/**
+	 * @scrambling: Sync supports scrambling for <=340 Mcsc TMDS
+	 * char rates. Above 340 Mcsc rates, scrambling is always reqd.
+	 */
+	bool scrambling;
+
+	/**
+	 * @independent_view_3d: Sink supports 3d independent view signaling
+	 * in HF-VSIF.
+	 */
+	bool independent_view_3d;
+
+	/**
+	 * @dual_view_3d: Sink supports 3d dual view signaling in HF-VSIF.
+	 */
+	bool dual_view_3d;
+
+	/**
+	 * @osd_disparity_3d: Sink supports 3d osd disparity indication
+	 * in HF-VSIF.
+	 */
+	bool osd_disparity_3d;
+
 	/**
 	 * @max_tmds_clock: Maximum TMDS clock rate supported by the
 	 * sink in kHz. 0 means undefined.
@@ -185,6 +225,14 @@ struct drm_display_info {
 	u8 edid_hdmi_dc_modes;
 
 	/**
+	 * @edid_yuv420_dc_modes: bpc for deep color yuv420 encoding.
+	 * various sinks can support 10/12/16 bit per channel deep
+	 * color encoding. edid_yuv420_dc_modes = 0 means sink doesn't
+	 * support deep color yuv420 encoding.
+	 */
+	u8 edid_yuv420_dc_modes;
+
+	/**
 	 * @cea_rev: CEA revision of the HDMI sink.
 	 */
 	u8 cea_rev;
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 38eabf6..5df8b9c 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -212,6 +212,11 @@ struct detailed_timing {
 #define DRM_EDID_HDMI_DC_30               (1 << 4)
 #define DRM_EDID_HDMI_DC_Y444             (1 << 3)
 
+/* YUV 420 deep color modes */
+#define DRM_EDID_YUV420_DC_48		  (1 << 6)
+#define DRM_EDID_YUV420_DC_36		  (1 << 5)
+#define DRM_EDID_YUV420_DC_30		  (1 << 5)
+
 /* ELD Header Block */
 #define DRM_ELD_HEADER_BLOCK_SIZE	4
 
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index edbb4fc..2d4e9ef 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -35,6 +35,7 @@ enum hdmi_infoframe_type {
 };
 
 #define HDMI_IEEE_OUI 0x000c03
+#define HDMI_IEEE_OUI_HFVSDB 0xC45DD8
 #define HDMI_INFOFRAME_HEADER_SIZE  4
 #define HDMI_AVI_INFOFRAME_SIZE    13
 #define HDMI_SPD_INFOFRAME_SIZE    25
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* ✗ Fi.CI.BAT: failure for drm: parse hf-vsdb
  2016-11-29 14:54 [PATCH] drm: parse hf-vsdb Shashank Sharma
@ 2016-11-29 18:23 ` Patchwork
  2016-12-05 16:59 ` [PATCH] " Thierry Reding
  1 sibling, 0 replies; 4+ messages in thread
From: Patchwork @ 2016-11-29 18:23 UTC (permalink / raw)
  To: Shashank Sharma; +Cc: intel-gfx

== Series Details ==

Series: drm: parse hf-vsdb
URL   : https://patchwork.freedesktop.org/series/16104/
State : failure

== Summary ==

Series 16104v1 drm: parse hf-vsdb
https://patchwork.freedesktop.org/api/1.0/series/16104/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> INCOMPLETE (fi-skl-6260u)

fi-bdw-5557u     total:245  pass:230  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:245  pass:205  dwarn:0   dfail:0   fail:0   skip:40 
fi-bxt-t5700     total:245  pass:217  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-j1900     total:245  pass:217  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-n2820     total:245  pass:213  dwarn:0   dfail:0   fail:0   skip:32 
fi-hsw-4770      total:245  pass:225  dwarn:0   dfail:0   fail:0   skip:20 
fi-hsw-4770r     total:245  pass:225  dwarn:0   dfail:0   fail:0   skip:20 
fi-ilk-650       total:245  pass:192  dwarn:0   dfail:0   fail:0   skip:53 
fi-ivb-3520m     total:245  pass:223  dwarn:0   dfail:0   fail:0   skip:22 
fi-ivb-3770      total:245  pass:223  dwarn:0   dfail:0   fail:0   skip:22 
fi-kbl-7500u     total:245  pass:223  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-6260u     total:208  pass:195  dwarn:0   dfail:0   fail:0   skip:12 
fi-skl-6700hq    total:245  pass:224  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6700k     total:245  pass:223  dwarn:1   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:245  pass:230  dwarn:1   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:245  pass:213  dwarn:0   dfail:0   fail:0   skip:32 
fi-snb-2600      total:245  pass:212  dwarn:0   dfail:0   fail:0   skip:33 

ccba3c78ee8dd04506b9a473f37450e7707c34da drm-tip: 2016y-11m-29d-15h-56m-34s UTC integration manifest
320de50c drm: parse hf-vsdb

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3142/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm: parse hf-vsdb
  2016-11-29 14:54 [PATCH] drm: parse hf-vsdb Shashank Sharma
  2016-11-29 18:23 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2016-12-05 16:59 ` Thierry Reding
  2016-12-19 14:19   ` Sharma, Shashank
  1 sibling, 1 reply; 4+ messages in thread
From: Thierry Reding @ 2016-12-05 16:59 UTC (permalink / raw)
  To: Shashank Sharma; +Cc: airlied, intel-gfx, dri-devel, daniel.vetter


[-- Attachment #1.1: Type: text/plain, Size: 7053 bytes --]

On Tue, Nov 29, 2016 at 08:24:39PM +0530, Shashank Sharma wrote:
> HDMI 2.0 / CEA-861-F specs define a new CEA extension data block,
> called hdmi-forum vendor specific data block (HF-VSDB). This block
> contains information about sink's support for HDMI 2.0 compliant
> features. These features are:
>     - Deep color YUV 420 support and BPC
>     - 3D flags for
>         - OSD Displarity
>         - Dual view signaling
>         - independent view signaling
>     - SCDC support
>     - Max TMDS char rate
>     - Scrambling support
> 
> This patch adds a parser function for this block, and add flags to
> indicate support for new features, in drm_display_info structure.
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c  | 73 +++++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_connector.h | 48 +++++++++++++++++++++++++++++
>  include/drm/drm_edid.h      |  5 ++++
>  include/linux/hdmi.h        |  1 +
>  4 files changed, 127 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 336be31..b18bfe0 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3223,6 +3223,27 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure,
>  	return 0;
>  }
>  
> +static bool cea_db_is_hf_vsdb(const u8 *db)
> +{
> +	u8 len;
> +	int hfvsdb_id;
> +
> +	if (cea_db_tag(db) != VENDOR_BLOCK)
> +		return false;
> +
> +	len = cea_db_payload_len(db);
> +	if (len < 7 || len > 31)

The second part of the check is unnecessary because there is no way that
cea_db_payload_len() will return a number larger than 31.

> +		return false;
> +
> +	/* version should be 1 */
> +	if (db[4]  != 1)

There's an extra space before !=. Also I'm not sure this is something
that we need to worry about. Future versions of the HF VSDB are likely
to be backwards compatible, so this seems like an unnecessary
restriction.

> +		return false;
> +
> +	hfvsdb_id = db[1] | (db[2] << 8) | (db[3] << 16);
> +
> +	return hfvsdb_id == HDMI_IEEE_OUI_HFVSDB;
> +}
> +
>  static bool cea_db_is_hdmi_vsdb(const u8 *db)
>  {
>  	int hdmi_id;
> @@ -3767,6 +3788,56 @@ bool drm_rgb_quant_range_selectable(struct edid *edid)
>  }
>  EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
>  
> +static void drm_parse_yuv420_deep_color_info(struct drm_connector *connector,
> +						const u8 *db)
> +{
> +	struct drm_display_info *info = &connector->display_info;
> +
> +	if (db[7] & DRM_EDID_YUV420_DC_48)
> +		info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_48;
> +	if (db[7] & DRM_EDID_YUV420_DC_36)
> +		info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_36;
> +	if (db[7] & DRM_EDID_YUV420_DC_30)
> +		info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_30;
> +
> +	if (!info->edid_yuv420_dc_modes) {
> +		DRM_DEBUG("%s: No YUV 420 deep color support in sink.\n",
> +			  connector->name);
> +		return;
> +	}
> +}
> +
> +static void
> +drm_parse_hf_vsdb(struct drm_connector *connector, const u8 *db)
> +{
> +	struct drm_display_info *info = &connector->display_info;
> +
> +	if (db[5]) {
> +		/*
> +		 * If the sink supplies max tmds char rate in db,
> +		 * the actual max tmds rate = db[5] * 5Mhz.
> +		 */
> +		info->max_tmds_clock = db[5] * 5000;
> +		DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
> +		info->max_tmds_clock);
> +	}
> +
> +	if (db[6] & DRM_HFVSDB_SCDC_SUPPORT)
> +		info->scdc_supported = true;
> +	if (db[6] & DRM_HFVSDB_SCDC_RR_CAP)
> +		info->scdc_rr_cap = true;
> +	if (db[6] & DRM_HFVSDB_SCRAMBLING)
> +		info->scrambling = true;
> +	if (db[6] & DRM_HFVSDB_INDEPENDENT_VIEW)
> +		info->independent_view_3d = true;
> +	if (db[6] & DRM_HFVSDB_DUAL_VIEW)
> +		info->dual_view_3d = true;
> +	if (db[6] & DRM_HFVSDB_3D_OSD)
> +		info->osd_disparity_3d = true;
> +
> +	drm_parse_yuv420_deep_color_info(connector, db);
> +}
> +
>  static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
>  					   const u8 *hdmi)
>  {
> @@ -3881,6 +3952,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>  
>  		if (cea_db_is_hdmi_vsdb(db))
>  			drm_parse_hdmi_vsdb_video(connector, db);
> +		if (cea_db_is_hf_vsdb(db))
> +			drm_parse_hf_vsdb(connector, db);
>  	}
>  }
>  
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 34f9741..d011dd5 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -167,6 +167,46 @@ struct drm_display_info {
>  	 */
>  	u32 bus_flags;
>  
> +#define DRM_HFVSDB_SCDC_SUPPORT	(1<<7)
> +#define DRM_HFVSDB_SCDC_RR_CAP		(1<<6)
> +#define DRM_HFVSDB_SCRAMBLING		(1<<3)
> +#define DRM_HFVSDB_INDEPENDENT_VIEW	(1<<2)
> +#define DRM_HFVSDB_DUAL_VIEW		(1<<1)
> +#define DRM_HFVSDB_3D_OSD		(1<<0)

This looks to me like the wrong place for these defines. They should
probably go into include/drm/drm_edid.h instead.

> +
> +	/**
> +	 * @scdc_supported: Sink supports SCDC functionality.
> +	 */
> +	bool scdc_supported;
> +
> +	/**
> +	 * @scdc_rr_cap: Sink has SCDC read request capability.
> +	 */
> +	bool scdc_rr_cap;
> +
> +	/**
> +	 * @scrambling: Sync supports scrambling for <=340 Mcsc TMDS
> +	 * char rates. Above 340 Mcsc rates, scrambling is always reqd.
> +	 */
> +	bool scrambling;
> +
> +	/**
> +	 * @independent_view_3d: Sink supports 3d independent view signaling
> +	 * in HF-VSIF.
> +	 */
> +	bool independent_view_3d;
> +
> +	/**
> +	 * @dual_view_3d: Sink supports 3d dual view signaling in HF-VSIF.
> +	 */
> +	bool dual_view_3d;
> +
> +	/**
> +	 * @osd_disparity_3d: Sink supports 3d osd disparity indication
> +	 * in HF-VSIF.
> +	 */
> +	bool osd_disparity_3d;
> +
>  	/**
>  	 * @max_tmds_clock: Maximum TMDS clock rate supported by the
>  	 * sink in kHz. 0 means undefined.
> @@ -185,6 +225,14 @@ struct drm_display_info {
>  	u8 edid_hdmi_dc_modes;
>  
>  	/**
> +	 * @edid_yuv420_dc_modes: bpc for deep color yuv420 encoding.
> +	 * various sinks can support 10/12/16 bit per channel deep
> +	 * color encoding. edid_yuv420_dc_modes = 0 means sink doesn't
> +	 * support deep color yuv420 encoding.
> +	 */
> +	u8 edid_yuv420_dc_modes;

Why not store the additional formats in the edid_hdmi_dc_modes field?

> +
> +	/**
>  	 * @cea_rev: CEA revision of the HDMI sink.
>  	 */
>  	u8 cea_rev;
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 38eabf6..5df8b9c 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -212,6 +212,11 @@ struct detailed_timing {
>  #define DRM_EDID_HDMI_DC_30               (1 << 4)
>  #define DRM_EDID_HDMI_DC_Y444             (1 << 3)
>  
> +/* YUV 420 deep color modes */
> +#define DRM_EDID_YUV420_DC_48		  (1 << 6)
> +#define DRM_EDID_YUV420_DC_36		  (1 << 5)
> +#define DRM_EDID_YUV420_DC_30		  (1 << 5)

36- and 30-bit depths have the same value. That probably wasn't
intended.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm: parse hf-vsdb
  2016-12-05 16:59 ` [PATCH] " Thierry Reding
@ 2016-12-19 14:19   ` Sharma, Shashank
  0 siblings, 0 replies; 4+ messages in thread
From: Sharma, Shashank @ 2016-12-19 14:19 UTC (permalink / raw)
  To: Thierry Reding; +Cc: airlied, intel-gfx, dri-devel, daniel.vetter


[-- Attachment #1.1: Type: text/plain, Size: 7608 bytes --]

Thanks for the review, Thierry. My comments inline.

Regards

Shashank


On 12/5/2016 10:29 PM, Thierry Reding wrote:
> On Tue, Nov 29, 2016 at 08:24:39PM +0530, Shashank Sharma wrote:
>> HDMI 2.0 / CEA-861-F specs define a new CEA extension data block,
>> called hdmi-forum vendor specific data block (HF-VSDB). This block
>> contains information about sink's support for HDMI 2.0 compliant
>> features. These features are:
>>      - Deep color YUV 420 support and BPC
>>      - 3D flags for
>>          - OSD Displarity
>>          - Dual view signaling
>>          - independent view signaling
>>      - SCDC support
>>      - Max TMDS char rate
>>      - Scrambling support
>>
>> This patch adds a parser function for this block, and add flags to
>> indicate support for new features, in drm_display_info structure.
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/drm_edid.c  | 73 +++++++++++++++++++++++++++++++++++++++++++++
>>   include/drm/drm_connector.h | 48 +++++++++++++++++++++++++++++
>>   include/drm/drm_edid.h      |  5 ++++
>>   include/linux/hdmi.h        |  1 +
>>   4 files changed, 127 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 336be31..b18bfe0 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -3223,6 +3223,27 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure,
>>   	return 0;
>>   }
>>   
>> +static bool cea_db_is_hf_vsdb(const u8 *db)
>> +{
>> +	u8 len;
>> +	int hfvsdb_id;
>> +
>> +	if (cea_db_tag(db) != VENDOR_BLOCK)
>> +		return false;
>> +
>> +	len = cea_db_payload_len(db);
>> +	if (len < 7 || len > 31)
> The second part of the check is unnecessary because there is no way that
> cea_db_payload_len() will return a number larger than 31.
Agree. Will remove this check.
>> +		return false;
>> +
>> +	/* version should be 1 */
>> +	if (db[4]  != 1)
> There's an extra space before !=.
Oh, I am surprised that checkpatch dint complaint. Will check this out.
> Also I'm not sure this is something
> that we need to worry about. Future versions of the HF VSDB are likely
> to be backwards compatible, so this seems like an unnecessary
> restriction.
Agree.
>
>> +		return false;
>> +
>> +	hfvsdb_id = db[1] | (db[2] << 8) | (db[3] << 16);
>> +
>> +	return hfvsdb_id == HDMI_IEEE_OUI_HFVSDB;
>> +}
>> +
>>   static bool cea_db_is_hdmi_vsdb(const u8 *db)
>>   {
>>   	int hdmi_id;
>> @@ -3767,6 +3788,56 @@ bool drm_rgb_quant_range_selectable(struct edid *edid)
>>   }
>>   EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
>>   
>> +static void drm_parse_yuv420_deep_color_info(struct drm_connector *connector,
>> +						const u8 *db)
>> +{
>> +	struct drm_display_info *info = &connector->display_info;
>> +
>> +	if (db[7] & DRM_EDID_YUV420_DC_48)
>> +		info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_48;
>> +	if (db[7] & DRM_EDID_YUV420_DC_36)
>> +		info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_36;
>> +	if (db[7] & DRM_EDID_YUV420_DC_30)
>> +		info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_30;
>> +
>> +	if (!info->edid_yuv420_dc_modes) {
>> +		DRM_DEBUG("%s: No YUV 420 deep color support in sink.\n",
>> +			  connector->name);
>> +		return;
>> +	}
>> +}
>> +
>> +static void
>> +drm_parse_hf_vsdb(struct drm_connector *connector, const u8 *db)
>> +{
>> +	struct drm_display_info *info = &connector->display_info;
>> +
>> +	if (db[5]) {
>> +		/*
>> +		 * If the sink supplies max tmds char rate in db,
>> +		 * the actual max tmds rate = db[5] * 5Mhz.
>> +		 */
>> +		info->max_tmds_clock = db[5] * 5000;
>> +		DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
>> +		info->max_tmds_clock);
>> +	}
>> +
>> +	if (db[6] & DRM_HFVSDB_SCDC_SUPPORT)
>> +		info->scdc_supported = true;
>> +	if (db[6] & DRM_HFVSDB_SCDC_RR_CAP)
>> +		info->scdc_rr_cap = true;
>> +	if (db[6] & DRM_HFVSDB_SCRAMBLING)
>> +		info->scrambling = true;
>> +	if (db[6] & DRM_HFVSDB_INDEPENDENT_VIEW)
>> +		info->independent_view_3d = true;
>> +	if (db[6] & DRM_HFVSDB_DUAL_VIEW)
>> +		info->dual_view_3d = true;
>> +	if (db[6] & DRM_HFVSDB_3D_OSD)
>> +		info->osd_disparity_3d = true;
>> +
>> +	drm_parse_yuv420_deep_color_info(connector, db);
>> +}
>> +
>>   static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
>>   					   const u8 *hdmi)
>>   {
>> @@ -3881,6 +3952,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>>   
>>   		if (cea_db_is_hdmi_vsdb(db))
>>   			drm_parse_hdmi_vsdb_video(connector, db);
>> +		if (cea_db_is_hf_vsdb(db))
>> +			drm_parse_hf_vsdb(connector, db);
>>   	}
>>   }
>>   
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index 34f9741..d011dd5 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -167,6 +167,46 @@ struct drm_display_info {
>>   	 */
>>   	u32 bus_flags;
>>   
>> +#define DRM_HFVSDB_SCDC_SUPPORT	(1<<7)
>> +#define DRM_HFVSDB_SCDC_RR_CAP		(1<<6)
>> +#define DRM_HFVSDB_SCRAMBLING		(1<<3)
>> +#define DRM_HFVSDB_INDEPENDENT_VIEW	(1<<2)
>> +#define DRM_HFVSDB_DUAL_VIEW		(1<<1)
>> +#define DRM_HFVSDB_3D_OSD		(1<<0)
> This looks to me like the wrong place for these defines. They should
> probably go into include/drm/drm_edid.h instead.
I saw few other defines in this same structure, like 
DRM_COLOR_FORMAT_RGB444 etc, so thought this would
be the right place.
>
>> +
>> +	/**
>> +	 * @scdc_supported: Sink supports SCDC functionality.
>> +	 */
>> +	bool scdc_supported;
>> +
>> +	/**
>> +	 * @scdc_rr_cap: Sink has SCDC read request capability.
>> +	 */
>> +	bool scdc_rr_cap;
>> +
>> +	/**
>> +	 * @scrambling: Sync supports scrambling for <=340 Mcsc TMDS
>> +	 * char rates. Above 340 Mcsc rates, scrambling is always reqd.
>> +	 */
>> +	bool scrambling;
>> +
>> +	/**
>> +	 * @independent_view_3d: Sink supports 3d independent view signaling
>> +	 * in HF-VSIF.
>> +	 */
>> +	bool independent_view_3d;
>> +
>> +	/**
>> +	 * @dual_view_3d: Sink supports 3d dual view signaling in HF-VSIF.
>> +	 */
>> +	bool dual_view_3d;
>> +
>> +	/**
>> +	 * @osd_disparity_3d: Sink supports 3d osd disparity indication
>> +	 * in HF-VSIF.
>> +	 */
>> +	bool osd_disparity_3d;
>> +
>>   	/**
>>   	 * @max_tmds_clock: Maximum TMDS clock rate supported by the
>>   	 * sink in kHz. 0 means undefined.
>> @@ -185,6 +225,14 @@ struct drm_display_info {
>>   	u8 edid_hdmi_dc_modes;
>>   
>>   	/**
>> +	 * @edid_yuv420_dc_modes: bpc for deep color yuv420 encoding.
>> +	 * various sinks can support 10/12/16 bit per channel deep
>> +	 * color encoding. edid_yuv420_dc_modes = 0 means sink doesn't
>> +	 * support deep color yuv420 encoding.
>> +	 */
>> +	u8 edid_yuv420_dc_modes;
> Why not store the additional formats in the edid_hdmi_dc_modes field?
Now, as per our discussion in mail thread, I will create nested 
drm_hdmi_info within drm_display_info and add it there.
>
>> +
>> +	/**
>>   	 * @cea_rev: CEA revision of the HDMI sink.
>>   	 */
>>   	u8 cea_rev;
>> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
>> index 38eabf6..5df8b9c 100644
>> --- a/include/drm/drm_edid.h
>> +++ b/include/drm/drm_edid.h
>> @@ -212,6 +212,11 @@ struct detailed_timing {
>>   #define DRM_EDID_HDMI_DC_30               (1 << 4)
>>   #define DRM_EDID_HDMI_DC_Y444             (1 << 3)
>>   
>> +/* YUV 420 deep color modes */
>> +#define DRM_EDID_YUV420_DC_48		  (1 << 6)
>> +#define DRM_EDID_YUV420_DC_36		  (1 << 5)
>> +#define DRM_EDID_YUV420_DC_30		  (1 << 5)
> 36- and 30-bit depths have the same value. That probably wasn't
> intended.
Oops, thanks for pointing this out.
> Thierry


[-- Attachment #1.2: Type: text/html, Size: 46542 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-12-19 14:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-29 14:54 [PATCH] drm: parse hf-vsdb Shashank Sharma
2016-11-29 18:23 ` ✗ Fi.CI.BAT: failure for " Patchwork
2016-12-05 16:59 ` [PATCH] " Thierry Reding
2016-12-19 14:19   ` Sharma, Shashank

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox