On Wed, Jun 21, 2023 at 09:37:32PM +0530, Sanjay R Mehta wrote: > > > On 6/21/2023 6:24 PM, Mika Westerberg wrote: > > On Wed, Jun 21, 2023 at 05:48:21PM +0530, Sanjay R Mehta wrote: > >> > >> > >> On 6/21/2023 4:45 PM, Mika Westerberg wrote: > >>> On Wed, Jun 21, 2023 at 05:37:22AM -0500, Sanjay R Mehta wrote: > >>>> From: Sanath S <Sanath.S@xxxxxxx> > >>>> > >>>> Since TMU is enabled by default on Intel SOCs for USB4 before Alpine > >>>> Ridge, explicit enabling or disabling of TMU is not required. > >>>> > >>>> However, the current implementation of enabling or disabling TMU based > >>>> on CLx state is inadequate as not all SOCs with CLx disabled have TMU > >>>> enabled by default, such as AMD Yellow Carp and Pink Sardine. > >>>> > >>>> To address this, a quirk named "QUIRK_TMU_DEFAULT_ENABLED" is > >>>> implemented to skip the enabling or disabling of TMU for SOCs where it > >>>> is already enabled by default, such as Intel SOCs prior to Alpine Ridge. > >>> > >>> If it is enabled by default "enabling" it again should not be a problem. > >>> Can you elaborate this more? > >> > >> Although that is correct, Mika, we are facing an issue of display > >> flickering on Alpine Ridge and older device routers, from the second > >> hotplug onwards. This issue arises as the TMU is enabled and disabled > >> for each plug and unplug. > > > > Okay thanks for clarifying. > > > >> Upon reviewing the old code, it appears that this issue was already > >> addressed with the following code block: > >> > >> /* > >> * No need to enable TMU on devices that don't support CLx since on > >> * these devices e.g. Alpine Ridge and earlier, the TMU mode HiFi > >> * bi-directional is enabled by default. > >> */ > >> if (!tb_switch_is_clx_supported(sw)) > >> return 0; > >> > >> > >> However, it seems that this code has been removed in recent changes, as > >> the CLX-related code has been moved to a different file. > > > > Yes, I removed it because TMU code should not really be calling CLx > > functions. > > > > However, we have in tb_enable_tmu() this: > > > > /* If it is already enabled in correct mode, don't touch it */ > > if (tb_switch_tmu_is_enabled(sw)) > > return 0; > > > > and tb_switch_tmu_init() reads the hardware state so this code should > > basically leave TMU enabling untouched on Alpine Ridge for instance. I > > wonder if you can try with the latest "next" branch and see if it works > > there or you are already doing so? > > > Yes Mika, we have already verified with the latest thunderbolt/next > branch. This patch is built on top of next branch. Okay. > >> Canonical has also reported this issue and has tested this patch that > >> appears to resolve the issue.. > > > > Right, however let's figure out if the problem is already solved with > > the recent code as above or if not why it does not work as expected. I > > don't really think we want to add any quirks for this because even in > > the USB4 spec the TMU of TBT3 devices is expected to be enabled already > > so this is expected functionality and the driver should be doing the > > right thing here. > > Agree. we will have to see what is going wrong in this case. You should be able to see in the dmesg (once thunderbolt.dyndbg=+p is passed in command line) what the initial state of TMU is and how the driver programs it or does it skip the enabling as expected.
