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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Feb 01, 2017 at 06:14:39PM +0530, Shashank Sharma wrote:
> HDMI 2.0 spec mandates scrambling for modes with pixel clock higher
> than 340Mhz. 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.
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c  | 131 +++++++++++++++++++++++++++++++++++++++++++-
>  include/drm/drm_connector.h |  24 ++++++++
>  include/drm/drm_edid.h      |   6 +-
>  3 files changed, 159 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 37902e5..f0d940a 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)) || \
> @@ -3814,6 +3815,132 @@ static void drm_detect_hdmi_scdc(struct drm_connector *connector,
>  	}
>  }
>  
> +static void drm_detect_hdmi_scrambling(struct drm_connector *connector,
> +				 const u8 *hf_vsdb)

That names seems off. Should probably be drm_parse_hdmi_forum_vsdb() or
something.

> +{
> +	struct drm_display_info *display = &connector->display_info;
> +	struct drm_hdmi_info *hdmi = &display->hdmi_info;
> +
> +	/*
> +	 * All HDMI 2.0 monitors must support scrambling at rates > 340M.
> +	 * 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
> +	 * * SCDC support available
> +	 * Lets check it out.
> +	 */
> +
> +	if (hf_vsdb[5]) {
> +		display->max_tmds_clock = hf_vsdb[5] * 5000;
> +		DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
> +			display->max_tmds_clock);

I wonder if we should be a little paranoid and ignore this if it's
<=340?

> +
> +		if (hdmi->scdc_supported) {
> +			hdmi->scr_info.supported = true;
> +
> +			/* Few sinks support scrambling for cloks < 340M */
> +			if ((hf_vsdb[6] & 0x8))
> +				hdmi->scr_info.low_clocks = true;
> +		}
> +	}
> +}
> +
> +/**
> + * drm_check_scrambling_status - what is status of scrambling?
> + * @adapter: i2c adapter for SCDC channel
> + *
> + * Read the scrambler status over SCDC channel, and check the
> + * scrambling status.
> + *
> + * Return: True if the scrambling is enabled, false otherwise.
> + */
> +
> +bool drm_check_scrambling_status(struct i2c_adapter *adapter)

The name is again a little wonky. And it looks like something that
should live alognside the SCDC helper stuff.

> +{
> +	u8 status;
> +
> +	if (drm_scdc_readb(adapter, SCDC_SCRAMBLER_STATUS, &status) < 0) {
> +		DRM_ERROR("Failed to read scrambling status\n");
> +		return false;
> +	}
> +
> +	status &= SCDC_SCRAMBLING_STATUS;
> +	return status != 0;

return status & SCDC_SCRAMBLING_STATUS

> +}
> +
> +/**
> + * drm_enable_scrambling - enable scrambling
> + * @connector: target drm_connector
> + * @adapter: i2c adapter for SCDC channel
> + * @force: enable scrambling, even if its already enabled
> + *
> + * Write the TMDS config over SCDC channel, and enable scrambling
> + * Return: True if scrambling is successfully enabled, false otherwise.
> + */
> +
> +bool drm_enable_scrambling(struct drm_connector *connector,
> +					struct i2c_adapter *adapter, bool force)
> +{
> +	u8 config;
> +	struct drm_hdmi_info *hdmi_info = &connector->display_info.hdmi_info;
> +
> +	if (hdmi_info->scr_info.status && !force) {
> +		DRM_DEBUG_KMS("Scrambling already enabled\n");
> +		return true;
> +	}

I don't think we want to track any dynamic state in the display info.
That belongs to the atomic state stuff.

And the function again looks like something that belongs in the SCDC
helper.

Since the two pieces of infromaton in this registrer are the scramble
control and the clock ratio, I think we might want to just have one
function to configure both.

> +
> +	if (drm_scdc_readb(adapter, SCDC_TMDS_CONFIG, &config) < 0) {
> +		DRM_ERROR("Failed to read tmds config\n");
> +		return false;
> +	}
> +
> +	config |= SCDC_SCRAMBLING_ENABLE;
> +
> +	if (drm_scdc_writeb(adapter, SCDC_TMDS_CONFIG, config) < 0) {
> +		DRM_ERROR("Failed to enable scrambling, write error\n");
> +		return false;
> +	}
> +
> +	hdmi_info->scr_info.status = drm_check_scrambling_status(adapter);
> +	return hdmi_info->scr_info.status;
> +}
> +
> +/**
> + * drm_disable_scrambling - disable scrambling
> + * @connector: target drm_connector
> + * @adapter: i2c adapter for SCDC channel
> + * @force: disable scrambling, even if its already disabled
> + *
> + * Write the TMDS config over SCDC channel, and disable scrambling
> + * Return: True if scrambling is successfully disabled, false otherwise.
> + */
> +bool drm_disable_scrambling(struct drm_connector *connector,
> +					struct i2c_adapter *adapter, bool force)
> +{
> +	u8 config;
> +	struct drm_hdmi_info *hdmi_info = &connector->display_info.hdmi_info;
> +
> +	if (!hdmi_info->scr_info.status && !force) {
> +		DRM_DEBUG_KMS("Scrambling already disabled\n");
> +		return true;
> +	}
> +
> +	if (drm_scdc_readb(adapter, SCDC_TMDS_CONFIG, &config) < 0) {
> +		DRM_ERROR("Failed to read tmds config\n");
> +		return false;
> +	}
> +
> +	config &= ~SCDC_SCRAMBLING_ENABLE;
> +
> +	if (drm_scdc_writeb(adapter, SCDC_TMDS_CONFIG, config) < 0) {
> +		DRM_ERROR("Failed to enable scrambling, write error\n");
> +		return false;
> +	}
> +
> +	hdmi_info->scr_info.status = drm_check_scrambling_status(adapter);
> +	return !hdmi_info->scr_info.status;
> +}
> +
>  static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
>  					   const u8 *hdmi)
>  {
> @@ -3928,8 +4055,10 @@ 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))
> +		if (cea_db_is_hdmi_forum_vsdb(db)) {
>  			drm_detect_hdmi_scdc(connector, db);
> +			drm_detect_hdmi_scrambling(connector, db);
> +		}
>  	}
>  }
>  
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 2435598..b9735bd 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -90,6 +90,26 @@ enum subpixel_order {
>  
>  };
>  
> +
> +/**
> + * struct scrambling: sink's scrambling support.
> + */
> +struct scrambling {
> +	/**
> +	 * @supported: scrambling supported for rates > 340Mhz.
> +	 */
> +	bool supported;
> +	/**
> +	 * @low_clocks: scrambling supported for rates <= 340Mhz.
> +	 */
> +	bool low_clocks;
> +	/**
> +	 * @status: scrambling enabled/disabled ?
> +	 */
> +	bool status;
> +};
> +
> +
>  /**
>   * struct drm_hdmi_info - runtime data about the connected sink
>   *
> @@ -108,6 +128,10 @@ struct drm_hdmi_info {
>  	 * @scdc_rr: sink is capable of generating scdc read request.
>  	 */
>  	bool scdc_rr;
> +	/**
> +	 * scr_info: sink's scrambling support and capabilities.
> +	 */
> +	struct scrambling scr_info;
>  };
>  
>  /**
> 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__ */
> -- 
> 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