All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jon Hunter <jonathanh@nvidia.com>
To: Peter Ujfalusi <peter.ujfalusi@ti.com>,
	Sameer Pujar <spujar@nvidia.com>, Vinod Koul <vkoul@kernel.org>
Cc: <dan.j.williams@intel.com>, <tiwai@suse.com>,
	<dmaengine@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<sharadg@nvidia.com>, <rlokhande@nvidia.com>,
	<dramesh@nvidia.com>, <mkumard@nvidia.com>,
	linux-tegra <linux-tegra@vger.kernel.org>
Subject: Re: [PATCH] [RFC] dmaengine: add fifo_size member
Date: Thu, 6 Jun 2019 11:49:16 +0100	[thread overview]
Message-ID: <a36302fc-3173-070b-5c97-7d2c55d5e2cc@nvidia.com> (raw)
In-Reply-To: <3f836a10-eaf3-f59b-7170-6fe937cf2e43@ti.com>


On 06/06/2019 11:22, Peter Ujfalusi wrote:

...

>>>> It does sounds like that FIFO_SIZE == src/dst_maxburst in your case as
>>>> well.
>>> Not exactly equal.
>>> ADMA burst_size can range from 1(WORD) to 16(WORDS)
>>> FIFO_SIZE can be adjusted from 16(WORDS) to 1024(WORDS) [can vary in
>>> multiples of 16]
>>
>> So I think that the key thing to highlight here, is that the as Sameer
>> highlighted above for the Tegra ADMA there are two values that need to
>> be programmed; the DMA client FIFO size and the max burst size. The ADMA
>> has register fields for both of these.
> 
> How does the ADMA uses the 'client FIFO size' and 'max burst size'
> values and what is the relation of these values to the peripheral side
> (ADMAIF)?

Per Sameer's previous comment, the FIFO size is used by the ADMA to
determine how much space is available in the FIFO. I assume the burst
size just limits how much data is transferred per transaction.

>> As you can see from the above the FIFO size can be much greater than the
>> burst size and so ideally both of these values would be passed to the DMA.
>>
>> We could get by with just passing the FIFO size (as the max burst size)
>> and then have the DMA driver set the max burst size depending on this,
>> but this does feel quite correct for this DMA. Hence, ideally, we would
>> like to pass both.
>>
>> We are also open to other ideas.
> 
> I can not find public documentation (I think they are walled off by
> registration), but correct me if I'm wrong:

No unfortunately, you are not wrong here :-(

> ADMAIF - peripheral side
>  - kind of a small DMA for audio preipheral(s)?

Yes this is the interface to the APE (audio processing engine) and data
sent to the ADMAIF is then sent across a crossbar to one of many
devices/interfaces (I2S, DMIC, etc). Basically a large mux that is user
configurable depending on the use-case.

>  - Variable FIFO size

Yes.

>  - sends DMA request to ADMA per words

From Sameer's notes it says the ADMAIF send a signal to the ADMA per
word, yes.

> ADMA - system DMA
>  - receives the DMA requests from ADMAIF
>  - counts the requests
>  - based on some threshold of the counter it will send/read from ADMAIF?
>   - maxburst number of words probably?

Sounds about right to me.

> ADMA needs to know the ADMAIF's FIFO size because, it is the one who is
> managing that FIFO from the outside, making sure that it does not over
> or underrun?

Yes.

> And it is the one who sets the pace (in effect the DMA burst size - how
> many bytes the DMA jumps between refills) of refills to the ADMAIF's FIFO?

Yes.

So currently, if you look at the ADMA driver
(drivers/dma/tegra210-adma.c) you will see we use the src/dst_maxburst
for the burst, but the FIFO size is hard-coded (see the
TEGRA210_FIFO_CTRL_DEFAULT and TEGRA186_FIFO_CTRL_DEFAULT definitions).
Ideally, we should not hard-code this but pass it.

Given that there are no current users of the ADMA upstream, we could
change the usage of the src/dst_maxburst, but being able to set the FIFO
size as well would be ideal.

Cheers
Jon

-- 
nvpublic

WARNING: multiple messages have this Message-ID (diff)
From: Jon Hunter <jonathanh@nvidia.com>
To: Peter Ujfalusi <peter.ujfalusi@ti.com>,
	Sameer Pujar <spujar@nvidia.com>, Vinod Koul <vkoul@kernel.org>
Cc: dan.j.williams@intel.com, tiwai@suse.com,
	dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
	sharadg@nvidia.com, rlokhande@nvidia.com, dramesh@nvidia.com,
	mkumard@nvidia.com, linux-tegra <linux-tegra@vger.kernel.org>
Subject: Re: [PATCH] [RFC] dmaengine: add fifo_size member
Date: Thu, 6 Jun 2019 11:49:16 +0100	[thread overview]
Message-ID: <a36302fc-3173-070b-5c97-7d2c55d5e2cc@nvidia.com> (raw)
In-Reply-To: <3f836a10-eaf3-f59b-7170-6fe937cf2e43@ti.com>


On 06/06/2019 11:22, Peter Ujfalusi wrote:

...

>>>> It does sounds like that FIFO_SIZE == src/dst_maxburst in your case as
>>>> well.
>>> Not exactly equal.
>>> ADMA burst_size can range from 1(WORD) to 16(WORDS)
>>> FIFO_SIZE can be adjusted from 16(WORDS) to 1024(WORDS) [can vary in
>>> multiples of 16]
>>
>> So I think that the key thing to highlight here, is that the as Sameer
>> highlighted above for the Tegra ADMA there are two values that need to
>> be programmed; the DMA client FIFO size and the max burst size. The ADMA
>> has register fields for both of these.
> 
> How does the ADMA uses the 'client FIFO size' and 'max burst size'
> values and what is the relation of these values to the peripheral side
> (ADMAIF)?

Per Sameer's previous comment, the FIFO size is used by the ADMA to
determine how much space is available in the FIFO. I assume the burst
size just limits how much data is transferred per transaction.

>> As you can see from the above the FIFO size can be much greater than the
>> burst size and so ideally both of these values would be passed to the DMA.
>>
>> We could get by with just passing the FIFO size (as the max burst size)
>> and then have the DMA driver set the max burst size depending on this,
>> but this does feel quite correct for this DMA. Hence, ideally, we would
>> like to pass both.
>>
>> We are also open to other ideas.
> 
> I can not find public documentation (I think they are walled off by
> registration), but correct me if I'm wrong:

No unfortunately, you are not wrong here :-(

> ADMAIF - peripheral side
>  - kind of a small DMA for audio preipheral(s)?

Yes this is the interface to the APE (audio processing engine) and data
sent to the ADMAIF is then sent across a crossbar to one of many
devices/interfaces (I2S, DMIC, etc). Basically a large mux that is user
configurable depending on the use-case.

>  - Variable FIFO size

Yes.

>  - sends DMA request to ADMA per words

From Sameer's notes it says the ADMAIF send a signal to the ADMA per
word, yes.

> ADMA - system DMA
>  - receives the DMA requests from ADMAIF
>  - counts the requests
>  - based on some threshold of the counter it will send/read from ADMAIF?
>   - maxburst number of words probably?

Sounds about right to me.

> ADMA needs to know the ADMAIF's FIFO size because, it is the one who is
> managing that FIFO from the outside, making sure that it does not over
> or underrun?

Yes.

> And it is the one who sets the pace (in effect the DMA burst size - how
> many bytes the DMA jumps between refills) of refills to the ADMAIF's FIFO?

Yes.

So currently, if you look at the ADMA driver
(drivers/dma/tegra210-adma.c) you will see we use the src/dst_maxburst
for the burst, but the FIFO size is hard-coded (see the
TEGRA210_FIFO_CTRL_DEFAULT and TEGRA186_FIFO_CTRL_DEFAULT definitions).
Ideally, we should not hard-code this but pass it.

Given that there are no current users of the ADMA upstream, we could
change the usage of the src/dst_maxburst, but being able to set the FIFO
size as well would be ideal.

Cheers
Jon

-- 
nvpublic

  reply	other threads:[~2019-06-06 10:49 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-30 11:30 [RFC] dmaengine: add fifo_size member Sameer Pujar
2019-04-30 11:30 ` [PATCH] " Sameer Pujar
2019-05-02  6:04 ` Vinod Koul
2019-05-02  6:04   ` [PATCH] " Vinod Koul
2019-05-02 10:53   ` Sameer Pujar
2019-05-02 12:25     ` Vinod Koul
2019-05-02 13:29       ` Sameer Pujar
2019-05-03 19:10         ` Peter Ujfalusi
2019-05-04 10:23         ` Vinod Koul
2019-05-06 13:04           ` Sameer Pujar
2019-05-06 15:50             ` Vinod Koul
2019-06-06  3:49               ` Sameer Pujar
2019-06-06  6:00                 ` Peter Ujfalusi
2019-06-06  6:41                   ` Sameer Pujar
2019-06-06  7:14                     ` Jon Hunter
2019-06-06 10:22                       ` Peter Ujfalusi
2019-06-06 10:49                         ` Jon Hunter [this message]
2019-06-06 10:49                           ` Jon Hunter
2019-06-06 11:54                           ` Peter Ujfalusi
2019-06-06 11:54                             ` Peter Ujfalusi
2019-06-06 12:37                             ` Jon Hunter
2019-06-06 12:37                               ` Jon Hunter
2019-06-06 13:45                               ` Dmitry Osipenko
2019-06-06 13:55                                 ` Dmitry Osipenko
2019-06-06 14:26                                   ` Jon Hunter
2019-06-06 14:26                                     ` Jon Hunter
2019-06-06 14:36                                     ` Jon Hunter
2019-06-06 14:36                                       ` Jon Hunter
2019-06-06 14:36                                     ` Dmitry Osipenko
2019-06-06 14:47                                       ` Jon Hunter
2019-06-06 14:47                                         ` Jon Hunter
2019-06-06 14:25                                 ` Jon Hunter
2019-06-06 14:25                                   ` Jon Hunter
2019-06-06 15:18                                   ` Dmitry Osipenko
2019-06-06 16:32                                     ` Jon Hunter
2019-06-06 16:32                                       ` Jon Hunter
2019-06-06 16:44                                       ` Dmitry Osipenko
2019-06-06 16:53                                         ` Jon Hunter
2019-06-06 16:53                                           ` Jon Hunter
2019-06-06 17:25                                           ` Dmitry Osipenko
2019-06-06 17:56                                             ` Dmitry Osipenko
2019-06-07  9:24                                             ` Jon Hunter
2019-06-07  9:24                                               ` Jon Hunter
2019-06-07  5:50                               ` Peter Ujfalusi
2019-06-07  5:50                                 ` Peter Ujfalusi
2019-06-07  9:18                                 ` Jon Hunter
2019-06-07  9:18                                   ` Jon Hunter
2019-06-07 10:27                                   ` Jon Hunter
2019-06-07 10:27                                     ` Jon Hunter
2019-06-07 12:17                                     ` Peter Ujfalusi
2019-06-07 12:17                                       ` Peter Ujfalusi
2019-06-07 12:58                                       ` Jon Hunter
2019-06-07 12:58                                         ` Jon Hunter
2019-06-07 13:35                                         ` Peter Ujfalusi
2019-06-07 13:35                                           ` Peter Ujfalusi
2019-06-07 20:53                                           ` Dmitry Osipenko
2019-06-10  8:01                                             ` Jon Hunter
2019-06-10  8:01                                               ` Jon Hunter
2019-06-10  7:59                                           ` Jon Hunter
2019-06-10  7:59                                             ` Jon Hunter
2019-06-13  4:43                 ` Vinod Koul
2019-06-17  7:07                   ` Sameer Pujar
2019-06-18  4:33                     ` Vinod Koul
2019-06-20 10:29                       ` Sameer Pujar
2019-06-24  6:26                         ` Vinod Koul
2019-06-25  2:57                           ` Sameer Pujar
2019-07-05  6:15                             ` Sameer Pujar
2019-07-15 15:42                               ` Sameer Pujar
2019-07-19  5:04                               ` Vinod Koul
2019-07-23  5:54                                 ` Sameer Pujar
2019-07-29  6:10                                   ` Vinod Koul
2019-07-31  9:48                                     ` Jon Hunter
2019-07-31 15:16                                       ` Vinod Koul
2019-08-02  8:51                                         ` Jon Hunter
2019-08-08 12:38                                           ` Vinod Koul
2019-08-19 15:56                                             ` Jon Hunter
2019-08-20 11:05                                               ` Vinod Koul
2019-09-16  9:02                                                 ` Sameer Pujar

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=a36302fc-3173-070b-5c97-7d2c55d5e2cc@nvidia.com \
    --to=jonathanh@nvidia.com \
    --cc=dan.j.williams@intel.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=dramesh@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=mkumard@nvidia.com \
    --cc=peter.ujfalusi@ti.com \
    --cc=rlokhande@nvidia.com \
    --cc=sharadg@nvidia.com \
    --cc=spujar@nvidia.com \
    --cc=tiwai@suse.com \
    --cc=vkoul@kernel.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 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.