Re: [PATCH v12] ALSA: hda/tas2781: Add tas2781 hda SPI driver

Takashi Iwai
2025-01-17T10:12:22Z

On Fri, 03 Jan 2025 17:44:57 +0100,
Takashi Iwai wrote:
>
> On Mon, 16 Dec 2024 13:20:08 +0100,
> Baojun Xu wrote:
> >
> > This patch was used to add TAS2781 devices on SPI support in sound/pci/hda.
> > It use ACPI node descript about parameters of TAS2781 on SPI, it like:
> > Scope (_SB.PC00.SPI0)
> > {
> > Device (GSPK)
> > {
> > Name (_HID, "TXNW2781") // _HID: Hardware ID
> > Method (_CRS, 0, NotSerialized)
> > {
> > Name (RBUF, ResourceTemplate ()
> > {
> > SpiSerialBusV2 (...)
> > SpiSerialBusV2 (...)
> > }
> > }
> > }
> > }
> >
> > And in platform/x86/serial-multi-instantiate.c, those spi devices will be
> > added into system as a single SPI device, so TAS2781 SPI driver will
> > probe twice for every single SPI device. And driver will also parser
> > mono DSP firmware binary and RCA binary for itself.
> > The code support Realtek as the primary codec.
> > In patch version-10, add multi devices firmware binary support,
> > to compatble with windows driver, they can share same firmware binary.
> >
> > Signed-off-by: Baojun Xu <baojun.xu@ti.com>
> >
> > ---
> > v12:
> > - Reture real error in tas2781_save_calibration().
> > - Change tas_priv->rcabin.profile_cfg_id to 0 after reset.
> > - Add tas2781_spi_reset() in system suspend.
> > - Remove apply_calibration in system resume, it will cause no sound.
> > - Change unaligned.h include directory from asm to linux.
> > v11:
> > - Define config number for pre-reset and post-reset scripts.
> > - Remove acpi_subsystem_id as it was not used.
> > - Change variables order in structure, to avoid possible memory holes.
> > - Remove pre-reset and post-reset scripts in reset function.
> > - Change variables name to more common.
> > - Add pre-reset and post-reset scripts in fw_ready.
> > - Add empty line after end of function.
> > - Remove reset function before fw ready.
> > - Add result check for tasdevice_spi_prmg_load().
> > - Return real result of tasdevice_load_block_kernel().
> > - Change code for ppcver check, and update debug message.
> > - Remove load_calib_data() as no calibration data in firmware now.
> > - Remove tasdevice_spi_select_tuningprm_cfg() as config will be forced
> > downloaded after program download.
> > - Change comments for tasdevice_spi_tuning_switch().
> > - Remove unnecessary brace.
> > v10:
> > - Remove project id 103c8b93 in Patch_realtek.c, as it was used
> > for test only, offcial project id is 103c8daa (Gemtree).
> > - Add TAS2781_REG_CLK_CONFIG for check if AMP has been reset.
> > - Add table for pre-reset and post-reset.
> > - Perform pre-reset config before reset AMP.
> > - Perform post-reset config after reset AMP.
> > - Fix wrong offset in calibration data issue.
> > - Remove tuning switch after firmware download.
> > - Force stop play in runtime suspend.
> > - Apply patch for component change in bind & unbind.
> > - Remove program download and calibration data apply,
> > switch tuning only in runtime resume.
> > - Check if AMP has reseted in system resume,
> > and do firmware download and calibration data apply.
> > - Change flag 0xCx to 0x8x for coeff data force download.
> > - Add device index for multi device firmware binary file compatible.
> > - Check device index before registers write during firmware download.
> > - Force config data download after program download.
> > - Remove config download in tuning switch.
> > v9:
> > - Add parentheses for variables in micro define.
> > - Add define for calibration registers address identify.
> > - Change variable name from calibration_data to cali_data.
> > - Add variable for calibration registers address in struct tasdevice_priv.
> > - Change format for alignment to parentheses.
> > - Remove tas2781_reg_defaults as it will be over write in firmware download.
> > - Remove RESET for second device in tas2781_spi_reset(),
> > as it will cause no sound on second device.
> > - Add calibration registers address initial in tasdevice_spi_init().
> > - Remove unnecessary parentheses.
> > - Remove exception in tas2781_hda_playback_hook().
> > - Remove hard code for calibration registers address.
> > - Add support for calibration data format V3 for registers address identify,
> > as new firmware tool will define calibration registers address dynamic,
> > so can't hard code it, need read it from BIOS with calibration data.
> > - Remove some debug print.
> > - Change hard code value (3) to ARRAY_SIZE(tas2781_snd_controls)/2.
> > - New patch apply for change position of component_del().
> > - Change label from out, out1 to config_err and block_err.
> > - Remove unnecessary parentheses in tas2781_spi_fwlib.c.
> > - Remove tasdevice_spi_prmg_calibdata_load().
> > v8:
> > - Move *-y position in sound/pci/hda/Makefile.
> > - Change + to | for bit operation in macro define.
> > - Change TASDEVICE_Checksum to TASDEVICE_CHECKSUM.
> > v7:
> > - Add new project Gemtree (0x103c8daa) support for tas2781 spi driver.
> > v6:
> > - Remove client in tasdevice_priv struct as it was not used.
> > - Change format for tasdevice_spi_dev_write() in tas2781_hda_spi.c.
> > - Change update bits function to read & write mode, as tas2781 use last bit
> > to mask read & write, it cause regmap_update_bits work not as our expected.
> > - Add element in tas2781_snd_controls, tas2781_prof_ctrl,
> > tas2781_dsp_conf_ctrl, to support second device.
> > Previous version add sound card for first index only.
> > - Change calibration registers address declaire to TASDEVICE_REG() mode.
> > - Add snd_ctl_new1() for second device.
> > - Change format for tas2781_hda_unbind() to fit 79 chars in one line.
> > - Remove client for spi as it was not used.
> > - Remove unused variables.
> > - Remove cur_prog clean in runtime_suspend().
> > - Remove unused variables.
> > - Add tasdevice_spi_tuning_switch() in runtime_resume.
> > - Remove cur_prog clean in system_resume().
> > - Add tuning switch if playback started in system_resume().
> > - Change TASDEVICE_PAGE_REG to TASDEVICE_REG_ID in tas2781_spi_fwlib.c.
> > - Remove calibration binary load functions as data will be loaded from EFI.
> > v5:
> > - Combined all modification into one patch for pass the compile.
> > - Remove old hardware id "TIAS2781" as no production on market.
> > - Change max page size to 256 in tas2781-spi.h.
> > - Change book reg from 127 to GENMASK(7, 1), as one bit shift is needed.
> > - Change register address define as one bit shift needed for offset.
> > - Change block check sum comments to Firmware block from I2C.
> > - Change define of (page + reg) to TASDEVICE_PAGE_REG from TASDEVICE_PGRG.
> > - Change to lower case in defined value.
> > - Add registers default value table in tas2781_hda_spi.c
> > - Change rang_max to 256 * 256.
> > - Add zero_flag_mask in regmap_config.
> > - Change max_register to 256 * 256.
> > - Add registers default table into structure regmap_config.
> > - Change parameter from book number to whole register address.
> > - Remove page 0 switch as SPI driver will do it.
> > - Add bit(0) for read function as tas2781 will check last bit of address,
> > to determine operation mode is read (1) or write (0).
> > - Change switch case to if check in tas2781_hda_playback_hook(),
> > as it may cause compile problem in clang-19 building.
> > - Change variable declaration position.
> > - Change order in if check.
> > - Remove old hardware id ("TIAS2781") support.
> > - Remove debug message in runtime suspend and resume.
> > - Remove regmap.h include in tas2781_spi_fwlib.c.
> > - Remove MODULE_ in fwlib as this file will work with tas2781_hda_spi.c only.
> > v4:
> > - Add old hardware id "TIAS2781" for compatible with old production
> > - Add 2 devices in struct smi_node tas2781_hda, to compatible with 4 AMPs
> > v4:
> > - Add old hardware id "TIAS2781" for compatible with old production
> > - Add 2 devices in struct smi_node tas2781_hda, to compatible with 4 AMPs
> > - Move include of sound/tas2781-dsp.h to c source file tas2781_hda_spi.c,
> > tas2781_spi_fwlib.c, as it's not needed in header file tas2781-spi.h
> > - Use page and book number define in other micro define
> > - Change 4000000 to (4 * HZ_PER_MHZ)
> > - Add define for calibration variable name and size in UEFI.
> > - Remove structure tasdevice & tasdevice_irqinfo, and move variables in to
> > structure tasdevice_priv
> > - Remove some unused variables in sttructure tasdevice_priv
> > - Remove function declare of tascodec_init(), use tascodec_spi_init()
> > - Remove function declare of tasdevice_remove()
> > - Add array_size.h included in tas2781_hda_spi.c
> > - Add cleanup.h included for change mutex to guard
> > - Add units.h included
> > - Include tas2781-dsp.h
> > - Remove unused variables in tas2781_hda structure
> > - Change 0xff to GENMASK(7, 0)
> > - Change 128 to define of max page size
> > - Change code as all variables was moved to tasdevice_priv structure
> > - Change comments for error in book write, or page write
> > - Remove initial of ret, and return 0 directly
> > - Change comments for wrong typo, add space in front and end
> > - Add check for bulk read, max size should be a page size
> > - Change usleep_rang() to fsleep()
> > - Change mutex_lock to guard(mutex), and remove mutex_unlock()
> > - Change tasdevice_spi_init() to void from int as no value return required
> > - Change second parameter to sizeof
> > - Remove tasdevice_clamp(), use clamp()
> > - Add compatible with old hardware id "TIAS2781"
> > - Remove cs_gpio and spi device in tas2781_read_acpi() as use default CS
> > - Change dev-index check, return -EINVAL if it's <= 0
> > - Change 0xFF to U8_MAX
> > - Remove GPIO get for chip-select, use default from SPI device
> > - Perform RESET in first AMP only as all AMPs share same RESET pin
> > - return 0 directly in some simple routine to avoid variable initial
> > - Change comments for function which was used by system
> > - Change "ON" "OFF" to function str_on_off(...)
> > - Change format of GUID
> > - Add temp buffer for first efi.get_variable()
> > - Free data if it was allocated
> > - Change format of debug message of calibration date time print
> > - Remove total_sz = 0, as it's not needed
> > - Move save_calibration to after tuning_switch to avoid calibration data
> > overwrite
> > - Remove dev from structure tasdevice_hda, use dev in tasdevice_priv
> > - Remove 0 from { "tas2781-hda", 0}, & {"TXNW2781", 0},
> > - Add old hardware id "TIAS2781" for compatible with old production
> > - Add 2 devices in struct smi_node tas2781_hda, to compatible with 4 AMPs
> > v3:
> > - Move HID up to above /* Non-conforming _HID ... */ in scan.c,
> > for avoid misunderstanding.
> > - Move HID up to above /* Non-conforming _HID ... */ in
> > serial-multi-instantiate.c, for avoid misunderstanding.
> > - Change objs to y for snd-hda-scodec-tas2781-spi- in Makefile.
> > - Change format for some define in tas2781-spi.h.
> > - Add define for TASDEVICE_MAX_BOOK_NUM, TASDEVICE_MAX_PAGE.
> > - Move declare of "enum calib_data {" to header file from source file.
> > - Remove "enum device_catlog_id {" as only one platform was supported now.
> > - Remove "struct calidata", as we will save pure calibration data only.
> > - Remove "struct calidata", "enum device_catlog_id" in tasdevice_priv.
> > - Add calibration_data in tasdevice_priv to save pure calibration data.
> > - Remove declare of tasdevice_save_calibration() and
> > tasdevice_apply_calibration(), as it will be used within same c file.
> > - Add below header files included in tas2781_hda_spi.c:
> > bits.h, mutex.h, property.h, time.h, types.h.
> > - Move two micro define to header file tas2781-spi.h.
> > - Change format of some micro define.
> > - Change format of some structure.
> > - Remove irq in tas2781_hda, as have it already in tasdevice_priv.
> > - Remove some local functions declare as not necessarily.
> > - Return error if regmap_write() failed.
> > - Change fixed value 2 to sizeof(data).
> > - Remove all of EXPORT_SYMBOL_GPL,
> > as all of function was called within same module.
> > - Remove empty line after last return in some functions.
> > - Remove some variable initialyzation.
> > - Remove variable sub, store acpi_subsystem_id directly.
> > - Remove wrong comments for device index, it's must, can't be NULL.
> > - Remove some local variables, use elements in structure directly.
> > - Append commas in last element of an array.
> > - Change calibration data process, didn't save all of calibration data in EFI,
> > get in local, and save pure calibration data in tasdevice_priv.
> > - Call tasdevice_save_calibration() from function pointer in tasdevice_priv;
> > - Remove subid as one platform was supported only, needn't check.
> > - Add initialyzation for local variable.
> > - Move regmap initialyzation to before acpi read.
> > - Call tasdevice_apply_calibration() from function pointer in tasdevice_priv;
> > - Remove MODULE_IMPORT_NS(SND_SOC_TAS2781_FMWLIB), as all functions was
> > performed within same module.
> > - Update format and variables declare order.
> > - Change format of multi conditions for if.
> > - Remove casting which is not needed.
> > - Change variables type to avoid casting.
> > - Remove Unneeded parentheses.
> > - Change if check to avoid goto.
> > - Add {} for keep same style.
> > - Remove some local variables, use elements in structure directly.
> > v2:
> > - Change depends on SPI to SPI_MASTER in sound/pci/hda/Kconfig
> > - Add select SND_HDA_SCODEC_COMPONENT in sound/pci/hda/Kconfig
> > - Change comp_generic_fixup() 5th parameter from "-%s:00" to
> > "-%s:00-tas2781-hda.%d" in sound/pci/hda/patch_realtek.c
> > - Change chain_id from ALC285_FIXUP_THINKPAD_HEADSET_JACK to
> > ALC285_FIXUP_HP_GPIO_LED in sound/pci/hda/patch_realtek.c
> > - Remove project name "Gemtree"
> > - Update information in file header of tas2781-spi.h.
> > - Remove useless micro define.
> > - Change TASDEVICE_I2CChecksum to TASDEVICE_Checksum
> > - Remove enum control_bus as current code support SPI only.
> > - Remove device define as current code support TAS2781 only.
> > - Remove spi_device **spi_devs in structure tasdevice_priv.
> > - Remove cal_binaryname in structure tasdevice_priv.
> > - Remove ndev in structure tasdevice_priv.
> > - Change isacpi and isspi, replace by control_bus(I2C or SPI).
> > - Remove void tasdevice_spi_dsp_remove(void *context).
> > - Add acpi_device and irq in structure tas2781_hda in tas2781_hda_spi.c.
> > - Remove parameter chn in all of registers access functions.
> > - Add tascodec_spi_init().
> > - Add chip select gpio set for SPI device.
> > - Change tasdevice_tuning_switch() to tasdevice_spi_tuning_switch().
> > - Change device offset from tas_priv->ndev to tas_priv->index.
> > - Change tasdevice_dsp_remove() to tasdevice_spi_dsp_remove().
> > - tasdevice_prmg_load() to tasdevice_spi_prmg_load().
> > - Change tasdevice_config_info_remove() to
> > tasdevice_spi_config_info_remove().
> > - Add one dummy byte shift when read from page 2~127, or not book 0.
> > - Change tasdevice_spi_change_chn_book() to tasdevice_spi_switch_book()
> > - Perform put_device(physdev) before error return.
> > - Change calibration data id to 2781 from 2783 in EFI
> > - Change calibration data structure name in comments.
> > - Remove file name in tas2781_spi_fwlib.c.
> > - Remove #include <linux/i2c.h> as it not needed in SPI mode.
> > - Change TAB to speace in micro define in tas2781_spi_fwlib.c.
> > - Change to up-case for DSP.
> > - Change all of multi-line comments format, empty first line.
> > - Change all of sizeof(struct xx) to sizeof(*xx).
> > - Remove Explicit casting for variables compare to avoid possible fault.
> > - Return directly without goto.
> > - Change 1 << xx to BIT(xx).
> > - Remove deviceNumber[] as current SPI device will support one device only.
> > - Add memory free before return error.
> > - Change "buf[offset]; offset += 1" to buf[offset++].
> > - Remove some debug information print.
> > - Change firmware binary to single device mode.
> > - Change all bexx_to_cpup() to get_unaligned_bexx().
> > - Remove ndev process as current SPI device support single device only.
> > - Remove chn and chnend for single device support only.
> > - Change all of registers read/write function, remove parameter chn.
> > - Create new function for single write, burst write, delay, field write
> > in tasdevice_process_block().
> > ---
> > drivers/acpi/scan.c | 1 +
> > .../platform/x86/serial-multi-instantiate.c | 12 +
> > sound/pci/hda/Kconfig | 14 +
> > sound/pci/hda/Makefile | 2 +
> > sound/pci/hda/patch_realtek.c | 14 +
> > sound/pci/hda/tas2781-spi.h | 158 ++
> > sound/pci/hda/tas2781_hda_spi.c | 1271 +++++++++++
> > sound/pci/hda/tas2781_spi_fwlib.c | 2008 +++++++++++++++++
> > 8 files changed, 3480 insertions(+)
>
> So this series reached already v12, and we'd really like to take in
> some form. Although the code is still much more complex than I
> wished, it's more or less readable and understandable in some level,
> so I'm fine about the sound/* part.
>
> OTOH, this contains also the changes for drivers/acpi/scan.c and
> drivers/platform/x86/serial-multi-instantiate.c, and those need Acks
> from the corresponding people, but it missed Cc to them (and MLs).
> Now I put them to Cc.
>
> Rafael, Hans, Ilpo, Andy, anyone else - could you guys check it?

FYI, I merged the patch now for 6.14.
Let me know if you see any issues.

thanks,

Takashi