dmaengine Archive on lore.kernel.org
 help / color / Atom feed
From: Peter Ujfalusi <peter.ujfalusi@ti.com>
To: Vinod Koul <vkoul@kernel.org>
Cc: <dan.j.williams@intel.com>, <dmaengine@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-omap@vger.kernel.org>
Subject: Re: [PATCH] dmaengine: ti: omap-dma: Improved memcpy polling support
Date: Sat, 13 Jul 2019 00:10:04 +0300
Message-ID: <b180a077-a2c7-9f1e-cf0b-5bdf986a78fc@ti.com> (raw)
In-Reply-To: <20190705062334.GV2911@vkoul-mobl>



On 5.7.2019 9.23, Vinod Koul wrote:
> On 18-06-19, 16:24, Peter Ujfalusi wrote:
>> When a DMA client driver does not set the DMA_PREP_INTERRUPT because it
>> does not want to use interrupts for DMA completion or because it can not
>> rely on DMA interrupts due to executing the memcpy when interrupts are
>> disabled it will poll the status of the transfer.
>>
>> If the interrupts are enabled then the cookie will be set completed in the
>> interrupt handler so only check in HW completion when the polling is really
>> needed.
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> ---
>> Hi,
>>
>> This patch fine-tunes the omap-dma polled memcpy support to be inline with how
>> the EDMA driver is handling it.
>>
>> The polled completion can be tested by applying:
>> https://patchwork.kernel.org/patch/10966499/
>>
>> and run the dmatest with polled = 1 on boards where sDMA is used.
>>
>> Or boot up any dra7 family device with display enabled. The workaround for DMM
>> errata i878 uses polled DMA memcpy.
>>
>> Regards,
>> Peter
>>
>>  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
>> index 5ba7d8485026..75d8f7e13c8d 100644
>> --- a/drivers/dma/ti/omap-dma.c
>> +++ b/drivers/dma/ti/omap-dma.c
>> @@ -94,6 +94,7 @@ struct omap_desc {
>>  	bool using_ll;
>>  	enum dma_transfer_direction dir;
>>  	dma_addr_t dev_addr;
>> +	bool polled;
>>  
>>  	int32_t fi;		/* for OMAP_DMA_SYNC_PACKET / double indexing */
>>  	int16_t ei;		/* for double indexing */
>> @@ -834,20 +835,10 @@ static enum dma_status omap_dma_tx_status(struct dma_chan *chan,
>>  
>>  	ret = dma_cookie_status(chan, cookie, txstate);
>>  
>> -	if (!c->paused && c->running) {
>> -		uint32_t ccr = omap_dma_chan_read(c, CCR);
>> -		/*
>> -		 * The channel is no longer active, set the return value
>> -		 * accordingly
>> -		 */
>> -		if (!(ccr & CCR_ENABLE))
>> -			ret = DMA_COMPLETE;
>> -	}
>> -
>> +	spin_lock_irqsave(&c->vc.lock, flags);
>>  	if (ret == DMA_COMPLETE || !txstate)
>> -		return ret;
>> +		goto out;
>>  
>> -	spin_lock_irqsave(&c->vc.lock, flags);
>>  	vd = vchan_find_desc(&c->vc, cookie);
>>  	if (vd) {
>>  		txstate->residue = omap_dma_desc_size(to_omap_dma_desc(&vd->tx));
>> @@ -868,6 +859,23 @@ static enum dma_status omap_dma_tx_status(struct dma_chan *chan,
>>  	}
>>  	if (ret == DMA_IN_PROGRESS && c->paused)
>>  		ret = DMA_PAUSED;
>> +
>> +out:
>> +	if (ret == DMA_IN_PROGRESS && c->running && c->desc &&
>> +	    c->desc->polled && c->desc->vd.tx.cookie == cookie) {
> 
> heh, that makes quite a read!
> 
> checking DMA_IN_PROGRESS should not make sense, as we should bail out at
> the start if it is completed
> 
> I think other can be optimzed to make it a better read!

True, a simple re-ordering should make it easier to read or to split
them up as two if() checks. I'll try to figure out something to simplify it.

> 
>> +		uint32_t ccr = omap_dma_chan_read(c, CCR);
>> +		/*
>> +		 * The channel is no longer active, set the return value
>> +		 * accordingly
>> +		 */
>> +		if (!(ccr & CCR_ENABLE)) {
>> +			struct omap_desc *d = c->desc;
>> +			ret = DMA_COMPLETE;
>> +			omap_dma_start_desc(c);
>> +			vchan_cookie_complete(&d->vd);
>> +		}
>> +	}
>> +
>>  	spin_unlock_irqrestore(&c->vc.lock, flags);
>>  
>>  	return ret;
>> @@ -1233,7 +1241,10 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_memcpy(
>>  	d->ccr = c->ccr;
>>  	d->ccr |= CCR_DST_AMODE_POSTINC | CCR_SRC_AMODE_POSTINC;
>>  
>> -	d->cicr = CICR_DROP_IE | CICR_FRAME_IE;
>> +	if (tx_flags & DMA_PREP_INTERRUPT)
>> +		d->cicr |= CICR_FRAME_IE;
>> +	else
>> +		d->polled = true;
>>  
>>  	d->csdp = data_type;
>>  
>> -- 
>> Peter
>>
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

-- 
Peter

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

      reply index

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-18 13:24 Peter Ujfalusi
2019-07-05  6:23 ` Vinod Koul
2019-07-12 21:10   ` Peter Ujfalusi [this message]

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b180a077-a2c7-9f1e-cf0b-5bdf986a78fc@ti.com \
    --to=peter.ujfalusi@ti.com \
    --cc=dan.j.williams@intel.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

dmaengine Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dmaengine/0 dmaengine/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dmaengine dmaengine/ https://lore.kernel.org/dmaengine \
		dmaengine@vger.kernel.org dmaengine@archiver.kernel.org
	public-inbox-index dmaengine


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.dmaengine


AGPL code for this site: git clone https://public-inbox.org/ public-inbox