mbox series

[00/35] ASoC: Intel: Clenaup SST initialization

Message ID 20190822190425.23001-1-cezary.rojewski@intel.com (mailing list archive)
Headers show
Series ASoC: Intel: Clenaup SST initialization | expand

Message

Cezary Rojewski Aug. 22, 2019, 7:03 p.m. UTC
This segment targets initialization procedures for all SST drivers.
For legacy:
- it concentrates on updating ACPI loader so generic DSP framework is
  not fed any platform specific data. sst-acpi module ceases to exist
  and is replaced by hsw-acpi and byt-acpi

For cAVS (Skylake+):
- Repairs what is currently an initialization mess, given the order of
  invocation of engaged handlers and sheer amount of them.
- provides interface to offload hardware-specifics away from driver

Following is the order of Skylake initialization currently:
- skl_probe
   -> schedule skl_probe_work
- skl_first_init
- skl_create

-> skl_init_dsp
--> skl/ bxt/ cnl_sst_dsp_init
---> skl_sst_ctx_init
----> skl_dsp_ctx_init
-----> sst_dsp_new
------> sst_ops::init (invoked but unimplemented!)

Listing all the types engaged together with the naming chosen for the
above paints an even darker picture. Code is unreadable and hides
initialization for diffenret members is various handlers. Moreover, due
to existing ill relationship between skl_dev, sst_dsp and
sst_generic_ipc, one must be extra careful when accessing so called
"dsp/ thread_context" as they all don't get initialized immediately
before object is yield for further processing. However, 100/100 series
is nonsense and thus cleanups have been divided into chunks and
prioritized.

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.

!!! IMPORTANT !!!

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.

As explanined once, five main FW branches are available:
- kbl, 1.5 cAVS (supports SKL, KBL, KBL-R, ABL, more..)
- apl_auto, 1.5+ cAVS (supports APL, GLK)
- cnl, 1.8 cAVS (supports CNL, CFL, WHL, CML)
- icl, 2.0 cAVS (supports ICL, LKF)
- master, 2.5 cAVS (supports TGL, EHL and everything above)

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.

The same story goes with bxtn binary, a descendant of apl branch,
replaced by apl_auto 2-3 years ago. All vendors, entire validation and
development is located on apl_auto.

Message: all FW binaries are to be updated as we cannot guarantee these
are still functional. Given the fact that all vendors are fed with new
binaries on regular basis from all main branches, it's highly probable
some scenarios fail with existing FWs. In consequence, linux-firmware
patch will be provided with fresh, updated binaries, soon.
Once more, please note, we do not support, nor test platforms using
obsolete FW binaries. That includes all platforms, not just SKL or APL.

Amadeusz Sławiński (1):
  ASoC: Intel: Skylake: Put FW runtime params defs in one place

Cezary Rojewski (34):
  ASoC: Intel: Skylake: Add FIRMWARE_CONFIG IPC request
  ASoC: Intel: Skylake: Add HARDWARE_CONFIG IPC request
  ASoC: Intel: Skylake: Unify firmware loading mechanism
  ASoC: Intel: Skylake: Reload libraries on D0 entry for CNL
  ASoC: Intel: Skylake: Unhardcode dsp cores number
  ASoC: Intel: Skylake: Update interrupt disabling routine
  ASoC: Intel: Skylake: Inline ipc free operations
  ASoC: Intel: Skylake: Unify driver cleanup mechanism
  ASoC: Intel: Relocate irq thread header to sst_ops
  ASoC: Intel: Merge sst_dsp_device into sst_pdata
  ASoC: Intel: Skylake: Reuse sst_dsp_free
  ASoC: Intel: Skylake: Reuse sst_dsp_new
  ASoC: Intel: Skylake: Remove skl_dsp_acquire_irq
  ASoC: Intel: Skylake: Use dsp loading functions directly
  ASoC: Intel: Skylake: Make dsp_ops::stream_tag obsolete
  ASoC: Intel: Skylake: Remove skl_dsp_loader_ops
  ASoC: Intel: Skylake: Remove window0 sst_addr fields
  ASoC: Intel: Skylake: Remove redundant W0 and W1 macros
  ASoC: Intel: Skylake: Remove redundant SRAM fields
  ASoC: Intel: Expose ACPI loading members
  ASoC: Intel: Haswell: Define separate ACPI loader
  ASoC: Intel: Baytrail: Define separate ACPI loader
  ASoC: Intel: Refactor probing of ACPI devices
  ASoC: Intel: Skylake: Simplify skl_sst_ctx_init declaration
  ASoC: Intel: Skylake: Simplify all sst_dsp_init declarations
  ASoC: Intel: Skylake: Define platform descriptors
  ASoC: Intel: Skylake: Update skl_ids table
  ASoC: Intel: Skylake: Flip SST initialization order
  ASoC: Intel: Reuse sst_pdata::fw_name field
  ASoC: Intel: Reuse sst_pdata::fw field
  ASoC: Intel: Skylake: Remove skl_dsp_ops
  ASoC: Intel: Skylake: Privatize SST init handlers
  ASoC: Intel: Skylake: Merge skl_sst_ctx_init into skl_init_dsp
  ASoC: Intel: Remove obsolete firmware fields

 sound/soc/intel/Kconfig                       |  14 +-
 sound/soc/intel/baytrail/Makefile             |   2 +
 sound/soc/intel/baytrail/acpi.c               |  64 +++++
 sound/soc/intel/baytrail/sst-baytrail-dsp.c   |   2 +-
 sound/soc/intel/baytrail/sst-baytrail-ipc.c   |  13 +-
 sound/soc/intel/baytrail/sst-baytrail-ipc.h   |   2 +
 sound/soc/intel/common/Makefile               |   4 +-
 .../intel/common/soc-acpi-intel-bxt-match.c   |   2 -
 .../intel/common/soc-acpi-intel-byt-match.c   |   2 -
 .../intel/common/soc-acpi-intel-cnl-match.c   |   1 -
 .../intel/common/soc-acpi-intel-glk-match.c   |   3 -
 .../intel/common/soc-acpi-intel-hda-match.c   |   2 -
 .../common/soc-acpi-intel-hsw-bdw-match.c     |   4 -
 .../intel/common/soc-acpi-intel-icl-match.c   |   1 -
 .../intel/common/soc-acpi-intel-kbl-match.c   |  12 -
 .../intel/common/soc-acpi-intel-skl-match.c   |   3 -
 sound/soc/intel/common/sst-acpi.c             | 117 +--------
 sound/soc/intel/common/sst-dsp-priv.h         |   8 +-
 sound/soc/intel/common/sst-dsp.h              |  37 +--
 sound/soc/intel/common/sst-firmware.c         |  13 +-
 sound/soc/intel/haswell/Makefile              |   2 +
 sound/soc/intel/haswell/acpi.c                |  78 ++++++
 sound/soc/intel/haswell/sst-haswell-dsp.c     |   1 +
 sound/soc/intel/haswell/sst-haswell-ipc.c     |  13 +-
 sound/soc/intel/haswell/sst-haswell-ipc.h     |   2 +
 sound/soc/intel/skylake/bxt-sst.c             | 143 ++++-------
 sound/soc/intel/skylake/cnl-sst-dsp.c         |  13 +-
 sound/soc/intel/skylake/cnl-sst-dsp.h         |  15 +-
 sound/soc/intel/skylake/cnl-sst.c             | 142 ++++-------
 sound/soc/intel/skylake/skl-debug.c           |   2 +-
 sound/soc/intel/skylake/skl-messages.c        | 194 ++------------
 sound/soc/intel/skylake/skl-pcm.c             |  22 +-
 sound/soc/intel/skylake/skl-sst-cldma.c       |  10 +-
 sound/soc/intel/skylake/skl-sst-dsp.c         |  79 ++----
 sound/soc/intel/skylake/skl-sst-dsp.h         |  55 ++--
 sound/soc/intel/skylake/skl-sst-ipc.c         | 238 ++++++++++++++++--
 sound/soc/intel/skylake/skl-sst-ipc.h         | 124 ++++++++-
 sound/soc/intel/skylake/skl-sst-utils.c       |  28 +--
 sound/soc/intel/skylake/skl-sst.c             | 150 ++++++-----
 sound/soc/intel/skylake/skl.c                 |  65 +++--
 sound/soc/intel/skylake/skl.h                 |  26 +-
 41 files changed, 877 insertions(+), 831 deletions(-)
 create mode 100644 sound/soc/intel/baytrail/acpi.c
 create mode 100644 sound/soc/intel/haswell/acpi.c

Comments

Pierre-Louis Bossart Aug. 22, 2019, 8:55 p.m. UTC | #1
On 8/22/19 2:03 PM, Cezary Rojewski wrote:
> This segment targets initialization procedures for all SST drivers.
> For legacy:
> - it concentrates on updating ACPI loader so generic DSP framework is
>    not fed any platform specific data. sst-acpi module ceases to exist
>    and is replaced by hsw-acpi and byt-acpi
> 
> For cAVS (Skylake+):
> - Repairs what is currently an initialization mess, given the order of
>    invocation of engaged handlers and sheer amount of them.
> - provides interface to offload hardware-specifics away from driver
> 
> Following is the order of Skylake initialization currently:
> - skl_probe
>     -> schedule skl_probe_work
> - skl_first_init
> - skl_create
> 
> -> skl_init_dsp
> --> skl/ bxt/ cnl_sst_dsp_init
> ---> skl_sst_ctx_init
> ----> skl_dsp_ctx_init
> -----> sst_dsp_new
> ------> sst_ops::init (invoked but unimplemented!)
> 
> Listing all the types engaged together with the naming chosen for the
> above paints an even darker picture. Code is unreadable and hides
> initialization for diffenret members is various handlers. Moreover, due
> to existing ill relationship between skl_dev, sst_dsp and
> sst_generic_ipc, one must be extra careful when accessing so called
> "dsp/ thread_context" as they all don't get initialized immediately
> before object is yield for further processing. However, 100/100 series
> is nonsense and thus cleanups have been divided into chunks and
> prioritized.
> 
> 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/.

> 
> !!! IMPORTANT !!!
> 
> 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.
> 
> As explanined once, five main FW branches are available:
> - kbl, 1.5 cAVS (supports SKL, KBL, KBL-R, ABL, more..)
> - apl_auto, 1.5+ cAVS (supports APL, GLK)
> - cnl, 1.8 cAVS (supports CNL, CFL, WHL, CML)
> - icl, 2.0 cAVS (supports ICL, LKF)
> - master, 2.5 cAVS (supports TGL, EHL and everything above)
> 
> 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.
> 
> The same story goes with bxtn binary, a descendant of apl branch,
> replaced by apl_auto 2-3 years ago. All vendors, entire validation and
> development is located on apl_auto.
> 
> Message: all FW binaries are to be updated as we cannot guarantee these
> are still functional. Given the fact that all vendors are fed with new
> binaries on regular basis from all main branches, it's highly probable
> some scenarios fail with existing FWs. In consequence, linux-firmware
> patch will be provided with fresh, updated binaries, soon.
> Once more, please note, we do not support, nor test platforms using
> obsolete FW binaries. That includes all platforms, not just SKL or APL.

"we don't break userspace" means you have to be backwards compatible 
with whatever the user has on their filesystem when the kernel is updated.
Or if you go ahead, you need to detect that the firmware is not 
supported and make it known to the user that it needs to be updated.

> 
> Amadeusz Sławiński (1):
>    ASoC: Intel: Skylake: Put FW runtime params defs in one place
> 
> Cezary Rojewski (34):
>    ASoC: Intel: Skylake: Add FIRMWARE_CONFIG IPC request
>    ASoC: Intel: Skylake: Add HARDWARE_CONFIG IPC request
>    ASoC: Intel: Skylake: Unify firmware loading mechanism
>    ASoC: Intel: Skylake: Reload libraries on D0 entry for CNL
>    ASoC: Intel: Skylake: Unhardcode dsp cores number
>    ASoC: Intel: Skylake: Update interrupt disabling routine
>    ASoC: Intel: Skylake: Inline ipc free operations
>    ASoC: Intel: Skylake: Unify driver cleanup mechanism
>    ASoC: Intel: Relocate irq thread header to sst_ops
>    ASoC: Intel: Merge sst_dsp_device into sst_pdata
>    ASoC: Intel: Skylake: Reuse sst_dsp_free
>    ASoC: Intel: Skylake: Reuse sst_dsp_new
>    ASoC: Intel: Skylake: Remove skl_dsp_acquire_irq
>    ASoC: Intel: Skylake: Use dsp loading functions directly
>    ASoC: Intel: Skylake: Make dsp_ops::stream_tag obsolete
>    ASoC: Intel: Skylake: Remove skl_dsp_loader_ops
>    ASoC: Intel: Skylake: Remove window0 sst_addr fields
>    ASoC: Intel: Skylake: Remove redundant W0 and W1 macros
>    ASoC: Intel: Skylake: Remove redundant SRAM fields
>    ASoC: Intel: Expose ACPI loading members
>    ASoC: Intel: Haswell: Define separate ACPI loader
>    ASoC: Intel: Baytrail: Define separate ACPI loader
>    ASoC: Intel: Refactor probing of ACPI devices
>    ASoC: Intel: Skylake: Simplify skl_sst_ctx_init declaration
>    ASoC: Intel: Skylake: Simplify all sst_dsp_init declarations
>    ASoC: Intel: Skylake: Define platform descriptors
>    ASoC: Intel: Skylake: Update skl_ids table
>    ASoC: Intel: Skylake: Flip SST initialization order
>    ASoC: Intel: Reuse sst_pdata::fw_name field
>    ASoC: Intel: Reuse sst_pdata::fw field
>    ASoC: Intel: Skylake: Remove skl_dsp_ops
>    ASoC: Intel: Skylake: Privatize SST init handlers
>    ASoC: Intel: Skylake: Merge skl_sst_ctx_init into skl_init_dsp
>    ASoC: Intel: Remove obsolete firmware fields
> 
>   sound/soc/intel/Kconfig                       |  14 +-
>   sound/soc/intel/baytrail/Makefile             |   2 +
>   sound/soc/intel/baytrail/acpi.c               |  64 +++++
>   sound/soc/intel/baytrail/sst-baytrail-dsp.c   |   2 +-
>   sound/soc/intel/baytrail/sst-baytrail-ipc.c   |  13 +-
>   sound/soc/intel/baytrail/sst-baytrail-ipc.h   |   2 +
>   sound/soc/intel/common/Makefile               |   4 +-
>   .../intel/common/soc-acpi-intel-bxt-match.c   |   2 -
>   .../intel/common/soc-acpi-intel-byt-match.c   |   2 -
>   .../intel/common/soc-acpi-intel-cnl-match.c   |   1 -
>   .../intel/common/soc-acpi-intel-glk-match.c   |   3 -
>   .../intel/common/soc-acpi-intel-hda-match.c   |   2 -
>   .../common/soc-acpi-intel-hsw-bdw-match.c     |   4 -
>   .../intel/common/soc-acpi-intel-icl-match.c   |   1 -
>   .../intel/common/soc-acpi-intel-kbl-match.c   |  12 -
>   .../intel/common/soc-acpi-intel-skl-match.c   |   3 -
>   sound/soc/intel/common/sst-acpi.c             | 117 +--------
>   sound/soc/intel/common/sst-dsp-priv.h         |   8 +-
>   sound/soc/intel/common/sst-dsp.h              |  37 +--
>   sound/soc/intel/common/sst-firmware.c         |  13 +-
>   sound/soc/intel/haswell/Makefile              |   2 +
>   sound/soc/intel/haswell/acpi.c                |  78 ++++++
>   sound/soc/intel/haswell/sst-haswell-dsp.c     |   1 +
>   sound/soc/intel/haswell/sst-haswell-ipc.c     |  13 +-
>   sound/soc/intel/haswell/sst-haswell-ipc.h     |   2 +
>   sound/soc/intel/skylake/bxt-sst.c             | 143 ++++-------
>   sound/soc/intel/skylake/cnl-sst-dsp.c         |  13 +-
>   sound/soc/intel/skylake/cnl-sst-dsp.h         |  15 +-
>   sound/soc/intel/skylake/cnl-sst.c             | 142 ++++-------
>   sound/soc/intel/skylake/skl-debug.c           |   2 +-
>   sound/soc/intel/skylake/skl-messages.c        | 194 ++------------
>   sound/soc/intel/skylake/skl-pcm.c             |  22 +-
>   sound/soc/intel/skylake/skl-sst-cldma.c       |  10 +-
>   sound/soc/intel/skylake/skl-sst-dsp.c         |  79 ++----
>   sound/soc/intel/skylake/skl-sst-dsp.h         |  55 ++--
>   sound/soc/intel/skylake/skl-sst-ipc.c         | 238 ++++++++++++++++--
>   sound/soc/intel/skylake/skl-sst-ipc.h         | 124 ++++++++-
>   sound/soc/intel/skylake/skl-sst-utils.c       |  28 +--
>   sound/soc/intel/skylake/skl-sst.c             | 150 ++++++-----
>   sound/soc/intel/skylake/skl.c                 |  65 +++--
>   sound/soc/intel/skylake/skl.h                 |  26 +-
>   41 files changed, 877 insertions(+), 831 deletions(-)
>   create mode 100644 sound/soc/intel/baytrail/acpi.c
>   create mode 100644 sound/soc/intel/haswell/acpi.c
>
Cezary Rojewski Aug. 23, 2019, 8:29 a.m. UTC | #2
On 2019-08-22 22:55, Pierre-Louis Bossart wrote:
> 
> 
> On 8/22/19 2:03 PM, Cezary Rojewski wrote:
>> This segment targets initialization procedures for all SST drivers.
>> For legacy:
>> - it concentrates on updating ACPI loader so generic DSP framework is
>>    not fed any platform specific data. sst-acpi module ceases to exist
>>    and is replaced by hsw-acpi and byt-acpi
>>
>> For cAVS (Skylake+):
>> - Repairs what is currently an initialization mess, given the order of
>>    invocation of engaged handlers and sheer amount of them.
>> - provides interface to offload hardware-specifics away from driver
>>
>> Following is the order of Skylake initialization currently:
>> - skl_probe
>>     -> schedule skl_probe_work
>> - skl_first_init
>> - skl_create
>>
>> -> skl_init_dsp
>> --> skl/ bxt/ cnl_sst_dsp_init
>> ---> skl_sst_ctx_init
>> ----> skl_dsp_ctx_init
>> -----> sst_dsp_new
>> ------> sst_ops::init (invoked but unimplemented!)
>>
>> Listing all the types engaged together with the naming chosen for the
>> above paints an even darker picture. Code is unreadable and hides
>> initialization for diffenret members is various handlers. Moreover, due
>> to existing ill relationship between skl_dev, sst_dsp and
>> sst_generic_ipc, one must be extra careful when accessing so called
>> "dsp/ thread_context" as they all don't get initialized immediately
>> before object is yield for further processing. However, 100/100 series
>> is nonsense and thus cleanups have been divided into chunks and
>> prioritized.
>>
>> 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/.
> 

+Filip
+Michal

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?

I don't mind being extra careful, but testing every existing machine 
board is nonsense and you know it.
Extra careful: this should have been the main topic enforced by Intel 
audio representatives when existing stuff been upstreamed - years ago.

>>
>> !!! IMPORTANT !!!
>>
>> 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.
>>
>> As explanined once, five main FW branches are available:
>> - kbl, 1.5 cAVS (supports SKL, KBL, KBL-R, ABL, more..)
>> - apl_auto, 1.5+ cAVS (supports APL, GLK)
>> - cnl, 1.8 cAVS (supports CNL, CFL, WHL, CML)
>> - icl, 2.0 cAVS (supports ICL, LKF)
>> - master, 2.5 cAVS (supports TGL, EHL and everything above)
>>
>> 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.
>>
>> The same story goes with bxtn binary, a descendant of apl branch,
>> replaced by apl_auto 2-3 years ago. All vendors, entire validation and
>> development is located on apl_auto.
>>
>> Message: all FW binaries are to be updated as we cannot guarantee these
>> are still functional. Given the fact that all vendors are fed with new
>> binaries on regular basis from all main branches, it's highly probable
>> some scenarios fail with existing FWs. In consequence, linux-firmware
>> patch will be provided with fresh, updated binaries, soon.
>> Once more, please note, we do not support, nor test platforms using
>> obsolete FW binaries. That includes all platforms, not just SKL or APL.
> 
> "we don't break userspace" means you have to be backwards compatible 
> with whatever the user has on their filesystem when the kernel is updated.
> Or if you go ahead, you need to detect that the firmware is not 
> supported and make it known to the user that it needs to be updated.
> 

Gotta be honest here. I'm surprised SPT binary even made it here. Same 
goes for bxtn one. The only real-life example of bxtn for linux is for 
4.1.X kernels - where YES, /skylake and topology and a lot more stuff 
does not even exist and is simply backported internally.

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.

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.


Czarek

>>
>> Amadeusz Sławiński (1):
>>    ASoC: Intel: Skylake: Put FW runtime params defs in one place
>>
>> Cezary Rojewski (34):
>>    ASoC: Intel: Skylake: Add FIRMWARE_CONFIG IPC request
>>    ASoC: Intel: Skylake: Add HARDWARE_CONFIG IPC request
>>    ASoC: Intel: Skylake: Unify firmware loading mechanism
>>    ASoC: Intel: Skylake: Reload libraries on D0 entry for CNL
>>    ASoC: Intel: Skylake: Unhardcode dsp cores number
>>    ASoC: Intel: Skylake: Update interrupt disabling routine
>>    ASoC: Intel: Skylake: Inline ipc free operations
>>    ASoC: Intel: Skylake: Unify driver cleanup mechanism
>>    ASoC: Intel: Relocate irq thread header to sst_ops
>>    ASoC: Intel: Merge sst_dsp_device into sst_pdata
>>    ASoC: Intel: Skylake: Reuse sst_dsp_free
>>    ASoC: Intel: Skylake: Reuse sst_dsp_new
>>    ASoC: Intel: Skylake: Remove skl_dsp_acquire_irq
>>    ASoC: Intel: Skylake: Use dsp loading functions directly
>>    ASoC: Intel: Skylake: Make dsp_ops::stream_tag obsolete
>>    ASoC: Intel: Skylake: Remove skl_dsp_loader_ops
>>    ASoC: Intel: Skylake: Remove window0 sst_addr fields
>>    ASoC: Intel: Skylake: Remove redundant W0 and W1 macros
>>    ASoC: Intel: Skylake: Remove redundant SRAM fields
>>    ASoC: Intel: Expose ACPI loading members
>>    ASoC: Intel: Haswell: Define separate ACPI loader
>>    ASoC: Intel: Baytrail: Define separate ACPI loader
>>    ASoC: Intel: Refactor probing of ACPI devices
>>    ASoC: Intel: Skylake: Simplify skl_sst_ctx_init declaration
>>    ASoC: Intel: Skylake: Simplify all sst_dsp_init declarations
>>    ASoC: Intel: Skylake: Define platform descriptors
>>    ASoC: Intel: Skylake: Update skl_ids table
>>    ASoC: Intel: Skylake: Flip SST initialization order
>>    ASoC: Intel: Reuse sst_pdata::fw_name field
>>    ASoC: Intel: Reuse sst_pdata::fw field
>>    ASoC: Intel: Skylake: Remove skl_dsp_ops
>>    ASoC: Intel: Skylake: Privatize SST init handlers
>>    ASoC: Intel: Skylake: Merge skl_sst_ctx_init into skl_init_dsp
>>    ASoC: Intel: Remove obsolete firmware fields
>>
>>   sound/soc/intel/Kconfig                       |  14 +-
>>   sound/soc/intel/baytrail/Makefile             |   2 +
>>   sound/soc/intel/baytrail/acpi.c               |  64 +++++
>>   sound/soc/intel/baytrail/sst-baytrail-dsp.c   |   2 +-
>>   sound/soc/intel/baytrail/sst-baytrail-ipc.c   |  13 +-
>>   sound/soc/intel/baytrail/sst-baytrail-ipc.h   |   2 +
>>   sound/soc/intel/common/Makefile               |   4 +-
>>   .../intel/common/soc-acpi-intel-bxt-match.c   |   2 -
>>   .../intel/common/soc-acpi-intel-byt-match.c   |   2 -
>>   .../intel/common/soc-acpi-intel-cnl-match.c   |   1 -
>>   .../intel/common/soc-acpi-intel-glk-match.c   |   3 -
>>   .../intel/common/soc-acpi-intel-hda-match.c   |   2 -
>>   .../common/soc-acpi-intel-hsw-bdw-match.c     |   4 -
>>   .../intel/common/soc-acpi-intel-icl-match.c   |   1 -
>>   .../intel/common/soc-acpi-intel-kbl-match.c   |  12 -
>>   .../intel/common/soc-acpi-intel-skl-match.c   |   3 -
>>   sound/soc/intel/common/sst-acpi.c             | 117 +--------
>>   sound/soc/intel/common/sst-dsp-priv.h         |   8 +-
>>   sound/soc/intel/common/sst-dsp.h              |  37 +--
>>   sound/soc/intel/common/sst-firmware.c         |  13 +-
>>   sound/soc/intel/haswell/Makefile              |   2 +
>>   sound/soc/intel/haswell/acpi.c                |  78 ++++++
>>   sound/soc/intel/haswell/sst-haswell-dsp.c     |   1 +
>>   sound/soc/intel/haswell/sst-haswell-ipc.c     |  13 +-
>>   sound/soc/intel/haswell/sst-haswell-ipc.h     |   2 +
>>   sound/soc/intel/skylake/bxt-sst.c             | 143 ++++-------
>>   sound/soc/intel/skylake/cnl-sst-dsp.c         |  13 +-
>>   sound/soc/intel/skylake/cnl-sst-dsp.h         |  15 +-
>>   sound/soc/intel/skylake/cnl-sst.c             | 142 ++++-------
>>   sound/soc/intel/skylake/skl-debug.c           |   2 +-
>>   sound/soc/intel/skylake/skl-messages.c        | 194 ++------------
>>   sound/soc/intel/skylake/skl-pcm.c             |  22 +-
>>   sound/soc/intel/skylake/skl-sst-cldma.c       |  10 +-
>>   sound/soc/intel/skylake/skl-sst-dsp.c         |  79 ++----
>>   sound/soc/intel/skylake/skl-sst-dsp.h         |  55 ++--
>>   sound/soc/intel/skylake/skl-sst-ipc.c         | 238 ++++++++++++++++--
>>   sound/soc/intel/skylake/skl-sst-ipc.h         | 124 ++++++++-
>>   sound/soc/intel/skylake/skl-sst-utils.c       |  28 +--
>>   sound/soc/intel/skylake/skl-sst.c             | 150 ++++++-----
>>   sound/soc/intel/skylake/skl.c                 |  65 +++--
>>   sound/soc/intel/skylake/skl.h                 |  26 +-
>>   41 files changed, 877 insertions(+), 831 deletions(-)
>>   create mode 100644 sound/soc/intel/baytrail/acpi.c
>>   create mode 100644 sound/soc/intel/haswell/acpi.c
>>
Mark Brown Aug. 23, 2019, 10:26 a.m. UTC | #3
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.

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

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

> 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?
Cezary Rojewski Aug. 23, 2019, 10:43 a.m. UTC | #4
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
Pierre-Louis Bossart Aug. 23, 2019, 4:26 p.m. UTC | #5
On 8/23/19 5:43 AM, Cezary Rojewski wrote:
> 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.

The machine driver defines how many links are used, and in what mode for 
the older cases where the topology is not used. You have configurations 
with very complicated links, e.g. with amplifiers in TDM mode plus IV 
feedback that will stress the firmware in ways that regular RVPs don't. 
Same for the case where the SSP clock is turned on at the request of the 
machine drivers. That's another case that can't be tested on RVPs.

I am not saying you need to test with every single commercial device, 
but that testing on RVPs is not a representative sample of the 
configurations and actual workloads.

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

My understanding is that the SST driver is used for Skylake for 
Chromebooks only. For platforms defined for Windows the cases where the 
DSP is used are marginal. I'd view the risk of updating the firmware for 
Skylake as very limited but that's my personal opinion only.
For APL/KBL it's a lot harder to track, there are industrial/embedded 
cases and that's where we'd really want to trap any incompatibilities.

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

With the request_firmware() mechanism, the kernel cannot parse the file 
ahead of time, but don't you have a version information reported by the 
firmware post-boot that can be used by the kernel so track that the 
firmware isn't likely to work?

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

That's typically handled by distributions updating the linux-firmware 
package. Only advanced users and developers can change these symlinks.

The other point that comes to my mind is whether we are going to see 
dependencies between firmware and topology files? Can you use an 'old' 
topology with a 'newer' firmware, or is this a 3-way interoperability issue?
Cezary Rojewski Aug. 23, 2019, 6:44 p.m. UTC | #6
On 2019-08-23 18:26, Pierre-Louis Bossart wrote:
> 
> 
> On 8/23/19 5:43 AM, Cezary Rojewski wrote:
>> 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.
> 
> The machine driver defines how many links are used, and in what mode for 
> the older cases where the topology is not used. You have configurations 
> with very complicated links, e.g. with amplifiers in TDM mode plus IV 
> feedback that will stress the firmware in ways that regular RVPs don't. 
> Same for the case where the SSP clock is turned on at the request of the 
> machine drivers. That's another case that can't be tested on RVPs.
> 
> I am not saying you need to test with every single commercial device, 
> but that testing on RVPs is not a representative sample of the 
> configurations and actual workloads.
> 

Each and every FW coming from main branch gets tested on both RVP and 
production devices what is done with cooperation with integration teams, 
PAEs and such. Windows teams alone ensures each binary gets smashed by 
ten of thousands tests each week - this is true for any release 
candidate, the standards are very high. Moreover, array of platforms is 
engaged per target (e.g.: TGL) as single platform alone does not cut it.

So, I'd not worry about FW being vulnerable to any scenario as long as 
recommended protocol is followed.

>>>
>>> ...
>>>
>>>> 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?
> 
> My understanding is that the SST driver is used for Skylake for 
> Chromebooks only. For platforms defined for Windows the cases where the 
> DSP is used are marginal. I'd view the risk of updating the firmware for 
> Skylake as very limited but that's my personal opinion only.
> For APL/KBL it's a lot harder to track, there are industrial/embedded 
> cases and that's where we'd really want to trap any incompatibilities.
> 

APL/ KBL - are there any examples I should be aware of? To my knowledge 
we are handling all of them internally and they have not seen any 
obsolete binary for quite some time already. For some, FW has been 
updated even this week..

>>>
>>>> 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
> 
> With the request_firmware() mechanism, the kernel cannot parse the file 
> ahead of time, but don't you have a version information reported by the 
> firmware post-boot that can be used by the kernel so track that the 
> firmware isn't likely to work?
> 

Wasn't lying about FW version being unreliable. Let's say vendor 
receives quick FW drop with new RCR.. such eng drop may carry invalid 
numbers such as 0.0.0.0..
In general, I try to avoid relying on FW version whenever possible. It 
can be dumped for debug reasons, true, but to be relied on? Not really.

>>
>> - 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
> 
> That's typically handled by distributions updating the linux-firmware 
> package. Only advanced users and developers can change these symlinks.
> 
> The other point that comes to my mind is whether we are going to see 
> dependencies between firmware and topology files? Can you use an 'old' 
> topology with a 'newer' firmware, or is this a 3-way interoperability 
> issue?

Precisely! Three-way-tie!
It's best FW get updated together with topology as old FW may enforce 
different constraints on pipeline modules.

Yay, between rock and hard place. On one side we got old buggy FWs which 
should (more like should NOT be even here..) be updated to improve 
user's experience but updating these alone won't cut it as host side 
needs to be aligned too.
On the other we want to align upstream /skylake with actual working 
example, which will quickly fail if it encounters obsolete FW binary.
And if that wasn't enough, lovely topologies come into picture where 
some of these were developed behind FDK's back and thus completely 
bypassing deployment process.

First thing we will do now is prioritizing topology refactor so all 
initialization/ load oriented thingies will be visible for upstream 
review. By doing so, we got all elephants in one room and can discuss 
how to handle it in best fashion: seamless transition for end-users.

There aren't many options available: notify user -or- fallback to 
defaults (hardcodes)? in case encountered binaries do not meet cAVS 
design criteria.

Personally, I'm against all hardcodes and would simply recommend all 
user to redirect their symlinks when they do switch kernel - along with 
dumping warning/ error message in dmesg. Hardcodes bring problems with 
forward compatibility and that's why host should offload them away to FW.

Czarek
Pierre-Louis Bossart Aug. 23, 2019, 8:12 p.m. UTC | #7
On 8/23/19 1:44 PM, Cezary Rojewski wrote:
> On 2019-08-23 18:26, Pierre-Louis Bossart wrote:
>>
>>
>> On 8/23/19 5:43 AM, Cezary Rojewski wrote:
>>> 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.
>>
>> The machine driver defines how many links are used, and in what mode 
>> for the older cases where the topology is not used. You have 
>> configurations with very complicated links, e.g. with amplifiers in 
>> TDM mode plus IV feedback that will stress the firmware in ways that 
>> regular RVPs don't. Same for the case where the SSP clock is turned on 
>> at the request of the machine drivers. That's another case that can't 
>> be tested on RVPs.
>>
>> I am not saying you need to test with every single commercial device, 
>> but that testing on RVPs is not a representative sample of the 
>> configurations and actual workloads.
>>
> 
> Each and every FW coming from main branch gets tested on both RVP and 
> production devices what is done with cooperation with integration teams, 
> PAEs and such. Windows teams alone ensures each binary gets smashed by 
> ten of thousands tests each week - this is true for any release 
> candidate, the standards are very high. Moreover, array of platforms is 
> engaged per target (e.g.: TGL) as single platform alone does not cut it.
> 
> So, I'd not worry about FW being vulnerable to any scenario as long as 
> recommended protocol is followed.

I didn't mean to diss the validation work, but the Chromebook cases and 
amplifiers over TDM with IV feedback are certainly not configurations 
tested by Windows folks who are using HDaudio+DMIC only.

>> With the request_firmware() mechanism, the kernel cannot parse the 
>> file ahead of time, but don't you have a version information reported 
>> by the firmware post-boot that can be used by the kernel so track that 
>> the firmware isn't likely to work?
>>
> 
> Wasn't lying about FW version being unreliable. Let's say vendor 
> receives quick FW drop with new RCR.. such eng drop may carry invalid 
> numbers such as 0.0.0.0..
> In general, I try to avoid relying on FW version whenever possible. It 
> can be dumped for debug reasons, true, but to be relied on? Not really.

Goodness, that's really bad. I didn't realize this.

> 
>>>
>>> - 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
>>
>> That's typically handled by distributions updating the linux-firmware 
>> package. Only advanced users and developers can change these symlinks.
>>
>> The other point that comes to my mind is whether we are going to see 
>> dependencies between firmware and topology files? Can you use an 'old' 
>> topology with a 'newer' firmware, or is this a 3-way interoperability 
>> issue?
> 
> Precisely! Three-way-tie!
> It's best FW get updated together with topology as old FW may enforce 
> different constraints on pipeline modules.
> 
> Yay, between rock and hard place. On one side we got old buggy FWs which 
> should (more like should NOT be even here..) be updated to improve 
> user's experience but updating these alone won't cut it as host side 
> needs to be aligned too.
> On the other we want to align upstream /skylake with actual working 
> example, which will quickly fail if it encounters obsolete FW binary.
> And if that wasn't enough, lovely topologies come into picture where 
> some of these were developed behind FDK's back and thus completely 
> bypassing deployment process.
> 
> First thing we will do now is prioritizing topology refactor so all 
> initialization/ load oriented thingies will be visible for upstream 
> review. By doing so, we got all elephants in one room and can discuss 
> how to handle it in best fashion: seamless transition for end-users.
> 
> There aren't many options available: notify user -or- fallback to 
> defaults (hardcodes)? in case encountered binaries do not meet cAVS 
> design criteria.
> 
> Personally, I'm against all hardcodes and would simply recommend all 
> user to redirect their symlinks when they do switch kernel - along with 
> dumping warning/ error message in dmesg. Hardcodes bring problems with 
> forward compatibility and that's why host should offload them away to FW.

Cezary, I know you are not responsible for all this, but at this point 
if we (Intel) can't guarantee any sort of interoperability with both 
firmware and topology we should make it clear that this driver is not 
recommended unless specific versions of the firmware/topology are used, 
and as a consequence the typical client distros and desktop/laptop users 
should use HDaudio legacy or SOF (for DMICs)
Mark Brown Aug. 23, 2019, 9:39 p.m. UTC | #8
On Fri, Aug 23, 2019 at 03:12:18PM -0500, Pierre-Louis Bossart wrote:
> On 8/23/19 1:44 PM, Cezary Rojewski wrote:

> > Wasn't lying about FW version being unreliable. Let's say vendor
> > receives quick FW drop with new RCR.. such eng drop may carry invalid
> > numbers such as 0.0.0.0..
> > In general, I try to avoid relying on FW version whenever possible. It
> > can be dumped for debug reasons, true, but to be relied on? Not really.

> Goodness, that's really bad. I didn't realize this.

At a previous employer I modified our build stamping
infrastructure to also include both a timestamp and a serialized
build number in the version number since one of my colleagues was
fond of sending people prereleases of what he was working on to
other people with identical version numbers on different
binaries leading to much confusion and checksumming.  You do see
a lot of things with those serialized version numbers, especially
SVN based projects.

> > Personally, I'm against all hardcodes and would simply recommend all
> > user to redirect their symlinks when they do switch kernel - along with
> > dumping warning/ error message in dmesg. Hardcodes bring problems with
> > forward compatibility and that's why host should offload them away to
> > FW.

> Cezary, I know you are not responsible for all this, but at this point if we
> (Intel) can't guarantee any sort of interoperability with both firmware and
> topology we should make it clear that this driver is not recommended unless
> specific versions of the firmware/topology are used, and as a consequence
> the typical client distros and desktop/laptop users should use HDaudio
> legacy or SOF (for DMICs)

Not the most elegent solution but I'm wondering if keeping a copy
of the driver as is around and using new locations for the fixed
firmware might be the safest way to handle this.  We could have a
wrapper which tries to load the newer firmware and uses the fixed
driver code if that's there, otherwise tries the old driver with
the existing firmware paths.  This is obviously a horror show and
leaves the old code sitting there but given the mistakes that
have been made the whole situation looks like a house of cards.
Cezary Rojewski Aug. 24, 2019, 1:51 p.m. UTC | #9
On 2019-08-23 23:39, Mark Brown wrote:
> On Fri, Aug 23, 2019 at 03:12:18PM -0500, Pierre-Louis Bossart wrote:
>> On 8/23/19 1:44 PM, Cezary Rojewski wrote:
> 
>>> Wasn't lying about FW version being unreliable. Let's say vendor
>>> receives quick FW drop with new RCR.. such eng drop may carry invalid
>>> numbers such as 0.0.0.0..
>>> In general, I try to avoid relying on FW version whenever possible. It
>>> can be dumped for debug reasons, true, but to be relied on? Not really.
> 
>> Goodness, that's really bad. I didn't realize this.
> 
> At a previous employer I modified our build stamping
> infrastructure to also include both a timestamp and a serialized
> build number in the version number since one of my colleagues was
> fond of sending people prereleases of what he was working on to
> other people with identical version numbers on different
> binaries leading to much confusion and checksumming.  You do see
> a lot of things with those serialized version numbers, especially
> SVN based projects.
> 
>>> Personally, I'm against all hardcodes and would simply recommend all
>>> user to redirect their symlinks when they do switch kernel - along with
>>> dumping warning/ error message in dmesg. Hardcodes bring problems with
>>> forward compatibility and that's why host should offload them away to
>>> FW.
> 
>> Cezary, I know you are not responsible for all this, but at this point if we
>> (Intel) can't guarantee any sort of interoperability with both firmware and
>> topology we should make it clear that this driver is not recommended unless
>> specific versions of the firmware/topology are used, and as a consequence
>> the typical client distros and desktop/laptop users should use HDaudio
>> legacy or SOF (for DMICs)
> 
> Not the most elegent solution but I'm wondering if keeping a copy
> of the driver as is around and using new locations for the fixed
> firmware might be the safest way to handle this.  We could have a
> wrapper which tries to load the newer firmware and uses the fixed
> driver code if that's there, otherwise tries the old driver with
> the existing firmware paths.  This is obviously a horror show and
> leaves the old code sitting there but given the mistakes that
> have been made the whole situation looks like a house of cards.
> 

Thanks for the feedback Mark. While I'm not yet on the "SOF will fix 
this" train, I'm keen to agree to leaving this entirely to SOF if it 
comes down to us duplicating /skylake.

However, we are not going to give up that easily. I'll see if some 
"golden config" hardcodes can't be provided in some legacy.c file which 
would be fetched if initial setup fails. E.g.: 2cores, 3ssps, 1PAGE_SIZE 
per trace buffer.. and such. There are quite a few factors to take into 
consideration though. If "asking" user via dmesg to upgrade the firmware 
if his/her setup contains obsolete binary is really not an option, then 
some magic words got to be involved.

Czarek
Cezary Rojewski Aug. 25, 2019, 11:06 a.m. UTC | #10
On 2019-08-24 15:51, Cezary Rojewski wrote:
> On 2019-08-23 23:39, Mark Brown wrote:
>> On Fri, Aug 23, 2019 at 03:12:18PM -0500, Pierre-Louis Bossart wrote:
>>> On 8/23/19 1:44 PM, Cezary Rojewski wrote:
>>
>>>> Wasn't lying about FW version being unreliable. Let's say vendor
>>>> receives quick FW drop with new RCR.. such eng drop may carry invalid
>>>> numbers such as 0.0.0.0..
>>>> In general, I try to avoid relying on FW version whenever possible. It
>>>> can be dumped for debug reasons, true, but to be relied on? Not really.
>>
>>> Goodness, that's really bad. I didn't realize this.
>>
>> At a previous employer I modified our build stamping
>> infrastructure to also include both a timestamp and a serialized
>> build number in the version number since one of my colleagues was
>> fond of sending people prereleases of what he was working on to
>> other people with identical version numbers on different
>> binaries leading to much confusion and checksumming.  You do see
>> a lot of things with those serialized version numbers, especially
>> SVN based projects.
>>
>>>> Personally, I'm against all hardcodes and would simply recommend all
>>>> user to redirect their symlinks when they do switch kernel - along with
>>>> dumping warning/ error message in dmesg. Hardcodes bring problems with
>>>> forward compatibility and that's why host should offload them away to
>>>> FW.
>>
>>> Cezary, I know you are not responsible for all this, but at this 
>>> point if we
>>> (Intel) can't guarantee any sort of interoperability with both 
>>> firmware and
>>> topology we should make it clear that this driver is not recommended 
>>> unless
>>> specific versions of the firmware/topology are used, and as a 
>>> consequence
>>> the typical client distros and desktop/laptop users should use HDaudio
>>> legacy or SOF (for DMICs)
>>
>> Not the most elegent solution but I'm wondering if keeping a copy
>> of the driver as is around and using new locations for the fixed
>> firmware might be the safest way to handle this.  We could have a
>> wrapper which tries to load the newer firmware and uses the fixed
>> driver code if that's there, otherwise tries the old driver with
>> the existing firmware paths.  This is obviously a horror show and
>> leaves the old code sitting there but given the mistakes that
>> have been made the whole situation looks like a house of cards.
>>
> 
> Thanks for the feedback Mark. While I'm not yet on the "SOF will fix 
> this" train, I'm keen to agree to leaving this entirely to SOF if it 
> comes down to us duplicating /skylake.
> 
> However, we are not going to give up that easily. I'll see if some 
> "golden config" hardcodes can't be provided in some legacy.c file which 
> would be fetched if initial setup fails. E.g.: 2cores, 3ssps, 1PAGE_SIZE 
> per trace buffer.. and such. There are quite a few factors to take into 
> consideration though. If "asking" user via dmesg to upgrade the firmware 
> if his/her setup contains obsolete binary is really not an option, then 
> some magic words got to be involved.
> 
> Czarek

On the second thought what if instead of duplicating kernel code, 
binaries would be duplicated?
I.e. rather than targeting /intel/dsp_fw_cnl.bin, _new_ /skylake would 
be expecting /intel/dsp_fw_cnl_release.bin? Same with topology binaries.
In such case, we "only" need to figure out how to propagate new files to 
Linux distos so whenever someone updates their kernel, new binaries are 
already present in their /lib/firmware.

If such option is valid, we can postpone /skylake upgrade till 5.4 
merging window closes and the patches (rough estimation is 150) would 
descend upon alsa-devel in time between 5.4 and 5.5.
Wasko, Michal Aug. 26, 2019, 7:24 a.m. UTC | #11
On 8/25/2019 1:06 PM, Cezary Rojewski wrote:
> On 2019-08-24 15:51, Cezary Rojewski wrote:
>> On 2019-08-23 23:39, Mark Brown wrote:
>>> On Fri, Aug 23, 2019 at 03:12:18PM -0500, Pierre-Louis Bossart wrote:
>>>> On 8/23/19 1:44 PM, Cezary Rojewski wrote:
>>>
>>>>> Wasn't lying about FW version being unreliable. Let's say vendor
>>>>> receives quick FW drop with new RCR.. such eng drop may carry invalid
>>>>> numbers such as 0.0.0.0..
>>>>> In general, I try to avoid relying on FW version whenever 
>>>>> possible. It
>>>>> can be dumped for debug reasons, true, but to be relied on? Not 
>>>>> really.
>>>
>>>> Goodness, that's really bad. I didn't realize this.
>>>
>>> At a previous employer I modified our build stamping
>>> infrastructure to also include both a timestamp and a serialized
>>> build number in the version number since one of my colleagues was
>>> fond of sending people prereleases of what he was working on to
>>> other people with identical version numbers on different
>>> binaries leading to much confusion and checksumming.  You do see
>>> a lot of things with those serialized version numbers, especially
>>> SVN based projects.
>>>
>>>>> Personally, I'm against all hardcodes and would simply recommend all
>>>>> user to redirect their symlinks when they do switch kernel - along 
>>>>> with
>>>>> dumping warning/ error message in dmesg. Hardcodes bring problems 
>>>>> with
>>>>> forward compatibility and that's why host should offload them away to
>>>>> FW.
>>>
>>>> Cezary, I know you are not responsible for all this, but at this 
>>>> point if we
>>>> (Intel) can't guarantee any sort of interoperability with both 
>>>> firmware and
>>>> topology we should make it clear that this driver is not 
>>>> recommended unless
>>>> specific versions of the firmware/topology are used, and as a 
>>>> consequence
>>>> the typical client distros and desktop/laptop users should use HDaudio
>>>> legacy or SOF (for DMICs)
>>>
>>> Not the most elegent solution but I'm wondering if keeping a copy
>>> of the driver as is around and using new locations for the fixed
>>> firmware might be the safest way to handle this.  We could have a
>>> wrapper which tries to load the newer firmware and uses the fixed
>>> driver code if that's there, otherwise tries the old driver with
>>> the existing firmware paths.  This is obviously a horror show and
>>> leaves the old code sitting there but given the mistakes that
>>> have been made the whole situation looks like a house of cards.
>>>
>>
>> Thanks for the feedback Mark. While I'm not yet on the "SOF will fix 
>> this" train, I'm keen to agree to leaving this entirely to SOF if it 
>> comes down to us duplicating /skylake.
>>
>> However, we are not going to give up that easily. I'll see if some 
>> "golden config" hardcodes can't be provided in some legacy.c file 
>> which would be fetched if initial setup fails. E.g.: 2cores, 3ssps, 
>> 1PAGE_SIZE per trace buffer.. and such. There are quite a few factors 
>> to take into consideration though. If "asking" user via dmesg to 
>> upgrade the firmware if his/her setup contains obsolete binary is 
>> really not an option, then some magic words got to be involved.
>>
>> Czarek
>
> On the second thought what if instead of duplicating kernel code, 
> binaries would be duplicated?
> I.e. rather than targeting /intel/dsp_fw_cnl.bin, _new_ /skylake would 
> be expecting /intel/dsp_fw_cnl_release.bin? Same with topology binaries.
> In such case, we "only" need to figure out how to propagate new files 
> to Linux distos so whenever someone updates their kernel, new binaries 
> are already present in their /lib/firmware.
>
> If such option is valid, we can postpone /skylake upgrade till 5.4 
> merging window closes and the patches (rough estimation is 150) would 
> descend upon alsa-devel in time between 5.4 and 5.5.

If the driver and FW update will be within the same kernel release then IMHO
there should be no compatibility problem between those two components, 
right?
This way kernel users willing to stick to old FW can stay on older 
kernel version while
others can update and receive all the latest FW functionality that was 
developed and enabled.

In terms of FW topology compatibility there is an option to read from 
topology manifest
a FW version that it was build for and in  case if it does not match FW 
version present on
the platform then print warning that the FW topology binary should be 
rebuild for current
FW version (x.x.x.x).

The above approach at the end may be less confusing then source code or 
binary duplication.
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Pierre-Louis Bossart Aug. 26, 2019, 4:51 p.m. UTC | #12
On 8/26/19 2:24 AM, Wasko, Michal wrote:
> 
> On 8/25/2019 1:06 PM, Cezary Rojewski wrote:
>> On 2019-08-24 15:51, Cezary Rojewski wrote:
>>> On 2019-08-23 23:39, Mark Brown wrote:
>>>> On Fri, Aug 23, 2019 at 03:12:18PM -0500, Pierre-Louis Bossart wrote:
>>>>> On 8/23/19 1:44 PM, Cezary Rojewski wrote:
>>>>
>>>>>> Wasn't lying about FW version being unreliable. Let's say vendor
>>>>>> receives quick FW drop with new RCR.. such eng drop may carry invalid
>>>>>> numbers such as 0.0.0.0..
>>>>>> In general, I try to avoid relying on FW version whenever 
>>>>>> possible. It
>>>>>> can be dumped for debug reasons, true, but to be relied on? Not 
>>>>>> really.
>>>>
>>>>> Goodness, that's really bad. I didn't realize this.
>>>>
>>>> At a previous employer I modified our build stamping
>>>> infrastructure to also include both a timestamp and a serialized
>>>> build number in the version number since one of my colleagues was
>>>> fond of sending people prereleases of what he was working on to
>>>> other people with identical version numbers on different
>>>> binaries leading to much confusion and checksumming.  You do see
>>>> a lot of things with those serialized version numbers, especially
>>>> SVN based projects.
>>>>
>>>>>> Personally, I'm against all hardcodes and would simply recommend all
>>>>>> user to redirect their symlinks when they do switch kernel - along 
>>>>>> with
>>>>>> dumping warning/ error message in dmesg. Hardcodes bring problems 
>>>>>> with
>>>>>> forward compatibility and that's why host should offload them away to
>>>>>> FW.
>>>>
>>>>> Cezary, I know you are not responsible for all this, but at this 
>>>>> point if we
>>>>> (Intel) can't guarantee any sort of interoperability with both 
>>>>> firmware and
>>>>> topology we should make it clear that this driver is not 
>>>>> recommended unless
>>>>> specific versions of the firmware/topology are used, and as a 
>>>>> consequence
>>>>> the typical client distros and desktop/laptop users should use HDaudio
>>>>> legacy or SOF (for DMICs)
>>>>
>>>> Not the most elegent solution but I'm wondering if keeping a copy
>>>> of the driver as is around and using new locations for the fixed
>>>> firmware might be the safest way to handle this.  We could have a
>>>> wrapper which tries to load the newer firmware and uses the fixed
>>>> driver code if that's there, otherwise tries the old driver with
>>>> the existing firmware paths.  This is obviously a horror show and
>>>> leaves the old code sitting there but given the mistakes that
>>>> have been made the whole situation looks like a house of cards.
>>>>
>>>
>>> Thanks for the feedback Mark. While I'm not yet on the "SOF will fix 
>>> this" train, I'm keen to agree to leaving this entirely to SOF if it 
>>> comes down to us duplicating /skylake.
>>>
>>> However, we are not going to give up that easily. I'll see if some 
>>> "golden config" hardcodes can't be provided in some legacy.c file 
>>> which would be fetched if initial setup fails. E.g.: 2cores, 3ssps, 
>>> 1PAGE_SIZE per trace buffer.. and such. There are quite a few factors 
>>> to take into consideration though. If "asking" user via dmesg to 
>>> upgrade the firmware if his/her setup contains obsolete binary is 
>>> really not an option, then some magic words got to be involved.
>>>
>>> Czarek
>>
>> On the second thought what if instead of duplicating kernel code, 
>> binaries would be duplicated?
>> I.e. rather than targeting /intel/dsp_fw_cnl.bin, _new_ /skylake would 
>> be expecting /intel/dsp_fw_cnl_release.bin? Same with topology binaries.
>> In such case, we "only" need to figure out how to propagate new files 
>> to Linux distos so whenever someone updates their kernel, new binaries 
>> are already present in their /lib/firmware.
>>
>> If such option is valid, we can postpone /skylake upgrade till 5.4 
>> merging window closes and the patches (rough estimation is 150) would 
>> descend upon alsa-devel in time between 5.4 and 5.5.
> 
> If the driver and FW update will be within the same kernel release then 
> IMHO
> there should be no compatibility problem between those two components, 
> right?
> This way kernel users willing to stick to old FW can stay on older 
> kernel version while
> others can update and receive all the latest FW functionality that was 
> developed and enabled.

I am not comfortable with precluding a kernel update because of a single 
firmware file. There are all sort of reasons for updating a kernel, 
security, sideband attacks and Android CDD compatibility being the most 
obvious ones.

> 
> In terms of FW topology compatibility there is an option to read from 
> topology manifest
> a FW version that it was build for and in  case if it does not match FW 
> version present on
> the platform then print warning that the FW topology binary should be 
> rebuild for current
> FW version (x.x.x.x).

Can you provide a pointer on how the FW version is embedded in a 
.conf/.tplg file? I see a couple where that information does not seem 
present.

> The above approach at the end may be less confusing then source code or 
> binary duplication.
Cezary Rojewski Aug. 26, 2019, 8:08 p.m. UTC | #13
On 2019-08-26 18:51, Pierre-Louis Bossart wrote:
> 
> 
> On 8/26/19 2:24 AM, Wasko, Michal wrote:
>>
>> On 8/25/2019 1:06 PM, Cezary Rojewski wrote:
>>> On 2019-08-24 15:51, Cezary Rojewski wrote:
>>>> On 2019-08-23 23:39, Mark Brown wrote:
>>>>> On Fri, Aug 23, 2019 at 03:12:18PM -0500, Pierre-Louis Bossart wrote:
>>>>>> On 8/23/19 1:44 PM, Cezary Rojewski wrote:
>>>>>
>>>>>>> Wasn't lying about FW version being unreliable. Let's say vendor
>>>>>>> receives quick FW drop with new RCR.. such eng drop may carry 
>>>>>>> invalid
>>>>>>> numbers such as 0.0.0.0..
>>>>>>> In general, I try to avoid relying on FW version whenever 
>>>>>>> possible. It
>>>>>>> can be dumped for debug reasons, true, but to be relied on? Not 
>>>>>>> really.
>>>>>
>>>>>> Goodness, that's really bad. I didn't realize this.
>>>>>
>>>>> At a previous employer I modified our build stamping
>>>>> infrastructure to also include both a timestamp and a serialized
>>>>> build number in the version number since one of my colleagues was
>>>>> fond of sending people prereleases of what he was working on to
>>>>> other people with identical version numbers on different
>>>>> binaries leading to much confusion and checksumming.  You do see
>>>>> a lot of things with those serialized version numbers, especially
>>>>> SVN based projects.
>>>>>
>>>>>>> Personally, I'm against all hardcodes and would simply recommend all
>>>>>>> user to redirect their symlinks when they do switch kernel - 
>>>>>>> along with
>>>>>>> dumping warning/ error message in dmesg. Hardcodes bring problems 
>>>>>>> with
>>>>>>> forward compatibility and that's why host should offload them 
>>>>>>> away to
>>>>>>> FW.
>>>>>
>>>>>> Cezary, I know you are not responsible for all this, but at this 
>>>>>> point if we
>>>>>> (Intel) can't guarantee any sort of interoperability with both 
>>>>>> firmware and
>>>>>> topology we should make it clear that this driver is not 
>>>>>> recommended unless
>>>>>> specific versions of the firmware/topology are used, and as a 
>>>>>> consequence
>>>>>> the typical client distros and desktop/laptop users should use 
>>>>>> HDaudio
>>>>>> legacy or SOF (for DMICs)
>>>>>
>>>>> Not the most elegent solution but I'm wondering if keeping a copy
>>>>> of the driver as is around and using new locations for the fixed
>>>>> firmware might be the safest way to handle this.  We could have a
>>>>> wrapper which tries to load the newer firmware and uses the fixed
>>>>> driver code if that's there, otherwise tries the old driver with
>>>>> the existing firmware paths.  This is obviously a horror show and
>>>>> leaves the old code sitting there but given the mistakes that
>>>>> have been made the whole situation looks like a house of cards.
>>>>>
>>>>
>>>> Thanks for the feedback Mark. While I'm not yet on the "SOF will fix 
>>>> this" train, I'm keen to agree to leaving this entirely to SOF if it 
>>>> comes down to us duplicating /skylake.
>>>>
>>>> However, we are not going to give up that easily. I'll see if some 
>>>> "golden config" hardcodes can't be provided in some legacy.c file 
>>>> which would be fetched if initial setup fails. E.g.: 2cores, 3ssps, 
>>>> 1PAGE_SIZE per trace buffer.. and such. There are quite a few 
>>>> factors to take into consideration though. If "asking" user via 
>>>> dmesg to upgrade the firmware if his/her setup contains obsolete 
>>>> binary is really not an option, then some magic words got to be 
>>>> involved.
>>>>
>>>> Czarek
>>>
>>> On the second thought what if instead of duplicating kernel code, 
>>> binaries would be duplicated?
>>> I.e. rather than targeting /intel/dsp_fw_cnl.bin, _new_ /skylake 
>>> would be expecting /intel/dsp_fw_cnl_release.bin? Same with topology 
>>> binaries.
>>> In such case, we "only" need to figure out how to propagate new files 
>>> to Linux distos so whenever someone updates their kernel, new 
>>> binaries are already present in their /lib/firmware.
>>>
>>> If such option is valid, we can postpone /skylake upgrade till 5.4 
>>> merging window closes and the patches (rough estimation is 150) would 
>>> descend upon alsa-devel in time between 5.4 and 5.5.
>>
>> If the driver and FW update will be within the same kernel release 
>> then IMHO
>> there should be no compatibility problem between those two components, 
>> right?
>> This way kernel users willing to stick to old FW can stay on older 
>> kernel version while
>> others can update and receive all the latest FW functionality that was 
>> developed and enabled.
> 
> I am not comfortable with precluding a kernel update because of a single 
> firmware file. There are all sort of reasons for updating a kernel, 
> security, sideband attacks and Android CDD compatibility being the most 
> obvious ones.
> 
>>
>> In terms of FW topology compatibility there is an option to read from 
>> topology manifest
>> a FW version that it was build for and in  case if it does not match 
>> FW version present on
>> the platform then print warning that the FW topology binary should be 
>> rebuild for current
>> FW version (x.x.x.x).
> 
> Can you provide a pointer on how the FW version is embedded in a 
> .conf/.tplg file? I see a couple where that information does not seem 
> present.
> 
>> The above approach at the end may be less confusing then source code 
>> or binary duplication.
> 

Indeed. Our existing topology skips that part of internal .xml and thus 
such information is not propagated to kernel.

Pierre, how about the binary-duplication - as described above? Btw, 
that's not a single firmware file ^)^ We would immediately update all of 
them, together with topologies.
Pierre-Louis Bossart Aug. 26, 2019, 9:57 p.m. UTC | #14
On 8/26/19 3:08 PM, Cezary Rojewski wrote:
> On 2019-08-26 18:51, Pierre-Louis Bossart wrote:
>>
>>
>> On 8/26/19 2:24 AM, Wasko, Michal wrote:
>>>
>>> On 8/25/2019 1:06 PM, Cezary Rojewski wrote:
>>>> On 2019-08-24 15:51, Cezary Rojewski wrote:
>>>>> On 2019-08-23 23:39, Mark Brown wrote:
>>>>>> On Fri, Aug 23, 2019 at 03:12:18PM -0500, Pierre-Louis Bossart wrote:
>>>>>>> On 8/23/19 1:44 PM, Cezary Rojewski wrote:
>>>>>>
>>>>>>>> Wasn't lying about FW version being unreliable. Let's say vendor
>>>>>>>> receives quick FW drop with new RCR.. such eng drop may carry 
>>>>>>>> invalid
>>>>>>>> numbers such as 0.0.0.0..
>>>>>>>> In general, I try to avoid relying on FW version whenever 
>>>>>>>> possible. It
>>>>>>>> can be dumped for debug reasons, true, but to be relied on? Not 
>>>>>>>> really.
>>>>>>
>>>>>>> Goodness, that's really bad. I didn't realize this.
>>>>>>
>>>>>> At a previous employer I modified our build stamping
>>>>>> infrastructure to also include both a timestamp and a serialized
>>>>>> build number in the version number since one of my colleagues was
>>>>>> fond of sending people prereleases of what he was working on to
>>>>>> other people with identical version numbers on different
>>>>>> binaries leading to much confusion and checksumming.  You do see
>>>>>> a lot of things with those serialized version numbers, especially
>>>>>> SVN based projects.
>>>>>>
>>>>>>>> Personally, I'm against all hardcodes and would simply recommend 
>>>>>>>> all
>>>>>>>> user to redirect their symlinks when they do switch kernel - 
>>>>>>>> along with
>>>>>>>> dumping warning/ error message in dmesg. Hardcodes bring 
>>>>>>>> problems with
>>>>>>>> forward compatibility and that's why host should offload them 
>>>>>>>> away to
>>>>>>>> FW.
>>>>>>
>>>>>>> Cezary, I know you are not responsible for all this, but at this 
>>>>>>> point if we
>>>>>>> (Intel) can't guarantee any sort of interoperability with both 
>>>>>>> firmware and
>>>>>>> topology we should make it clear that this driver is not 
>>>>>>> recommended unless
>>>>>>> specific versions of the firmware/topology are used, and as a 
>>>>>>> consequence
>>>>>>> the typical client distros and desktop/laptop users should use 
>>>>>>> HDaudio
>>>>>>> legacy or SOF (for DMICs)
>>>>>>
>>>>>> Not the most elegent solution but I'm wondering if keeping a copy
>>>>>> of the driver as is around and using new locations for the fixed
>>>>>> firmware might be the safest way to handle this.  We could have a
>>>>>> wrapper which tries to load the newer firmware and uses the fixed
>>>>>> driver code if that's there, otherwise tries the old driver with
>>>>>> the existing firmware paths.  This is obviously a horror show and
>>>>>> leaves the old code sitting there but given the mistakes that
>>>>>> have been made the whole situation looks like a house of cards.
>>>>>>
>>>>>
>>>>> Thanks for the feedback Mark. While I'm not yet on the "SOF will 
>>>>> fix this" train, I'm keen to agree to leaving this entirely to SOF 
>>>>> if it comes down to us duplicating /skylake.
>>>>>
>>>>> However, we are not going to give up that easily. I'll see if some 
>>>>> "golden config" hardcodes can't be provided in some legacy.c file 
>>>>> which would be fetched if initial setup fails. E.g.: 2cores, 3ssps, 
>>>>> 1PAGE_SIZE per trace buffer.. and such. There are quite a few 
>>>>> factors to take into consideration though. If "asking" user via 
>>>>> dmesg to upgrade the firmware if his/her setup contains obsolete 
>>>>> binary is really not an option, then some magic words got to be 
>>>>> involved.
>>>>>
>>>>> Czarek
>>>>
>>>> On the second thought what if instead of duplicating kernel code, 
>>>> binaries would be duplicated?
>>>> I.e. rather than targeting /intel/dsp_fw_cnl.bin, _new_ /skylake 
>>>> would be expecting /intel/dsp_fw_cnl_release.bin? Same with topology 
>>>> binaries.
>>>> In such case, we "only" need to figure out how to propagate new 
>>>> files to Linux distos so whenever someone updates their kernel, new 
>>>> binaries are already present in their /lib/firmware.
>>>>
>>>> If such option is valid, we can postpone /skylake upgrade till 5.4 
>>>> merging window closes and the patches (rough estimation is 150) 
>>>> would descend upon alsa-devel in time between 5.4 and 5.5.
>>>
>>> If the driver and FW update will be within the same kernel release 
>>> then IMHO
>>> there should be no compatibility problem between those two 
>>> components, right?
>>> This way kernel users willing to stick to old FW can stay on older 
>>> kernel version while
>>> others can update and receive all the latest FW functionality that 
>>> was developed and enabled.
>>
>> I am not comfortable with precluding a kernel update because of a 
>> single firmware file. There are all sort of reasons for updating a 
>> kernel, security, sideband attacks and Android CDD compatibility being 
>> the most obvious ones.
>>
>>>
>>> In terms of FW topology compatibility there is an option to read from 
>>> topology manifest
>>> a FW version that it was build for and in  case if it does not match 
>>> FW version present on
>>> the platform then print warning that the FW topology binary should be 
>>> rebuild for current
>>> FW version (x.x.x.x).
>>
>> Can you provide a pointer on how the FW version is embedded in a 
>> .conf/.tplg file? I see a couple where that information does not seem 
>> present.
>>
>>> The above approach at the end may be less confusing then source code 
>>> or binary duplication.
>>
> 
> Indeed. Our existing topology skips that part of internal .xml and thus 
> such information is not propagated to kernel.
> 
> Pierre, how about the binary-duplication - as described above? Btw, 
> that's not a single firmware file ^)^ We would immediately update all of 
> them, together with topologies.

I didn't understand how you would select the new firmwares? Some code 
change needs to happen as well?
Wasko, Michal Aug. 27, 2019, 8:33 a.m. UTC | #15
On 8/26/2019 11:57 PM, Pierre-Louis Bossart wrote:

>
>
> On 8/26/19 3:08 PM, Cezary Rojewski wrote:
>> On 2019-08-26 18:51, Pierre-Louis Bossart wrote:
>>>
>>>
>>> On 8/26/19 2:24 AM, Wasko, Michal wrote:
>>>>
>>>> On 8/25/2019 1:06 PM, Cezary Rojewski wrote:
>>>>> On 2019-08-24 15:51, Cezary Rojewski wrote:
>>>>>> On 2019-08-23 23:39, Mark Brown wrote:
>>>>>>> On Fri, Aug 23, 2019 at 03:12:18PM -0500, Pierre-Louis Bossart 
>>>>>>> wrote:
>>>>>>>> On 8/23/19 1:44 PM, Cezary Rojewski wrote:
>>>>>>>
>>>>>>>>> Wasn't lying about FW version being unreliable. Let's say vendor
>>>>>>>>> receives quick FW drop with new RCR.. such eng drop may carry 
>>>>>>>>> invalid
>>>>>>>>> numbers such as 0.0.0.0..
>>>>>>>>> In general, I try to avoid relying on FW version whenever 
>>>>>>>>> possible. It
>>>>>>>>> can be dumped for debug reasons, true, but to be relied on? 
>>>>>>>>> Not really.
>>>>>>>
>>>>>>>> Goodness, that's really bad. I didn't realize this.
>>>>>>>
>>>>>>> At a previous employer I modified our build stamping
>>>>>>> infrastructure to also include both a timestamp and a serialized
>>>>>>> build number in the version number since one of my colleagues was
>>>>>>> fond of sending people prereleases of what he was working on to
>>>>>>> other people with identical version numbers on different
>>>>>>> binaries leading to much confusion and checksumming. You do see
>>>>>>> a lot of things with those serialized version numbers, especially
>>>>>>> SVN based projects.
>>>>>>>
>>>>>>>>> Personally, I'm against all hardcodes and would simply 
>>>>>>>>> recommend all
>>>>>>>>> user to redirect their symlinks when they do switch kernel - 
>>>>>>>>> along with
>>>>>>>>> dumping warning/ error message in dmesg. Hardcodes bring 
>>>>>>>>> problems with
>>>>>>>>> forward compatibility and that's why host should offload them 
>>>>>>>>> away to
>>>>>>>>> FW.
>>>>>>>
>>>>>>>> Cezary, I know you are not responsible for all this, but at 
>>>>>>>> this point if we
>>>>>>>> (Intel) can't guarantee any sort of interoperability with both 
>>>>>>>> firmware and
>>>>>>>> topology we should make it clear that this driver is not 
>>>>>>>> recommended unless
>>>>>>>> specific versions of the firmware/topology are used, and as a 
>>>>>>>> consequence
>>>>>>>> the typical client distros and desktop/laptop users should use 
>>>>>>>> HDaudio
>>>>>>>> legacy or SOF (for DMICs)
>>>>>>>
>>>>>>> Not the most elegent solution but I'm wondering if keeping a copy
>>>>>>> of the driver as is around and using new locations for the fixed
>>>>>>> firmware might be the safest way to handle this.  We could have a
>>>>>>> wrapper which tries to load the newer firmware and uses the fixed
>>>>>>> driver code if that's there, otherwise tries the old driver with
>>>>>>> the existing firmware paths.  This is obviously a horror show and
>>>>>>> leaves the old code sitting there but given the mistakes that
>>>>>>> have been made the whole situation looks like a house of cards.
>>>>>>>
>>>>>>
>>>>>> Thanks for the feedback Mark. While I'm not yet on the "SOF will 
>>>>>> fix this" train, I'm keen to agree to leaving this entirely to 
>>>>>> SOF if it comes down to us duplicating /skylake.
>>>>>>
>>>>>> However, we are not going to give up that easily. I'll see if 
>>>>>> some "golden config" hardcodes can't be provided in some legacy.c 
>>>>>> file which would be fetched if initial setup fails. E.g.: 2cores, 
>>>>>> 3ssps, 1PAGE_SIZE per trace buffer.. and such. There are quite a 
>>>>>> few factors to take into consideration though. If "asking" user 
>>>>>> via dmesg to upgrade the firmware if his/her setup contains 
>>>>>> obsolete binary is really not an option, then some magic words 
>>>>>> got to be involved.
>>>>>>
>>>>>> Czarek
>>>>>
>>>>> On the second thought what if instead of duplicating kernel code, 
>>>>> binaries would be duplicated?
>>>>> I.e. rather than targeting /intel/dsp_fw_cnl.bin, _new_ /skylake 
>>>>> would be expecting /intel/dsp_fw_cnl_release.bin? Same with 
>>>>> topology binaries.
>>>>> In such case, we "only" need to figure out how to propagate new 
>>>>> files to Linux distos so whenever someone updates their kernel, 
>>>>> new binaries are already present in their /lib/firmware.
>>>>>
>>>>> If such option is valid, we can postpone /skylake upgrade till 5.4 
>>>>> merging window closes and the patches (rough estimation is 150) 
>>>>> would descend upon alsa-devel in time between 5.4 and 5.5.
>>>>
>>>> If the driver and FW update will be within the same kernel release 
>>>> then IMHO
>>>> there should be no compatibility problem between those two 
>>>> components, right?
>>>> This way kernel users willing to stick to old FW can stay on older 
>>>> kernel version while
>>>> others can update and receive all the latest FW functionality that 
>>>> was developed and enabled.
>>>
>>> I am not comfortable with precluding a kernel update because of a 
>>> single firmware file. There are all sort of reasons for updating a 
>>> kernel, security, sideband attacks and Android CDD compatibility 
>>> being the most obvious ones.
>>>
The single firmware file will not be a blocker as the driver included in 
updated kernel will support it.
All you have to do is the little effort to re-generate your custom 
topology for the new firmware target.
The entire operation should not be a problem as there are dedicated 
utilities like FDK to do that.

Your statement Pierre suggest that everyone should avoid any functional 
changes in kernel
that are not critical because that would be problematic for others who 
switch from older kernel version.
>>>>
>>>> In terms of FW topology compatibility there is an option to read 
>>>> from topology manifest
>>>> a FW version that it was build for and in  case if it does not 
>>>> match FW version present on
>>>> the platform then print warning that the FW topology binary should 
>>>> be rebuild for current
>>>> FW version (x.x.x.x).
>>>
>>> Can you provide a pointer on how the FW version is embedded in a 
>>> .conf/.tplg file? I see a couple where that information does not 
>>> seem present.
>>>
>>>> The above approach at the end may be less confusing then source 
>>>> code or binary duplication.
>>>
>>
>> Indeed. Our existing topology skips that part of internal .xml and 
>> thus such information is not propagated to kernel.
>>
>> Pierre, how about the binary-duplication - as described above? Btw, 
>> that's not a single firmware file ^)^ We would immediately update all 
>> of them, together with topologies.
>
> I didn't understand how you would select the new firmwares? Some code 
> change needs to happen as well?
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Pierre-Louis Bossart Aug. 27, 2019, 1:52 p.m. UTC | #16
>>>>>>>> Not the most elegent solution but I'm wondering if keeping a copy
>>>>>>>> of the driver as is around and using new locations for the fixed
>>>>>>>> firmware might be the safest way to handle this.  We could have a
>>>>>>>> wrapper which tries to load the newer firmware and uses the fixed
>>>>>>>> driver code if that's there, otherwise tries the old driver with
>>>>>>>> the existing firmware paths.  This is obviously a horror show and
>>>>>>>> leaves the old code sitting there but given the mistakes that
>>>>>>>> have been made the whole situation looks like a house of cards.
>>>>>>>>
>>>>>>>
>>>>>>> Thanks for the feedback Mark. While I'm not yet on the "SOF will 
>>>>>>> fix this" train, I'm keen to agree to leaving this entirely to 
>>>>>>> SOF if it comes down to us duplicating /skylake.
>>>>>>>
>>>>>>> However, we are not going to give up that easily. I'll see if 
>>>>>>> some "golden config" hardcodes can't be provided in some legacy.c 
>>>>>>> file which would be fetched if initial setup fails. E.g.: 2cores, 
>>>>>>> 3ssps, 1PAGE_SIZE per trace buffer.. and such. There are quite a 
>>>>>>> few factors to take into consideration though. If "asking" user 
>>>>>>> via dmesg to upgrade the firmware if his/her setup contains 
>>>>>>> obsolete binary is really not an option, then some magic words 
>>>>>>> got to be involved.
>>>>>>>
>>>>>>> Czarek
>>>>>>
>>>>>> On the second thought what if instead of duplicating kernel code, 
>>>>>> binaries would be duplicated?
>>>>>> I.e. rather than targeting /intel/dsp_fw_cnl.bin, _new_ /skylake 
>>>>>> would be expecting /intel/dsp_fw_cnl_release.bin? Same with 
>>>>>> topology binaries.
>>>>>> In such case, we "only" need to figure out how to propagate new 
>>>>>> files to Linux distos so whenever someone updates their kernel, 
>>>>>> new binaries are already present in their /lib/firmware.
>>>>>>
>>>>>> If such option is valid, we can postpone /skylake upgrade till 5.4 
>>>>>> merging window closes and the patches (rough estimation is 150) 
>>>>>> would descend upon alsa-devel in time between 5.4 and 5.5.
>>>>>
>>>>> If the driver and FW update will be within the same kernel release 
>>>>> then IMHO
>>>>> there should be no compatibility problem between those two 
>>>>> components, right?
>>>>> This way kernel users willing to stick to old FW can stay on older 
>>>>> kernel version while
>>>>> others can update and receive all the latest FW functionality that 
>>>>> was developed and enabled.
>>>>
>>>> I am not comfortable with precluding a kernel update because of a 
>>>> single firmware file. There are all sort of reasons for updating a 
>>>> kernel, security, sideband attacks and Android CDD compatibility 
>>>> being the most obvious ones.
>>>>
> The single firmware file will not be a blocker as the driver included in 
> updated kernel will support it.
> All you have to do is the little effort to re-generate your custom 
> topology for the new firmware target.
> The entire operation should not be a problem as there are dedicated 
> utilities like FDK to do that.

The issue is the same whether it's a topology file or a firmware file. 
The ideal situation is that when the kernel is updated it handles both 
in backwards compatible ways.

If to deal with a new firmware file you have to regenerate a new 
topology, you are in a different model altogether.

> Your statement Pierre suggest that everyone should avoid any functional 
> changes in kernel
> that are not critical because that would be problematic for others who 
> switch from older kernel version.
  All I said was that you cannot assume that people who are using an old 
firmware/driver will remain on an old kernel.

Mark made an initial proposal to essentially freeze the current 
solution, which would make it possible to update the kernel but keep the 
same skylake driver in legacy/maintenance mode only, and an 'new' option 
that would rely on an updated distribution of firmware/driver. I did not 
get the counter proposal from Cezary at all.
Cezary Rojewski Aug. 27, 2019, 2:58 p.m. UTC | #17
On 2019-08-27 15:52, Pierre-Louis Bossart wrote:
> 
>>>>>>>>> Not the most elegent solution but I'm wondering if keeping a copy
>>>>>>>>> of the driver as is around and using new locations for the fixed
>>>>>>>>> firmware might be the safest way to handle this.  We could have a
>>>>>>>>> wrapper which tries to load the newer firmware and uses the fixed
>>>>>>>>> driver code if that's there, otherwise tries the old driver with
>>>>>>>>> the existing firmware paths.  This is obviously a horror show and
>>>>>>>>> leaves the old code sitting there but given the mistakes that
>>>>>>>>> have been made the whole situation looks like a house of cards.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks for the feedback Mark. While I'm not yet on the "SOF will 
>>>>>>>> fix this" train, I'm keen to agree to leaving this entirely to 
>>>>>>>> SOF if it comes down to us duplicating /skylake.
>>>>>>>>
>>>>>>>> However, we are not going to give up that easily. I'll see if 
>>>>>>>> some "golden config" hardcodes can't be provided in some 
>>>>>>>> legacy.c file which would be fetched if initial setup fails. 
>>>>>>>> E.g.: 2cores, 3ssps, 1PAGE_SIZE per trace buffer.. and such. 
>>>>>>>> There are quite a few factors to take into consideration though. 
>>>>>>>> If "asking" user via dmesg to upgrade the firmware if his/her 
>>>>>>>> setup contains obsolete binary is really not an option, then 
>>>>>>>> some magic words got to be involved.
>>>>>>>>
>>>>>>>> Czarek
>>>>>>>
>>>>>>> On the second thought what if instead of duplicating kernel code, 
>>>>>>> binaries would be duplicated?
>>>>>>> I.e. rather than targeting /intel/dsp_fw_cnl.bin, _new_ /skylake 
>>>>>>> would be expecting /intel/dsp_fw_cnl_release.bin? Same with 
>>>>>>> topology binaries.
>>>>>>> In such case, we "only" need to figure out how to propagate new 
>>>>>>> files to Linux distos so whenever someone updates their kernel, 
>>>>>>> new binaries are already present in their /lib/firmware.
>>>>>>>
>>>>>>> If such option is valid, we can postpone /skylake upgrade till 
>>>>>>> 5.4 merging window closes and the patches (rough estimation is 
>>>>>>> 150) would descend upon alsa-devel in time between 5.4 and 5.5.
>>>>>>
>>>>>> If the driver and FW update will be within the same kernel release 
>>>>>> then IMHO
>>>>>> there should be no compatibility problem between those two 
>>>>>> components, right?
>>>>>> This way kernel users willing to stick to old FW can stay on older 
>>>>>> kernel version while
>>>>>> others can update and receive all the latest FW functionality that 
>>>>>> was developed and enabled.
>>>>>
>>>>> I am not comfortable with precluding a kernel update because of a 
>>>>> single firmware file. There are all sort of reasons for updating a 
>>>>> kernel, security, sideband attacks and Android CDD compatibility 
>>>>> being the most obvious ones.
>>>>>
>> The single firmware file will not be a blocker as the driver included 
>> in updated kernel will support it.
>> All you have to do is the little effort to re-generate your custom 
>> topology for the new firmware target.
>> The entire operation should not be a problem as there are dedicated 
>> utilities like FDK to do that.
> 
> The issue is the same whether it's a topology file or a firmware file. 
> The ideal situation is that when the kernel is updated it handles both 
> in backwards compatible ways.
> 
> If to deal with a new firmware file you have to regenerate a new 
> topology, you are in a different model altogether.
> 
>> Your statement Pierre suggest that everyone should avoid any 
>> functional changes in kernel
>> that are not critical because that would be problematic for others who 
>> switch from older kernel version.
>   All I said was that you cannot assume that people who are using an old 
> firmware/driver will remain on an old kernel.
> 
> Mark made an initial proposal to essentially freeze the current 
> solution, which would make it possible to update the kernel but keep the 
> same skylake driver in legacy/maintenance mode only, and an 'new' option 
> that would rely on an updated distribution of firmware/driver. I did not 
> get the counter proposal from Cezary at all.

Ain't my previous message:

-

On the second thought what if instead of duplicating kernel code, 
binaries would be duplicated?
I.e. rather than targeting /intel/dsp_fw_cnl.bin, _new_ /skylake would 
be expecting /intel/dsp_fw_cnl_release.bin? Same with topology binaries.
In such case, we "only" need to figure out how to propagate new files to 
Linux distos so whenever someone updates their kernel, new binaries are 
already present in their /lib/firmware.

If such option is valid, we can postpone /skylake upgrade till 5.4 
merging window closes and the patches (rough estimation is 150) would 
descend upon alsa-devel in time between 5.4 and 5.5.

-

a counter proposal?
Pierre-Louis Bossart Aug. 27, 2019, 3 p.m. UTC | #18
On 8/27/19 9:58 AM, Cezary Rojewski wrote:
> On 2019-08-27 15:52, Pierre-Louis Bossart wrote:
>>
>>>>>>>>>> Not the most elegent solution but I'm wondering if keeping a copy
>>>>>>>>>> of the driver as is around and using new locations for the fixed
>>>>>>>>>> firmware might be the safest way to handle this.  We could have a
>>>>>>>>>> wrapper which tries to load the newer firmware and uses the fixed
>>>>>>>>>> driver code if that's there, otherwise tries the old driver with
>>>>>>>>>> the existing firmware paths.  This is obviously a horror show and
>>>>>>>>>> leaves the old code sitting there but given the mistakes that
>>>>>>>>>> have been made the whole situation looks like a house of cards.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks for the feedback Mark. While I'm not yet on the "SOF 
>>>>>>>>> will fix this" train, I'm keen to agree to leaving this 
>>>>>>>>> entirely to SOF if it comes down to us duplicating /skylake.
>>>>>>>>>
>>>>>>>>> However, we are not going to give up that easily. I'll see if 
>>>>>>>>> some "golden config" hardcodes can't be provided in some 
>>>>>>>>> legacy.c file which would be fetched if initial setup fails. 
>>>>>>>>> E.g.: 2cores, 3ssps, 1PAGE_SIZE per trace buffer.. and such. 
>>>>>>>>> There are quite a few factors to take into consideration 
>>>>>>>>> though. If "asking" user via dmesg to upgrade the firmware if 
>>>>>>>>> his/her setup contains obsolete binary is really not an option, 
>>>>>>>>> then some magic words got to be involved.
>>>>>>>>>
>>>>>>>>> Czarek
>>>>>>>>
>>>>>>>> On the second thought what if instead of duplicating kernel 
>>>>>>>> code, binaries would be duplicated?
>>>>>>>> I.e. rather than targeting /intel/dsp_fw_cnl.bin, _new_ /skylake 
>>>>>>>> would be expecting /intel/dsp_fw_cnl_release.bin? Same with 
>>>>>>>> topology binaries.
>>>>>>>> In such case, we "only" need to figure out how to propagate new 
>>>>>>>> files to Linux distos so whenever someone updates their kernel, 
>>>>>>>> new binaries are already present in their /lib/firmware.
>>>>>>>>
>>>>>>>> If such option is valid, we can postpone /skylake upgrade till 
>>>>>>>> 5.4 merging window closes and the patches (rough estimation is 
>>>>>>>> 150) would descend upon alsa-devel in time between 5.4 and 5.5.
>>>>>>>
>>>>>>> If the driver and FW update will be within the same kernel 
>>>>>>> release then IMHO
>>>>>>> there should be no compatibility problem between those two 
>>>>>>> components, right?
>>>>>>> This way kernel users willing to stick to old FW can stay on 
>>>>>>> older kernel version while
>>>>>>> others can update and receive all the latest FW functionality 
>>>>>>> that was developed and enabled.
>>>>>>
>>>>>> I am not comfortable with precluding a kernel update because of a 
>>>>>> single firmware file. There are all sort of reasons for updating a 
>>>>>> kernel, security, sideband attacks and Android CDD compatibility 
>>>>>> being the most obvious ones.
>>>>>>
>>> The single firmware file will not be a blocker as the driver included 
>>> in updated kernel will support it.
>>> All you have to do is the little effort to re-generate your custom 
>>> topology for the new firmware target.
>>> The entire operation should not be a problem as there are dedicated 
>>> utilities like FDK to do that.
>>
>> The issue is the same whether it's a topology file or a firmware file. 
>> The ideal situation is that when the kernel is updated it handles both 
>> in backwards compatible ways.
>>
>> If to deal with a new firmware file you have to regenerate a new 
>> topology, you are in a different model altogether.
>>
>>> Your statement Pierre suggest that everyone should avoid any 
>>> functional changes in kernel
>>> that are not critical because that would be problematic for others 
>>> who switch from older kernel version.
>>   All I said was that you cannot assume that people who are using an 
>> old firmware/driver will remain on an old kernel.
>>
>> Mark made an initial proposal to essentially freeze the current 
>> solution, which would make it possible to update the kernel but keep 
>> the same skylake driver in legacy/maintenance mode only, and an 'new' 
>> option that would rely on an updated distribution of firmware/driver. 
>> I did not get the counter proposal from Cezary at all.
> 
> Ain't my previous message:
> 
> -
> 
> On the second thought what if instead of duplicating kernel code, 
> binaries would be duplicated?
> I.e. rather than targeting /intel/dsp_fw_cnl.bin, _new_ /skylake would 
> be expecting /intel/dsp_fw_cnl_release.bin? Same with topology binaries.
> In such case, we "only" need to figure out how to propagate new files to 
> Linux distos so whenever someone updates their kernel, new binaries are 
> already present in their /lib/firmware.
> 
> If such option is valid, we can postpone /skylake upgrade till 5.4 
> merging window closes and the patches (rough estimation is 150) would 
> descend upon alsa-devel in time between 5.4 and 5.5.
> 
> -
> 
> a counter proposal?

you didn't explain how the 'duplicated binaries' would be selected. And 
'instead of' means you suggested an alternative to Mark's proposal.
Cezary Rojewski Aug. 27, 2019, 3:08 p.m. UTC | #19
On 2019-08-27 17:00, Pierre-Louis Bossart wrote:

>>>>>>>>> On the second thought what if instead of duplicating kernel 
>>>>>>>>> code, binaries would be duplicated?
>>>>>>>>> I.e. rather than targeting /intel/dsp_fw_cnl.bin, _new_ 
>>>>>>>>> /skylake would be expecting /intel/dsp_fw_cnl_release.bin? Same 
>>>>>>>>> with topology binaries.
>>>>>>>>> In such case, we "only" need to figure out how to propagate new 
>>>>>>>>> files to Linux distos so whenever someone updates their kernel, 
>>>>>>>>> new binaries are already present in their /lib/firmware.
>>>>>>>>>
>>>>>>>>> If such option is valid, we can postpone /skylake upgrade till 
>>>>>>>>> 5.4 merging window closes and the patches (rough estimation is 
>>>>>>>>> 150) would descend upon alsa-devel in time between 5.4 and 5.5.
>>>>>>>>
>>>>>>>> If the driver and FW update will be within the same kernel 
>>>>>>>> release then IMHO
>>>>>>>> there should be no compatibility problem between those two 
>>>>>>>> components, right?
>>>>>>>> This way kernel users willing to stick to old FW can stay on 
>>>>>>>> older kernel version while
>>>>>>>> others can update and receive all the latest FW functionality 
>>>>>>>> that was developed and enabled.
>>>>>>>
>>>>>>> I am not comfortable with precluding a kernel update because of a 
>>>>>>> single firmware file. There are all sort of reasons for updating 
>>>>>>> a kernel, security, sideband attacks and Android CDD 
>>>>>>> compatibility being the most obvious ones.
>>>>>>>
>>>> The single firmware file will not be a blocker as the driver 
>>>> included in updated kernel will support it.
>>>> All you have to do is the little effort to re-generate your custom 
>>>> topology for the new firmware target.
>>>> The entire operation should not be a problem as there are dedicated 
>>>> utilities like FDK to do that.
>>>
>>> The issue is the same whether it's a topology file or a firmware 
>>> file. The ideal situation is that when the kernel is updated it 
>>> handles both in backwards compatible ways.
>>>
>>> If to deal with a new firmware file you have to regenerate a new 
>>> topology, you are in a different model altogether.
>>>
>>>> Your statement Pierre suggest that everyone should avoid any 
>>>> functional changes in kernel
>>>> that are not critical because that would be problematic for others 
>>>> who switch from older kernel version.
>>>   All I said was that you cannot assume that people who are using an 
>>> old firmware/driver will remain on an old kernel.
>>>
>>> Mark made an initial proposal to essentially freeze the current 
>>> solution, which would make it possible to update the kernel but keep 
>>> the same skylake driver in legacy/maintenance mode only, and an 'new' 
>>> option that would rely on an updated distribution of firmware/driver. 
>>> I did not get the counter proposal from Cezary at all.
>>
>> Ain't my previous message:
>>
>> -
>>
>> On the second thought what if instead of duplicating kernel code, 
>> binaries would be duplicated?
>> I.e. rather than targeting /intel/dsp_fw_cnl.bin, _new_ /skylake would 
>> be expecting /intel/dsp_fw_cnl_release.bin? Same with topology binaries.
>> In such case, we "only" need to figure out how to propagate new files 
>> to Linux distos so whenever someone updates their kernel, new binaries 
>> are already present in their /lib/firmware.
>>
>> If such option is valid, we can postpone /skylake upgrade till 5.4 
>> merging window closes and the patches (rough estimation is 150) would 
>> descend upon alsa-devel in time between 5.4 and 5.5.
>>
>> -
>>
>> a counter proposal?
> 
> you didn't explain how the 'duplicated binaries' would be selected. And 
> 'instead of' means you suggested an alternative to Mark's proposal.

What I have in mind:

We leave the old stuff as is, e.g:
/lib/firmware/intel/dsp_fw_cnl.bin -> points to _old_ FW binaries
/lib/firmware/<PCI-ID>-INTEL-<oem_data_from_NHLT -> points to old topology

Existing /skylake i.e. before our initialization refactor would (kernels 
<5.5?) would still point to these and since they are not being removed 
from linux-firmware, nothing gets broken.


And then we "duplicate" and simply append the new ones:
/lib/firmware/intel/dsp_fw_cnl_release.bin -> points to _new_ FW
/lib/firmware/dfw_cnl_rt274 -> points to _new_ topology

Updated /skylake would simply expect the _new_ files and totally ignore 
the old ones i.e.: descriptors would be pointing to dsp_fw_cnl_release 
and dfw_cnl_rt274.
Pierre-Louis Bossart Aug. 27, 2019, 5:18 p.m. UTC | #20
On 8/27/19 10:08 AM, Cezary Rojewski wrote:
> On 2019-08-27 17:00, Pierre-Louis Bossart wrote:
> 
>>>>>>>>>> On the second thought what if instead of 
>>>>>>>>>> duplicating kernel code, binaries would be 
>>>>>>>>>> duplicated? I.e. rather than targeting 
>>>>>>>>>> /intel/dsp_fw_cnl.bin, _new_ /skylake would be 
>>>>>>>>>> expecting /intel/dsp_fw_cnl_release.bin? Same with 
>>>>>>>>>> topology binaries. In such case, we "only" need to 
>>>>>>>>>> figure out how to propagate new files to Linux 
>>>>>>>>>> distos so whenever someone updates their kernel, 
>>>>>>>>>> new binaries are already present in their 
>>>>>>>>>> /lib/firmware.
>>>>>>>>>> 
>>>>>>>>>> If such option is valid, we can postpone /skylake 
>>>>>>>>>> upgrade till 5.4 merging window closes and the 
>>>>>>>>>> patches (rough estimation is 150) would descend 
>>>>>>>>>> upon alsa-devel in time between 5.4 and 5.5.
>>>>>>>>> 
>>>>>>>>> If the driver and FW update will be within the same 
>>>>>>>>> kernel release then IMHO there should be no 
>>>>>>>>> compatibility problem between those two components, 
>>>>>>>>> right? This way kernel users willing to stick to old 
>>>>>>>>> FW can stay on older kernel version while others can 
>>>>>>>>> update and receive all the latest FW functionality 
>>>>>>>>> that was developed and enabled.
>>>>>>>> 
>>>>>>>> I am not comfortable with precluding a kernel update 
>>>>>>>> because of a single firmware file. There are all sort 
>>>>>>>> of reasons for updating a kernel, security, sideband 
>>>>>>>> attacks and Android CDD compatibility being the most 
>>>>>>>> obvious ones.
>>>>>>>> 
>>>>> The single firmware file will not be a blocker as the driver 
>>>>> included in updated kernel will support it. All you have to 
>>>>> do is the little effort to re-generate your custom topology 
>>>>> for the new firmware target. The entire operation should not 
>>>>> be a problem as there are dedicated utilities like FDK to do 
>>>>> that.
>>>> 
>>>> The issue is the same whether it's a topology file or a 
>>>> firmware file. The ideal situation is that when the kernel is 
>>>> updated it handles both in backwards compatible ways.
>>>> 
>>>> If to deal with a new firmware file you have to regenerate a 
>>>> new topology, you are in a different model altogether.
>>>> 
>>>>> Your statement Pierre suggest that everyone should avoid any 
>>>>> functional changes in kernel that are not critical because 
>>>>> that would be problematic for others who switch from older 
>>>>> kernel version.
>>>> All I said was that you cannot assume that people who are
>>>> using an old firmware/driver will remain on an old kernel.
>>>> 
>>>> Mark made an initial proposal to essentially freeze the
>>>> current solution, which would make it possible to update the
>>>> kernel but keep the same skylake driver in legacy/maintenance
>>>> mode only, and an 'new' option that would rely on an updated 
>>>> distribution of firmware/driver. I did not get the counter 
>>>> proposal from Cezary at all.
>>> 
>>> Ain't my previous message:
>>> 
>>> -
>>> 
>>> On the second thought what if instead of duplicating kernel
>>> code, binaries would be duplicated? I.e. rather than targeting 
>>> /intel/dsp_fw_cnl.bin, _new_ /skylake would be expecting 
>>> /intel/dsp_fw_cnl_release.bin? Same with topology binaries. In 
>>> such case, we "only" need to figure out how to propagate new 
>>> files to Linux distos so whenever someone updates their kernel, 
>>> new binaries are already present in their /lib/firmware.
>>> 
>>> If such option is valid, we can postpone /skylake upgrade till 
>>> 5.4 merging window closes and the patches (rough estimation is 
>>> 150) would descend upon alsa-devel in time between 5.4 and 5.5.
>>> 
>>> -
>>> 
>>> a counter proposal?
>> 
>> you didn't explain how the 'duplicated binaries' would be
>> selected. And 'instead of' means you suggested an alternative to
>> Mark's proposal.
> 
> What I have in mind:
> 
> We leave the old stuff as is, e.g:
> /lib/firmware/intel/dsp_fw_cnl.bin -> points to _old_ FW binaries 
> /lib/firmware/<PCI-ID>-INTEL-<oem_data_from_NHLT -> points to old 
> topology
> 
> Existing /skylake i.e. before our initialization refactor would 
> (kernels <5.5?) would still point to these and since they are not 
> being removed from linux-firmware, nothing gets broken.
> 
> 
> And then we "duplicate" and simply append the new ones: 
> /lib/firmware/intel/dsp_fw_cnl_release.bin -> points to _new_ FW 
> /lib/firmware/dfw_cnl_rt274 -> points to _new_ topology
> 
> Updated /skylake would simply expect the _new_ files and totally 
> ignore the old ones i.e.: descriptors would be pointing to 
> dsp_fw_cnl_release and dfw_cnl_rt274.

What if those new files are not present on the filesystem?

Mark suggested:
"We could have a wrapper which tries to load the newer firmware and uses
the fixed driver code if that's there, otherwise tries the old driver
with the existing firmware paths."

Maybe that's too complicated, I had in mind some sort of opt-in Kconfig 
where you only use the new firmware/topology when the user/distro gives 
a clear hint than it's fine to use newer stuff.

I also wonder how you are going to deal with all these topology files 
with a name derived from the OEM/NHLT. There's just so many of 
them...For upstream you probably want to provide ONE per platform 
variant, which limits you to the number of machine drivers supported.
Cezary Rojewski Aug. 27, 2019, 6:13 p.m. UTC | #21
On 2019-08-27 19:18, Pierre-Louis Bossart wrote:
> 
> 
> On 8/27/19 10:08 AM, Cezary Rojewski wrote:
>> On 2019-08-27 17:00, Pierre-Louis Bossart wrote:
>>
>>>>>>>>>>> On the second thought what if instead of duplicating kernel 
>>>>>>>>>>> code, binaries would be duplicated? I.e. rather than 
>>>>>>>>>>> targeting /intel/dsp_fw_cnl.bin, _new_ /skylake would be 
>>>>>>>>>>> expecting /intel/dsp_fw_cnl_release.bin? Same with topology 
>>>>>>>>>>> binaries. In such case, we "only" need to figure out how to 
>>>>>>>>>>> propagate new files to Linux distos so whenever someone 
>>>>>>>>>>> updates their kernel, new binaries are already present in 
>>>>>>>>>>> their /lib/firmware.
>>>>>>>>>>>
>>>>>>>>>>> If such option is valid, we can postpone /skylake upgrade 
>>>>>>>>>>> till 5.4 merging window closes and the patches (rough 
>>>>>>>>>>> estimation is 150) would descend upon alsa-devel in time 
>>>>>>>>>>> between 5.4 and 5.5.
>>>>>>>>>>
>>>>>>>>>> If the driver and FW update will be within the same kernel 
>>>>>>>>>> release then IMHO there should be no compatibility problem 
>>>>>>>>>> between those two components, right? This way kernel users 
>>>>>>>>>> willing to stick to old FW can stay on older kernel version 
>>>>>>>>>> while others can update and receive all the latest FW 
>>>>>>>>>> functionality that was developed and enabled.
>>>>>>>>>
>>>>>>>>> I am not comfortable with precluding a kernel update because of 
>>>>>>>>> a single firmware file. There are all sort of reasons for 
>>>>>>>>> updating a kernel, security, sideband attacks and Android CDD 
>>>>>>>>> compatibility being the most obvious ones.
>>>>>>>>>
>>>>>> The single firmware file will not be a blocker as the driver 
>>>>>> included in updated kernel will support it. All you have to do is 
>>>>>> the little effort to re-generate your custom topology for the new 
>>>>>> firmware target. The entire operation should not be a problem as 
>>>>>> there are dedicated utilities like FDK to do that.
>>>>>
>>>>> The issue is the same whether it's a topology file or a firmware 
>>>>> file. The ideal situation is that when the kernel is updated it 
>>>>> handles both in backwards compatible ways.
>>>>>
>>>>> If to deal with a new firmware file you have to regenerate a new 
>>>>> topology, you are in a different model altogether.
>>>>>
>>>>>> Your statement Pierre suggest that everyone should avoid any 
>>>>>> functional changes in kernel that are not critical because that 
>>>>>> would be problematic for others who switch from older kernel version.
>>>>> All I said was that you cannot assume that people who are
>>>>> using an old firmware/driver will remain on an old kernel.
>>>>>
>>>>> Mark made an initial proposal to essentially freeze the
>>>>> current solution, which would make it possible to update the
>>>>> kernel but keep the same skylake driver in legacy/maintenance
>>>>> mode only, and an 'new' option that would rely on an updated 
>>>>> distribution of firmware/driver. I did not get the counter proposal 
>>>>> from Cezary at all.
>>>>
>>>> Ain't my previous message:
>>>>
>>>> -
>>>>
>>>> On the second thought what if instead of duplicating kernel
>>>> code, binaries would be duplicated? I.e. rather than targeting 
>>>> /intel/dsp_fw_cnl.bin, _new_ /skylake would be expecting 
>>>> /intel/dsp_fw_cnl_release.bin? Same with topology binaries. In such 
>>>> case, we "only" need to figure out how to propagate new files to 
>>>> Linux distos so whenever someone updates their kernel, new binaries 
>>>> are already present in their /lib/firmware.
>>>>
>>>> If such option is valid, we can postpone /skylake upgrade till 5.4 
>>>> merging window closes and the patches (rough estimation is 150) 
>>>> would descend upon alsa-devel in time between 5.4 and 5.5.
>>>>
>>>> -
>>>>
>>>> a counter proposal?
>>>
>>> you didn't explain how the 'duplicated binaries' would be
>>> selected. And 'instead of' means you suggested an alternative to
>>> Mark's proposal.
>>
>> What I have in mind:
>>
>> We leave the old stuff as is, e.g:
>> /lib/firmware/intel/dsp_fw_cnl.bin -> points to _old_ FW binaries 
>> /lib/firmware/<PCI-ID>-INTEL-<oem_data_from_NHLT -> points to old 
>> topology
>>
>> Existing /skylake i.e. before our initialization refactor would 
>> (kernels <5.5?) would still point to these and since they are not 
>> being removed from linux-firmware, nothing gets broken.
>>
>>
>> And then we "duplicate" and simply append the new ones: 
>> /lib/firmware/intel/dsp_fw_cnl_release.bin -> points to _new_ FW 
>> /lib/firmware/dfw_cnl_rt274 -> points to _new_ topology
>>
>> Updated /skylake would simply expect the _new_ files and totally 
>> ignore the old ones i.e.: descriptors would be pointing to 
>> dsp_fw_cnl_release and dfw_cnl_rt274.
> 
> What if those new files are not present on the filesystem?

That's the hard part - we need to propagate these the Linux distos, much 
like older topologies are.

5.5+ (?) /skylake would rely on those new files as if the _old_ ones 
never existed.

> 
> Mark suggested:
> "We could have a wrapper which tries to load the newer firmware and uses
> the fixed driver code if that's there, otherwise tries the old driver
> with the existing firmware paths."
> 
> Maybe that's too complicated, I had in mind some sort of opt-in Kconfig 
> where you only use the new firmware/topology when the user/distro gives 
> a clear hint than it's fine to use newer stuff.


In one of the email you mentioned resources - human resources. If 
/skylake was to be duplicated, I fear maintenance of both would require 
too many resources. In such case we cannot guarantee same level of 
quality and coverage as in the _new_ /skylake-only case.

> 
> I also wonder how you are going to deal with all these topology files 
> with a name derived from the OEM/NHLT. There's just so many of 
> them...For upstream you probably want to provide ONE per platform 
> variant, which limits you to the number of machine drivers supported.


Precisely! That's why we resign from these and move to a simpler format 
- dfw_cnl_rt274 or something of that sort. And no, we would provide as 
many topologies (e.g. dfw_cnl_my_wondeful_board123) as it's necessary. 
_Old_ topologies are not even propagated for every OEM/NHLT - there are 
sightings such as:
https://bugzilla.kernel.org/show_bug.cgi?id=200963
or:
https://github.com/GalliumOS/galliumos-distro/issues/379

See the -ENOENT (-2) in the logs dumped. The debug-only dfw_sst.bin 
fallback plays a role there too when in fact, it should not be even 
present on upstream : )

I even saw cases where peps are copying binaries (FW) from Windows machines.

"we don't break userspace" - I'm all aboard, Pierre, but our ship has 
too many holes already. For a short while it was possible not to notice 
the water pouring in through them. But now ship is literally sinking. 
Userspace is broken.

Improper process led to distributed topologies missing or not even being 
compatible with all upstreamed FWs. These FWs are also carrying some 
bugs as they are deprecated for quite a while. In order to update them, 
host side (driver) needs to be aligned - there is no escaping that. And 
so the loop closes.

We want to - rather MUST - fix that and make Intel SST works as it 
should for the sake of all users.
Mark Brown Aug. 27, 2019, 7:06 p.m. UTC | #22
On Mon, Aug 26, 2019 at 11:51:32AM -0500, Pierre-Louis Bossart wrote:
> On 8/26/19 2:24 AM, Wasko, Michal wrote:

> > If the driver and FW update will be within the same kernel release then
> > IMHO
> > there should be no compatibility problem between those two components,
> > right?
> > This way kernel users willing to stick to old FW can stay on older
> > kernel version while
> > others can update and receive all the latest FW functionality that was
> > developed and enabled.

> I am not comfortable with precluding a kernel update because of a single
> firmware file. There are all sort of reasons for updating a kernel,
> security, sideband attacks and Android CDD compatibility being the most
> obvious ones.

Right, this is the whole ABI guarantee thing - we'd need an
incredibly strong reason to require a change in firmware for
upgrade of existing systems.
Mark Brown Aug. 27, 2019, 7:20 p.m. UTC | #23
On Tue, Aug 27, 2019 at 12:18:12PM -0500, Pierre-Louis Bossart wrote:

> Mark suggested:
> "We could have a wrapper which tries to load the newer firmware and uses
> the fixed driver code if that's there, otherwise tries the old driver
> with the existing firmware paths."

> Maybe that's too complicated, I had in mind some sort of opt-in Kconfig
> where you only use the new firmware/topology when the user/distro gives a
> clear hint than it's fine to use newer stuff.

To be clear I don't think this is a *good* idea, but I'm not sure
that there are any options that are good ideas.

> I also wonder how you are going to deal with all these topology files with a
> name derived from the OEM/NHLT. There's just so many of them...For upstream
> you probably want to provide ONE per platform variant, which limits you to
> the number of machine drivers supported.

Unless the way they're generated is consistent (or often
consistent) in which case you can just derive it?