diff mbox series

[RESEND,v2,3/3] dmaengine: tegra: Add support for dma-channel-mask

Message ID 20221020083322.36431-4-akhilrajeev@nvidia.com
State Superseded
Headers show
Series Tegra GCPDMA: Add dma-channel-mask support | expand

Commit Message

Akhil R Oct. 20, 2022, 8:33 a.m. UTC
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(-)

Comments

Rob Herring Oct. 21, 2022, 2:16 a.m. UTC | #1
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
Akhil R Oct. 26, 2022, 4:44 a.m. UTC | #2
> 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
Jon Hunter Oct. 26, 2022, 9:30 a.m. UTC | #3
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 mbox series

Patch

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);
 	}