linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: zonque@gmail.com (Daniel Mack)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5 4/5] dma: mmp_pdma: add support for residue reporting
Date: Mon, 09 Dec 2013 20:00:51 +0100	[thread overview]
Message-ID: <52A61363.7060500@gmail.com> (raw)
In-Reply-To: <20130913045135.GI17188@intel.com>

Hi Vinod,

Very sorry for such a long delay. I only get to pick up on this topic
now. Hope you can remember the context of the discussion :)

On 09/13/2013 06:51 AM, Vinod Koul wrote:
> On Tue, Sep 10, 2013 at 05:46:19PM +0200, Daniel Mack wrote:
>> Hi Vinod,
>>
>> Sorry for the late response, I've been on vacations.
>>
>> On 25.08.2013 18:33, Vinod Koul wrote:
>>> On Wed, Aug 21, 2013 at 02:08:57PM +0200, Daniel Mack wrote:
>>>> In order to report the channel's residue, we walk the list of running
>>>> descriptors, look for those which match the cookie, and then try to find
>>>> the descriptor which defines upper and lower boundaries that embrace the
>>>> current transport pointer.
>>>
>>>>
>>>> +		/*
>>>> +		 * 'passed' will be latched once we found the descriptor which
>>>> +		 * lies inside the boundaries of the curr pointer. All
>>>> +		 * descriptors that occur in the list _after_ we found that
>>>> +		 * partially handled descriptor are still to be processed and
>>>> +		 * are hence added to the residual bytes counter.
>>>> +		 */
>>> do you have multiple descriptors for one transaction? Should be No.
>>
>> Sure, that can be the case. One transaction could span across multiple
>> descriptors, especially if its overall length exceeds the maximum length
>> of one descriptor.
> Yes and these addiional descriptor for a transacation should be child
> descriptors. 
> The child descriptors should have ->parent point to parent descriptor and ->next
> to next in the chain of children.

I looked into your idea and implemented it, but I really feel having an
explicit chain pointer for each descriptor is redundant information.

Hence, let me summarize again how the driver currently works:

* Any of the prep_* function will allocate a number of descriptors to
accommodate the payload, and link them all together via the 'node'
list_head, forming a transaction.

* The last descriptor in a transaction has the DCMD_ENDIRQEN flag set.

* When tx_submit() is called, each descriptor in the linked list will be
assigned a cookie, and then entire list of the transaction is appended
to the chain_pending list. At this point, the information of where a
transaction ends is no longer visible in the 'node' list head.

* start_pending_queue() moves the chain_pending entries to chain_running.

When determining the channel's residue, we need to find the transaction
currently in progress, and then count upwards until we reach the end of
the transaction chain.

So, while I could add a ->next pointer to the descriptors, and only use
that to link up descriptors of one transaction in the prep_*()
functions, that extra information doesn't actually buy us anything, as
the same information is already stored in the DCMD_ENDIRQEN flag.


I rebased the residue patch on top of Joe's cleanup work, and I can
resubmit if you want me to. Maybe that serves as a better foundation for
the ongoing discussion :)


Many thanks,
Daniel

  reply	other threads:[~2013-12-09 19:00 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-21 12:08 [PATCH v5 0/5] dma: pdma: cyclic, residue, DMA_PRIVATE Daniel Mack
2013-08-21 12:08 ` [PATCH v5 1/5] dma: mmp_pdma: only complete one transaction from dma_do_tasklet() Daniel Mack
2013-08-21 12:08 ` [PATCH v5 2/5] dma: mmp_pdma: don't clear DCMD_ENDIRQEN at end of pending chain Daniel Mack
2013-08-21 12:08 ` [PATCH v5 3/5] dma: mmp_pdma: add support for cyclic DMA descriptors Daniel Mack
2013-08-21 12:08 ` [PATCH v5 4/5] dma: mmp_pdma: add support for residue reporting Daniel Mack
2013-08-25 16:33   ` Vinod Koul
2013-09-10 15:46     ` Daniel Mack
2013-09-13  4:51       ` Vinod Koul
2013-12-09 19:00         ` Daniel Mack [this message]
2013-12-10 16:11           ` Vinod Koul
2013-12-11 15:09             ` Daniel Mack
2013-08-25 18:08   ` Russell King - ARM Linux
2013-09-10 15:46     ` Daniel Mack
2013-08-21 12:08 ` [PATCH v5 5/5] dma: mmp_pdma: set DMA_PRIVATE Daniel Mack
2013-08-25 16:35 ` [PATCH v5 0/5] dma: pdma: cyclic, residue, DMA_PRIVATE Vinod Koul
2013-08-26  9:22   ` Daniel Mack

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=52A61363.7060500@gmail.com \
    --to=zonque@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).