On Thu, 06 Apr 2023 09:33:41 +0200, Oswald Buddenhagen wrote:
On Thu, Apr 06, 2023 at 08:55:27AM +0200, Takashi Iwai wrote:
On Wed, 05 Apr 2023 22:12:20 +0200, Oswald Buddenhagen wrote:
@@ -2262,7 +2156,7 @@ Typical code would be like the following. /* over the period boundary? */ if (chip->size >= runtime->period_size) { /* reset the accumulator */
chip->size %= runtime->period_size;
chip->size -= runtime->period_size; /* call updater */ spin_unlock(&chip->lock); snd_pcm_period_elapsed(substream);Hmm, this kind of change shouldn't be sneaked in. That's more than the typo fixes etc,
true, the patch grew too big and i missed this hunk. (i only kept it as one patch, because some pieces overlap and i didn't want to add churn.)
Maybe the changes could have been split from the first place, the mechanical changes to drop empty lines before "::", typo fixes, and text improvements, etc. But I guess it's too late and I'm fine to apply this whole change as a single patch with the correction.
and even worse, it's a wrong replacement.
hmm, yeah, if the timer ints are configured to occur too rarely, this wouldn't do the right thing. but then, why would they be? that would basically defeat the point of using many periods in the first place. should i instead change the text to emphasize that the ints should occur at least once per period? (i've actually pondered the timer option in the context of the emu10k1 driver as well, and concluded that there should be two timer ints per period, so snd_pcm_period_elapsed() is reliably called in the first half of the next period, which is critical when only two periods are configured.)
No, the point is that, if an irq handler misses the multiple period updates, it has to handle the situation in a single shot, and the offset gets corrected to the value within the period size; using "-=" instead of "%=" may leave the size over the period size in such a case, and this may return an invalid PCM pointer value in the end.
The text about the irq handling could be improved, of course.
thanks,
Takashi