[alsa-devel] [PATCH] ASoC: Intel: hdac_hdmi: add Icelake support

[Takashi Iwai]

On Fri, 16 Nov 2018 15:11:27 +0100,
Pierre-Louis Bossart wrote:
> 
> On 11/16/18 7:49 AM, Takashi Iwai wrote:
> 
>     On Wed, 14 Nov 2018 15:53:23 +0100,
>     Takashi Iwai wrote:
>     
>         On Mon, 12 Nov 2018 15:00:36 +0100,
>         Pierre-Louis Bossart wrote:
>         
>             And btw the big topic is still how we provide distributions the means
>             to handle a 'graceful' fallback from DSP-enabled solutions (SST or
>             SOF) to legacy HDAudio, it's already popped up for cases where we have
>             HDaudio solutions with DMICs.
>             
>         Yeah, that's a long-standing problem.  I've experimented some
>         scenarios in the past, and the conclusion is that there is no really
>         working fallback mechanism in general in Linux driver binding.
>         That is, the only reasonable way seems to make a dedicated driver for
>         the specific PCI ID (SKL+) doing the probe-and-fallback by itself,
>         while excluding these IDs from other existing driver entries.
>         
>         I'd love to proceed this but unfortunately I have no machine that can
>         run SKL+ SST driver right now.  I have a new CFL devel box, but it has
>         no support (PCI ID 8086:a348) as well as no firmware...
>         
>     So, with a working machine, I could finally hack this a bit.
>     Below is a freshly cooked patch(set).  The first one is just to add
>     the support for my CFL machine, and another one is to add the fallback
>     binding with the legacy HD-audio on snd-soc-skl.
>     
>     The changes aren't that big.  And you can still control the binding
>     via a module option, e.g. snd_soc_skl.legacy=1 will let it bound only
>     with the legacy driver, snd_soc_skl.legacy=2 for ASoC only.  The
>     default is 0, the fallback to legacy if ASoC binding fails.
>     
>     It's still a PoC, no proper patch description is put yet.
>     
> Thanks Takashi, much appreciate. will give it a try later today.
> 
> One comment is that we will have to deal with SOF as well. While the end-goal
> is to converge on a single SOF-based driver, it'll take time to support all
> shipping platforms and deal with firmware authentication issues, so the
> fallback could be SOF->SST->legacy (hopefully on a small set of platforms).
> SST->SOF will likely never happen.

OK, good to know.

> I also wanted to try what happens with your solution if the authentication
> fails with SST (as in your case with the current CNL firmware). It's my
> understanding that the firmware download is deferred with a work queue and
> takes time anyways, so maybe the decision not to go back to legacy would be
> made too early? We'd also need to check with a platform where the DSP is not
> enabled to see if the fallback happens immediately (no need to try and
> download firmware to a non-enabled DSP).

It's a good question, and I guess it's hard to give a fallback in that
level.  IOW, the fallback to the legacy is only for the case where no
DSP is available at all in the hardware level.  If the legacy driver
is required by some reason (faulty hardware detection or the
non-public firmware), we can still enforce to the legacy mode via a
module option.

But for the fallback of SOF to SST is a different question.  I guess
we should check the availability of the firmware at the beginning of
probe?  But request_firmware() inside the probe is basically no-go
(although it might work), so the fallback point would be pushed later
in that case.

Even my simple PoC has a potential problem: as it doesn't reach to the
probe error in the driver core, the previously called devres and other
stuff aren't cleared at calling the fallback driver.  This problem
will remain even if we change the fallback place, so this needs to be
solved (or just ignore if it's only a few memory allocations).

> And while I am at it, we may want to think about a single table for PCI IDs so
> that by construction we remove the risk of a platform supported with legacy
> but not the others, and vice versa.

It's also what I thought, too, but it turned out to be difficult
because both drivers require the own driver_data field.


thanks,

Takashi