At Fri, 8 Feb 2013 18:31:42 -0800, Ian Minett wrote:

From: Ian Minett ian_minett@creativelabs.com

Use request_firmware() to fetch an image containing EQ data. This is then loaded onto the DSP chip for use by the SpeakerEQ effect. The DSP will carry on as normal without the EQ settings in the event that the SpeakerEQ f/w is missing or fails to load.

Well, it's almost close to feature freeze for the next merge window, I'll postpone this. (And unless we get the firmware for testing.)

In addition, some questions about the patch:

Signed-off-by: Ian Minett ian_minett@creativelabs.com

diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index 639a282..a4b61a9 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -73,9 +73,11 @@ #define SCP_SET 0 #define SCP_GET 1

+#define SPEQ_FILE "ctspeq.bin" #define EFX_FILE "ctefx.bin"

#ifdef CONFIG_SND_HDA_CODEC_CA0132_DSP +MODULE_FIRMWARE(SPEQ_FILE); MODULE_FIRMWARE(EFX_FILE); #endif

@@ -2516,6 +2518,42 @@ exit: return status; }

+/**

• • Write the SpeakerEQ coefficient data to DSP memories
• • @codec: the HDA codec
• • @x,y: location to write x and y coeff data to
• • Returns zero or a negative error code.
• */

+static int dspload_get_speakereq_addx(struct hda_codec *codec,

• 		unsigned int *x,
  

• 		unsigned int *y)
  

+{

• int status = 0;
• struct { unsigned short y, x; } speakereq_info;

Is this endian-independent?

• unsigned int size = sizeof(speakereq_info);

This must be 32bit, right?

Takashi

• snd_printdd(KERN_INFO "dspload_get_speakereq_addx() -- begin");
• status = dspio_scp(codec, MASTERCONTROL,

• 	MASTERCONTROL_QUERY_SPEAKER_EQ_ADDRESS,
  

• 	SCP_GET, NULL, 0, &speakereq_info, &size);
  

• if (status < 0) {

• snd_printdd(KERN_INFO "dspload_get_speakereq_addx: SCP Failed");
  

• return -EIO;
  

• }
• *x = speakereq_info.x;
• *y = speakereq_info.y;
• snd_printdd(KERN_INFO "dspload_get_speakereq_addx: X=0x%x Y=0x%x\n",

•     *x, *y);
  

• snd_printdd(KERN_INFO "dspload_get_speakereq_addx() -- complete");
• return status;

+}

/*

• CA0132 DSP download stuffs.

*/ @@ -2602,6 +2640,35 @@ static int dspload_image(struct hda_codec *codec, return status; }

+static int dspload_speakereq(struct hda_codec *codec) +{

• int status = 0;
• const struct dsp_image_seg *image;
• unsigned int x, y;
• const struct firmware *fw_speq;
• snd_printdd(KERN_INFO "dspload_speakereq() -- begin");
• if (request_firmware(&fw_speq, SPEQ_FILE,

• 	     codec->bus->card->dev) != 0)
  

• return -EIO;
  

• image = (struct dsp_image_seg *)(fw_speq->data);
• status = dspload_get_speakereq_addx(codec, &x, &y);
• if (status < 0)

• goto done;
  

• status = dspload_image(codec, image, 1, y, 0, 8);

+done:

• release_firmware(fw_speq);
• snd_printdd(KERN_INFO "dspload_speakereq() -- complete");
• return status;

+}

static bool dspload_is_loaded(struct hda_codec *codec) { unsigned int data = 0; @@ -4335,6 +4402,8 @@ static bool ca0132_download_dsp_images(struct hda_codec *codec)

release_firmware(fw_entry);

• if (dsp_loaded)

• dspload_speakereq(codec);
  
  

  return dsp_loaded;

}

1.7.4.1

At Fri, 8 Feb 2013 18:31:43 -0800, Ian Minett wrote:

From: Ian Minett ian_minett@creativelabs.com

Base the DSP firmware transfer and communication timeouts on jiffy values.

Signed-off-by: Ian Minett ian_minett@creativelabs.com

OK, applied this one.

thanks,

Takashi

diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index a4b61a9..b52f00c 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -785,7 +785,7 @@ static int chipio_send(struct hda_codec *codec, unsigned int data) { unsigned int res;

• int retry = 50;

• unsigned long timeout = jiffies + msecs_to_jiffies(1000);

  /* send bits of data specified by reg */ do {

@@ -793,7 +793,9 @@ static int chipio_send(struct hda_codec *codec, reg, data); if (res == VENDOR_STATUS_CHIPIO_OK) return 0;

• } while (--retry);

• msleep(20);
  

• } while (time_before(jiffies, timeout));
• return -EIO;

}

@@ -1059,14 +1061,15 @@ static int dspio_send(struct hda_codec *codec, unsigned int reg, unsigned int data) { int res;

• int retry = 50;

• unsigned long timeout = jiffies + msecs_to_jiffies(1000);

  /* send bits of data specified by reg to dsp */ do { res = snd_hda_codec_read(codec, WIDGET_DSP_CTRL, 0, reg, data); if ((res >= 0) && (res != VENDOR_STATUS_DSPIO_BUSY)) return res;

• } while (--retry);

• msleep(20);
  

• } while (time_before(jiffies, timeout));

  return -EIO;

} @@ -1298,7 +1301,6 @@ static int dspio_send_scp_message(struct hda_codec *codec, unsigned int *bytes_returned) { struct ca0132_spec *spec = codec->spec;

• int retry; int status = -1; unsigned int scp_send_size = 0; unsigned int total_size;

@@ -1345,13 +1347,13 @@ static int dspio_send_scp_message(struct hda_codec *codec, }

if (waiting_for_resp) {

• unsigned long timeout = jiffies + msecs_to_jiffies(1000);
  

  memset(return_buf, 0, return_buf_size);

• retry = 50;
  

  do { msleep(20);

• } while (spec->wait_scp && (--retry != 0));
  

• } while (spec->wait_scp && time_before(jiffies, timeout));
  

  waiting_for_resp = false;

• if (retry != 0) {
  

• if (!spec->wait_scp) {
  ret_msg = (struct scp_msg *)return_buf;
  memcpy(&ret_msg->hdr, &spec->scp_resp_header, 4);
  memcpy(&ret_msg->data, spec->scp_resp_data,
  

@@ -2244,7 +2246,8 @@ static int dspxfr_one_seg(struct hda_codec *codec, u32 chip_addx_remainder; unsigned int run_size_words; const struct dsp_image_seg *hci_write = NULL;

• int retry;

• unsigned long timeout;

• bool dma_active;

  if (fls == NULL) return -EINVAL;

@@ -2362,11 +2365,17 @@ static int dspxfr_one_seg(struct hda_codec *codec, status = dspxfr_hci_write(codec, hci_write); hci_write = NULL; }

• retry = 5000;
  

• while (dsp_is_dma_active(codec, dma_chan)) {
  

• 	if (--retry <= 0)
  

• timeout = jiffies + msecs_to_jiffies(2000);
  

• do {
  

• 	dma_active = dsp_is_dma_active(codec, dma_chan);
  

• 	if (!dma_active)
  	break;
  

• }
  

• 	msleep(20);
  

• } while (time_before(jiffies, timeout));
  

• if (dma_active)
  

• 	break;
  

• snd_printdd(KERN_INFO "+++++ DMA complete"); dma_set_state(dma_engine, DMA_STATE_STOP); dma_reset(dma_engine);

@@ -2683,15 +2692,15 @@ static bool dspload_is_loaded(struct hda_codec *codec)

static bool dspload_wait_loaded(struct hda_codec *codec) {

• int retry = 100;

• unsigned long timeout = jiffies + msecs_to_jiffies(2000);

  do {

• msleep(20);
  

  if (dspload_is_loaded(codec)) { pr_info("ca0132 DOWNLOAD OK :-) DSP IS RUNNING.\n"); return true; }
• } while (--retry);

• msleep(20);
  

• } while (time_before(jiffies, timeout));

  pr_err("ca0132 DOWNLOAD FAILED!!! DSP IS NOT RUNNING.\n"); return false;

-- 1.7.4.1

At Fri, 8 Feb 2013 18:31:44 -0800, Ian Minett wrote:

From: Ian Minett ian_minett@creativelabs.com

Add latency monitoring for playback, capture and DSP effects.

Please give a bit more descriptions. Looking at the patch, it doesn't implement only the latency monitoring but actually reflecting to the runtime->delay.

Also..

Signed-off-by: Ian Minett ian_minett@creativelabs.com

diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index b52f00c..c797f65 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -133,6 +133,12 @@ enum { /* Effects values size*/ #define EFFECT_VALS_MAX_COUNT 12

+#define DSP_CAPTURE_INIT_LATENCY 0 +#define DSP_CRYSTAL_VOICE_LATENCY 124 +#define DSP_PLAYBACK_INIT_LATENCY 13 +#define DSP_PLAY_ENHANCEMENT_LATENCY 30 +#define DSP_SPEAKER_OUT_LATENCY 7

struct ct_effect { char name[44]; hda_nid_t nid; @@ -2761,8 +2767,27 @@ static void ca0132_cleanup_stream(struct hda_codec *codec, hda_nid_t nid) }

/*

• • PCM callbacks

• • PCM playbacks
  */

+static unsigned int ca0132_get_playback_latency(struct hda_codec *codec) +{

• struct ca0132_spec *spec = codec->spec;
• unsigned int latency = DSP_PLAYBACK_INIT_LATENCY;
• if (!dspload_is_loaded(codec))

• return 0;
  

• if (spec->effects_switch[PLAY_ENHANCEMENT - EFFECT_START_NID]) {

• if ((spec->effects_switch[SURROUND - EFFECT_START_NID]) ||
  

•     (spec->effects_switch[DIALOG_PLUS - EFFECT_START_NID]))
  

• 	latency += DSP_PLAY_ENHANCEMENT_LATENCY;
  

• }
• if (spec->cur_out_type == SPEAKER_OUT)

• latency += DSP_SPEAKER_OUT_LATENCY;
  

• return latency;

+}

static int ca0132_playback_pcm_prepare(struct hda_pcm_stream *hinfo, struct hda_codec *codec, unsigned int stream_tag, @@ -2770,6 +2795,16 @@ static int ca0132_playback_pcm_prepare(struct hda_pcm_stream *hinfo, struct snd_pcm_substream *substream) { struct ca0132_spec *spec = codec->spec;

• struct snd_pcm_runtime *runtime = substream->runtime;

• unsigned int latency = ca0132_get_playback_latency(codec);

• if (spec->dsp_state == DSP_DOWNLOADING) {

• spec->dsp_stream_id = stream_tag;
  

• return 0;
  

• }

• runtime->delay = bytes_to_frames(runtime, (latency * runtime->rate *

• 			runtime->byte_align) / 1000);
  
  

  ca0132_setup_stream(codec, spec->dacs[0], stream_tag, 0, format);

@@ -2834,8 +2869,22 @@ static int ca0132_dig_playback_pcm_close(struct hda_pcm_stream *hinfo, }

/*

• • Analog capture

• • PCM capture
  */

+static unsigned int ca0132_get_capture_latency(struct hda_codec *codec) +{

• struct ca0132_spec *spec = codec->spec;
• unsigned int latency = DSP_CAPTURE_INIT_LATENCY;
• if (!dspload_is_loaded(codec))

• return 0;
  

• if (spec->effects_switch[CRYSTAL_VOICE - EFFECT_START_NID])

• latency += DSP_CRYSTAL_VOICE_LATENCY;
  

• return latency;

+}

static int ca0132_capture_pcm_prepare(struct hda_pcm_stream *hinfo, struct hda_codec *codec, unsigned int stream_tag, @@ -2843,9 +2892,16 @@ static int ca0132_capture_pcm_prepare(struct hda_pcm_stream *hinfo, struct snd_pcm_substream *substream) { struct ca0132_spec *spec = codec->spec;

• struct snd_pcm_runtime *runtime = substream->runtime;
• unsigned int latency = ca0132_get_capture_latency(codec);

• ca0132_setup_stream(codec, spec->adcs[substream->number],

• 	    stream_tag, 0, format);
  

• if (spec->dsp_state == DSP_DOWNLOADING)

• return 0;
  

• runtime->delay = bytes_to_frames(runtime, (latency * runtime->rate *

• 			runtime->byte_align) / 1000);
  

• ca0132_setup_stream(codec, hinfo->nid, stream_tag, 0, format);

  return 0;

} @@ -3429,6 +3485,33 @@ static int ca0132_voicefx_set(struct hda_codec *codec, int enable) return 1; }

+static void ca0132_update_latency(struct hda_codec *codec,

• 		  unsigned int direction)
  

+{

• struct ca0132_spec *spec = codec->spec;
• unsigned int latency;
• struct snd_pcm_substream *substr;
• struct snd_pcm_runtime *runtime;
• if (spec->dsp_state == DSP_DOWNLOADING)

• return;
  

• latency = (direction == SNDRV_PCM_STREAM_PLAYBACK)

• 	? ca0132_get_playback_latency(codec)
  

• 	: ca0132_get_capture_latency(codec);
  

• substr = codec->pcm_info->pcm->streams[direction].substream;
• while (substr) {

• runtime = substr->runtime;
  

• /* update latency only when frame_bits is set */
  

• if (runtime && runtime->frame_bits)
  

• 	runtime->delay = bytes_to_frames(runtime,
  

• 			(latency * runtime->rate *
  

• 			runtime->byte_align) / 1000);
  

• substr = substr->next;
  

• }

I guess this isn't needed. Usually the pointer callback is performed always before the inquiry of runtime->delay.

Takashi

+}

/*

• Set the effects parameters

*/ @@ -3468,6 +3551,9 @@ static int ca0132_effects_set(struct hda_codec *codec, hda_nid_t nid, long val) err = dspio_set_uint_param(codec, ca0132_effects[idx].mid, ca0132_effects[idx].reqs[0], on);

• ca0132_update_latency(codec, SNDRV_PCM_STREAM_PLAYBACK);
• ca0132_update_latency(codec, SNDRV_PCM_STREAM_CAPTURE);
• if (err < 0) return 0; /* no changed */

-- 1.7.4.1

At Mon, 25 Mar 2013 10:47:31 -0700, Dylan Reid wrote:

On Sun, Feb 10, 2013 at 2:51 AM, Takashi Iwai tiwai@suse.de wrote:

...

At Fri, 8 Feb 2013 18:31:44 -0800, Ian Minett wrote:

...

From: Ian Minett ian_minett@creativelabs.com

Add latency monitoring for playback, capture and DSP effects.

Please give a bit more descriptions. Looking at the patch, it doesn't implement only the latency monitoring but actually reflecting to the runtime->delay.

Also..

...

Signed-off-by: Ian Minett ian_minett@creativelabs.com

diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index b52f00c..c797f65 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -133,6 +133,12 @@ enum { /* Effects values size*/ #define EFFECT_VALS_MAX_COUNT 12

+#define DSP_CAPTURE_INIT_LATENCY 0 +#define DSP_CRYSTAL_VOICE_LATENCY 124 +#define DSP_PLAYBACK_INIT_LATENCY 13 +#define DSP_PLAY_ENHANCEMENT_LATENCY 30 +#define DSP_SPEAKER_OUT_LATENCY 7

struct ct_effect { char name[44]; hda_nid_t nid; @@ -2761,8 +2767,27 @@ static void ca0132_cleanup_stream(struct hda_codec *codec, hda_nid_t nid) }

/*

• • PCM callbacks

• • PCM playbacks
  */

+static unsigned int ca0132_get_playback_latency(struct hda_codec *codec) +{

• struct ca0132_spec *spec = codec->spec;
  

• unsigned int latency = DSP_PLAYBACK_INIT_LATENCY;
  

• if (!dspload_is_loaded(codec))
  

•         return 0;
  

• if (spec->effects_switch[PLAY_ENHANCEMENT - EFFECT_START_NID]) {
  

•         if ((spec->effects_switch[SURROUND - EFFECT_START_NID]) ||
  

•             (spec->effects_switch[DIALOG_PLUS - EFFECT_START_NID]))
  

•                 latency += DSP_PLAY_ENHANCEMENT_LATENCY;
  

• }
  

• if (spec->cur_out_type == SPEAKER_OUT)
  

•         latency += DSP_SPEAKER_OUT_LATENCY;
  

• return latency;
  

+}

static int ca0132_playback_pcm_prepare(struct hda_pcm_stream *hinfo, struct hda_codec *codec, unsigned int stream_tag, @@ -2770,6 +2795,16 @@ static int ca0132_playback_pcm_prepare(struct hda_pcm_stream *hinfo, struct snd_pcm_substream *substream) { struct ca0132_spec *spec = codec->spec;

• struct snd_pcm_runtime *runtime = substream->runtime;
  

• unsigned int latency = ca0132_get_playback_latency(codec);
  

• if (spec->dsp_state == DSP_DOWNLOADING) {
  

•         spec->dsp_stream_id = stream_tag;
  

•         return 0;
  

• }
  

• runtime->delay = bytes_to_frames(runtime, (latency * runtime->rate *
  

•                                 runtime->byte_align) / 1000);
  
  ca0132_setup_stream(codec, spec->dacs[0], stream_tag, 0, format);
  
  

@@ -2834,8 +2869,22 @@ static int ca0132_dig_playback_pcm_close(struct hda_pcm_stream *hinfo, }

/*

• • Analog capture

• • PCM capture
  */

+static unsigned int ca0132_get_capture_latency(struct hda_codec *codec) +{

• struct ca0132_spec *spec = codec->spec;
  

• unsigned int latency = DSP_CAPTURE_INIT_LATENCY;
  

• if (!dspload_is_loaded(codec))
  

•         return 0;
  

• if (spec->effects_switch[CRYSTAL_VOICE - EFFECT_START_NID])
  

•         latency += DSP_CRYSTAL_VOICE_LATENCY;
  

• return latency;
  

+}

static int ca0132_capture_pcm_prepare(struct hda_pcm_stream *hinfo, struct hda_codec *codec, unsigned int stream_tag, @@ -2843,9 +2892,16 @@ static int ca0132_capture_pcm_prepare(struct hda_pcm_stream *hinfo, struct snd_pcm_substream *substream) { struct ca0132_spec *spec = codec->spec;

• struct snd_pcm_runtime *runtime = substream->runtime;
  

• unsigned int latency = ca0132_get_capture_latency(codec);
  
  

• ca0132_setup_stream(codec, spec->adcs[substream->number],
  

•                     stream_tag, 0, format);
  

• if (spec->dsp_state == DSP_DOWNLOADING)
  

•         return 0;
  

• runtime->delay = bytes_to_frames(runtime, (latency * runtime->rate *
  

•                                 runtime->byte_align) / 1000);
  

• ca0132_setup_stream(codec, hinfo->nid, stream_tag, 0, format);
  
  return 0;
  

} @@ -3429,6 +3485,33 @@ static int ca0132_voicefx_set(struct hda_codec *codec, int enable) return 1; }

+static void ca0132_update_latency(struct hda_codec *codec,

•                           unsigned int direction)
  

+{

• struct ca0132_spec *spec = codec->spec;
  

• unsigned int latency;
  

• struct snd_pcm_substream *substr;
  

• struct snd_pcm_runtime *runtime;
  

• if (spec->dsp_state == DSP_DOWNLOADING)
  

•         return;
  

• latency = (direction == SNDRV_PCM_STREAM_PLAYBACK)
  

•                 ? ca0132_get_playback_latency(codec)
  

•                 : ca0132_get_capture_latency(codec);
  

• substr = codec->pcm_info->pcm->streams[direction].substream;
  

• while (substr) {
  

•         runtime = substr->runtime;
  

•         /* update latency only when frame_bits is set */
  

•         if (runtime && runtime->frame_bits)
  

•                 runtime->delay = bytes_to_frames(runtime,
  

•                                 (latency * runtime->rate *
  

•                                 runtime->byte_align) / 1000);
  

•         substr = substr->next;
  

• }
  

I guess this isn't needed. Usually the pointer callback is performed always before the inquiry of runtime->delay.

Thanks for pointing that out Takashi. This will have no effect, as azx_get_position now recalculates the delay based on LPIB position.

What was trying to be solved was that there is a large delay in the codec, and other codecs with integrated DSP. In this case it is greater than 100ms, depending on what effects are enabled. Is there a good was to add this delay to what is reported? Maybe allow the codec to specify a delay that is added to the calculation in azx_get_position?

I'm fine to add a new codec PCM ops for each azx_get_position call if it helps.

Takashi

At Tue, 2 Apr 2013 13:24:31 -0700, Dylan Reid wrote:

On Tue, Apr 2, 2013 at 2:30 AM, Takashi Iwai tiwai@suse.de wrote:

...

At Mon, 25 Mar 2013 10:47:31 -0700, Dylan Reid wrote:

...

On Sun, Feb 10, 2013 at 2:51 AM, Takashi Iwai tiwai@suse.de wrote:

...

At Fri, 8 Feb 2013 18:31:44 -0800, Ian Minett wrote:

...

From: Ian Minett ian_minett@creativelabs.com

Add latency monitoring for playback, capture and DSP effects.

Please give a bit more descriptions. Looking at the patch, it doesn't implement only the latency monitoring but actually reflecting to the runtime->delay.

Also..

...

Signed-off-by: Ian Minett ian_minett@creativelabs.com

diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index b52f00c..c797f65 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -133,6 +133,12 @@ enum { /* Effects values size*/ #define EFFECT_VALS_MAX_COUNT 12

+#define DSP_CAPTURE_INIT_LATENCY 0 +#define DSP_CRYSTAL_VOICE_LATENCY 124 +#define DSP_PLAYBACK_INIT_LATENCY 13 +#define DSP_PLAY_ENHANCEMENT_LATENCY 30 +#define DSP_SPEAKER_OUT_LATENCY 7

struct ct_effect { char name[44]; hda_nid_t nid; @@ -2761,8 +2767,27 @@ static void ca0132_cleanup_stream(struct hda_codec *codec, hda_nid_t nid) }

/*

• • PCM callbacks

• • PCM playbacks
  */

+static unsigned int ca0132_get_playback_latency(struct hda_codec *codec) +{

• struct ca0132_spec *spec = codec->spec;
  

• unsigned int latency = DSP_PLAYBACK_INIT_LATENCY;
  

• if (!dspload_is_loaded(codec))
  

•         return 0;
  

• if (spec->effects_switch[PLAY_ENHANCEMENT - EFFECT_START_NID]) {
  

•         if ((spec->effects_switch[SURROUND - EFFECT_START_NID]) ||
  

•             (spec->effects_switch[DIALOG_PLUS - EFFECT_START_NID]))
  

•                 latency += DSP_PLAY_ENHANCEMENT_LATENCY;
  

• }
  

• if (spec->cur_out_type == SPEAKER_OUT)
  

•         latency += DSP_SPEAKER_OUT_LATENCY;
  

• return latency;
  

+}

static int ca0132_playback_pcm_prepare(struct hda_pcm_stream *hinfo, struct hda_codec *codec, unsigned int stream_tag, @@ -2770,6 +2795,16 @@ static int ca0132_playback_pcm_prepare(struct hda_pcm_stream *hinfo, struct snd_pcm_substream *substream) { struct ca0132_spec *spec = codec->spec;

• struct snd_pcm_runtime *runtime = substream->runtime;
  

• unsigned int latency = ca0132_get_playback_latency(codec);
  

• if (spec->dsp_state == DSP_DOWNLOADING) {
  

•         spec->dsp_stream_id = stream_tag;
  

•         return 0;
  

• }
  

• runtime->delay = bytes_to_frames(runtime, (latency * runtime->rate *
  

•                                 runtime->byte_align) / 1000);
  
  ca0132_setup_stream(codec, spec->dacs[0], stream_tag, 0, format);
  
  

@@ -2834,8 +2869,22 @@ static int ca0132_dig_playback_pcm_close(struct hda_pcm_stream *hinfo, }

/*

• • Analog capture

• • PCM capture
  */

+static unsigned int ca0132_get_capture_latency(struct hda_codec *codec) +{

• struct ca0132_spec *spec = codec->spec;
  

• unsigned int latency = DSP_CAPTURE_INIT_LATENCY;
  

• if (!dspload_is_loaded(codec))
  

•         return 0;
  

• if (spec->effects_switch[CRYSTAL_VOICE - EFFECT_START_NID])
  

•         latency += DSP_CRYSTAL_VOICE_LATENCY;
  

• return latency;
  

+}

static int ca0132_capture_pcm_prepare(struct hda_pcm_stream *hinfo, struct hda_codec *codec, unsigned int stream_tag, @@ -2843,9 +2892,16 @@ static int ca0132_capture_pcm_prepare(struct hda_pcm_stream *hinfo, struct snd_pcm_substream *substream) { struct ca0132_spec *spec = codec->spec;

• struct snd_pcm_runtime *runtime = substream->runtime;
  

• unsigned int latency = ca0132_get_capture_latency(codec);
  
  

• ca0132_setup_stream(codec, spec->adcs[substream->number],
  

•                     stream_tag, 0, format);
  

• if (spec->dsp_state == DSP_DOWNLOADING)
  

•         return 0;
  

• runtime->delay = bytes_to_frames(runtime, (latency * runtime->rate *
  

•                                 runtime->byte_align) / 1000);
  

• ca0132_setup_stream(codec, hinfo->nid, stream_tag, 0, format);
  
  return 0;
  

} @@ -3429,6 +3485,33 @@ static int ca0132_voicefx_set(struct hda_codec *codec, int enable) return 1; }

+static void ca0132_update_latency(struct hda_codec *codec,

•                           unsigned int direction)
  

+{

• struct ca0132_spec *spec = codec->spec;
  

• unsigned int latency;
  

• struct snd_pcm_substream *substr;
  

• struct snd_pcm_runtime *runtime;
  

• if (spec->dsp_state == DSP_DOWNLOADING)
  

•         return;
  

• latency = (direction == SNDRV_PCM_STREAM_PLAYBACK)
  

•                 ? ca0132_get_playback_latency(codec)
  

•                 : ca0132_get_capture_latency(codec);
  

• substr = codec->pcm_info->pcm->streams[direction].substream;
  

• while (substr) {
  

•         runtime = substr->runtime;
  

•         /* update latency only when frame_bits is set */
  

•         if (runtime && runtime->frame_bits)
  

•                 runtime->delay = bytes_to_frames(runtime,
  

•                                 (latency * runtime->rate *
  

•                                 runtime->byte_align) / 1000);
  

•         substr = substr->next;
  

• }
  

I guess this isn't needed. Usually the pointer callback is performed always before the inquiry of runtime->delay.

Thanks for pointing that out Takashi. This will have no effect, as azx_get_position now recalculates the delay based on LPIB position.

What was trying to be solved was that there is a large delay in the codec, and other codecs with integrated DSP. In this case it is greater than 100ms, depending on what effects are enabled. Is there a good was to add this delay to what is reported? Maybe allow the codec to specify a delay that is added to the calculation in azx_get_position?

I'm fine to add a new codec PCM ops for each azx_get_position call if it helps.

That sounds like the right idea. I'll take a stab at it and post later today. Thinking something like "snd_hda_codec_get_processing_delay()" and a corresponding patch_op. Sound right?

Yes.

Takashi

At Fri, 8 Feb 2013 18:31:45 -0800, Ian Minett wrote:

From: Ian Minett ian_minett@creativelabs.com

Manage the delayed work queue in hp unsol event handler.

Could you explain why this is needed at all? We can't take a patch changing something without reasons.

thanks,

Takashi

Signed-off-by: Ian Minett ian_minett@creativelabs.com

diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index c797f65..61d74db 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -749,6 +749,9 @@ struct ca0132_spec { long voicefx_val; long cur_mic_boost;

• struct hda_codec *codec;
• struct delayed_work unsol_hp_work;

#ifdef ENABLE_TUNING_CONTROLS long cur_ctl_vals[TUNING_CTLS_COUNT]; #endif @@ -3338,6 +3341,14 @@ exit: return err < 0 ? err : 0; }

+static void ca0132_unsol_hp_delayed(struct work_struct *work) +{

• struct ca0132_spec *spec = container_of(

• to_delayed_work(work), struct ca0132_spec, unsol_hp_work);
  

• ca0132_select_out(spec->codec);
• snd_hda_jack_report_sync(spec->codec);

+}

static void ca0132_set_dmic(struct hda_codec *codec, int enable); static int ca0132_mic_boost_set(struct hda_codec *codec, long val); static int ca0132_effects_set(struct hda_codec *codec, hda_nid_t nid, long val); @@ -4540,8 +4551,9 @@ static void ca0132_process_dsp_response(struct hda_codec *codec)

static void ca0132_unsol_event(struct hda_codec *codec, unsigned int res) {

• snd_printdd(KERN_INFO "ca0132_unsol_event: 0x%x\n", res);

• struct ca0132_spec *spec = codec->spec;

• snd_printdd(KERN_INFO "ca0132_unsol_event: 0x%x\n", res);

  if (((res >> AC_UNSOL_RES_TAG_SHIFT) & 0x3f) == UNSOL_TAG_DSP) { ca0132_process_dsp_response(codec);

@@ -4553,8 +4565,10 @@ static void ca0132_unsol_event(struct hda_codec *codec, unsigned int res)

switch (res) {
case UNSOL_TAG_HP:

• 	ca0132_select_out(codec);
  

• 	snd_hda_jack_report_sync(codec);
  

• 	cancel_delayed_work_sync(&spec->unsol_hp_work);
  

• 	queue_delayed_work(codec->bus->workq,
  

• 			   &spec->unsol_hp_work,
  

• 			   msecs_to_jiffies(500));
  break;
  

  case UNSOL_TAG_AMIC1: ca0132_select_mic(codec);

@@ -4729,6 +4743,7 @@ static void ca0132_free(struct hda_codec *codec) { struct ca0132_spec *spec = codec->spec;

• cancel_delayed_work_sync(&spec->unsol_hp_work); snd_hda_power_up(codec); snd_hda_sequence_write(codec, spec->base_exit_verbs); ca0132_exit_chip(codec);

@@ -4794,6 +4809,7 @@ static int patch_ca0132(struct hda_codec *codec) if (!spec) return -ENOMEM; codec->spec = spec;

• spec->codec = codec;

  spec->num_mixers = 1; spec->mixers[0] = ca0132_mixer;

@@ -4804,6 +4820,8 @@ static int patch_ca0132(struct hda_codec *codec) spec->init_verbs[1] = ca0132_init_verbs1; spec->num_init_verbs = 2;

• INIT_DELAYED_WORK(&spec->unsol_hp_work, ca0132_unsol_hp_delayed);

• ca0132_init_chip(codec);

  ca0132_config(codec);

-- 1.7.4.1

On Fri, Mar 15, 2013 at 1:25 AM, Takashi Iwai tiwai@suse.de wrote:

At Thu, 14 Mar 2013 17:34:34 -0700, Dylan Reid wrote:

...

On Sun, Feb 10, 2013 at 2:57 AM, Takashi Iwai tiwai@suse.de wrote:

...

Hi Ian,

At Fri, 8 Feb 2013 18:31:41 -0800, Ian Minett wrote:

...

From: Ian Minett ian_minett@creativelabs.com

Hi,

This series contains the following updates to CA0132:

1: Add firmware loading for the SpeakerEQ DSP effect. 2: Improve DSP transfer timeout calculations. 3: Add stream and effect latency monitoring routines 4: Update hp unsolicited event handler to manage queued work.

Thanks, I took the one now but leave others. See my reply to each.

...

These were based on the latest sound.git, but please let me know if they should be against sound-unstable tree instead.

sound.git tree for-next branch would be the place to look at. If I start a new branch for ca0132, I'll let you know.

But, before going further: as I already mailed you, I could get a test board and started testing. The result wasn't good. At the first time the driver loads, the DSP loading fails always. When the driver is reloaded after that, it's working. So, something is still pretty fragile.

I tried this today and I can't get it to work. It is failing in snd_hda_lock_devices called from azx_load_dsp_prepare. The DSP is loaded from ca0132_init which is called as the result of open/resume. Both the check for open control files and attached substreams are failing. How can that be avoided?

snd_hda_lock_devices() is obviously not suitable for the power-saving or PM resume. OK, this must be fixed indeed...

How about the patch below? (Note that it's just compile tested.)

Thanks Takashi. This works for the init case, but it deadlocks in the case the load originates from open: azx_pcm_open -> calls snd_hda_power_up_d3wait -> ... -> ca0132_init -> azx_load_dsp_prepare

both open and load_dsp_prepare grab open_mutex.

thanks,

Takashi

---

From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Fix abuse of snd_hda_lock_devices() for DSP loader

The current DSP loader code abuses snd_hda_lock_devices() for ensuring the DSP loader not conflicting with the other normal operations. But this trick obviously doesn't work for the PM resume since the streams are kept opened there where snd_hda_lock_devices() returns -EBUSY. That means we need another lock mechanism instead of abuse.

This patch provides the new lock state to azx_dev. Theoretically it's possible that the DSP loader conflicts with the stream that has been already assigned for another PCM. If it's running, the DSP loader should simply fail. If not -- it's the case for PM resume --, we should assign this stream temporarily to the DSP loader, and take it back to the PCM after finishing DSP loading. If the PCM is operated during the DSP loading, it should get an error, too.

Reported-by: Dylan Reid dgreid@chromium.org Signed-off-by: Takashi Iwai tiwai@suse.de

---

sound/pci/hda/hda_intel.c | 119 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 100 insertions(+), 19 deletions(-)

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index 4cea6bb6..61abf8b 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -415,6 +415,8 @@ struct azx_dev { unsigned int opened :1; unsigned int running :1; unsigned int irq_pending :1;

•   unsigned int prepared:1;
  

•   unsigned int locked:1;
    /*
     * For VIA:
     *  A flag to ensure DMA position is 0
  

@@ -426,8 +428,25 @@ struct azx_dev {

    struct timecounter  azx_tc;
    struct cyclecounter azx_cc;

+#ifdef CONFIG_SND_HDA_DSP_LOADER

•   struct mutex dsp_mutex;
  

+#endif };

+/* DSP lock helpers */ +#ifdef CONFIG_SND_HDA_DSP_LOADER +#define dsp_lock_init(dev) mutex_init(&(dev)->dsp_mutex) +#define dsp_lock(dev) mutex_lock(&(dev)->dsp_mutex) +#define dsp_unlock(dev) mutex_unlock(&(dev)->dsp_mutex) +#define dsp_is_locked(dev) ((dev)->locked) +#else +#define dsp_lock_init(dev) do {} while (0) +#define dsp_lock(dev) do {} while (0) +#define dsp_unlock(dev) do {} while (0) +#define dsp_is_locked(dev) 0 +#endif

/* CORB/RIRB */ struct azx_rb { u32 *buf; /* CORB/RIRB buffer @@ -527,6 +546,10 @@ struct azx {

    /* card list (for power_save trigger) */
    struct list_head list;

+#ifdef CONFIG_SND_HDA_DSP_LOADER

•   struct azx_dev saved_azx_dev;
  

+#endif };

#define CREATE_TRACE_POINTS @@ -1794,7 +1817,8 @@ azx_assign_device(struct azx *chip, struct snd_pcm_substream *substream) nums = chip->capture_streams; } for (i = 0; i < nums; i++, dev++)

•           if (!chip->azx_dev[dev].opened) {
  

•           if (!chip->azx_dev[dev].opened &&
  

•               !dsp_is_locked(&chip->azx_dev[dev])) {
                    res = &chip->azx_dev[dev];
                    if (res->assigned_key == key)
                            break;
  

@@ -2009,6 +2033,12 @@ static int azx_pcm_hw_params(struct snd_pcm_substream *substream, struct azx_dev *azx_dev = get_azx_dev(substream); int ret;

•   dsp_lock(azx_dev);
  

•   if (dsp_is_locked(azx_dev)) {
  

•           ret = -EBUSY;
  

•           goto unlock;
  

•   }
  

•   mark_runtime_wc(chip, azx_dev, substream, false);
    azx_dev->bufsize = 0;
    azx_dev->period_bytes = 0;
  

@@ -2016,8 +2046,10 @@ static int azx_pcm_hw_params(struct snd_pcm_substream *substream, ret = snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(hw_params)); if (ret < 0)

•           return ret;
  

•           goto unlock;
    mark_runtime_wc(chip, azx_dev, substream, true);
  

• unlock:

•   dsp_unlock(azx_dev);
    return ret;
  

}

@@ -2029,16 +2061,21 @@ static int azx_pcm_hw_free(struct snd_pcm_substream *substream) struct hda_pcm_stream *hinfo = apcm->hinfo[substream->stream];

    /* reset BDL address */

•   azx_sd_writel(azx_dev, SD_BDLPL, 0);
  

•   azx_sd_writel(azx_dev, SD_BDLPU, 0);
  

•   azx_sd_writel(azx_dev, SD_CTL, 0);
  

•   azx_dev->bufsize = 0;
  

•   azx_dev->period_bytes = 0;
  

•   azx_dev->format_val = 0;
  

•   dsp_lock(azx_dev);
  

•   if (!dsp_is_locked(azx_dev)) {
  

•           azx_sd_writel(azx_dev, SD_BDLPL, 0);
  

•           azx_sd_writel(azx_dev, SD_BDLPU, 0);
  

•           azx_sd_writel(azx_dev, SD_CTL, 0);
  

•           azx_dev->bufsize = 0;
  

•           azx_dev->period_bytes = 0;
  

•           azx_dev->format_val = 0;
  

•   }
  
    snd_hda_codec_cleanup(apcm->codec, hinfo, substream);
  
    mark_runtime_wc(chip, azx_dev, substream, false);
  

•   azx_dev->prepared = 0;
  

•   dsp_unlock(azx_dev);
    return snd_pcm_lib_free_pages(substream);
  

}

@@ -2055,6 +2092,12 @@ static int azx_pcm_prepare(struct snd_pcm_substream *substream) snd_hda_spdif_out_of_nid(apcm->codec, hinfo->nid); unsigned short ctls = spdif ? spdif->ctls : 0;

•   dsp_lock(azx_dev);
  

•   if (dsp_is_locked(azx_dev)) {
  

•           err = -EBUSY;
  

•           goto unlock;
  

•   }
  

•   azx_stream_reset(chip, azx_dev);
    format_val = snd_hda_calc_stream_format(runtime->rate,
                                            runtime->channels,
  

@@ -2065,7 +2108,8 @@ static int azx_pcm_prepare(struct snd_pcm_substream *substream) snd_printk(KERN_ERR SFX "%s: invalid format_val, rate=%d, ch=%d, format=%d\n", pci_name(chip->pci), runtime->rate, runtime->channels, runtime->format);

•           return -EINVAL;
  

•           err = -EINVAL;
  

•           goto unlock;
    }
  
    bufsize = snd_pcm_lib_buffer_bytes(substream);
  

@@ -2084,7 +2128,7 @@ static int azx_pcm_prepare(struct snd_pcm_substream *substream) azx_dev->no_period_wakeup = runtime->no_period_wakeup; err = azx_setup_periods(chip, substream, azx_dev); if (err < 0)

•                   return err;
  

•                   goto unlock;
    }
  
    /* wallclk has 24Mhz clock source */
  

@@ -2101,8 +2145,14 @@ static int azx_pcm_prepare(struct snd_pcm_substream *substream) if ((chip->driver_caps & AZX_DCAPS_CTX_WORKAROUND) && stream_tag > chip->capture_streams) stream_tag -= chip->capture_streams;

•   return snd_hda_codec_prepare(apcm->codec, hinfo, stream_tag,
  

•   err = snd_hda_codec_prepare(apcm->codec, hinfo, stream_tag,
                                 azx_dev->format_val, substream);
  

• unlock:

•   if (!err)
  

•           azx_dev->prepared = 1;
  

•   dsp_unlock(azx_dev);
  

•   return err;
  

}

static int azx_pcm_trigger(struct snd_pcm_substream *substream, int cmd) @@ -2117,6 +2167,9 @@ static int azx_pcm_trigger(struct snd_pcm_substream *substream, int cmd) azx_dev = get_azx_dev(substream); trace_azx_pcm_trigger(chip, azx_dev, cmd);

•   if (!dsp_is_locked(azx_dev) || !azx_dev->prepared)
  

•           return -EPIPE;
  

•   switch (cmd) {
    case SNDRV_PCM_TRIGGER_START:
            rstart = 1;
  

@@ -2621,17 +2674,28 @@ static int azx_load_dsp_prepare(struct hda_bus *bus, unsigned int format, struct azx_dev *azx_dev; int err;

•   if (snd_hda_lock_devices(bus))
  

•           return -EBUSY;
  

•   azx_dev = azx_get_dsp_loader_dev(chip);
  

•   mutex_lock(&chip->open_mutex);
  

This mutex could already be held if this call originates from azx_pcm_open powering the codec back up.

•   dsp_lock(azx_dev);
  

•   spin_lock_irq(&chip->reg_lock);
  

•   if (azx_dev->running || azx_dev->locked) {
  

•           spin_unlock_irq(&chip->reg_lock);
  

•           err = -EBUSY;
  

•           goto unlock;
  

•   }
  

•   azx_dev->prepared = 0;
  

•   chip->saved_azx_dev = *azx_dev;
  

•   azx_dev->locked = 1;
  

•   spin_unlock_irq(&chip->reg_lock);
  
    err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV_SG,
                              snd_dma_pci_data(chip->pci),
                              byte_size, bufp);
    if (err < 0)
  

•           goto unlock;
  

•           goto err_alloc;
  
    mark_pages_wc(chip, bufp, true);
  

•   azx_dev = azx_get_dsp_loader_dev(chip);
    azx_dev->bufsize = byte_size;
    azx_dev->period_bytes = byte_size;
    azx_dev->format_val = format;
  

@@ -2649,13 +2713,21 @@ static int azx_load_dsp_prepare(struct hda_bus *bus, unsigned int format, goto error;

    azx_setup_controller(chip, azx_dev);

Does this need a dsp_unlock(azx_dev)?

•   mutex_unlock(&chip->open_mutex);
    return azx_dev->stream_tag;
  
  

  error: mark_pages_wc(chip, bufp, false); snd_dma_free_pages(bufp);

-unlock:

•   snd_hda_unlock_devices(bus);
  

• err_alloc:

•   spin_lock_irq(&chip->reg_lock);
  

•   if (azx_dev->opened)
  

•           *azx_dev = chip->saved_azx_dev;
  

•   azx_dev->locked = 0;
  

•   spin_unlock_irq(&chip->reg_lock);
  

• unlock:

•   dsp_unlock(azx_dev);
  

•   mutex_unlock(&chip->open_mutex);
    return err;
  

}

@@ -2677,9 +2749,11 @@ static void azx_load_dsp_cleanup(struct hda_bus *bus, struct azx *chip = bus->private_data; struct azx_dev *azx_dev = azx_get_dsp_loader_dev(chip);

•   if (!dmab->area)
  

•   if (!dmab->area || !azx_dev->locked)
            return;
  
  

•   mutex_lock(&chip->open_mutex);
  

•   dsp_lock(azx_dev);
    /* reset BDL address */
    azx_sd_writel(azx_dev, SD_BDLPL, 0);
    azx_sd_writel(azx_dev, SD_BDLPU, 0);
  

@@ -2692,7 +2766,13 @@ static void azx_load_dsp_cleanup(struct hda_bus *bus, snd_dma_free_pages(dmab); dmab->area = NULL;

•   snd_hda_unlock_devices(bus);
  

•   spin_lock_irq(&chip->reg_lock);
  

•   if (azx_dev->opened)
  

•           *azx_dev = chip->saved_azx_dev;
  

•   azx_dev->locked = 0;
  

•   spin_unlock_irq(&chip->reg_lock);
  

•   dsp_unlock(azx_dev);
  

•   mutex_unlock(&chip->open_mutex);
  

} #endif /* CONFIG_SND_HDA_DSP_LOADER */

@@ -3481,6 +3561,7 @@ static int azx_first_init(struct azx *chip) }

    for (i = 0; i < chip->num_streams; i++) {

•           dsp_lock_init(&chip->azx_dev[i]);
            /* allocate memory for the BDL for each stream */
            err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV,
                                      snd_dma_pci_data(chip->pci),
  

-- 1.8.1.4

On Fri, Mar 15, 2013 at 1:23 PM, Takashi Iwai tiwai@suse.de wrote:

At Fri, 15 Mar 2013 11:39:50 -0700, Dylan Reid wrote:

...

On Fri, Mar 15, 2013 at 1:25 AM, Takashi Iwai tiwai@suse.de wrote:

...

At Thu, 14 Mar 2013 17:34:34 -0700, Dylan Reid wrote:

...

On Sun, Feb 10, 2013 at 2:57 AM, Takashi Iwai tiwai@suse.de wrote:

...

Hi Ian,

At Fri, 8 Feb 2013 18:31:41 -0800, Ian Minett wrote:

...

From: Ian Minett ian_minett@creativelabs.com

Hi,

This series contains the following updates to CA0132:

1: Add firmware loading for the SpeakerEQ DSP effect. 2: Improve DSP transfer timeout calculations. 3: Add stream and effect latency monitoring routines 4: Update hp unsolicited event handler to manage queued work.

Thanks, I took the one now but leave others. See my reply to each.

...

These were based on the latest sound.git, but please let me know if they should be against sound-unstable tree instead.

sound.git tree for-next branch would be the place to look at. If I start a new branch for ca0132, I'll let you know.

But, before going further: as I already mailed you, I could get a test board and started testing. The result wasn't good. At the first time the driver loads, the DSP loading fails always. When the driver is reloaded after that, it's working. So, something is still pretty fragile.

I tried this today and I can't get it to work. It is failing in snd_hda_lock_devices called from azx_load_dsp_prepare. The DSP is loaded from ca0132_init which is called as the result of open/resume. Both the check for open control files and attached substreams are failing. How can that be avoided?

snd_hda_lock_devices() is obviously not suitable for the power-saving or PM resume. OK, this must be fixed indeed...

How about the patch below? (Note that it's just compile tested.)

Thanks Takashi. This works for the init case, but it deadlocks in the case the load originates from open: azx_pcm_open -> calls snd_hda_power_up_d3wait -> ... -> ca0132_init -> azx_load_dsp_prepare

both open and load_dsp_prepare grab open_mutex.

Right. The revised patch is below.

With the two changes I noted inline this works. I tested it at startup, open/close and suspend/resume.

Thanks for the speedy fixes!

Takashi

---

From: Takashi Iwai tiwai@suse.de Subject: [PATCH v2] ALSA: hda - Fix abuse of snd_hda_lock_devices() for DSP loader

The current DSP loader code abuses snd_hda_lock_devices() for ensuring the DSP loader not conflicting with the other normal operations. But this trick obviously doesn't work for the PM resume since the streams are kept opened there where snd_hda_lock_devices() returns -EBUSY. That means we need another lock mechanism instead of abuse.

This patch provides the new lock state to azx_dev. Theoretically it's possible that the DSP loader conflicts with the stream that has been already assigned for another PCM. If it's running, the DSP loader should simply fail. If not -- it's the case for PM resume --, we should assign this stream temporarily to the DSP loader, and take it back to the PCM after finishing DSP loading. If the PCM is operated during the DSP loading, it should get an error, too.

Reported-by: Dylan Reid dgreid@chromium.org Signed-off-by: Takashi Iwai tiwai@suse.de

---

sound/pci/hda/hda_intel.c | 132 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 109 insertions(+), 23 deletions(-)

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index 4cea6bb6..880bdc7 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -415,6 +415,8 @@ struct azx_dev { unsigned int opened :1; unsigned int running :1; unsigned int irq_pending :1;

•   unsigned int prepared:1;
  

•   unsigned int locked:1;
    /*
     * For VIA:
     *  A flag to ensure DMA position is 0
  

@@ -426,8 +428,25 @@ struct azx_dev {

    struct timecounter  azx_tc;
    struct cyclecounter azx_cc;

+#ifdef CONFIG_SND_HDA_DSP_LOADER

•   struct mutex dsp_mutex;
  

+#endif };

+/* DSP lock helpers */ +#ifdef CONFIG_SND_HDA_DSP_LOADER +#define dsp_lock_init(dev) mutex_init(&(dev)->dsp_mutex) +#define dsp_lock(dev) mutex_lock(&(dev)->dsp_mutex) +#define dsp_unlock(dev) mutex_unlock(&(dev)->dsp_mutex) +#define dsp_is_locked(dev) (dev)->locked +#else +#define dsp_lock_init(dev) do {} while (0) +#define dsp_lock(dev) do {} while (0) +#define dsp_unlock(dev) do {} while (0) +#define dsp_is_locked(dev) 0 +#endif

/* CORB/RIRB */ struct azx_rb { u32 *buf; /* CORB/RIRB buffer @@ -527,6 +546,10 @@ struct azx {

    /* card list (for power_save trigger) */
    struct list_head list;

+#ifdef CONFIG_SND_HDA_DSP_LOADER

•   struct azx_dev saved_azx_dev;
  

+#endif };

#define CREATE_TRACE_POINTS @@ -1793,15 +1816,25 @@ azx_assign_device(struct azx *chip, struct snd_pcm_substream *substream) dev = chip->capture_index_offset; nums = chip->capture_streams; }

•   for (i = 0; i < nums; i++, dev++)
  

•           if (!chip->azx_dev[dev].opened) {
  

•                   res = &chip->azx_dev[dev];
  

•                   if (res->assigned_key == key)
  

•                           break;
  

•   for (i = 0; i < nums; i++, dev++) {
  

•           struct azx_dev *azx_dev = &chip->azx_dev[dev];
  

•           dsp_lock(azx_dev);
  

•           if (!azx_dev->opened && !dsp_is_locked(azx_dev)) {
  

•                   res = azx_dev;
  

•                   if (res->assigned_key == key) {
  

•                           res->opened = 1;
  

•                           res->assigned_key = key;
  

•                           dsp_unlock(azx_dev);
  

•                           return res;
  

•                   }
            }
  

•           dsp_unlock(azx_dev);
  

•   }
    if (res) {
  

•           dsp_lock(res);
            res->opened = 1;
            res->assigned_key = key;
  

•           dsp_unlock(res);
    }
    return res;
  

} @@ -2009,6 +2042,12 @@ static int azx_pcm_hw_params(struct snd_pcm_substream *substream, struct azx_dev *azx_dev = get_azx_dev(substream); int ret;

•   dsp_lock(azx_dev);
  

•   if (dsp_is_locked(azx_dev)) {
  

•           ret = -EBUSY;
  

•           goto unlock;
  

•   }
  

•   mark_runtime_wc(chip, azx_dev, substream, false);
    azx_dev->bufsize = 0;
    azx_dev->period_bytes = 0;
  

@@ -2016,8 +2055,10 @@ static int azx_pcm_hw_params(struct snd_pcm_substream *substream, ret = snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(hw_params)); if (ret < 0)

•           return ret;
  

•           goto unlock;
    mark_runtime_wc(chip, azx_dev, substream, true);
  

• unlock:

•   dsp_unlock(azx_dev);
    return ret;
  

}

@@ -2029,16 +2070,21 @@ static int azx_pcm_hw_free(struct snd_pcm_substream *substream) struct hda_pcm_stream *hinfo = apcm->hinfo[substream->stream];

    /* reset BDL address */

•   azx_sd_writel(azx_dev, SD_BDLPL, 0);
  

•   azx_sd_writel(azx_dev, SD_BDLPU, 0);
  

•   azx_sd_writel(azx_dev, SD_CTL, 0);
  

•   azx_dev->bufsize = 0;
  

•   azx_dev->period_bytes = 0;
  

•   azx_dev->format_val = 0;
  

•   dsp_lock(azx_dev);
  

•   if (!dsp_is_locked(azx_dev)) {
  

•           azx_sd_writel(azx_dev, SD_BDLPL, 0);
  

•           azx_sd_writel(azx_dev, SD_BDLPU, 0);
  

•           azx_sd_writel(azx_dev, SD_CTL, 0);
  

•           azx_dev->bufsize = 0;
  

•           azx_dev->period_bytes = 0;
  

•           azx_dev->format_val = 0;
  

•   }
  
    snd_hda_codec_cleanup(apcm->codec, hinfo, substream);
  
    mark_runtime_wc(chip, azx_dev, substream, false);
  

•   azx_dev->prepared = 0;
  

•   dsp_unlock(azx_dev);
    return snd_pcm_lib_free_pages(substream);
  

}

@@ -2055,6 +2101,12 @@ static int azx_pcm_prepare(struct snd_pcm_substream *substream) snd_hda_spdif_out_of_nid(apcm->codec, hinfo->nid); unsigned short ctls = spdif ? spdif->ctls : 0;

•   dsp_lock(azx_dev);
  

•   if (dsp_is_locked(azx_dev)) {
  

•           err = -EBUSY;
  

•           goto unlock;
  

•   }
  

•   azx_stream_reset(chip, azx_dev);
    format_val = snd_hda_calc_stream_format(runtime->rate,
                                            runtime->channels,
  

@@ -2065,7 +2117,8 @@ static int azx_pcm_prepare(struct snd_pcm_substream *substream) snd_printk(KERN_ERR SFX "%s: invalid format_val, rate=%d, ch=%d, format=%d\n", pci_name(chip->pci), runtime->rate, runtime->channels, runtime->format);

•           return -EINVAL;
  

•           err = -EINVAL;
  

•           goto unlock;
    }
  
    bufsize = snd_pcm_lib_buffer_bytes(substream);
  

@@ -2084,7 +2137,7 @@ static int azx_pcm_prepare(struct snd_pcm_substream *substream) azx_dev->no_period_wakeup = runtime->no_period_wakeup; err = azx_setup_periods(chip, substream, azx_dev); if (err < 0)

•                   return err;
  

•                   goto unlock;
    }
  
    /* wallclk has 24Mhz clock source */
  

@@ -2101,8 +2154,14 @@ static int azx_pcm_prepare(struct snd_pcm_substream *substream) if ((chip->driver_caps & AZX_DCAPS_CTX_WORKAROUND) && stream_tag > chip->capture_streams) stream_tag -= chip->capture_streams;

•   return snd_hda_codec_prepare(apcm->codec, hinfo, stream_tag,
  

•   err = snd_hda_codec_prepare(apcm->codec, hinfo, stream_tag,
                                 azx_dev->format_val, substream);
  

• unlock:

•   if (!err)
  

•           azx_dev->prepared = 1;
  

•   dsp_unlock(azx_dev);
  

•   return err;
  

}

static int azx_pcm_trigger(struct snd_pcm_substream *substream, int cmd) @@ -2117,6 +2176,9 @@ static int azx_pcm_trigger(struct snd_pcm_substream *substream, int cmd) azx_dev = get_azx_dev(substream); trace_azx_pcm_trigger(chip, azx_dev, cmd);

•   if (!dsp_is_locked(azx_dev) || !azx_dev->prepared)
  

I inverted this check. I think the intent here is that either the dsp load is active or a substream is prepared. If that's true this should return when both are false.

•           return -EPIPE;
  

•   switch (cmd) {
    case SNDRV_PCM_TRIGGER_START:
            rstart = 1;
  

@@ -2621,17 +2683,27 @@ static int azx_load_dsp_prepare(struct hda_bus *bus, unsigned int format, struct azx_dev *azx_dev; int err;

•   if (snd_hda_lock_devices(bus))
  

•           return -EBUSY;
  

•   azx_dev = azx_get_dsp_loader_dev(chip);
  

•   dsp_lock(azx_dev);
  

•   spin_lock_irq(&chip->reg_lock);
  

•   if (azx_dev->running || azx_dev->locked) {
  

•           spin_unlock_irq(&chip->reg_lock);
  

•           err = -EBUSY;
  

•           goto unlock;
  

•   }
  

•   azx_dev->prepared = 0;
  

•   chip->saved_azx_dev = *azx_dev;
  

•   azx_dev->locked = 1;
  

•   spin_unlock_irq(&chip->reg_lock);
  
    err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV_SG,
                              snd_dma_pci_data(chip->pci),
                              byte_size, bufp);
    if (err < 0)
  

•           goto unlock;
  

•           goto err_alloc;
  
    mark_pages_wc(chip, bufp, true);
  

•   azx_dev = azx_get_dsp_loader_dev(chip);
    azx_dev->bufsize = byte_size;
    azx_dev->period_bytes = byte_size;
    azx_dev->format_val = format;
  

@@ -2649,13 +2721,20 @@ static int azx_load_dsp_prepare(struct hda_bus *bus, unsigned int format, goto error;

    azx_setup_controller(chip, azx_dev);

I added dsp_unlock(azx_dev); here.

•   mutex_unlock(&chip->open_mutex);
    return azx_dev->stream_tag;
  
  

  error: mark_pages_wc(chip, bufp, false); snd_dma_free_pages(bufp);

-unlock:

•   snd_hda_unlock_devices(bus);
  

• err_alloc:

•   spin_lock_irq(&chip->reg_lock);
  

•   if (azx_dev->opened)
  

•           *azx_dev = chip->saved_azx_dev;
  

•   azx_dev->locked = 0;
  

•   spin_unlock_irq(&chip->reg_lock);
  

• unlock:

•   dsp_unlock(azx_dev);
    return err;
  

}

@@ -2677,9 +2756,10 @@ static void azx_load_dsp_cleanup(struct hda_bus *bus, struct azx *chip = bus->private_data; struct azx_dev *azx_dev = azx_get_dsp_loader_dev(chip);

•   if (!dmab->area)
  

•   if (!dmab->area || !azx_dev->locked)
            return;
  
  

•   dsp_lock(azx_dev);
    /* reset BDL address */
    azx_sd_writel(azx_dev, SD_BDLPL, 0);
    azx_sd_writel(azx_dev, SD_BDLPU, 0);
  

@@ -2692,7 +2772,12 @@ static void azx_load_dsp_cleanup(struct hda_bus *bus, snd_dma_free_pages(dmab); dmab->area = NULL;

•   snd_hda_unlock_devices(bus);
  

•   spin_lock_irq(&chip->reg_lock);
  

•   if (azx_dev->opened)
  

•           *azx_dev = chip->saved_azx_dev;
  

•   azx_dev->locked = 0;
  

•   spin_unlock_irq(&chip->reg_lock);
  

•   dsp_unlock(azx_dev);
  

} #endif /* CONFIG_SND_HDA_DSP_LOADER */

@@ -3481,6 +3566,7 @@ static int azx_first_init(struct azx *chip) }

    for (i = 0; i < chip->num_streams; i++) {

•           dsp_lock_init(&chip->azx_dev[i]);
            /* allocate memory for the BDL for each stream */
            err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV,
                                      snd_dma_pci_data(chip->pci),
  

-- 1.8.2