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. >> 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.