On Thu, Sep 17, 2020 at 04:12:28PM +0200, Cezary Rojewski wrote: > Implement support for Lynxpoint and Wildcat Point AudioDSP. Catpt > solution deprecates existing sound/soc/intel/haswell which is removed in > the following series. This cover-letter is followed by 'Developer's deep > dive' message schedding light on catpt's key concepts and areas > addressed. > > Due to high range of errors and desynchronization from recommendations > set by Windows solution, re-write came as a lower-cost solution compared > to refactoring /haswell/ with several series of patches. > > Series is dependent on linux-spi change: > spi: pxa2xx: Add SSC2 and SSPSP2 SSP registers > https://www.spinics.net/lists/linux-spi/msg23885.html > which has been already merged and is now part of linux-spi tree. > > Bulk of series content is device driver core code - everything up to > patch 7/14 - with fs entries and trace macros introduced right after. > While each core patch is shaped in such a way that no unavailable > members are ever called, until patch 10/14 is applied, no code > compilation can occur as no Makefile is present. Once said patch is > added, Makefile and Kconfig are implemented and driver module compiles > as expected. > > > Special thanks go to Marcin Barlik and Piotr Papierkowski for sharing > their LPT/WPT AudioDSP architecture expertise as well as helping > backtrack its historical background. > My thanks go to Amadeusz Slawinski for reviews and improvements proposed > on and off the internal list. Most of internal diff below is his > contribution. > Krzysztof Hejmowski helped me setup my own Xtensa environment and > recompile LPT/WPT FW binary sources what sped up the development greatly. > > This would not have been possible without help from these champions, > especially considering how quickly the catpt was written: 2 weeks > features, 3 weeks optimizations. Thank you. > > Userspace-exposed members are compatible with what is exposed by > deprecated solution as well as FW binary being re-used thus no harm is > done. The only visible differences are: the newly added 'Loopback Mute' > kcontrol and volume support extending to quad from stereo. > > On top of fixing erros and design flows, catpt also adds module reload, > dynamic SRAM memory allocation during PCM runtime and exposes missing > userspace API: 'Loopback Mute' kcontrol, quad volume controls and sysfs > fw-version entries. Event tracing is provided to easy solution > debugging. > > Following are not included in this update and are scheduled as later > addition: > - fw logging > - module (library) support > > Note: LPT power up/down sequences might get aligned with WPT once enough > testing is done as capabilities are shared for both DSPs. > Note #2: Both LPT and WPT power up/down sequences may get optimized in > future updates as thanks to help from the Windows team, most of nuances > behind why/what/when in regard to hw registers have been backtracked and > reviewed again. > > Link to developer's deep dive message: > https://www.spinics.net/lists/alsa-devel/msg113563.html Okay, I don't see any reason to resend a series right now and all my comments that left are rather nit-picks, so Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Changes in v6: > - reordered and reorganized code for patches 1/13 - 8/13 of v5, so each > patches makes use of no member or function which is unavailable to it. > Series size increased from 13 to 14 patches: addition of base members > e.g.: registers has been split from addition of device.c file which > describes acpi device behavior > > > Changes in v5: > https://www.spinics.net/lists/alsa-devel/msg115621.html > Basically everything below is result of Andy's review. Thank you Andy > for taking time into this detailed review > > - catpt now makes use of common linux/pxa2xx_ssp.h header file, removing > redundant SSP register declarations in the process. As stated in the > opening, this is dependent upon linux-spi change: > spi: pxa2xx: Add SSC2 and SSPSP2 SSP registers > > - updated Kconfig by removing DMADEVICES and adding COMPILE_TEST > as optional depends-on > - updated all register macros definitions to be more safe against common > arithmetics when specifying macro's parameters > - removed CONFIG_PM and CONFIG_PM_SLEEP usage in favor of __maybe_unused > - all 'if (ret < 0)' converted to simple 'if (ret)' whenever possible > - fixed erroneous check for platform_device_register_data within > catpt_register_board() > - _SLAVE/_MASTER replaced with more inclusive _CONSUMER/_PROVIDER for > enum catpt_ssp_mode > - catpt_acpi_probe() is now making use of high-level wrappers for > ioremapping and resource assignment, reducing function's code size > - due to improved catpt_acpi_probe() behavior, catpt_acpi_remove() needs > not to cast dma_free_coherent() any longer > - DMA source and destrination maxburst now of value 16, see: > https://www.spinics.net/lists/alsa-devel/msg114394.html > > - simplified catpt_dsp_update_lpclock() as list_for_each_entry() is > empty-safe by default > - dropped '_SSP_' from all names of all CATPT_SSP_SSXXX_DEFAULT macros > - catpt_updatel_pci now makes use of linux/pci.h and uapi/linux/pci.h > constants such as: PCI_PM_CTRL_STATE_MASK and PCI_D3hot > > > Changes in v4: > https://www.spinics.net/lists/alsa-devel/msg113762.html > - fixed compilation with i386 kconfig (conflicting names) > - streamlined naming for SHIM and PCI registers to match SSP ones > (SHIM_REG -> SHIM) > - catpt_component_probe removed and kcontrols again initializzed > statically via snd_kcontrol_new array: this is to remove > kctl->id.device shenanigans > - renamed catpt_set_ctlvol to catpt_set_dspvol - function name wasn't > matching its purpose > > > Changes in v3: > - fixed IRAM mask usage in lpt_dsp_power_up (dsp.c) > - updated dbg message formatting in catpt_restore_fwimage as suggested > by Andy > - fixed alignment for struct catpt_ssp_device_format > - catpt_set_ctlvol now verifies all-equal scenario based on all > channels rather than just first two as requested by Amadeo > - fixed SPDX for registers.h > > > Changes in v2: > https://www.spinics.net/lists/alsa-devel/msg113660.html > - fixed SPDX formatting for all header files as well as pcm.c > - fixed size provided to memcpy() in fw_build_read() as reported by Mark > - renamed struct catpt_pdata to struct catpt_spec (cosmetic) > - fixed erroneous path in catpt_load_block: region is properly released > - trace.h events for updating registers have been removed and usages > replaced by dev_dbg (SRAMPGE/ LPCS) > > - as requested by Andy, struct resource has replaced struct catpt_mbank > and struct catpt_mregion. This change cascaded into: > > - catpt_mbank_size and catpt_mregion_size replaced by resource_size > - catpt_mregion_intersects replaced by resource_overlaps > - all catpt_mbank_ and catpt_mregion_ handlers found in loader.c > (_request, _reserve, _release, _extract, _split, _join) have been > removed > - __request_region and __release_region have been enlisted in their > place > - catpt_mregion_intersecting renamed to catpt_resource_overlapping > - catpt_request_region helper has been provided to deal with -size > based requests > o haven't found direct replacements in resource.c/ ioport.h for > both functions > > - catpt_mbank_create and catpt_mbank_remove renamed to catpt_sram_init > and catpt_sram_free respectively > - catpt_sram_init now returns void instead of int and has been > converted to simple initialized. This change ultimately cascaded > into: > o both SRAM banks initialization being moved to catpt_dev_init > from catpt_acpi_probe (device.c) > o catpt_dev::spec is now initialized first, with catpt_dev_init > following it soon after > o catpt_acpi_probe erroneous path has been simplified as SRAM > banks no longer need to be freed > > - catpt_sram_free now frees all resources via child -> sibling > enumeration rather than region_list iteration > - catpt_dsp_update_srampge and catpt_dsp_set_srampge now accept new > argument: unsigned long mask. Caused by removal of catpt_mbank - > mask is taken directly from catpt_dev::spec::d/iram_mask > - trace.h events for catpt_mbank and catpt_mregion have been removed > > > Diff against last drop on internal list: > https://www.spinics.net/lists/alsa-devel/msg113549.html > - replaced spinlock with mutex for mregion allocation and release to > address sleeping in atomic context warnings > - fixed coredump fw_hash dumping > - kcontrol values are now always stored regardless of stream of interest > is running or not > - kcontrol values are now applied after stream is prepared instead of > ignoring what has been set by user initially > - catpt_pdata instances have been renamed from hsw_ and bdw_ to lpt_ and > wpt_ respectively > - reordered Makefile .o(s) (cosmetic) > > Cezary Rojewski (14): > ASoC: Intel: Add catpt base members > ASoC: Intel: catpt: Implement IPC protocol > ASoC: Intel: catpt: Add IPC message handlers > ASoC: Intel: catpt: Define DSP operations > ASoC: Intel: catpt: Firmware loading and context restore > ASoC: Intel: catpt: PCM operations > ASoC: Intel: catpt: Device driver lifecycle > ASoC: Intel: catpt: Event tracing > ASoC: Intel: catpt: Simple sysfs attributes > ASoC: Intel: Select catpt and deprecate haswell > ASoC: Intel: haswell: Remove haswell-solution specific code > ASoC: Intel: broadwell: Remove haswell-solution specific code > ASoC: Intel: bdw-5650: Remove haswell-solution specific code > ASoC: Intel: bdw-5677: Remove haswell-solution specific code > > sound/soc/intel/Kconfig | 24 +- > sound/soc/intel/Makefile | 2 +- > sound/soc/intel/boards/Kconfig | 8 +- > sound/soc/intel/boards/bdw-rt5650.c | 36 - > sound/soc/intel/boards/bdw-rt5677.c | 33 - > sound/soc/intel/boards/broadwell.c | 33 - > sound/soc/intel/boards/haswell.c | 28 +- > sound/soc/intel/catpt/Makefile | 6 + > sound/soc/intel/catpt/core.h | 189 +++++ > sound/soc/intel/catpt/device.c | 354 ++++++++ > sound/soc/intel/catpt/dsp.c | 572 +++++++++++++ > sound/soc/intel/catpt/fs.c | 79 ++ > sound/soc/intel/catpt/ipc.c | 298 +++++++ > sound/soc/intel/catpt/loader.c | 671 +++++++++++++++ > sound/soc/intel/catpt/messages.c | 313 +++++++ > sound/soc/intel/catpt/messages.h | 401 +++++++++ > sound/soc/intel/catpt/pcm.c | 1212 +++++++++++++++++++++++++++ > sound/soc/intel/catpt/registers.h | 178 ++++ > sound/soc/intel/catpt/trace.h | 83 ++ > 19 files changed, 4377 insertions(+), 143 deletions(-) > create mode 100644 sound/soc/intel/catpt/Makefile > create mode 100644 sound/soc/intel/catpt/core.h > create mode 100644 sound/soc/intel/catpt/device.c > create mode 100644 sound/soc/intel/catpt/dsp.c > create mode 100644 sound/soc/intel/catpt/fs.c > create mode 100644 sound/soc/intel/catpt/ipc.c > create mode 100644 sound/soc/intel/catpt/loader.c > create mode 100644 sound/soc/intel/catpt/messages.c > create mode 100644 sound/soc/intel/catpt/messages.h > create mode 100644 sound/soc/intel/catpt/pcm.c > create mode 100644 sound/soc/intel/catpt/registers.h > create mode 100644 sound/soc/intel/catpt/trace.h > > -- > 2.17.1 > -- With Best Regards, Andy Shevchenko