diff mbox series

[v2] dmaengine: k3-udma: Add system suspend/resume support

Message ID 20221103203021.83929-1-g-vlaev@ti.com
State New
Headers
Series [v2] dmaengine: k3-udma: Add system suspend/resume support |

Commit Message

Georgi Vlaev Nov. 3, 2022, 8:30 p.m. UTC 
  From: Vignesh Raghavendra <vigneshr@ti.com>

The K3 platforms configure the DMA resources with the
help of the TI's System Firmware's Device Manager(DM)
over TISCI. The group of DMA related Resource Manager[1]
TISCI messages includes: INTA, RINGACC, UDMAP, and PSI-L.
This configuration however, does not persist in the DM
after leaving from Suspend-to-RAM state. We have to restore
the DMA channel configuration over TISCI for all configured
channels when returning from suspend.

The TISCI resource management calls for each DMA type (UDMA,
PKTDMA, BCDMA) happen in device_free_chan_resources() and
device_alloc_chan_resources(). In pm_suspend() we store
the current udma_chan_config for channels that still have
attached clients and call device_free_chan_resources().
In pm_resume() restore the udma_channel_config from backup
and call device_alloc_chan_resources() for those channels.
Drivers like CPSW can do their own DMA resource management,
so use the late system suspend/resume hooks.

[1] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/index.html#resource-management-rm

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
[g-vlaev@ti.com: Add patch description and config backup]
[g-vlaev@ti.com: Supend only channels with clients]
Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
---
Changes:

v2:
* Update the commit message
* Use list_for_each_entry() to iterate over the channel list.

 drivers/dma/ti/k3-udma.c | 54 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)


base-commit: 81214a573d19ae2fa5b528286ba23cd1cb17feec

Comments

Péter Ujfalusi Nov. 5, 2022, 6:49 p.m. UTC | #1 
On 03/11/2022 22:30, Georgi Vlaev wrote:
> From: Vignesh Raghavendra <vigneshr@ti.com>
> 
> The K3 platforms configure the DMA resources with the
> help of the TI's System Firmware's Device Manager(DM)
> over TISCI. The group of DMA related Resource Manager[1]
> TISCI messages includes: INTA, RINGACC, UDMAP, and PSI-L.
> This configuration however, does not persist in the DM
> after leaving from Suspend-to-RAM state. We have to restore
> the DMA channel configuration over TISCI for all configured
> channels when returning from suspend.
> 
> The TISCI resource management calls for each DMA type (UDMA,
> PKTDMA, BCDMA) happen in device_free_chan_resources() and
> device_alloc_chan_resources(). In pm_suspend() we store
> the current udma_chan_config for channels that still have
> attached clients and call device_free_chan_resources().
> In pm_resume() restore the udma_channel_config from backup
> and call device_alloc_chan_resources() for those channels.
> Drivers like CPSW can do their own DMA resource management,
> so use the late system suspend/resume hooks.

It is wrong to push the DMA context store/restore task to a client 
driver (cpsw or icss), it has to be done by the glue layer.

With this patch the DMAengine side of the UDMA/BCDMA/PKTDMA will support 
suspend/resume while the networking will remain broken, right?

I can not test this atm since my setup relies solely on NFS rootfs via 
cpsw, I might be able to check with a USB-ethernet dongle..

Please do a followup for the glue layer support.

Acked-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>

> [1] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/index.html#resource-management-rm
> 
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> [g-vlaev@ti.com: Add patch description and config backup]
> [g-vlaev@ti.com: Supend only channels with clients]
> Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> ---
> Changes:
> 
> v2:
> * Update the commit message
> * Use list_for_each_entry() to iterate over the channel list.
> 
>   drivers/dma/ti/k3-udma.c | 54 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 54 insertions(+)
> 
> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
> index ce8b80bb34d7..29844044132c 100644
> --- a/drivers/dma/ti/k3-udma.c
> +++ b/drivers/dma/ti/k3-udma.c
> @@ -304,6 +304,8 @@ struct udma_chan {
>   
>   	/* Channel configuration parameters */
>   	struct udma_chan_config config;
> +	/* Channel configuration parameters (backup) */
> +	struct udma_chan_config backup_config;
>   
>   	/* dmapool for packet mode descriptors */
>   	bool use_dma_pool;
> @@ -5491,11 +5493,63 @@ static int udma_probe(struct platform_device *pdev)
>   	return ret;
>   }
>   
> +static int udma_pm_suspend(struct device *dev)
> +{
> +	struct udma_dev *ud = dev_get_drvdata(dev);
> +	struct dma_device *dma_dev = &ud->ddev;
> +	struct dma_chan *chan;
> +	struct udma_chan *uc;
> +
> +	list_for_each_entry(chan, &dma_dev->channels, device_node) {
> +		if (chan->client_count) {
> +			uc = to_udma_chan(chan);
> +			/* backup the channel configuration */
> +			memcpy(&uc->backup_config, &uc->config,
> +			       sizeof(struct udma_chan_config));
> +			dev_dbg(dev, "Suspending channel %s\n",
> +				dma_chan_name(chan));
> +			ud->ddev.device_free_chan_resources(chan);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int udma_pm_resume(struct device *dev)
> +{
> +	struct udma_dev *ud = dev_get_drvdata(dev);
> +	struct dma_device *dma_dev = &ud->ddev;
> +	struct dma_chan *chan;
> +	struct udma_chan *uc;
> +	int ret;
> +
> +	list_for_each_entry(chan, &dma_dev->channels, device_node) {
> +		if (chan->client_count) {
> +			uc = to_udma_chan(chan);
> +			/* restore the channel configuration */
> +			memcpy(&uc->config, &uc->backup_config,
> +			       sizeof(struct udma_chan_config));
> +			dev_dbg(dev, "Resuming channel %s\n",
> +				dma_chan_name(chan));
> +			ret = ud->ddev.device_alloc_chan_resources(chan);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops udma_pm_ops = {
> +	SET_LATE_SYSTEM_SLEEP_PM_OPS(udma_pm_suspend, udma_pm_resume)
> +};
> +
>   static struct platform_driver udma_driver = {
>   	.driver = {
>   		.name	= "ti-udma",
>   		.of_match_table = udma_of_match,
>   		.suppress_bind_attrs = true,
> +		.pm = &udma_pm_ops,
>   	},
>   	.probe		= udma_probe,
>   };
> 
> base-commit: 81214a573d19ae2fa5b528286ba23cd1cb17feec
Georgi Vlaev Nov. 7, 2022, 1:23 p.m. UTC | #2 
Hi Peter,

On 11/5/22 20:49, Péter Ujfalusi wrote:
> 
> 
> On 03/11/2022 22:30, Georgi Vlaev wrote:
>> From: Vignesh Raghavendra <vigneshr@ti.com>
>>
>> The K3 platforms configure the DMA resources with the
>> help of the TI's System Firmware's Device Manager(DM)
>> over TISCI. The group of DMA related Resource Manager[1]
>> TISCI messages includes: INTA, RINGACC, UDMAP, and PSI-L.
>> This configuration however, does not persist in the DM
>> after leaving from Suspend-to-RAM state. We have to restore
>> the DMA channel configuration over TISCI for all configured
>> channels when returning from suspend.
>>
>> The TISCI resource management calls for each DMA type (UDMA,
>> PKTDMA, BCDMA) happen in device_free_chan_resources() and
>> device_alloc_chan_resources(). In pm_suspend() we store
>> the current udma_chan_config for channels that still have
>> attached clients and call device_free_chan_resources().
>> In pm_resume() restore the udma_channel_config from backup
>> and call device_alloc_chan_resources() for those channels.
>> Drivers like CPSW can do their own DMA resource management,
>> so use the late system suspend/resume hooks.
> 
> It is wrong to push the DMA context store/restore task to a client driver (cpsw or icss), it has to be done by the glue layer.
> 
> With this patch the DMAengine side of the UDMA/BCDMA/PKTDMA will support suspend/resume while the networking will remain broken, right?

The CPSW suspend/resume patch [0] releases the DMA resources in
suspend() and this one follows up in suspend_late() to deal with
what's left. The order is reversed when we resume back from suspend. 

> 
> I can not test this atm since my setup relies solely on NFS rootfs via cpsw, I might be able to check with a USB-ethernet dongle..
> 

In this case you'll probably need CPSW suspend/resume patches [0]
and apply this one on top of that sequence.

> Please do a followup for the glue layer support.
> 

Okay, will do. This may have some effect on the cpsw sequence though.

> Acked-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>
> 

Thanks.

[0] https://lore.kernel.org/netdev/20221104132310.31577-1-rogerq@kernel.org/

>> [1] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/index.html#resource-management-rm
>>
>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>> [g-vlaev@ti.com: Add patch description and config backup]
>> [g-vlaev@ti.com: Supend only channels with clients]
>> Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
>> ---
>> Changes:
>>
>> v2:
>> * Update the commit message
>> * Use list_for_each_entry() to iterate over the channel list.
>>
>>   drivers/dma/ti/k3-udma.c | 54 ++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 54 insertions(+)
>>
>> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
>> index ce8b80bb34d7..29844044132c 100644
>> --- a/drivers/dma/ti/k3-udma.c
>> +++ b/drivers/dma/ti/k3-udma.c
>> @@ -304,6 +304,8 @@ struct udma_chan {
>>         /* Channel configuration parameters */
>>       struct udma_chan_config config;
>> +    /* Channel configuration parameters (backup) */
>> +    struct udma_chan_config backup_config;
>>         /* dmapool for packet mode descriptors */
>>       bool use_dma_pool;
>> @@ -5491,11 +5493,63 @@ static int udma_probe(struct platform_device *pdev)
>>       return ret;
>>   }
>>   +static int udma_pm_suspend(struct device *dev)
>> +{
>> +    struct udma_dev *ud = dev_get_drvdata(dev);
>> +    struct dma_device *dma_dev = &ud->ddev;
>> +    struct dma_chan *chan;
>> +    struct udma_chan *uc;
>> +
>> +    list_for_each_entry(chan, &dma_dev->channels, device_node) {
>> +        if (chan->client_count) {
>> +            uc = to_udma_chan(chan);
>> +            /* backup the channel configuration */
>> +            memcpy(&uc->backup_config, &uc->config,
>> +                   sizeof(struct udma_chan_config));
>> +            dev_dbg(dev, "Suspending channel %s\n",
>> +                dma_chan_name(chan));
>> +            ud->ddev.device_free_chan_resources(chan);
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int udma_pm_resume(struct device *dev)
>> +{
>> +    struct udma_dev *ud = dev_get_drvdata(dev);
>> +    struct dma_device *dma_dev = &ud->ddev;
>> +    struct dma_chan *chan;
>> +    struct udma_chan *uc;
>> +    int ret;
>> +
>> +    list_for_each_entry(chan, &dma_dev->channels, device_node) {
>> +        if (chan->client_count) {
>> +            uc = to_udma_chan(chan);
>> +            /* restore the channel configuration */
>> +            memcpy(&uc->config, &uc->backup_config,
>> +                   sizeof(struct udma_chan_config));
>> +            dev_dbg(dev, "Resuming channel %s\n",
>> +                dma_chan_name(chan));
>> +            ret = ud->ddev.device_alloc_chan_resources(chan);
>> +            if (ret)
>> +                return ret;
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static const struct dev_pm_ops udma_pm_ops = {
>> +    SET_LATE_SYSTEM_SLEEP_PM_OPS(udma_pm_suspend, udma_pm_resume)
>> +};
>> +
>>   static struct platform_driver udma_driver = {
>>       .driver = {
>>           .name    = "ti-udma",
>>           .of_match_table = udma_of_match,
>>           .suppress_bind_attrs = true,
>> +        .pm = &udma_pm_ops,
>>       },
>>       .probe        = udma_probe,
>>   };
>>
>> base-commit: 81214a573d19ae2fa5b528286ba23cd1cb17feec
>
diff mbox series

Patch

diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
index ce8b80bb34d7..29844044132c 100644
--- a/drivers/dma/ti/k3-udma.c
+++ b/drivers/dma/ti/k3-udma.c
@@ -304,6 +304,8 @@  struct udma_chan {
 
 	/* Channel configuration parameters */
 	struct udma_chan_config config;
+	/* Channel configuration parameters (backup) */
+	struct udma_chan_config backup_config;
 
 	/* dmapool for packet mode descriptors */
 	bool use_dma_pool;
@@ -5491,11 +5493,63 @@  static int udma_probe(struct platform_device *pdev)
 	return ret;
 }
 
+static int udma_pm_suspend(struct device *dev)
+{
+	struct udma_dev *ud = dev_get_drvdata(dev);
+	struct dma_device *dma_dev = &ud->ddev;
+	struct dma_chan *chan;
+	struct udma_chan *uc;
+
+	list_for_each_entry(chan, &dma_dev->channels, device_node) {
+		if (chan->client_count) {
+			uc = to_udma_chan(chan);
+			/* backup the channel configuration */
+			memcpy(&uc->backup_config, &uc->config,
+			       sizeof(struct udma_chan_config));
+			dev_dbg(dev, "Suspending channel %s\n",
+				dma_chan_name(chan));
+			ud->ddev.device_free_chan_resources(chan);
+		}
+	}
+
+	return 0;
+}
+
+static int udma_pm_resume(struct device *dev)
+{
+	struct udma_dev *ud = dev_get_drvdata(dev);
+	struct dma_device *dma_dev = &ud->ddev;
+	struct dma_chan *chan;
+	struct udma_chan *uc;
+	int ret;
+
+	list_for_each_entry(chan, &dma_dev->channels, device_node) {
+		if (chan->client_count) {
+			uc = to_udma_chan(chan);
+			/* restore the channel configuration */
+			memcpy(&uc->config, &uc->backup_config,
+			       sizeof(struct udma_chan_config));
+			dev_dbg(dev, "Resuming channel %s\n",
+				dma_chan_name(chan));
+			ret = ud->ddev.device_alloc_chan_resources(chan);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops udma_pm_ops = {
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(udma_pm_suspend, udma_pm_resume)
+};
+
 static struct platform_driver udma_driver = {
 	.driver = {
 		.name	= "ti-udma",
 		.of_match_table = udma_of_match,
 		.suppress_bind_attrs = true,
+		.pm = &udma_pm_ops,
 	},
 	.probe		= udma_probe,
 };