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

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

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
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel