| Message ID | 1436267131-19556-1-git-send-email-prahal@yahoo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2015-07-07 20:05 GMT+09:00 Alban Browaeys <alban.browaeys@gmail.com>: > Removed the residual rest to zero on last descriptor. > > Also set residue granularity to BURST instead of SEGMENT. > This last item needs more investigation before I could give a rationale > for it. > > Signed-off-by: Alban Browaeys <prahal@yahoo.com> > Reported-by: gabriel@unseen.is > Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > Fixes: aee4d1fac887 ("dmaengine: pl330: improve pl330_tx_status() > function") > Hi, I am not quite sure what bug you are trying to fix here. It isn't described in commit message. Also I don't get why the residue=0 is being removed. This will break the residue calculation... Some time ago I posted a patch fixing sound issues on Odroid XU3. Today Vinod applied it: https://git.kernel.org/cgit/linux/kernel/git/vkoul/slave-dma.git/commit/?h=fixes&id=5dd90e5b91e0f5c925b12b132c7cd27538870256 I don't know if this is the same issue... can you describe exactly what you are trying to fix, why and how? Best regards, Krzysztof > --- > > My older local version of the residue : > https://github.com/prahal/linux/blob/odroid-3.19/drivers/dma/pl330.c#L2230 > > I took a second look at both and noticed that in the upstream version, > I was not able to make sense of the "if desc->last then reset residual > to zero" in the loop. Early tests gives a proper sound ouput with it > removed. > > More tests are welcome, and it could be the flag change is wrong. > > There is also a bad interaction with pulseaudio when the buffer size is > small : https://bugs.freedesktop.org/show_bug.cgi?id=84878. This > might affect testing. > --- > drivers/dma/pl330.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c > index f513f77..0851f41 100644 > --- a/drivers/dma/pl330.c > +++ b/drivers/dma/pl330.c > @@ -2275,8 +2275,6 @@ pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie, > } > break; > } > - if (desc->last) > - residual = 0; > } > spin_unlock_irqrestore(&pch->lock, flags); > > @@ -2890,7 +2888,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) > pd->src_addr_widths = PL330_DMA_BUSWIDTHS; > pd->dst_addr_widths = PL330_DMA_BUSWIDTHS; > pd->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV); > - pd->residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT; > + pd->residue_granularity = DMA_RESIDUE_GRANULARITY_BURST; > > ret = dma_async_device_register(pd); > if (ret) { > -- > 2.1.4 > > -- > To unsubscribe from this list: send the line "unsubscribe dmaengine" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le mardi 07 juillet 2015 à 20:53 +0900, Krzysztof Kozlowski a écrit : > 2015-07-07 20:05 GMT+09:00 Alban Browaeys <alban.browaeys@gmail.com>: > > Removed the residual rest to zero on last descriptor. > > > > Also set residue granularity to BURST instead of SEGMENT. > > This last item needs more investigation before I could give a > > rationale > > for it. > > > > Signed-off-by: Alban Browaeys <prahal@yahoo.com> > > Reported-by: gabriel@unseen.is > > Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > > Fixes: aee4d1fac887 ("dmaengine: pl330: improve pl330_tx_status() > > function") > > > > Hi, > > I am not quite sure what bug you are trying to fix here. It isn't > described in commit message. Also I don't get why the residue=0 is > being removed. This will break the residue calculation... > > Some time ago I posted a patch fixing sound issues on Odroid XU3. > Today Vinod applied it: > https://git.kernel.org/cgit/linux/kernel/git/vkoul/slave > -dma.git/commit/?h=fixes&id=5dd90e5b91e0f5c925b12b132c7cd27538870256 > > I don't know if this is the same issue... can you describe exactly > what you are trying to fix, why and how? > > This was about "choppy sound" issue left after the dma pause fix: https://www.mail-archive.com/linux-samsung -soc%40vger.kernel.org/msg44567.html From a glance at the commit you mention I would say it is the same issue. Will try this commit instead in a few hours and report. Thanks Alban > choppy sound) on Odroid XU3-Lite. On current linux-next > (next-20150612) the sound is awful (choppy) after few seconds of > play. > For example first four of: > $ aplay /usr/share/sounds/alsa/Front_Right.wav > work fine. But then it just gets worse and underruns are reported: > $ Playing WAVE '/usr/share/sounds/alsa/Front_Right.wav' : Signed 16 > bit Little Endian, Rate 48000 Hz, Mono > $ underrun!!! (at least 0.095 ms long) -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le mardi 07 juillet 2015 à 14:13 +0200, Alban Browaeys a écrit : > Le mardi 07 juillet 2015 à 20:53 +0900, Krzysztof Kozlowski a écrit > > > > > I am not quite sure what bug you are trying to fix here. It isn't > > described in commit message. Also I don't get why the residue=0 is > > being removed. This will break the residue calculation... > > > > Some time ago I posted a patch fixing sound issues on Odroid XU3. > > Today Vinod applied it: > > https://git.kernel.org/cgit/linux/kernel/git/vkoul/slave > > -dma.git/commit/?h=fixes&id=5dd90e5b91e0f5c925b12b132c7cd2753887025 > > 6 > > > > I don't know if this is the same issue... can you describe exactly > > what you are trying to fix, why and how? > > > > > > This was about "choppy sound" issue left after the dma pause fix: > https://www.mail-archive.com/linux-samsung > -soc%40vger.kernel.org/msg44567.html > > From a glance at the commit you mention I would say it is the same > issue. Will try this commit instead in a few hours and report. The commit of yours, that is https://git.kernel.org/cgit/linux/kernel/git/vkoul/slave-dma.git/commit /?h=fixes&id=5dd90e5b91e0f5c925b12b132c7cd2753887025 , fixes Odroid U2/U3 "choppy sound" too. Thanks. Alban > > > choppy sound) on Odroid XU3-Lite. On current linux-next > > (next-20150612) the sound is awful (choppy) after few seconds of > > play. > > For example first four of: > > $ aplay /usr/share/sounds/alsa/Front_Right.wav > > work fine. But then it just gets worse and underruns are reported: > > $ Playing WAVE '/usr/share/sounds/alsa/Front_Right.wav' : Signed 16 > > bit Little Endian, Rate 48000 Hz, Mono -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index f513f77..0851f41 100644 --- a/drivers/dma/pl330.c +++ b/drivers/dma/pl330.c @@ -2275,8 +2275,6 @@ pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie, } break; } - if (desc->last) - residual = 0; } spin_unlock_irqrestore(&pch->lock, flags); @@ -2890,7 +2888,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) pd->src_addr_widths = PL330_DMA_BUSWIDTHS; pd->dst_addr_widths = PL330_DMA_BUSWIDTHS; pd->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV); - pd->residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT; + pd->residue_granularity = DMA_RESIDUE_GRANULARITY_BURST; ret = dma_async_device_register(pd); if (ret) {
Removed the residual rest to zero on last descriptor. Also set residue granularity to BURST instead of SEGMENT. This last item needs more investigation before I could give a rationale for it. Signed-off-by: Alban Browaeys <prahal@yahoo.com> Reported-by: gabriel@unseen.is Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Fixes: aee4d1fac887 ("dmaengine: pl330: improve pl330_tx_status() function") --- My older local version of the residue : https://github.com/prahal/linux/blob/odroid-3.19/drivers/dma/pl330.c#L2230 I took a second look at both and noticed that in the upstream version, I was not able to make sense of the "if desc->last then reset residual to zero" in the loop. Early tests gives a proper sound ouput with it removed. More tests are welcome, and it could be the flag change is wrong. There is also a bad interaction with pulseaudio when the buffer size is small : https://bugs.freedesktop.org/show_bug.cgi?id=84878. This might affect testing. --- drivers/dma/pl330.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)