This series reduces significantly the amount of interrupts on
fsl_qmc_audio device.
Patches 1 and 2 are preparatory patches.
Patch 3 is the main change
Patch 4 is a cleanup which is enabled by previous patch
Changes in v2:
- Don't remove UB bit (Patch 1, comment from Herve Codina)
- Make sure audio channels are ordered on TDM bus (Patch 2, new patch, comment from Herve Codina)
- Drop struct qmc_dai_chan (patch 4, new patch)
Backgroup (copied from patch 3):
In non-interleaved mode, several QMC channels are used in sync.
More details can be found in commit 188d9cae5438 ("ASoC: fsl:
fsl_qmc_audio: Add support for non-interleaved mode.")
At the time being, an interrupt is requested on each channel to
perform capture/playback completion, allthough the completion is
really performed only once all channels have completed their work.
This leads to a lot more interrupts than really needed. Looking at
/proc/interrupts shows ~3800 interrupts per second when using
4 capture and 4 playback devices with 5ms periods while
only 1600 (200 x 4 + 200 x 4) periods are processed during one second.
The QMC channels work in sync, the one started first is the one
finishing first and the one started last is the one finishing last,
so when the last one finishes it is guaranteed that the other ones are
finished as well. Therefore only request completion processing on the
last QMC channel.
On my board with the above exemple, on a kernel started with
'threadirqs' option, the QMC irq thread uses 16% CPU time with this
patch while it uses 26% CPU time without this patch.
Christophe Leroy (4):
soc: fsl: qmc: Only set completion interrupt when needed
ASoc: fsl: fsl_qmc_audio: Ensure audio channels are ordered in TDM bus
ASoC: fsl: fsl_qmc_audio: Only request completion on last channel
ASoc: fsl: fsl_qmc_audio: Drop struct qmc_dai_chan
drivers/soc/fsl/qe/qmc.c | 44 +++++++++---
sound/soc/fsl/fsl_qmc_audio.c | 125 +++++++++++++++-------------------
2 files changed, 87 insertions(+), 82 deletions(-)
--
2.49.0
prtd_tx and prtd_rx members are not used anymore and only qmc_chan
member remains so struct qmc_dai_chan has become pointless.
Use qmc_chan directly and drop struct qmc_dai_chan.
Signed-off-by: Christophe Leroy <redacted>
---
v2: New
---
sound/soc/fsl/fsl_qmc_audio.c | 50 +++++++++++++----------------------
1 file changed, 19 insertions(+), 31 deletions(-)
@@ -125,7 +119,7 @@ static int qmc_audio_pcm_write_submit(struct qmc_dai_prtd *prtd)intret;for(i=0;i<prtd->channels;i++){-ret=qmc_chan_write_submit(prtd->qmc_dai->chans[i].qmc_chan,+ret=qmc_chan_write_submit(prtd->qmc_dai->qmc_chans[i],prtd->ch_dma_addr_current+i*prtd->ch_dma_offset,prtd->ch_dma_size,i==prtd->channels-1?qmc_audio_pcm_write_complete:
@@ -165,7 +159,7 @@ static int qmc_audio_pcm_read_submit(struct qmc_dai_prtd *prtd)intret;for(i=0;i<prtd->channels;i++){-ret=qmc_chan_read_submit(prtd->qmc_dai->chans[i].qmc_chan,+ret=qmc_chan_read_submit(prtd->qmc_dai->qmc_chans[i],prtd->ch_dma_addr_current+i*prtd->ch_dma_offset,prtd->ch_dma_size,i==prtd->channels-1?qmc_audio_pcm_read_complete:
@@ -206,7 +200,6 @@ static int qmc_audio_pcm_trigger(struct snd_soc_component *component,structsnd_pcm_substream*substream,intcmd){structqmc_dai_prtd*prtd=substream->runtime->private_data;-unsignedinti;intret;if(!prtd->qmc_dai){
@@ -220,9 +213,6 @@ static int qmc_audio_pcm_trigger(struct snd_soc_component *component,prtd->ch_dma_addr_current=prtd->ch_dma_addr_start;if(substream->stream==SNDRV_PCM_STREAM_PLAYBACK){-for(i=0;i<prtd->channels;i++)-prtd->qmc_dai->chans[i].prtd_tx=prtd;-/* Submit first chunk ... */ret=qmc_audio_pcm_write_submit(prtd);if(ret)
@@ -238,9 +228,6 @@ static int qmc_audio_pcm_trigger(struct snd_soc_component *component,if(ret)returnret;}else{-for(i=0;i<prtd->channels;i++)-prtd->qmc_dai->chans[i].prtd_rx=prtd;-/* Submit first chunk ... */ret=qmc_audio_pcm_read_submit(prtd);if(ret)
@@ -610,9 +597,9 @@ static int qmc_dai_hw_params(struct snd_pcm_substream *substream,chan_param.mode=QMC_TRANSPARENT;chan_param.transp.max_rx_buf_size=params_period_bytes(params)/nb_chans_used;for(i=0;i<nb_chans_used;i++){-ret=qmc_chan_set_param(qmc_dai->chans[i].qmc_chan,&chan_param);+ret=qmc_chan_set_param(qmc_dai->qmc_chans[i],&chan_param);if(ret){-dev_err(dai->dev,"chans[%u], set param failed %d\n",+dev_err(dai->dev,"qmc_chans[%u], set param failed %d\n",i,ret);returnret;}
@@ -654,7 +641,7 @@ static int qmc_dai_trigger(struct snd_pcm_substream *substream, int cmd,caseSNDRV_PCM_TRIGGER_RESUME:caseSNDRV_PCM_TRIGGER_PAUSE_RELEASE:for(i=0;i<nb_chans_used;i++){-ret=qmc_chan_start(qmc_dai->chans[i].qmc_chan,direction);+ret=qmc_chan_start(qmc_dai->qmc_chans[i],direction);if(ret)gotoerr_stop;}
@@ -663,13 +650,13 @@ static int qmc_dai_trigger(struct snd_pcm_substream *substream, int cmd,caseSNDRV_PCM_TRIGGER_STOP:/* Stop and reset all QMC channels and return the first error encountered */for(i=0;i<nb_chans_used;i++){-ret_tmp=qmc_chan_stop(qmc_dai->chans[i].qmc_chan,direction);+ret_tmp=qmc_chan_stop(qmc_dai->qmc_chans[i],direction);if(!ret)ret=ret_tmp;if(ret_tmp)continue;-ret_tmp=qmc_chan_reset(qmc_dai->chans[i].qmc_chan,direction);+ret_tmp=qmc_chan_reset(qmc_dai->qmc_chans[i],direction);if(!ret)ret=ret_tmp;}
@@ -681,7 +668,7 @@ static int qmc_dai_trigger(struct snd_pcm_substream *substream, int cmd,caseSNDRV_PCM_TRIGGER_PAUSE_PUSH:/* Stop all QMC channels and return the first error encountered */for(i=0;i<nb_chans_used;i++){-ret_tmp=qmc_chan_stop(qmc_dai->chans[i].qmc_chan,direction);+ret_tmp=qmc_chan_stop(qmc_dai->qmc_chans[i],direction);if(!ret)ret=ret_tmp;}
@@ -697,8 +684,8 @@ static int qmc_dai_trigger(struct snd_pcm_substream *substream, int cmd,err_stop:while(i--){-qmc_chan_stop(qmc_dai->chans[i].qmc_chan,direction);-qmc_chan_reset(qmc_dai->chans[i].qmc_chan,direction);+qmc_chan_stop(qmc_dai->qmc_chans[i],direction);+qmc_chan_reset(qmc_dai->qmc_chans[i],direction);}returnret;}
@@ -794,19 +781,20 @@ static int qmc_audio_dai_parse(struct qmc_audio *qmc_audio, struct device_node *returndev_err_probe(qmc_audio->dev,-EINVAL,"dai %d no QMC channel defined\n",qmc_dai->id);-qmc_dai->chans=devm_kcalloc(qmc_audio->dev,count,sizeof(*qmc_dai->chans),GFP_KERNEL);-if(!qmc_dai->chans)+qmc_dai->qmc_chans=devm_kcalloc(qmc_audio->dev,count,sizeof(*qmc_dai->qmc_chans),+GFP_KERNEL);+if(!qmc_dai->qmc_chans)return-ENOMEM;for(i=0;i<count;i++){-qmc_dai->chans[i].qmc_chan=devm_qmc_chan_get_byphandles_index(qmc_audio->dev,np,-"fsl,qmc-chan",i);-if(IS_ERR(qmc_dai->chans[i].qmc_chan)){-returndev_err_probe(qmc_audio->dev,PTR_ERR(qmc_dai->chans[i].qmc_chan),+qmc_dai->qmc_chans[i]=devm_qmc_chan_get_byphandles_index(qmc_audio->dev,np,+"fsl,qmc-chan",i);+if(IS_ERR(qmc_dai->qmc_chans[i])){+returndev_err_probe(qmc_audio->dev,PTR_ERR(qmc_dai->qmc_chans[i]),"dai %d get QMC channel %d failed\n",qmc_dai->id,i);}-ret=qmc_chan_get_info(qmc_dai->chans[i].qmc_chan,&info);+ret=qmc_chan_get_info(qmc_dai->qmc_chans[i],&info);if(ret){dev_err(qmc_audio->dev,"dai %d get QMC %d channel info failed %d\n",qmc_dai->id,i,ret);
Hi Christophe,
On Tue, 12 Aug 2025 12:50:58 +0200
Christophe Leroy [off-list ref] wrote:
prtd_tx and prtd_rx members are not used anymore and only qmc_chan
member remains so struct qmc_dai_chan has become pointless.
Use qmc_chan directly and drop struct qmc_dai_chan.
Signed-off-by: Christophe Leroy <redacted>
---
v2: New
---
sound/soc/fsl/fsl_qmc_audio.c | 50 +++++++++++++----------------------
1 file changed, 19 insertions(+), 31 deletions(-)
Acked-by: Herve Codina <herve.codina@bootlin.com>
Best regards,
Hervé
To reduce complexity of interrupt handling in following patch, ensure
audio channels are configured in the same order as timeslots on the
TDM bus. If we need a given ordering of audio sources in the audio
frame, it is possible to re-order codecs on the TDM bus, no need to
mix up timeslots in channels.
Signed-off-by: Christophe Leroy <redacted>
---
v2: New
---
sound/soc/fsl/fsl_qmc_audio.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
On Tue, 12 Aug 2025 12:50:56 +0200
Christophe Leroy [off-list ref] wrote:
quoted hunk
To reduce complexity of interrupt handling in following patch, ensure
audio channels are configured in the same order as timeslots on the
TDM bus. If we need a given ordering of audio sources in the audio
frame, it is possible to re-order codecs on the TDM bus, no need to
mix up timeslots in channels.
Signed-off-by: Christophe Leroy <redacted>
---
v2: New
---
sound/soc/fsl/fsl_qmc_audio.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
qmc_chan_get_ts_info() need a struct qmc_chan as first parameter
qmc_dai->qmc_chans[i].qmc_chan instead of qmc_dai->qmc_chans[i].
The use of qmc_dai->qmc_chans[i] without .qmc_chan have to be done on patch 4 (cleanup patch).
Best regards,
Hervé
On Tue, 12 Aug 2025 12:50:56 +0200
Christophe Leroy [off-list ref] wrote:
quoted
To reduce complexity of interrupt handling in following patch, ensure
audio channels are configured in the same order as timeslots on the
TDM bus. If we need a given ordering of audio sources in the audio
frame, it is possible to re-order codecs on the TDM bus, no need to
mix up timeslots in channels.
Signed-off-by: Christophe Leroy <redacted>
---
v2: New
---
sound/soc/fsl/fsl_qmc_audio.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
qmc_chan_get_ts_info() need a struct qmc_chan as first parameter
qmc_dai->qmc_chans[i].qmc_chan instead of qmc_dai->qmc_chans[i].
The use of qmc_dai->qmc_chans[i] without .qmc_chan have to be done on patch 4 (cleanup patch).
In non-interleaved mode, several QMC channels are used in sync.
More details can be found in commit 188d9cae5438 ("ASoC: fsl:
fsl_qmc_audio: Add support for non-interleaved mode.")
At the time being, an interrupt is requested on each channel to
perform capture/playback completion, allthough the completion is
really performed only once all channels have completed their work.
This leads to a lot more interrupts than really needed. Looking at
/proc/interrupts shows ~3800 interrupts per second when using
4 capture and 4 playback devices with 5ms periods while
only 1600 (200 x 4 + 200 x 4) periods are processed during one second.
The QMC channels work in sync, the one started first is the one
finishing first and the one started last is the one finishing last,
so when the last one finishes it is guaranteed that the other ones are
finished as well. Therefore only request completion processing on the
last QMC channel.
On my board with the above exemple, on a kernel started with
'threadirqs' option, the QMC irq thread uses 16% CPU time with this
patch while it uses 26% CPU time without this patch.
Signed-off-by: Christophe Leroy <redacted>
---
sound/soc/fsl/fsl_qmc_audio.c | 46 +++++------------------------------
1 file changed, 6 insertions(+), 40 deletions(-)
@@ -146,20 +142,7 @@ static int qmc_audio_pcm_write_submit(struct qmc_dai_prtd *prtd)staticvoidqmc_audio_pcm_write_complete(void*context){-structqmc_dai_chan*chan=context;-structqmc_dai_prtd*prtd;--prtd=chan->prtd_tx;--/* Mark the current channel as completed */-bitmap_clear(prtd->chans_pending,chan-prtd->qmc_dai->chans,1);--/*-*AllQMCchannelsinvolvedmusthavecompletedtheirtransferbefore-*submittinganewone.-*/-if(!bitmap_empty(prtd->chans_pending,64))-return;+structqmc_dai_prtd*prtd=context;prtd->buffer_ended+=prtd->period_size;if(prtd->buffer_ended>=prtd->buffer_size)
@@ -202,26 +182,13 @@ static int qmc_audio_pcm_read_submit(struct qmc_dai_prtd *prtd)staticvoidqmc_audio_pcm_read_complete(void*context,size_tlength,unsignedintflags){-structqmc_dai_chan*chan=context;-structqmc_dai_prtd*prtd;--prtd=chan->prtd_rx;--/* Mark the current channel as completed */-bitmap_clear(prtd->chans_pending,chan-prtd->qmc_dai->chans,1);+structqmc_dai_prtd*prtd=context;if(length!=prtd->ch_dma_size){dev_err(prtd->qmc_dai->dev,"read complete length = %zu, exp %zu\n",length,prtd->ch_dma_size);}-/*-*AllQMCchannelsinvolvedmusthavecompletedtheirtransferbefore-*submittinganewone.-*/-if(!bitmap_empty(prtd->chans_pending,64))-return;-prtd->buffer_ended+=prtd->period_size;if(prtd->buffer_ended>=prtd->buffer_size)prtd->buffer_ended=0;
@@ -249,7 +216,6 @@ static int qmc_audio_pcm_trigger(struct snd_soc_component *component,switch(cmd){caseSNDRV_PCM_TRIGGER_START:-bitmap_zero(prtd->chans_pending,64);prtd->buffer_ended=0;prtd->ch_dma_addr_current=prtd->ch_dma_addr_start;
On Tue, 12 Aug 2025 12:50:57 +0200
Christophe Leroy [off-list ref] wrote:
In non-interleaved mode, several QMC channels are used in sync.
More details can be found in commit 188d9cae5438 ("ASoC: fsl:
fsl_qmc_audio: Add support for non-interleaved mode.")
At the time being, an interrupt is requested on each channel to
perform capture/playback completion, allthough the completion is
really performed only once all channels have completed their work.
This leads to a lot more interrupts than really needed. Looking at
/proc/interrupts shows ~3800 interrupts per second when using
4 capture and 4 playback devices with 5ms periods while
only 1600 (200 x 4 + 200 x 4) periods are processed during one second.
The QMC channels work in sync, the one started first is the one
finishing first and the one started last is the one finishing last,
so when the last one finishes it is guaranteed that the other ones are
finished as well. Therefore only request completion processing on the
last QMC channel.
On my board with the above exemple, on a kernel started with
'threadirqs' option, the QMC irq thread uses 16% CPU time with this
patch while it uses 26% CPU time without this patch.
Signed-off-by: Christophe Leroy <redacted>
---
sound/soc/fsl/fsl_qmc_audio.c | 46 +++++------------------------------
1 file changed, 6 insertions(+), 40 deletions(-)
Acked-by: Herve Codina <herve.codina@bootlin.com>
Best regards,
Hervé
--
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
When no post-completion processing is expected, don't waste time
handling useless interrupts.
Only set QMC_BD_[R/T]X_I when a completion function is passed in,
and perform seamless completion on submit for interruptless buffers.
Signed-off-by: Christophe Leroy <redacted>
---
v2: Keep the UB flag to mark not completed buffers and seamlessly flag them as completed during next submit.
---
drivers/soc/fsl/qe/qmc.c | 44 ++++++++++++++++++++++++++++++----------
1 file changed, 33 insertions(+), 11 deletions(-)
@@ -461,9 +461,16 @@ int qmc_chan_write_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,ctrl=qmc_read16(&bd->cbd_sc);if(ctrl&(QMC_BD_TX_R|QMC_BD_TX_UB)){-/* We are full ... */-ret=-EBUSY;-gotoend;+if(!(ctrl&QMC_BD_TX_I)&&bd==chan->txbd_done){+if(ctrl&QMC_BD_TX_W)+chan->txbd_done=chan->txbds;+else+chan->txbd_done++;+}else{+/* We are full ... */+ret=-EBUSY;+gotoend;+}}qmc_write16(&bd->cbd_datlen,length);
@@ -475,6 +482,10 @@ int qmc_chan_write_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,/* Activate the descriptor */ctrl|=(QMC_BD_TX_R|QMC_BD_TX_UB);+if(complete)+ctrl|=QMC_BD_TX_I;+else+ctrl&=~QMC_BD_TX_I;wmb();/* Be sure to flush the descriptor before control update */qmc_write16(&bd->cbd_sc,ctrl);
@@ -569,9 +580,16 @@ int qmc_chan_read_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,ctrl=qmc_read16(&bd->cbd_sc);if(ctrl&(QMC_BD_RX_E|QMC_BD_RX_UB)){-/* We are full ... */-ret=-EBUSY;-gotoend;+if(!(ctrl&QMC_BD_RX_I)&&bd==chan->rxbd_done){+if(ctrl&QMC_BD_RX_W)+chan->rxbd_done=chan->rxbds;+else+chan->rxbd_done++;+}else{+/* We are full ... */+ret=-EBUSY;+gotoend;+}}qmc_write16(&bd->cbd_datlen,0);/* data length is updated by the QMC */
@@ -587,6 +605,10 @@ int qmc_chan_read_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,/* Activate the descriptor */ctrl|=(QMC_BD_RX_E|QMC_BD_RX_UB);+if(complete)+ctrl|=QMC_BD_RX_I;+else+ctrl&=~QMC_BD_RX_I;wmb();/* Be sure to flush data before descriptor activation */qmc_write16(&bd->cbd_sc,ctrl);
@@ -1482,19 +1504,19 @@ static int qmc_setup_chan(struct qmc *qmc, struct qmc_chan *chan)/* Init Rx BDs and set Wrap bit on last descriptor */BUILD_BUG_ON(QMC_NB_RXBDS==0);-val=QMC_BD_RX_I;for(i=0;i<QMC_NB_RXBDS;i++){bd=chan->rxbds+i;-qmc_write16(&bd->cbd_sc,val);+qmc_write16(&bd->cbd_sc,0);}bd=chan->rxbds+QMC_NB_RXBDS-1;-qmc_write16(&bd->cbd_sc,val|QMC_BD_RX_W);+qmc_write16(&bd->cbd_sc,QMC_BD_RX_W);/* Init Tx BDs and set Wrap bit on last descriptor */BUILD_BUG_ON(QMC_NB_TXBDS==0);-val=QMC_BD_TX_I;if(chan->mode==QMC_HDLC)-val|=QMC_BD_TX_L|QMC_BD_TX_TC;+val=QMC_BD_TX_L|QMC_BD_TX_TC;+else+val=0;for(i=0;i<QMC_NB_TXBDS;i++){bd=chan->txbds+i;qmc_write16(&bd->cbd_sc,val);
Hi Christophe,
On Tue, 12 Aug 2025 12:50:55 +0200
Christophe Leroy [off-list ref] wrote:
quoted hunk
When no post-completion processing is expected, don't waste time
handling useless interrupts.
Only set QMC_BD_[R/T]X_I when a completion function is passed in,
and perform seamless completion on submit for interruptless buffers.
Signed-off-by: Christophe Leroy <redacted>
---
v2: Keep the UB flag to mark not completed buffers and seamlessly flag them as completed during next submit.
---
drivers/soc/fsl/qe/qmc.c | 44 ++++++++++++++++++++++++++++++----------
1 file changed, 33 insertions(+), 11 deletions(-)
@@ -461,9 +461,16 @@ int qmc_chan_write_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,ctrl=qmc_read16(&bd->cbd_sc);if(ctrl&(QMC_BD_TX_R|QMC_BD_TX_UB)){-/* We are full ... */-ret=-EBUSY;-gotoend;+if(!(ctrl&QMC_BD_TX_I)&&bd==chan->txbd_done){+if(ctrl&QMC_BD_TX_W)+chan->txbd_done=chan->txbds;+else+chan->txbd_done++;+}else{+/* We are full ... */+ret=-EBUSY;+gotoend;+}}qmc_write16(&bd->cbd_datlen,length);
@@ -475,6 +482,10 @@ int qmc_chan_write_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,/* Activate the descriptor */ctrl|=(QMC_BD_TX_R|QMC_BD_TX_UB);+if(complete)+ctrl|=QMC_BD_TX_I;+else+ctrl&=~QMC_BD_TX_I;wmb();/* Be sure to flush the descriptor before control update */qmc_write16(&bd->cbd_sc,ctrl);
You try to purge one descriptor for which the transfer is done but you do that
when you have no more free descriptors.
You end up with all descriptor "used". I think a better way to do that is
to purge all "done" descriptor configured to work without interrupts until a
descriptor with interrupt is found.
What do you think about the following (draft code not compiled and not tested) ?
/* must be called with tx_lock taken */
static void qmc_chan_write_purge(struct qmc_chan *chan)
{
cbd_t __iomem *bd;
u16 ctrl;
/*
* Purge descriptors configured to work without interrupts
* (I bit == 0) those descriptors have no completion
* callbacks.
*
* R bit UB bit
* 0 0 : The BD is free
* 1 1 : The BD is in used, waiting for transfer
* 0 1 : The BD is in used, waiting for completion
* 1 0 : Should not append
*
* We purge those descriptors which are in the state "waiting for
* completion" up to the first one configured to work with an interrupt.
*
*/
bd = chan->txbd_done;
ctrl = qmc_read16(&bd->cbd_sc);
while (!(ctrl & (QMC_BD_TX_R | QMC_BD_TX_I)) {
if (!(ctrl & QMC_BD_TX_UB))
return;
qmc_write16(&bd->cbd_sc, ctrl & ~QMC_BD_TX_UB);
if (ctrl & QMC_BD_TX_W)
chan->txbd_done = chan->txbds;
else
chan->txbd_done++;
bd = chan->txbd_done;
ctrl = qmc_read16(&bd->cbd_sc);
}
}
Then, qmc_chan_write_submit() calls the purge function as a first operation.
This will look to something like the following:
int qmc_chan_write_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,
void (*complete)(void *context), void *context)
{
struct qmc_xfer_desc *xfer_desc;
unsigned long flags;
cbd_t __iomem *bd;
u16 ctrl;
int ret;
/*
* R bit UB bit
* 0 0 : The BD is free
* 1 1 : The BD is in used, waiting for transfer
* 0 1 : The BD is in used, waiting for completion
* 1 0 : Should not append
*/
spin_lock_irqsave(&chan->tx_lock, flags);
qmc_chan_write_purge(chan);
bd = chan->txbd_free;
...
if (complete)
ctrl |= QMC_BD_TX_I;
else
ctrl &= ~QMC_BD_TX_I;
...
}
...
Exact same comment and proposal for read part.
quoted hunk
@@ -1482,19 +1504,19 @@ static int qmc_setup_chan(struct qmc *qmc, struct qmc_chan *chan) /* Init Rx BDs and set Wrap bit on last descriptor */ BUILD_BUG_ON(QMC_NB_RXBDS == 0);- val = QMC_BD_RX_I; for (i = 0; i < QMC_NB_RXBDS; i++) { bd = chan->rxbds + i;- qmc_write16(&bd->cbd_sc, val);+ qmc_write16(&bd->cbd_sc, 0); } bd = chan->rxbds + QMC_NB_RXBDS - 1;- qmc_write16(&bd->cbd_sc, val | QMC_BD_RX_W);+ qmc_write16(&bd->cbd_sc, QMC_BD_RX_W); /* Init Tx BDs and set Wrap bit on last descriptor */ BUILD_BUG_ON(QMC_NB_TXBDS == 0);- val = QMC_BD_TX_I; if (chan->mode == QMC_HDLC)- val |= QMC_BD_TX_L | QMC_BD_TX_TC;+ val = QMC_BD_TX_L | QMC_BD_TX_TC;+ else+ val = 0; for (i = 0; i < QMC_NB_TXBDS; i++) { bd = chan->txbds + i; qmc_write16(&bd->cbd_sc, val);
Ok for modifications in qmc_setup_chan().
Best regards,
Hervé
Hi Christophe,
On Wed, 13 Aug 2025 12:06:51 +0200
Herve Codina [off-list ref] wrote:
Hi Christophe,
On Tue, 12 Aug 2025 12:50:55 +0200
Christophe Leroy [off-list ref] wrote:
quoted
When no post-completion processing is expected, don't waste time
handling useless interrupts.
Only set QMC_BD_[R/T]X_I when a completion function is passed in,
and perform seamless completion on submit for interruptless buffers.
Signed-off-by: Christophe Leroy <redacted>
---
v2: Keep the UB flag to mark not completed buffers and seamlessly flag them as completed during next submit.
---
drivers/soc/fsl/qe/qmc.c | 44 ++++++++++++++++++++++++++++++----------
1 file changed, 33 insertions(+), 11 deletions(-)
@@ -461,9 +461,16 @@ int qmc_chan_write_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,ctrl=qmc_read16(&bd->cbd_sc);if(ctrl&(QMC_BD_TX_R|QMC_BD_TX_UB)){-/* We are full ... */-ret=-EBUSY;-gotoend;+if(!(ctrl&QMC_BD_TX_I)&&bd==chan->txbd_done){+if(ctrl&QMC_BD_TX_W)+chan->txbd_done=chan->txbds;+else+chan->txbd_done++;+}else{+/* We are full ... */+ret=-EBUSY;+gotoend;+}}qmc_write16(&bd->cbd_datlen,length);
@@ -475,6 +482,10 @@ int qmc_chan_write_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,/* Activate the descriptor */ctrl|=(QMC_BD_TX_R|QMC_BD_TX_UB);+if(complete)+ctrl|=QMC_BD_TX_I;+else+ctrl&=~QMC_BD_TX_I;wmb();/* Be sure to flush the descriptor before control update */qmc_write16(&bd->cbd_sc,ctrl);
You try to purge one descriptor for which the transfer is done but you do that
when you have no more free descriptors.
You end up with all descriptor "used". I think a better way to do that is
to purge all "done" descriptor configured to work without interrupts until a
descriptor with interrupt is found.
I have looked again at your code and looking for a free descriptor only when it
is needed is sufficient. You can forget my previous proposal.
Back to your code, I think you need to be sure that the descriptor you want to
re-use is really available and so you need to check the 'R' bit to be sure
that we are not with 'R' = 1 and 'UB' = 1 which means "BD is used, waiting for
a transfer".
For instance:
if (ctrl & (QMC_BD_TX_R | QMC_BD_TX_UB)) {
if (!(ctrl & (QMC_BD_TX_I | QMC_BD_TX_R) &&
bd == chan->txbd_done) {
if (ctrl & QMC_BD_TX_W)
chan->txbd_done = chan->txbds;
else
chan->txbd_done++;
} else {
/* We are full ... */
ret = -EBUSY;
goto end;
}
}
Best regards,
Hervé
Hi Hervé,
Le 14/08/2025 à 09:34, Herve Codina a écrit :
Hi Christophe,
On Wed, 13 Aug 2025 12:06:51 +0200
Herve Codina [off-list ref] wrote:
quoted
Hi Christophe,
On Tue, 12 Aug 2025 12:50:55 +0200
Christophe Leroy [off-list ref] wrote:
quoted
When no post-completion processing is expected, don't waste time
handling useless interrupts.
Only set QMC_BD_[R/T]X_I when a completion function is passed in,
and perform seamless completion on submit for interruptless buffers.
Signed-off-by: Christophe Leroy <redacted>
---
v2: Keep the UB flag to mark not completed buffers and seamlessly flag them as completed during next submit.
---
drivers/soc/fsl/qe/qmc.c | 44 ++++++++++++++++++++++++++++++----------
1 file changed, 33 insertions(+), 11 deletions(-)
@@ -461,9 +461,16 @@ int qmc_chan_write_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,ctrl=qmc_read16(&bd->cbd_sc);if(ctrl&(QMC_BD_TX_R|QMC_BD_TX_UB)){-/* We are full ... */-ret=-EBUSY;-gotoend;+if(!(ctrl&QMC_BD_TX_I)&&bd==chan->txbd_done){+if(ctrl&QMC_BD_TX_W)+chan->txbd_done=chan->txbds;+else+chan->txbd_done++;+}else{+/* We are full ... */+ret=-EBUSY;+gotoend;+}}qmc_write16(&bd->cbd_datlen,length);
@@ -475,6 +482,10 @@ int qmc_chan_write_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,/* Activate the descriptor */ctrl|=(QMC_BD_TX_R|QMC_BD_TX_UB);+if(complete)+ctrl|=QMC_BD_TX_I;+else+ctrl&=~QMC_BD_TX_I;wmb();/* Be sure to flush the descriptor before control update */qmc_write16(&bd->cbd_sc,ctrl);
You try to purge one descriptor for which the transfer is done but you do that
when you have no more free descriptors.
You end up with all descriptor "used". I think a better way to do that is
to purge all "done" descriptor configured to work without interrupts until a
descriptor with interrupt is found.
I have looked again at your code and looking for a free descriptor only when it
is needed is sufficient. You can forget my previous proposal.
Back to your code, I think you need to be sure that the descriptor you want to
re-use is really available and so you need to check the 'R' bit to be sure
that we are not with 'R' = 1 and 'UB' = 1 which means "BD is used, waiting for
a transfer".