linux icon indicating copy to clipboard operation
linux copied to clipboard

soundwire: bus: add CLOCK_STOP_MODE1 support back

Open bardliao opened this issue 1 month ago • 7 comments

CLOCK_STOP_MODE1 is used when the Peripheral might have entered a deeper power-saving mode that does not retain state while the Clock is stopped. It is useful when the device is more power consumption sensitive. Add it back to allow the Peripheral use CLOCK_STOP_MODE1.

bardliao avatar Dec 12 '25 12:12 bardliao

@bardliao how would you test this? In practice the intel host does a bus reset after restarting the clock which forces the device to lose context anyways. IOW it doesn't matter if the device supports clock stop0 or 1, context always needs to be restored.

plbossart avatar Dec 23 '25 10:12 plbossart

@plbossart Indeed, we will do bus reset after restarting the clock. Using mode 1 is just for power consumption during clock stop. On the other hands, the peripheral will need to restore the context anyway, why don't we use mode 1 If the peripheral supports it?

bardliao avatar Dec 24 '25 05:12 bardliao

I don't disagree @bardliao, if the host does a bus reset we would benefit from the MODE1 on the device side. There are still several opens a) how was MODE0 tested, if we do a bus reset then by construction we cannot test the instant restart b) if a device supports MODE1, do we always select MODE1? it's a trade-off between power and latency, not sure the property definition is that clear. c) if the host does a bus reset, should it also override all the properties to set the MODE1?

plbossart avatar Dec 27 '25 16:12 plbossart

I don't disagree @bardliao, if the host does a bus reset we would benefit from the MODE1 on the device side. There are still several opens a) how was MODE0 tested, if we do a bus reset then by construction we cannot test the instant restart

The existing code is using MODE0. I would expect that the behavior will not change on MODE0.

b) if a device supports MODE1, do we always select MODE1? it's a trade-off between power and latency, not sure the property definition is that clear.

No, the codec driver can implement the get_clk_stop_mode ops to set the condition of using MODE1

c) if the host does a bus reset, should it also override all the properties to set the MODE1?

sdw_get_clk_stop_mode() will set the clock mode before clock stops. I guess it should be enough?

bardliao avatar Dec 29 '25 06:12 bardliao

@vijendarmukunda Do you mind testing this PR on AMD platforms?

bardliao avatar Dec 29 '25 06:12 bardliao

@vijendarmukunda Do you mind testing this PR on AMD platforms?

@bardliao : Most of my team members are OOO. Is it okay if we can update the valiadation results some time early next week?

vijendarmukunda avatar Dec 29 '25 09:12 vijendarmukunda

@vijendarmukunda Do you mind testing this PR on AMD platforms?

@bardliao : Most of my team members are OOO. Is it okay if we can update the valiadation results some time early next week?

Yeah, same as us. It is totally fine to get the test result next week.

bardliao avatar Dec 29 '25 09:12 bardliao

@bardliao : Validated ClockStop Mode1 on AMD platform using RTK codec. It's working fine.

vijendarmukunda avatar Jan 06 '26 11:01 vijendarmukunda