Re: [PATCH v2 19/23] drm/amd/display: Add debug flags for USB4 DP link training

[Harry Wentland <harry.wentland@amd.com>]

On 2021-10-06 06:14, Lin, Wayne wrote:
> [Public]
> 
>> -----Original Message-----
>> From: Wentland, Harry <Harry.Wentland@amd.com>
>> Sent: Wednesday, October 6, 2021 1:11 AM
>> To: Lin, Wayne <Wayne.Lin@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>; Siqueira, Rodrigo
>> <Rodrigo.Siqueira@amd.com>; Wang, Chao-kai (Stylon) <Stylon.Wang@amd.com>; Shih, Jude <Jude.Shih@amd.com>; Kizito, Jimmy
>> <Jimmy.Kizito@amd.com>; Somasundaram, Meenakshikumar <Meenakshikumar.Somasundaram@amd.com>; Lei, Jun <Jun.Lei@amd.com>
>> Subject: Re: [PATCH v2 19/23] drm/amd/display: Add debug flags for USB4 DP link training
>>
>>
>>
>> On 2021-10-05 03:52, Wayne Lin wrote:
>>> From: Jimmy Kizito <Jimmy.Kizito@amd.com>
>>>
>>> [Why & How]
>>> Additional debug flags that can be useful for testing USB4 DP link
>>> training.
>>>
>>> Add flags:
>>> - 0x2 : Forces USB4 DP link to non-LTTPR mode
>>> - 0x4 : Extends status read intervals to about 60s.
>>>
>>> Reviewed-by: Meenakshikumar Somasundaram
>>> <meenakshikumar.somasundaram@amd.com>
>>> Reviewed-by: Jun Lei <Jun.Lei@amd.com>
>>> Acked-by: Wayne Lin <Wayne.Lin@amd.com>
>>> Acked-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>>> Signed-off-by: Jimmy Kizito <Jimmy.Kizito@amd.com>
>>> ---
>>>  drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c   | 6 ++++++
>>>  drivers/gpu/drm/amd/display/dc/core/dc_link_dpia.c | 6 ++++++
>>>  drivers/gpu/drm/amd/display/dc/dc.h                | 4 +++-
>>>  drivers/gpu/drm/amd/display/dc/inc/dc_link_dpia.h  | 3 +++
>>>  4 files changed, 18 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
>>> b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
>>> index bfba1d2c6a18..423fbd2b9b39 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
>>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
>>> @@ -4528,6 +4528,12 @@ bool dp_retrieve_lttpr_cap(struct dc_link *link)
>>>             else
>>>                     link->lttpr_mode = LTTPR_MODE_NON_TRANSPARENT;
>>>     }
>>> +#if defined(CONFIG_DRM_AMD_DC_DCN)
>>
>> Why is this guarded with DC_DCN when all other DPIA code isn't?
>> It looks like it might be unnecessary.
> Thanks Harry.
> 
> Since declaration of dpia_debug variable is guarded by CONFIG_DRM_AMD_DC_DCN,
> we should keep this here.
> 

Ah, that's the one I was missing.

We could probably move it out of the DCN guard in patch 16 but
that can be done with a follow-up patch.

Technically DPIA only makes sense for DCN but there is no reason
to guard it specifically for DCN. The only reason we have the DCN
guard is to allow builds of our driver without floating point on
older HW. I wonder if that's even still needed since we now have
the fixups of the floating point stuff for PPC and ARM.

Harry

> Thanks!
>>
>>> +   /* Check DP tunnel LTTPR mode debug option. */
>>> +   if (link->ep_type == DISPLAY_ENDPOINT_USB4_DPIA &&
>>> +       link->dc->debug.dpia_debug.bits.force_non_lttpr)
>>> +           link->lttpr_mode = LTTPR_MODE_NON_LTTPR; #endif
>>>
>>>     if (link->lttpr_mode == LTTPR_MODE_NON_TRANSPARENT || link->lttpr_mode == LTTPR_MODE_TRANSPARENT) {
>>>             /* By reading LTTPR capability, RX assumes that we will enable diff
>>> --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dpia.c
>>> b/drivers/gpu/drm/amd/display/dc/core/dc_link_dpia.c
>>> index 7407c755a73e..ce15a38c2aea 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dpia.c
>>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dpia.c
>>> @@ -528,6 +528,12 @@ static uint32_t dpia_get_eq_aux_rd_interval(const struct dc_link *link,
>>>                             dp_translate_training_aux_read_interval(
>>>                                     link->dpcd_caps.lttpr_caps.aux_rd_interval[hop - 1]);
>>>
>>> +#if defined(CONFIG_DRM_AMD_DC_DCN)
>>
>> Same here. Please drop this guard if we don't need it.
>>
>> Harry
>>
>>> +   /* Check debug option for extending aux read interval. */
>>> +   if (link->dc->debug.dpia_debug.bits.extend_aux_rd_interval)
>>> +           wait_time_microsec = DPIA_DEBUG_EXTENDED_AUX_RD_INTERVAL_US;
>>> +#endif
>>> +
>>>     return wait_time_microsec;
>>>  }
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/dc.h
>>> b/drivers/gpu/drm/amd/display/dc/dc.h
>>> index e3f884942e04..86fa94a2ef48 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/dc.h
>>> +++ b/drivers/gpu/drm/amd/display/dc/dc.h
>>> @@ -499,7 +499,9 @@ union root_clock_optimization_options {  union
>>> dpia_debug_options {
>>>     struct {
>>>             uint32_t disable_dpia:1;
>>> -           uint32_t reserved:31;
>>> +           uint32_t force_non_lttpr:1;
>>> +           uint32_t extend_aux_rd_interval:1;
>>> +           uint32_t reserved:29;
>>>     } bits;
>>>     uint32_t raw;
>>>  };
>>> diff --git a/drivers/gpu/drm/amd/display/dc/inc/dc_link_dpia.h
>>> b/drivers/gpu/drm/amd/display/dc/inc/dc_link_dpia.h
>>> index 790b904e37e1..e3dfe4c89ce0 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/inc/dc_link_dpia.h
>>> +++ b/drivers/gpu/drm/amd/display/dc/inc/dc_link_dpia.h
>>> @@ -34,6 +34,9 @@ struct dc_link_settings;
>>>  /* The approximate time (us) it takes to transmit 9 USB4 DP clock
>>> sync packets. */  #define DPIA_CLK_SYNC_DELAY 16000
>>>
>>> +/* Extend interval between training status checks for manual testing.
>>> +*/ #define DPIA_DEBUG_EXTENDED_AUX_RD_INTERVAL_US 60000000
>>> +
>>>  /** @note Can remove once DP tunneling registers in upstream
>>> include/drm/drm_dp_helper.h */
>>>  /* DPCD DP Tunneling over USB4 */
>>>  #define DP_TUNNELING_CAPABILITIES_SUPPORT 0xe000d
>>>
> --
> Regards,
> Wayne
>