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

Regards

Shashank

On 3/1/2017 8:28 PM, Ville Syrjälä wrote:

On Tue, Feb 28, 2017 at 02:09:08PM +0530, Shashank Sharma wrote:

...

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.

Signed-off-by: Shashank Sharma shashank.sharma@intel.com Reviewed-by: Thierry Reding treding@nvidia.com

---

drivers/gpu/drm/drm_edid.c | 33 +++++++++++- drivers/gpu/drm/drm_scdc_helper.c | 111 ++++++++++++++++++++++++++++++++++++++ include/drm/drm_connector.h | 19 +++++++ include/drm/drm_edid.h | 6 ++- include/drm/drm_scdc_helper.h | 27 ++++++++++ 5 files changed, 194 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 0881108..e84c5ab 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"

@@ -3817,13 +3818,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..180a7cd 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,112 @@ 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
• • 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

The 1/10 and 1/40 ratios confuse me a little since we never really deal with the bitrate in the driver. Instead we always deal with either pixel clock, character clock or TMDS clock (well prior to this TMDS clock == character clock). So the factor 10 never enters the picture elsewhere. So I would prefer to specify these in those terms as well.

Ok, I will add comments for pixel clock as well

An enum for the ratios might also help in making this a bit less cryptic.

Humm, not sure how helpful will be an enum, as thats only used in case of pixelclock>340 asso, only two entries 1/40 and 1/10, but let me see if I can do that.

...

• • 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 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 3ead84d..f4a91e0 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -476,5 +476,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);

Old leftovers?

Oops, my bad.

...

#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