Skip to content

Commit

Permalink
ALSA: hda: cs35l56: Perform firmware download in the background
Browse files Browse the repository at this point in the history 
It is possible that during system boot when there multiple devices
attempting simultaneous initialization on a slow control bus the
download of firmware and tuning data may take a user perceivable amount
of time (a slow I2C bus with 4 amps this work could take over 2
seconds).

Adopt a pattern used in the ASoC driver and perform this activity in a
background thread so that interactive performance is not impaired. The
system_long_wq is a parallel workqueue and driver instances will perform
their firmware downloads in parallel to make best use of available bus
bandwidth.

Signed-off-by: Simon Trimmer <simont@opensource.cirrus.com>
Link: https://lore.kernel.org/20240618130011.62860-1-simont@opensource.cirrus.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
  • Loading branch information
Simon Trimmer authored and Takashi Iwai committed Jun 18, 2024
1 parent f900a05 commit 634f3b4
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 17 deletions.
90 changes: 73 additions & 17 deletions sound/pci/hda/cs35l56_hda.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,19 @@ static const struct reg_sequence cs35l56_hda_dai_config[] = {

};

static void cs35l56_hda_wait_dsp_ready(struct cs35l56_hda *cs35l56)
{
/* Wait for patching to complete */
flush_work(&cs35l56->dsp_work);
}

static void cs35l56_hda_play(struct cs35l56_hda *cs35l56)
{
unsigned int val;
int ret;

cs35l56_hda_wait_dsp_ready(cs35l56);

pm_runtime_get_sync(cs35l56->base.dev);
ret = cs35l56_mbox_send(&cs35l56->base, CS35L56_MBOX_CMD_AUDIO_PLAY);
if (ret == 0) {
Expand Down Expand Up @@ -180,6 +188,8 @@ static int cs35l56_hda_mixer_get(struct snd_kcontrol *kcontrol,
unsigned int reg_val;
int i;

cs35l56_hda_wait_dsp_ready(cs35l56);

regmap_read(cs35l56->base.regmap, kcontrol->private_value, &reg_val);
reg_val &= CS35L56_ASP_TXn_SRC_MASK;

Expand All @@ -203,6 +213,8 @@ static int cs35l56_hda_mixer_put(struct snd_kcontrol *kcontrol,
if (item >= CS35L56_NUM_INPUT_SRC)
return -EINVAL;

cs35l56_hda_wait_dsp_ready(cs35l56);

regmap_update_bits_check(cs35l56->base.regmap, kcontrol->private_value,
CS35L56_INPUT_MASK, cs35l56_tx_input_values[item],
&changed);
Expand All @@ -227,6 +239,8 @@ static int cs35l56_hda_posture_get(struct snd_kcontrol *kcontrol,
unsigned int pos;
int ret;

cs35l56_hda_wait_dsp_ready(cs35l56);

ret = regmap_read(cs35l56->base.regmap, CS35L56_MAIN_POSTURE_NUMBER, &pos);
if (ret)
return ret;
Expand All @@ -248,6 +262,8 @@ static int cs35l56_hda_posture_put(struct snd_kcontrol *kcontrol,
(pos > CS35L56_MAIN_POSTURE_MAX))
return -EINVAL;

cs35l56_hda_wait_dsp_ready(cs35l56);

ret = regmap_update_bits_check(cs35l56->base.regmap,
CS35L56_MAIN_POSTURE_NUMBER,
CS35L56_MAIN_POSTURE_MASK,
Expand Down Expand Up @@ -291,6 +307,8 @@ static int cs35l56_hda_vol_get(struct snd_kcontrol *kcontrol,
int vol;
int ret;

cs35l56_hda_wait_dsp_ready(cs35l56);

ret = regmap_read(cs35l56->base.regmap, CS35L56_MAIN_RENDER_USER_VOLUME, &raw_vol);

if (ret)
Expand Down Expand Up @@ -323,6 +341,8 @@ static int cs35l56_hda_vol_put(struct snd_kcontrol *kcontrol,
raw_vol = (vol + CS35L56_MAIN_RENDER_USER_VOLUME_MIN) <<
CS35L56_MAIN_RENDER_USER_VOLUME_SHIFT;

cs35l56_hda_wait_dsp_ready(cs35l56);

ret = regmap_update_bits_check(cs35l56->base.regmap,
CS35L56_MAIN_RENDER_USER_VOLUME,
CS35L56_MAIN_RENDER_USER_VOLUME_MASK,
Expand Down Expand Up @@ -539,8 +559,9 @@ static void cs35l56_hda_release_firmware_files(const struct firmware *wmfw_firmw
kfree(coeff_filename);
}

static void cs35l56_hda_add_dsp_controls(struct cs35l56_hda *cs35l56)
static void cs35l56_hda_create_dsp_controls_work(struct work_struct *work)
{
struct cs35l56_hda *cs35l56 = container_of(work, struct cs35l56_hda, control_work);
struct hda_cs_dsp_ctl_info info;

info.device_name = cs35l56->amp_name;
Expand All @@ -566,23 +587,42 @@ static void cs35l56_hda_apply_calibration(struct cs35l56_hda *cs35l56)
dev_info(cs35l56->base.dev, "Calibration applied\n");
}

static int cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
static void cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
{
const struct firmware *coeff_firmware = NULL;
const struct firmware *wmfw_firmware = NULL;
char *coeff_filename = NULL;
char *wmfw_filename = NULL;
unsigned int preloaded_fw_ver;
bool firmware_missing;
int ret = 0;
bool add_dsp_controls_required = false;
int ret;

/*
* control_work must be flushed before proceeding, but we can't do that
* here as it would create a deadlock on controls_rwsem so it must be
* performed before queuing dsp_work.
*/
WARN_ON_ONCE(work_busy(&cs35l56->control_work));

/* Prepare for a new DSP power-up */
/*
* Prepare for a new DSP power-up. If the DSP has had firmware
* downloaded previously then it needs to be powered down so that it
* can be updated and if hadn't been patched before then the controls
* will need to be added once firmware download succeeds.
*/
if (cs35l56->base.fw_patched)
cs_dsp_power_down(&cs35l56->cs_dsp);
else
add_dsp_controls_required = true;

cs35l56->base.fw_patched = false;

pm_runtime_get_sync(cs35l56->base.dev);
ret = pm_runtime_resume_and_get(cs35l56->base.dev);
if (ret < 0) {
dev_err(cs35l56->base.dev, "Failed to resume and get %d\n", ret);
return;
}

/*
* The firmware can only be upgraded if it is currently running
Expand All @@ -606,7 +646,6 @@ static int cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
*/
if (!coeff_firmware && firmware_missing) {
dev_err(cs35l56->base.dev, ".bin file required but not found\n");
ret = -ENOENT;
goto err_fw_release;
}

Expand Down Expand Up @@ -659,6 +698,15 @@ static int cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
CS35L56_FIRMWARE_MISSING);
cs35l56->base.fw_patched = true;

/*
* Adding controls is deferred to prevent a lock inversion - ALSA takes
* the controls_rwsem when adding a control, the get() / put()
* functions of a control are called holding controls_rwsem and those
* that depend on running firmware wait for dsp_work() to complete.
*/
if (add_dsp_controls_required)
queue_work(system_long_wq, &cs35l56->control_work);

ret = cs_dsp_run(&cs35l56->cs_dsp);
if (ret)
dev_dbg(cs35l56->base.dev, "%s: cs_dsp_run ret %d\n", __func__, ret);
Expand All @@ -678,16 +726,20 @@ static int cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
coeff_firmware, coeff_filename);
err_pm_put:
pm_runtime_put(cs35l56->base.dev);
}

return ret;
static void cs35l56_hda_dsp_work(struct work_struct *work)
{
struct cs35l56_hda *cs35l56 = container_of(work, struct cs35l56_hda, dsp_work);

cs35l56_hda_fw_load(cs35l56);
}

static int cs35l56_hda_bind(struct device *dev, struct device *master, void *master_data)
{
struct cs35l56_hda *cs35l56 = dev_get_drvdata(dev);
struct hda_component_parent *parent = master_data;
struct hda_component *comp;
int ret;

comp = hda_component_from_index(parent, cs35l56->index);
if (!comp)
Expand All @@ -701,12 +753,10 @@ static int cs35l56_hda_bind(struct device *dev, struct device *master, void *mas
strscpy(comp->name, dev_name(dev), sizeof(comp->name));
comp->playback_hook = cs35l56_hda_playback_hook;

ret = cs35l56_hda_fw_load(cs35l56);
if (ret)
return ret;
flush_work(&cs35l56->control_work);
queue_work(system_long_wq, &cs35l56->dsp_work);

cs35l56_hda_create_controls(cs35l56);
cs35l56_hda_add_dsp_controls(cs35l56);

#if IS_ENABLED(CONFIG_SND_DEBUG)
cs35l56->debugfs_root = debugfs_create_dir(dev_name(cs35l56->base.dev), sound_debugfs_root);
Expand All @@ -724,6 +774,9 @@ static void cs35l56_hda_unbind(struct device *dev, struct device *master, void *
struct hda_component_parent *parent = master_data;
struct hda_component *comp;

cancel_work_sync(&cs35l56->dsp_work);
cancel_work_sync(&cs35l56->control_work);

cs35l56_hda_remove_controls(cs35l56);

#if IS_ENABLED(CONFIG_SND_DEBUG)
Expand Down Expand Up @@ -752,6 +805,9 @@ static int cs35l56_hda_system_suspend(struct device *dev)
{
struct cs35l56_hda *cs35l56 = dev_get_drvdata(dev);

cs35l56_hda_wait_dsp_ready(cs35l56);
flush_work(&cs35l56->control_work);

if (cs35l56->playing)
cs35l56_hda_pause(cs35l56);

Expand Down Expand Up @@ -850,11 +906,8 @@ static int cs35l56_hda_system_resume(struct device *dev)

ret = cs35l56_is_fw_reload_needed(&cs35l56->base);
dev_dbg(cs35l56->base.dev, "fw_reload_needed: %d\n", ret);
if (ret > 0) {
ret = cs35l56_hda_fw_load(cs35l56);
if (ret)
return ret;
}
if (ret > 0)
queue_work(system_long_wq, &cs35l56->dsp_work);

if (cs35l56->playing)
cs35l56_hda_play(cs35l56);
Expand Down Expand Up @@ -972,6 +1025,9 @@ int cs35l56_hda_common_probe(struct cs35l56_hda *cs35l56, int hid, int id)
mutex_init(&cs35l56->base.irq_lock);
dev_set_drvdata(cs35l56->base.dev, cs35l56);

INIT_WORK(&cs35l56->dsp_work, cs35l56_hda_dsp_work);
INIT_WORK(&cs35l56->control_work, cs35l56_hda_create_dsp_controls_work);

ret = cs35l56_hda_read_acpi(cs35l56, hid, id);
if (ret)
goto err;
Expand Down
3 changes: 3 additions & 0 deletions sound/pci/hda/cs35l56_hda.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,16 @@
#include <linux/firmware/cirrus/cs_dsp.h>
#include <linux/firmware/cirrus/wmfw.h>
#include <linux/regulator/consumer.h>
#include <linux/workqueue.h>
#include <sound/cs35l56.h>

struct dentry;

struct cs35l56_hda {
struct cs35l56_base base;
struct hda_codec *codec;
struct work_struct dsp_work;
struct work_struct control_work;

int index;
const char *system_name;
Expand Down

0 comments on commit 634f3b4

Please sign in to comment.