linux-kernel - RE: [PATCH] ASoC: Intel: Atom: use hardware counter to update hw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 28 Jul 2020 02:28:09 +0000
From:   "Lu, Brent" <brent.lu@...el.com>
To:     Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>
CC:     "Rojewski, Cezary" <cezary.rojewski@...el.com>,
        Liam Girdwood <liam.r.girdwood@...ux.intel.com>,
        Jie Yang <yang.jie@...ux.intel.com>,
        Mark Brown <broonie@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        "Takashi Iwai" <tiwai@...e.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] ASoC: Intel: Atom: use hardware counter to update hw_ptr

> 
> All the Atom firmware assumes data chunks in multiples of 1ms (typically 5,
> 10 or 20ms). I have never seen anyone use 256 frames, that's asking for
> trouble really.
> 
> it's actually the same with Skylake and SOF in most cases.
> 
> Is this a 'real' problem or a problem detected by the Chrome ALSA
> compliance tests, in the latter case that would hint at a too generic value of
> min_period.
> 

I've told them 240 is more reasonable since the sample rate is 48000 and our
android bsp also uses 240 for multimedia use case for many years but they don't
want to change the CRAS setting for some reason.

Google says it's a real issue for them: "The driver consumes frames quickly at the
beginning will make CRAS underrun because CRAS fills samples in the fixed rate."

Currently they implement constraint in machine driver of atom machines to force
240 period size so CRAS is using 240 for atom platforms and 256 for other big cores.

I'm curious why not just using hardware counter to update hw_ptr and get rid of
the period setting in hw_param? It seems to me the ring buffer counter does not
reflect the real status.

Regards,
Brent

> 
> and that seems also wrong? Why would the delay be zero?
> 

info->pcm_delay is the difference between ring buffer counter and hardware
counter. Because the ring buffer counter (hw_ptr) is running faster then it should,
so we add the info->pcm_delay to substream->runtime->delay as compensation.

Therefore, application could use snd_pcm_delay() to get the actual frame number
which are still in buffer.

snd_pcm_delay() = buffer_size - snd_pcm_avail() + runtime->delay

We don't need pcm_delay to compensate anything if using hardware counter.

> > -	info->pcm_delay = delay_frames;
> >   	dev_dbg(ctx->dev, "buffer ptr %llu pcm_delay rep: %llu\n",
> >   			info->buffer_ptr, info->pcm_delay);
> >   	return 0;
> >