On Tue, 31 Dec 2024, "Vodapalli, Ravi Kumar" <ravi.kumar.vodapalli@xxxxxxxxx> wrote:
> On 12/13/2024 11:33 AM, Suraj Kandpal wrote:
>> Usually retimers take around 30 to 40ms to exit all devices from
>> sleep state. Extended wake timeout request helps to give additional
>> time by reading the DPCD register through which sink requests the
>> minimal amount of time required to wake the sink up and giving the
>> same amount of wait requested by sink device.
>> Spec: DP v2.1 Section 3.6.12.3
>>
>> Signed-off-by: Suraj Kandpal <suraj.kandpal@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/display/intel_ddi.c | 2 +
>> .../drm/i915/display/intel_dp_link_training.c | 46 +++++++++++++++++++
>> .../drm/i915/display/intel_dp_link_training.h | 1 +
>> 3 files changed, 49 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index 4f9c50996446..d092c3ba0ccf 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -2624,6 +2624,8 @@ static void mtl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>> if (!is_mst)
>> intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
>>
>> + intel_dp_lttpr_wake_timeout_setup(intel_dp);
>> +
>> intel_dp_configure_protocol_converter(intel_dp, crtc_state);
>> if (!is_mst)
>> intel_dp_sink_enable_decompression(state,
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> index ea9b4730a176..d0f0da78794e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> @@ -134,6 +134,52 @@ static bool intel_dp_lttpr_transparent_mode_enabled(struct intel_dp *intel_dp)
>> DP_PHY_REPEATER_MODE_TRANSPARENT;
>> }
>>
>> +void intel_dp_lttpr_wake_timeout_setup(struct intel_dp *intel_dp)
>> +{
>> + struct intel_display *display = to_intel_display(intel_dp);
>> + u8 val = 1;
>> + int ret;
>> +
>> + if (intel_dp_lttpr_transparent_mode_enabled(intel_dp)) {
>> + static const u8 timeout_mapping[] = {
>> + [DP_DPRX_SLEEP_WAKE_TIMEOUT_PERIOD_1_MS] = 1,
>> + [DP_DPRX_SLEEP_WAKE_TIMEOUT_PERIOD_20_MS] = 20,
>> + [DP_DPRX_SLEEP_WAKE_TIMEOUT_PERIOD_40_MS] = 40,
>> + [DP_DPRX_SLEEP_WAKE_TIMEOUT_PERIOD_20_MS] = 20,
>> + [DP_DPRX_SLEEP_WAKE_TIMEOUT_PERIOD_80_MS] = 80,
>> + [DP_DPRX_SLEEP_WAKE_TIMEOUT_PERIOD_100_MS] = 100,
>> + };
Btw this kind of stuff totally belongs in generic DP helpers instead of
our driver.
>> +
>> + ret = drm_dp_dpcd_readb(&intel_dp->aux,
>> + DP_EXTENDED_DPRX_SLEEP_WAKE_TIMEOUT_REQUEST, &val);
>> + if (ret != 1) {
>> + drm_dbg_kms(display->drm,
>> + "Failed to read Extended sleep wake timeout request\n");
>> + return;
>
> Returning from function without return type, better to declare int in
> place of void and return the error value.
That depends on what you're going to do with that error value. We're not
going to check it anyway, are we?
>
> int intel_dp_lttpr_wake_timeout_setup(struct intel_dp *intel_dp)
>
>
> Regards,
> Ravi Kumar V
>> + }
>> +
>> + val = (val < sizeof(timeout_mapping) && timeout_mapping[val]) ?
>> + timeout_mapping[val] : 1;
What's the point with this? We don't do anything with val?
BR,
Jani.
>> +
>> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_EXTENDED_DPRX_SLEEP_WAKE_TIMEOUT_GRANT,
>> + DP_DPRX_SLEEP_WAKE_TIMEOUT_PERIOD_GRANTED);
>> + } else {
>> + ret = drm_dp_dpcd_readb(&intel_dp->aux,
>> + DP_PHY_REPEATER_EXTENDED_WAIT_TIMEOUT, &val);
>> + if (ret != 1) {
>> + drm_dbg_kms(display->drm,
>> + "Failed to read Extended sleep wake timeout request\n");
>> + return;
>> + }
>> +
>> + val = (val & DP_EXTENDED_WAKE_TIMEOUT_REQUEST_MASK) ?
>> + (val & DP_EXTENDED_WAKE_TIMEOUT_REQUEST_MASK) * 10 : 1;
>> +
>> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_PHY_REPEATER_EXTENDED_WAIT_TIMEOUT,
>> + DP_EXTENDED_WAKE_TIMEOUT_GRANT);
>> + }
>> +}
>> +
>> /*
>> * Read the LTTPR common capabilities and switch the LTTPR PHYs to
>> * non-transparent mode if this is supported. Preserve the
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.h b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
>> index 2066b9146762..cd4e0d6db6ed 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.h
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
>> @@ -15,6 +15,7 @@ struct intel_dp;
>>
>> int intel_dp_read_dprx_caps(struct intel_dp *intel_dp, u8 dpcd[DP_RECEIVER_CAP_SIZE]);
>> int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp *intel_dp);
>> +void intel_dp_lttpr_wake_timeout_setup(struct intel_dp *intel_dp);
>>
>> void intel_dp_link_training_set_mode(struct intel_dp *intel_dp,
>> int link_rate, bool is_vrr);
>
--
Jani Nikula, Intel
