Re: [alsa-devel] [PATCH] ASoC: soc-pcm: Use delay set in pointer function

[Pierre-Louis Bossart]

On 7/27/18 11:28 PM, Agrawal, Akshu wrote:
> On 7/27/2018 8:39 PM, Pierre-Louis Bossart wrote:
> > On 7/27/18 5:13 AM, Akshu Agrawal wrote:
> > > There are cases where a pointer function populates
> > > runtime->delay, such as:
> > > ./sound/pci/hda/hda_controller.c
> > > ./sound/soc/intel/atom/sst-mfld-platform-pcm.c
> > >
> > > Also, in some cases cpu dai used is generic and the pcm
> > > driver needs to set delay.
> > >
> > > This delay was getting lost and was overwritten by delays
> > > from codec or cpu dai delay function if exposed.
> >
> > Humm, yes the runtime->delay set in the .pointer function would be lost
> > without this change, but the delay would still be provided in the
> > followup call to .delay.
> > With your change, the same delay will be accounted for twice?
>
> It will not be accounted twice because no driver which is setting
> runtime->delay is defining .delay op for cpu_dai. Vice versa is also
> true, the drivers which define .delay for cpu_dai don't set
> runtime->delay. And I think this is expected from drivers else it would
> be a bug from their side.

what do you mean my 'no driver'? Can you clarify if this is based on 
analysis of the code or by-design. I don't recall having seen any 
guidelines on this topic, and it's quite likely that different people 
have different interpretation on how delay is supposed to be reported.

> .delay for codec_dai anyway is different and has to be accounted for.
>
> Thanks,
> Akshu
> > > Signed-off-by: Akshu Agrawal <akshu.agrawal@xxxxxxx>
> > > ---
> > >    sound/soc/soc-pcm.c | 5 ++++-
> > >    1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> > > index 98be04b..b1a2bc2 100644
> > > --- a/sound/soc/soc-pcm.c
> > > +++ b/sound/soc/soc-pcm.c
> > > @@ -1179,6 +1179,9 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
> > >    	snd_pcm_sframes_t codec_delay = 0;
> > >    	int i;
> > >    
> > > +	/* clearing the previous delay */
> > > +	runtime->delay = 0;
> > > +
> > >    	for_each_rtdcom(rtd, rtdcom) {
> > >    		component = rtdcom->component;
> > >    
> > > @@ -1203,7 +1206,7 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
> > >    	}
> > >    	delay += codec_delay;
> > >    
> > > -	runtime->delay = delay;
> > > +	runtime->delay += delay;
> > >    
> > >    	return offset;
> > >    }
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@xxxxxxxxxxxxxxxx
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel