dmaengine Archive on lore.kernel.org
 help / color / Atom feed
From: Vinod Koul <vkoul@kernel.org>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>,
	dmaengine@vger.kernel.org, Michal Simek <michal.simek@xilinx.com>,
	Hyun Kwon <hyun.kwon@xilinx.com>,
	Tejas Upadhyay <tejasu@xilinx.com>,
	Satish Kumar Nagireddy <SATISHNA@xilinx.com>
Subject: Re: [PATCH v3 2/6] dmaengine: Add interleaved cyclic transaction type
Date: Wed, 4 Mar 2020 10:43:01 +0530
Message-ID: <20200304051301.GS4148@vkoul-mobl> (raw)
In-Reply-To: <20200303192255.GN11333@pendragon.ideasonboard.com>

On 03-03-20, 21:22, Laurent Pinchart wrote:
> Hi Vinod,
> 
> On Tue, Mar 03, 2020 at 10:02:54AM +0530, Vinod Koul wrote:
> > On 02-03-20, 09:37, Laurent Pinchart wrote:
> > 
> > > > I would be more comfortable in calling an API to do so :)
> > > > The flow I am thinking is:
> > > > 
> > > > - prep cyclic1 txn
> > > > - submit cyclic1 txn
> > > > - call issue_pending() (cyclic one starts)
> > > > 
> > > > - prep cyclic2 txn
> > > > - submit cyclic2 txn
> > > > - signal_cyclic1_txn aka terminate_cookie()
> > > > - cyclic1 completes, switch to cyclic2 (dmaengine driver)
> > > > - get callback for cyclic1 (optional)
> > > > 
> > > > To check if hw supports terminate_cookie() or not we can check if the
> > > > callback support is implemented
> > > 
> > > Two questions though:
> > > 
> > > - Where is .issue_pending() called for cyclic2 in your above sequence ?
> > >   Surely it should be called somewhere, as the DMA engine API requires
> > >   .issue_pending() to be called for a transfer to be executed, otherwise
> > >   it stays in the submitted but not pending queue.
> > 
> > Sorry missed that one, I would do that after submit cyclic2 txn step and
> > then signal signal_cyclic1_txn termination
> 
> OK, that matches my understanding, good :-)
> 
> > > - With the introduction of a new .terminate_cookie() operation, we need
> > >   to specify that operation for all transfer types. What's its
> > 
> > Correct
> > 
> > >   envisioned semantics for non-cyclic transfers ? And how do DMA engine
> > >   drivers report that they support .terminate_cookie() for cyclic
> > >   transfers but not for other transfer types (the counterpart of
> > >   reporting, in my proposition, that .issue_pending() isn't supported
> > >   replace the current cyclic transfer) ?
> > 
> > Typically for dmaengine controller cyclic is *not* a special mode, only
> > change is that a list provided to controller is circular.
> 
> I don't agree with this. For cyclic transfers to be replaceable in a
> clean way, the feature must be specifically implemented at the hardware
> level. A DMA engine that supports chaining transfers with an explicit
> way to override that chaining, and without the logic to report if the
> inherent race was lost or not, really can't support this API.

Well chaining is a typical feature in dmaengine and making last chain
point to first makes it circular. I have seen couple of engines and this
was the implementation in the hardware.

There can exist special hardware for this purposes as well, but the
point is that the cyclic can be treated as circular list.

> Furthemore, for non-cyclic transfers, what would .terminate_cookie() do
> ? I need it to be defined as terminating the current transfer when it
> ends for the cyclic case, not terminating it immediately. All non-cyclic
> transfers terminate by themselves when they end, so what would this new
> operation do ?

I would use it for two purposes, cancelling txn but at the end of
current txn. I have couple of usages where this would
be helpful. Second in error handling where some engines do not support
aborting (unless we reset the whole controller)

But yes the .terminate_cookie() semantics should indicate if the
termination should be immediate or end of current txn. I see people
using it for both.

And with this I think it would make sense to also add this to
capabilities :)

> > So, the .terminate_cookie() should be a feature for all type of txn's.
> > If for some reason (dont discount what hw designers can do) a controller
> > supports this for some specific type(s), then they should return
> > -ENOTSUPP for cookies that do not support and let the caller know.
> 
> But then the caller can't know ahead of time, it will only find out when
> it's too late, and can't decide not to use the DMA engine if it doesn't
> support the feature. I don't think that's a very good option.

Agreed so lets go with adding these in caps.

-- 
~Vinod

  reply index

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-23  2:29 [PATCH v3 0/6] dma: Add Xilinx ZynqMP DPDMA driver Laurent Pinchart
2020-01-23  2:29 ` [PATCH v3 1/6] dt: bindings: dma: xilinx: dpdma: DT bindings for Xilinx DPDMA Laurent Pinchart
2020-01-23  2:29 ` [PATCH v3 2/6] dmaengine: Add interleaved cyclic transaction type Laurent Pinchart
2020-01-23  8:03   ` Peter Ujfalusi
2020-01-23  8:43     ` Vinod Koul
2020-01-23  8:51       ` Peter Ujfalusi
2020-01-23 12:23         ` Laurent Pinchart
2020-01-24  6:10           ` Vinod Koul
2020-01-24  8:50             ` Laurent Pinchart
2020-02-10 14:06               ` Laurent Pinchart
2020-02-13 13:29                 ` Vinod Koul
2020-02-13 13:48                   ` Laurent Pinchart
2020-02-13 14:07                     ` Vinod Koul
2020-02-13 14:15                       ` Peter Ujfalusi
2020-02-13 16:52                         ` Laurent Pinchart
2020-02-14  4:23                           ` Vinod Koul
2020-02-14 16:22                             ` Laurent Pinchart
2020-02-17 10:00                               ` Peter Ujfalusi
2020-02-19  9:25                                 ` Vinod Koul
2020-02-26 16:30                                   ` Laurent Pinchart
2020-03-02  3:47                                     ` Vinod Koul
2020-03-02  7:37                                       ` Laurent Pinchart
2020-03-03  4:32                                         ` Vinod Koul
2020-03-03 19:22                                           ` Laurent Pinchart
2020-03-04  5:13                                             ` Vinod Koul [this message]
2020-03-04  8:01                                               ` Laurent Pinchart
2020-03-04 15:37                                                 ` Vinod Koul
2020-03-04 16:00                                                   ` Laurent Pinchart
2020-03-04 16:24                                                     ` Vinod Koul
     [not found]                                                       ` <20200311155248.GA4772@pendragon.ideasonboard.com>
2020-03-18 15:14                                                         ` Laurent Pinchart
2020-03-25 16:00                                                           ` Laurent Pinchart
2020-03-26  7:02                                                         ` Vinod Koul
2020-04-08 17:00                                                           ` Laurent Pinchart
2020-04-15 15:12                                                             ` Laurent Pinchart
2020-03-06 14:49                                                     ` Peter Ujfalusi
2020-03-11 23:15                                                       ` Laurent Pinchart
2020-02-26 16:24                                 ` Laurent Pinchart
2020-03-02  3:42                                   ` Vinod Koul
2020-01-24  7:20           ` Peter Ujfalusi
2020-01-24  7:38             ` Peter Ujfalusi
2020-01-24  8:58               ` Laurent Pinchart
2020-01-24  8:56             ` Laurent Pinchart
2020-01-23  2:29 ` [PATCH v3 3/6] dmaengine: virt-dma: Use lockdep to check locking requirements Laurent Pinchart
2020-01-23  2:29 ` [PATCH v3 4/6] dmaengine: xilinx: dpdma: Add the Xilinx DisplayPort DMA engine driver Laurent Pinchart
2020-01-23  2:29 ` [PATCH v3 5/6] dmaengine: xilinx: dpdma: Add debugfs support Laurent Pinchart
2020-01-23  2:29 ` [PATCH v3 6/6] arm64: dts: zynqmp: Add DPDMA node Laurent Pinchart

Reply instructions:

You may reply publicly 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=20200304051301.GS4148@vkoul-mobl \
    --to=vkoul@kernel.org \
    --cc=SATISHNA@xilinx.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=hyun.kwon@xilinx.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=michal.simek@xilinx.com \
    --cc=peter.ujfalusi@ti.com \
    --cc=tejasu@xilinx.com \
    /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
	public-inbox-index dmaengine

Example config snippet for mirrors

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.git