mbox series

[v3,0/4] ALSA: hda: Abstract and update HOST-stream setup procedure

Message ID 20230926080623.43927-1-cezary.rojewski@intel.com (mailing list archive)
Headers show
Series ALSA: hda: Abstract and update HOST-stream setup procedure | expand

Message

Cezary Rojewski Sept. 26, 2023, 8:06 a.m. UTC
The patchset targets two intertwined topics:

The driver shall poll SDxFIFOS to ensure a valid value is set by the
controller after programming SDxFMT. Due to amount of users and
limited-number of configuration available in our CI when compared to
overall possibilities on the market, the check is non-blocking.

Second topic relates to stream setup procedure. The procedure differs
between HDAudio controller device revisions. Right now those differences
are handled directly by a platform driver. Existing top-level
'if (pci->device == APL)' could be replaced by a abstraction in lower
parts of the code instead.

With that done, the two users are updated accordingly. In avs-driver
case, this updates the flow to the recommended one.

Changes in v3:
- fixed issues pointed out by scripts/kernel-doc

Changes in v2:
- fixed ->host_setup assignment in patch 02/04

Cezary Rojewski (4):
  ALSA: hda: Poll SDxFIFOS after programming SDxFMT
  ALSA: hda: Introduce HOST stream setup mechanism
  ASoC: Intel: avs: Use helper to setup HOST stream
  ASoC: Intel: Skylake: Use helper to setup HOST stream

 include/sound/hda_register.h      |  2 ++
 include/sound/hdaudio.h           |  3 +++
 include/sound/hdaudio_ext.h       |  3 +++
 sound/hda/ext/hdac_ext_stream.c   | 41 +++++++++++++++++++++++++++++++
 sound/hda/hdac_stream.c           |  8 ++++++
 sound/soc/intel/avs/pcm.c         |  2 +-
 sound/soc/intel/skylake/skl-pcm.c | 14 +----------
 7 files changed, 59 insertions(+), 14 deletions(-)

Comments

Takashi Iwai Oct. 6, 2023, 8:44 a.m. UTC | #1
On Tue, 26 Sep 2023 10:06:19 +0200,
Cezary Rojewski wrote:
> 
> The patchset targets two intertwined topics:
> 
> The driver shall poll SDxFIFOS to ensure a valid value is set by the
> controller after programming SDxFMT. Due to amount of users and
> limited-number of configuration available in our CI when compared to
> overall possibilities on the market, the check is non-blocking.
> 
> Second topic relates to stream setup procedure. The procedure differs
> between HDAudio controller device revisions. Right now those differences
> are handled directly by a platform driver. Existing top-level
> 'if (pci->device == APL)' could be replaced by a abstraction in lower
> parts of the code instead.
> 
> With that done, the two users are updated accordingly. In avs-driver
> case, this updates the flow to the recommended one.
> 
> Changes in v3:
> - fixed issues pointed out by scripts/kernel-doc
> 
> Changes in v2:
> - fixed ->host_setup assignment in patch 02/04
> 
> Cezary Rojewski (4):
>   ALSA: hda: Poll SDxFIFOS after programming SDxFMT
>   ALSA: hda: Introduce HOST stream setup mechanism
>   ASoC: Intel: avs: Use helper to setup HOST stream
>   ASoC: Intel: Skylake: Use helper to setup HOST stream

Sorry for the late reaction, as I've been (still) off since the last
week.

Now applied now to for-next branch.


thanks,

Takashi
Cezary Rojewski Oct. 6, 2023, 9:01 a.m. UTC | #2
On 2023-10-06 10:44 AM, Takashi Iwai wrote:
> On Tue, 26 Sep 2023 10:06:19 +0200,
> Cezary Rojewski wrote:
>>
>> The patchset targets two intertwined topics:
>>
>> The driver shall poll SDxFIFOS to ensure a valid value is set by the
>> controller after programming SDxFMT. Due to amount of users and
>> limited-number of configuration available in our CI when compared to
>> overall possibilities on the market, the check is non-blocking.
>>
>> Second topic relates to stream setup procedure. The procedure differs
>> between HDAudio controller device revisions. Right now those differences
>> are handled directly by a platform driver. Existing top-level
>> 'if (pci->device == APL)' could be replaced by a abstraction in lower
>> parts of the code instead.
>>
>> With that done, the two users are updated accordingly. In avs-driver
>> case, this updates the flow to the recommended one.
>>
>> Changes in v3:
>> - fixed issues pointed out by scripts/kernel-doc
>>
>> Changes in v2:
>> - fixed ->host_setup assignment in patch 02/04
>>
>> Cezary Rojewski (4):
>>    ALSA: hda: Poll SDxFIFOS after programming SDxFMT
>>    ALSA: hda: Introduce HOST stream setup mechanism
>>    ASoC: Intel: avs: Use helper to setup HOST stream
>>    ASoC: Intel: Skylake: Use helper to setup HOST stream
> 
> Sorry for the late reaction, as I've been (still) off since the last
> week.
> 
> Now applied now to for-next branch.

Hello Takashi,

Now I'm the one sorry for the late replay - I've found two new things 
when fixing the debug message (reported by me in patch 1/4).
- null-ptr-deref when assigning hdac_stream (when type=COUPLED)
- azx_dev->fifo_size is initialized incorrectly

It's good to debug things, you never know what you may find!

May I send v4 as a collective update? It would address the two above and 
the false-positive debug message that appears when code-loading AudioDSP 
firmware.


Kind regards,
Czarek
Takashi Iwai Oct. 6, 2023, 9:14 a.m. UTC | #3
On Fri, 06 Oct 2023 11:01:44 +0200,
Cezary Rojewski wrote:
> 
> On 2023-10-06 10:44 AM, Takashi Iwai wrote:
> > On Tue, 26 Sep 2023 10:06:19 +0200,
> > Cezary Rojewski wrote:
> >> 
> >> The patchset targets two intertwined topics:
> >> 
> >> The driver shall poll SDxFIFOS to ensure a valid value is set by the
> >> controller after programming SDxFMT. Due to amount of users and
> >> limited-number of configuration available in our CI when compared to
> >> overall possibilities on the market, the check is non-blocking.
> >> 
> >> Second topic relates to stream setup procedure. The procedure differs
> >> between HDAudio controller device revisions. Right now those differences
> >> are handled directly by a platform driver. Existing top-level
> >> 'if (pci->device == APL)' could be replaced by a abstraction in lower
> >> parts of the code instead.
> >> 
> >> With that done, the two users are updated accordingly. In avs-driver
> >> case, this updates the flow to the recommended one.
> >> 
> >> Changes in v3:
> >> - fixed issues pointed out by scripts/kernel-doc
> >> 
> >> Changes in v2:
> >> - fixed ->host_setup assignment in patch 02/04
> >> 
> >> Cezary Rojewski (4):
> >>    ALSA: hda: Poll SDxFIFOS after programming SDxFMT
> >>    ALSA: hda: Introduce HOST stream setup mechanism
> >>    ASoC: Intel: avs: Use helper to setup HOST stream
> >>    ASoC: Intel: Skylake: Use helper to setup HOST stream
> > 
> > Sorry for the late reaction, as I've been (still) off since the last
> > week.
> > 
> > Now applied now to for-next branch.
> 
> Hello Takashi,
> 
> Now I'm the one sorry for the late replay - I've found two new things
> when fixing the debug message (reported by me in patch 1/4).
> - null-ptr-deref when assigning hdac_stream (when type=COUPLED)
> - azx_dev->fifo_size is initialized incorrectly
> 
> It's good to debug things, you never know what you may find!
> 
> May I send v4 as a collective update? It would address the two above
> and the false-positive debug message that appears when code-loading
> AudioDSP firmware.

As other patches have been already applied, could you submit the
incremental fixes on top of v3 instead?  You can see the latest state
in for-next branch of sound.git tree.


thanks,

Takashi