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

Thu Oct 7 09:22:25 UTC 2021

[Public]

> -----Original Message-----
> From: Wentland, Harry <Harry.Wentland at amd.com>
> Sent: Wednesday, October 6, 2021 10:03 PM
> To: Lin, Wayne <Wayne.Lin at amd.com>; amd-gfx at lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Kazlauskas, Nicholas <Nicholas.Kazlauskas at amd.com>; Siqueira, Rodrigo
> <Rodrigo.Siqueira at amd.com>; Wang, Chao-kai (Stylon) <Stylon.Wang at amd.com>; Shih, Jude <Jude.Shih at amd.com>; Kizito, Jimmy
> <Jimmy.Kizito at amd.com>; Somasundaram, Meenakshikumar <Meenakshikumar.Somasundaram at amd.com>; Lei, Jun
> <Jun.Lei at amd.com>
> Subject: Re: [PATCH v2 19/23] drm/amd/display: Add debug flags for USB4 DP link training
>
>
>
> On 2021-10-06 06:14, Lin, Wayne wrote:
> > [Public]
> >
> >> -----Original Message-----
> >> From: Wentland, Harry <Harry.Wentland at amd.com>
> >> Sent: Wednesday, October 6, 2021 1:11 AM
> >> To: Lin, Wayne <Wayne.Lin at amd.com>; amd-gfx at lists.freedesktop.org
> >> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Kazlauskas,
> >> Nicholas <Nicholas.Kazlauskas at amd.com>; Siqueira, Rodrigo
> >> <Rodrigo.Siqueira at amd.com>; Wang, Chao-kai (Stylon)
> >> <Stylon.Wang at amd.com>; Shih, Jude <Jude.Shih at amd.com>; Kizito, Jimmy
> >> <Jimmy.Kizito at amd.com>; Somasundaram, Meenakshikumar
> >> <Meenakshikumar.Somasundaram at amd.com>; Lei, Jun <Jun.Lei at 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 at 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 at amd.com>
> >>> Reviewed-by: Jun Lei <Jun.Lei at amd.com>
> >>> Acked-by: Wayne Lin <Wayne.Lin at amd.com>
> >>> Acked-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
> >>> Signed-off-by: Jimmy Kizito <Jimmy.Kizito at 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.
Thanks Harry.

Much appreciated for the elaborations. Will give a follow-up patch. Thanks!

>
> 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
> >
--
Regards,
Wayne