All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vinod Koul <vinod.koul@intel.com>
To: Peter Ujfalusi <peter.ujfalusi@ti.com>
Cc: dan.j.williams@intel.com, dmaengine@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, t-kristo@ti.com
Subject: Re: [PATCH 3/5] dmaengine: Support for querying maximum trasnfer length (of an SG element)
Date: Sun, 8 Oct 2017 10:55:15 +0530	[thread overview]
Message-ID: <20171008052514.GG30097@localhost> (raw)
In-Reply-To: <11471508-b61c-7842-2080-7f5c2f292c2b@ti.com>

On Mon, Oct 02, 2017 at 02:24:12PM +0300, Peter Ujfalusi wrote:
> 
> 
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
> On 2017-09-26 19:54, Vinod Koul wrote:
> >>>
> >>> not another callback :)
> >>>
> >>> on a serious note, why shouldn't this be one more capability in
> >>> dma_slave_caps. looking at next patch it seems static
> >>
> >> It is not really static, the size in bytes depends on the dev_width and
> >> the maxburst:
> >> dev_width * burst * (SZ_64K - 1);
> > 
> > well DMAengines work on FIFOs, in above you are giving length as SZ_64K - 1
> > 'items' which IIUC in DMAengine terms for bytes would always refer wrt width
> > used and burst applied.
> 
> I think we can live with this and let the user to figure out what to do
> with this information.

Right, plus a macro for conversion :) SO that users dont code buggy
conversions all over the place

> But I'm having hard time to figure out a good name for this. It is not
> the number of SGs we can support, but the number of 'items' within one
> SG that we have the limit. It could be:
> u32 max_bursts_per_sg;

this looks fine, another candidate I would use is words_per_sg and while at
it why tie it to sg? should we make it words_per_txn but then people should not
confuse with txn represented by a descriptor which can have multiple ....

> 
> which would also apply to period length (for cyclic) in a similar way.
> 
> > Return length in bytes does make sense (from user PoV), but then you need to
> > "know" the applied  width and burst. How do you decide those?
> 
> The number of items works eDMA and sDMA, but we also have the cpp41. It
> is a packet DMA and it has no understanding of bursts, address widths or
> any of the 'traditional' things. It only cares about the number of bytes
> we want to transfer and it has limitation of 4194303 bytes (21bits for
> length). This is again per SG. How this could report the
> 'max_bursts_per_sg' ?

hmmm that is intresting case, is this number coming from USB side?

> This was one of the reasons that I have settled with the callback.
> 
> What we can also do is to code this within the DMA drivers itself.
> 
> When setting up the transfer and we realize that one of the SG will not
> going to fit, we destroy what we have done so far, pass the sg list
> along with length/sg limit to create a new sg list where all sg item's
> length is under the limit. Then using this new sg list we can set up the
> transfer.
> 
> I'm not sure how hard is to do the sg list optimization, I see that
> sg_split() is not what we want so we might need to code this in
> dmaengine or in the scatterlist code.
> 
> We certainly don't want to verify all slave_sg transfers proactively to
> avoid adding latency when it is not necessary.

latency would be added at prepare, not when submitting..

-- 
~Vinod

WARNING: multiple messages have this Message-ID (diff)
From: vinod.koul@intel.com (Vinod Koul)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/5] dmaengine: Support for querying maximum trasnfer length (of an SG element)
Date: Sun, 8 Oct 2017 10:55:15 +0530	[thread overview]
Message-ID: <20171008052514.GG30097@localhost> (raw)
In-Reply-To: <11471508-b61c-7842-2080-7f5c2f292c2b@ti.com>

On Mon, Oct 02, 2017 at 02:24:12PM +0300, Peter Ujfalusi wrote:
> 
> 
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
> On 2017-09-26 19:54, Vinod Koul wrote:
> >>>
> >>> not another callback :)
> >>>
> >>> on a serious note, why shouldn't this be one more capability in
> >>> dma_slave_caps. looking at next patch it seems static
> >>
> >> It is not really static, the size in bytes depends on the dev_width and
> >> the maxburst:
> >> dev_width * burst * (SZ_64K - 1);
> > 
> > well DMAengines work on FIFOs, in above you are giving length as SZ_64K - 1
> > 'items' which IIUC in DMAengine terms for bytes would always refer wrt width
> > used and burst applied.
> 
> I think we can live with this and let the user to figure out what to do
> with this information.

Right, plus a macro for conversion :) SO that users dont code buggy
conversions all over the place

> But I'm having hard time to figure out a good name for this. It is not
> the number of SGs we can support, but the number of 'items' within one
> SG that we have the limit. It could be:
> u32 max_bursts_per_sg;

this looks fine, another candidate I would use is words_per_sg and while at
it why tie it to sg? should we make it words_per_txn but then people should not
confuse with txn represented by a descriptor which can have multiple ....

> 
> which would also apply to period length (for cyclic) in a similar way.
> 
> > Return length in bytes does make sense (from user PoV), but then you need to
> > "know" the applied  width and burst. How do you decide those?
> 
> The number of items works eDMA and sDMA, but we also have the cpp41. It
> is a packet DMA and it has no understanding of bursts, address widths or
> any of the 'traditional' things. It only cares about the number of bytes
> we want to transfer and it has limitation of 4194303 bytes (21bits for
> length). This is again per SG. How this could report the
> 'max_bursts_per_sg' ?

hmmm that is intresting case, is this number coming from USB side?

> This was one of the reasons that I have settled with the callback.
> 
> What we can also do is to code this within the DMA drivers itself.
> 
> When setting up the transfer and we realize that one of the SG will not
> going to fit, we destroy what we have done so far, pass the sg list
> along with length/sg limit to create a new sg list where all sg item's
> length is under the limit. Then using this new sg list we can set up the
> transfer.
> 
> I'm not sure how hard is to do the sg list optimization, I see that
> sg_split() is not what we want so we might need to code this in
> dmaengine or in the scatterlist code.
> 
> We certainly don't want to verify all slave_sg transfers proactively to
> avoid adding latency when it is not necessary.

latency would be added at prepare, not when submitting..

-- 
~Vinod

  reply	other threads:[~2017-10-08  5:25 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-12 10:44 [PATCH 0/5] dmaengine: core/edma/omap-dma: maximum SG len reporting Peter Ujfalusi
2017-09-12 10:44 ` Peter Ujfalusi
2017-09-12 10:44 ` Peter Ujfalusi
2017-09-12 10:44 ` [PATCH 1/5] dmaengine: edma: Implement protection for invalid max_burst Peter Ujfalusi
2017-09-12 10:44   ` Peter Ujfalusi
2017-09-12 10:44   ` Peter Ujfalusi
2017-09-21 17:10   ` Vinod Koul
2017-09-21 17:10     ` Vinod Koul
2017-09-21 17:10     ` Vinod Koul
2017-09-12 10:44 ` [PATCH 2/5] dmaengine: omap-dma: " Peter Ujfalusi
2017-09-12 10:44   ` Peter Ujfalusi
2017-09-12 10:44   ` Peter Ujfalusi
2017-09-21 17:12   ` Vinod Koul
2017-09-21 17:12     ` Vinod Koul
2017-09-21 17:12     ` Vinod Koul
2017-09-12 10:44 ` [PATCH 3/5] dmaengine: Support for querying maximum trasnfer length (of an SG element) Peter Ujfalusi
2017-09-12 10:44   ` Peter Ujfalusi
2017-09-12 10:44   ` Peter Ujfalusi
2017-09-21 17:14   ` Vinod Koul
2017-09-21 17:14     ` Vinod Koul
2017-09-21 17:14     ` Vinod Koul
2017-09-22  9:39     ` Peter Ujfalusi
2017-09-22  9:39       ` Peter Ujfalusi
2017-09-22  9:39       ` Peter Ujfalusi
2017-09-26 16:54       ` Vinod Koul
2017-09-26 16:54         ` Vinod Koul
2017-10-02 11:24         ` Peter Ujfalusi
2017-10-02 11:24           ` Peter Ujfalusi
2017-10-02 11:24           ` Peter Ujfalusi
2017-10-08  5:25           ` Vinod Koul [this message]
2017-10-08  5:25             ` Vinod Koul
2017-10-11 15:47             ` Peter Ujfalusi
2017-10-11 15:47               ` Peter Ujfalusi
2017-10-11 15:47               ` Peter Ujfalusi
2017-10-12 13:57               ` Vinod Koul
2017-10-12 13:57                 ` Vinod Koul
2017-09-12 10:44 ` [PATCH 4/5] dmaengine: edma: Implement device_get_max_len callback Peter Ujfalusi
2017-09-12 10:44   ` Peter Ujfalusi
2017-09-12 10:44   ` Peter Ujfalusi
2017-09-12 10:44 ` [PATCH 5/5] dmaengine: omap-dma: " Peter Ujfalusi
2017-09-12 10:44   ` Peter Ujfalusi
2017-09-12 10:44   ` Peter Ujfalusi

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=20171008052514.GG30097@localhost \
    --to=vinod.koul@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=peter.ujfalusi@ti.com \
    --cc=t-kristo@ti.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.