Hi Takashi, >> Hi Takashi, >> >> >> > >> >> >On Tue, 26 Mar 2019 06:42:15 +0100, Yang, Libin wrote: >> >> >(snip) >> >> >> Hi Takashi, >> >> >> Below is my draft patch, which doesn't work with >> >> >> pm_runtime_get_sync(). Is there anything wrong in my code? >> >> >(snip) >> >> >> And the dmesg is: >> >> >> [ 36.087224] HDMI HDA Codec ehdaudio0D2: in hdmi_codec_resume >> >1890 >> >> >ylb >> >> >> [ 36.087230] HDMI HDA Codec ehdaudio0D2: in >> >> >hdac_hdmi_runtime_resume 2122 ylb 1 >> >> >> [ 36.087335] HDMI HDA Codec ehdaudio0D2: in >> >> >hdac_hdmi_cvt_output_widget_event 812 ylb >> >> >> [ 40.097586] HDMI HDA Codec ehdaudio0D2: in >> >> >hdac_hdmi_runtime_resume 2135 ylb 2 >> >> >> [ 40.097766] HDMI HDA Codec ehdaudio0D2: in >> >> >hdac_hdmi_pin_output_widget_event 767 ylb >> >> >> [ 45.108632] HDMI HDA Codec ehdaudio0D2: in >> >> >hdac_hdmi_pin_mux_widget_event 857 ylb >> >> >> >> >> >> >From the dmesg, hdac_hdmi_cvt_output_widget_event() >> >> >> is called between "ylb 1" and "ylb 2" in runtime_resume(). >> >> >> This means xxx_event() registers setting runs before display >> >> >> power is turned on. >> >> > >> >> >OK, now I understood what went wrong. It's a similar issue as >> >> >we've hit for the legacy HD-audio and figured out recently. >> >> > >> >> >If my understanding is correct, the problem is the call of >> >> >pm_runtime_force_resume() in PM resume callback combined with an >> >> >async work. pm_runtime_force_resume() sets the runtime state >> >> >immediately to RPM_ACTIVE even before finishing the task. The >> >> >code seems assuming blindly that there is no other conflicting >> >> >task while >> >> >S3/S4 resume, but this is a problem. That's why the next >> >> >pm_runtime_get_sync() wasn't blocked but just went through; since >> >> >the RPM flag was already set to RPM_ACTIVE in >> >> >pm_runtime_force_resume(), it thought as if it were already >> >> >active, while the real runtime resume code >> >was still processing the callback. >> >> >> >> Yes, exactly right. And I have checked dev->power.runtime_status, >> >> it's RPM_ACTIVE when xxx_event() calls pm_runtime_get_sync(). >> >> >> >> > >> >> >In the case of legacy HD-audio, we "fixed" the problem by avoiding >> >> >the trigger of async work at resume, but let it be triggered from >> >> >runtime resume. In ASoC case, however, it's no option. >> >> > >> >> >Maybe a possible solution in the sound driver side would be to >> >> >move from system suspend/resume to ASoC component >suspend/resume. >> >> >The runtime suspend/resume can be kept as is, and call >> >> >pm_runtime_force_suspend and resume from the component >> >suspend/resume >> >> >callbacks. Since the component callbacks are certainly processed >> >> >before DAPM events, this should assure the order. >> >> >> >> I have worked out another patch. This patch decouples the >> >> xxx_event() and runtime suspend/resume. This means in whichever >> >> case, xxx_event() can make sure display power is in the correct status. >What do you think? >> >> On the same time, I will try the component suspend/resume. My >> >> understanding is that snd_hdac_display_power() should be called >> >> either in runtime_suspend/ resume or in component suspend/resume. >> > >> >This might work around the particular case, yes. However it still >> >makes me uneasy as the root cause isn't full covered -- the other >> >part of runtime resume might be still pending while executing the DAPM >event. >> > >> >Now, considering the idea with device_link_add() again: I guess it's >> >snd_soc_resume() who kicks off the DAPM even work? If so, and if >> >snd_soc_resume() gets called from the machine driver, we can assure >> >the PM order via device_link_add() so that the codec driver is >> >resumed before the machine driver. Then at the time the deferred >> >resume work is executed, the codec is ready, so the display power is on. >> >> Yes, snd_soc_resume() kicks off the DAPM event work. The device PM >> sequence is good. The root cause is when hdmi runtime_resume calls >> snd_hdac_display_power(), it may sleep. The flow is: >> >> 1. HDMI runtime_resume runs >> 2. HDMI runtime_resume calls snd_hdac_display_power(), but display >> driver is also operating display power and the mutex_lock is hold. So >> HDMI runtime_resume sleeps. >> 3. the deferred work is scheduled and xxx_event() is called. >> >> This is wrong. All these happen because of the mutex_lock is hold by >> display driver, which causes HDMI runtime_resume sleep. > >Well, the mutex lock itself is OK, and it's designed to behave so. > >As mentioned, the root cause is pm_runtime_force_resume() that sets >RPM_ACTIVE while a concurrent task is running. > >OTOH, this can be seen indeed as a PM dependency between devices, and if >we set the order explicitly, the problem can be avoided, too. Then the >runtime resume of codec finishes before snd_soc_resume() is called from the >machine driver resume. Do you have any idea to set the order explicitly? What I can think of is that we set HDMI device to be the machine device parent? Regards, Libin > >Also, moving to component PM would solve the problem, too, since it also >assures finishing the codec resume before the schedule_work() call in >snd_soc_resume(). > > >thanks, > >Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
