Re: [RFC PATCH 2/3] dmaengine: add peripheral configuration

[Peter Ujfalusi]

Hi Vinod,

On 25/08/2020 14.02, Vinod Koul wrote:
>> The only thing which might be an issue is that with the DMA_PREP_CMD the
>> config_data is dma_addr_t (via dmaengine_prep_slave_single).
> 
> Yes I came to same conclusion
> 
>>> I did have a prototype with metadata but didnt work very well, the
>>> problem is it assumes metadata for tx/rx but here i send the data
>>> everytime from client data.
>>
>> Yes, the intended use case for metadata (per descriptor!) is for
>> channels where each transfer might have different metadata needed for
>> the given transfer (tx/rx).
>>
>> In your case you have semi static peripheral configuration data, which
>> is not really changing between transfers.
>>
>> A compromise would be to add:
>> void *peripheral_config;
>> to the dma_slave_config, move the set_config inside of the device
>> specific struct you are passing from a client to the core?
> 
> That sounds more saner to me and uses existing interfaces cleanly. I
> think I like this option ;-)

The other option would be to use the descriptor metadata support and
that might be even cleaner.

In gpi_create_tre() via gpi_prep_slave_sg() you would set up the
desc->tre[1] and desc->tre[2] for TX
desc->tre[2] for RX
in the desc, you add a new variable, let's say first_tre and
set it to 1 for TX, 2 for RX.

If you need to send a config, you attach it via either way metadata
support allows you (get the pointer to desc->tre[0] or give a config
struct to the DMA driver.

In the metadata handler, you check if the transfer is TX, if it is, then
you update desc->tre[0] (or give the pointer to the client) and update
the first_tre to 0.

In issue_pending, or a small helper which can be used to start the
transfer you would do the queuing instead of prepare time:
for (i = gpi_desc->first_tre; i < MAX_TRE; i++)
	gpi_queue_xfer(gpii, gpii_chan,  &gpi_desc->tre[i], &wp);

With this change it should work neatly without any change to
dma_slave_config.

> 
>>>> I'm concerned about the size increase of dma_slave_config (it grows by
>>>>> 30 bytes) and for DMAs with hundreds of channels (UDMA) it will add up
>>>> to a sizeable amount.
>>>
>>> I agree that is indeed a valid concern, that is the reason I tagged this
>>> as a RFC patch ;-)
>>>
>>> I see the prep_cmd is a better approach for this, anyone else has better
>>> suggestions?
>>>
>>> Thanks for looking in.
>>>
>>
>> - Péter
>>
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

- Péter

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