Re: [PATCH Internal] thunderbolt: Remove enabling/disabling TMU based on CLx

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?

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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux