Re: [PATCH] ASoC: Intel: hdac_hdmi: add Icelake support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Takashi,

     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).

I took a longer look at the suggested code, and it's good enough for me - actually smarter than what I had in mind :-).

What we really want is detect if the DSP is present or not, and fall back to legacy if not.

The problems with firmware are one step too far, there are cases where we have firmware but it's the wrong one due to authentication issues that cannot be handled automagically.

Likewise the fallback between SOF and SST is also one bridge too far. If the problem is with firmware authentication, then likewise it's something that needs to be sorted out by Intel releasing the correct firmware and/or users selecting a different firmware which is specific to their platforms.

Also the SST driver depends on topology files that have not been shared in most cases, so a generic distribution that doesn't have access to all the parts would just not work. It's just simpler to select what is known to work (e.g. legacy until KBL, SST after), and gradually deprecate the SST driver as both features and topologies are provided with SOF.

You have a good point on the allocations, but if we modify the skylake driver to only allocate all the resources if the DSP is present, then it's only a couple of structures that matter. Today the test

/* check if dsp is there */
    if (bus->ppcap) {

in skl.c is done too late, there is no point in e.g allocating DMA pages and stuff if the DSP is not there is the first place...


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.

Yes agree. I think we can have a more granular approach though, it's simple enough to change the Skylake driver and select SKL/APL/GLK/KBL independently (as we do for SOF), so we could do this fallback on an SOC basis, e.g something like:

/* Sunrise Point-LP */
    { PCI_DEVICE(0x8086, 0x9d70),
      .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
      IS_ENABLED(CONFIG_SND_HDA_INTEL_SKL_SHARED) & AZX_DCAPS_INTEL_SHARED },

with SND_HDA_INTEL_SKL_SHARED selected by the SKL driver.

this would scale well with SOF as well.

Thanks for starting this POC, much appreciated.

-Pierre

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux