Re: [PATCH v3] drm/i915/dp: Increase idle pattern wait timeout to 2ms

["Chauhan, Shekhar" <shekhar.chauhan@intel.com>]

On 3/4/2024 14:16, Jani Nikula wrote:
> On Mon, 04 Mar 2024, Shekhar Chauhan <shekhar.chauhan@intel.com> wrote:
>> Currently, the driver is only waiting for 1ms for
>> idle patterns. But starting from LNL and beyond,
>> the MST wants the driver to wait for 1640us before
> What does it mean that "the MST wants"?
I wanted to convey that MST streams require the driver to wait for 2ms. 
I'll rephrase.
>
>> timing out (which we round up to 2ms).
>>
>> v1: Introduced the 2ms wait timeout.
>> v2: Segregated the wait timeout for platforms before & after LNL.
> I did not ask for this. I would rather all platforms used 2 ms. I even
> said the original change looked fine. But I'd like it to be explained in
> the commit message.
I felt that when you said "why bump for non lnl platforms", I felt that 
I should be segregating them.
But, as discussed offline, I'll revert back to the original design with 
a change in the commit message.
>
>> v3: Fixed 2 cosmetic changes.
>>
>> BSpec: 68849
>> Signed-off-by: Shekhar Chauhan <shekhar.chauhan@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_ddi.c | 15 +++++++++++----
>>   1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index bea441590204..b59adb7685b8 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -3677,12 +3677,19 @@ static void intel_ddi_set_idle_link_train(struct intel_dp *intel_dp,
>>   	 */
>>   	if (port == PORT_A && DISPLAY_VER(dev_priv) < 12)
>>   		return;
>> -
>> -	if (intel_de_wait_for_set(dev_priv,
>> -				  dp_tp_status_reg(encoder, crtc_state),
>> -				  DP_TP_STATUS_IDLE_DONE, 1))
>> +	if (DISPLAY_VER(dev_priv) >= 20) {
>> +		if (intel_de_wait_for_set(dev_priv,
>> +			dp_tp_status_reg(encoder, crtc_state),
>> +			DP_TP_STATUS_IDLE_DONE, 2))
>> +		drm_err(&dev_priv->drm,
>> +			"Timed out waiting for DP idle patterns\n");
>> +	} else {
>> +		if (intel_de_wait_for_set(dev_priv,
>> +			dp_tp_status_reg(encoder, crtc_state),
>> +			DP_TP_STATUS_IDLE_DONE, 1))
>>   		drm_err(&dev_priv->drm,
>>   			"Timed out waiting for DP idle patterns\n");
>> +	}
> So I'd like you to go back to how it was originally.
>
>
>>   }
>>   
>>   static bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,

-- 
-shekhar