| Message ID | 20221027082331.1561740-4-cezary.rojewski@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ASoC: Intel: avs: PCM power management | expand |
On Thu, 27 Oct 2022 10:23:25 +0200, Cezary Rojewski wrote: > --- a/sound/hda/hdac_stream.c > +++ b/sound/hda/hdac_stream.c > @@ -821,6 +821,27 @@ void snd_hdac_stream_drsm_enable(struct hdac_bus *bus, > } > EXPORT_SYMBOL_GPL(snd_hdac_stream_drsm_enable); > > +/* > + * snd_hdac_stream_wait_drsm - wait for HW to clear RSM for a stream > + * @azx_dev: HD-audio core stream to await RSM for > + * > + * Returns 0 on success and -ETIMEDOUT upon a timeout. > + */ > +int snd_hdac_stream_wait_drsm(struct hdac_stream *azx_dev) > +{ > + struct hdac_bus *bus = azx_dev->bus; > + u32 mask, reg; > + int ret; > + > + mask = 1 << azx_dev->index; > + > + ret = readb_poll_timeout(bus->drsmcap + AZX_REG_DRSM_CTL, reg, !(reg & mask), 250, 2000); Remember that HD-audio bus doesn't always allow readb(). Tegra requires the aligned access, for example. Takashi
On 10/27/2022 12:21 PM, Takashi Iwai wrote: > On Thu, 27 Oct 2022 10:23:25 +0200, > Cezary Rojewski wrote: >> --- a/sound/hda/hdac_stream.c >> +++ b/sound/hda/hdac_stream.c >> @@ -821,6 +821,27 @@ void snd_hdac_stream_drsm_enable(struct hdac_bus *bus, >> } >> EXPORT_SYMBOL_GPL(snd_hdac_stream_drsm_enable); >> >> +/* >> + * snd_hdac_stream_wait_drsm - wait for HW to clear RSM for a stream >> + * @azx_dev: HD-audio core stream to await RSM for >> + * >> + * Returns 0 on success and -ETIMEDOUT upon a timeout. >> + */ >> +int snd_hdac_stream_wait_drsm(struct hdac_stream *azx_dev) >> +{ >> + struct hdac_bus *bus = azx_dev->bus; >> + u32 mask, reg; >> + int ret; >> + >> + mask = 1 << azx_dev->index; >> + >> + ret = readb_poll_timeout(bus->drsmcap + AZX_REG_DRSM_CTL, reg, !(reg & mask), 250, 2000); > > Remember that HD-audio bus doesn't always allow readb(). Tegra > requires the aligned access, for example. > The readb_poll_timeout macro was updated to take care of that, https://lore.kernel.org/all/20221007084856.1638302-1-amadeuszx.slawinski@linux.intel.com/ so it should be fine?
On Thu, 27 Oct 2022 12:29:35 +0200, Amadeusz Sławiński wrote: > > On 10/27/2022 12:21 PM, Takashi Iwai wrote: > > On Thu, 27 Oct 2022 10:23:25 +0200, > > Cezary Rojewski wrote: > >> --- a/sound/hda/hdac_stream.c > >> +++ b/sound/hda/hdac_stream.c > >> @@ -821,6 +821,27 @@ void snd_hdac_stream_drsm_enable(struct hdac_bus *bus, > >> } > >> EXPORT_SYMBOL_GPL(snd_hdac_stream_drsm_enable); > >> +/* > >> + * snd_hdac_stream_wait_drsm - wait for HW to clear RSM for a stream > >> + * @azx_dev: HD-audio core stream to await RSM for > >> + * > >> + * Returns 0 on success and -ETIMEDOUT upon a timeout. > >> + */ > >> +int snd_hdac_stream_wait_drsm(struct hdac_stream *azx_dev) > >> +{ > >> + struct hdac_bus *bus = azx_dev->bus; > >> + u32 mask, reg; > >> + int ret; > >> + > >> + mask = 1 << azx_dev->index; > >> + > >> + ret = readb_poll_timeout(bus->drsmcap + AZX_REG_DRSM_CTL, reg, !(reg & mask), 250, 2000); > > > > Remember that HD-audio bus doesn't always allow readb(). Tegra > > requires the aligned access, for example. > > > > The readb_poll_timeout macro was updated to take care of that, > https://lore.kernel.org/all/20221007084856.1638302-1-amadeuszx.slawinski@linux.intel.com/ > so it should be fine? This patch doesn't use that macro... Takashi
On 10/27/2022 1:05 PM, Takashi Iwai wrote: > On Thu, 27 Oct 2022 12:29:35 +0200, > Amadeusz Sławiński wrote: >> >> On 10/27/2022 12:21 PM, Takashi Iwai wrote: >>> On Thu, 27 Oct 2022 10:23:25 +0200, >>> Cezary Rojewski wrote: >>>> --- a/sound/hda/hdac_stream.c >>>> +++ b/sound/hda/hdac_stream.c >>>> @@ -821,6 +821,27 @@ void snd_hdac_stream_drsm_enable(struct hdac_bus *bus, >>>> } >>>> EXPORT_SYMBOL_GPL(snd_hdac_stream_drsm_enable); >>>> +/* >>>> + * snd_hdac_stream_wait_drsm - wait for HW to clear RSM for a stream >>>> + * @azx_dev: HD-audio core stream to await RSM for >>>> + * >>>> + * Returns 0 on success and -ETIMEDOUT upon a timeout. >>>> + */ >>>> +int snd_hdac_stream_wait_drsm(struct hdac_stream *azx_dev) >>>> +{ >>>> + struct hdac_bus *bus = azx_dev->bus; >>>> + u32 mask, reg; >>>> + int ret; >>>> + >>>> + mask = 1 << azx_dev->index; >>>> + >>>> + ret = readb_poll_timeout(bus->drsmcap + AZX_REG_DRSM_CTL, reg, !(reg & mask), 250, 2000); >>> >>> Remember that HD-audio bus doesn't always allow readb(). Tegra >>> requires the aligned access, for example. >>> >> >> The readb_poll_timeout macro was updated to take care of that, >> https://lore.kernel.org/all/20221007084856.1638302-1-amadeuszx.slawinski@linux.intel.com/ >> so it should be fine? > > This patch doesn't use that macro... > Ah... right, sorry for confusion, I looked at diff part instead of macro name.
On 2022-10-27 1:05 PM, Takashi Iwai wrote: > On Thu, 27 Oct 2022 12:29:35 +0200, > Amadeusz Sławiński wrote: >> >> On 10/27/2022 12:21 PM, Takashi Iwai wrote: >>> On Thu, 27 Oct 2022 10:23:25 +0200, >>> Cezary Rojewski wrote: >>>> --- a/sound/hda/hdac_stream.c >>>> +++ b/sound/hda/hdac_stream.c >>>> @@ -821,6 +821,27 @@ void snd_hdac_stream_drsm_enable(struct hdac_bus *bus, >>>> } >>>> EXPORT_SYMBOL_GPL(snd_hdac_stream_drsm_enable); >>>> +/* >>>> + * snd_hdac_stream_wait_drsm - wait for HW to clear RSM for a stream >>>> + * @azx_dev: HD-audio core stream to await RSM for >>>> + * >>>> + * Returns 0 on success and -ETIMEDOUT upon a timeout. >>>> + */ >>>> +int snd_hdac_stream_wait_drsm(struct hdac_stream *azx_dev) >>>> +{ >>>> + struct hdac_bus *bus = azx_dev->bus; >>>> + u32 mask, reg; >>>> + int ret; >>>> + >>>> + mask = 1 << azx_dev->index; >>>> + >>>> + ret = readb_poll_timeout(bus->drsmcap + AZX_REG_DRSM_CTL, reg, !(reg & mask), 250, 2000); >>> >>> Remember that HD-audio bus doesn't always allow readb(). Tegra >>> requires the aligned access, for example. >>> >> >> The readb_poll_timeout macro was updated to take care of that, >> https://lore.kernel.org/all/20221007084856.1638302-1-amadeuszx.slawinski@linux.intel.com/ >> so it should be fine? > > This patch doesn't use that macro... Thanks for spotting this out! There's even more to that. The DRSMCTL register is u32 reg, not u8. Since we are using lower amount of streams in these tests, it works just fine but with a higher number, the scenario can fail. 'Just copy-paste things'. Will update the function to make use of u32 equivalent. Regards, Czarek
diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h index 78f1809a4ad6..a6872537724d 100644 --- a/include/sound/hdaudio.h +++ b/include/sound/hdaudio.h @@ -597,6 +597,7 @@ int snd_hdac_stream_get_spbmaxfifo(struct hdac_bus *bus, struct hdac_stream *azx_dev); void snd_hdac_stream_drsm_enable(struct hdac_bus *bus, bool enable, int index); +int snd_hdac_stream_wait_drsm(struct hdac_stream *azx_dev); int snd_hdac_stream_set_dpibr(struct hdac_bus *bus, struct hdac_stream *azx_dev, u32 value); int snd_hdac_stream_set_lpib(struct hdac_stream *azx_dev, u32 value); diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c index 35fe2bd582ac..59e8bfe4feca 100644 --- a/sound/hda/hdac_stream.c +++ b/sound/hda/hdac_stream.c @@ -821,6 +821,27 @@ void snd_hdac_stream_drsm_enable(struct hdac_bus *bus, } EXPORT_SYMBOL_GPL(snd_hdac_stream_drsm_enable); +/* + * snd_hdac_stream_wait_drsm - wait for HW to clear RSM for a stream + * @azx_dev: HD-audio core stream to await RSM for + * + * Returns 0 on success and -ETIMEDOUT upon a timeout. + */ +int snd_hdac_stream_wait_drsm(struct hdac_stream *azx_dev) +{ + struct hdac_bus *bus = azx_dev->bus; + u32 mask, reg; + int ret; + + mask = 1 << azx_dev->index; + + ret = readb_poll_timeout(bus->drsmcap + AZX_REG_DRSM_CTL, reg, !(reg & mask), 250, 2000); + if (ret) + dev_dbg(bus->dev, "polling RSM 0x%08x failed: %d\n", mask, ret); + return ret; +} +EXPORT_SYMBOL_GPL(snd_hdac_stream_wait_drsm); + /** * snd_hdac_stream_set_dpibr - sets the dpibr value of a stream * @bus: HD-audio core bus
Allow for waiting for DRSM bit for specified stream to be cleared from HDAudio library level. Drivers may utilize this optional step during the stream resume procedure. Suggested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com> --- include/sound/hdaudio.h | 1 + sound/hda/hdac_stream.c | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+)