[PATCH v8 0/6] HDMI 2.0: Scrambling in DRM layer

[PATCH v8 0/6] HDMI 2.0: Scrambling in DRM layer

[Shashank Sharma]

HDMI 2.0 spec defines a method to reduce the RF footprint while
operating at higher pixel clocks, which is called Scrambling.
Scrambling can be controlled over a new set of I2C registers
which are accessible over existing DDC I2C lines, called SCDC
register set.

This patch series contains 6 patches:
- First two patches add generic drm helper functions to read and
  write into SCDC registers. These patches are written by Thierry,
  in a patch series published here:
  https://patchwork.kernel.org/patch/9459051/
  Minor changes were done to map the patches into this series.

- Next two patches add functions for scrambling detection and
  scrambling control.

- Next two patches use this infrastructure in DRM layer from
  I915 driver, to enable scrambling on a GLK deivce which sports
  a native HDMI 2.0 controller.

V2:
 - addressed review comments from Thierry, Ville and Dhinakaran
 - added signed-off-by:self in first two patches(Jani N)

V3:
 - addressed review comments from Jose and Jani

V4:
 - addressed review comments from Maarten on patch 5,
   rebase all other patches

V5:
 - addressed review comments from Ville/Ander on patch 1, 4, 5
   rebase all other patches

V6:
 - addressed review comments from Ville on patch 4, 5
   rebase all other patches

V7:
 - addressed review comments from Ander on patch 5
   rebase all other patches,
   added r-b from Jose on patch 3,4

V8:
 - addressed review comments from Ander on patch 5
 - rebased and fixed merge conflict

Shashank Sharma (4):
  drm/edid: detect SCDC support in HF-VSDB
  drm/edid: detect SCDC support in HF-VSDB
  drm/i915: enable scrambling
  drm/i915: allow HDMI 2.0 clock rates

Thierry Reding (2):
  drm: Add SCDC helpers
  drm/edid: check for HF-VSDB block

 Documentation/gpu/drm-kms-helpers.rst |  12 ++
 drivers/gpu/drm/Makefile              |   3 +-
 drivers/gpu/drm/drm_edid.c            |  60 +++++++++
 drivers/gpu/drm/drm_scdc_helper.c     | 244 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h       |   7 +
 drivers/gpu/drm/i915/intel_ddi.c      |  19 +++
 drivers/gpu/drm/i915/intel_display.c  |   3 +
 drivers/gpu/drm/i915/intel_drv.h      |  10 ++
 drivers/gpu/drm/i915/intel_hdmi.c     |  64 +++++++++
 include/drm/drm_connector.h           |  52 ++++++++
 include/drm/drm_edid.h                |   1 -
 include/drm/drm_scdc_helper.h         | 159 ++++++++++++++++++++++
 include/linux/hdmi.h                  |   1 +
 13 files changed, 633 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_scdc_helper.c
 create mode 100644 include/drm/drm_scdc_helper.h

-- 
1.9.1

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

[PATCH v8 1/6] drm: Add SCDC helpers

[Shashank Sharma]

From: Thierry Reding <treding@nvidia.com>

SCDC is a mechanism defined in the HDMI 2.0 specification that allows
the source and sink devices to communicate.

This commit introduces helpers to access the SCDC and provides the
symbolic names for the various registers defined in the specification.

V2: Rebase.
V3: Added R-B from Jose.
V4: Rebase
V5: Addressed review comments from Ville
 - Handle the I2c return values in a better way (dp_dual_mode)
 - Make the macros for SCDC Major/Minor more readable, by adding
   a 'GET' in the macro names
V6: Rebase
V7: Rebase
V8: Rebase

Signed-off-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 Documentation/gpu/drm-kms-helpers.rst |  12 ++++
 drivers/gpu/drm/Makefile              |   3 +-
 drivers/gpu/drm/drm_scdc_helper.c     | 123 +++++++++++++++++++++++++++++++
 include/drm/drm_scdc_helper.h         | 132 ++++++++++++++++++++++++++++++++++
 4 files changed, 269 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/drm_scdc_helper.c
 create mode 100644 include/drm/drm_scdc_helper.h

diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index 03040aa..7592756 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -217,6 +217,18 @@ EDID Helper Functions Reference
 .. kernel-doc:: drivers/gpu/drm/drm_edid.c
    :export:
 
+SCDC Helper Functions Reference
+===============================
+
+.. kernel-doc:: drivers/gpu/drm/drm_scdc_helper.c
+   :doc: scdc helpers
+
+.. kernel-doc:: include/drm/drm_scdc_helper.h
+   :internal:
+
+.. kernel-doc:: drivers/gpu/drm/drm_scdc_helper.c
+   :export:
+
 Rectangle Utilities Reference
 =============================
 
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 3ee9579..da5b08c 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -31,7 +31,8 @@ drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
 drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
 		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
 		drm_kms_helper_common.o drm_dp_dual_mode_helper.o \
-		drm_simple_kms_helper.o drm_modeset_helper.o
+		drm_simple_kms_helper.o drm_modeset_helper.o \
+		drm_scdc_helper.o
 
 drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
 drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
diff --git a/drivers/gpu/drm/drm_scdc_helper.c b/drivers/gpu/drm/drm_scdc_helper.c
new file mode 100644
index 0000000..c2dd33f
--- /dev/null
+++ b/drivers/gpu/drm/drm_scdc_helper.c
@@ -0,0 +1,123 @@
+/*
+ * Copyright (c) 2015 NVIDIA Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sub license,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include <linux/slab.h>
+
+#include <drm/drm_scdc_helper.h>
+
+/**
+ * DOC: scdc helpers
+ *
+ * Status and Control Data Channel (SCDC) is a mechanism introduced by the
+ * HDMI 2.0 specification. It is a point-to-point protocol that allows the
+ * HDMI source and HDMI sink to exchange data. The same I2C interface that
+ * is used to access EDID serves as the transport mechanism for SCDC.
+ */
+
+#define SCDC_I2C_SLAVE_ADDRESS 0x54
+
+/**
+ * drm_scdc_read - read a block of data from SCDC
+ * @adapter: I2C controller
+ * @offset: start offset of block to read
+ * @buffer: return location for the block to read
+ * @size: size of the block to read
+ *
+ * Reads a block of data from SCDC, starting at a given offset.
+ *
+ * Returns:
+ * 0 on success, negative error code on failure.
+ */
+ssize_t drm_scdc_read(struct i2c_adapter *adapter, u8 offset, void *buffer,
+		      size_t size)
+{
+	int ret;
+	struct i2c_msg msgs[2] = {
+		{
+			.addr = SCDC_I2C_SLAVE_ADDRESS,
+			.flags = 0,
+			.len = 1,
+			.buf = &offset,
+		}, {
+			.addr = SCDC_I2C_SLAVE_ADDRESS,
+			.flags = I2C_M_RD,
+			.len = size,
+			.buf = buffer,
+		}
+	};
+
+	ret = i2c_transfer(adapter, msgs, ARRAY_SIZE(msgs));
+	if (ret < 0)
+		return ret;
+	if (ret != ARRAY_SIZE(msgs))
+		return -EPROTO;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_scdc_read);
+
+/**
+ * drm_scdc_write - write a block of data to SCDC
+ * @adapter: I2C controller
+ * @offset: start offset of block to write
+ * @buffer: block of data to write
+ * @size: size of the block to write
+ *
+ * Writes a block of data to SCDC, starting at a given offset.
+ *
+ * Returns:
+ * 0 on success, negative error code on failure.
+ */
+ssize_t drm_scdc_write(struct i2c_adapter *adapter, u8 offset,
+		       const void *buffer, size_t size)
+{
+	struct i2c_msg msg = {
+		.addr = SCDC_I2C_SLAVE_ADDRESS,
+		.flags = 0,
+		.len = 1 + size,
+		.buf = NULL,
+	};
+	void *data;
+	int err;
+
+	data = kmalloc(1 + size, GFP_TEMPORARY);
+	if (!data)
+		return -ENOMEM;
+
+	msg.buf = data;
+
+	memcpy(data, &offset, sizeof(offset));
+	memcpy(data + 1, buffer, size);
+
+	err = i2c_transfer(adapter, &msg, 1);
+
+	kfree(data);
+
+	if (err < 0)
+		return err;
+	if (err != 1)
+		return -EPROTO;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_scdc_write);
diff --git a/include/drm/drm_scdc_helper.h b/include/drm/drm_scdc_helper.h
new file mode 100644
index 0000000..9c52deb
--- /dev/null
+++ b/include/drm/drm_scdc_helper.h
@@ -0,0 +1,132 @@
+/*
+ * Copyright (c) 2015 NVIDIA Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sub license,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef DRM_SCDC_HELPER_H
+#define DRM_SCDC_HELPER_H
+
+#include <linux/i2c.h>
+#include <linux/types.h>
+
+#define SCDC_SINK_VERSION 0x01
+
+#define SCDC_SOURCE_VERSION 0x02
+
+#define SCDC_UPDATE_0 0x10
+#define  SCDC_READ_REQUEST_TEST (1 << 2)
+#define  SCDC_CED_UPDATE (1 << 1)
+#define  SCDC_STATUS_UPDATE (1 << 0)
+
+#define SCDC_UPDATE_1 0x11
+
+#define SCDC_TMDS_CONFIG 0x20
+#define  SCDC_TMDS_BIT_CLOCK_RATIO_BY_40 (1 << 1)
+#define  SCDC_TMDS_BIT_CLOCK_RATIO_BY_10 (0 << 1)
+#define  SCDC_SCRAMBLING_ENABLE (1 << 0)
+
+#define SCDC_SCRAMBLER_STATUS 0x21
+#define  SCDC_SCRAMBLING_STATUS (1 << 0)
+
+#define SCDC_CONFIG_0 0x30
+#define  SCDC_READ_REQUEST_ENABLE (1 << 0)
+
+#define SCDC_STATUS_FLAGS_0 0x40
+#define  SCDC_CH2_LOCK (1 < 3)
+#define  SCDC_CH1_LOCK (1 < 2)
+#define  SCDC_CH0_LOCK (1 < 1)
+#define  SCDC_CH_LOCK_MASK (SCDC_CH2_LOCK | SCDC_CH1_LOCK | SCDC_CH0_LOCK)
+#define  SCDC_CLOCK_DETECT (1 << 0)
+
+#define SCDC_STATUS_FLAGS_1 0x41
+
+#define SCDC_ERR_DET_0_L 0x50
+#define SCDC_ERR_DET_0_H 0x51
+#define SCDC_ERR_DET_1_L 0x52
+#define SCDC_ERR_DET_1_H 0x53
+#define SCDC_ERR_DET_2_L 0x54
+#define SCDC_ERR_DET_2_H 0x55
+#define  SCDC_CHANNEL_VALID (1 << 7)
+
+#define SCDC_ERR_DET_CHECKSUM 0x56
+
+#define SCDC_TEST_CONFIG_0 0xc0
+#define  SCDC_TEST_READ_REQUEST (1 << 7)
+#define  SCDC_TEST_READ_REQUEST_DELAY(x) ((x) & 0x7f)
+
+#define SCDC_MANUFACTURER_IEEE_OUI 0xd0
+#define SCDC_MANUFACTURER_IEEE_OUI_SIZE 3
+
+#define SCDC_DEVICE_ID 0xd3
+#define SCDC_DEVICE_ID_SIZE 8
+
+#define SCDC_DEVICE_HARDWARE_REVISION 0xdb
+#define  SCDC_GET_DEVICE_HARDWARE_REVISION_MAJOR(x) (((x) >> 4) & 0xf)
+#define  SCDC_GET_DEVICE_HARDWARE_REVISION_MINOR(x) (((x) >> 0) & 0xf)
+
+#define SCDC_DEVICE_SOFTWARE_MAJOR_REVISION 0xdc
+#define SCDC_DEVICE_SOFTWARE_MINOR_REVISION 0xdd
+
+#define SCDC_MANUFACTURER_SPECIFIC 0xde
+#define SCDC_MANUFACTURER_SPECIFIC_SIZE 34
+
+ssize_t drm_scdc_read(struct i2c_adapter *adapter, u8 offset, void *buffer,
+		      size_t size);
+ssize_t drm_scdc_write(struct i2c_adapter *adapter, u8 offset,
+		       const void *buffer, size_t size);
+
+/**
+ * drm_scdc_readb - read a single byte from SCDC
+ * @adapter: I2C adapter
+ * @offset: offset of register to read
+ * @value: return location for the register value
+ *
+ * Reads a single byte from SCDC. This is a convenience wrapper around the
+ * drm_scdc_read() function.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+static inline int drm_scdc_readb(struct i2c_adapter *adapter, u8 offset,
+				 u8 *value)
+{
+	return drm_scdc_read(adapter, offset, value, sizeof(*value));
+}
+
+/**
+ * drm_scdc_writeb - write a single byte to SCDC
+ * @adapter: I2C adapter
+ * @offset: offset of register to read
+ * @value: return location for the register value
+ *
+ * Writes a single byte to SCDC. This is a convenience wrapper around the
+ * drm_scdc_write() function.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+static inline int drm_scdc_writeb(struct i2c_adapter *adapter, u8 offset,
+				  u8 value)
+{
+	return drm_scdc_write(adapter, offset, &value, sizeof(value));
+}
+
+#endif
-- 
1.9.1

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

[PATCH v8 2/6] drm/edid: check for HF-VSDB block

[Shashank Sharma]

From: Thierry Reding <treding@nvidia.com>

This patch implements a small function that finds if a
given CEA db is hdmi-forum vendor specific data block
or not.

V2: Rebase.
V3: Added R-B from Jose.
V4: Rebase
V5: Rebase
V6: Rebase
V7: Rebase
V8: Rebase

Signed-off-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/drm_edid.c | 15 +++++++++++++++
 include/linux/hdmi.h       |  1 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 5da5a85..3e0aafa 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3251,6 +3251,21 @@ static bool cea_db_is_hdmi_vsdb(const u8 *db)
 	return hdmi_id == HDMI_IEEE_OUI;
 }
 
+static bool cea_db_is_hdmi_forum_vsdb(const u8 *db)
+{
+	unsigned int oui;
+
+	if (cea_db_tag(db) != VENDOR_BLOCK)
+		return false;
+
+	if (cea_db_payload_len(db) < 7)
+		return false;
+
+	oui = db[3] << 16 | db[2] << 8 | db[1];
+
+	return oui == HDMI_FORUM_IEEE_OUI;
+}
+
 #define for_each_cea_db(cea, i, start, end) \
 	for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1)
 
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index edbb4fc..d271ff2 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_FORUM_IEEE_OUI 0xc45dd8
 #define HDMI_INFOFRAME_HEADER_SIZE  4
 #define HDMI_AVI_INFOFRAME_SIZE    13
 #define HDMI_SPD_INFOFRAME_SIZE    25
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v8 3/6] drm/edid: detect SCDC support in HF-VSDB

[Shashank Sharma]

This patch does following:
- Adds a new structure (drm_hdmi_info) in drm_display_info.
  This structure will be used to save and indicate if sink
  supports advanced HDMI 2.0 features
- Adds another structure drm_scdc within drm_hdmi_info, to
  reflect scdc support and capabilities in connected HDMI 2.0 sink.
- Checks the HF-VSDB block for presence of SCDC, and marks it
  in scdc structure
- If SCDC is present, checks if sink is capable of generating
  SCDC read request, and marks it in scdc structure.

V2: Addressed review comments
 Thierry:
 - Fix typos in commit message and make abbreviation consistent
   across the commit message.
 - Change structure object name from hdmi_info -> hdmi
 - Fix typos and abbreviations in description of structure drm_hdmi_info
   end the description with a full stop.
 - Create a structure drm_scdc, and keep all information related to SCDC
   register set (supported, read request supported) etc in it.

Ville:
 - Change rr -> read_request
 - Call drm_detect_scrambling function drm_parse_hf_vsdb so that all
   of HF-VSDB parsing can be kept in same function, in incremental
   patches.

V3: Rebase.
V4: Rebase.
V5: Rebase.
V6: Rebase.
V7: Added R-B from Jose.
V8: Rebase.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Reviewed-by: Thierry Reding <treding@nvidia.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/drm_edid.c  | 14 ++++++++++++++
 include/drm/drm_connector.h | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 3e0aafa..d64b7bd 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3817,6 +3817,18 @@ enum hdmi_quantization_range
 }
 EXPORT_SYMBOL(drm_default_rgb_quant_range);
 
+static void drm_parse_hdmi_forum_vsdb(struct drm_connector *connector,
+				 const u8 *hf_vsdb)
+{
+	struct drm_hdmi_info *hdmi = &connector->display_info.hdmi;
+
+	if (hf_vsdb[6] & 0x80) {
+		hdmi->scdc.supported = true;
+		if (hf_vsdb[6] & 0x40)
+			hdmi->scdc.read_request = true;
+	}
+}
+
 static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
 					   const u8 *hdmi)
 {
@@ -3931,6 +3943,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_hdmi_forum_vsdb(db))
+			drm_parse_hdmi_forum_vsdb(connector, db);
 	}
 }
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index fabb35a..bf9d6f5 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -87,6 +87,34 @@ enum subpixel_order {
 	SubPixelVerticalRGB,
 	SubPixelVerticalBGR,
 	SubPixelNone,
+
+};
+
+/*
+ * struct drm_scdc - Information about scdc capabilities of a HDMI 2.0 sink
+ *
+ * Provides SCDC register support and capabilities related information on a
+ * HDMI 2.0 sink. In case of a HDMI 1.4 sink, all parameter must be 0.
+ */
+struct drm_scdc {
+	/**
+	 * @supported: status control & data channel present.
+	 */
+	bool supported;
+	/**
+	 * @read_request: sink is capable of generating scdc read request.
+	 */
+	bool read_request;
+};
+
+/**
+ * struct drm_hdmi_info - runtime information about the connected HDMI sink
+ *
+ * Describes if a given display supports advanced HDMI 2.0 features.
+ * This information is available in CEA-861-F extension blocks (like HF-VSDB).
+ */
+struct drm_hdmi_info {
+	struct drm_scdc scdc;
 };
 
 /**
@@ -204,6 +232,11 @@ struct drm_display_info {
 	 * @cea_rev: CEA revision of the HDMI sink.
 	 */
 	u8 cea_rev;
+
+	/**
+	 * @hdmi: advance features of a HDMI sink.
+	 */
+	struct drm_hdmi_info hdmi;
 };
 
 int drm_display_info_set_bus_formats(struct drm_display_info *info,
-- 
1.9.1

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

[PATCH v8 4/6] drm/edid: detect SCDC support in HF-VSDB

[Shashank Sharma]

This patch does following:
- Adds a new structure (drm_hdmi_info) in drm_display_info.
  This structure will be used to save and indicate if sink
  supports advanced HDMI 2.0 features
- Adds another structure drm_scdc within drm_hdmi_info, to
  reflect scdc support and capabilities in connected HDMI 2.0 sink.
- Checks the HF-VSDB block for presence of SCDC, and marks it
  in scdc structure
- If SCDC is present, checks if sink is capable of generating
  SCDC read request, and marks it in scdc structure.

V2: Addressed review comments
  Thierry:
  - Fix typos in commit message and make abbreviation consistent
    across the commit message.
  - Change structure object name from hdmi_info -> hdmi
  - Fix typos and abbreviations in description of structure drm_hdmi_info
    end the description with a full stop.
  - Create a structure drm_scdc, and keep all information related to SCDC
    register set (supported, read request supported) etc in it.

  Ville:
  - Change rr -> read_request
  - Call drm_detect_scrambling function drm_parse_hf_vsdb so that all
    of HF-VSDB parsing can be kept in same function, in incremental
    patches.

V3: Rebase.
V4: Rebase.
V5: Rebase.
V6: Addressed review comments from Ville
  - Add clock rate calculations for 1/10 and 1/40 ratios
  - Remove leftovers from old patchset
V7: Added R-B from Jose.
V8: Rebase.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Reviewed-by: Thierry Reding <treding@nvidia.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/gpu/drm/drm_edid.c        |  33 ++++++++++-
 drivers/gpu/drm/drm_scdc_helper.c | 121 ++++++++++++++++++++++++++++++++++++++
 include/drm/drm_connector.h       |  19 ++++++
 include/drm/drm_edid.h            |   1 -
 include/drm/drm_scdc_helper.h     |  27 +++++++++
 5 files changed, 199 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index d64b7bd..fad3d44 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -37,6 +37,7 @@
 #include <drm/drm_edid.h>
 #include <drm/drm_encoder.h>
 #include <drm/drm_displayid.h>
+#include <drm/drm_scdc_helper.h>
 
 #include "drm_crtc_internal.h"
 
@@ -3820,13 +3821,43 @@ enum hdmi_quantization_range
 static void drm_parse_hdmi_forum_vsdb(struct drm_connector *connector,
 				 const u8 *hf_vsdb)
 {
-	struct drm_hdmi_info *hdmi = &connector->display_info.hdmi;
+	struct drm_display_info *display = &connector->display_info;
+	struct drm_hdmi_info *hdmi = &display->hdmi;
 
 	if (hf_vsdb[6] & 0x80) {
 		hdmi->scdc.supported = true;
 		if (hf_vsdb[6] & 0x40)
 			hdmi->scdc.read_request = true;
 	}
+
+	/*
+	 * All HDMI 2.0 monitors must support scrambling at rates > 340 MHz.
+	 * And as per the spec, three factors confirm this:
+	 * * Availability of a HF-VSDB block in EDID (check)
+	 * * Non zero Max_TMDS_Char_Rate filed in HF-VSDB (let's check)
+	 * * SCDC support available (let's check)
+	 * Lets check it out.
+	 */
+
+	if (hf_vsdb[5]) {
+		/* max clock is 5000 KHz times block value */
+		u32 max_tmds_clock = hf_vsdb[5] * 5000;
+		struct drm_scdc *scdc = &hdmi->scdc;
+
+		if (max_tmds_clock > 340000) {
+			display->max_tmds_clock = max_tmds_clock;
+			DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
+				display->max_tmds_clock);
+		}
+
+		if (scdc->supported) {
+			scdc->scrambling.supported = true;
+
+			/* Few sinks support scrambling for cloks < 340M */
+			if ((hf_vsdb[6] & 0x8))
+				scdc->scrambling.low_rates = true;
+		}
+	}
 }
 
 static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
diff --git a/drivers/gpu/drm/drm_scdc_helper.c b/drivers/gpu/drm/drm_scdc_helper.c
index c2dd33f..3cd96a9 100644
--- a/drivers/gpu/drm/drm_scdc_helper.c
+++ b/drivers/gpu/drm/drm_scdc_helper.c
@@ -22,8 +22,10 @@
  */
 
 #include <linux/slab.h>
+#include <linux/delay.h>
 
 #include <drm/drm_scdc_helper.h>
+#include <drm/drmP.h>
 
 /**
  * DOC: scdc helpers
@@ -121,3 +123,122 @@ ssize_t drm_scdc_write(struct i2c_adapter *adapter, u8 offset,
 	return 0;
 }
 EXPORT_SYMBOL(drm_scdc_write);
+
+/**
+ * drm_scdc_check_scrambling_status - what is status of scrambling?
+ * @adapter: I2C adapter for DDC channel
+ *
+ * Reads the scrambler status over SCDC, and checks the
+ * scrambling status.
+ *
+ * Returns:
+ * True if the scrambling is enabled, false otherwise.
+ */
+
+bool drm_scdc_get_scrambling_status(struct i2c_adapter *adapter)
+{
+	u8 status;
+	int ret;
+
+	ret = drm_scdc_readb(adapter, SCDC_SCRAMBLER_STATUS, &status);
+	if (ret < 0) {
+		DRM_ERROR("Failed to read scrambling status, error %d\n", ret);
+		return false;
+	}
+
+	return status & SCDC_SCRAMBLING_STATUS;
+}
+EXPORT_SYMBOL(drm_scdc_get_scrambling_status);
+
+/**
+ * drm_scdc_set_scrambling - enable scrambling
+ * @adapter: I2C adapter for DDC channel
+ * @enable: bool to indicate if scrambling is to be enabled/disabled
+ *
+ * Writes the TMDS config register over SCDC channel, and:
+ * enables scrambling when enable = 1
+ * disables scrambling when enable = 0
+ *
+ * Returns:
+ * True if scrambling is set/reset successfully, false otherwise.
+ */
+
+bool drm_scdc_set_scrambling(struct i2c_adapter *adapter, bool enable)
+{
+	u8 config;
+	int ret;
+
+	ret = drm_scdc_readb(adapter, SCDC_TMDS_CONFIG, &config);
+	if (ret < 0) {
+		DRM_ERROR("Failed to read tmds config, err=%d\n", ret);
+		return false;
+	}
+
+	if (enable)
+		config |= SCDC_SCRAMBLING_ENABLE;
+	else
+		config &= ~SCDC_SCRAMBLING_ENABLE;
+
+	ret = drm_scdc_writeb(adapter, SCDC_TMDS_CONFIG, config);
+	if (ret < 0) {
+		DRM_ERROR("Failed to enable scrambling, error %d\n", ret);
+		return false;
+	}
+
+	return true;
+}
+EXPORT_SYMBOL(drm_scdc_set_scrambling);
+
+/**
+ * drm_scdc_set_high_tmds_clock_ratio - set TMDS clock ratio
+ * @adapter: I2C adapter for DDC channel
+ * @set: ret or reset the high clock ratio
+ *
+ * TMDS clock ratio calculations go like this:
+ * TMDS character = 10 bit TMDS encoded value
+ * TMDS character rate = The rate at which TMDS characters are transmitted(Mcsc)
+ * TMDS bit rate = 10x TMDS character rate
+ * As per the spec:
+ * TMDS clock rate for pixel clock < 340 MHz = 1x the character rate
+ *	= 1/10 pixel clock rate
+ * TMDS clock rate for pixel clock > 340 MHz = 0.25x the character rate
+ *	= 1/40 pixel clock rate
+ *
+ * Writes to the TMDS config register over SCDC channel, and:
+ * sets TMDS clock ratio to 1/40 when set = 1
+ * sets TMDS clock ratio to 1/10 when set = 0
+ *
+ * Returns:
+ * True if write is successful, false otherwise.
+ */
+bool drm_scdc_set_high_tmds_clock_ratio(struct i2c_adapter *adapter, bool set)
+{
+	u8 config;
+	int ret;
+
+	ret = drm_scdc_readb(adapter, SCDC_TMDS_CONFIG, &config);
+	if (ret < 0) {
+		DRM_ERROR("Failed to read tmds config, err=%d\n", ret);
+		return false;
+	}
+
+	if (set)
+		config |= SCDC_TMDS_BIT_CLOCK_RATIO_BY_40;
+	else
+		config &= ~SCDC_TMDS_BIT_CLOCK_RATIO_BY_40;
+
+	ret = drm_scdc_writeb(adapter, SCDC_TMDS_CONFIG, config);
+	if (ret < 0) {
+		DRM_ERROR("Failed to set TMDS clock ratio, error %d\n", ret);
+		return false;
+	}
+
+	/*
+	 * The spec says that a source should wait minimum 1ms and maximum
+	 * 100ms after writing the TMDS config for clock ratio. Lets allow a
+	 * wait of upto 2ms here.
+	 */
+	usleep_range(1000, 2000);
+	return true;
+}
+EXPORT_SYMBOL(drm_scdc_set_high_tmds_clock_ratio);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index bf9d6f5..f8b766d 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -90,6 +90,20 @@ enum subpixel_order {
 
 };
 
+/**
+ * struct drm_scrambling: sink's scrambling support.
+ */
+struct drm_scrambling {
+	/**
+	 * @supported: scrambling supported for rates > 340 Mhz.
+	 */
+	bool supported;
+	/**
+	 * @low_rates: scrambling supported for rates <= 340 Mhz.
+	 */
+	bool low_rates;
+};
+
 /*
  * struct drm_scdc - Information about scdc capabilities of a HDMI 2.0 sink
  *
@@ -105,8 +119,13 @@ struct drm_scdc {
 	 * @read_request: sink is capable of generating scdc read request.
 	 */
 	bool read_request;
+	/**
+	 * @scrambling: sink's scrambling capabilities
+	 */
+	struct drm_scrambling scrambling;
 };
 
+
 /**
  * struct drm_hdmi_info - runtime information about the connected HDMI sink
  *
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 3ead84d..7b9f48b 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -476,5 +476,4 @@ void drm_edid_get_monitor_name(struct edid *edid, char *name,
 struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev,
 					   int hsize, int vsize, int fresh,
 					   bool rb);
-
 #endif /* __DRM_EDID_H__ */
diff --git a/include/drm/drm_scdc_helper.h b/include/drm/drm_scdc_helper.h
index 9c52deb..ab6bcfb 100644
--- a/include/drm/drm_scdc_helper.h
+++ b/include/drm/drm_scdc_helper.h
@@ -129,4 +129,31 @@ static inline int drm_scdc_writeb(struct i2c_adapter *adapter, u8 offset,
 	return drm_scdc_write(adapter, offset, &value, sizeof(value));
 }
 
+/**
+ * drm_scdc_set_scrambling - enable scrambling
+ * @adapter: I2C adapter for DDC channel
+ * @enable: bool to indicate if scrambling is to be enabled/disabled
+ *
+ * Writes the TMDS config register over SCDC channel, and:
+ * enables scrambling when enable = 1
+ * disables scrambling when enable = 0
+ *
+ * Returns:
+ * True if scrambling is set/reset successfully, false otherwise.
+ */
+bool drm_scdc_set_scrambling(struct i2c_adapter *adapter, bool enable);
+
+/**
+ * drm_scdc_set_high_tmds_clock_ratio - set TMDS clock ratio
+ * @adapter: I2C adapter for DDC channel
+ * @set: ret or reset the high clock ratio
+ *
+ * Writes to the TMDS config register over SCDC channel, and:
+ * sets TMDS clock ratio to 1/40 when set = 1
+ * sets TMDS clock ratio to 1/10 when set = 0
+ *
+ * Returns:
+ * True if write is successful, false otherwise.
+ */
+bool drm_scdc_set_high_tmds_clock_ratio(struct i2c_adapter *adapter, bool set);
 #endif
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v8 5/6] drm/i915: enable scrambling

[Shashank Sharma]

 Geminilake platform sports a native HDMI 2.0 controller, and is
 capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec
 mendates scrambling for these higher clocks, for reduced RF footprint.

 This patch checks if the monitor supports scrambling, and if required,
 enables it during the modeset.

 V2: Addressed review comments from Ville:
 - Do not track scrambling status in DRM layer, track somewhere in
   driver like in intel_crtc_state.
 - Don't talk to monitor at such a low layer, set monitor scrambling
   in intel_enable_ddi() before enabling the port.

 V3: Addressed review comments from Jani
 - In comments, function names, use "sink" instead of "monitor",
   so that the implementation could be close to the language of
   HDMI spec.

V4: Addressed review comment from Maarten
 - scrambling -> hdmi_scrambling
 - high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio

V5: Addressed review comments from Ville and Ander
 - Do not modifiy the crtc_state after compute_config. Move all
   scrambling and tmds_clock_ratio calcutations to compute_config.
 - While setting scrambling for source/sink, do not check the
   conditions again, just go by the crtc_state flags. This will
   simplyfy the condition checks.

V6: Addressed review comments from Ville
 - Do not add IS_GLK check in disable/enable function, instead add it
   in compute_config, while setting state flags.
 - Remove unnecessary paranthesis.
 - Simplyfy handle_sink_scrambling function as suggested.
 - Add readout code for scrambling status in get_ddi_config and add a
   check for the same in pipe_config_compare.

V7: Addressed review comments from Ander/Ville
 - No separate function for source scrambling, make it inline
 - Align the last line of the macro TRANS_DDI_HDMI_SCRAMBLING_MASK
 - Do not add platform check while setting source scrambling
 - Use pipe_config instead of crtc->config to set sink scrambling
 - To readout scrambling status, Compare with SCRAMBLING_MASK
   not any of its bits
 - Remove platform check in intel_pipe_config_compare while checking
   scrambling status

V8: Fixed mege conflict, Addressed review comments from Ander
 - Remove the desciption/comment about scrambling fom the caller, move
   it to the function
 - Move the IS_GLK check into scrambling function
 - Fix alignment

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  7 ++++
 drivers/gpu/drm/i915/intel_ddi.c     | 19 +++++++++++
 drivers/gpu/drm/i915/intel_display.c |  3 ++
 drivers/gpu/drm/i915/intel_drv.h     | 10 ++++++
 drivers/gpu/drm/i915/intel_hdmi.c    | 62 ++++++++++++++++++++++++++++++++++++
 5 files changed, 101 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cc843f9..2d50fdc 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7830,7 +7830,14 @@ enum {
 #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5<<12)
 #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6<<12)
 #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC	(1<<8)
+#define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7)
+#define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6)
 #define  TRANS_DDI_BFI_ENABLE		(1<<4)
+#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE	(1<<4)
+#define  TRANS_DDI_HDMI_SCRAMBLING	(1<<0)
+#define  TRANS_DDI_HDMI_SCRAMBLING_MASK (TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE \
+					| TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ \
+					| TRANS_DDI_HDMI_SCRAMBLING)
 
 /* DisplayPort Transport Control */
 #define _DP_TP_CTL_A			0x64040
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 0467676..b3e4c4a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1309,6 +1309,11 @@ void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state)
 			temp |= TRANS_DDI_MODE_SELECT_HDMI;
 		else
 			temp |= TRANS_DDI_MODE_SELECT_DVI;
+
+		if (crtc_state->hdmi_scrambling)
+			temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
+		if (crtc_state->hdmi_high_tmds_clock_ratio)
+			temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
 	} else if (type == INTEL_OUTPUT_ANALOG) {
 		temp |= TRANS_DDI_MODE_SELECT_FDI;
 		temp |= (crtc_state->fdi_lanes - 1) << 1;
@@ -1882,6 +1887,9 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder,
 		struct intel_digital_port *intel_dig_port =
 			enc_to_dig_port(encoder);
 
+		intel_hdmi_handle_sink_scrambling(intel_encoder,
+			conn_state->connector, pipe_config, true);
+
 		/* In HDMI/DVI mode, the port width, and swing/emphasis values
 		 * are ignored so nothing special needs to be done besides
 		 * enabling the port.
@@ -1914,6 +1922,11 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
 	if (old_crtc_state->has_audio)
 		intel_audio_codec_disable(intel_encoder);
 
+	if (type == INTEL_OUTPUT_HDMI) {
+		intel_hdmi_handle_sink_scrambling(intel_encoder,
+			old_conn_state->connector, old_crtc_state, false);
+	}
+
 	if (type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
@@ -2040,6 +2053,12 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 
 		if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
 			pipe_config->has_infoframe = true;
+
+		if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) ==
+				TRANS_DDI_HDMI_SCRAMBLING_MASK)
+			pipe_config->hdmi_scrambling = true;
+		if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
+			pipe_config->hdmi_high_tmds_clock_ratio = true;
 		/* fall through */
 	case TRANS_DDI_MODE_SELECT_DVI:
 		pipe_config->lane_count = 4;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e77ca7d..fb5e670 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11701,6 +11701,9 @@ static void __printf(3, 4)
 	if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
 	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		PIPE_CONF_CHECK_I(limited_color_range);
+
+	PIPE_CONF_CHECK_I(hdmi_scrambling);
+	PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio);
 	PIPE_CONF_CHECK_I(has_infoframe);
 
 	PIPE_CONF_CHECK_I(has_audio);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3af20c1..50d680c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -729,6 +729,12 @@ struct intel_crtc_state {
 
 	/* bitmask of visible planes (enum plane_id) */
 	u8 active_planes;
+
+	/* HDMI scrambling status */
+	bool hdmi_scrambling;
+
+	/* HDMI High TMDS char rate ratio */
+	bool hdmi_high_tmds_clock_ratio;
 };
 
 struct intel_crtc {
@@ -1623,6 +1629,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 			       struct intel_crtc_state *pipe_config,
 			       struct drm_connector_state *conn_state);
+void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
+					struct drm_connector *connector,
+					struct intel_crtc_state *config,
+					bool enable);
 void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
 
 
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index c2184f7..4585aa8 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -34,6 +34,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_edid.h>
+#include <drm/drm_scdc_helper.h>
 #include "intel_drv.h"
 #include <drm/i915_drm.h>
 #include <drm/intel_lpe_audio.h>
@@ -1316,6 +1317,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
+	struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc;
 	int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
 	int clock_12bpc = clock_8bpc * 3 / 2;
 	int desired_bpp;
@@ -1385,6 +1387,16 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 
 	pipe_config->lane_count = 4;
 
+	if (scdc->scrambling.supported && IS_GEMINILAKE(dev_priv)) {
+		if (scdc->scrambling.low_rates)
+			pipe_config->hdmi_scrambling = true;
+
+		if (pipe_config->port_clock > 340000) {
+			pipe_config->hdmi_scrambling = true;
+			pipe_config->hdmi_high_tmds_clock_ratio = true;
+		}
+	}
+
 	return true;
 }
 
@@ -1794,6 +1806,56 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
 	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
 }
 
+/*
+ * This function handles scrambling on HDMI 2.0 capable sinks.
+ * If required clock rate is > 340 Mhz && scrambling is supported by sink
+ * it enables scrambling. This should be called before enabling the HDMI
+ * 2.0 port, as the sink can choose to disable the scrambling if it doesn't
+ * detect a scrambled clock within 100 ms.
+ */
+void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
+				       struct drm_connector *connector,
+				       struct intel_crtc_state *config,
+				       bool enable)
+{
+	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base);
+	struct drm_i915_private *dev_priv = connector->dev->dev_private;
+	struct drm_scrambling *scrambling =
+				&connector->display_info.hdmi.scdc.scrambling;
+	struct i2c_adapter *adptr = intel_gmbus_get_adapter(dev_priv,
+							  intel_hdmi->ddc_bus);
+	bool ret;
+
+	if (!scrambling->supported)
+		return;
+
+	if (!IS_GEMINILAKE(dev_priv))
+		return;
+
+	DRM_DEBUG_KMS("Setting sink scrambling for enc:%s connector:%s\n",
+			intel_encoder->base.name, connector->name);
+
+	if (config->hdmi_high_tmds_clock_ratio) {
+		/* Set TMDS bit clock ratio to 1/40 or 1/10 */
+		ret = drm_scdc_set_high_tmds_clock_ratio(adptr, enable);
+		if (!ret) {
+			DRM_ERROR("Set TMDS ratio failed\n");
+			return;
+		}
+	}
+
+	if (config->hdmi_scrambling) {
+		/* Enable/disable sink scrambling */
+		ret = drm_scdc_set_scrambling(adptr, enable);
+		if (!ret) {
+			DRM_ERROR("Set sink scrambling failed\n");
+			return;
+		}
+	}
+
+	DRM_DEBUG_KMS("sink scrambling handled\n");
+}
+
 static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
 			     enum port port)
 {
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v8 6/6] drm/i915: allow HDMI 2.0 clock rates

[Shashank Sharma]

Geminilake has a native HDMI 2.0 controller, which is capable of
driving clocks upto 594Mhz. This patch updates the max tmds clock
limit for the same.

V2: rebase
V3: rebase
V4: added r-b from Ander
V5: rebase
V6: rebase
V7: rebase
V8: rebase

Cc: Ander Conselvan De Oliveira <ander.conselvan.de.oliveira@intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Reviewed-by: Ander Conselvan De Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 4585aa8..eadf06e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1209,6 +1209,8 @@ static int intel_hdmi_source_max_tmds_clock(struct drm_i915_private *dev_priv)
 {
 	if (IS_G4X(dev_priv))
 		return 165000;
+	else if (IS_GEMINILAKE(dev_priv))
+		return 594000;
 	else if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)
 		return 300000;
 	else
-- 
1.9.1

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

✗ Fi.CI.BAT: warning for HDMI 2.0: Scrambling in DRM layer (rev8)

[Patchwork]

== Series Details ==

Series: HDMI 2.0: Scrambling in DRM layer (rev8)
URL   : https://patchwork.freedesktop.org/series/19161/
State : warning

== Summary ==

Series 19161v8 HDMI 2.0: Scrambling in DRM layer
https://patchwork.freedesktop.org/api/1.0/series/19161/revisions/8/mbox/

Test gem_exec_suspend:
        Subgroup basic-s3:
                fail       -> PASS       (fi-kbl-7500u)
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-bxt-t5700)
                fail       -> DMESG-WARN (fi-kbl-7500u) fdo#100099
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                fail       -> PASS       (fi-kbl-7500u)
        Subgroup suspend-read-crc-pipe-b:
                fail       -> PASS       (fi-kbl-7500u)
        Subgroup suspend-read-crc-pipe-c:
                fail       -> PASS       (fi-kbl-7500u) fdo#100044
                pass       -> DMESG-WARN (fi-bsw-n3050) fdo#100113
Test prime_busy:
        Subgroup basic-wait-before-default:
                pass       -> DMESG-WARN (fi-bsw-n3050)

fdo#100099 https://bugs.freedesktop.org/show_bug.cgi?id=100099
fdo#100044 https://bugs.freedesktop.org/show_bug.cgi?id=100044
fdo#100113 https://bugs.freedesktop.org/show_bug.cgi?id=100113

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 473s
fi-bsw-n3050     total:278  pass:237  dwarn:2   dfail:0   fail:0   skip:39  time: 605s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 546s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 618s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 502s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 501s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 440s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 437s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 437s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 506s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 495s
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time: 476s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 512s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 599s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 500s
fi-skl-6770hq    total:53   pass:51   dwarn:0   dfail:0   fail:0   skip:1   time: 0s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 559s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time: 420s

6f8585956c95605b9c797ffde8f65e408b8d1e33 drm-tip: 2017y-03m-08d-14h-47m-28s UTC integration manifest
979d9b2 drm/i915: allow HDMI 2.0 clock rates
135919c drm/i915: enable scrambling
cce2ed9 drm/edid: detect SCDC support in HF-VSDB
86caafe drm/edid: detect SCDC support in HF-VSDB
bd6588b drm/edid: check for HF-VSDB block
225bdbb drm: Add SCDC helpers

== Logs ==

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

Re: [PATCH v8 5/6] drm/i915: enable scrambling

[Ville Syrjälä]

On Wed, Mar 08, 2017 at 07:07:48PM +0530, Shashank Sharma wrote:
>  Geminilake platform sports a native HDMI 2.0 controller, and is
>  capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec
>  mendates scrambling for these higher clocks, for reduced RF footprint.
> 
>  This patch checks if the monitor supports scrambling, and if required,
>  enables it during the modeset.
> 
>  V2: Addressed review comments from Ville:
>  - Do not track scrambling status in DRM layer, track somewhere in
>    driver like in intel_crtc_state.
>  - Don't talk to monitor at such a low layer, set monitor scrambling
>    in intel_enable_ddi() before enabling the port.
> 
>  V3: Addressed review comments from Jani
>  - In comments, function names, use "sink" instead of "monitor",
>    so that the implementation could be close to the language of
>    HDMI spec.
> 
> V4: Addressed review comment from Maarten
>  - scrambling -> hdmi_scrambling
>  - high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio
> 
> V5: Addressed review comments from Ville and Ander
>  - Do not modifiy the crtc_state after compute_config. Move all
>    scrambling and tmds_clock_ratio calcutations to compute_config.
>  - While setting scrambling for source/sink, do not check the
>    conditions again, just go by the crtc_state flags. This will
>    simplyfy the condition checks.
> 
> V6: Addressed review comments from Ville
>  - Do not add IS_GLK check in disable/enable function, instead add it
>    in compute_config, while setting state flags.
>  - Remove unnecessary paranthesis.
>  - Simplyfy handle_sink_scrambling function as suggested.
>  - Add readout code for scrambling status in get_ddi_config and add a
>    check for the same in pipe_config_compare.
> 
> V7: Addressed review comments from Ander/Ville
>  - No separate function for source scrambling, make it inline
>  - Align the last line of the macro TRANS_DDI_HDMI_SCRAMBLING_MASK
>  - Do not add platform check while setting source scrambling
>  - Use pipe_config instead of crtc->config to set sink scrambling
>  - To readout scrambling status, Compare with SCRAMBLING_MASK
>    not any of its bits
>  - Remove platform check in intel_pipe_config_compare while checking
>    scrambling status
> 
> V8: Fixed mege conflict, Addressed review comments from Ander
>  - Remove the desciption/comment about scrambling fom the caller, move
>    it to the function
>  - Move the IS_GLK check into scrambling function
>  - Fix alignment
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  7 ++++
>  drivers/gpu/drm/i915/intel_ddi.c     | 19 +++++++++++
>  drivers/gpu/drm/i915/intel_display.c |  3 ++
>  drivers/gpu/drm/i915/intel_drv.h     | 10 ++++++
>  drivers/gpu/drm/i915/intel_hdmi.c    | 62 ++++++++++++++++++++++++++++++++++++
>  5 files changed, 101 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index cc843f9..2d50fdc 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7830,7 +7830,14 @@ enum {
>  #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5<<12)
>  #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6<<12)
>  #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC	(1<<8)
> +#define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7)
> +#define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6)
>  #define  TRANS_DDI_BFI_ENABLE		(1<<4)
> +#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE	(1<<4)
> +#define  TRANS_DDI_HDMI_SCRAMBLING	(1<<0)
> +#define  TRANS_DDI_HDMI_SCRAMBLING_MASK (TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE \
> +					| TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ \
> +					| TRANS_DDI_HDMI_SCRAMBLING)
>  
>  /* DisplayPort Transport Control */
>  #define _DP_TP_CTL_A			0x64040
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 0467676..b3e4c4a 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1309,6 +1309,11 @@ void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state)
>  			temp |= TRANS_DDI_MODE_SELECT_HDMI;
>  		else
>  			temp |= TRANS_DDI_MODE_SELECT_DVI;
> +
> +		if (crtc_state->hdmi_scrambling)
> +			temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
> +		if (crtc_state->hdmi_high_tmds_clock_ratio)
> +			temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
>  	} else if (type == INTEL_OUTPUT_ANALOG) {
>  		temp |= TRANS_DDI_MODE_SELECT_FDI;
>  		temp |= (crtc_state->fdi_lanes - 1) << 1;
> @@ -1882,6 +1887,9 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder,
>  		struct intel_digital_port *intel_dig_port =
>  			enc_to_dig_port(encoder);
>  
> +		intel_hdmi_handle_sink_scrambling(intel_encoder,
> +			conn_state->connector, pipe_config, true);
> +
>  		/* In HDMI/DVI mode, the port width, and swing/emphasis values
>  		 * are ignored so nothing special needs to be done besides
>  		 * enabling the port.
> @@ -1914,6 +1922,11 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
>  	if (old_crtc_state->has_audio)
>  		intel_audio_codec_disable(intel_encoder);
>  
> +	if (type == INTEL_OUTPUT_HDMI) {
> +		intel_hdmi_handle_sink_scrambling(intel_encoder,
> +			old_conn_state->connector, old_crtc_state, false);
> +	}
> +
>  	if (type == INTEL_OUTPUT_EDP) {
>  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  
> @@ -2040,6 +2053,12 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  
>  		if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
>  			pipe_config->has_infoframe = true;
> +
> +		if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) ==
> +				TRANS_DDI_HDMI_SCRAMBLING_MASK)
> +			pipe_config->hdmi_scrambling = true;
> +		if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
> +			pipe_config->hdmi_high_tmds_clock_ratio = true;
>  		/* fall through */
>  	case TRANS_DDI_MODE_SELECT_DVI:
>  		pipe_config->lane_count = 4;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e77ca7d..fb5e670 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11701,6 +11701,9 @@ static void __printf(3, 4)
>  	if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
>  	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>  		PIPE_CONF_CHECK_I(limited_color_range);
> +
> +	PIPE_CONF_CHECK_I(hdmi_scrambling);
> +	PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio);
>  	PIPE_CONF_CHECK_I(has_infoframe);
>  
>  	PIPE_CONF_CHECK_I(has_audio);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3af20c1..50d680c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -729,6 +729,12 @@ struct intel_crtc_state {
>  
>  	/* bitmask of visible planes (enum plane_id) */
>  	u8 active_planes;
> +
> +	/* HDMI scrambling status */
> +	bool hdmi_scrambling;
> +
> +	/* HDMI High TMDS char rate ratio */
> +	bool hdmi_high_tmds_clock_ratio;
>  };
>  
>  struct intel_crtc {
> @@ -1623,6 +1629,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  			       struct intel_crtc_state *pipe_config,
>  			       struct drm_connector_state *conn_state);
> +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
> +					struct drm_connector *connector,
> +					struct intel_crtc_state *config,
> +					bool enable);
>  void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
>  
>  
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index c2184f7..4585aa8 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -34,6 +34,7 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_edid.h>
> +#include <drm/drm_scdc_helper.h>
>  #include "intel_drv.h"
>  #include <drm/i915_drm.h>
>  #include <drm/intel_lpe_audio.h>
> @@ -1316,6 +1317,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> +	struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc;
>  	int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
>  	int clock_12bpc = clock_8bpc * 3 / 2;
>  	int desired_bpp;
> @@ -1385,6 +1387,16 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  
>  	pipe_config->lane_count = 4;
>  
> +	if (scdc->scrambling.supported && IS_GEMINILAKE(dev_priv)) {
> +		if (scdc->scrambling.low_rates)
> +			pipe_config->hdmi_scrambling = true;
> +
> +		if (pipe_config->port_clock > 340000) {
> +			pipe_config->hdmi_scrambling = true;
> +			pipe_config->hdmi_high_tmds_clock_ratio = true;
> +		}
> +	}
> +
>  	return true;
>  }
>  
> @@ -1794,6 +1806,56 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
>  	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>  }
>  
> +/*
> + * This function handles scrambling on HDMI 2.0 capable sinks.
> + * If required clock rate is > 340 Mhz && scrambling is supported by sink
> + * it enables scrambling. This should be called before enabling the HDMI
> + * 2.0 port, as the sink can choose to disable the scrambling if it doesn't
> + * detect a scrambled clock within 100 ms.
> + */
> +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
> +				       struct drm_connector *connector,
> +				       struct intel_crtc_state *config,
> +				       bool enable)
> +{
> +	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base);
> +	struct drm_i915_private *dev_priv = connector->dev->dev_private;
> +	struct drm_scrambling *scrambling =
> +				&connector->display_info.hdmi.scdc.scrambling;
> +	struct i2c_adapter *adptr = intel_gmbus_get_adapter(dev_priv,
> +							  intel_hdmi->ddc_bus);
> +	bool ret;
> +
> +	if (!scrambling->supported)
> +		return;
> +
> +	if (!IS_GEMINILAKE(dev_priv))
> +		return;

I'd still like to see this getting done uncoditionally. That way we
might get a bit of extra testing for the SCDC helper no matter what
platform the user happens to own.

I presume the spec says the sink will reset these to some default value
on HPD or something like that? But I'm not sure trusting them to get
this right is a good idea because reality often doesn't match the
specs. So the safer option IMO would be always tell the sink exactly
what we want.

> +
> +	DRM_DEBUG_KMS("Setting sink scrambling for enc:%s connector:%s\n",
> +			intel_encoder->base.name, connector->name);
> +
> +	if (config->hdmi_high_tmds_clock_ratio) {
> +		/* Set TMDS bit clock ratio to 1/40 or 1/10 */
> +		ret = drm_scdc_set_high_tmds_clock_ratio(adptr, enable);
> +		if (!ret) {
> +			DRM_ERROR("Set TMDS ratio failed\n");
> +			return;
> +		}
> +	}
> +
> +	if (config->hdmi_scrambling) {
> +		/* Enable/disable sink scrambling */
> +		ret = drm_scdc_set_scrambling(adptr, enable);
> +		if (!ret) {
> +			DRM_ERROR("Set sink scrambling failed\n");
> +			return;
> +		}
> +	}
> +
> +	DRM_DEBUG_KMS("sink scrambling handled\n");
> +}
> +
>  static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
>  			     enum port port)
>  {
> -- 
> 1.9.1

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v8 5/6] drm/i915: enable scrambling

[Sharma, Shashank]

Regards

Shashank


On 3/8/2017 7:17 PM, Ville Syrjälä wrote:
> On Wed, Mar 08, 2017 at 07:07:48PM +0530, Shashank Sharma wrote:
>>   Geminilake platform sports a native HDMI 2.0 controller, and is
>>   capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec
>>   mendates scrambling for these higher clocks, for reduced RF footprint.
>>
>>   This patch checks if the monitor supports scrambling, and if required,
>>   enables it during the modeset.
>>
>>   V2: Addressed review comments from Ville:
>>   - Do not track scrambling status in DRM layer, track somewhere in
>>     driver like in intel_crtc_state.
>>   - Don't talk to monitor at such a low layer, set monitor scrambling
>>     in intel_enable_ddi() before enabling the port.
>>
>>   V3: Addressed review comments from Jani
>>   - In comments, function names, use "sink" instead of "monitor",
>>     so that the implementation could be close to the language of
>>     HDMI spec.
>>
>> V4: Addressed review comment from Maarten
>>   - scrambling -> hdmi_scrambling
>>   - high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio
>>
>> V5: Addressed review comments from Ville and Ander
>>   - Do not modifiy the crtc_state after compute_config. Move all
>>     scrambling and tmds_clock_ratio calcutations to compute_config.
>>   - While setting scrambling for source/sink, do not check the
>>     conditions again, just go by the crtc_state flags. This will
>>     simplyfy the condition checks.
>>
>> V6: Addressed review comments from Ville
>>   - Do not add IS_GLK check in disable/enable function, instead add it
>>     in compute_config, while setting state flags.
>>   - Remove unnecessary paranthesis.
>>   - Simplyfy handle_sink_scrambling function as suggested.
>>   - Add readout code for scrambling status in get_ddi_config and add a
>>     check for the same in pipe_config_compare.
>>
>> V7: Addressed review comments from Ander/Ville
>>   - No separate function for source scrambling, make it inline
>>   - Align the last line of the macro TRANS_DDI_HDMI_SCRAMBLING_MASK
>>   - Do not add platform check while setting source scrambling
>>   - Use pipe_config instead of crtc->config to set sink scrambling
>>   - To readout scrambling status, Compare with SCRAMBLING_MASK
>>     not any of its bits
>>   - Remove platform check in intel_pipe_config_compare while checking
>>     scrambling status
>>
>> V8: Fixed mege conflict, Addressed review comments from Ander
>>   - Remove the desciption/comment about scrambling fom the caller, move
>>     it to the function
>>   - Move the IS_GLK check into scrambling function
>>   - Fix alignment
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h      |  7 ++++
>>   drivers/gpu/drm/i915/intel_ddi.c     | 19 +++++++++++
>>   drivers/gpu/drm/i915/intel_display.c |  3 ++
>>   drivers/gpu/drm/i915/intel_drv.h     | 10 ++++++
>>   drivers/gpu/drm/i915/intel_hdmi.c    | 62 ++++++++++++++++++++++++++++++++++++
>>   5 files changed, 101 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index cc843f9..2d50fdc 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7830,7 +7830,14 @@ enum {
>>   #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5<<12)
>>   #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6<<12)
>>   #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC	(1<<8)
>> +#define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7)
>> +#define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6)
>>   #define  TRANS_DDI_BFI_ENABLE		(1<<4)
>> +#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE	(1<<4)
>> +#define  TRANS_DDI_HDMI_SCRAMBLING	(1<<0)
>> +#define  TRANS_DDI_HDMI_SCRAMBLING_MASK (TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE \
>> +					| TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ \
>> +					| TRANS_DDI_HDMI_SCRAMBLING)
>>   
>>   /* DisplayPort Transport Control */
>>   #define _DP_TP_CTL_A			0x64040
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index 0467676..b3e4c4a 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -1309,6 +1309,11 @@ void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state)
>>   			temp |= TRANS_DDI_MODE_SELECT_HDMI;
>>   		else
>>   			temp |= TRANS_DDI_MODE_SELECT_DVI;
>> +
>> +		if (crtc_state->hdmi_scrambling)
>> +			temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
>> +		if (crtc_state->hdmi_high_tmds_clock_ratio)
>> +			temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
>>   	} else if (type == INTEL_OUTPUT_ANALOG) {
>>   		temp |= TRANS_DDI_MODE_SELECT_FDI;
>>   		temp |= (crtc_state->fdi_lanes - 1) << 1;
>> @@ -1882,6 +1887,9 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder,
>>   		struct intel_digital_port *intel_dig_port =
>>   			enc_to_dig_port(encoder);
>>   
>> +		intel_hdmi_handle_sink_scrambling(intel_encoder,
>> +			conn_state->connector, pipe_config, true);
>> +
>>   		/* In HDMI/DVI mode, the port width, and swing/emphasis values
>>   		 * are ignored so nothing special needs to be done besides
>>   		 * enabling the port.
>> @@ -1914,6 +1922,11 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
>>   	if (old_crtc_state->has_audio)
>>   		intel_audio_codec_disable(intel_encoder);
>>   
>> +	if (type == INTEL_OUTPUT_HDMI) {
>> +		intel_hdmi_handle_sink_scrambling(intel_encoder,
>> +			old_conn_state->connector, old_crtc_state, false);
>> +	}
>> +
>>   	if (type == INTEL_OUTPUT_EDP) {
>>   		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>>   
>> @@ -2040,6 +2053,12 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>>   
>>   		if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
>>   			pipe_config->has_infoframe = true;
>> +
>> +		if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) ==
>> +				TRANS_DDI_HDMI_SCRAMBLING_MASK)
>> +			pipe_config->hdmi_scrambling = true;
>> +		if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
>> +			pipe_config->hdmi_high_tmds_clock_ratio = true;
>>   		/* fall through */
>>   	case TRANS_DDI_MODE_SELECT_DVI:
>>   		pipe_config->lane_count = 4;
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index e77ca7d..fb5e670 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -11701,6 +11701,9 @@ static void __printf(3, 4)
>>   	if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
>>   	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>>   		PIPE_CONF_CHECK_I(limited_color_range);
>> +
>> +	PIPE_CONF_CHECK_I(hdmi_scrambling);
>> +	PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio);
>>   	PIPE_CONF_CHECK_I(has_infoframe);
>>   
>>   	PIPE_CONF_CHECK_I(has_audio);
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 3af20c1..50d680c 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -729,6 +729,12 @@ struct intel_crtc_state {
>>   
>>   	/* bitmask of visible planes (enum plane_id) */
>>   	u8 active_planes;
>> +
>> +	/* HDMI scrambling status */
>> +	bool hdmi_scrambling;
>> +
>> +	/* HDMI High TMDS char rate ratio */
>> +	bool hdmi_high_tmds_clock_ratio;
>>   };
>>   
>>   struct intel_crtc {
>> @@ -1623,6 +1629,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>>   bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>>   			       struct intel_crtc_state *pipe_config,
>>   			       struct drm_connector_state *conn_state);
>> +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
>> +					struct drm_connector *connector,
>> +					struct intel_crtc_state *config,
>> +					bool enable);
>>   void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
>>   
>>   
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index c2184f7..4585aa8 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -34,6 +34,7 @@
>>   #include <drm/drm_atomic_helper.h>
>>   #include <drm/drm_crtc.h>
>>   #include <drm/drm_edid.h>
>> +#include <drm/drm_scdc_helper.h>
>>   #include "intel_drv.h"
>>   #include <drm/i915_drm.h>
>>   #include <drm/intel_lpe_audio.h>
>> @@ -1316,6 +1317,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>>   	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>>   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>   	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>> +	struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc;
>>   	int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
>>   	int clock_12bpc = clock_8bpc * 3 / 2;
>>   	int desired_bpp;
>> @@ -1385,6 +1387,16 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>>   
>>   	pipe_config->lane_count = 4;
>>   
>> +	if (scdc->scrambling.supported && IS_GEMINILAKE(dev_priv)) {
>> +		if (scdc->scrambling.low_rates)
>> +			pipe_config->hdmi_scrambling = true;
>> +
>> +		if (pipe_config->port_clock > 340000) {
>> +			pipe_config->hdmi_scrambling = true;
>> +			pipe_config->hdmi_high_tmds_clock_ratio = true;
>> +		}
>> +	}
>> +
>>   	return true;
>>   }
>>   
>> @@ -1794,6 +1806,56 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
>>   	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>>   }
>>   
>> +/*
>> + * This function handles scrambling on HDMI 2.0 capable sinks.
>> + * If required clock rate is > 340 Mhz && scrambling is supported by sink
>> + * it enables scrambling. This should be called before enabling the HDMI
>> + * 2.0 port, as the sink can choose to disable the scrambling if it doesn't
>> + * detect a scrambled clock within 100 ms.
>> + */
>> +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
>> +				       struct drm_connector *connector,
>> +				       struct intel_crtc_state *config,
>> +				       bool enable)
>> +{
>> +	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base);
>> +	struct drm_i915_private *dev_priv = connector->dev->dev_private;
>> +	struct drm_scrambling *scrambling =
>> +				&connector->display_info.hdmi.scdc.scrambling;
>> +	struct i2c_adapter *adptr = intel_gmbus_get_adapter(dev_priv,
>> +							  intel_hdmi->ddc_bus);
>> +	bool ret;
>> +
>> +	if (!scrambling->supported)
>> +		return;
>> +
>> +	if (!IS_GEMINILAKE(dev_priv))
>> +		return;
> I'd still like to see this getting done uncoditionally. That way we
> might get a bit of extra testing for the SCDC helper no matter what
> platform the user happens to own.
Sure, seems a good idea. I just added this hear to address the review 
comment from Ander.
Also, I can move this IS_GLK condition to compute_config, and once we 
check this in compute_config
we might never have check it again.
> I presume the spec says the sink will reset these to some default value
> on HPD or something like that? But I'm not sure trusting them to get
> this right is a good idea because reality often doesn't match the
> specs. So the safer option IMO would be always tell the sink exactly
> what we want.
Yep, You are correct about the spec too, that it says if there is no 
scrambled clock post hpd_out for
100ms, the sink should disable scrambling. But connected modeset 
scenarios need our attention, where
for example we are switching mode from, 4k@60 (scrambling reqd) -> 
1080@60 (scrambling not reqd).
For such cases too, we have to tell sink to disable scrambling.
>
>> +
>> +	DRM_DEBUG_KMS("Setting sink scrambling for enc:%s connector:%s\n",
>> +			intel_encoder->base.name, connector->name);
>> +
>> +	if (config->hdmi_high_tmds_clock_ratio) {
>> +		/* Set TMDS bit clock ratio to 1/40 or 1/10 */
>> +		ret = drm_scdc_set_high_tmds_clock_ratio(adptr, enable);
>> +		if (!ret) {
>> +			DRM_ERROR("Set TMDS ratio failed\n");
>> +			return;
>> +		}
>> +	}
>> +
>> +	if (config->hdmi_scrambling) {
>> +		/* Enable/disable sink scrambling */
>> +		ret = drm_scdc_set_scrambling(adptr, enable);
>> +		if (!ret) {
>> +			DRM_ERROR("Set sink scrambling failed\n");
>> +			return;
>> +		}
>> +	}
>> +
>> +	DRM_DEBUG_KMS("sink scrambling handled\n");
>> +}
>> +
>>   static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
>>   			     enum port port)
>>   {
>> -- 
>> 1.9.1

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

Re: [PATCH v8 5/6] drm/i915: enable scrambling

[Ville Syrjälä]

On Wed, Mar 08, 2017 at 07:26:45PM +0200, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 3/8/2017 7:17 PM, Ville Syrjälä wrote:
> > On Wed, Mar 08, 2017 at 07:07:48PM +0530, Shashank Sharma wrote:
> >>   Geminilake platform sports a native HDMI 2.0 controller, and is
> >>   capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec
> >>   mendates scrambling for these higher clocks, for reduced RF footprint.
> >>
> >>   This patch checks if the monitor supports scrambling, and if required,
> >>   enables it during the modeset.
> >>
> >>   V2: Addressed review comments from Ville:
> >>   - Do not track scrambling status in DRM layer, track somewhere in
> >>     driver like in intel_crtc_state.
> >>   - Don't talk to monitor at such a low layer, set monitor scrambling
> >>     in intel_enable_ddi() before enabling the port.
> >>
> >>   V3: Addressed review comments from Jani
> >>   - In comments, function names, use "sink" instead of "monitor",
> >>     so that the implementation could be close to the language of
> >>     HDMI spec.
> >>
> >> V4: Addressed review comment from Maarten
> >>   - scrambling -> hdmi_scrambling
> >>   - high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio
> >>
> >> V5: Addressed review comments from Ville and Ander
> >>   - Do not modifiy the crtc_state after compute_config. Move all
> >>     scrambling and tmds_clock_ratio calcutations to compute_config.
> >>   - While setting scrambling for source/sink, do not check the
> >>     conditions again, just go by the crtc_state flags. This will
> >>     simplyfy the condition checks.
> >>
> >> V6: Addressed review comments from Ville
> >>   - Do not add IS_GLK check in disable/enable function, instead add it
> >>     in compute_config, while setting state flags.
> >>   - Remove unnecessary paranthesis.
> >>   - Simplyfy handle_sink_scrambling function as suggested.
> >>   - Add readout code for scrambling status in get_ddi_config and add a
> >>     check for the same in pipe_config_compare.
> >>
> >> V7: Addressed review comments from Ander/Ville
> >>   - No separate function for source scrambling, make it inline
> >>   - Align the last line of the macro TRANS_DDI_HDMI_SCRAMBLING_MASK
> >>   - Do not add platform check while setting source scrambling
> >>   - Use pipe_config instead of crtc->config to set sink scrambling
> >>   - To readout scrambling status, Compare with SCRAMBLING_MASK
> >>     not any of its bits
> >>   - Remove platform check in intel_pipe_config_compare while checking
> >>     scrambling status
> >>
> >> V8: Fixed mege conflict, Addressed review comments from Ander
> >>   - Remove the desciption/comment about scrambling fom the caller, move
> >>     it to the function
> >>   - Move the IS_GLK check into scrambling function
> >>   - Fix alignment
> >>
> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/i915_reg.h      |  7 ++++
> >>   drivers/gpu/drm/i915/intel_ddi.c     | 19 +++++++++++
> >>   drivers/gpu/drm/i915/intel_display.c |  3 ++
> >>   drivers/gpu/drm/i915/intel_drv.h     | 10 ++++++
> >>   drivers/gpu/drm/i915/intel_hdmi.c    | 62 ++++++++++++++++++++++++++++++++++++
> >>   5 files changed, 101 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >> index cc843f9..2d50fdc 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -7830,7 +7830,14 @@ enum {
> >>   #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5<<12)
> >>   #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6<<12)
> >>   #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC	(1<<8)
> >> +#define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7)
> >> +#define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6)
> >>   #define  TRANS_DDI_BFI_ENABLE		(1<<4)
> >> +#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE	(1<<4)
> >> +#define  TRANS_DDI_HDMI_SCRAMBLING	(1<<0)
> >> +#define  TRANS_DDI_HDMI_SCRAMBLING_MASK (TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE \
> >> +					| TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ \
> >> +					| TRANS_DDI_HDMI_SCRAMBLING)
> >>   
> >>   /* DisplayPort Transport Control */
> >>   #define _DP_TP_CTL_A			0x64040
> >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >> index 0467676..b3e4c4a 100644
> >> --- a/drivers/gpu/drm/i915/intel_ddi.c
> >> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >> @@ -1309,6 +1309,11 @@ void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state)
> >>   			temp |= TRANS_DDI_MODE_SELECT_HDMI;
> >>   		else
> >>   			temp |= TRANS_DDI_MODE_SELECT_DVI;
> >> +
> >> +		if (crtc_state->hdmi_scrambling)
> >> +			temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
> >> +		if (crtc_state->hdmi_high_tmds_clock_ratio)
> >> +			temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
> >>   	} else if (type == INTEL_OUTPUT_ANALOG) {
> >>   		temp |= TRANS_DDI_MODE_SELECT_FDI;
> >>   		temp |= (crtc_state->fdi_lanes - 1) << 1;
> >> @@ -1882,6 +1887,9 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder,
> >>   		struct intel_digital_port *intel_dig_port =
> >>   			enc_to_dig_port(encoder);
> >>   
> >> +		intel_hdmi_handle_sink_scrambling(intel_encoder,
> >> +			conn_state->connector, pipe_config, true);
> >> +
> >>   		/* In HDMI/DVI mode, the port width, and swing/emphasis values
> >>   		 * are ignored so nothing special needs to be done besides
> >>   		 * enabling the port.
> >> @@ -1914,6 +1922,11 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
> >>   	if (old_crtc_state->has_audio)
> >>   		intel_audio_codec_disable(intel_encoder);
> >>   
> >> +	if (type == INTEL_OUTPUT_HDMI) {
> >> +		intel_hdmi_handle_sink_scrambling(intel_encoder,
> >> +			old_conn_state->connector, old_crtc_state, false);
> >> +	}
> >> +
> >>   	if (type == INTEL_OUTPUT_EDP) {
> >>   		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> >>   
> >> @@ -2040,6 +2053,12 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >>   
> >>   		if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
> >>   			pipe_config->has_infoframe = true;
> >> +
> >> +		if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) ==
> >> +				TRANS_DDI_HDMI_SCRAMBLING_MASK)
> >> +			pipe_config->hdmi_scrambling = true;
> >> +		if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
> >> +			pipe_config->hdmi_high_tmds_clock_ratio = true;
> >>   		/* fall through */
> >>   	case TRANS_DDI_MODE_SELECT_DVI:
> >>   		pipe_config->lane_count = 4;
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index e77ca7d..fb5e670 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -11701,6 +11701,9 @@ static void __printf(3, 4)
> >>   	if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
> >>   	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> >>   		PIPE_CONF_CHECK_I(limited_color_range);
> >> +
> >> +	PIPE_CONF_CHECK_I(hdmi_scrambling);
> >> +	PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio);
> >>   	PIPE_CONF_CHECK_I(has_infoframe);
> >>   
> >>   	PIPE_CONF_CHECK_I(has_audio);
> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >> index 3af20c1..50d680c 100644
> >> --- a/drivers/gpu/drm/i915/intel_drv.h
> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> @@ -729,6 +729,12 @@ struct intel_crtc_state {
> >>   
> >>   	/* bitmask of visible planes (enum plane_id) */
> >>   	u8 active_planes;
> >> +
> >> +	/* HDMI scrambling status */
> >> +	bool hdmi_scrambling;
> >> +
> >> +	/* HDMI High TMDS char rate ratio */
> >> +	bool hdmi_high_tmds_clock_ratio;
> >>   };
> >>   
> >>   struct intel_crtc {
> >> @@ -1623,6 +1629,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >>   bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> >>   			       struct intel_crtc_state *pipe_config,
> >>   			       struct drm_connector_state *conn_state);
> >> +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
> >> +					struct drm_connector *connector,
> >> +					struct intel_crtc_state *config,
> >> +					bool enable);
> >>   void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
> >>   
> >>   
> >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> >> index c2184f7..4585aa8 100644
> >> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >> @@ -34,6 +34,7 @@
> >>   #include <drm/drm_atomic_helper.h>
> >>   #include <drm/drm_crtc.h>
> >>   #include <drm/drm_edid.h>
> >> +#include <drm/drm_scdc_helper.h>
> >>   #include "intel_drv.h"
> >>   #include <drm/i915_drm.h>
> >>   #include <drm/intel_lpe_audio.h>
> >> @@ -1316,6 +1317,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> >>   	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> >>   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >>   	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> >> +	struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc;
> >>   	int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
> >>   	int clock_12bpc = clock_8bpc * 3 / 2;
> >>   	int desired_bpp;
> >> @@ -1385,6 +1387,16 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> >>   
> >>   	pipe_config->lane_count = 4;
> >>   
> >> +	if (scdc->scrambling.supported && IS_GEMINILAKE(dev_priv)) {
> >> +		if (scdc->scrambling.low_rates)
> >> +			pipe_config->hdmi_scrambling = true;
> >> +
> >> +		if (pipe_config->port_clock > 340000) {
> >> +			pipe_config->hdmi_scrambling = true;
> >> +			pipe_config->hdmi_high_tmds_clock_ratio = true;
> >> +		}
> >> +	}
> >> +
> >>   	return true;
> >>   }
> >>   
> >> @@ -1794,6 +1806,56 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
> >>   	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> >>   }
> >>   
> >> +/*
> >> + * This function handles scrambling on HDMI 2.0 capable sinks.
> >> + * If required clock rate is > 340 Mhz && scrambling is supported by sink
> >> + * it enables scrambling. This should be called before enabling the HDMI
> >> + * 2.0 port, as the sink can choose to disable the scrambling if it doesn't
> >> + * detect a scrambled clock within 100 ms.
> >> + */
> >> +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
> >> +				       struct drm_connector *connector,
> >> +				       struct intel_crtc_state *config,
> >> +				       bool enable)
> >> +{
> >> +	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base);
> >> +	struct drm_i915_private *dev_priv = connector->dev->dev_private;
> >> +	struct drm_scrambling *scrambling =
> >> +				&connector->display_info.hdmi.scdc.scrambling;
> >> +	struct i2c_adapter *adptr = intel_gmbus_get_adapter(dev_priv,
> >> +							  intel_hdmi->ddc_bus);
> >> +	bool ret;
> >> +
> >> +	if (!scrambling->supported)
> >> +		return;
> >> +
> >> +	if (!IS_GEMINILAKE(dev_priv))
> >> +		return;
> > I'd still like to see this getting done uncoditionally. That way we
> > might get a bit of extra testing for the SCDC helper no matter what
> > platform the user happens to own.
> Sure, seems a good idea. I just added this hear to address the review 
> comment from Ander.
> Also, I can move this IS_GLK condition to compute_config, and once we 
> check this in compute_config
> we might never have check it again.

I thought you already had the check in there? Hmm. Looks that way to me.

> > I presume the spec says the sink will reset these to some default value
> > on HPD or something like that? But I'm not sure trusting them to get
> > this right is a good idea because reality often doesn't match the
> > specs. So the safer option IMO would be always tell the sink exactly
> > what we want.
> Yep, You are correct about the spec too, that it says if there is no 
> scrambled clock post hpd_out for
> 100ms, the sink should disable scrambling. But connected modeset 
> scenarios need our attention, where
> for example we are switching mode from, 4k@60 (scrambling reqd) -> 
> 1080@60 (scrambling not reqd).
> For such cases too, we have to tell sink to disable scrambling.
> >
> >> +
> >> +	DRM_DEBUG_KMS("Setting sink scrambling for enc:%s connector:%s\n",
> >> +			intel_encoder->base.name, connector->name);
> >> +
> >> +	if (config->hdmi_high_tmds_clock_ratio) {
> >> +		/* Set TMDS bit clock ratio to 1/40 or 1/10 */
> >> +		ret = drm_scdc_set_high_tmds_clock_ratio(adptr, enable);
> >> +		if (!ret) {
> >> +			DRM_ERROR("Set TMDS ratio failed\n");
> >> +			return;
> >> +		}
> >> +	}
> >> +
> >> +	if (config->hdmi_scrambling) {
> >> +		/* Enable/disable sink scrambling */
> >> +		ret = drm_scdc_set_scrambling(adptr, enable);
> >> +		if (!ret) {
> >> +			DRM_ERROR("Set sink scrambling failed\n");
> >> +			return;
> >> +		}
> >> +	}
> >> +
> >> +	DRM_DEBUG_KMS("sink scrambling handled\n");
> >> +}
> >> +
> >>   static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
> >>   			     enum port port)
> >>   {
> >> -- 
> >> 1.9.1

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v8 5/6] drm/i915: enable scrambling

[Sharma, Shashank]

Regards

Shashank


On 3/8/2017 7:44 PM, Ville Syrjälä wrote:
> On Wed, Mar 08, 2017 at 07:26:45PM +0200, Sharma, Shashank wrote:
>> Regards
>>
>> Shashank
>>
>>
>> On 3/8/2017 7:17 PM, Ville Syrjälä wrote:
>>> On Wed, Mar 08, 2017 at 07:07:48PM +0530, Shashank Sharma wrote:
>>>>    Geminilake platform sports a native HDMI 2.0 controller, and is
>>>>    capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec
>>>>    mendates scrambling for these higher clocks, for reduced RF footprint.
>>>>
>>>>    This patch checks if the monitor supports scrambling, and if required,
>>>>    enables it during the modeset.
>>>>
>>>>    V2: Addressed review comments from Ville:
>>>>    - Do not track scrambling status in DRM layer, track somewhere in
>>>>      driver like in intel_crtc_state.
>>>>    - Don't talk to monitor at such a low layer, set monitor scrambling
>>>>      in intel_enable_ddi() before enabling the port.
>>>>
>>>>    V3: Addressed review comments from Jani
>>>>    - In comments, function names, use "sink" instead of "monitor",
>>>>      so that the implementation could be close to the language of
>>>>      HDMI spec.
>>>>
>>>> V4: Addressed review comment from Maarten
>>>>    - scrambling -> hdmi_scrambling
>>>>    - high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio
>>>>
>>>> V5: Addressed review comments from Ville and Ander
>>>>    - Do not modifiy the crtc_state after compute_config. Move all
>>>>      scrambling and tmds_clock_ratio calcutations to compute_config.
>>>>    - While setting scrambling for source/sink, do not check the
>>>>      conditions again, just go by the crtc_state flags. This will
>>>>      simplyfy the condition checks.
>>>>
>>>> V6: Addressed review comments from Ville
>>>>    - Do not add IS_GLK check in disable/enable function, instead add it
>>>>      in compute_config, while setting state flags.
>>>>    - Remove unnecessary paranthesis.
>>>>    - Simplyfy handle_sink_scrambling function as suggested.
>>>>    - Add readout code for scrambling status in get_ddi_config and add a
>>>>      check for the same in pipe_config_compare.
>>>>
>>>> V7: Addressed review comments from Ander/Ville
>>>>    - No separate function for source scrambling, make it inline
>>>>    - Align the last line of the macro TRANS_DDI_HDMI_SCRAMBLING_MASK
>>>>    - Do not add platform check while setting source scrambling
>>>>    - Use pipe_config instead of crtc->config to set sink scrambling
>>>>    - To readout scrambling status, Compare with SCRAMBLING_MASK
>>>>      not any of its bits
>>>>    - Remove platform check in intel_pipe_config_compare while checking
>>>>      scrambling status
>>>>
>>>> V8: Fixed mege conflict, Addressed review comments from Ander
>>>>    - Remove the desciption/comment about scrambling fom the caller, move
>>>>      it to the function
>>>>    - Move the IS_GLK check into scrambling function
>>>>    - Fix alignment
>>>>
>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/i915_reg.h      |  7 ++++
>>>>    drivers/gpu/drm/i915/intel_ddi.c     | 19 +++++++++++
>>>>    drivers/gpu/drm/i915/intel_display.c |  3 ++
>>>>    drivers/gpu/drm/i915/intel_drv.h     | 10 ++++++
>>>>    drivers/gpu/drm/i915/intel_hdmi.c    | 62 ++++++++++++++++++++++++++++++++++++
>>>>    5 files changed, 101 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>>> index cc843f9..2d50fdc 100644
>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>>> @@ -7830,7 +7830,14 @@ enum {
>>>>    #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5<<12)
>>>>    #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6<<12)
>>>>    #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC	(1<<8)
>>>> +#define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7)
>>>> +#define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6)
>>>>    #define  TRANS_DDI_BFI_ENABLE		(1<<4)
>>>> +#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE	(1<<4)
>>>> +#define  TRANS_DDI_HDMI_SCRAMBLING	(1<<0)
>>>> +#define  TRANS_DDI_HDMI_SCRAMBLING_MASK (TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE \
>>>> +					| TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ \
>>>> +					| TRANS_DDI_HDMI_SCRAMBLING)
>>>>    
>>>>    /* DisplayPort Transport Control */
>>>>    #define _DP_TP_CTL_A			0x64040
>>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>>>> index 0467676..b3e4c4a 100644
>>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>>> @@ -1309,6 +1309,11 @@ void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state)
>>>>    			temp |= TRANS_DDI_MODE_SELECT_HDMI;
>>>>    		else
>>>>    			temp |= TRANS_DDI_MODE_SELECT_DVI;
>>>> +
>>>> +		if (crtc_state->hdmi_scrambling)
>>>> +			temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
>>>> +		if (crtc_state->hdmi_high_tmds_clock_ratio)
>>>> +			temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
>>>>    	} else if (type == INTEL_OUTPUT_ANALOG) {
>>>>    		temp |= TRANS_DDI_MODE_SELECT_FDI;
>>>>    		temp |= (crtc_state->fdi_lanes - 1) << 1;
>>>> @@ -1882,6 +1887,9 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder,
>>>>    		struct intel_digital_port *intel_dig_port =
>>>>    			enc_to_dig_port(encoder);
>>>>    
>>>> +		intel_hdmi_handle_sink_scrambling(intel_encoder,
>>>> +			conn_state->connector, pipe_config, true);
>>>> +
>>>>    		/* In HDMI/DVI mode, the port width, and swing/emphasis values
>>>>    		 * are ignored so nothing special needs to be done besides
>>>>    		 * enabling the port.
>>>> @@ -1914,6 +1922,11 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
>>>>    	if (old_crtc_state->has_audio)
>>>>    		intel_audio_codec_disable(intel_encoder);
>>>>    
>>>> +	if (type == INTEL_OUTPUT_HDMI) {
>>>> +		intel_hdmi_handle_sink_scrambling(intel_encoder,
>>>> +			old_conn_state->connector, old_crtc_state, false);
>>>> +	}
>>>> +
>>>>    	if (type == INTEL_OUTPUT_EDP) {
>>>>    		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>>>>    
>>>> @@ -2040,6 +2053,12 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>>>>    
>>>>    		if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
>>>>    			pipe_config->has_infoframe = true;
>>>> +
>>>> +		if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) ==
>>>> +				TRANS_DDI_HDMI_SCRAMBLING_MASK)
>>>> +			pipe_config->hdmi_scrambling = true;
>>>> +		if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
>>>> +			pipe_config->hdmi_high_tmds_clock_ratio = true;
>>>>    		/* fall through */
>>>>    	case TRANS_DDI_MODE_SELECT_DVI:
>>>>    		pipe_config->lane_count = 4;
>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>>> index e77ca7d..fb5e670 100644
>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>> @@ -11701,6 +11701,9 @@ static void __printf(3, 4)
>>>>    	if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
>>>>    	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>>>>    		PIPE_CONF_CHECK_I(limited_color_range);
>>>> +
>>>> +	PIPE_CONF_CHECK_I(hdmi_scrambling);
>>>> +	PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio);
>>>>    	PIPE_CONF_CHECK_I(has_infoframe);
>>>>    
>>>>    	PIPE_CONF_CHECK_I(has_audio);
>>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>>> index 3af20c1..50d680c 100644
>>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>>> @@ -729,6 +729,12 @@ struct intel_crtc_state {
>>>>    
>>>>    	/* bitmask of visible planes (enum plane_id) */
>>>>    	u8 active_planes;
>>>> +
>>>> +	/* HDMI scrambling status */
>>>> +	bool hdmi_scrambling;
>>>> +
>>>> +	/* HDMI High TMDS char rate ratio */
>>>> +	bool hdmi_high_tmds_clock_ratio;
>>>>    };
>>>>    
>>>>    struct intel_crtc {
>>>> @@ -1623,6 +1629,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>>>>    bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>>>>    			       struct intel_crtc_state *pipe_config,
>>>>    			       struct drm_connector_state *conn_state);
>>>> +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
>>>> +					struct drm_connector *connector,
>>>> +					struct intel_crtc_state *config,
>>>> +					bool enable);
>>>>    void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
>>>>    
>>>>    
>>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>>>> index c2184f7..4585aa8 100644
>>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>>>> @@ -34,6 +34,7 @@
>>>>    #include <drm/drm_atomic_helper.h>
>>>>    #include <drm/drm_crtc.h>
>>>>    #include <drm/drm_edid.h>
>>>> +#include <drm/drm_scdc_helper.h>
>>>>    #include "intel_drv.h"
>>>>    #include <drm/i915_drm.h>
>>>>    #include <drm/intel_lpe_audio.h>
>>>> @@ -1316,6 +1317,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>>>>    	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>>>>    	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>>>    	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>>>> +	struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc;
>>>>    	int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
>>>>    	int clock_12bpc = clock_8bpc * 3 / 2;
>>>>    	int desired_bpp;
>>>> @@ -1385,6 +1387,16 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>>>>    
>>>>    	pipe_config->lane_count = 4;
>>>>    
>>>> +	if (scdc->scrambling.supported && IS_GEMINILAKE(dev_priv)) {
>>>> +		if (scdc->scrambling.low_rates)
>>>> +			pipe_config->hdmi_scrambling = true;
>>>> +
>>>> +		if (pipe_config->port_clock > 340000) {
>>>> +			pipe_config->hdmi_scrambling = true;
>>>> +			pipe_config->hdmi_high_tmds_clock_ratio = true;
>>>> +		}
>>>> +	}
>>>> +
>>>>    	return true;
>>>>    }
>>>>    
>>>> @@ -1794,6 +1806,56 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
>>>>    	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>>>>    }
>>>>    
>>>> +/*
>>>> + * This function handles scrambling on HDMI 2.0 capable sinks.
>>>> + * If required clock rate is > 340 Mhz && scrambling is supported by sink
>>>> + * it enables scrambling. This should be called before enabling the HDMI
>>>> + * 2.0 port, as the sink can choose to disable the scrambling if it doesn't
>>>> + * detect a scrambled clock within 100 ms.
>>>> + */
>>>> +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
>>>> +				       struct drm_connector *connector,
>>>> +				       struct intel_crtc_state *config,
>>>> +				       bool enable)
>>>> +{
>>>> +	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base);
>>>> +	struct drm_i915_private *dev_priv = connector->dev->dev_private;
>>>> +	struct drm_scrambling *scrambling =
>>>> +				&connector->display_info.hdmi.scdc.scrambling;
>>>> +	struct i2c_adapter *adptr = intel_gmbus_get_adapter(dev_priv,
>>>> +							  intel_hdmi->ddc_bus);
>>>> +	bool ret;
>>>> +
>>>> +	if (!scrambling->supported)
>>>> +		return;
>>>> +
>>>> +	if (!IS_GEMINILAKE(dev_priv))
>>>> +		return;
>>> I'd still like to see this getting done uncoditionally. That way we
>>> might get a bit of extra testing for the SCDC helper no matter what
>>> platform the user happens to own.
>> Sure, seems a good idea. I just added this hear to address the review
>> comment from Ander.
>> Also, I can move this IS_GLK condition to compute_config, and once we
>> check this in compute_config
>> we might never have check it again.
> I thought you already had the check in there? Hmm. Looks that way to me.
Yep, I already have that (this shows I have too many versions of code in 
my mind too :P),
what I meant was, we can use some of those state checks here before 
enabling/disabling sink side scrambling.
But then I realized it won't be the unconditional stuff you actually 
want, so please rub my last comment off :-)

- Shashank
>>> I presume the spec says the sink will reset these to some default value
>>> on HPD or something like that? But I'm not sure trusting them to get
>>> this right is a good idea because reality often doesn't match the
>>> specs. So the safer option IMO would be always tell the sink exactly
>>> what we want.
>> Yep, You are correct about the spec too, that it says if there is no
>> scrambled clock post hpd_out for
>> 100ms, the sink should disable scrambling. But connected modeset
>> scenarios need our attention, where
>> for example we are switching mode from, 4k@60 (scrambling reqd) ->
>> 1080@60 (scrambling not reqd).
>> For such cases too, we have to tell sink to disable scrambling.
>>>> +
>>>> +	DRM_DEBUG_KMS("Setting sink scrambling for enc:%s connector:%s\n",
>>>> +			intel_encoder->base.name, connector->name);
>>>> +
>>>> +	if (config->hdmi_high_tmds_clock_ratio) {
>>>> +		/* Set TMDS bit clock ratio to 1/40 or 1/10 */
>>>> +		ret = drm_scdc_set_high_tmds_clock_ratio(adptr, enable);
>>>> +		if (!ret) {
>>>> +			DRM_ERROR("Set TMDS ratio failed\n");
>>>> +			return;
>>>> +		}
>>>> +	}
>>>> +
>>>> +	if (config->hdmi_scrambling) {
>>>> +		/* Enable/disable sink scrambling */
>>>> +		ret = drm_scdc_set_scrambling(adptr, enable);
>>>> +		if (!ret) {
>>>> +			DRM_ERROR("Set sink scrambling failed\n");
>>>> +			return;
>>>> +		}
>>>> +	}
>>>> +
>>>> +	DRM_DEBUG_KMS("sink scrambling handled\n");
>>>> +}
>>>> +
>>>>    static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
>>>>    			     enum port port)
>>>>    {
>>>> -- 
>>>> 1.9.1

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

Re: [PATCH v8 5/6] drm/i915: enable scrambling

[Ville Syrjälä]

On Wed, Mar 08, 2017 at 07:50:45PM +0200, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 3/8/2017 7:44 PM, Ville Syrjälä wrote:
> > On Wed, Mar 08, 2017 at 07:26:45PM +0200, Sharma, Shashank wrote:
> >> Regards
> >>
> >> Shashank
> >>
> >>
> >> On 3/8/2017 7:17 PM, Ville Syrjälä wrote:
> >>> On Wed, Mar 08, 2017 at 07:07:48PM +0530, Shashank Sharma wrote:
> >>>>    Geminilake platform sports a native HDMI 2.0 controller, and is
> >>>>    capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec
> >>>>    mendates scrambling for these higher clocks, for reduced RF footprint.
> >>>>
> >>>>    This patch checks if the monitor supports scrambling, and if required,
> >>>>    enables it during the modeset.
> >>>>
> >>>>    V2: Addressed review comments from Ville:
> >>>>    - Do not track scrambling status in DRM layer, track somewhere in
> >>>>      driver like in intel_crtc_state.
> >>>>    - Don't talk to monitor at such a low layer, set monitor scrambling
> >>>>      in intel_enable_ddi() before enabling the port.
> >>>>
> >>>>    V3: Addressed review comments from Jani
> >>>>    - In comments, function names, use "sink" instead of "monitor",
> >>>>      so that the implementation could be close to the language of
> >>>>      HDMI spec.
> >>>>
> >>>> V4: Addressed review comment from Maarten
> >>>>    - scrambling -> hdmi_scrambling
> >>>>    - high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio
> >>>>
> >>>> V5: Addressed review comments from Ville and Ander
> >>>>    - Do not modifiy the crtc_state after compute_config. Move all
> >>>>      scrambling and tmds_clock_ratio calcutations to compute_config.
> >>>>    - While setting scrambling for source/sink, do not check the
> >>>>      conditions again, just go by the crtc_state flags. This will
> >>>>      simplyfy the condition checks.
> >>>>
> >>>> V6: Addressed review comments from Ville
> >>>>    - Do not add IS_GLK check in disable/enable function, instead add it
> >>>>      in compute_config, while setting state flags.
> >>>>    - Remove unnecessary paranthesis.
> >>>>    - Simplyfy handle_sink_scrambling function as suggested.
> >>>>    - Add readout code for scrambling status in get_ddi_config and add a
> >>>>      check for the same in pipe_config_compare.
> >>>>
> >>>> V7: Addressed review comments from Ander/Ville
> >>>>    - No separate function for source scrambling, make it inline
> >>>>    - Align the last line of the macro TRANS_DDI_HDMI_SCRAMBLING_MASK
> >>>>    - Do not add platform check while setting source scrambling
> >>>>    - Use pipe_config instead of crtc->config to set sink scrambling
> >>>>    - To readout scrambling status, Compare with SCRAMBLING_MASK
> >>>>      not any of its bits
> >>>>    - Remove platform check in intel_pipe_config_compare while checking
> >>>>      scrambling status
> >>>>
> >>>> V8: Fixed mege conflict, Addressed review comments from Ander
> >>>>    - Remove the desciption/comment about scrambling fom the caller, move
> >>>>      it to the function
> >>>>    - Move the IS_GLK check into scrambling function
> >>>>    - Fix alignment
> >>>>
> >>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >>>> ---
> >>>>    drivers/gpu/drm/i915/i915_reg.h      |  7 ++++
> >>>>    drivers/gpu/drm/i915/intel_ddi.c     | 19 +++++++++++
> >>>>    drivers/gpu/drm/i915/intel_display.c |  3 ++
> >>>>    drivers/gpu/drm/i915/intel_drv.h     | 10 ++++++
> >>>>    drivers/gpu/drm/i915/intel_hdmi.c    | 62 ++++++++++++++++++++++++++++++++++++
> >>>>    5 files changed, 101 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >>>> index cc843f9..2d50fdc 100644
> >>>> --- a/drivers/gpu/drm/i915/i915_reg.h
> >>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >>>> @@ -7830,7 +7830,14 @@ enum {
> >>>>    #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5<<12)
> >>>>    #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6<<12)
> >>>>    #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC	(1<<8)
> >>>> +#define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7)
> >>>> +#define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6)
> >>>>    #define  TRANS_DDI_BFI_ENABLE		(1<<4)
> >>>> +#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE	(1<<4)
> >>>> +#define  TRANS_DDI_HDMI_SCRAMBLING	(1<<0)
> >>>> +#define  TRANS_DDI_HDMI_SCRAMBLING_MASK (TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE \
> >>>> +					| TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ \
> >>>> +					| TRANS_DDI_HDMI_SCRAMBLING)
> >>>>    
> >>>>    /* DisplayPort Transport Control */
> >>>>    #define _DP_TP_CTL_A			0x64040
> >>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >>>> index 0467676..b3e4c4a 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >>>> @@ -1309,6 +1309,11 @@ void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state)
> >>>>    			temp |= TRANS_DDI_MODE_SELECT_HDMI;
> >>>>    		else
> >>>>    			temp |= TRANS_DDI_MODE_SELECT_DVI;
> >>>> +
> >>>> +		if (crtc_state->hdmi_scrambling)
> >>>> +			temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
> >>>> +		if (crtc_state->hdmi_high_tmds_clock_ratio)
> >>>> +			temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
> >>>>    	} else if (type == INTEL_OUTPUT_ANALOG) {
> >>>>    		temp |= TRANS_DDI_MODE_SELECT_FDI;
> >>>>    		temp |= (crtc_state->fdi_lanes - 1) << 1;
> >>>> @@ -1882,6 +1887,9 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder,
> >>>>    		struct intel_digital_port *intel_dig_port =
> >>>>    			enc_to_dig_port(encoder);
> >>>>    
> >>>> +		intel_hdmi_handle_sink_scrambling(intel_encoder,
> >>>> +			conn_state->connector, pipe_config, true);
> >>>> +
> >>>>    		/* In HDMI/DVI mode, the port width, and swing/emphasis values
> >>>>    		 * are ignored so nothing special needs to be done besides
> >>>>    		 * enabling the port.
> >>>> @@ -1914,6 +1922,11 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
> >>>>    	if (old_crtc_state->has_audio)
> >>>>    		intel_audio_codec_disable(intel_encoder);
> >>>>    
> >>>> +	if (type == INTEL_OUTPUT_HDMI) {
> >>>> +		intel_hdmi_handle_sink_scrambling(intel_encoder,
> >>>> +			old_conn_state->connector, old_crtc_state, false);
> >>>> +	}
> >>>> +
> >>>>    	if (type == INTEL_OUTPUT_EDP) {
> >>>>    		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> >>>>    
> >>>> @@ -2040,6 +2053,12 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >>>>    
> >>>>    		if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
> >>>>    			pipe_config->has_infoframe = true;
> >>>> +
> >>>> +		if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) ==
> >>>> +				TRANS_DDI_HDMI_SCRAMBLING_MASK)
> >>>> +			pipe_config->hdmi_scrambling = true;
> >>>> +		if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
> >>>> +			pipe_config->hdmi_high_tmds_clock_ratio = true;
> >>>>    		/* fall through */
> >>>>    	case TRANS_DDI_MODE_SELECT_DVI:
> >>>>    		pipe_config->lane_count = 4;
> >>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>>> index e77ca7d..fb5e670 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>>> @@ -11701,6 +11701,9 @@ static void __printf(3, 4)
> >>>>    	if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
> >>>>    	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> >>>>    		PIPE_CONF_CHECK_I(limited_color_range);
> >>>> +
> >>>> +	PIPE_CONF_CHECK_I(hdmi_scrambling);
> >>>> +	PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio);
> >>>>    	PIPE_CONF_CHECK_I(has_infoframe);
> >>>>    
> >>>>    	PIPE_CONF_CHECK_I(has_audio);
> >>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >>>> index 3af20c1..50d680c 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_drv.h
> >>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >>>> @@ -729,6 +729,12 @@ struct intel_crtc_state {
> >>>>    
> >>>>    	/* bitmask of visible planes (enum plane_id) */
> >>>>    	u8 active_planes;
> >>>> +
> >>>> +	/* HDMI scrambling status */
> >>>> +	bool hdmi_scrambling;
> >>>> +
> >>>> +	/* HDMI High TMDS char rate ratio */
> >>>> +	bool hdmi_high_tmds_clock_ratio;
> >>>>    };
> >>>>    
> >>>>    struct intel_crtc {
> >>>> @@ -1623,6 +1629,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >>>>    bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> >>>>    			       struct intel_crtc_state *pipe_config,
> >>>>    			       struct drm_connector_state *conn_state);
> >>>> +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
> >>>> +					struct drm_connector *connector,
> >>>> +					struct intel_crtc_state *config,
> >>>> +					bool enable);
> >>>>    void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
> >>>>    
> >>>>    
> >>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> >>>> index c2184f7..4585aa8 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >>>> @@ -34,6 +34,7 @@
> >>>>    #include <drm/drm_atomic_helper.h>
> >>>>    #include <drm/drm_crtc.h>
> >>>>    #include <drm/drm_edid.h>
> >>>> +#include <drm/drm_scdc_helper.h>
> >>>>    #include "intel_drv.h"
> >>>>    #include <drm/i915_drm.h>
> >>>>    #include <drm/intel_lpe_audio.h>
> >>>> @@ -1316,6 +1317,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> >>>>    	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> >>>>    	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >>>>    	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> >>>> +	struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc;
> >>>>    	int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
> >>>>    	int clock_12bpc = clock_8bpc * 3 / 2;
> >>>>    	int desired_bpp;
> >>>> @@ -1385,6 +1387,16 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> >>>>    
> >>>>    	pipe_config->lane_count = 4;
> >>>>    
> >>>> +	if (scdc->scrambling.supported && IS_GEMINILAKE(dev_priv)) {
> >>>> +		if (scdc->scrambling.low_rates)
> >>>> +			pipe_config->hdmi_scrambling = true;
> >>>> +
> >>>> +		if (pipe_config->port_clock > 340000) {
> >>>> +			pipe_config->hdmi_scrambling = true;
> >>>> +			pipe_config->hdmi_high_tmds_clock_ratio = true;
> >>>> +		}
> >>>> +	}
> >>>> +
> >>>>    	return true;
> >>>>    }
> >>>>    
> >>>> @@ -1794,6 +1806,56 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
> >>>>    	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> >>>>    }
> >>>>    
> >>>> +/*
> >>>> + * This function handles scrambling on HDMI 2.0 capable sinks.
> >>>> + * If required clock rate is > 340 Mhz && scrambling is supported by sink
> >>>> + * it enables scrambling. This should be called before enabling the HDMI
> >>>> + * 2.0 port, as the sink can choose to disable the scrambling if it doesn't
> >>>> + * detect a scrambled clock within 100 ms.
> >>>> + */
> >>>> +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
> >>>> +				       struct drm_connector *connector,
> >>>> +				       struct intel_crtc_state *config,
> >>>> +				       bool enable)
> >>>> +{
> >>>> +	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base);
> >>>> +	struct drm_i915_private *dev_priv = connector->dev->dev_private;
> >>>> +	struct drm_scrambling *scrambling =
> >>>> +				&connector->display_info.hdmi.scdc.scrambling;
> >>>> +	struct i2c_adapter *adptr = intel_gmbus_get_adapter(dev_priv,
> >>>> +							  intel_hdmi->ddc_bus);
> >>>> +	bool ret;
> >>>> +
> >>>> +	if (!scrambling->supported)
> >>>> +		return;
> >>>> +
> >>>> +	if (!IS_GEMINILAKE(dev_priv))
> >>>> +		return;
> >>> I'd still like to see this getting done uncoditionally. That way we
> >>> might get a bit of extra testing for the SCDC helper no matter what
> >>> platform the user happens to own.
> >> Sure, seems a good idea. I just added this hear to address the review
> >> comment from Ander.
> >> Also, I can move this IS_GLK condition to compute_config, and once we
> >> check this in compute_config
> >> we might never have check it again.
> > I thought you already had the check in there? Hmm. Looks that way to me.
> Yep, I already have that (this shows I have too many versions of code in 
> my mind too :P),
> what I meant was, we can use some of those state checks here before 
> enabling/disabling sink side scrambling.
> But then I realized it won't be the unconditional stuff you actually 
> want, so please rub my last comment off :-)

Well I think it should work just fine if you simply pass both into this
function as bools, and then in the disable path we pass explicit 'false'
for both, and in the enable path we just pass config->whatever.

> 
> - Shashank
> >>> I presume the spec says the sink will reset these to some default value
> >>> on HPD or something like that? But I'm not sure trusting them to get
> >>> this right is a good idea because reality often doesn't match the
> >>> specs. So the safer option IMO would be always tell the sink exactly
> >>> what we want.
> >> Yep, You are correct about the spec too, that it says if there is no
> >> scrambled clock post hpd_out for
> >> 100ms, the sink should disable scrambling. But connected modeset
> >> scenarios need our attention, where
> >> for example we are switching mode from, 4k@60 (scrambling reqd) ->
> >> 1080@60 (scrambling not reqd).
> >> For such cases too, we have to tell sink to disable scrambling.
> >>>> +
> >>>> +	DRM_DEBUG_KMS("Setting sink scrambling for enc:%s connector:%s\n",
> >>>> +			intel_encoder->base.name, connector->name);
> >>>> +
> >>>> +	if (config->hdmi_high_tmds_clock_ratio) {
> >>>> +		/* Set TMDS bit clock ratio to 1/40 or 1/10 */
> >>>> +		ret = drm_scdc_set_high_tmds_clock_ratio(adptr, enable);
> >>>> +		if (!ret) {
> >>>> +			DRM_ERROR("Set TMDS ratio failed\n");
> >>>> +			return;
> >>>> +		}
> >>>> +	}
> >>>> +
> >>>> +	if (config->hdmi_scrambling) {
> >>>> +		/* Enable/disable sink scrambling */
> >>>> +		ret = drm_scdc_set_scrambling(adptr, enable);
> >>>> +		if (!ret) {
> >>>> +			DRM_ERROR("Set sink scrambling failed\n");
> >>>> +			return;
> >>>> +		}
> >>>> +	}
> >>>> +
> >>>> +	DRM_DEBUG_KMS("sink scrambling handled\n");
> >>>> +}
> >>>> +
> >>>>    static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
> >>>>    			     enum port port)
> >>>>    {
> >>>> -- 
> >>>> 1.9.1

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v8 5/6] drm/i915: enable scrambling

[Sharma, Shashank]

Regards

Shashank


On 3/8/2017 7:56 PM, Ville Syrjälä wrote:
> On Wed, Mar 08, 2017 at 07:50:45PM +0200, Sharma, Shashank wrote:
>> Regards
>>
>> Shashank
>>
>>
>> On 3/8/2017 7:44 PM, Ville Syrjälä wrote:
>>> On Wed, Mar 08, 2017 at 07:26:45PM +0200, Sharma, Shashank wrote:
>>>> Regards
>>>>
>>>> Shashank
>>>>
>>>>
>>>> On 3/8/2017 7:17 PM, Ville Syrjälä wrote:
>>>>> On Wed, Mar 08, 2017 at 07:07:48PM +0530, Shashank Sharma wrote:
>>>>>>     Geminilake platform sports a native HDMI 2.0 controller, and is
>>>>>>     capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec
>>>>>>     mendates scrambling for these higher clocks, for reduced RF footprint.
>>>>>>
>>>>>>     This patch checks if the monitor supports scrambling, and if required,
>>>>>>     enables it during the modeset.
>>>>>>
>>>>>>     V2: Addressed review comments from Ville:
>>>>>>     - Do not track scrambling status in DRM layer, track somewhere in
>>>>>>       driver like in intel_crtc_state.
>>>>>>     - Don't talk to monitor at such a low layer, set monitor scrambling
>>>>>>       in intel_enable_ddi() before enabling the port.
>>>>>>
>>>>>>     V3: Addressed review comments from Jani
>>>>>>     - In comments, function names, use "sink" instead of "monitor",
>>>>>>       so that the implementation could be close to the language of
>>>>>>       HDMI spec.
>>>>>>
>>>>>> V4: Addressed review comment from Maarten
>>>>>>     - scrambling -> hdmi_scrambling
>>>>>>     - high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio
>>>>>>
>>>>>> V5: Addressed review comments from Ville and Ander
>>>>>>     - Do not modifiy the crtc_state after compute_config. Move all
>>>>>>       scrambling and tmds_clock_ratio calcutations to compute_config.
>>>>>>     - While setting scrambling for source/sink, do not check the
>>>>>>       conditions again, just go by the crtc_state flags. This will
>>>>>>       simplyfy the condition checks.
>>>>>>
>>>>>> V6: Addressed review comments from Ville
>>>>>>     - Do not add IS_GLK check in disable/enable function, instead add it
>>>>>>       in compute_config, while setting state flags.
>>>>>>     - Remove unnecessary paranthesis.
>>>>>>     - Simplyfy handle_sink_scrambling function as suggested.
>>>>>>     - Add readout code for scrambling status in get_ddi_config and add a
>>>>>>       check for the same in pipe_config_compare.
>>>>>>
>>>>>> V7: Addressed review comments from Ander/Ville
>>>>>>     - No separate function for source scrambling, make it inline
>>>>>>     - Align the last line of the macro TRANS_DDI_HDMI_SCRAMBLING_MASK
>>>>>>     - Do not add platform check while setting source scrambling
>>>>>>     - Use pipe_config instead of crtc->config to set sink scrambling
>>>>>>     - To readout scrambling status, Compare with SCRAMBLING_MASK
>>>>>>       not any of its bits
>>>>>>     - Remove platform check in intel_pipe_config_compare while checking
>>>>>>       scrambling status
>>>>>>
>>>>>> V8: Fixed mege conflict, Addressed review comments from Ander
>>>>>>     - Remove the desciption/comment about scrambling fom the caller, move
>>>>>>       it to the function
>>>>>>     - Move the IS_GLK check into scrambling function
>>>>>>     - Fix alignment
>>>>>>
>>>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>>>>> ---
>>>>>>     drivers/gpu/drm/i915/i915_reg.h      |  7 ++++
>>>>>>     drivers/gpu/drm/i915/intel_ddi.c     | 19 +++++++++++
>>>>>>     drivers/gpu/drm/i915/intel_display.c |  3 ++
>>>>>>     drivers/gpu/drm/i915/intel_drv.h     | 10 ++++++
>>>>>>     drivers/gpu/drm/i915/intel_hdmi.c    | 62 ++++++++++++++++++++++++++++++++++++
>>>>>>     5 files changed, 101 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>>>>> index cc843f9..2d50fdc 100644
>>>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>>>>> @@ -7830,7 +7830,14 @@ enum {
>>>>>>     #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5<<12)
>>>>>>     #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6<<12)
>>>>>>     #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC	(1<<8)
>>>>>> +#define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7)
>>>>>> +#define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6)
>>>>>>     #define  TRANS_DDI_BFI_ENABLE		(1<<4)
>>>>>> +#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE	(1<<4)
>>>>>> +#define  TRANS_DDI_HDMI_SCRAMBLING	(1<<0)
>>>>>> +#define  TRANS_DDI_HDMI_SCRAMBLING_MASK (TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE \
>>>>>> +					| TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ \
>>>>>> +					| TRANS_DDI_HDMI_SCRAMBLING)
>>>>>>     
>>>>>>     /* DisplayPort Transport Control */
>>>>>>     #define _DP_TP_CTL_A			0x64040
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>>>>>> index 0467676..b3e4c4a 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>>>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>>>>> @@ -1309,6 +1309,11 @@ void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state)
>>>>>>     			temp |= TRANS_DDI_MODE_SELECT_HDMI;
>>>>>>     		else
>>>>>>     			temp |= TRANS_DDI_MODE_SELECT_DVI;
>>>>>> +
>>>>>> +		if (crtc_state->hdmi_scrambling)
>>>>>> +			temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
>>>>>> +		if (crtc_state->hdmi_high_tmds_clock_ratio)
>>>>>> +			temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
>>>>>>     	} else if (type == INTEL_OUTPUT_ANALOG) {
>>>>>>     		temp |= TRANS_DDI_MODE_SELECT_FDI;
>>>>>>     		temp |= (crtc_state->fdi_lanes - 1) << 1;
>>>>>> @@ -1882,6 +1887,9 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder,
>>>>>>     		struct intel_digital_port *intel_dig_port =
>>>>>>     			enc_to_dig_port(encoder);
>>>>>>     
>>>>>> +		intel_hdmi_handle_sink_scrambling(intel_encoder,
>>>>>> +			conn_state->connector, pipe_config, true);
>>>>>> +
>>>>>>     		/* In HDMI/DVI mode, the port width, and swing/emphasis values
>>>>>>     		 * are ignored so nothing special needs to be done besides
>>>>>>     		 * enabling the port.
>>>>>> @@ -1914,6 +1922,11 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
>>>>>>     	if (old_crtc_state->has_audio)
>>>>>>     		intel_audio_codec_disable(intel_encoder);
>>>>>>     
>>>>>> +	if (type == INTEL_OUTPUT_HDMI) {
>>>>>> +		intel_hdmi_handle_sink_scrambling(intel_encoder,
>>>>>> +			old_conn_state->connector, old_crtc_state, false);
>>>>>> +	}
>>>>>> +
>>>>>>     	if (type == INTEL_OUTPUT_EDP) {
>>>>>>     		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>>>>>>     
>>>>>> @@ -2040,6 +2053,12 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>>>>>>     
>>>>>>     		if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
>>>>>>     			pipe_config->has_infoframe = true;
>>>>>> +
>>>>>> +		if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) ==
>>>>>> +				TRANS_DDI_HDMI_SCRAMBLING_MASK)
>>>>>> +			pipe_config->hdmi_scrambling = true;
>>>>>> +		if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
>>>>>> +			pipe_config->hdmi_high_tmds_clock_ratio = true;
>>>>>>     		/* fall through */
>>>>>>     	case TRANS_DDI_MODE_SELECT_DVI:
>>>>>>     		pipe_config->lane_count = 4;
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>>>>> index e77ca7d..fb5e670 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>>>> @@ -11701,6 +11701,9 @@ static void __printf(3, 4)
>>>>>>     	if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
>>>>>>     	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>>>>>>     		PIPE_CONF_CHECK_I(limited_color_range);
>>>>>> +
>>>>>> +	PIPE_CONF_CHECK_I(hdmi_scrambling);
>>>>>> +	PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio);
>>>>>>     	PIPE_CONF_CHECK_I(has_infoframe);
>>>>>>     
>>>>>>     	PIPE_CONF_CHECK_I(has_audio);
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>>>>> index 3af20c1..50d680c 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>>>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>>>>> @@ -729,6 +729,12 @@ struct intel_crtc_state {
>>>>>>     
>>>>>>     	/* bitmask of visible planes (enum plane_id) */
>>>>>>     	u8 active_planes;
>>>>>> +
>>>>>> +	/* HDMI scrambling status */
>>>>>> +	bool hdmi_scrambling;
>>>>>> +
>>>>>> +	/* HDMI High TMDS char rate ratio */
>>>>>> +	bool hdmi_high_tmds_clock_ratio;
>>>>>>     };
>>>>>>     
>>>>>>     struct intel_crtc {
>>>>>> @@ -1623,6 +1629,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>>>>>>     bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>>>>>>     			       struct intel_crtc_state *pipe_config,
>>>>>>     			       struct drm_connector_state *conn_state);
>>>>>> +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
>>>>>> +					struct drm_connector *connector,
>>>>>> +					struct intel_crtc_state *config,
>>>>>> +					bool enable);
>>>>>>     void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
>>>>>>     
>>>>>>     
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>>>>>> index c2184f7..4585aa8 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>>>>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>>>>>> @@ -34,6 +34,7 @@
>>>>>>     #include <drm/drm_atomic_helper.h>
>>>>>>     #include <drm/drm_crtc.h>
>>>>>>     #include <drm/drm_edid.h>
>>>>>> +#include <drm/drm_scdc_helper.h>
>>>>>>     #include "intel_drv.h"
>>>>>>     #include <drm/i915_drm.h>
>>>>>>     #include <drm/intel_lpe_audio.h>
>>>>>> @@ -1316,6 +1317,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>>>>>>     	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>>>>>>     	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>>>>>     	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>>>>>> +	struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc;
>>>>>>     	int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
>>>>>>     	int clock_12bpc = clock_8bpc * 3 / 2;
>>>>>>     	int desired_bpp;
>>>>>> @@ -1385,6 +1387,16 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>>>>>>     
>>>>>>     	pipe_config->lane_count = 4;
>>>>>>     
>>>>>> +	if (scdc->scrambling.supported && IS_GEMINILAKE(dev_priv)) {
>>>>>> +		if (scdc->scrambling.low_rates)
>>>>>> +			pipe_config->hdmi_scrambling = true;
>>>>>> +
>>>>>> +		if (pipe_config->port_clock > 340000) {
>>>>>> +			pipe_config->hdmi_scrambling = true;
>>>>>> +			pipe_config->hdmi_high_tmds_clock_ratio = true;
>>>>>> +		}
>>>>>> +	}
>>>>>> +
>>>>>>     	return true;
>>>>>>     }
>>>>>>     
>>>>>> @@ -1794,6 +1806,56 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
>>>>>>     	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>>>>>>     }
>>>>>>     
>>>>>> +/*
>>>>>> + * This function handles scrambling on HDMI 2.0 capable sinks.
>>>>>> + * If required clock rate is > 340 Mhz && scrambling is supported by sink
>>>>>> + * it enables scrambling. This should be called before enabling the HDMI
>>>>>> + * 2.0 port, as the sink can choose to disable the scrambling if it doesn't
>>>>>> + * detect a scrambled clock within 100 ms.
>>>>>> + */
>>>>>> +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
>>>>>> +				       struct drm_connector *connector,
>>>>>> +				       struct intel_crtc_state *config,
>>>>>> +				       bool enable)
>>>>>> +{
>>>>>> +	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base);
>>>>>> +	struct drm_i915_private *dev_priv = connector->dev->dev_private;
>>>>>> +	struct drm_scrambling *scrambling =
>>>>>> +				&connector->display_info.hdmi.scdc.scrambling;
>>>>>> +	struct i2c_adapter *adptr = intel_gmbus_get_adapter(dev_priv,
>>>>>> +							  intel_hdmi->ddc_bus);
>>>>>> +	bool ret;
>>>>>> +
>>>>>> +	if (!scrambling->supported)
>>>>>> +		return;
>>>>>> +
>>>>>> +	if (!IS_GEMINILAKE(dev_priv))
>>>>>> +		return;
>>>>> I'd still like to see this getting done uncoditionally. That way we
>>>>> might get a bit of extra testing for the SCDC helper no matter what
>>>>> platform the user happens to own.
>>>> Sure, seems a good idea. I just added this hear to address the review
>>>> comment from Ander.
>>>> Also, I can move this IS_GLK condition to compute_config, and once we
>>>> check this in compute_config
>>>> we might never have check it again.
>>> I thought you already had the check in there? Hmm. Looks that way to me.
>> Yep, I already have that (this shows I have too many versions of code in
>> my mind too :P),
>> what I meant was, we can use some of those state checks here before
>> enabling/disabling sink side scrambling.
>> But then I realized it won't be the unconditional stuff you actually
>> want, so please rub my last comment off :-)
> Well I think it should work just fine if you simply pass both into this
> function as bools, and then in the disable path we pass explicit 'false'
> for both, and in the enable path we just pass config->whatever.
Humm, let me try this.

- Shashank
>> - Shashank
>>>>> I presume the spec says the sink will reset these to some default value
>>>>> on HPD or something like that? But I'm not sure trusting them to get
>>>>> this right is a good idea because reality often doesn't match the
>>>>> specs. So the safer option IMO would be always tell the sink exactly
>>>>> what we want.
>>>> Yep, You are correct about the spec too, that it says if there is no
>>>> scrambled clock post hpd_out for
>>>> 100ms, the sink should disable scrambling. But connected modeset
>>>> scenarios need our attention, where
>>>> for example we are switching mode from, 4k@60 (scrambling reqd) ->
>>>> 1080@60 (scrambling not reqd).
>>>> For such cases too, we have to tell sink to disable scrambling.
>>>>>> +
>>>>>> +	DRM_DEBUG_KMS("Setting sink scrambling for enc:%s connector:%s\n",
>>>>>> +			intel_encoder->base.name, connector->name);
>>>>>> +
>>>>>> +	if (config->hdmi_high_tmds_clock_ratio) {
>>>>>> +		/* Set TMDS bit clock ratio to 1/40 or 1/10 */
>>>>>> +		ret = drm_scdc_set_high_tmds_clock_ratio(adptr, enable);
>>>>>> +		if (!ret) {
>>>>>> +			DRM_ERROR("Set TMDS ratio failed\n");
>>>>>> +			return;
>>>>>> +		}
>>>>>> +	}
>>>>>> +
>>>>>> +	if (config->hdmi_scrambling) {
>>>>>> +		/* Enable/disable sink scrambling */
>>>>>> +		ret = drm_scdc_set_scrambling(adptr, enable);
>>>>>> +		if (!ret) {
>>>>>> +			DRM_ERROR("Set sink scrambling failed\n");
>>>>>> +			return;
>>>>>> +		}
>>>>>> +	}
>>>>>> +
>>>>>> +	DRM_DEBUG_KMS("sink scrambling handled\n");
>>>>>> +}
>>>>>> +
>>>>>>     static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
>>>>>>     			     enum port port)
>>>>>>     {
>>>>>> -- 
>>>>>> 1.9.1

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

Re: [PATCH v8 5/6] drm/i915: enable scrambling

[Ander Conselvan De Oliveira]

On Wed, 2017-03-08 at 19:07 +0530, Shashank Sharma wrote:
>  Geminilake platform sports a native HDMI 2.0 controller, and is
>  capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec
>  mendates scrambling for these higher clocks, for reduced RF footprint.
> 
>  This patch checks if the monitor supports scrambling, and if required,
>  enables it during the modeset.
> 
>  V2: Addressed review comments from Ville:
>  - Do not track scrambling status in DRM layer, track somewhere in
>    driver like in intel_crtc_state.
>  - Don't talk to monitor at such a low layer, set monitor scrambling
>    in intel_enable_ddi() before enabling the port.
> 
>  V3: Addressed review comments from Jani
>  - In comments, function names, use "sink" instead of "monitor",
>    so that the implementation could be close to the language of
>    HDMI spec.
> 
> V4: Addressed review comment from Maarten
>  - scrambling -> hdmi_scrambling
>  - high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio
> 
> V5: Addressed review comments from Ville and Ander
>  - Do not modifiy the crtc_state after compute_config. Move all
>    scrambling and tmds_clock_ratio calcutations to compute_config.
>  - While setting scrambling for source/sink, do not check the
>    conditions again, just go by the crtc_state flags. This will
>    simplyfy the condition checks.
> 
> V6: Addressed review comments from Ville
>  - Do not add IS_GLK check in disable/enable function, instead add it
>    in compute_config, while setting state flags.
>  - Remove unnecessary paranthesis.
>  - Simplyfy handle_sink_scrambling function as suggested.
>  - Add readout code for scrambling status in get_ddi_config and add a
>    check for the same in pipe_config_compare.
> 
> V7: Addressed review comments from Ander/Ville
>  - No separate function for source scrambling, make it inline
>  - Align the last line of the macro TRANS_DDI_HDMI_SCRAMBLING_MASK
>  - Do not add platform check while setting source scrambling
>  - Use pipe_config instead of crtc->config to set sink scrambling
>  - To readout scrambling status, Compare with SCRAMBLING_MASK
>    not any of its bits
>  - Remove platform check in intel_pipe_config_compare while checking
>    scrambling status
> 
> V8: Fixed mege conflict, Addressed review comments from Ander
>  - Remove the desciption/comment about scrambling fom the caller, move
>    it to the function
>  - Move the IS_GLK check into scrambling function
>  - Fix alignment
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  7 ++++
>  drivers/gpu/drm/i915/intel_ddi.c     | 19 +++++++++++
>  drivers/gpu/drm/i915/intel_display.c |  3 ++
>  drivers/gpu/drm/i915/intel_drv.h     | 10 ++++++
>  drivers/gpu/drm/i915/intel_hdmi.c    | 62 ++++++++++++++++++++++++++++++++++++
>  5 files changed, 101 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index cc843f9..2d50fdc 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7830,7 +7830,14 @@ enum {
>  #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5<<12)
>  #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6<<12)
>  #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC	(1<<8)
> +#define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7)
> +#define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6)
>  #define  TRANS_DDI_BFI_ENABLE		(1<<4)
> +#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE	(1<<4)
> +#define  TRANS_DDI_HDMI_SCRAMBLING	(1<<0)
> +#define  TRANS_DDI_HDMI_SCRAMBLING_MASK (TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE \
> +					| TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ \
> +					| TRANS_DDI_HDMI_SCRAMBLING)
>  
>  /* DisplayPort Transport Control */
>  #define _DP_TP_CTL_A			0x64040
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 0467676..b3e4c4a 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1309,6 +1309,11 @@ void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state)
>  			temp |= TRANS_DDI_MODE_SELECT_HDMI;
>  		else
>  			temp |= TRANS_DDI_MODE_SELECT_DVI;
> +
> +		if (crtc_state->hdmi_scrambling)
> +			temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
> +		if (crtc_state->hdmi_high_tmds_clock_ratio)
> +			temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
>  	} else if (type == INTEL_OUTPUT_ANALOG) {
>  		temp |= TRANS_DDI_MODE_SELECT_FDI;
>  		temp |= (crtc_state->fdi_lanes - 1) << 1;
> @@ -1882,6 +1887,9 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder,
>  		struct intel_digital_port *intel_dig_port =
>  			enc_to_dig_port(encoder);
>  
> +		intel_hdmi_handle_sink_scrambling(intel_encoder,
> +			conn_state->connector, pipe_config, true);
> +

I'd prefer if we followed the coding style here (and everywhere!), i.e., 

		intel_hdmi_handle_sink_scrambling(intel_encoder,
						  conn_state->connector,
						  pipe_config, true);

If you want to make this neater, you could add an extra variable for the
connector and throw in a clean up patch before this one changing the
"intel_encoder" variables to be called just "encoder". Since we prefer to have
the most specialized type as function arguments and we only support intel
hardware, we don't need to constantly remind ourselves that this is an intel
encoder.

$ grep -o -E 'struct intel_encoder \*(intel_)?encoder' *.[ch] | \
	cut -d: -f2 | sort | uniq -c
    257 struct intel_encoder *encoder
     69 struct intel_encoder *intel_encoder

Could change that to 270 vs 56 by cleaning intel_ddi.c up.

>  		/* In HDMI/DVI mode, the port width, and swing/emphasis values
>  		 * are ignored so nothing special needs to be done besides
>  		 * enabling the port.
> @@ -1914,6 +1922,11 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
>  	if (old_crtc_state->has_audio)
>  		intel_audio_codec_disable(intel_encoder);
>  
> +	if (type == INTEL_OUTPUT_HDMI) {
> +		intel_hdmi_handle_sink_scrambling(intel_encoder,
> +			old_conn_state->connector, old_crtc_state, false);
> +	}
> +

Ditto.

>  	if (type == INTEL_OUTPUT_EDP) {
>  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  
> @@ -2040,6 +2053,12 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  
>  		if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
>  			pipe_config->has_infoframe = true;
> +
> +		if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) ==
> +				TRANS_DDI_HDMI_SCRAMBLING_MASK)
> +			pipe_config->hdmi_scrambling = true;
> +		if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
> +			pipe_config->hdmi_high_tmds_clock_ratio = true;
>  		/* fall through */
>  	case TRANS_DDI_MODE_SELECT_DVI:
>  		pipe_config->lane_count = 4;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e77ca7d..fb5e670 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11701,6 +11701,9 @@ static void __printf(3, 4)
>  	if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
>  	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>  		PIPE_CONF_CHECK_I(limited_color_range);
> +
> +	PIPE_CONF_CHECK_I(hdmi_scrambling);
> +	PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio);
>  	PIPE_CONF_CHECK_I(has_infoframe);
>  
>  	PIPE_CONF_CHECK_I(has_audio);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3af20c1..50d680c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -729,6 +729,12 @@ struct intel_crtc_state {
>  
>  	/* bitmask of visible planes (enum plane_id) */
>  	u8 active_planes;
> +
> +	/* HDMI scrambling status */
> +	bool hdmi_scrambling;
> +
> +	/* HDMI High TMDS char rate ratio */
> +	bool hdmi_high_tmds_clock_ratio;
>  };
>  
>  struct intel_crtc {
> @@ -1623,6 +1629,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  			       struct intel_crtc_state *pipe_config,
>  			       struct drm_connector_state *conn_state);
> +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
> +					struct drm_connector *connector,
> +					struct intel_crtc_state *config,
> +					bool enable);
>  void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
>  
>  
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index c2184f7..4585aa8 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -34,6 +34,7 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_edid.h>
> +#include <drm/drm_scdc_helper.h>
>  #include "intel_drv.h"
>  #include <drm/i915_drm.h>
>  #include <drm/intel_lpe_audio.h>
> @@ -1316,6 +1317,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> +	struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc;
>  	int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
>  	int clock_12bpc = clock_8bpc * 3 / 2;
>  	int desired_bpp;
> @@ -1385,6 +1387,16 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  
>  	pipe_config->lane_count = 4;
>  
> +	if (scdc->scrambling.supported && IS_GEMINILAKE(dev_priv)) {
> +		if (scdc->scrambling.low_rates)
> +			pipe_config->hdmi_scrambling = true;
> +
> +		if (pipe_config->port_clock > 340000) {
> +			pipe_config->hdmi_scrambling = true;
> +			pipe_config->hdmi_high_tmds_clock_ratio = true;
> +		}
> +	}
> +
>  	return true;
>  }
>  
> @@ -1794,6 +1806,56 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
>  	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>  }
>  
> +/*
> + * This function handles scrambling on HDMI 2.0 capable sinks.
> + * If required clock rate is > 340 Mhz && scrambling is supported by sink
> + * it enables scrambling. This should be called before enabling the HDMI
> + * 2.0 port, as the sink can choose to disable the scrambling if it doesn't
> + * detect a scrambled clock within 100 ms.
> + */

I actually meant to put this in kernel doc format. It is standard practice to
document functions that are not static, even though we don't seem to follow this
for intel_hdmi.c. Have to start somewhere.

Ander

> +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
> +				       struct drm_connector *connector,
> +				       struct intel_crtc_state *config,
> +				       bool enable)
> +{
> +	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base);
> +	struct drm_i915_private *dev_priv = connector->dev->dev_private;
> +	struct drm_scrambling *scrambling =
> +				&connector->display_info.hdmi.scdc.scrambling;
> +	struct i2c_adapter *adptr = intel_gmbus_get_adapter(dev_priv,
> +							  intel_hdmi->ddc_bus);
> +	bool ret;
> +
> +	if (!scrambling->supported)
> +		return;
> +
> +	if (!IS_GEMINILAKE(dev_priv))
> +		return;
> +
> +	DRM_DEBUG_KMS("Setting sink scrambling for enc:%s connector:%s\n",
> +			intel_encoder->base.name, connector->name);
> +
> +	if (config->hdmi_high_tmds_clock_ratio) {
> +		/* Set TMDS bit clock ratio to 1/40 or 1/10 */
> +		ret = drm_scdc_set_high_tmds_clock_ratio(adptr, enable);
> +		if (!ret) {
> +			DRM_ERROR("Set TMDS ratio failed\n");
> +			return;
> +		}
> +	}
> +
> +	if (config->hdmi_scrambling) {
> +		/* Enable/disable sink scrambling */
> +		ret = drm_scdc_set_scrambling(adptr, enable);
> +		if (!ret) {
> +			DRM_ERROR("Set sink scrambling failed\n");
> +			return;
> +		}
> +	}
> +
> +	DRM_DEBUG_KMS("sink scrambling handled\n");
> +}
> +
>  static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
>  			     enum port port)
>  {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

RE: [PATCH v9] drm/i915: enable scrambling

[Sharma, Shashank]

+ intel-gfx 
+ dri-devl
(missed in in-reply-to)

Regards
Shashank
-----Original Message-----
From: Sharma, Shashank 
Sent: Thursday, March 9, 2017 6:02 PM
To: ville.syrjala@linux.intel.com; conselvan2@gmail.com
Cc: Sharma, Shashank <shashank.sharma@intel.com>
Subject: [PATCH v9] drm/i915: enable scrambling

Geminilake platform sports a native HDMI 2.0 controller, and is capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec mendates scrambling for these higher clocks, for reduced RF footprint.

This patch checks if the monitor supports scrambling, and if required, enables it during the modeset.

V2: Addressed review comments from Ville:
 - Do not track scrambling status in DRM layer, track somewhere in
   driver like in intel_crtc_state.
 - Don't talk to monitor at such a low layer, set monitor scrambling
   in intel_enable_ddi() before enabling the port.

V3: Addressed review comments from Jani
 - In comments, function names, use "sink" instead of "monitor",
   so that the implementation could be close to the language of
   HDMI spec.

V4: Addressed review comment from Maarten
 - scrambling -> hdmi_scrambling
 - high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio

V5: Addressed review comments from Ville and Ander
 - Do not modifiy the crtc_state after compute_config. Move all
   scrambling and tmds_clock_ratio calcutations to compute_config.
 - While setting scrambling for source/sink, do not check the
   conditions again, just go by the crtc_state flags. This will
   simplyfy the condition checks.

V6: Addressed review comments from Ville
 - Do not add IS_GLK check in disable/enable function, instead add it
   in compute_config, while setting state flags.
 - Remove unnecessary paranthesis.
 - Simplyfy handle_sink_scrambling function as suggested.
 - Add readout code for scrambling status in get_ddi_config and add a
   check for the same in pipe_config_compare.

V7: Addressed review comments from Ander/Ville
 - No separate function for source scrambling, make it inline
 - Align the last line of the macro TRANS_DDI_HDMI_SCRAMBLING_MASK
 - Do not add platform check while setting source scrambling
 - Use pipe_config instead of crtc->config to set sink scrambling
 - To readout scrambling status, Compare with SCRAMBLING_MASK
   not any of its bits
 - Remove platform check in intel_pipe_config_compare while checking
   scrambling status

V8: Fixed mege conflict, Addressed review comments from Ander
 - Remove the desciption/comment about scrambling fom the caller, move
   it to the function
 - Move the IS_GLK check into scrambling function
 - Fix alignment

V9: Fixed review comments from Ville, Ander
 - Pass the scrambling state variables as bool input to the sink_scrambling
   function and let the disable call be unconditional.
 - Fix alignments in function calls and debug messages.
 - Add kernel doc for function intel_hdmi_handle_sink_scrambling

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  7 ++++
 drivers/gpu/drm/i915/intel_ddi.c     | 23 +++++++++++++
 drivers/gpu/drm/i915/intel_display.c |  3 ++
 drivers/gpu/drm/i915/intel_drv.h     | 10 ++++++
 drivers/gpu/drm/i915/intel_hdmi.c    | 63 ++++++++++++++++++++++++++++++++++++
 5 files changed, 106 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index cc843f9..2d50fdc 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7830,7 +7830,14 @@ enum {
 #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5<<12)
 #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6<<12)
 #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC	(1<<8)
+#define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7) #define  
+TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6)
 #define  TRANS_DDI_BFI_ENABLE		(1<<4)
+#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE	(1<<4)
+#define  TRANS_DDI_HDMI_SCRAMBLING	(1<<0)
+#define  TRANS_DDI_HDMI_SCRAMBLING_MASK (TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE \
+					| TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ \
+					| TRANS_DDI_HDMI_SCRAMBLING)
 
 /* DisplayPort Transport Control */
 #define _DP_TP_CTL_A			0x64040
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index ee341ef..169d2b4 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1309,6 +1309,11 @@ void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state)
 			temp |= TRANS_DDI_MODE_SELECT_HDMI;
 		else
 			temp |= TRANS_DDI_MODE_SELECT_DVI;
+
+		if (crtc_state->hdmi_scrambling)
+			temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
+		if (crtc_state->hdmi_high_tmds_clock_ratio)
+			temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
 	} else if (type == INTEL_OUTPUT_ANALOG) {
 		temp |= TRANS_DDI_MODE_SELECT_FDI;
 		temp |= (crtc_state->fdi_lanes - 1) << 1; @@ -1881,6 +1886,12 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder,
 	if (type == INTEL_OUTPUT_HDMI) {
 		struct intel_digital_port *intel_dig_port =
 			enc_to_dig_port(encoder);
+		bool clock_ratio = pipe_config->hdmi_high_tmds_clock_ratio;
+		bool scrambling = pipe_config->hdmi_scrambling;
+
+		intel_hdmi_handle_sink_scrambling(intel_encoder,
+						  conn_state->connector,
+						  clock_ratio, scrambling);
 
 		/* In HDMI/DVI mode, the port width, and swing/emphasis values
 		 * are ignored so nothing special needs to be done besides @@ -1914,6 +1925,12 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
 	if (old_crtc_state->has_audio)
 		intel_audio_codec_disable(intel_encoder);
 
+	if (type == INTEL_OUTPUT_HDMI) {
+		intel_hdmi_handle_sink_scrambling(intel_encoder,
+						  old_conn_state->connector,
+						  false, false);
+	}
+
 	if (type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
@@ -2040,6 +2057,12 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 
 		if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
 			pipe_config->has_infoframe = true;
+
+		if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) ==
+			TRANS_DDI_HDMI_SCRAMBLING_MASK)
+			pipe_config->hdmi_scrambling = true;
+		if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
+			pipe_config->hdmi_high_tmds_clock_ratio = true;
 		/* fall through */
 	case TRANS_DDI_MODE_SELECT_DVI:
 		pipe_config->lane_count = 4;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e77ca7d..fb5e670 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11701,6 +11701,9 @@ static void __printf(3, 4)
 	if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
 	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		PIPE_CONF_CHECK_I(limited_color_range);
+
+	PIPE_CONF_CHECK_I(hdmi_scrambling);
+	PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio);
 	PIPE_CONF_CHECK_I(has_infoframe);
 
 	PIPE_CONF_CHECK_I(has_audio);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3af20c1..efb11cc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -729,6 +729,12 @@ struct intel_crtc_state {
 
 	/* bitmask of visible planes (enum plane_id) */
 	u8 active_planes;
+
+	/* HDMI scrambling status */
+	bool hdmi_scrambling;
+
+	/* HDMI High TMDS char rate ratio */
+	bool hdmi_high_tmds_clock_ratio;
 };
 
 struct intel_crtc {
@@ -1623,6 +1629,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,  bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 			       struct intel_crtc_state *pipe_config,
 			       struct drm_connector_state *conn_state);
+void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
+				       struct drm_connector *connector,
+				       bool high_tmds_clock_ratio,
+				       bool scrambling);
 void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
 
 
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index c2184f7..23b3387 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -34,6 +34,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_edid.h>
+#include <drm/drm_scdc_helper.h>
 #include "intel_drv.h"
 #include <drm/i915_drm.h>
 #include <drm/intel_lpe_audio.h>
@@ -1316,6 +1317,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
+	struct drm_scdc *scdc = 
+&conn_state->connector->display_info.hdmi.scdc;
 	int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
 	int clock_12bpc = clock_8bpc * 3 / 2;
 	int desired_bpp;
@@ -1385,6 +1387,16 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 
 	pipe_config->lane_count = 4;
 
+	if (scdc->scrambling.supported && IS_GEMINILAKE(dev_priv)) {
+		if (scdc->scrambling.low_rates)
+			pipe_config->hdmi_scrambling = true;
+
+		if (pipe_config->port_clock > 340000) {
+			pipe_config->hdmi_scrambling = true;
+			pipe_config->hdmi_high_tmds_clock_ratio = true;
+		}
+	}
+
 	return true;
 }
 
@@ -1794,6 +1806,57 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
 	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;  }
 
+/*
+ * intel_hdmi_handle_sink_scrambling: handle sink scrambling/clock 
+ratio setup
+ * @encoder: intel_encoder
+ * @connector: drm_connector
+ * @high_tmds_clock_ratio = bool to indicate if the function needs to 
+set
+ *  or reset the high tmds clock ratio for scrambling
+ * @scrambling: bool to Indicate if the function needs to set or reset
+ *  sink scrambling
+ *
+ * This function handles scrambling on HDMI 2.0 capable sinks.
+ * If required clock rate is > 340 Mhz && scrambling is supported by 
+sink
+ * it enables scrambling. This should be called before enabling the 
+HDMI
+ * 2.0 port, as the sink can choose to disable the scrambling if it 
+doesn't
+ * detect a scrambled clock within 100 ms.
+ */
+void intel_hdmi_handle_sink_scrambling(struct intel_encoder *encoder,
+				       struct drm_connector *connector,
+				       bool high_tmds_clock_ratio,
+				       bool scrambling)
+{
+	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
+	struct drm_i915_private *dev_priv = connector->dev->dev_private;
+	struct drm_scrambling *sink_scrambling =
+				&connector->display_info.hdmi.scdc.scrambling;
+	struct i2c_adapter *adptr = intel_gmbus_get_adapter(dev_priv,
+							   intel_hdmi->ddc_bus);
+	bool ret;
+
+	if (!sink_scrambling->supported)
+		return;
+
+	DRM_DEBUG_KMS("Setting sink scrambling for enc:%s connector:%s\n",
+		      encoder->base.name, connector->name);
+
+	/* Set TMDS bit clock ratio to 1/40 or 1/10 */
+	ret = drm_scdc_set_high_tmds_clock_ratio(adptr, high_tmds_clock_ratio);
+	if (!ret) {
+		DRM_ERROR("Set TMDS ratio failed\n");
+		return;
+	}
+
+	/* Enable/disable sink scrambling */
+	ret = drm_scdc_set_scrambling(adptr, scrambling);
+	if (!ret) {
+		DRM_ERROR("Set sink scrambling failed\n");
+		return;
+	}
+
+	DRM_DEBUG_KMS("sink scrambling handled\n"); }
+
 static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
 			     enum port port)
 {
--
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel