Re: [PATCH] ASoC: SDCA: Add NO_DIRECT_COMPLETE flag to class driver

[Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>]

>>> The SDCA class driver currently expects the device will be fully powered
>>> down on system suspend but not on runtime suspend. This is typically
>>> required as when audio is not active (ie. runtime suspend) jack detect
>>> is expected to still function, but when the whole system is hibernated
>>> there is no need to recognise audio jack events. This means the class
>>> driver needs to always be informed of a system suspend, so the direct
>>> complete optimisation (where PM will skip calling system suspend if the
>>> device is runtime suspended) is not appropriate for the SDCA class
>>> driver.
>>>
>>> Add the NO_DIRECT_COMPLETE flag to prevent this optimisation from
>>> running against this driver.
>>
>> Humm, this flag makes sure the class driver .suspend method is
>> called, but does the core also force a pm_runtime resume before?
> 
> Yeah this ensures the suspend method is always called.
> 
>> In other words, what is the state of the device when the
>> system suspend routine is invoked by the PM core when this
>> flag is set?
> 
> Can be either runtime resumed or suspended.

ok, so if the child driver wants to resume to full power it has to be done 'manually', the PM core does not help.

>> Going back to this SDCA driver, if the PM core does NOT
>> change the pm_runtime state, then should the class driver
>> perform a full pm_runtime resume and then do the expected
>> system suspend sequence?
> 
> Yeah we do perform a resume in the system suspend although to be
> honest its mostly just to force the pm_runtime_force_resume() in
> the system resume handler to always do a runtime resume.

I think we are talking about different actions...

a) pm_runtime resume BEFORE system suspend. That would be necessary to deal with e.g. clock stop modes that are valid in pm_runtime suspend but not system suspend. It's not clear to me for example if all SDCA chips will support direct transitions from PS3 to PS4, or be ok with power rails cut while in PS3.

b) pm_runtime resume DURING system resume. That may be useful to restore functionality, but it's too late to handle problems that occur during the system suspend phase.

Either way, that's a bit orthogonal to this patch, more a discussion on how to deal with class power management.

>> Sorry if I am splitting hair, in the past we played with
>> these PM flags and always understood them in the wrong way...
> 
> There is definitely a lot of subtlety in this part of the kernel,
> hence why I CCed the PM people on this patch :-) Its a bit of an
> odd optimisation this direct complete thing, as far as I can see
> lots of drives don't run it without really deliberately avoiding
> it. For SoundWire itself the soundwire host resumes all children
> before a suspend, which dodges the direct complete. I would
> imagine a lot of other peripheral buses do similar. For MFD
> children they are almost always on the same fwnode as their
> parent so they don't hit it either. Turns out this class driver
> is the first thing I have worked on that actually hits the direct
> complete optimimisation as far as I can see.

You lost me with this sentence:

"
For SoundWire itself the soundwire host resumes all children
before a suspend, which dodges the direct complete.
"

If the direct-complete optimization is not used, then do you actually see a different behavior with this NO_DIRECT_COMPLETE flag set?