| Message ID | 20161212181043.12512-8-jerome.anand@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 12 Dec 2016 19:10:43 +0100, Jerome Anand wrote: > > When the display resolution changes, the drm disables the > display pipes due to which audio rendering stops. At this > time, we need to ensure the existing audio pointers and > buffers are cleared out so that the playback can restarted > once the display pipe is enabled with a different N/CTS values > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > Signed-off-by: Jerome Anand <jerome.anand@intel.com> > --- > sound/x86/intel_hdmi_audio.c | 21 ++++++++++++++++++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c > index 9249521..d6fd638 100644 > --- a/sound/x86/intel_hdmi_audio.c > +++ b/sound/x86/intel_hdmi_audio.c > @@ -43,6 +43,7 @@ static DEFINE_MUTEX(had_mutex); > static int hdmi_card_index = SNDRV_DEFAULT_IDX1; > static char *hdmi_card_id = SNDRV_DEFAULT_STR1; > static struct snd_intelhad *had_data; > +static int underrun_count; > > module_param(hdmi_card_index, int, 0444); > MODULE_PARM_DESC(hdmi_card_index, > @@ -1114,6 +1115,7 @@ static int snd_intelhad_open(struct snd_pcm_substream *substream) > intelhaddata = snd_pcm_substream_chip(substream); > had_stream = intelhaddata->private_data; > runtime = substream->runtime; > + underrun_count = 0; > > pm_runtime_get(intelhaddata->dev); > > @@ -1506,10 +1508,23 @@ static snd_pcm_uframes_t snd_intelhad_pcm_pointer( > > buf_id = intelhaddata->curr_buf % 4; > had_read_register(AUD_BUF_A_LENGTH + (buf_id * HAD_REG_WIDTH), &t); > - if (t == 0) { > - pr_debug("discovered buffer done for buf %d\n", buf_id); > - /* had_process_buffer_done(intelhaddata); */ > + > + if ((t == 0) || (t == ((u32)-1L))) { > + underrun_count++; > + pr_debug("discovered buffer done for buf %d, count = %d\n", > + buf_id, underrun_count); > + > + if (underrun_count > (HAD_MIN_PERIODS/2)) { > + pr_debug("assume audio_codec_reset, underrun = %d - do xrun\n", > + underrun_count); > + underrun_count = 0; > + return SNDRV_PCM_POS_XRUN; > + } > + } else { > + /* Reset Counter */ > + underrun_count = 0; > } > + > t = intelhaddata->buf_info[buf_id].buf_size - t; > > if (intelhaddata->stream_info.buffer_rendered) This change itself is OK, but this made me wonder about the driver implementation: the current MAX_PB=1 is the hardware limitation or the soft limitation? That is, can't we play back two streams when we connect two HDMI monitors? thanks, Takashi
> -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Wednesday, December 14, 2016 6:31 PM > To: Anand, Jerome <jerome.anand@intel.com> > Cc: intel-gfx@lists.freedesktop.org; alsa-devel@alsa-project.org; > ville.syrjala@linux.intel.com; broonie@kernel.org; pierre- > louis.bossart@linux.intel.com; Ughreja, Rakesh A > <rakesh.a.ughreja@intel.com> > Subject: Re: [PATCH 7/7] ALSA: x86: hdmi: continue playback even when > display resolution changes > > On Mon, 12 Dec 2016 19:10:43 +0100, > Jerome Anand wrote: > > > > When the display resolution changes, the drm disables the display > > pipes due to which audio rendering stops. At this time, we need to > > ensure the existing audio pointers and buffers are cleared out so that > > the playback can restarted once the display pipe is enabled with a > > different N/CTS values > > > > Signed-off-by: Pierre-Louis Bossart > > <pierre-louis.bossart@linux.intel.com> > > Signed-off-by: Jerome Anand <jerome.anand@intel.com> > > --- > > sound/x86/intel_hdmi_audio.c | 21 ++++++++++++++++++--- > > 1 file changed, 18 insertions(+), 3 deletions(-) > > > > diff --git a/sound/x86/intel_hdmi_audio.c > > b/sound/x86/intel_hdmi_audio.c index 9249521..d6fd638 100644 > > --- a/sound/x86/intel_hdmi_audio.c > > +++ b/sound/x86/intel_hdmi_audio.c > > @@ -43,6 +43,7 @@ static DEFINE_MUTEX(had_mutex); static int > > hdmi_card_index = SNDRV_DEFAULT_IDX1; static char *hdmi_card_id = > > SNDRV_DEFAULT_STR1; static struct snd_intelhad *had_data; > > +static int underrun_count; > > > > module_param(hdmi_card_index, int, 0444); > > MODULE_PARM_DESC(hdmi_card_index, @@ -1114,6 +1115,7 @@ static > int > > snd_intelhad_open(struct snd_pcm_substream *substream) > > intelhaddata = snd_pcm_substream_chip(substream); > > had_stream = intelhaddata->private_data; > > runtime = substream->runtime; > > + underrun_count = 0; > > > > pm_runtime_get(intelhaddata->dev); > > > > @@ -1506,10 +1508,23 @@ static snd_pcm_uframes_t > > snd_intelhad_pcm_pointer( > > > > buf_id = intelhaddata->curr_buf % 4; > > had_read_register(AUD_BUF_A_LENGTH + (buf_id * > HAD_REG_WIDTH), &t); > > - if (t == 0) { > > - pr_debug("discovered buffer done for buf %d\n", buf_id); > > - /* had_process_buffer_done(intelhaddata); */ > > + > > + if ((t == 0) || (t == ((u32)-1L))) { > > + underrun_count++; > > + pr_debug("discovered buffer done for buf %d, count = > %d\n", > > + buf_id, underrun_count); > > + > > + if (underrun_count > (HAD_MIN_PERIODS/2)) { > > + pr_debug("assume audio_codec_reset, underrun = > %d - do xrun\n", > > + underrun_count); > > + underrun_count = 0; > > + return SNDRV_PCM_POS_XRUN; > > + } > > + } else { > > + /* Reset Counter */ > > + underrun_count = 0; > > } > > + > > t = intelhaddata->buf_info[buf_id].buf_size - t; > > > > if (intelhaddata->stream_info.buffer_rendered) > > This change itself is OK, but this made me wonder about the driver > implementation: the current MAX_PB=1 is the hardware limitation or the > soft limitation? That is, can't we play back two streams when we connect > two HDMI monitors? > Two streams was never validated from hardware per se. So setting the limitation in software. > > thanks, > > Takashi
>> This change itself is OK, but this made me wonder about the driver >> implementation: the current MAX_PB=1 is the hardware limitation or the >> soft limitation? That is, can't we play back two streams when we connect >> two HDMI monitors? >> > > Two streams was never validated from hardware per se. So setting the limitation in software. To be clearer, the IP itself does support 2 streams going to two different pipes in parallel but we never had a validation platform for Atom gizmos with two connectors and no use case for tablets/phone. Never say never as usual, now we see CHT mini-PC devices with 2 connectors, either both DP or DP+HDMI so we'll have to see how things go when both are enabled. It's on the TODO list.
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 9249521..d6fd638 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -43,6 +43,7 @@ static DEFINE_MUTEX(had_mutex); static int hdmi_card_index = SNDRV_DEFAULT_IDX1; static char *hdmi_card_id = SNDRV_DEFAULT_STR1; static struct snd_intelhad *had_data; +static int underrun_count; module_param(hdmi_card_index, int, 0444); MODULE_PARM_DESC(hdmi_card_index, @@ -1114,6 +1115,7 @@ static int snd_intelhad_open(struct snd_pcm_substream *substream) intelhaddata = snd_pcm_substream_chip(substream); had_stream = intelhaddata->private_data; runtime = substream->runtime; + underrun_count = 0; pm_runtime_get(intelhaddata->dev); @@ -1506,10 +1508,23 @@ static snd_pcm_uframes_t snd_intelhad_pcm_pointer( buf_id = intelhaddata->curr_buf % 4; had_read_register(AUD_BUF_A_LENGTH + (buf_id * HAD_REG_WIDTH), &t); - if (t == 0) { - pr_debug("discovered buffer done for buf %d\n", buf_id); - /* had_process_buffer_done(intelhaddata); */ + + if ((t == 0) || (t == ((u32)-1L))) { + underrun_count++; + pr_debug("discovered buffer done for buf %d, count = %d\n", + buf_id, underrun_count); + + if (underrun_count > (HAD_MIN_PERIODS/2)) { + pr_debug("assume audio_codec_reset, underrun = %d - do xrun\n", + underrun_count); + underrun_count = 0; + return SNDRV_PCM_POS_XRUN; + } + } else { + /* Reset Counter */ + underrun_count = 0; } + t = intelhaddata->buf_info[buf_id].buf_size - t; if (intelhaddata->stream_info.buffer_rendered)