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

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

 




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.



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

  Powered by Linux