Re: [PATCH v2 4/6] drm: scrambling support in drm layer

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

 



Regards

Shashank


On 2/7/2017 4:44 PM, Jose Abreu wrote:
Hi Shashank,



On 06-02-2017 13:59, Shashank Sharma wrote:
HDMI 2.0 spec mandates scrambling for modes with pixel clock higher
than 340 MHz. This patch adds few new functions in drm layer for
core drivers to enable/disable scrambling.

This patch adds:
- A function to detect scrambling support parsing HF-VSDB
- A function to check scrambling status runtime using SCDC read.
- Two functions to enable/disable scrambling using SCDC read/write.
- Few new bools to reflect scrambling support and status.

V2: Addressed review comments from Thierry, Ville and Dhinakaran
Thierry:
- Mhz -> MHz in comments and commit message.
- i2c -> I2C in comments.
- Fix the function documentations, keep in sync with drm_scdc_helper.c
- drm_connector -> DRM connector.
- Create structure for SCDC, and save scrambling status inside that,
  in a sub-structure.
- Call this sub-structure scrambling instead of scr_info.
- low_rates -> low_clocks in scrambling status structure.
- Store the return value of I2C read/write and print the error code
    in case of failure.

Thierry and Ville:
- Move the scrambling enable/disable/query functions in
  drm_scdc_helper.c file.
- Add drm_SCDC prefix for the functions.
- Optimize the return statement from function
  drm_SCDC_check_scrambling_status.

Ville:
- Dont overwrite saved max TMDS clock value in display_info,
  if max tmds clock from HF-VSDB is not > 340 MHz.
- drm_detect_hdmi_scrambling -> drm_parse_hdmi_forum_vsdb.
- Remove dynamic tracking of SCDC status from DRM layer, force bool.
- Program clock ratio bit also, while enabling scrambling.

Dhinakaran:
 - Add a comment about *5000 while extracting max clock supported.

Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
---
 drivers/gpu/drm/drm_edid.c        |  33 ++++++++++++-
 drivers/gpu/drm/drm_scdc_helper.c | 100 ++++++++++++++++++++++++++++++++++++++
 include/drm/drm_connector.h       |  19 ++++++++
 include/drm/drm_edid.h            |   6 ++-
 include/drm/drm_scdc_helper.h     |  20 ++++++++
 5 files changed, 176 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index a487b80..dc85eb9 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>
 
 #define version_greater(edid, maj, min) \
 	(((edid)->version > (maj)) || \
@@ -3805,13 +3806,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))
BIT(3) ?
Yes, bit 3 is LTE_340Mcsc_scramble, indicating that the sink support scrambling at rates below 340Mhz too, isn't it ?

+				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 fe0e121..311f62e 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
@@ -109,3 +111,101 @@ ssize_t drm_scdc_write(struct i2c_adapter *adapter, u8 offset,
 	return err;
 }
 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_check_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;
"return (status & SCDC_SCRAMBLING_STATUS) > 0;" ?
I think Jani has made an agreement already on this.

+}
+EXPORT_SYMBOL(drm_scdc_check_scrambling_status);
+
+/**
+ * drm_scdc_enable_scrambling - enable scrambling
+ * @adapter: I2C adapter for DDC channel
+ *
+ * Writes the TMDS config over SCDC channel, and enables scrambling
+ * Returns:
+ * True if scrambling is successfully enabled, false otherwise.
+ */
+
+bool drm_scdc_enable_scrambling(struct i2c_adapter *adapter)
+{
+	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;
+	}
+
+	config |= (SCDC_SCRAMBLING_ENABLE | SCDC_TMDS_BIT_CLOCK_RATIO_BY_40);
Hmm, I did not read the spec exhaustively but shouldn't the clock
ratio by 40 only be set for data rates above 3.4Gbps?
You are right, for few monitors scrambling can be done below 340 MHz too, and I am not sure if we should
set the clock ratio bit on that. Let me check the spec for those cases. 

      
+
+	ret = drm_scdc_writeb(adapter, SCDC_TMDS_CONFIG, config);
+	if (ret < 0) {
+		DRM_ERROR("Failed to enable scrambling, error %d\n", ret);
+		return false;
+	}
+
+	/*
+	 * The spec says that the source should wait min 1ms and max 100ms
+	 * after writing the TMDS config for clock ratio. Lets obey the spec.
+	 */
+	usleep_range(1000, 100000);
Shall we read here the clock_detected status to make sure the
sink is okay?
This is optional in spec, so I am afraid few monitors wont implement this, and we will unnecessary add
lot of noise in code. Do you think so ?

- Shashank

      
+	return true;
+}
+EXPORT_SYMBOL(drm_scdc_enable_scrambling);
+
+/**
+ * drm_scdc_disable_scrambling - disable scrambling
+ * @adapter: I2C adapter for DDC channel
+ *
+ * Write the TMDS config over SCDC channel, and disable scrambling
+ * Return: True if scrambling is successfully disabled, false otherwise.
+ */
+bool drm_scdc_disable_scrambling(struct i2c_adapter *adapter)
+{
+	u8 config;
+	int ret;
+
+	ret = drm_scdc_readb(adapter, SCDC_TMDS_CONFIG, &config);
+	if (ret < 0) {
+		DRM_ERROR("Failed to read tmds config, error %d\n", ret);
+		return false;
+	}
+
+	config &= ~(SCDC_SCRAMBLING_ENABLE | SCDC_TMDS_BIT_CLOCK_RATIO_BY_40);
+
+	ret = drm_scdc_writeb(adapter, SCDC_TMDS_CONFIG, config);
+	if (ret < 0) {
+		DRM_ERROR("Failed to enable scrambling, error %d\n", ret);
+		return false;
+	}
+
+	/*
+	 * The spec says that the source should wait min 1ms and max 100ms
+	 * after writing the TMDS config for clock ratio. Lets obey the spec.
+	 */
+	usleep_range(1000, 100000);
+	return true;
+}
+EXPORT_SYMBOL(drm_scdc_disable_scrambling);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 6d5304e..78618308 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 43fb0ac..d24c974 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -462,5 +462,9 @@ 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);
-
+bool drm_enable_scrambling(struct drm_connector *connector,
+				struct i2c_adapter *adapter, bool force);
+bool drm_disable_scrambling(struct drm_connector *connector,
+				struct i2c_adapter *adapter, bool force);
+bool drm_check_scrambling_status(struct i2c_adapter *adapter);
 #endif /* __DRM_EDID_H__ */
diff --git a/include/drm/drm_scdc_helper.h b/include/drm/drm_scdc_helper.h
index 93b07bc..dc727a5 100644
--- a/include/drm/drm_scdc_helper.h
+++ b/include/drm/drm_scdc_helper.h
@@ -129,4 +129,24 @@ static inline int drm_scdc_writeb(struct i2c_adapter *adapter, u8 offset,
 	return drm_scdc_write(adapter, offset, &value, sizeof(value));
 }
 
+/**
+ * drm_scdc_enable_scrambling - enable scrambling
+ * @adapter: I2C adapter for DDC channel
+ *
+ * Writes the TMDS config over SCDC channel, and enables scrambling
+ * Returns:
+ * True if scrambling is successfully enabled, false otherwise.
+ */
+
+bool drm_scdc_enable_scrambling(struct i2c_adapter *adapter);
+
+/**
+ * drm_scdc_disable_scrambling - disable scrambling
+ * @adapter: I2C adapter for DDC channel
+ *
+ * Write the TMDS config over SCDC channel, and disable scrambling
+ * Return: True if scrambling is successfully disabled, false otherwise.
+ */
+bool drm_scdc_disable_scrambling(struct i2c_adapter *adapter);
+
 #endif
Best regards,
Jose Miguel Abreu


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

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux