On Fri, May 29, 2020 at 01:24:00AM +0300, Serge Semin wrote:
> IP core of the DW DMA controller may be synthesized with different
> max burst length of the transfers per each channel. According to Synopsis
> having the fixed maximum burst transactions length may provide some
> performance gain. At the same time setting up the source and destination
> multi size exceeding the max burst length limitation may cause a serious
> problems. In our case the DMA transaction just hangs up. In order to fix
> this lets introduce the max burst length platform config of the DW DMA
> controller device and don't let the DMA channels configuration code
> exceed the burst length hardware limitation.
>
> Note the maximum burst length parameter can be detected either in runtime
> from the DWC parameter registers or from the dedicated DT property.
> Depending on the IP core configuration the maximum value can vary from
> channel to channel so by overriding the channel slave max_burst capability
> we make sure a DMA consumer will get the channel-specific max burst
> length.
LGTM, but consider comment to previous patch (in that case perhaps definition
of min and max should be moved there).
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
>
> Signed-off-by: Serge Semin <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx>
> Cc: Alexey Malahov <Alexey.Malahov@xxxxxxxxxxxxxxxxxxxx>
> Cc: Thomas Bogendoerfer <tsbogend@xxxxxxxxxxxxxxxx>
> Cc: Arnd Bergmann <arnd@xxxxxxxx>
> Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> Cc: linux-mips@xxxxxxxxxxxxxxx
> Cc: devicetree@xxxxxxxxxxxxxxx
>
> ---
>
> Changelog v2:
> - Rearrange SoBs.
> - Discard dwc_get_maxburst() accessor. It's enough to have a clamping
> guard against exceeding the hardware max burst limitation.
>
> Changelog v3:
> - Override the slave channel max_burst capability instead of calculating
> the minimum value of max burst lengths and setting the DMA-device
> generic capability.
>
> Changelog v4:
> - Clamp the dst and src burst lengths in the generic dwc_config() method
> instead of doing that in the encode_maxburst() callback.
> - Define max_burst with u32 type in struct dw_dma_platform_data.
> - Perform of_property_read_u32_array() directly into the platform data
> max_burst member.
> ---
> drivers/dma/dw/core.c | 10 ++++++++++
> drivers/dma/dw/of.c | 5 +++++
> drivers/dma/dw/regs.h | 2 ++
> include/linux/platform_data/dma-dw.h | 4 ++++
> 4 files changed, 21 insertions(+)
>
> diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
> index a8cebb1dbb68..60ef779fc5e0 100644
> --- a/drivers/dma/dw/core.c
> +++ b/drivers/dma/dw/core.c
> @@ -791,6 +791,11 @@ static int dwc_config(struct dma_chan *chan, struct dma_slave_config *sconfig)
>
> memcpy(&dwc->dma_sconfig, sconfig, sizeof(*sconfig));
>
> + dwc->dma_sconfig.src_maxburst =
> + clamp(dwc->dma_sconfig.src_maxburst, 0U, dwc->max_burst);
> + dwc->dma_sconfig.dst_maxburst =
> + clamp(dwc->dma_sconfig.dst_maxburst, 0U, dwc->max_burst);
> +
> dw->encode_maxburst(dwc, &dwc->dma_sconfig.src_maxburst);
> dw->encode_maxburst(dwc, &dwc->dma_sconfig.dst_maxburst);
>
> @@ -1051,7 +1056,9 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
>
> static void dwc_caps(struct dma_chan *chan, struct dma_slave_caps *caps)
> {
> + struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
>
> + caps->max_burst = dwc->max_burst;
> }
>
> int do_dma_probe(struct dw_dma_chip *chip)
> @@ -1194,9 +1201,12 @@ int do_dma_probe(struct dw_dma_chip *chip)
> dwc->nollp =
> (dwc_params >> DWC_PARAMS_MBLK_EN & 0x1) == 0 ||
> (dwc_params >> DWC_PARAMS_HC_LLP & 0x1) == 1;
> + dwc->max_burst =
> + (0x4 << (dwc_params >> DWC_PARAMS_MSIZE & 0x7));
> } else {
> dwc->block_size = pdata->block_size;
> dwc->nollp = !pdata->multi_block[i];
> + dwc->max_burst = pdata->max_burst[i] ?: DW_DMA_MAX_BURST;
> }
> }
>
> diff --git a/drivers/dma/dw/of.c b/drivers/dma/dw/of.c
> index 9e27831dee32..1474b3817ef4 100644
> --- a/drivers/dma/dw/of.c
> +++ b/drivers/dma/dw/of.c
> @@ -98,6 +98,11 @@ struct dw_dma_platform_data *dw_dma_parse_dt(struct platform_device *pdev)
> pdata->multi_block[tmp] = 1;
> }
>
> + if (of_property_read_u32_array(np, "snps,max-burst-len", pdata->max_burst,
> + nr_channels)) {
> + memset32(pdata->max_burst, DW_DMA_MAX_BURST, nr_channels);
> + }
> +
> if (!of_property_read_u32(np, "snps,dma-protection-control", &tmp)) {
> if (tmp > CHAN_PROTCTL_MASK)
> return NULL;
> diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
> index 1ab840b06e79..76654bd13c1a 100644
> --- a/drivers/dma/dw/regs.h
> +++ b/drivers/dma/dw/regs.h
> @@ -126,6 +126,7 @@ struct dw_dma_regs {
> /* Bitfields in DWC_PARAMS */
> #define DWC_PARAMS_MBLK_EN 11 /* multi block transfer */
> #define DWC_PARAMS_HC_LLP 13 /* set LLP register to zero */
> +#define DWC_PARAMS_MSIZE 16 /* max group transaction size */
>
> /* bursts size */
> enum dw_dma_msize {
> @@ -284,6 +285,7 @@ struct dw_dma_chan {
> /* hardware configuration */
> unsigned int block_size;
> bool nollp;
> + u32 max_burst;
>
> /* custom slave configuration */
> struct dw_dma_slave dws;
> diff --git a/include/linux/platform_data/dma-dw.h b/include/linux/platform_data/dma-dw.h
> index f3eaf9ec00a1..29c484da2979 100644
> --- a/include/linux/platform_data/dma-dw.h
> +++ b/include/linux/platform_data/dma-dw.h
> @@ -12,6 +12,7 @@
>
> #define DW_DMA_MAX_NR_MASTERS 4
> #define DW_DMA_MAX_NR_CHANNELS 8
> +#define DW_DMA_MAX_BURST 256
>
> /**
> * struct dw_dma_slave - Controller-specific information about a slave
> @@ -42,6 +43,8 @@ struct dw_dma_slave {
> * @data_width: Maximum data width supported by hardware per AHB master
> * (in bytes, power of 2)
> * @multi_block: Multi block transfers supported by hardware per channel.
> + * @max_burst: Maximum value of burst transaction size supported by hardware
> + * per channel (in units of CTL.SRC_TR_WIDTH/CTL.DST_TR_WIDTH).
> * @protctl: Protection control signals setting per channel.
> */
> struct dw_dma_platform_data {
> @@ -56,6 +59,7 @@ struct dw_dma_platform_data {
> unsigned char nr_masters;
> unsigned char data_width[DW_DMA_MAX_NR_MASTERS];
> unsigned char multi_block[DW_DMA_MAX_NR_CHANNELS];
> + u32 max_burst[DW_DMA_MAX_NR_CHANNELS];
> #define CHAN_PROTCTL_PRIVILEGED BIT(0)
> #define CHAN_PROTCTL_BUFFERABLE BIT(1)
> #define CHAN_PROTCTL_CACHEABLE BIT(2)
> --
> 2.26.2
>
--
With Best Regards,
Andy Shevchenko
