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

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

 



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

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




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

  Powered by Linux