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

Sharma, Shashank shashank.sharma at intel.com
Wed Mar 8 18:00:25 UTC 2017


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 at 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 at 60 (scrambling reqd) ->
>>>> 1080 at 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



More information about the Intel-gfx mailing list