All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Ujfalusi <peter.ujfalusi@ti.com>
To: Vinod Koul <vinod.koul@intel.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: Mon, 2 Oct 2017 14:24:12 +0300	[thread overview]
Message-ID: <11471508-b61c-7842-2080-7f5c2f292c2b@ti.com> (raw)
In-Reply-To: <20170926165413.GQ30097@localhost>




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.

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;

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' ?

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.


>>
>> The number of (dev_width * burst) is static, yes. Other DMA engines
>> might have similar interpretation, but returning the maximum length in
>> bytes sounded more generic for other engines to be able to adopt.
>>
>> Initially I had maxburst_cnt in struct dma_device for maximum burst
>> count within one SG, but it felt clumsy and not too intuitive either.
> 
>>
>> - Péter
>>
> 

- Péter

WARNING: multiple messages have this Message-ID (diff)
From: Peter Ujfalusi <peter.ujfalusi@ti.com>
To: Vinod Koul <vinod.koul@intel.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: Mon, 2 Oct 2017 14:24:12 +0300	[thread overview]
Message-ID: <11471508-b61c-7842-2080-7f5c2f292c2b@ti.com> (raw)
In-Reply-To: <20170926165413.GQ30097@localhost>




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.

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;

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' ?

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.


>>
>> The number of (dev_width * burst) is static, yes. Other DMA engines
>> might have similar interpretation, but returning the maximum length in
>> bytes sounded more generic for other engines to be able to adopt.
>>
>> Initially I had maxburst_cnt in struct dma_device for maximum burst
>> count within one SG, but it felt clumsy and not too intuitive either.
> 
>>
>> - Péter
>>
> 

- Péter

WARNING: multiple messages have this Message-ID (diff)
From: peter.ujfalusi@ti.com (Peter Ujfalusi)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/5] dmaengine: Support for querying maximum trasnfer length (of an SG element)
Date: Mon, 2 Oct 2017 14:24:12 +0300	[thread overview]
Message-ID: <11471508-b61c-7842-2080-7f5c2f292c2b@ti.com> (raw)
In-Reply-To: <20170926165413.GQ30097@localhost>

?


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.

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;

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' ?

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.


>>
>> The number of (dev_width * burst) is static, yes. Other DMA engines
>> might have similar interpretation, but returning the maximum length in
>> bytes sounded more generic for other engines to be able to adopt.
>>
>> Initially I had maxburst_cnt in struct dma_device for maximum burst
>> count within one SG, but it felt clumsy and not too intuitive either.
> 
>>
>> - P?ter
>>
> 

- P?ter

  reply	other threads:[~2017-10-02 11: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 [this message]
2017-10-02 11:24           ` Peter Ujfalusi
2017-10-02 11:24           ` Peter Ujfalusi
2017-10-08  5:25           ` Vinod Koul
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=11471508-b61c-7842-2080-7f5c2f292c2b@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-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=t-kristo@ti.com \
    --cc=vinod.koul@intel.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.