[v4,3/5] firmware: ti_sci: Allocate memory for the LPM modes

Message ID 20221116181307.198209-4-g-vlaev@ti.com
State New
Headers
Series firmware: ti_sci: Introduce system suspend support |

Commit Message

Georgi Vlaev Nov. 16, 2022, 6:13 p.m. UTC
  From: Dave Gerlach <d-gerlach@ti.com>

A region of memory in DDR must be used during Deep Sleep for saving
of some system context when using the ti_sci firmware. From DM's point
of view, this can be any contiguous region in the DDR, so can allocate
512KB of DMA reserved memory in probe(), instead of another carveout.

Also send a TISCI_MSG_PREPARE_SUSPEND message to the firmware during
probe to determine if system suspend is supported and if
ti_sci_init_suspend should be called based on the response received.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
Tested-by: Roger Quadros <rogerq@kernel.org>
---
 drivers/firmware/ti_sci.c | 55 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)
  

Comments

Nishanth Menon Nov. 21, 2022, 6:44 p.m. UTC | #1
On 20:13-20221116, Georgi Vlaev wrote:
[...]

> +static int ti_sci_init_suspend(struct platform_device *pdev,
> +			       struct ti_sci_info *info)
> +{
> +	struct device *dev = &pdev->dev;
> +	int ret;
> +
> +	dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> +	info->ctx_mem_buf = dma_alloc_coherent(info->dev, LPM_CTX_MEM_SIZE,
> +					       &info->ctx_mem_addr,
> +					       GFP_KERNEL);
> +	if (!info->ctx_mem_buf) {
> +		dev_err(info->dev, "Failed to allocate LPM context memory\n");
> +		return -ENOMEM;
> +	}
> +
> +	/*
> +	 * Attempt to call prepare_sleep, this will be NAK'd if suspend is not
> +	 * supported by firmware in use, in which case we will not attempt to
> +	 * init suspend.
> +	 */
> +	ret = ti_sci_cmd_prepare_sleep(&info->handle, 0,
> +				       (u32)(info->ctx_mem_addr & 0xffffffff),
> +				       (u32)((u64)info->ctx_mem_addr >> 32), 0);
> +
> +	if (ret)
> +		goto err;
> +
> +	return 0;
> +err:
> +	dma_free_coherent(info->dev, LPM_CTX_MEM_SIZE,
> +			  info->ctx_mem_buf,
> +			  info->ctx_mem_addr);
> +	return ret;
> +}
> +
>  /* Description for K2G */
>  static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
>  	.default_host_id = 2,
> @@ -3639,6 +3682,14 @@ static int ti_sci_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	ret = ti_sci_init_suspend(pdev, info);
> +	if (ret)
> +		dev_warn(dev,
> +			 "ti_sci_init_suspend failed, mem suspend will be non-functional.\n");
> +
> +	/* Suspend is an optional feature, reset return value and continue. */
> +	ret = 0;

We end up getting this warning on all platforms with TISCI - even if
LPM sequence is capable or not - what does the message mean? firmware is
not capable of supporting sleep or it is a firmware capable of
supporting, but failed to allocate LPM context memory?

If it is optional (since it is probing to see if it has functionality),
then do we need a dev_warn - maybe a softer of form?

[...]
  
Nishanth Menon Nov. 21, 2022, 6:56 p.m. UTC | #2
On 20:13-20221116, Georgi Vlaev wrote:
> +	/*
> +	 * Attempt to call prepare_sleep, this will be NAK'd if suspend is not
> +	 * supported by firmware in use, in which case we will not attempt to
> +	 * init suspend.
> +	 */
> +	ret = ti_sci_cmd_prepare_sleep(&info->handle, 0,
> +				       (u32)(info->ctx_mem_addr & 0xffffffff),
> +				       (u32)((u64)info->ctx_mem_addr >> 32), 0);
> +

https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html#tisci-msg-prepare-sleep
"Prepare the SOC for entering into a low power mode."

But we are in the init process here. From the documentation, firmware
does'nt seem to guarantee it would do something unexpected (like setup
io daisy chain or something like that which normal LP entry state
would have to do) - How is it safe to use it as a discovery of
capability API?
  
Georgi Vlaev Nov. 21, 2022, 9:45 p.m. UTC | #3
Hi Nishanth,

On 11/21/22 20:44, Nishanth Menon wrote:
> On 20:13-20221116, Georgi Vlaev wrote:
> [...]
> 
>> +static int ti_sci_init_suspend(struct platform_device *pdev,
>> +			       struct ti_sci_info *info)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	int ret;
>> +
>> +	dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
>> +	info->ctx_mem_buf = dma_alloc_coherent(info->dev, LPM_CTX_MEM_SIZE,
>> +					       &info->ctx_mem_addr,
>> +					       GFP_KERNEL);
>> +	if (!info->ctx_mem_buf) {
>> +		dev_err(info->dev, "Failed to allocate LPM context memory\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	/*
>> +	 * Attempt to call prepare_sleep, this will be NAK'd if suspend is not
>> +	 * supported by firmware in use, in which case we will not attempt to
>> +	 * init suspend.
>> +	 */
>> +	ret = ti_sci_cmd_prepare_sleep(&info->handle, 0,
>> +				       (u32)(info->ctx_mem_addr & 0xffffffff),
>> +				       (u32)((u64)info->ctx_mem_addr >> 32), 0);
>> +
>> +	if (ret)
>> +		goto err;
>> +
>> +	return 0;
>> +err:
>> +	dma_free_coherent(info->dev, LPM_CTX_MEM_SIZE,
>> +			  info->ctx_mem_buf,
>> +			  info->ctx_mem_addr);
>> +	return ret;
>> +}
>> +
>>  /* Description for K2G */
>>  static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
>>  	.default_host_id = 2,
>> @@ -3639,6 +3682,14 @@ static int ti_sci_probe(struct platform_device *pdev)
>>  		}
>>  	}
>>  
>> +	ret = ti_sci_init_suspend(pdev, info);
>> +	if (ret)
>> +		dev_warn(dev,
>> +			 "ti_sci_init_suspend failed, mem suspend will be non-functional.\n");
>> +
>> +	/* Suspend is an optional feature, reset return value and continue. */
>> +	ret = 0;
> 
> We end up getting this warning on all platforms with TISCI - even if
> LPM sequence is capable or not - what does the message mean? firmware is
> not capable of supporting sleep or it is a firmware capable of
> supporting, but failed to allocate LPM context memory?
> 
> If it is optional (since it is probing to see if it has functionality),
> then do we need a dev_warn - maybe a softer of form?
> 

Yeah, I agree, the message looks confusing. In both cases we can't enter suspend-to-ram,
but we consider that an optional feature, so a softer message will be more appropriate.


> [...]
>
  
Georgi Vlaev Nov. 21, 2022, 10:03 p.m. UTC | #4
Hi Nishanth,

On 11/21/22 20:56, Nishanth Menon wrote:
> On 20:13-20221116, Georgi Vlaev wrote:
>> +	/*
>> +	 * Attempt to call prepare_sleep, this will be NAK'd if suspend is not
>> +	 * supported by firmware in use, in which case we will not attempt to
>> +	 * init suspend.
>> +	 */
>> +	ret = ti_sci_cmd_prepare_sleep(&info->handle, 0,
>> +				       (u32)(info->ctx_mem_addr & 0xffffffff),
>> +				       (u32)((u64)info->ctx_mem_addr >> 32), 0);
>> +
> 
> https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html#tisci-msg-prepare-sleep
> "Prepare the SOC for entering into a low power mode."
> 
> But we are in the init process here. From the documentation, firmware
> does'nt seem to guarantee it would do something unexpected (like setup
> io daisy chain or something like that which normal LP entry state
> would have to do) - How is it safe to use it as a discovery of
> capability API?
> 
> 

Well, you're correct, there's no guarantee. It is safe to call it now on AM62x
in both places we actually use it. However, this may change in the future
and it's not a good idea to misuse that API. We'll switch the detection part
to a more appropriate message, that's better suited for this purpose on all
K3 platforms.
  

Patch

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index bace9e9cd478..acd4d3c040a2 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -10,6 +10,7 @@ 
 
 #include <linux/bitmap.h>
 #include <linux/debugfs.h>
+#include <linux/dma-mapping.h>
 #include <linux/export.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
@@ -25,6 +26,9 @@ 
 
 #include "ti_sci.h"
 
+/* Low power mode memory context size */
+#define LPM_CTX_MEM_SIZE 0x80000
+
 /* List of all TI SCI devices active in system */
 static LIST_HEAD(ti_sci_list);
 /* Protection for the entire list */
@@ -96,6 +100,8 @@  struct ti_sci_desc {
  * @minfo:	Message info
  * @node:	list head
  * @host_id:	Host ID
+ * @ctx_mem_addr: Low power context memory phys address
+ * @ctx_mem_buf: Low power context memory buffer
  * @users:	Number of users of this instance
  * @is_suspending: Flag set to indicate in suspend path.
  */
@@ -114,6 +120,8 @@  struct ti_sci_info {
 	struct ti_sci_xfers_info minfo;
 	struct list_head node;
 	u8 host_id;
+	dma_addr_t ctx_mem_addr;
+	void *ctx_mem_buf;
 	/* protected by ti_sci_list_mutex */
 	int users;
 	bool is_suspending;
@@ -3487,6 +3495,41 @@  static int ti_sci_resume(struct device *dev)
 
 static DEFINE_SIMPLE_DEV_PM_OPS(ti_sci_pm_ops, ti_sci_suspend, ti_sci_resume);
 
+static int ti_sci_init_suspend(struct platform_device *pdev,
+			       struct ti_sci_info *info)
+{
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+	info->ctx_mem_buf = dma_alloc_coherent(info->dev, LPM_CTX_MEM_SIZE,
+					       &info->ctx_mem_addr,
+					       GFP_KERNEL);
+	if (!info->ctx_mem_buf) {
+		dev_err(info->dev, "Failed to allocate LPM context memory\n");
+		return -ENOMEM;
+	}
+
+	/*
+	 * Attempt to call prepare_sleep, this will be NAK'd if suspend is not
+	 * supported by firmware in use, in which case we will not attempt to
+	 * init suspend.
+	 */
+	ret = ti_sci_cmd_prepare_sleep(&info->handle, 0,
+				       (u32)(info->ctx_mem_addr & 0xffffffff),
+				       (u32)((u64)info->ctx_mem_addr >> 32), 0);
+
+	if (ret)
+		goto err;
+
+	return 0;
+err:
+	dma_free_coherent(info->dev, LPM_CTX_MEM_SIZE,
+			  info->ctx_mem_buf,
+			  info->ctx_mem_addr);
+	return ret;
+}
+
 /* Description for K2G */
 static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
 	.default_host_id = 2,
@@ -3639,6 +3682,14 @@  static int ti_sci_probe(struct platform_device *pdev)
 		}
 	}
 
+	ret = ti_sci_init_suspend(pdev, info);
+	if (ret)
+		dev_warn(dev,
+			 "ti_sci_init_suspend failed, mem suspend will be non-functional.\n");
+
+	/* Suspend is an optional feature, reset return value and continue. */
+	ret = 0;
+
 	dev_info(dev, "ABI: %d.%d (firmware rev 0x%04x '%s')\n",
 		 info->handle.version.abi_major, info->handle.version.abi_minor,
 		 info->handle.version.firmware_revision,
@@ -3686,6 +3737,10 @@  static int ti_sci_remove(struct platform_device *pdev)
 		mbox_free_channel(info->chan_rx);
 	}
 
+	if (info->ctx_mem_buf)
+		dma_free_coherent(info->dev, LPM_CTX_MEM_SIZE,
+				  info->ctx_mem_buf,
+				  info->ctx_mem_addr);
 	return ret;
 }