Messages in this thread | | | Subject | Re: [RESEND PATCH 3/3] dmaengine: tegra210-adma: Override ADMA FIFO size | From | Jon Hunter <> | Date | Fri, 17 Sep 2021 10:45:08 +0100 |
| |
On 15/09/2021 17:07, Sameer Pujar wrote: > ADMAIF FIFO uses a ring buffer and it is divided amongst the available > channels. The default FIFO size (in multiples of 16 words) of ADMAIF TX/RX > channels is as below: > * On Tegra210, > channel 1 to 2 : size = 3 > channel 3 to 10: size = 2 > * On Tegra186 and later, > channel 1 to 4 : size = 3 > channel 5 to 20: size = 2 > > As per recommendation from HW, FIFO size of ADMA channel should be same as > the corresponding ADMAIF channel it maps to. FIFO corruption is observed if > the sizes do not match. We are using the default FIFO sizes for ADMAIF and > there is no plan to support any custom values. > > Thus at runtime, override the ADMA channel FIFO size value depending on the > corresponding ADMAIF channel. > > Fixes: 9ab59bf5dd63 ("dmaengine: tegra210-adma: Fix channel FIFO configuration") > Signed-off-by: Sameer Pujar <spujar@nvidia.com> > --- > drivers/dma/tegra210-adma.c | 48 ++++++++++++++++++++++++++++++++------------- > 1 file changed, 34 insertions(+), 14 deletions(-) > > diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c > index 03f9776..911533c 100644 > --- a/drivers/dma/tegra210-adma.c > +++ b/drivers/dma/tegra210-adma.c > @@ -43,10 +43,8 @@ > #define TEGRA186_ADMA_CH_CONFIG_OUTSTANDING_REQS(reqs) (reqs << 4) > > #define ADMA_CH_FIFO_CTRL 0x2c > -#define TEGRA210_ADMA_CH_FIFO_CTRL_TXSIZE(val) (((val) & 0xf) << 8) > -#define TEGRA210_ADMA_CH_FIFO_CTRL_RXSIZE(val) ((val) & 0xf) > -#define TEGRA186_ADMA_CH_FIFO_CTRL_TXSIZE(val) (((val) & 0x1f) << 8) > -#define TEGRA186_ADMA_CH_FIFO_CTRL_RXSIZE(val) ((val) & 0x1f) > +#define ADMA_CH_TX_FIFO_SIZE_SHIFT 8 > +#define ADMA_CH_RX_FIFO_SIZE_SHIFT 0 > > #define ADMA_CH_LOWER_SRC_ADDR 0x34 > #define ADMA_CH_LOWER_TRG_ADDR 0x3c > @@ -61,12 +59,6 @@ > > #define TEGRA_ADMA_BURST_COMPLETE_TIME 20 > > -#define TEGRA210_FIFO_CTRL_DEFAULT (TEGRA210_ADMA_CH_FIFO_CTRL_TXSIZE(3) | \ > - TEGRA210_ADMA_CH_FIFO_CTRL_RXSIZE(3)) > - > -#define TEGRA186_FIFO_CTRL_DEFAULT (TEGRA186_ADMA_CH_FIFO_CTRL_TXSIZE(3) | \ > - TEGRA186_ADMA_CH_FIFO_CTRL_RXSIZE(3)) > - > #define ADMA_CH_REG_FIELD_VAL(val, mask, shift) (((val) & mask) << shift) > > struct tegra_adma; > @@ -84,6 +76,8 @@ struct tegra_adma; > * @ch_req_max: Maximum number of Tx or Rx channels available. > * @ch_reg_size: Size of DMA channel register space. > * @nr_channels: Number of DMA channels available. > + * @ch_fifo_size_mask: Mask for FIFO size field. > + * @sreq_index_offset: Slave channel index offset. > * @has_outstanding_reqs: If DMA channel can have outstanding requests. > */ > struct tegra_adma_chip_data { > @@ -98,6 +92,8 @@ struct tegra_adma_chip_data { > unsigned int ch_req_max; > unsigned int ch_reg_size; > unsigned int nr_channels; > + unsigned int ch_fifo_size_mask; > + unsigned int sreq_index_offset; > bool has_outstanding_reqs; > }; > > @@ -561,13 +557,14 @@ static int tegra_adma_set_xfer_params(struct tegra_adma_chan *tdc, > { > struct tegra_adma_chan_regs *ch_regs = &desc->ch_regs; > const struct tegra_adma_chip_data *cdata = tdc->tdma->cdata; > - unsigned int burst_size, adma_dir; > + unsigned int burst_size, adma_dir, fifo_size_shift; > > if (desc->num_periods > ADMA_CH_CONFIG_MAX_BUFS) > return -EINVAL; > > switch (direction) { > case DMA_MEM_TO_DEV: > + fifo_size_shift = ADMA_CH_TX_FIFO_SIZE_SHIFT; > adma_dir = ADMA_CH_CTRL_DIR_MEM2AHUB; > burst_size = tdc->sconfig.dst_maxburst; > ch_regs->config = ADMA_CH_CONFIG_SRC_BUF(desc->num_periods - 1); > @@ -578,6 +575,7 @@ static int tegra_adma_set_xfer_params(struct tegra_adma_chan *tdc, > break; > > case DMA_DEV_TO_MEM: > + fifo_size_shift = ADMA_CH_RX_FIFO_SIZE_SHIFT; > adma_dir = ADMA_CH_CTRL_DIR_AHUB2MEM; > burst_size = tdc->sconfig.src_maxburst; > ch_regs->config = ADMA_CH_CONFIG_TRG_BUF(desc->num_periods - 1); > @@ -599,7 +597,27 @@ static int tegra_adma_set_xfer_params(struct tegra_adma_chan *tdc, > ch_regs->config |= ADMA_CH_CONFIG_WEIGHT_FOR_WRR(1); > if (cdata->has_outstanding_reqs) > ch_regs->config |= TEGRA186_ADMA_CH_CONFIG_OUTSTANDING_REQS(8); > - ch_regs->fifo_ctrl = cdata->ch_fifo_ctrl; > + > + /* > + * 'sreq_index' represents the current ADMAIF channel number and as per > + * HW recommendation its FIFO size should match with the corresponding > + * ADMA channel. > + * > + * ADMA FIFO size is set as per below (based on default ADMAIF channel > + * FIFO sizes): > + * fifo_size = 0x2 (sreq_index > sreq_index_offset) > + * fifo_size = 0x3 (sreq_index <= sreq_index_offset) > + * > + */ > + if (tdc->sreq_index > cdata->sreq_index_offset) > + ch_regs->fifo_ctrl = > + ADMA_CH_REG_FIELD_VAL(2, cdata->ch_fifo_size_mask, > + fifo_size_shift); > + else > + ch_regs->fifo_ctrl = > + ADMA_CH_REG_FIELD_VAL(3, cdata->ch_fifo_size_mask, > + fifo_size_shift); > + > ch_regs->tc = desc->period_len & ADMA_CH_TC_COUNT_MASK; > > return tegra_adma_request_alloc(tdc, direction); > @@ -783,11 +801,12 @@ static const struct tegra_adma_chip_data tegra210_chip_data = { > .ch_req_tx_shift = 28, > .ch_req_rx_shift = 24, > .ch_base_offset = 0, > - .ch_fifo_ctrl = TEGRA210_FIFO_CTRL_DEFAULT, > .ch_req_mask = 0xf, > .ch_req_max = 10, > .ch_reg_size = 0x80, > .nr_channels = 22, > + .ch_fifo_size_mask = 0xf, > + .sreq_index_offset = 2, > .has_outstanding_reqs = false, > }; > > @@ -798,11 +817,12 @@ static const struct tegra_adma_chip_data tegra186_chip_data = { > .ch_req_tx_shift = 27, > .ch_req_rx_shift = 22, > .ch_base_offset = 0x10000, > - .ch_fifo_ctrl = TEGRA186_FIFO_CTRL_DEFAULT, > .ch_req_mask = 0x1f, > .ch_req_max = 20, > .ch_reg_size = 0x100, > .nr_channels = 32, > + .ch_fifo_size_mask = 0x1f, > + .sreq_index_offset = 4, > .has_outstanding_reqs = true, > };
Looks good to me. The only minor comment I have is that 'sreq_index_offset' is not very descriptive. Maybe fifo_size_sreq_index? It is not great either, so no big deal either way.
Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
Cheers Jon
-- nvpublic
| |