Re: [PATCH v2 1/6] dma: Indicate residue granularity in dma_slave_caps

[Vinod Koul <vinod.koul@intel.com>]

On Mon, Jan 06, 2014 at 11:45:57AM +0100, Lars-Peter Clausen wrote:
> This patch adds a new field to the dma_slave_caps struct which indicates the
> granularity with which the driver is able to update the residue field of the
> dma_tx_state struct. Making this information available to dmaengine users allows
> them to make better decisions on how to operate. E.g. for audio certain features
> like wakeup less operation or timer based scheduling only make sense and work
> correctly if the reported residue is fine-grained enough.
> 
> Right now four different levels of granularity are supported:
> 	* DESCRIPTOR: The DMA channel is only able to tell whether a descriptor has
> 	  been completed or not, which means residue reporting is not supported by
> 	  this channel. The residue field of the dma_tx_state field will always be
> 	  0.
> 	* SEGMENT: The DMA channel updates the residue field after each successfully
> 	  completed segment of the transfer (For cyclic transfers this is after each
> 	  period). This is typically implemented by having the hardware generate an
> 	  interrupt after each transferred segment and then the drivers updates the
> 	  outstanding residue by the size of the segment. Another possibility is if
> 	  the hardware supports SG and the segment descriptor has a field which gets
> 	  set after the segment has been completed. The driver then counts the
> 	  number of segments without the flag set to compute the residue.
> 	* BURST: The DMA channel updates the residue field after each transferred
> 	  burst. This is typically only supported if the hardware has a progress
> 	  register of some sort (E.g. a register with the current read/write address
> 	  or a register with the amount of bursts/beats/bytes that have been
> 	  transferred or still need to be transferred).
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
> Changes since v1:
> 	* Drop BYTE granularity level as there has been consent that it is not
> 	  really useful.
> ---
>  include/linux/dmaengine.h | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index ed92b30..74d5cb2 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -364,6 +364,19 @@ struct dma_slave_config {
>  	unsigned int slave_id;
>  };
>  
> +/**
> + * enum dma_residue_granularity - Granularity of the reported transfer residue
> + * @DMA_RESIDUE_GRANULATRITY_DESCRIPTOR: Residue reporting is not supported.
> + * @DMA_RESIDUE_GRANULATRITY_SEGMENT: Residue is updated with each completed
> + *  segment in the descriptor.
> + * @DMA_RESIDUE_GRANULATRITY_BURST: Residue is updated with each transfered burst
> + */
> +enum dma_residue_granularity {
> +	DMA_RESIDUE_GRANULARITY_DESCRIPTOR = 0,
> +	DMA_RESIDUE_GRANULARITY_SEGMENT = 1,
> +	DMA_RESIDUE_GRANULARITY_BURST = 2,
This is too longish macro :) Cna we just have DMA_RESIDUE_XXXX. Do we really
need granularity?

Also segment is not too obvious, block is genrally used in DMA controllers for
this, though not too relgious about it!

Can you also add bit more detail in enum descriptions. Move a bit from patch
description above :)


> +};
> +
>  /* struct dma_slave_caps - expose capabilities of a slave channel only
>   *
>   * @src_addr_widths: bit mask of src addr widths the channel supports
> @@ -374,6 +387,7 @@ struct dma_slave_config {
>   * 	should be checked by controller as well
>   * @cmd_pause: true, if pause and thereby resume is supported
>   * @cmd_terminate: true, if terminate cmd is supported
> + * @residue_granularity: granularity of the reported transfer residue
>   */
>  struct dma_slave_caps {
>  	u32 src_addr_widths;
> @@ -381,6 +395,7 @@ struct dma_slave_caps {
>  	u32 directions;
>  	bool cmd_pause;
>  	bool cmd_terminate;
> +	enum dma_residue_granularity residue_granularity;
enum dma_residue granularity; would be fine too...

--
~Vinod