mbox series

[RFC,0/7] adapt SOF to use snd-hda-codec-hdmi

Message ID 20190829135348.23569-1-kai.vehmanen@linux.intel.com (mailing list archive)
Headers show
Series adapt SOF to use snd-hda-codec-hdmi | expand

Message

Kai Vehmanen Aug. 29, 2019, 1:53 p.m. UTC
Hi all,
here's a RFC patch series that adapts SOF (and one example machine
driver) to use snd-hda-codec-hdmi (patch_hdmi.c) codec driver
instead of hdac_hdmi (soc/codecs/hdac_hdmi.c). The primary goal
is unify the HDMI codec implementation between DSP and non-DSP HDA
configurations, offer same interface to user-space and reduce
maintenance load for all.

Main points I'd like your input on:

1) Is the high-level approach ok?
 - SOF already uses pci/hda/ for all others codecs, so HDMI
   has been the sole exception where code has been duplicated.
 - I've tried to keep changes to hda/hdmi minimal.
 - This series implements logic to parse the PCM topology
   in a dynamic fashion, so we do not have to change all
   existing (and future) DSP topologis to use fixed PCM device
   numbers for HDMI, and we avoid need to hardcode PCM
   device numbers in machine driver code.

2) Can we drop hdac_hdmi and its support from machine drivers, or
   do we need to make it optional and keep it around?

 - Current series does not add any Kconfig options, but
   simply switches SOF to use HDA codecs for all, including
   HDMI/DP. This means hdac_hdmi is never used with SOF
   and could be dropped (if SST is ok as well).
 - This may break some usage with SST (input is welcome!)
 - The change is visible to applications. The ALSA mixer
   interface is different (OTOH with the new driver, playback
   works out-of-the-box while with hdac_hdmi you needed to
   set the multiple controls first, to have any audio out).
 - Alternatively I could add a KConfig option and we could
   have a deprecation period for hdac_hdmi, allowing people
   to compile with the old driver during transition time.
   This will require #ifdef'ing in all the machine drivers.

.. once these are addressed, I can proceed to extend the patchset
with all affected machine drivers.

Feature and testing info:

 - Tested on multiple Intel platforms supported by SOF.
 - Tested with ALSA console tools as well as with Pulseaudio.
      - requires Pulseaudio 12.x or newer, see
        https://lists.freedesktop.org/archives/pulseaudio-discuss/2019-August/031358.html
 - HDMI, DP, DP-MST with multi-monitor use-scenarios work ok.
 - New feature for SOF: ELD /proc fs works just like in
   DSP-less mode.
 - New feature for SOF: jack detection works out-of-the-box
   with Pulseaudio (no need for card specific UCM for HDMI)
 - Pre-reviewed at:
   https://github.com/thesofproject/linux/pull/1155

Kai Vehmanen (7):
  ALSA: hda - add mst_no_extra_pcms flag to hda_codec
  ASoC: Intel: skl-hda-dsp-generic: use snd-hda-codec-hdmi
  ASoC: hdac_hda: add support for HDMI/DP as a HDA codec
  ALSA: hda/hdmi - allow control creation without a linked pcm
  ALSA: hda/hdmi - implement mst_no_extra_pcms flag
  ALSA: hda/hdmi - complete pcm_setup_pin without snd_pcm link
  ASoC: SOF: Intel: load hda codec module also for HDMI/DP

 include/sound/hda_codec.h                    |  1 +
 sound/pci/hda/patch_hdmi.c                   | 31 ++++---
 sound/soc/codecs/hdac_hda.c                  | 95 +++++++++++++++++---
 sound/soc/codecs/hdac_hda.h                  | 10 ++-
 sound/soc/intel/boards/skl_hda_dsp_common.c  | 27 ++----
 sound/soc/intel/boards/skl_hda_dsp_common.h  | 61 +++++++++++++
 sound/soc/intel/boards/skl_hda_dsp_generic.c |  7 --
 sound/soc/sof/intel/hda-codec.c              | 11 +--
 sound/soc/sof/intel/hda.h                    |  5 +-
 9 files changed, 189 insertions(+), 59 deletions(-)

Comments

Takashi Iwai Aug. 29, 2019, 2:16 p.m. UTC | #1
On Thu, 29 Aug 2019 15:53:41 +0200,
Kai Vehmanen wrote:
> 
> Hi all,
> here's a RFC patch series that adapts SOF (and one example machine
> driver) to use snd-hda-codec-hdmi (patch_hdmi.c) codec driver
> instead of hdac_hdmi (soc/codecs/hdac_hdmi.c). The primary goal
> is unify the HDMI codec implementation between DSP and non-DSP HDA
> configurations, offer same interface to user-space and reduce
> maintenance load for all.
> 
> Main points I'd like your input on:
> 
> 1) Is the high-level approach ok?
>  - SOF already uses pci/hda/ for all others codecs, so HDMI
>    has been the sole exception where code has been duplicated.
>  - I've tried to keep changes to hda/hdmi minimal.
>  - This series implements logic to parse the PCM topology
>    in a dynamic fashion, so we do not have to change all
>    existing (and future) DSP topologis to use fixed PCM device
>    numbers for HDMI, and we avoid need to hardcode PCM
>    device numbers in machine driver code.
> 
> 2) Can we drop hdac_hdmi and its support from machine drivers, or
>    do we need to make it optional and keep it around?
> 
>  - Current series does not add any Kconfig options, but
>    simply switches SOF to use HDA codecs for all, including
>    HDMI/DP. This means hdac_hdmi is never used with SOF
>    and could be dropped (if SST is ok as well).
>  - This may break some usage with SST (input is welcome!)
>  - The change is visible to applications. The ALSA mixer
>    interface is different (OTOH with the new driver, playback
>    works out-of-the-box while with hdac_hdmi you needed to
>    set the multiple controls first, to have any audio out).
>  - Alternatively I could add a KConfig option and we could
>    have a deprecation period for hdac_hdmi, allowing people
>    to compile with the old driver during transition time.
>    This will require #ifdef'ing in all the machine drivers.
> 
> .. once these are addressed, I can proceed to extend the patchset
> with all affected machine drivers.
> 
> Feature and testing info:
> 
>  - Tested on multiple Intel platforms supported by SOF.
>  - Tested with ALSA console tools as well as with Pulseaudio.
>       - requires Pulseaudio 12.x or newer, see
>         https://lists.freedesktop.org/archives/pulseaudio-discuss/2019-August/031358.html
>  - HDMI, DP, DP-MST with multi-monitor use-scenarios work ok.
>  - New feature for SOF: ELD /proc fs works just like in
>    DSP-less mode.
>  - New feature for SOF: jack detection works out-of-the-box
>    with Pulseaudio (no need for card specific UCM for HDMI)
>  - Pre-reviewed at:
>    https://github.com/thesofproject/linux/pull/1155

IMO, the only and the most important point is whether it works as-is
without changing the existing user-space, or exactly what scenario
would be broken.  If the breakage is significant, we may introduce a
Kconfig, as you suggested.

I don't think the mixer contents change are problematic.  In the case
of HDMI/DP, it's mostly read-only for fetching ELD or jack state.

Other than that, I like the idea, the code change looks simple
enough, and it'd make maintenance easier.


thanks,

Takashi

> 
> Kai Vehmanen (7):
>   ALSA: hda - add mst_no_extra_pcms flag to hda_codec
>   ASoC: Intel: skl-hda-dsp-generic: use snd-hda-codec-hdmi
>   ASoC: hdac_hda: add support for HDMI/DP as a HDA codec
>   ALSA: hda/hdmi - allow control creation without a linked pcm
>   ALSA: hda/hdmi - implement mst_no_extra_pcms flag
>   ALSA: hda/hdmi - complete pcm_setup_pin without snd_pcm link
>   ASoC: SOF: Intel: load hda codec module also for HDMI/DP
> 
>  include/sound/hda_codec.h                    |  1 +
>  sound/pci/hda/patch_hdmi.c                   | 31 ++++---
>  sound/soc/codecs/hdac_hda.c                  | 95 +++++++++++++++++---
>  sound/soc/codecs/hdac_hda.h                  | 10 ++-
>  sound/soc/intel/boards/skl_hda_dsp_common.c  | 27 ++----
>  sound/soc/intel/boards/skl_hda_dsp_common.h  | 61 +++++++++++++
>  sound/soc/intel/boards/skl_hda_dsp_generic.c |  7 --
>  sound/soc/sof/intel/hda-codec.c              | 11 +--
>  sound/soc/sof/intel/hda.h                    |  5 +-
>  9 files changed, 189 insertions(+), 59 deletions(-)
> 
> -- 
> 2.17.1
>
Kai Vehmanen Sept. 3, 2019, 2:18 p.m. UTC | #2
Hi,

On Thu, 29 Aug 2019, Takashi Iwai wrote:

>> here's a RFC patch series that adapts SOF (and one example machine
>> driver) to use snd-hda-codec-hdmi (patch_hdmi.c) codec driver
>> instead of hdac_hdmi (soc/codecs/hdac_hdmi.c). The primary goal
[...]
>> 2) Can we drop hdac_hdmi and its support from machine drivers, or
>>    do we need to make it optional and keep it around?
> 
> IMO, the only and the most important point is whether it works as-is
> without changing the existing user-space, or exactly what scenario
> would be broken.  If the breakage is significant, we may introduce a
> Kconfig, as you suggested.
> 
> I don't think the mixer contents change are problematic.  In the case
> of HDMI/DP, it's mostly read-only for fetching ELD or jack state.

I've been now continuing testing with different combinations of 
kernel/user-space and the two main problematic areas are:

1) systems with UCM defined with hdac-hdmi style controls

-> as the card name will not change, the UCM usage will fail 
when kernel is updated to use different HDMI codec driver

On some systems this is manageable as e.g. pulseaudio will fallback to 
legacy non-UCM path and e.g. HDMI/DP audio keeps working. But, but, this 
may be problematic if UCM is needed for other functionality on these 
systems.

2) machine drivers shared with SST/SOF

Doing the HDMI codec change for old platforms handled with SST driver 
looks difficult to do, so we probably need to keep hdac-hdmi around for 
SST usage. That does mean machine drivers that are shared, need to support 
both options.

Combining 1+2, it would seem safer to have a opt-in/opt-out possibility 
via Kconfig. I'm preparing a patchset for this -- let's see how it will 
look.

Br, Kai
Takashi Iwai Sept. 3, 2019, 3:11 p.m. UTC | #3
On Tue, 03 Sep 2019 16:18:11 +0200,
Kai Vehmanen wrote:
> 
> Hi,
> 
> On Thu, 29 Aug 2019, Takashi Iwai wrote:
> 
> >> here's a RFC patch series that adapts SOF (and one example machine
> >> driver) to use snd-hda-codec-hdmi (patch_hdmi.c) codec driver
> >> instead of hdac_hdmi (soc/codecs/hdac_hdmi.c). The primary goal
> [...]
> >> 2) Can we drop hdac_hdmi and its support from machine drivers, or
> >>    do we need to make it optional and keep it around?
> > 
> > IMO, the only and the most important point is whether it works as-is
> > without changing the existing user-space, or exactly what scenario
> > would be broken.  If the breakage is significant, we may introduce a
> > Kconfig, as you suggested.
> > 
> > I don't think the mixer contents change are problematic.  In the case
> > of HDMI/DP, it's mostly read-only for fetching ELD or jack state.
> 
> I've been now continuing testing with different combinations of 
> kernel/user-space and the two main problematic areas are:
> 
> 1) systems with UCM defined with hdac-hdmi style controls
> 
> -> as the card name will not change, the UCM usage will fail 
> when kernel is updated to use different HDMI codec driver
> 
> On some systems this is manageable as e.g. pulseaudio will fallback to 
> legacy non-UCM path and e.g. HDMI/DP audio keeps working. But, but, this 
> may be problematic if UCM is needed for other functionality on these 
> systems.

Just out of curiosity: which systems are with such UCM profiles?
Chromebooks?

> 2) machine drivers shared with SST/SOF
> 
> Doing the HDMI codec change for old platforms handled with SST driver 
> looks difficult to do, so we probably need to keep hdac-hdmi around for 
> SST usage. That does mean machine drivers that are shared, need to support 
> both options.
> 
> Combining 1+2, it would seem safer to have a opt-in/opt-out possibility 
> via Kconfig. I'm preparing a patchset for this -- let's see how it will 
> look.

Agreed, some Kconfig is a safer option for now.


thanks,

Takashi
Kai Vehmanen Sept. 3, 2019, 4:16 p.m. UTC | #4
Hi,

On Tue, 3 Sep 2019, Takashi Iwai wrote:

> On Tue, 03 Sep 2019 16:18:11 +0200, Kai Vehmanen wrote:
> > On some systems this is manageable as e.g. pulseaudio will fallback to 
> > legacy non-UCM path and e.g. HDMI/DP audio keeps working. But, but, this 
> > may be problematic if UCM is needed for other functionality on these 
> > systems.
> 
> Just out of curiosity: which systems are with such UCM profiles?
> Chromebooks?

I believe this year's XPS 13 Developer edition is one such device. I'll 
try to confirm if there are more cases.

Br, Kai