| Message ID | 20221020083322.36431-4-akhilrajeev@nvidia.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Tegra GCPDMA: Add dma-channel-mask support | expand |
On Thu, Oct 20, 2022 at 02:03:22PM +0530, Akhil R wrote: > Add support for dma-channel-mask so that only the specified channels > are used. This helps to reserve some channels for the firmware. > > This was initially achieved by limiting the channel number to 31 in > the driver and adjusting the register address to skip channel0 which > was reserved for a firmware. Now, with this change, the driver can > align more to the actual hardware which has 32 channels. > > Signed-off-by: Akhil R <akhilrajeev@nvidia.com> > Reviewed-by: Jon Hunter <jonathanh@nvidia.com> > --- > drivers/dma/tegra186-gpc-dma.c | 37 +++++++++++++++++++++++++++------- > 1 file changed, 30 insertions(+), 7 deletions(-) > > diff --git a/drivers/dma/tegra186-gpc-dma.c b/drivers/dma/tegra186-gpc-dma.c > index fa9bda4a2bc6..1d1180db6d4e 100644 > --- a/drivers/dma/tegra186-gpc-dma.c > +++ b/drivers/dma/tegra186-gpc-dma.c > @@ -161,7 +161,10 @@ > #define TEGRA_GPCDMA_BURST_COMPLETION_TIMEOUT 5000 /* 5 msec */ > > /* Channel base address offset from GPCDMA base address */ > -#define TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET 0x20000 > +#define TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET 0x10000 > + > +/* Default channel mask reserving channel0 */ > +#define TEGRA_GPCDMA_DEFAULT_CHANNEL_MASK 0xfffffffe > > struct tegra_dma; > struct tegra_dma_channel; > @@ -246,6 +249,7 @@ struct tegra_dma { > const struct tegra_dma_chip_data *chip_data; > unsigned long sid_m2d_reserved; > unsigned long sid_d2m_reserved; > + u32 chan_mask; > void __iomem *base_addr; > struct device *dev; > struct dma_device dma_dev; > @@ -1288,7 +1292,7 @@ static struct dma_chan *tegra_dma_of_xlate(struct of_phandle_args *dma_spec, > } > > static const struct tegra_dma_chip_data tegra186_dma_chip_data = { > - .nr_channels = 31, > + .nr_channels = 32, This is an ABI break. A new kernel with an old DTB will use 32 channels instead of 31. You should leave this and use the dma-channel-mask to enable all 32 channels. Rob
> On Thu, Oct 20, 2022 at 02:03:22PM +0530, Akhil R wrote: > > Add support for dma-channel-mask so that only the specified channels > > are used. This helps to reserve some channels for the firmware. > > > > This was initially achieved by limiting the channel number to 31 in > > the driver and adjusting the register address to skip channel0 which > > was reserved for a firmware. Now, with this change, the driver can > > align more to the actual hardware which has 32 channels. > > > > Signed-off-by: Akhil R <akhilrajeev@nvidia.com> > > Reviewed-by: Jon Hunter <jonathanh@nvidia.com> > > --- > > drivers/dma/tegra186-gpc-dma.c | 37 +++++++++++++++++++++++++++------- > > 1 file changed, 30 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/dma/tegra186-gpc-dma.c b/drivers/dma/tegra186-gpc- > dma.c > > index fa9bda4a2bc6..1d1180db6d4e 100644 > > --- a/drivers/dma/tegra186-gpc-dma.c > > +++ b/drivers/dma/tegra186-gpc-dma.c > > @@ -161,7 +161,10 @@ > > #define TEGRA_GPCDMA_BURST_COMPLETION_TIMEOUT 5000 /* 5 > msec */ > > > > /* Channel base address offset from GPCDMA base address */ > > -#define TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET 0x20000 > > +#define TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET 0x10000 > > + > > +/* Default channel mask reserving channel0 */ > > +#define TEGRA_GPCDMA_DEFAULT_CHANNEL_MASK 0xfffffffe > > > > struct tegra_dma; > > struct tegra_dma_channel; > > @@ -246,6 +249,7 @@ struct tegra_dma { > > const struct tegra_dma_chip_data *chip_data; > > unsigned long sid_m2d_reserved; > > unsigned long sid_d2m_reserved; > > + u32 chan_mask; > > void __iomem *base_addr; > > struct device *dev; > > struct dma_device dma_dev; > > @@ -1288,7 +1292,7 @@ static struct dma_chan *tegra_dma_of_xlate(struct > of_phandle_args *dma_spec, > > } > > > > static const struct tegra_dma_chip_data tegra186_dma_chip_data = { > > - .nr_channels = 31, > > + .nr_channels = 32, > > This is an ABI break. A new kernel with an old DTB will use 32 channels > instead of 31. You should leave this and use the dma-channel-mask to > enable all 32 channels. > Hi Rob, If using an old DTB, tdma->chan_mask will be default to 0xfffffffe since it would not have the dma-channel-mask property. The driver would still use 31 channels even if it uses an old DTB. Shouldn't it prevent the ABI break? Regards, Akhil
On 26/10/2022 05:44, Akhil R wrote: >> On Thu, Oct 20, 2022 at 02:03:22PM +0530, Akhil R wrote: >>> Add support for dma-channel-mask so that only the specified channels >>> are used. This helps to reserve some channels for the firmware. >>> >>> This was initially achieved by limiting the channel number to 31 in >>> the driver and adjusting the register address to skip channel0 which >>> was reserved for a firmware. Now, with this change, the driver can >>> align more to the actual hardware which has 32 channels. >>> >>> Signed-off-by: Akhil R <akhilrajeev@nvidia.com> >>> Reviewed-by: Jon Hunter <jonathanh@nvidia.com> >>> --- >>> drivers/dma/tegra186-gpc-dma.c | 37 +++++++++++++++++++++++++++------- >>> 1 file changed, 30 insertions(+), 7 deletions(-) >>> >>> diff --git a/drivers/dma/tegra186-gpc-dma.c b/drivers/dma/tegra186-gpc- >> dma.c >>> index fa9bda4a2bc6..1d1180db6d4e 100644 >>> --- a/drivers/dma/tegra186-gpc-dma.c >>> +++ b/drivers/dma/tegra186-gpc-dma.c >>> @@ -161,7 +161,10 @@ >>> #define TEGRA_GPCDMA_BURST_COMPLETION_TIMEOUT 5000 /* 5 >> msec */ >>> >>> /* Channel base address offset from GPCDMA base address */ >>> -#define TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET 0x20000 >>> +#define TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET 0x10000 >>> + >>> +/* Default channel mask reserving channel0 */ >>> +#define TEGRA_GPCDMA_DEFAULT_CHANNEL_MASK 0xfffffffe >>> >>> struct tegra_dma; >>> struct tegra_dma_channel; >>> @@ -246,6 +249,7 @@ struct tegra_dma { >>> const struct tegra_dma_chip_data *chip_data; >>> unsigned long sid_m2d_reserved; >>> unsigned long sid_d2m_reserved; >>> + u32 chan_mask; >>> void __iomem *base_addr; >>> struct device *dev; >>> struct dma_device dma_dev; >>> @@ -1288,7 +1292,7 @@ static struct dma_chan *tegra_dma_of_xlate(struct >> of_phandle_args *dma_spec, >>> } >>> >>> static const struct tegra_dma_chip_data tegra186_dma_chip_data = { >>> - .nr_channels = 31, >>> + .nr_channels = 32, >> >> This is an ABI break. A new kernel with an old DTB will use 32 channels >> instead of 31. You should leave this and use the dma-channel-mask to >> enable all 32 channels. >> > Hi Rob, > > If using an old DTB, tdma->chan_mask will be default to 0xfffffffe since it > would not have the dma-channel-mask property. The driver would still > use 31 channels even if it uses an old DTB. Shouldn't it prevent the > ABI break? Unfortunately no. Yes for an old DTB without the dma-channel-mask property, we set the channel mask to 0xfffffffe, but this is not correct because it only has 31 interrupts/channels and not 32. So I think we will need to use of_irq_count() here. Jon
diff --git a/drivers/dma/tegra186-gpc-dma.c b/drivers/dma/tegra186-gpc-dma.c index fa9bda4a2bc6..1d1180db6d4e 100644 --- a/drivers/dma/tegra186-gpc-dma.c +++ b/drivers/dma/tegra186-gpc-dma.c @@ -161,7 +161,10 @@ #define TEGRA_GPCDMA_BURST_COMPLETION_TIMEOUT 5000 /* 5 msec */ /* Channel base address offset from GPCDMA base address */ -#define TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET 0x20000 +#define TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET 0x10000 + +/* Default channel mask reserving channel0 */ +#define TEGRA_GPCDMA_DEFAULT_CHANNEL_MASK 0xfffffffe struct tegra_dma; struct tegra_dma_channel; @@ -246,6 +249,7 @@ struct tegra_dma { const struct tegra_dma_chip_data *chip_data; unsigned long sid_m2d_reserved; unsigned long sid_d2m_reserved; + u32 chan_mask; void __iomem *base_addr; struct device *dev; struct dma_device dma_dev; @@ -1288,7 +1292,7 @@ static struct dma_chan *tegra_dma_of_xlate(struct of_phandle_args *dma_spec, } static const struct tegra_dma_chip_data tegra186_dma_chip_data = { - .nr_channels = 31, + .nr_channels = 32, .channel_reg_size = SZ_64K, .max_dma_count = SZ_1G, .hw_support_pause = false, @@ -1296,7 +1300,7 @@ static const struct tegra_dma_chip_data tegra186_dma_chip_data = { }; static const struct tegra_dma_chip_data tegra194_dma_chip_data = { - .nr_channels = 31, + .nr_channels = 32, .channel_reg_size = SZ_64K, .max_dma_count = SZ_1G, .hw_support_pause = true, @@ -1304,7 +1308,7 @@ static const struct tegra_dma_chip_data tegra194_dma_chip_data = { }; static const struct tegra_dma_chip_data tegra234_dma_chip_data = { - .nr_channels = 31, + .nr_channels = 32, .channel_reg_size = SZ_64K, .max_dma_count = SZ_1G, .hw_support_pause = true, @@ -1380,15 +1384,28 @@ static int tegra_dma_probe(struct platform_device *pdev) } stream_id = iommu_spec->ids[0] & 0xffff; + ret = device_property_read_u32(&pdev->dev, "dma-channel-mask", + &tdma->chan_mask); + if (ret) { + dev_warn(&pdev->dev, + "Missing dma-channel-mask property, using default channel mask %#x\n", + TEGRA_GPCDMA_DEFAULT_CHANNEL_MASK); + tdma->chan_mask = TEGRA_GPCDMA_DEFAULT_CHANNEL_MASK; + } + INIT_LIST_HEAD(&tdma->dma_dev.channels); for (i = 0; i < cdata->nr_channels; i++) { struct tegra_dma_channel *tdc = &tdma->channels[i]; + /* Check for channel mask */ + if (!(tdma->chan_mask & BIT(i))) + continue; + tdc->irq = platform_get_irq(pdev, i); if (tdc->irq < 0) return tdc->irq; - tdc->chan_base_offset = TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET + + tdc->chan_base_offset = TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET + i * cdata->channel_reg_size; snprintf(tdc->name, sizeof(tdc->name), "gpcdma.%d", i); tdc->tdma = tdma; @@ -1449,8 +1466,8 @@ static int tegra_dma_probe(struct platform_device *pdev) return ret; } - dev_info(&pdev->dev, "GPC DMA driver register %d channels\n", - cdata->nr_channels); + dev_info(&pdev->dev, "GPC DMA driver register %lu channels\n", + hweight_long(tdma->chan_mask)); return 0; } @@ -1473,6 +1490,9 @@ static int __maybe_unused tegra_dma_pm_suspend(struct device *dev) for (i = 0; i < tdma->chip_data->nr_channels; i++) { struct tegra_dma_channel *tdc = &tdma->channels[i]; + if (!(tdma->chan_mask & BIT(i))) + continue; + if (tdc->dma_desc) { dev_err(tdma->dev, "channel %u busy\n", i); return -EBUSY; @@ -1492,6 +1512,9 @@ static int __maybe_unused tegra_dma_pm_resume(struct device *dev) for (i = 0; i < tdma->chip_data->nr_channels; i++) { struct tegra_dma_channel *tdc = &tdma->channels[i]; + if (!(tdma->chan_mask & BIT(i))) + continue; + tegra_dma_program_sid(tdc, tdc->stream_id); }