[alsa-devel] [PATCH 00/35] ASoC: Intel: Clenaup SST initialization

[Cezary Rojewski]

On 2019-08-23 12:26, Mark Brown wrote:
> On Fri, Aug 23, 2019 at 10:29:59AM +0200, Cezary Rojewski wrote:
>> On 2019-08-22 22:55, Pierre-Louis Bossart wrote:
>>> On 8/22/19 2:03 PM, Cezary Rojewski wrote:
> 
>>>> Code seen here is part of new Skylake fundament, located at the very
>>>> bottom of internal mainline. Said mainline is tested constantly on at
>>>> least sigle platform from every cAVS bucket (description below). This
>>>> week, BDW has been added to the CI family and was essential in
>>>> validating legacy changes. Baytrail platform is still missing. Changes
>>>> for BYT directly mirror HSW/ BDW but due to current lack of platform
>>>> were untested.
>>>> Boards engaged in testing: rt286, rt298, rt274.
> 
>>> this is not enough, sorry. these are RVPs and you need to check with
>>> commercial devices supported in sound/soc/intel/boards/.
> 
>> What machine board has to do with FW and host side? If it has, we better
>> notify the owner so he can fix codec's code at once. All boards MUST follow
>> recommended protocol whether its HDA or I2S in communicating with /skylake.
>> This is hardware IP we taking about. I could as well test all platforms with
>> AudioPrecision and say: shipit.
> 
> ...
> 
>> DSP "commercial devices" with 99% of home audio being routed through
>> HD-Audio legacy? I do contact representatives of "commercial devices" daily,
>> you of all should be aware of fact that in almost all cases they are fed
>> neither with upstream code nor upstream binaries.
>> For the first time since eons sound/soc/intel/skylake code is tested before
>> upstreaming, yet you still defend the mistakes of the past?
> 
> System vendors don't really matter here, end users with their
> desktops and laptops do.  If a user has a system and they for
> whatever reason upgrade their kernel from one upstream version to
> another and don't touch any other aspect of their system the
> expectation is that they'll still have everything working after
> the upgrade.  This means that if there's bugs in how things were
> deployed in the past the kernel ought to try to work with those
> bugs.
> 

Noted, see below comments.

>>>> Some upstream FW binaries are not compatible with existing /skylake
>>>> driver while changes found here (HARDWARE_CONFIG/ FIRMWARE_CONFIG) make
>>>> use of firmware ability to offload hardware-specifics away from driver.
>>>> These and more are core part of any cAVS design and are to be
>>>> implemented and used by host. This too is missing on Linux upstream.
> 
> This sounds like it might be a problem.
> 

Problem is, HARDWARE/ FIRMWARE_CONFIG (and more upcoming) should be the 
core part of cAVS driver, implemented before any PCM related code is added.

>>>> SKL FW binary existing on upstream is a descendant of old spt branch,
>>>> obsoleted for 4-5 years now. That FW is a stub, quickly replaced by
>>>> kbl which is to be used on all 1.5 cAVS platforms.
> 
> That's well within the lifespan people will expect from a PC
> these days, my personal systems are mostly older than that and
> do fine at most things except for big builds.
> 

It's not about age itself. It's about the fact that FW binaries from 
non-supported or main FW branches ended here and given the date these 
have been added, it has already been recommended to make use of kbl or 
apl_auto branches.

>> If I could, I'd rather prefer the "detect and notify" as it is impossible to
>> repair all the mistakes made in /sound/soc/intel/skylake.
> 
> Do we have a sense of how many such systems exist?
> 
>> However, in practice there isn't any reliable way to verify the actual
>> usability of old FW binary against host site as the interface is volatile
>> and numbers alone don't mean much.
>> Patch with FW binaries would not remove old ones, simply add new versions to
>> the directory.
> 
> Can you do things the other way around and positively identify
> firmwares that meet whatever standards you're interested in here?
> 

The only thing that comes to my mind is the following:
- during boot up sequence, in response to any INVALID_REQUEST or such 
coming from FW, collapse and dump: "upgrade firmware" message

- once boot up sequence is completed, disregard INVALID_REQUEST check as 
it is also the common response of FW in various scenarios

- user removes existing sym link from /lib/firmware/intel and creates 
new one, pointing to updated FW binary that should also be present in 
/lib/firmware/intel