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

[Sharma, Shashank]

Regards

Shashank


On 2/1/2017 10:06 PM, Ville Syrjälä wrote:
> On Wed, Feb 01, 2017 at 06:14:40PM +0530, Shashank Sharma wrote:
>> Geminilake platform has 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.
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma at intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h   |  2 ++
>>   drivers/gpu/drm/i915/intel_ddi.c  |  5 +++++
>>   drivers/gpu/drm/i915/intel_drv.h  |  2 ++
>>   drivers/gpu/drm/i915/intel_hdmi.c | 42 +++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 51 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 495b789..cc85892 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7807,6 +7807,8 @@ enum {
>>   #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6<<12)
>>   #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC	(1<<8)
>>   #define  TRANS_DDI_BFI_ENABLE		(1<<4)
>> +#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE	(1<<4)
>> +#define  TRANS_DDI_HDMI_SCRAMBLING	(1<<0)
>>   
>>   /* 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 9a9a670..aea81ce 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -1278,6 +1278,11 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
>>   			temp |= TRANS_DDI_MODE_SELECT_HDMI;
>>   		else
>>   			temp |= TRANS_DDI_MODE_SELECT_DVI;
>> +
>> +		if (IS_GEMINILAKE(dev_priv)) {
>> +			temp |= intel_hdmi_check_scrambling(intel_encoder,
>> +				&intel_crtc->config->base.adjusted_mode);
>> +		}
>>   	} else if (type == INTEL_OUTPUT_ANALOG) {
>>   		temp |= TRANS_DDI_MODE_SELECT_FDI;
>>   		temp |= (intel_crtc->config->fdi_lanes - 1) << 1;
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 393f243..aafce7f 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1588,6 +1588,8 @@ 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);
>> +uint32_t intel_hdmi_check_scrambling(struct intel_encoder *intel_encoder,
>> +				struct drm_display_mode *mode);
>>   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 ebae2bd..92dd9bc 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -1795,6 +1795,48 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
>>   	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>>   }
>>   
>> +static void
>> +intel_hdmi_enable_scrambling(struct drm_connector *connector)
>> +{
>> +	struct drm_i915_private *dev_priv = connector->dev->dev_private;
>> +	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
>> +	struct i2c_adapter *adapter =
>> +			intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
>> +
>> +	if (!drm_enable_scrambling(connector, adapter, true))
>> +		DRM_ERROR("Request to enable scrambling failed\n");
>> +}
> I don't like hiding this somewhere deep like this. It should be
> somewhere much higher up.
Why ? All we need to do here is enable two bits in transcoder control 
register, which is already being
programmed in a calling function, so I dont see the use case, but I 
might be missing some bigger picture.
Can you please elaborate on this ?
>
> And I'm thinkign we might want to track the scrambler state
> in the crtc state.
Yes, that's a pretty good way to track dynamic status of scrambler, do 
you think we should add this in
drm_crtc_state itself ?

- Shashank
>> +
>> +uint32_t intel_hdmi_check_scrambling(struct intel_encoder *intel_encoder,
>> +				struct drm_display_mode *mode)
>> +{
>> +	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base);
>> +	struct drm_connector *connector = &intel_hdmi->attached_connector->base;
>> +	struct drm_hdmi_info *hdmi_info = &connector->display_info.hdmi_info;
>> +	uint32_t hdmi_config = 0;
>> +
>> +	DRM_DEBUG_KMS("Checking scrambling for enc:%s connector:%s\n",
>> +			intel_encoder->base.name, connector->name);
>> +
>> +	if (mode->clock < 340000) {
>> +		if (hdmi_info->scr_info.low_clocks) {
>> +			intel_hdmi_enable_scrambling(connector);
>> +			hdmi_config |= TRANS_DDI_HDMI_SCRAMBLING;
>> +		}
>> +		return hdmi_config;
>> +	}
>> +
>> +	/* Enable scrambling for clocks > 340M */
>> +	if (hdmi_info->scr_info.supported) {
>> +		intel_hdmi_enable_scrambling(connector);
>> +		hdmi_config |= TRANS_DDI_HDMI_SCRAMBLING;
>> +	}
>> +
>> +	/* Scrambling or not, if clock > 340M, set high char rate */
>> +	hdmi_config |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
>> +	return hdmi_config;
>> +}
>> +
>>   static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
>>   			     enum port port)
>>   {
>> -- 
>> 1.9.1