Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v7 03/12] dmaengine: doc: Add sections for per descriptor
 metadata support
To:     Vinod Koul <vkoul@kernel.org>
CC:     <robh+dt@kernel.org>, <nm@ti.com>, <ssantosh@kernel.org>,
        <dan.j.williams@intel.com>, <dmaengine@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <grygorii.strashko@ti.com>, <lokeshvutla@ti.com>,
        <t-kristo@ti.com>, <tony@atomide.com>, <j-keerthy@ti.com>,
        <vigneshr@ti.com>
References: <20191209094332.4047-1-peter.ujfalusi@ti.com>
 <20191209094332.4047-4-peter.ujfalusi@ti.com>
 <20191220082810.GJ2536@vkoul-mobl>
From:   Peter Ujfalusi <peter.ujfalusi@ti.com>
Message-ID: <4508bc1c-d424-3285-cb47-d32a4d25b2c9@ti.com>
Date:   Fri, 20 Dec 2019 11:52:10 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.9.0
MIME-Version: 1.0
In-Reply-To: <20191220082810.GJ2536@vkoul-mobl>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Vinod,

On 20/12/2019 10.28, Vinod Koul wrote:
> Hi Peter,
> 
> On 09-12-19, 11:43, Peter Ujfalusi wrote:
> 
>> +  Optional: per descriptor metadata
>> +  ---------------------------------
>> +  DMAengine provides two ways for metadata support.
>> +
>> +  DESC_METADATA_CLIENT
>> +
>> +    The metadata buffer is allocated/provided by the client driver and it is
>> +    attached to the descriptor.
>> +
>> +  .. code-block:: c
>> +
>> +     int dmaengine_desc_attach_metadata(struct dma_async_tx_descriptor *desc,
>> +				   void *data, size_t len);
>> +
>> +  DESC_METADATA_ENGINE
>> +
>> +    The metadata buffer is allocated/managed by the DMA driver. The client
> 
> and when would it be freed?

It is not defined as it could be driver dependent, but afaik we have
defined (which I'm not sure why it is not here or in the code) that in
DESC_METADATA_ENGINE case the metadata pointer is valid for the client
between the time it got the desc (via prep call) and the execution of
the completion callback.
Iow, DESC_METADATA_ENGINE does not make any sense if the client want to
receive metadata back and does not provide a callback.

I will extend the documentation and comment in the code to reflect this.

>> +    driver can ask for the pointer, maximum size and the currently used size of
>> +    the metadata and can directly update or read it.
>> +
>> +  .. code-block:: c
>> +
>> +     void *dmaengine_desc_get_metadata_ptr(struct dma_async_tx_descriptor *desc,
>> +		size_t *payload_len, size_t *max_len);
>> +
>> +     int dmaengine_desc_set_metadata_len(struct dma_async_tx_descriptor *desc,
>> +		size_t payload_len);
>> +
>> +  Client drivers can query if a given mode is supported with:
>> +
>> +  .. code-block:: c
>> +
>> +     bool dmaengine_is_metadata_mode_supported(struct dma_chan *chan,
>> +		enum dma_desc_metadata_mode mode);
>> +
>> +  Depending on the used mode client drivers must follow different flow.
>> +
>> +  DESC_METADATA_CLIENT
>> +
>> +    - DMA_MEM_TO_DEV / DEV_MEM_TO_MEM:
>> +      1. prepare the descriptor (dmaengine_prep_*)
>> +         construct the metadata in the client's buffer
>> +      2. use dmaengine_desc_attach_metadata() to attach the buffer to the
>> +         descriptor
>> +      3. submit the transfer
> 
> This is simpler, txn finished the metadata would be freed up right?

It is up to the client driver what it does with the provided buffer. As
for what the DMA driver does is not documented as it is not relevant and
can be different by different HW or SW implementation.

>> +    - DMA_DEV_TO_MEM:
>> +      1. prepare the descriptor (dmaengine_prep_*)
>> +      2. use dmaengine_desc_attach_metadata() to attach the buffer to the
>> +         descriptor
>> +      3. submit the transfer
>> +      4. when the transfer is completed, the metadata should be available in the
>> +         attached buffer
> 
> and when and how would driver free that up :)

It is up to the client, it manages the buffer. It can reuse it and
attach it to another descriptor or free it up or pass it to other
layers, return it to other layer.

>> +
>> +  DESC_METADATA_ENGINE
>> +
>> +    - DMA_MEM_TO_DEV / DEV_MEM_TO_MEM:
>> +      1. prepare the descriptor (dmaengine_prep_*)
>> +      2. use dmaengine_desc_get_metadata_ptr() to get the pointer to the
>> +         engine's metadata area
>> +      3. update the metadata at the pointer
>> +      4. use dmaengine_desc_set_metadata_len()  to tell the DMA engine the
>> +         amount of data the client has placed into the metadata buffer
>> +      5. submit the transfer
>> +    - DMA_DEV_TO_MEM:
>> +      1. prepare the descriptor (dmaengine_prep_*)
>> +      2. submit the transfer
>> +      3. on transfer completion, use dmaengine_desc_get_metadata_ptr() to get the
>> +         pointer to the engine's metadata area
>> +      4. Read out the metadata from the pointer
>> +
>> +  .. note::
>> +
>> +     Mixed use of DESC_METADATA_CLIENT / DESC_METADATA_ENGINE is not allowed,
>> +     client drivers must use either of the modes per descriptor.
> 
> We should check that if not done already!

Yes, we check it.


- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki