Re: [Intel-gfx] [PATCH v2] drm/i915/hdmi: Go for scrambling only if platform supports TMDS clock > 340MHz

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 12/20/2022 2:31 PM, Jani Nikula wrote:
> On Tue, 20 Dec 2022, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
>> There are cases, where devices have an HDMI1.4 retimer, and TMDS clock rate
>> is capped to 340MHz via VBT. In such cases scrambling might be supported
>> by the platform and an HDMI2.0 sink for lower TMDS rates, but not
>> supported by the retimer, causing blankouts.
>>
>> So avoid enabling scrambling, if the TMDS clock is capped to <= 340MHz.
>>
>> v2: Added comment, documenting the rationale to check for TMDS clock,
>> before going for scrambling. (Arun)
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_hdmi.c | 21 ++++++++++++++++++++-
>>   1 file changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> index efa2da080f62..7603426af9a0 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> @@ -2244,6 +2244,25 @@ static bool intel_hdmi_is_cloned(const struct intel_crtc_state *crtc_state)
>>   		!is_power_of_2(crtc_state->uapi.encoder_mask);
>>   }
>>   
>> +static bool source_can_support_scrambling(struct intel_encoder *encoder)
> "source can support scrambling" reads like "if this function returns
> true, source might support scrambing, but also might not, depends on
> something else".
>
> So does this mean "source supports scrambling" or "source can support
> scrambling"? If the latter, as it is now named, what *else* is required
> for source to support scrambling?

Hmm.. the intention is to have a function to check if source supports 
scrambling.

Sink side support is checked by 'scdc->scrambling.supported' so we want 
to have a function that tells us, whether source supports scrambling.

I will modify the function name to : source_supports_scrambling() to 
avoid the confusion.

Thanks & Regards,

Ankit


> BR,
> Jani.
>
>
>
>> +{
>> +	/*
>> +	 * Gen 10+ support HDMI 2.0 : the max tmds clock is 594MHz, and
>> +	 * scrambling is supported.
>> +	 * But there seem to be cases where certain platforms that support
>> +	 * HDMI 2.0, have an HDMI1.4 retimer chip, and the max tmds clock is
>> +	 * capped by VBT to less than 340MHz.
>> +	 *
>> +	 * In such cases when an HDMI2.0 sink is connected, it creates a
>> +	 * problem : the platform and the sink both support scrambling but the
>> +	 * HDMI 1.4 retimer chip doesn't.
>> +	 *
>> +	 * So go for scrambling, based on the max tmds clock taking into account,
>> +	 * restrictions coming from VBT.
>> +	 */
>> +	return intel_hdmi_source_max_tmds_clock(encoder) > 340000;
>> +}
>> +
>>   int intel_hdmi_compute_config(struct intel_encoder *encoder,
>>   			      struct intel_crtc_state *pipe_config,
>>   			      struct drm_connector_state *conn_state)
>> @@ -2301,7 +2320,7 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
>>   
>>   	pipe_config->lane_count = 4;
>>   
>> -	if (scdc->scrambling.supported && DISPLAY_VER(dev_priv) >= 10) {
>> +	if (scdc->scrambling.supported && source_can_support_scrambling(encoder)) {
>>   		if (scdc->scrambling.low_rates)
>>   			pipe_config->hdmi_scrambling = true;