LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Rameshwar Sahu <rsahu@apm.com>
To: Vinod Koul <vinod.koul@intel.com>
Cc: dan.j.williams@intel.com, dmaengine@vger.kernel.org,
	Arnd Bergmann <arnd@arndb.de>,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, jcm@redhat.com,
	patches@apm.com, Loc Ho <lho@apm.com>
Subject: Re: [PATCH v7 1/3] dmaengine: Add support for APM X-Gene SoC DMA engine driver
Date: Mon, 16 Mar 2015 23:01:56 +0530	[thread overview]
Message-ID: <CAFd313xG4EZdoS4vMxq4TW7v+xc_OEd3uC1R4LW3=Xs3+JzY9w@mail.gmail.com> (raw)
In-Reply-To: <20150316162613.GO32683@intel.com>

Hi Vinod,

On Mon, Mar 16, 2015 at 9:56 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Mon, Mar 16, 2015 at 05:24:34PM +0530, Rameshwar Sahu wrote:
>> >> >> +static void xgene_dma_free_desc_list_reverse(struct xgene_dma_chan *chan,
>> >> >> +                                          struct list_head *list)
>> >> > do we really care about free order?
>> >>
>> >> Yes it start dellocation of descriptor by tail.
>> > and why by tail is not clear.
>> We can free allocated descriptor in forward order from head or in
>> reverse order, I just followed here fsldma.c driver.
>> Does this make sense ??
> No, you have two APIs to free list. Why do you need two?

Yes, basically we have tow API to free list.
xgene_dma_free_desc_list_reverse will call if any failure in
allocation of memory from DMA pool in prep routines.
Like e.g. in prep routing we have some descriptors allocated and still
need to get descriptor to complete the DMA request and failure happen,
so we need to free all allocated descriptor.

>
>>
>>
>> >
>> >> > where are you mapping dma buffers?
>> >>
>> >>  I didn't get you here. Can you please explain me here what you mean.
>> >> As per my understanding client should map the dma buffer and give the
>> >> physical address and size to this callback prep routines.
>> > not for memcpy, that is true for slave transfers
>> >
>> > For mempcy the idea is that drivers will do buffer mapping
>>
>> Still I am clear here, why memcpy will do buffer mapping, I see other
>> drivers and also async_memcpy.c , they only map it and pass mapped
>> physical dma address to driver.
>>
>> Buffer mapping mean you here is dma_map_xxx ?? Am I correct.
> Yes

I have confusion here, I don't see any driver dma buffer mapping in
prep_dma_memcpy.
Can you please clear me here if driver does this on behalf of client,
like any example so that I can proceed further.
>
>>
>> >
>> >> > why are you calling this here, status check shouldnt do this...
>> >>
>> >> Okay, I will remove it.
>> >>
>> >>
>> >> >> +                     spin_unlock_bh(&chan->lock);
>> >> >> +                     return DMA_IN_PROGRESS;
>> >> > residue here is size of transacation.
>> >>
>> >> We can't calculate here residue size. We don't have any controller
>> >> register which will tell about remaining transaction size.
>> > Okay if you cant calculate residue why do we have this fn?
>>
>> So basically case here for me is completion of dma descriptor
>> submitted to hw is not same as order of submission to hw.
>> So scenario coming in multithread running :e.g. let's assume we have
>> submitted two descriptors first has cookie 1001 and second has 1002,
>> now 1002 is completed first, so updated last_completed_cookie as 1002
>> but not yer checked for dma_tx_status, and then first cookie completes
>> and update last_completed_cookie as 1001, now second transaction check
>> for tx_status and it get DMA_IN_PROGRESS, because
>> last_completed_cookie(1001) is less than second transaction's
>> cookie(1002).
>>
>> Due to this issue I am traversing that transaction in pending list and
>> running list, if not there means we are done.
>>
>> Does this make sense??
> That only convinces me that there is something not so correct.
>
> To help me understand pls let me know if below is fine:
> - for a physical channel, do you submit multiple transactions?

Yes

> - if yes, how does DMA deal with multiple transactions, how does it schedule
>   them?

So , basically we submit multiple descriptor to dma physical channel,
and dma engine execute it one by one and give us completion callback.
So in this way we expect callback on same order as submission order
and it does also, no issue.

But problem is with supporting p+q offload, here we have P
functionality supports in dma physical channel 0 and Q functionality
supports in dma physical channel 1. So for pq we need to submit two
descriptor, one to channel 0 and second to channel1, in this case we
can't expect the completion order, because channnel 0 can finish P
before Q or vice versa, and we need to wait to complete both before
calling client callback() and completing cookie.
Second thing we submit memcpy and sg on same channel, and can complete
before even though if it submitted after PQ.

>
> --
> ~Vinod
> --
> 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

  reply	other threads:[~2015-03-16 17:32 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-12 11:15 [PATCH v7 0/3] dmaengine: APM X-Gene SoC DMA engine driver support Rameshwar Prasad Sahu
2015-03-12 11:15 ` [PATCH v7 1/3] dmaengine: Add support for APM X-Gene SoC DMA engine driver Rameshwar Prasad Sahu
2015-03-16  9:27   ` Vinod Koul
2015-03-16 10:30     ` Rameshwar Sahu
2015-03-16 11:27       ` Vinod Koul
2015-03-16 11:54         ` Rameshwar Sahu
2015-03-16 16:26           ` Vinod Koul
2015-03-16 17:31             ` Rameshwar Sahu [this message]
2015-03-17  9:33               ` Rameshwar Sahu
2015-03-17 10:19                 ` Vinod Koul
2015-03-17 10:38                   ` Rameshwar Sahu
2015-03-16 11:02     ` Rameshwar Sahu
2015-03-12 11:15 ` [PATCH v7 2/3] arm64: dts: Add APM X-Gene DMA device and DMA clock DTS nodes Rameshwar Prasad Sahu
2015-03-12 11:15 ` [PATCH v7 3/3] Documentation: dma: Add APM X-Gene SoC DMA engine driver documentation Rameshwar Prasad Sahu
2015-03-16  9:29   ` Vinod Koul
2015-03-16 10:31     ` Rameshwar Sahu

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='CAFd313xG4EZdoS4vMxq4TW7v+xc_OEd3uC1R4LW3=Xs3+JzY9w@mail.gmail.com' \
    --to=rsahu@apm.com \
    --cc=arnd@arndb.de \
    --cc=dan.j.williams@intel.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=jcm@redhat.com \
    --cc=lho@apm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@apm.com \
    --cc=vinod.koul@intel.com \
    --subject='Re: [PATCH v7 1/3] dmaengine: Add support for APM X-Gene SoC DMA engine driver' \
    /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

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