ALSA: loongson: Fix invalid position error in ls_pcm_pointer

[PATCH] ALSA: loongson: Fix invalid position error in ls_pcm_pointer

Posted by Li Jun 1 week, 3 days ago

The "invalid position" error occurred when the DMA position descriptor
returned an invalid address value (e.g., pos = -1048838144). This happened
because the `bytes_to_frames()` function returns a signed value, but when
`addr < runtime->dma_addr`, the subtraction produces a negative result that
gets interpreted as a large unsigned integer in comparisons.

[   32.834431][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
[   32.845019][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
[   32.855588][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
[   32.866145][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
[   32.995394][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
[   33.006025][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
[   33.016748][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144

Signed-off-by: Li Jun <lijun01@kylinos.cn>
---
 sound/soc/loongson/loongson_dma.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/sound/soc/loongson/loongson_dma.c b/sound/soc/loongson/loongson_dma.c
index a149b643175c..f0a7065ddae7 100644
--- a/sound/soc/loongson/loongson_dma.c
+++ b/sound/soc/loongson/loongson_dma.c
@@ -207,9 +207,15 @@ loongson_pcm_pointer(struct snd_soc_component *component,
 	desc = dma_desc_save(prtd);
 	addr = ((u64)desc->saddr_hi << 32) | desc->saddr;
 
-	x = bytes_to_frames(runtime, addr - runtime->dma_addr);
-	if (x == runtime->buffer_size)
+	if (addr < runtime->dma_addr ||
+	    addr >= runtime->dma_addr + runtime->dma_bytes) {
 		x = 0;
+	} else {
+		x = bytes_to_frames(runtime, addr - runtime->dma_addr);
+		if (x >= runtime->buffer_size)
+			x = 0;
+	}
+
 	return x;
 }
 
-- 
2.25.1

Re: [PATCH] ALSA: loongson: Fix invalid position error in ls_pcm_pointer

Posted by Takashi Iwai 1 week, 3 days ago

On Fri, 29 May 2026 04:48:30 +0200,
Li Jun wrote:
> 
> The "invalid position" error occurred when the DMA position descriptor
> returned an invalid address value (e.g., pos = -1048838144). This happened
> because the `bytes_to_frames()` function returns a signed value, but when
> `addr < runtime->dma_addr`, the subtraction produces a negative result that
> gets interpreted as a large unsigned integer in comparisons.
> 
> [   32.834431][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> [   32.845019][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> [   32.855588][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> [   32.866145][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> [   32.995394][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> [   33.006025][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> [   33.016748][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> 
> Signed-off-by: Li Jun <lijun01@kylinos.cn>

Since it's a change for ASoC, use "ASoC" prefix for the subject line.

About the code change: how does this negative position come?  Is it an
utterly bogus value to be ignored, or is it a position overlap or
such?  Your change seems just ignoring and returning 0, and if it's a
spontaneous bogus value, it may lead to an unexpected jump of the PCM
position, too, for example.


thanks,

Takashi

> ---
>  sound/soc/loongson/loongson_dma.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/loongson/loongson_dma.c b/sound/soc/loongson/loongson_dma.c
> index a149b643175c..f0a7065ddae7 100644
> --- a/sound/soc/loongson/loongson_dma.c
> +++ b/sound/soc/loongson/loongson_dma.c
> @@ -207,9 +207,15 @@ loongson_pcm_pointer(struct snd_soc_component *component,
>  	desc = dma_desc_save(prtd);
>  	addr = ((u64)desc->saddr_hi << 32) | desc->saddr;
>  
> -	x = bytes_to_frames(runtime, addr - runtime->dma_addr);
> -	if (x == runtime->buffer_size)
> +	if (addr < runtime->dma_addr ||
> +	    addr >= runtime->dma_addr + runtime->dma_bytes) {
>  		x = 0;
> +	} else {
> +		x = bytes_to_frames(runtime, addr - runtime->dma_addr);
> +		if (x >= runtime->buffer_size)
> +			x = 0;
> +	}
> +
>  	return x;
>  }
>  
> -- 
> 2.25.1
>

Re: [PATCH] ALSA: loongson: Fix invalid position error in ls_pcm_pointer

Posted by lijun 1 week, 2 days ago

When the addr is abnormal, such as when the DMA controller is abnormal,
I think x=0 should not be a point, but a range, which includes the addr
address being less than runtime ->dma1-adr, and the addr exceeding
the DMA address range.

在 2026/5/29 17:01, Takashi Iwai 写道:
> On Fri, 29 May 2026 04:48:30 +0200,
> Li Jun wrote:
>> The "invalid position" error occurred when the DMA position descriptor
>> returned an invalid address value (e.g., pos = -1048838144). This happened
>> because the `bytes_to_frames()` function returns a signed value, but when
>> `addr < runtime->dma_addr`, the subtraction produces a negative result that
>> gets interpreted as a large unsigned integer in comparisons.
>>
>> [   32.834431][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
>> [   32.845019][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
>> [   32.855588][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
>> [   32.866145][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
>> [   32.995394][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
>> [   33.006025][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
>> [   33.016748][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
>>
>> Signed-off-by: Li Jun <lijun01@kylinos.cn>
> Since it's a change for ASoC, use "ASoC" prefix for the subject line.
>
> About the code change: how does this negative position come?  Is it an
> utterly bogus value to be ignored, or is it a position overlap or
> such?  Your change seems just ignoring and returning 0, and if it's a
> spontaneous bogus value, it may lead to an unexpected jump of the PCM
> position, too, for example.
>
>
> thanks,
>
> Takashi
>
>> ---
>>   sound/soc/loongson/loongson_dma.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/sound/soc/loongson/loongson_dma.c b/sound/soc/loongson/loongson_dma.c
>> index a149b643175c..f0a7065ddae7 100644
>> --- a/sound/soc/loongson/loongson_dma.c
>> +++ b/sound/soc/loongson/loongson_dma.c
>> @@ -207,9 +207,15 @@ loongson_pcm_pointer(struct snd_soc_component *component,
>>   	desc = dma_desc_save(prtd);
>>   	addr = ((u64)desc->saddr_hi << 32) | desc->saddr;
>>   
>> -	x = bytes_to_frames(runtime, addr - runtime->dma_addr);
>> -	if (x == runtime->buffer_size)
>> +	if (addr < runtime->dma_addr ||
>> +	    addr >= runtime->dma_addr + runtime->dma_bytes) {
>>   		x = 0;
>> +	} else {
>> +		x = bytes_to_frames(runtime, addr - runtime->dma_addr);
>> +		if (x >= runtime->buffer_size)
>> +			x = 0;
>> +	}
>> +
>>   	return x;
>>   }
>>   
>> -- 
>> 2.25.1
>>

Re: [PATCH] ALSA: loongson: Fix invalid position error in ls_pcm_pointer

Posted by Takashi Iwai 1 week, 2 days ago

On Fri, 29 May 2026 15:23:18 +0200,
lijun wrote:
> 
> When the addr is abnormal, such as when the DMA controller is abnormal,
> I think x=0 should not be a point, but a range, which includes the addr
> address being less than runtime ->dma1-adr, and the addr exceeding
> the DMA address range.

The question is how often it happens.  If this happens frequently,
reporting a position jump just confuses user-space.
If it's an abnormal situation, one can treat it rather like an XRUN
and let stop the stream / restart, too.


Takashi

> 
> 在 2026/5/29 17:01, Takashi Iwai 写道:
> > On Fri, 29 May 2026 04:48:30 +0200,
> > Li Jun wrote:
> >> The "invalid position" error occurred when the DMA position descriptor
> >> returned an invalid address value (e.g., pos = -1048838144). This happened
> >> because the `bytes_to_frames()` function returns a signed value, but when
> >> `addr < runtime->dma_addr`, the subtraction produces a negative result that
> >> gets interpreted as a large unsigned integer in comparisons.
> >> 
> >> [   32.834431][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> >> [   32.845019][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> >> [   32.855588][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> >> [   32.866145][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> >> [   32.995394][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> >> [   33.006025][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> >> [   33.016748][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> >> 
> >> Signed-off-by: Li Jun <lijun01@kylinos.cn>
> > Since it's a change for ASoC, use "ASoC" prefix for the subject line.
> > 
> > About the code change: how does this negative position come?  Is it an
> > utterly bogus value to be ignored, or is it a position overlap or
> > such?  Your change seems just ignoring and returning 0, and if it's a
> > spontaneous bogus value, it may lead to an unexpected jump of the PCM
> > position, too, for example.
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> >> ---
> >>   sound/soc/loongson/loongson_dma.c | 10 ++++++++--
> >>   1 file changed, 8 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/sound/soc/loongson/loongson_dma.c b/sound/soc/loongson/loongson_dma.c
> >> index a149b643175c..f0a7065ddae7 100644
> >> --- a/sound/soc/loongson/loongson_dma.c
> >> +++ b/sound/soc/loongson/loongson_dma.c
> >> @@ -207,9 +207,15 @@ loongson_pcm_pointer(struct snd_soc_component *component,
> >>   	desc = dma_desc_save(prtd);
> >>   	addr = ((u64)desc->saddr_hi << 32) | desc->saddr;
> >>   -	x = bytes_to_frames(runtime, addr - runtime->dma_addr);
> >> -	if (x == runtime->buffer_size)
> >> +	if (addr < runtime->dma_addr ||
> >> +	    addr >= runtime->dma_addr + runtime->dma_bytes) {
> >>   		x = 0;
> >> +	} else {
> >> +		x = bytes_to_frames(runtime, addr - runtime->dma_addr);
> >> +		if (x >= runtime->buffer_size)
> >> +			x = 0;
> >> +	}
> >> +
> >>   	return x;
> >>   }
> >>   
> >> -- 
> >> 2.25.1
> >>

Re: [PATCH] ALSA: loongson: Fix invalid position error in ls_pcm_pointer

Posted by lijun 1 week ago

I am debugging a new hardware sound card, and every time the machine
power on, addr=0 occurs. I am analyzing the cause. This negative is 
caused by addr=0.

在 2026/5/29 22:04, Takashi Iwai 写道:
> this negative