[Public] > -----Original Message----- > From: Wentland, Harry <Harry.Wentland@xxxxxxx> > Sent: Wednesday, October 6, 2021 10:03 PM > To: Lin, Wayne <Wayne.Lin@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Kazlauskas, Nicholas <Nicholas.Kazlauskas@xxxxxxx>; Siqueira, Rodrigo > <Rodrigo.Siqueira@xxxxxxx>; Wang, Chao-kai (Stylon) <Stylon.Wang@xxxxxxx>; Shih, Jude <Jude.Shih@xxxxxxx>; Kizito, Jimmy > <Jimmy.Kizito@xxxxxxx>; Somasundaram, Meenakshikumar <Meenakshikumar.Somasundaram@xxxxxxx>; Lei, Jun > <Jun.Lei@xxxxxxx> > 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@xxxxxxx> > >> Sent: Wednesday, October 6, 2021 1:11 AM > >> To: Lin, Wayne <Wayne.Lin@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx > >> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Kazlauskas, > >> Nicholas <Nicholas.Kazlauskas@xxxxxxx>; Siqueira, Rodrigo > >> <Rodrigo.Siqueira@xxxxxxx>; Wang, Chao-kai (Stylon) > >> <Stylon.Wang@xxxxxxx>; Shih, Jude <Jude.Shih@xxxxxxx>; Kizito, Jimmy > >> <Jimmy.Kizito@xxxxxxx>; Somasundaram, Meenakshikumar > >> <Meenakshikumar.Somasundaram@xxxxxxx>; Lei, Jun <Jun.Lei@xxxxxxx> > >> 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@xxxxxxx> > >>> > >>> [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@xxxxxxx> > >>> Reviewed-by: Jun Lei <Jun.Lei@xxxxxxx> > >>> Acked-by: Wayne Lin <Wayne.Lin@xxxxxxx> > >>> Acked-by: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx> > >>> Signed-off-by: Jimmy Kizito <Jimmy.Kizito@xxxxxxx> > >>> --- > >>> 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