Re: [PATCH] drm: parse hf-vsdb

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

 



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

_______________________________________________
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