Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 15 Jul 2022 16:25:55 +0200
Message-ID: <87k08ev4ek.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     Vitaly Rodionov <vitalyr@opensource.cirrus.com>
Cc:     Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
        Mark Brown <broonie@kernel.org>, <alsa-devel@alsa-project.org>,
        <patches@opensource.cirrus.com>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v8 00/14] ALSA: hda: cirrus: Add initial DSP support and firmware loading
In-Reply-To: <e4c1eb0c-329a-a8c4-0d72-76475f82c01f@opensource.cirrus.com>
References: <20220630002335.366545-1-vitalyr@opensource.cirrus.com>
        <87zghpxcsh.wl-tiwai@suse.de>
        <e4c1eb0c-329a-a8c4-0d72-76475f82c01f@opensource.cirrus.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Wed, 06 Jul 2022 12:40:36 +0200,
Vitaly Rodionov wrote:
> 
> 
> Hi Takashi,
> 
> On 04/07/2022 13:50, Takashi Iwai wrote:
> 
>     On Thu, 30 Jun 2022 02:23:21 +0200,
>     Vitaly Rodionov wrote:
>     
>         The CS35L41 Amplifier contains a DSP, capable of running firmware.
>         The firmware can run algorithms such as Speaker Protection, to ensure
>         that playback at high gains do not harm the speakers.
>         Adding support for CS35L41 firmware into the CS35L41 HDA driver also
>         allows us to support several extra features, such as hiberation 
>         and interrupts.
>         
>         The chain adds support in stages:
>         - General fixes to improve generalization and code re-use inside
>           the CS35L41 HDA driver.
>         - Add support for interrupts into the driver, which is required
>           for complete support of the firmware.
>         - Refactor ASoC CS35L41 code which deals with firmware to allow
>           for code re-use inside the CS35L41 HDA driver.
>         - Add support for loading firmware and tuning files from file system,
>           and creating alsa controls to control it.
>         - Support firmware load paths for different hardware systems.
>         - Support suspend/resume in the driver when using firmware. The firmware
>           supports hibernation, which allows the CS35L41 to drop into a low
>           power mode during suspend.
>         - Support the ability to unload firmware, swap and reload the firmware.
>           This is to allow different firmware to run during calibration.
>         
>         The intended use-case is to load the firmware once on boot, and the driver
>         autmatically tries to load the firmware after it binds to the HDA driver.
>         This behaviour can be switched off using a kconfig, if desired.
>         
>         changes since v7:
>          - Use private_data rather than private_value to save control info
>          - Clean up alsa control memory allocation/deallocation
>          - Remove unnecessary whitespace
>          - Get subsystem id from codec, rather than saving it separately
>         
>         changes since v6:
>          - Fix warning by kernel test robot <lkp@intel.com>
>          
>         changes since v5:
>          - Fix warning by kernel test robot <lkp@intel.com>
>          
>         changes since v4:
>         - Fully remove tlv remnants from control add apis
>         - Remove unnecessary debug
>         - Rename variable to be more generic
>         - Remove redundent length check from read/write control apis
> 
>         - Use SNDRV_CTL_ELEM_IFACE_CARD for firmware load controls
>         - Make kcontrol add/remove synchronous
>         - Load firmware asynchronous when loading via control
>         - Used cached controls when reloading firmware; only delete
>         controls when removing the driver itself
> 
>         - Improve kcontrol remove
>         - Fix control write + notify
>         - Cleanup of unnecessary code
>         - Fix race condition when loading firmware before playback
>         - Ensure errors are properly propogated
>         - Fix include for Module parameters
>         
>         Stefan Binding (13):
>           ALSA: hda: hda_cs_dsp_ctl: Add Library to support CS_DSP ALSA controls
>           ALSA: hda: hda_cs_dsp_ctl: Add apis to write the controls directly
>           ALSA: hda: cs35l41: Save codec object inside component struct
>           ALSA: hda: cs35l41: Save Subsystem ID inside CS35L41 Driver
>           ALSA: hda: cs35l41: Support reading subsystem id from ACPI
>           ALSA: hda: cs35l41: Support multiple load paths for firmware
>           ALSA: hda: cs35l41: Support Speaker ID for laptops
>           ALSA: hda: cs35l41: Support Hibernation during Suspend
>           ALSA: hda: cs35l41: Read Speaker Calibration data from UEFI variables
>           ALSA: hda: hda_cs_dsp_ctl: Add fw id strings
>           ALSA: hda: cs35l41: Add defaulted values into dsp bypass config
>             sequence
>           ALSA: hda: cs35l41: Support Firmware switching and reloading
>           ALSA: hda: cs35l41: Add module parameter to control firmware load
>         
>         Vitaly Rodionov (1):
>           ALSA: hda: cs35l41: Add initial DSP support and firmware loading
>         
>     Thanks, this version looks better than previous ones, and I'm fine to
>     apply as is, to make things going forward.  But this seems requiring
>     the prerequisite in ASoC codec side.
>     
>     Mark, could you send a PR to merge into my tree so that I can apply
>     those series?
> 
>     And, one still uncertain thing is about the handling of system
>     suspend/resume, especially about the firmware loading work.  e.g. what
>     happens if a f/w work is in run and the system goes to suspend at the
>     very same time?  Or does the S3/S4 work in general at all?
> 
> Sorry for the delay, but your comments raised our attention and we wanted to
> run extensive testing again to make sure there is no hidden issue. No issues
> were found during tests. When we start FW download work we call "resume", so
> the resume counter should be incremented and suspend will not happen till we
> finish the download process.

Now all patches have been merged to for-next branch.


thanks,

Takashi