Re: [PATCH 17/53] ALSA: x86: Drop unused hw_silence field

From: "Ughreja, Rakesh A" <rakesh.a.ughreja@intel.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	Takashi Iwai <tiwai@suse.de>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	"Anand, Jerome" <jerome.anand@intel.com>
Subject: Re: [PATCH 17/53] ALSA: x86: Drop unused hw_silence field
Date: Fri, 3 Feb 2017 04:25:43 +0000	[thread overview]
Message-ID: <85DFEED57DC57344B2483EF7BF8CB60552CD1551@BGSMSX104.gar.corp.intel.com> (raw)
In-Reply-To: <1cc13469-6060-71bd-c10b-2e60ed4d4b0f@linux.intel.com>

[-- Attachment #1: Type: text/plain, Size: 1835 bytes --]



>-----Original Message-----
>From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-
>project.org] On Behalf Of Pierre-Louis Bossart
>Sent: Friday, February 3, 2017 1:57 AM
>To: Takashi Iwai <tiwai@suse.de>
>Cc: alsa-devel@alsa-project.org; Anand, Jerome <jerome.anand@intel.com>
>Subject: Re: [alsa-devel] [PATCH 17/53] ALSA: x86: Drop unused hw_silence
>field
>
>
>
>On 02/02/2017 02:04 PM, Takashi Iwai wrote:
>> On Thu, 02 Feb 2017 20:11:10 +0100,
>> Pierre-Louis Bossart wrote:
>>> On 2/2/17 11:02 AM, Takashi Iwai wrote:
>>>> It's nowhere used.  Let's drop it.
>>> That's one of the things I wanted to add. When you start playback
>>> there is a 1-2s period where the receiver is still looking at the data
>>> to figure out if it's PCM or compressed, and you lose those 1-2s of
>>> audio. The hw silence mode is a way to keep the link active.
>> I see.  Yes, the feature would be useful.
>>
>>> But I guess we can remove first and re-add later so no objections.
>> Agreed.
>>
>> How would it be actually implemented?  Just avoid to turn off the
>> audio config bits at trigger callback, etc?
>>
>> Since the runtime PM will be in the same game, we'll have to cope with
>> it, too.
>I must admit I don't remember the details, just that's it's the same
>behavior as the HDAudio KAE (Keep Alive Enable) functionality.
>

I have this patch from the past. It won't apply with the recent changes
in the code. But the silence filling code can be used from there.

The idea was to use hardware functionality to generate silence
when the hotplug event is detected.


>>
>>
>> thanks,
>>
>> Takashi
>
>_______________________________________________
>Alsa-devel mailing list
>Alsa-devel@alsa-project.org
>http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[-- Attachment #2: 0001-Audio-Silence-fix.patch --]
[-- Type: application/octet-stream, Size: 15718 bytes --]

From 85ea8eb633e537cf2c4afdac636ed462e23a80d0 Mon Sep 17 00:00:00 2001
From: kumaran <kumaran@kumaran-ubuntu.(none)>
Date: Wed, 10 Sep 2014 15:39:55 +0530
Subject: [PATCH] Audio" Silence fix Signed-off-by: kumaran
 <kumaran.s@intel.com>

---
 .../intel_media/hdmi_audio/intel_mid_hdmi_audio.c  |  175 +++++++++++---------
 .../intel_media/hdmi_audio/intel_mid_hdmi_audio.h  |   19 ++-
 .../hdmi_audio/intel_mid_hdmi_audio_if.c           |    9 +-
 3 files changed, 120 insertions(+), 83 deletions(-)

diff --git a/drivers/external_drivers/intel_media/hdmi_audio/intel_mid_hdmi_audio.c b/drivers/external_drivers/intel_media/hdmi_audio/intel_mid_hdmi_audio.c
index ca9d92c..6e308f0 100755
--- a/drivers/external_drivers/intel_media/hdmi_audio/intel_mid_hdmi_audio.c
+++ b/drivers/external_drivers/intel_media/hdmi_audio/intel_mid_hdmi_audio.c
@@ -424,12 +424,11 @@ static void snd_intelhad_reset_audio_v1(u8 reset)
  *
  * This function is called in the prepare callback
  */
-static int had_prog_status_reg(struct snd_pcm_substream *substream,
-			struct snd_intelhad *intelhaddata)
+static int had_prog_status_reg(struct snd_intelhad *intelhaddata)
 {
 	union aud_ch_status_0 ch_stat0 = {.status_0_regval = 0};
 	union aud_ch_status_1 ch_stat1 = {.status_1_regval = 0};
-	int format;
+	u32 format;
 
 	pr_debug("Entry %s\n", __func__);
 
@@ -437,7 +436,7 @@ static int had_prog_status_reg(struct snd_pcm_substream *substream,
 						IEC958_AES0_NONAUDIO)>>1;
 	ch_stat0.status_0_regx.clk_acc = (intelhaddata->aes_bits &
 						IEC958_AES3_CON_CLOCK)>>4;
-	switch (substream->runtime->rate) {
+	switch (intelhaddata->rate) {
 	case AUD_SAMPLE_RATE_32:
 		ch_stat0.status_0_regx.samp_freq = CH_STATUS_MAP_32KHZ;
 		break;
@@ -469,8 +468,7 @@ static int had_prog_status_reg(struct snd_pcm_substream *substream,
 	}
 	had_write_register(AUD_CH_STATUS_0, ch_stat0.status_0_regval);
 
-	format = substream->runtime->format;
-
+	format = intelhaddata->format;
 	if (format == SNDRV_PCM_FORMAT_S16_LE) {
 		ch_stat1.status_1_regx.max_wrd_len = MAX_SMPL_WIDTH_20;
 		ch_stat1.status_1_regx.wrd_len = SMPL_WIDTH_16BITS;
@@ -486,41 +484,6 @@ static int had_prog_status_reg(struct snd_pcm_substream *substream,
 }
 
 /**
- * snd_intelhad_prog_audio_ctrl_v2 - to initialize audio
- * registers and buffer confgiuration registers
- *
- * @substream:substream for which the prepare function is called
- * @intelhaddata:substream private data
- *
- * This function is called in the prepare callback
- */
-int snd_intelhad_prog_audio_ctrl_v2(struct snd_pcm_substream *substream,
-					struct snd_intelhad *intelhaddata)
-{
-	union aud_cfg cfg_val = {.cfg_regval = 0};
-	union aud_buf_config buf_cfg = {.buf_cfgval = 0};
-	u8 channels;
-
-	had_prog_status_reg(substream, intelhaddata);
-
-	buf_cfg.buf_cfg_regx_v2.audio_fifo_watermark = FIFO_THRESHOLD;
-	buf_cfg.buf_cfg_regx_v2.dma_fifo_watermark = DMA_FIFO_THRESHOLD;
-	buf_cfg.buf_cfg_regx_v2.aud_delay = 0;
-	had_write_register(AUD_BUF_CONFIG, buf_cfg.buf_cfgval);
-
-	channels = substream->runtime->channels;
-	cfg_val.cfg_regx_v2.num_ch = channels - 2;
-	if (channels <= 2)
-		cfg_val.cfg_regx_v2.layout = LAYOUT0;
-	else
-		cfg_val.cfg_regx_v2.layout = LAYOUT1;
-
-	cfg_val.cfg_regx_v2.val_bit = 1;
-	had_write_register(AUD_CONFIG, cfg_val.cfg_regval);
-	return 0;
-}
-
-/**
  * snd_intelhad_prog_audio_ctrl_v1 - to initialize audio
  * registers and buffer confgiuration registers
  *
@@ -529,22 +492,18 @@ int snd_intelhad_prog_audio_ctrl_v2(struct snd_pcm_substream *substream,
  *
  * This function is called in the prepare callback
  */
-int snd_intelhad_prog_audio_ctrl_v1(struct snd_pcm_substream *substream,
-					struct snd_intelhad *intelhaddata)
+int snd_intelhad_prog_audio_ctrl_v1(struct snd_intelhad *intelhaddata)
 {
 	union aud_cfg cfg_val = {.cfg_regval = 0};
 	union aud_buf_config buf_cfg = {.buf_cfgval = 0};
-	u8 channels;
 
-	had_prog_status_reg(substream, intelhaddata);
+	had_prog_status_reg(intelhaddata);
 
 	buf_cfg.buf_cfg_regx.fifo_width = FIFO_THRESHOLD;
 	buf_cfg.buf_cfg_regx.aud_delay = 0;
 	had_write_register(AUD_BUF_CONFIG, buf_cfg.buf_cfgval);
 
-	channels = substream->runtime->channels;
-
-	switch (channels) {
+	switch (intelhaddata->channels) {
 	case 1:
 	case 2:
 		cfg_val.cfg_regx.num_ch = CH_STEREO;
@@ -575,6 +534,21 @@ int snd_intelhad_prog_audio_ctrl_v1(struct snd_pcm_substream *substream,
 	had_write_register(AUD_CONFIG, cfg_val.cfg_regval);
 	return 0;
 }
+
+/**
+ * snd_intelhad_prog_silence_v1 - to enable silence stream
+ *
+ * @substream: Current substream or NULL if no active substream.
+ * @enable: 1 if audio is to be enabled; 0 if audio is to be disabled.
+ */
+static void snd_intelhad_prog_silence_v1(struct snd_pcm_substream *substream,
+					u8 enable)
+{
+	had_read_modify(AUD_CONFIG, (enable << 8) , BIT(8));
+	had_read_modify(AUD_CONFIG, (enable << 11) , BIT(11));
+
+}
+
 /*
  * Compute derived values in channel_allocations[].
  */
@@ -1165,24 +1139,20 @@ static int had_create_audio_ctls(struct snd_card *card, struct snd_intelhad *int
  *
  * This function is called in the prepare callback
  */
-static void snd_intelhad_prog_dip_v1(struct snd_pcm_substream *substream,
-				struct snd_intelhad *intelhaddata)
+static void snd_intelhad_prog_dip_v1(struct snd_intelhad *intelhaddata)
 {
 	int i;
 	union aud_ctrl_st ctrl_state = {.ctrl_val = 0};
 	union aud_info_frame2 frame2 = {.fr2_val = 0};
 	union aud_info_frame3 frame3 = {.fr3_val = 0};
 	u8 checksum = 0;
-	int channels;
-
-	channels = substream->runtime->channels;
 
 	had_write_register(AUD_CNTL_ST, ctrl_state.ctrl_val);
 
-	frame2.fr2_regx.chnl_cnt = substream->runtime->channels - 1;
+	frame2.fr2_regx.chnl_cnt = intelhaddata->channels - 1;
 
 	frame3.fr3_regx.chnl_alloc = snd_intelhad_channel_allocation(
-					intelhaddata, channels);
+					intelhaddata, intelhaddata->channels);
 
 	/*Calculte the byte wide checksum for all valid DIP words*/
 	for (i = 0; i < BYTES_PER_WORD; i++)
@@ -1294,12 +1264,10 @@ inline int snd_intelhad_read_len(struct snd_intelhad *intelhaddata)
  * @aud_samp_freq: sampling frequency of audio data
  * @tmds: sampling frequency of the display data
  * @n_param: N value, depends on aud_samp_freq
- * @intelhaddata:substream private data
  *
  * Program CTS register based on the audio and display sampling frequency
  */
-static void snd_intelhad_prog_cts_v1(u32 aud_samp_freq, u32 tmds, u32 n_param,
-				struct snd_intelhad *intelhaddata)
+static void snd_intelhad_prog_cts_v1(u32 aud_samp_freq, u32 tmds, u32 n_param)
 {
 	u32 cts_val;
 	u64 dividend, divisor;
@@ -1364,8 +1332,7 @@ static int had_calculate_n_value(u32 aud_samp_freq)
  * This function is called in the prepare callback.
  * It programs based on the audio and display sampling frequency
  */
-static int snd_intelhad_prog_n_v1(u32 aud_samp_freq, u32 *n_param,
-				struct snd_intelhad *intelhaddata)
+static int snd_intelhad_prog_n_v1(u32 aud_samp_freq, u32 *n_param)
 {
 	s32 n_val;
 
@@ -1637,6 +1604,7 @@ static int snd_intelhad_hw_free(struct snd_pcm_substream *substream)
 * @cmd:the stream commamd thats requested from upper layer
 * This function is called whenever an a stream activity is invoked
 */
+
 static int snd_intelhad_pcm_trigger(struct snd_pcm_substream *substream,
 					int cmd)
 {
@@ -1673,6 +1641,7 @@ static int snd_intelhad_pcm_trigger(struct snd_pcm_substream *substream,
 		caps = HDMI_AUDIO_BUFFER_DONE;
 		retval = had_set_caps(HAD_SET_ENABLE_AUDIO_INT, &caps);
 		retval = had_set_caps(HAD_SET_ENABLE_AUDIO, NULL);
+
 		intelhaddata->ops->enable_audio(substream, 1);
 
 		pr_debug("Processed _Start\n");
@@ -1689,18 +1658,16 @@ static int snd_intelhad_pcm_trigger(struct snd_pcm_substream *substream,
 
 		had_stream->stream_type = HAD_INIT;
 		spin_unlock_irqrestore(&intelhaddata->had_spinlock, flag_irq);
-		/* Disable Audio */
-		/* ToDo: Need to disable UNDERRUN interrupts as well
-		   caps = HDMI_AUDIO_UNDERRUN | HDMI_AUDIO_BUFFER_DONE;
-		   */
+
 		caps = HDMI_AUDIO_BUFFER_DONE;
 		had_set_caps(HAD_SET_DISABLE_AUDIO_INT, &caps);
-		intelhaddata->ops->enable_audio(substream, 0);
+
 		/* Reset buffer pointers */
 		intelhaddata->ops->reset_audio(1);
 		intelhaddata->ops->reset_audio(0);
+
 		stream->stream_status = STREAM_DROPPED;
-		had_set_caps(HAD_SET_DISABLE_AUDIO, NULL);
+
 		break;
 
 	default:
@@ -1756,6 +1723,9 @@ static int snd_intelhad_pcm_prepare(struct snd_pcm_substream *substream)
 	if (retval)
 		goto prep_end;
 
+	intelhaddata->channels = substream->runtime->channels;
+	intelhaddata->rate = substream->runtime->rate;
+	intelhaddata->format = substream->runtime->format;
 
 	/* Get N value in KHz */
 	retval = had_get_caps(HAD_GET_SAMPLING_FREQ, &disp_samp_freq);
@@ -1766,18 +1736,19 @@ static int snd_intelhad_pcm_prepare(struct snd_pcm_substream *substream)
 
 	had_get_caps(HAD_GET_ELD, &intelhaddata->eeld);
 
-	retval = intelhaddata->ops->prog_n(substream->runtime->rate, &n_param,
-								intelhaddata);
+	retval = intelhaddata->ops->prog_n(substream->runtime->rate, &n_param);
 	if (retval) {
 		pr_err("programming N value failed %#x\n", retval);
 		goto prep_end;
 	}
 	intelhaddata->ops->prog_cts(substream->runtime->rate,
-					disp_samp_freq, n_param, intelhaddata);
+					disp_samp_freq, n_param);
+
+	intelhaddata->ops->prog_dip(intelhaddata);
 
-	intelhaddata->ops->prog_dip(substream, intelhaddata);
+	retval = intelhaddata->ops->audio_ctrl(intelhaddata);
 
-	retval = intelhaddata->ops->audio_ctrl(substream, intelhaddata);
+	intelhaddata->ops->silence(substream, 1);
 
 	/* Prog buffer address */
 	retval = snd_intelhad_prog_buffer(intelhaddata,
@@ -1793,6 +1764,59 @@ prep_end:
 }
 
 /**
+ * snd_intelhad_pcm_prepare- internal preparation before starting a stream
+ * @substream:  substream for which the function is called
+ * This function is called when a stream is started for internal preparation.
+ * */
+
+int snd_intelhad_set_silence(void)
+{
+	int retval, caps;
+	struct snd_intelhad *intelhaddata = had_data;
+	u32 disp_samp_freq, n_param;
+
+	pr_debug("Set silence stream called\n");
+
+	/* Get N value in KHz */
+	retval = had_get_caps(HAD_GET_SAMPLING_FREQ, &disp_samp_freq);
+	if (retval) {
+		pr_err("querying display sampling freq failed %#x\n", retval);
+		goto prep_end;
+	}
+
+	had_get_caps(HAD_GET_ELD, &intelhaddata->eeld);
+
+	retval = intelhaddata->ops->prog_n(intelhaddata->rate, &n_param);
+	if (retval) {
+		pr_err("programming N value failed %#x\n", retval);
+		goto prep_end;
+	}
+	intelhaddata->ops->prog_cts(intelhaddata->rate,
+			disp_samp_freq, n_param);
+
+
+	intelhaddata->ops->prog_dip(intelhaddata);
+
+	retval = intelhaddata->ops->audio_ctrl(intelhaddata);
+
+	/* Reset buffer pointers */
+	intelhaddata->ops->reset_audio(1);
+	intelhaddata->ops->reset_audio(0);
+
+	had_write_register(AUD_BUF_CH_SWAP, SWAP_LFE_CENTER);
+
+	intelhaddata->ops->silence(NULL, 1);
+
+	caps = HDMI_AUDIO_BUFFER_DONE;
+	retval = had_set_caps(HAD_SET_ENABLE_AUDIO_INT, &caps);
+  	retval = had_set_caps(HAD_SET_ENABLE_AUDIO, NULL);
+
+	intelhaddata->ops->enable_audio(NULL, 1);
+
+prep_end:
+	return retval;
+}
+/**
  * snd_intelhad_pcm_pointer- to send the current buffer pointerprocessed by hw
  *
  * @substream:  substream for which the function is called
@@ -1850,6 +1874,8 @@ int hdmi_audio_mode_change(struct snd_pcm_substream *substream)
 	u32 disp_samp_freq, n_param;
 	struct snd_intelhad *intelhaddata;
 
+	pr_debug("entring %s\n", __func__);
+
 	intelhaddata = snd_pcm_substream_chip(substream);
 
 	/* Disable Audio */
@@ -1862,14 +1888,14 @@ int hdmi_audio_mode_change(struct snd_pcm_substream *substream)
 		goto out;
 	}
 
-	retval = intelhaddata->ops->prog_n(substream->runtime->rate, &n_param,
-								intelhaddata);
+	retval = intelhaddata->ops->prog_n(substream->runtime->rate, &n_param);
+
 	if (retval) {
 		pr_err("programming N value failed %#x\n", retval);
 		goto out;
 	}
 	intelhaddata->ops->prog_cts(substream->runtime->rate,
-					disp_samp_freq, n_param, intelhaddata);
+					disp_samp_freq, n_param);
 
 	/* Enable Audio */
 	intelhaddata->ops->enable_audio(substream, 1);
@@ -2000,6 +2026,7 @@ static struct had_ops had_ops_v1 = {
 	.prog_n =	snd_intelhad_prog_n_v1,
 	.prog_cts =	snd_intelhad_prog_cts_v1,
 	.audio_ctrl =	snd_intelhad_prog_audio_ctrl_v1,
+	.silence =	snd_intelhad_prog_silence_v1,
 	.prog_dip =	snd_intelhad_prog_dip_v1,
 	.handle_underrun =  had_clear_underrun_intr_v1,
 };
diff --git a/drivers/external_drivers/intel_media/hdmi_audio/intel_mid_hdmi_audio.h b/drivers/external_drivers/intel_media/hdmi_audio/intel_mid_hdmi_audio.h
index ea37420..102e686 100644
--- a/drivers/external_drivers/intel_media/hdmi_audio/intel_mid_hdmi_audio.h
+++ b/drivers/external_drivers/intel_media/hdmi_audio/intel_mid_hdmi_audio.h
@@ -568,6 +568,11 @@ struct snd_intelhad {
 	enum		intel_had_aud_buf_type buff_done;
 	struct device *dev;
 	struct snd_kcontrol *kctl;
+
+	u32		channels;
+	u32		rate;
+	u32		format;
+
 	struct snd_pcm_chmap *chmap;
 	unsigned int	audio_reg_base;
 	bool		hw_silence;
@@ -580,14 +585,11 @@ struct had_ops {
 	void (*enable_audio) (struct snd_pcm_substream *substream,
 			u8 enable);
 	void (*reset_audio) (u8 reset);
-	int (*prog_n) (u32 aud_samp_freq, u32 *n_param,
-			struct snd_intelhad *intelhaddata);
-	void (*prog_cts) (u32 aud_samp_freq, u32 tmds, u32 n_param,
-			struct snd_intelhad *intelhaddata);
-	int (*audio_ctrl) (struct snd_pcm_substream *substream,
-				struct snd_intelhad *intelhaddata);
-	void (*prog_dip) (struct snd_pcm_substream *substream,
-				struct snd_intelhad *intelhaddata);
+	int (*prog_n) (u32 aud_samp_freq, u32 *n_param);
+	void (*prog_cts) (u32 aud_samp_freq, u32 tmds, u32 n_param);
+	int (*audio_ctrl) (struct snd_intelhad *intelhaddata);
+	void (*silence) (struct snd_pcm_substream *substream, u8 enable);
+	void (*prog_dip) (struct snd_intelhad *intelhaddata);
 	void (*handle_underrun) (struct snd_intelhad *intelhaddata);
 };
 
@@ -605,6 +607,7 @@ int snd_intelhad_init_audio_ctrl(struct snd_pcm_substream *substream,
 					int flag_silence);
 int snd_intelhad_prog_buffer(struct snd_intelhad *intelhaddata,
 					int start, int end);
+int snd_intelhad_set_silence(void);
 int snd_intelhad_invd_buffer(int start, int end);
 inline int snd_intelhad_read_len(struct snd_intelhad *intelhaddata);
 void had_build_channel_allocation_map(struct snd_intelhad *intelhaddata);
diff --git a/drivers/external_drivers/intel_media/hdmi_audio/intel_mid_hdmi_audio_if.c b/drivers/external_drivers/intel_media/hdmi_audio/intel_mid_hdmi_audio_if.c
index 1750b27..b69df90 100644
--- a/drivers/external_drivers/intel_media/hdmi_audio/intel_mid_hdmi_audio_if.c
+++ b/drivers/external_drivers/intel_media/hdmi_audio/intel_mid_hdmi_audio_if.c
@@ -65,6 +65,7 @@ int hdmi_audio_query(void *haddata, hdmi_audio_event_t event)
 			pr_debug("Audio stream active\n");
 			return -EBUSY;
 		}
+
 		spin_unlock_irqrestore(&intelhaddata->had_spinlock, flag_irqs);
 	break;
 
@@ -385,7 +386,7 @@ int had_process_hot_plug(struct snd_intelhad *intelhaddata)
 	intelhaddata->drv_status = HAD_DRV_CONNECTED;
 	spin_unlock_irqrestore(&intelhaddata->had_spinlock, flag_irqs);
 
-	pr_debug("Processing HOT_PLUG, buf_id = %d\n", buf_id);
+	pr_debug("Processing HOT_PLUG, buf_id = %d and hw_silence = %d\n", buf_id, intelhaddata->hw_silence);
 
 	/* Safety check */
 	if (substream) {
@@ -396,8 +397,14 @@ int had_process_hot_plug(struct snd_intelhad *intelhaddata)
 		snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP);
 	}
 
+	intelhaddata->channels = 2;
+	intelhaddata->rate = AUD_SAMPLE_RATE_48;
+	intelhaddata->format = SNDRV_PCM_FORMAT_S24_LE;
+
 	had_build_channel_allocation_map(intelhaddata);
 
+	snd_intelhad_set_silence();
+
 	return retval;
 }
 
-- 
1.7.9.5


[-- Attachment #3: Type: text/plain, Size: 0 bytes --]


