On 7/3/23 17:18, Mukunda,Vijendar wrote: > On 03/07/23 20:15, Pierre-Louis Bossart wrote: >> >> On 7/3/23 16:46, Mukunda,Vijendar wrote: >>> On 03/07/23 19:50, Pierre-Louis Bossart wrote: >>>> On 7/3/23 15:31, Mukunda,Vijendar wrote: >>>>> On 03/07/23 18:30, Shuming [范書銘] wrote: >>>>>>>>>> During ClockStop Mode0, peripheral interrupts are disabled. >>>>>>>>> I can see that the interrupts are disabled in >>>>>>>>> rt5682_dev_system_suspend(), which is NOT a mode where the clock stop >>>>>>>>> is used... I don't think this commit message is correct. >>>>>>>>> >>>>>>>>> The IMPL_DEF interrupt which is used for jack detection is not >>>>>>>>> disabled at all during any clock stop mode, and it shouldn't >>>>>>>>> otherwise that would break the jack detection. >>>>>>>> You are right. The commit message is wrong and not clear. >>>>>>>> The situation is that the manager driver uses the clock stop mode0 to do >>>>>>> system suspension. >>>>>>> >>>>>>> No it does not. The clock stop is ONLY used for pm_runtime, never for system >>>>>>> suspend. We cannot go to system suspend with the link in clock-stop mode, >>>>>>> that will create lots of issues, that's why we perform a full pm_runtime resume >>>>>>> in the .prepare stage. >>>>>> OK, I got your point. Thanks. However, this issue reported by AMD. >>>>>> The AMD platform validated system level pm and runtime pm ops with the different modes. >>>>>> >>>>>> Hi Vijendar, >>>>>> Do you have any comments? >>>>> On AMD platforms, we are supporting two power modes. >>>>> 1) Bus reset mode >>>>> 2) Clock Stop Mode >>>>> >>>>> In Bus reset mode, bus will re-enumerate the peripheral devices >>>>> whereas in ClockStop Mode, applying ClockStop Mode0 >>>>> in both pm ops (runtime pm ops and system level pm ops). >>>>> >>>>> Currently, SDCA interrupts are disabled on peripheral side, when system level >>>>> suspend is invoked. >>>>> For ClockStop mode SDW manager is not receiving any jack alert when >>>>> SoundWire manager device is in D3 state. >>>> That was precisely the point of clock stop mode: a peripheral can >>>> restart the system even when it's in lower-power mode. >>>> >>>> If there's no means to let a peripheral restart, the only benefit is >>>> maybe to skip the enumeration time. That's not what the spec intended.... >>> As per our understanding, you are pointing to ClockStopMode1. >>> ClockStopMode1 requires re-enumeration as peripherals move >>> to unattached state. >>> We have cross-checked ClockStopMode0 description in spec. >>> It doesn't specify about peripheral device state as Unattached. >>> We are referring here "ClockStopMode0" only. >> No I was describing the difference between the 'Bus reset mode' and the >> 'clock stop mode' on the manager side. >> >> There's also nothing in the spec preventing the manager from doing a >> reset at any time, including after exiting the clock mode0 stop. >> >> > Partly I agree. As per our understanding, If any of the peripherals lost's sync, > and re-enumeration is required. > If continuous parity errors/bus clash conditions are reported over the link, > Sdw Manager bus reset sequence should be invoked. This is a different scenario. > Both the scenarios are asynchronous. > > Going with Spec definition for ClockStopMode0, as it's Imp defined for > SoundWire Manager, want to stick to Clockstop when D3 call is invoked > and restore the clock when D0 call is invoked for our platforms. The problem is that 'D3' can be used for two separate scenarios - S0/D3: that's pm_runtime suspend - Sx/D4: that's system suspend It's very unclear what the benefit of clock stop mode would be for the latter case.
