RE: [PATCH 1/3] dmaengine: Add support for 2D DMA operation

["Aatif Mushtaq/Aatif Mushtaq" <aatif4.m@samsung.com>]

Hi Vinod,

> -----Original Message-----
> From: Aatif Mushtaq/Aatif Mushtaq <aatif4.m@samsung.com>
> Sent: 06 March 2025 12:06
> To: 'Vinod Koul' <vkoul@kernel.org>
> Cc: 'dmaengine@vger.kernel.org' <dmaengine@vger.kernel.org>; 'linux-
> kernel@vger.kernel.org' <linux-kernel@vger.kernel.org>;
> 'pankaj.dubey@samsung.com' <pankaj.dubey@samsung.com>;
> 'aswani.reddy@samsung.com' <aswani.reddy@samsung.com>
> Subject: RE: [PATCH 1/3] dmaengine: Add support for 2D DMA operation
> 
> 
> 
> > -----Original Message-----
> > From: Aatif Mushtaq/Aatif Mushtaq <aatif4.m@samsung.com>
> > Sent: 28 February 2025 15:03
> > To: 'Vinod Koul' <vkoul@kernel.org>
> > Cc: 'dmaengine@vger.kernel.org' <dmaengine@vger.kernel.org>; 'linux-
> > kernel@vger.kernel.org' <linux-kernel@vger.kernel.org>;
> > 'pankaj.dubey@samsung.com' <pankaj.dubey@samsung.com>;
> > 'aswani.reddy@samsung.com' <aswani.reddy@samsung.com>
> > Subject: RE: [PATCH 1/3] dmaengine: Add support for 2D DMA operation
> >
> >
> >
> > > -----Original Message-----
> > > From: Vinod Koul <vkoul@kernel.org>
> > > Sent: 27 February 2025 13:32
> > > To: Aatif Mushtaq <aatif4.m@samsung.com>
> > > Cc: dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > pankaj.dubey@samsung.com; aswani.reddy@samsung.com
> > > Subject: Re: [PATCH 1/3] dmaengine: Add support for 2D DMA operation
> > >
> > > On 10-02-25, 11:49, Aatif Mushtaq wrote:
> > > > Add a new dma engine API to support 2D DMA operations.
> > > > The API will be used to get the descriptor for 2D transfer based on
> > > > the 16-bit immediate to define the stride length between
> > > > consecuitive source address or destination address after every DMA
> > > > load and store instruction is processed.
> > >
> > > Why should we define a new API for this...? Why not use the sg or
> > > interleaved api for this?
> > >
> >
> > Thanks for pointing out, interleaved API can be used for this.
> > I will make the change
> >
> 
> While trying to make the change I realised that sg and interleaved
> APIs cannot be used for our use case.
> Interleaved API is used to transfer data from non-contiguous
> buffer to non-contiguous buffer and sg API is used to transfer
> non-contiguous buffer to contiguous buffer but both the APIs work
> on multiple data chunks where each chunk has its individual attributes
> and there is a tx descriptor for each data chunk.
> But in our case we have single tx descriptor to increment the source or
> destination after every DMA LOAD and STORE operation till the desired
> length of transfer is achieved, which means we don't have multiple data
> chunks which is required in case of interleaved and sg.
> The use case is to do memory to memory copy but not in a linear way,
> such that we can define a gap between each burst.
> 

I want to gently remind you to please review my approach.

> > > >
> > > > Signed-off-by: Aatif Mushtaq <aatif4.m@samsung.com>
> > > > ---
> > > >  include/linux/dmaengine.h | 25 +++++++++++++++++++++++++
> > > >  1 file changed, 25 insertions(+)
> > > >
> > > > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > > > index b137fdb56093..8a73b2147983 100644
> > > > --- a/include/linux/dmaengine.h
> > > > +++ b/include/linux/dmaengine.h
> > > > @@ -833,6 +833,7 @@ struct dma_filter {
> > > >   *	be called after period_len bytes have been transferred.
> > > >   * @device_prep_interleaved_dma: Transfer expression in a generic
> > way.
> > > >   * @device_prep_dma_imm_data: DMA's 8 byte immediate data to
> the
> > > dst
> > > > address
> > > > + * @device_prep_2d_dma_memcpy: prepares a 2D memcpy
> operation
> > > >   * @device_caps: May be used to override the generic DMA slave
> > > capabilities
> > > >   *	with per-channel specific ones
> > > >   * @device_config: Pushes a new configuration to a channel, return
> > > > 0 or an error @@ -938,6 +939,9 @@ struct dma_device {
> > > >  	struct dma_async_tx_descriptor *(*device_prep_dma_imm_data)(
> > > >  		struct dma_chan *chan, dma_addr_t dst, u64 data,
> > > >  		unsigned long flags);
> > > > +	struct dma_async_tx_descriptor
> > > *(*device_prep_2d_dma_memcpy)(
> > > > +		struct dma_chan *chan, dma_addr_t dest, dma_addr_t
src,
> > > > +		size_t len, u16 src_imm, u16 dest_imm, unsigned long
flags);
> > > >
> > > >  	void (*device_caps)(struct dma_chan *chan, struct
dma_slave_caps
> > > *caps);
> > > >  	int (*device_config)(struct dma_chan *chan, struct
> > > dma_slave_config
> > > > *config); @@ -1087,6 +1091,27 @@ static inline struct
> > > dma_async_tx_descriptor *dmaengine_prep_dma_memcpy(
> > > >  						    len, flags);
> > > >  }
> > > >
> > > > +/**
> > > > + * device_prep_2d_dma_memcpy() - Prepare a DMA 2D memcpy
> > > descriptor.
> > > > + * @chan: The channel to be used for this descriptor
> > > > + * @dest: Address of the destination data for a DMA channel
> > > > + * @src: Address of the source data for a DMA channel
> > > > + * @len: The total size of data
> > > > + * @src_imm: The immediate value to be added to the src address
> > > > +register
> > > > + * @dest_imm: The immediate value to be added to the dst address
> > > > +register
> > > > + * @flags: DMA engine flags
> > > > + */
> > > > +static inline struct dma_async_tx_descriptor
> > > *device_prep_2d_dma_memcpy(
> > > > +		struct dma_chan *chan, dma_addr_t dest, dma_addr_t
src,
> > > > +		size_t len, u16 src_imm, u16 dest_imm, unsigned long
flags) {
> > > > +	if (!chan || !chan->device || !chan->device-
> > > >device_prep_2d_dma_memcpy)
> > > > +		return NULL;
> > > > +
> > > > +	return chan->device->device_prep_2d_dma_memcpy(chan, dest,
> > > src, len,
> > > > +						       src_imm,
dest_imm,
> > > flags); }
> > > > +
> > > >  static inline bool dmaengine_is_metadata_mode_supported(struct
> > > dma_chan *chan,
> > > >  		enum dma_desc_metadata_mode mode)  {
> > > > --
> > > > 2.17.1
> > >
> > > --
> > > ~Vinod