[PATCH] dmaengine: ti: omap-dma: Block PM if SDMA is busy to fix audio

[PATCH] dmaengine: ti: omap-dma: Block PM if SDMA is busy to fix audio

[Tony Lindgren]

We now use cpu_pm for saving and restoring device context for deeper SoC
idle states. But for omap3, we must also block idle if SDMA is busy.

If we don't block idle when SDMA is busy, we eventually end up saving and
restoring SDMA register state on PER domain idle while SDMA is active and
that causes at least audio playback to fail.

Fixes: 4c74ecf79227 ("dmaengine: ti: omap-dma: Add device tree match data and use it for cpu_pm")
Reported-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/dma/ti/omap-dma.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1522,29 +1522,38 @@ static void omap_dma_free(struct omap_dmadev *od)
 	}
 }
 
+/* Currently used by omap2 & 3 to block deeper SoC idle states */
+static bool omap_dma_busy(struct omap_dmadev *od)
+{
+	struct omap_chan *c;
+	int lch = -1;
+
+	while (1) {
+		lch = find_next_bit(od->lch_bitmap, od->lch_count, lch + 1);
+		if (lch >= od->lch_count)
+			break;
+		c = od->lch_map[lch];
+		if (!c)
+			continue;
+		if (omap_dma_chan_read(c, CCR) & CCR_ENABLE)
+			return true;
+	}
+
+	return false;
+}
+
 /* Currently only used for omap2. For omap1, also a check for lcd_dma is needed */
 static int omap_dma_busy_notifier(struct notifier_block *nb,
 				  unsigned long cmd, void *v)
 {
 	struct omap_dmadev *od;
-	struct omap_chan *c;
-	int lch = -1;
 
 	od = container_of(nb, struct omap_dmadev, nb);
 
 	switch (cmd) {
 	case CPU_CLUSTER_PM_ENTER:
-		while (1) {
-			lch = find_next_bit(od->lch_bitmap, od->lch_count,
-					    lch + 1);
-			if (lch >= od->lch_count)
-				break;
-			c = od->lch_map[lch];
-			if (!c)
-				continue;
-			if (omap_dma_chan_read(c, CCR) & CCR_ENABLE)
-				return NOTIFY_BAD;
-		}
+		if (omap_dma_busy(od))
+			return NOTIFY_BAD;
 		break;
 	case CPU_CLUSTER_PM_ENTER_FAILED:
 	case CPU_CLUSTER_PM_EXIT:
@@ -1595,6 +1604,8 @@ static int omap_dma_context_notifier(struct notifier_block *nb,
 
 	switch (cmd) {
 	case CPU_CLUSTER_PM_ENTER:
+		if (omap_dma_busy(od))
+			return NOTIFY_BAD;
 		omap_dma_context_save(od);
 		break;
 	case CPU_CLUSTER_PM_ENTER_FAILED:
-- 
2.29.2

Re: [PATCH] dmaengine: ti: omap-dma: Block PM if SDMA is busy to fix audio

[Peter Ujfalusi]

Hi Tony,

On 09/11/2020 17.40, Tony Lindgren wrote:
> We now use cpu_pm for saving and restoring device context for deeper SoC
> idle states. But for omap3, we must also block idle if SDMA is busy.
> 
> If we don't block idle when SDMA is busy, we eventually end up saving and
> restoring SDMA register state on PER domain idle while SDMA is active and
> that causes at least audio playback to fail.

Thanks for the fix!

Tested-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

Vinod: Can you take this for 5.10 as a fix? The off mode got enabled by
default in 5.10-rc1 and audio got broken out of box.

Thanks,
- Péter

> Fixes: 4c74ecf79227 ("dmaengine: ti: omap-dma: Add device tree match data and use it for cpu_pm")
> Reported-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/dma/ti/omap-dma.c | 37 ++++++++++++++++++++++++-------------
>  1 file changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
> --- a/drivers/dma/ti/omap-dma.c
> +++ b/drivers/dma/ti/omap-dma.c
> @@ -1522,29 +1522,38 @@ static void omap_dma_free(struct omap_dmadev *od)
>  	}
>  }
>  
> +/* Currently used by omap2 & 3 to block deeper SoC idle states */
> +static bool omap_dma_busy(struct omap_dmadev *od)
> +{
> +	struct omap_chan *c;
> +	int lch = -1;
> +
> +	while (1) {
> +		lch = find_next_bit(od->lch_bitmap, od->lch_count, lch + 1);
> +		if (lch >= od->lch_count)
> +			break;
> +		c = od->lch_map[lch];
> +		if (!c)
> +			continue;
> +		if (omap_dma_chan_read(c, CCR) & CCR_ENABLE)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
>  /* Currently only used for omap2. For omap1, also a check for lcd_dma is needed */
>  static int omap_dma_busy_notifier(struct notifier_block *nb,
>  				  unsigned long cmd, void *v)
>  {
>  	struct omap_dmadev *od;
> -	struct omap_chan *c;
> -	int lch = -1;
>  
>  	od = container_of(nb, struct omap_dmadev, nb);
>  
>  	switch (cmd) {
>  	case CPU_CLUSTER_PM_ENTER:
> -		while (1) {
> -			lch = find_next_bit(od->lch_bitmap, od->lch_count,
> -					    lch + 1);
> -			if (lch >= od->lch_count)
> -				break;
> -			c = od->lch_map[lch];
> -			if (!c)
> -				continue;
> -			if (omap_dma_chan_read(c, CCR) & CCR_ENABLE)
> -				return NOTIFY_BAD;
> -		}
> +		if (omap_dma_busy(od))
> +			return NOTIFY_BAD;
>  		break;
>  	case CPU_CLUSTER_PM_ENTER_FAILED:
>  	case CPU_CLUSTER_PM_EXIT:
> @@ -1595,6 +1604,8 @@ static int omap_dma_context_notifier(struct notifier_block *nb,
>  
>  	switch (cmd) {
>  	case CPU_CLUSTER_PM_ENTER:
> +		if (omap_dma_busy(od))
> +			return NOTIFY_BAD;
>  		omap_dma_context_save(od);
>  		break;
>  	case CPU_CLUSTER_PM_ENTER_FAILED:
> 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH] dmaengine: ti: omap-dma: Block PM if SDMA is busy to fix audio

[Peter Ujfalusi]

Hi,

On 10/11/2020 9.58, Peter Ujfalusi wrote:
> Hi Tony,
> 
> On 09/11/2020 17.40, Tony Lindgren wrote:
>> We now use cpu_pm for saving and restoring device context for deeper SoC
>> idle states. But for omap3, we must also block idle if SDMA is busy.
>>
>> If we don't block idle when SDMA is busy, we eventually end up saving and
>> restoring SDMA register state on PER domain idle while SDMA is active and
>> that causes at least audio playback to fail.
> 
> Thanks for the fix!
> 
> Tested-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> 
> Vinod: Can you take this for 5.10 as a fix? The off mode got enabled by
> default in 5.10-rc1 and audio got broken out of box.

to Vinod with corrected email address..

> Thanks,
> - Péter
> 
>> Fixes: 4c74ecf79227 ("dmaengine: ti: omap-dma: Add device tree match data and use it for cpu_pm")
>> Reported-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> Signed-off-by: Tony Lindgren <tony@atomide.com>
>> ---
>>  drivers/dma/ti/omap-dma.c | 37 ++++++++++++++++++++++++-------------
>>  1 file changed, 24 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
>> --- a/drivers/dma/ti/omap-dma.c
>> +++ b/drivers/dma/ti/omap-dma.c
>> @@ -1522,29 +1522,38 @@ static void omap_dma_free(struct omap_dmadev *od)
>>  	}
>>  }
>>  
>> +/* Currently used by omap2 & 3 to block deeper SoC idle states */
>> +static bool omap_dma_busy(struct omap_dmadev *od)
>> +{
>> +	struct omap_chan *c;
>> +	int lch = -1;
>> +
>> +	while (1) {
>> +		lch = find_next_bit(od->lch_bitmap, od->lch_count, lch + 1);
>> +		if (lch >= od->lch_count)
>> +			break;
>> +		c = od->lch_map[lch];
>> +		if (!c)
>> +			continue;
>> +		if (omap_dma_chan_read(c, CCR) & CCR_ENABLE)
>> +			return true;
>> +	}
>> +
>> +	return false;
>> +}
>> +
>>  /* Currently only used for omap2. For omap1, also a check for lcd_dma is needed */
>>  static int omap_dma_busy_notifier(struct notifier_block *nb,
>>  				  unsigned long cmd, void *v)
>>  {
>>  	struct omap_dmadev *od;
>> -	struct omap_chan *c;
>> -	int lch = -1;
>>  
>>  	od = container_of(nb, struct omap_dmadev, nb);
>>  
>>  	switch (cmd) {
>>  	case CPU_CLUSTER_PM_ENTER:
>> -		while (1) {
>> -			lch = find_next_bit(od->lch_bitmap, od->lch_count,
>> -					    lch + 1);
>> -			if (lch >= od->lch_count)
>> -				break;
>> -			c = od->lch_map[lch];
>> -			if (!c)
>> -				continue;
>> -			if (omap_dma_chan_read(c, CCR) & CCR_ENABLE)
>> -				return NOTIFY_BAD;
>> -		}
>> +		if (omap_dma_busy(od))
>> +			return NOTIFY_BAD;
>>  		break;
>>  	case CPU_CLUSTER_PM_ENTER_FAILED:
>>  	case CPU_CLUSTER_PM_EXIT:
>> @@ -1595,6 +1604,8 @@ static int omap_dma_context_notifier(struct notifier_block *nb,
>>  
>>  	switch (cmd) {
>>  	case CPU_CLUSTER_PM_ENTER:
>> +		if (omap_dma_busy(od))
>> +			return NOTIFY_BAD;
>>  		omap_dma_context_save(od);
>>  		break;
>>  	case CPU_CLUSTER_PM_ENTER_FAILED:
>>
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH] dmaengine: ti: omap-dma: Block PM if SDMA is busy to fix audio

[Tony Lindgren]

* Peter Ujfalusi <peter.ujfalusi@ti.com> [201110 07:59]:
> Hi,
> 
> On 10/11/2020 9.58, Peter Ujfalusi wrote:
> > Hi Tony,
> > 
> > On 09/11/2020 17.40, Tony Lindgren wrote:
> >> We now use cpu_pm for saving and restoring device context for deeper SoC
> >> idle states. But for omap3, we must also block idle if SDMA is busy.
> >>
> >> If we don't block idle when SDMA is busy, we eventually end up saving and
> >> restoring SDMA register state on PER domain idle while SDMA is active and
> >> that causes at least audio playback to fail.
> > 
> > Thanks for the fix!
> > 
> > Tested-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> > Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> > 
> > Vinod: Can you take this for 5.10 as a fix? The off mode got enabled by
> > default in 5.10-rc1 and audio got broken out of box.
> 
> to Vinod with corrected email address..

Sorry about that, I've bounced the original patch too. Vinod let me know if
you want me to resend the patch.

Regards,

Tony

Re: [PATCH] dmaengine: ti: omap-dma: Block PM if SDMA is busy to fix audio

[Vinod Koul]

On 10-11-20, 13:33, Tony Lindgren wrote:
> * Peter Ujfalusi <peter.ujfalusi@ti.com> [201110 07:59]:
> > Hi,
> > 
> > On 10/11/2020 9.58, Peter Ujfalusi wrote:
> > > Hi Tony,
> > > 
> > > On 09/11/2020 17.40, Tony Lindgren wrote:
> > >> We now use cpu_pm for saving and restoring device context for deeper SoC
> > >> idle states. But for omap3, we must also block idle if SDMA is busy.
> > >>
> > >> If we don't block idle when SDMA is busy, we eventually end up saving and
> > >> restoring SDMA register state on PER domain idle while SDMA is active and
> > >> that causes at least audio playback to fail.
> > > 
> > > Thanks for the fix!
> > > 
> > > Tested-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> > > Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> > > 
> > > Vinod: Can you take this for 5.10 as a fix? The off mode got enabled by
> > > default in 5.10-rc1 and audio got broken out of box.
> > 
> > to Vinod with corrected email address..
> 
> Sorry about that, I've bounced the original patch too. Vinod let me know if
> you want me to resend the patch.

That is okay, we have b4 now, so not much of a hassle to grab, review
and apply, will do that shortly

And here is my rant to use get_maintainers.pl which should have pointed
you to the correct address ;-)

-- 
~Vinod

Re: [PATCH] dmaengine: ti: omap-dma: Block PM if SDMA is busy to fix audio

[Vinod Koul]

On 09-11-20, 17:40, Tony Lindgren wrote:
> We now use cpu_pm for saving and restoring device context for deeper SoC
> idle states. But for omap3, we must also block idle if SDMA is busy.
> 
> If we don't block idle when SDMA is busy, we eventually end up saving and
> restoring SDMA register state on PER domain idle while SDMA is active and
> that causes at least audio playback to fail.


Grabbed the patch with b4 and looks good, so applied now

-- 
~Vinod

Re: [PATCH] dmaengine: ti: omap-dma: Block PM if SDMA is busy to fix audio

[Tony Lindgren]

* Vinod Koul <vkoul@kernel.org> [201110 12:41]:
> On 10-11-20, 13:33, Tony Lindgren wrote:
> > Sorry about that, I've bounced the original patch too. Vinod let me know if
> > you want me to resend the patch.
> 
> That is okay, we have b4 now, so not much of a hassle to grab, review
> and apply, will do that shortly

OK thanks!

> And here is my rant to use get_maintainers.pl which should have pointed
> you to the correct address ;-)

Right :)

Tony