2020-07-09 09:39:01

by Rajendra Nayak

[permalink] [raw]
Subject: [PATCH] tty: serial: qcom-geni-serial: Drop the icc bw votes in suspend for console

When using the geni-serial as console, its important to be
able to hit the lowest possible power state in suspend,
even with no_console_suspend.
The only thing that prevents it today on platforms like the sc7180
is the interconnect BW votes, which we certainly don't need when
the system is in suspend. So in the suspend handler mark them as
ACTIVE_ONLY (0x3) and on resume switch them back to the ALWAYS tag (0x7)

Signed-off-by: Rajendra Nayak <[email protected]>
---
Note: Patch applies on qcom for-next, which has the ICC support patches.
The tag macros are currently not exported and hence the hardcoded values.
Perhaps if and when https://patchwork.kernel.org/patch/11619705/ lands I
can refresh this patch to use the macros.

drivers/soc/qcom/qcom-geni-se.c | 9 +++++++++
drivers/tty/serial/qcom_geni_serial.c | 16 +++++++++++++++-
include/linux/qcom-geni-se.h | 1 +
3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index e2a0ba2..355d503 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -771,6 +771,15 @@ int geni_icc_set_bw(struct geni_se *se)
}
EXPORT_SYMBOL(geni_icc_set_bw);

+void geni_icc_set_tag(struct geni_se *se, u32 tag)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(se->icc_paths); i++)
+ icc_set_tag(se->icc_paths[i].path, tag);
+}
+EXPORT_SYMBOL(geni_icc_set_tag);
+
/* To do: Replace this by icc_bulk_enable once it's implemented in ICC core */
int geni_icc_enable(struct geni_se *se)
{
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 0300867..7337e8b 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -1459,15 +1459,29 @@ static int __maybe_unused qcom_geni_serial_sys_suspend(struct device *dev)
struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
struct uart_port *uport = &port->uport;

+ /*
+ * This is done so we can hit the lowest possible state in suspend
+ * even with no_console_suspend
+ */
+ if (uart_console(uport)) {
+ geni_icc_set_tag(&port->se, 0x3);
+ geni_icc_set_bw(&port->se);
+ }
return uart_suspend_port(uport->private_data, uport);
}

static int __maybe_unused qcom_geni_serial_sys_resume(struct device *dev)
{
+ int ret;
struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
struct uart_port *uport = &port->uport;

- return uart_resume_port(uport->private_data, uport);
+ ret = uart_resume_port(uport->private_data, uport);
+ if (uart_console(uport)) {
+ geni_icc_set_tag(&port->se, 0x7);
+ geni_icc_set_bw(&port->se);
+ }
+ return ret;
}

static const struct dev_pm_ops qcom_geni_serial_pm_ops = {
diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
index afa511e..8f385fb 100644
--- a/include/linux/qcom-geni-se.h
+++ b/include/linux/qcom-geni-se.h
@@ -454,6 +454,7 @@ void geni_se_rx_dma_unprep(struct geni_se *se, dma_addr_t iova, size_t len);
int geni_icc_get(struct geni_se *se, const char *icc_ddr);

int geni_icc_set_bw(struct geni_se *se);
+void geni_icc_set_tag(struct geni_se *se, u32 tag);

int geni_icc_enable(struct geni_se *se);

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


2020-07-09 14:22:52

by Akash Asthana

[permalink] [raw]
Subject: Re: [PATCH] tty: serial: qcom-geni-serial: Drop the icc bw votes in suspend for console


On 7/9/2020 3:07 PM, Rajendra Nayak wrote:
> When using the geni-serial as console, its important to be
> able to hit the lowest possible power state in suspend,
> even with no_console_suspend.
> The only thing that prevents it today on platforms like the sc7180
> is the interconnect BW votes, which we certainly don't need when
> the system is in suspend. So in the suspend handler mark them as
> ACTIVE_ONLY (0x3) and on resume switch them back to the ALWAYS tag (0x7)
Reviewed-by: Akash Asthana <[email protected]>

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project

2020-07-09 15:27:28

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH] tty: serial: qcom-geni-serial: Drop the icc bw votes in suspend for console

On Thu, Jul 09, 2020 at 03:07:00PM +0530, Rajendra Nayak wrote:
> When using the geni-serial as console, its important to be
> able to hit the lowest possible power state in suspend,
> even with no_console_suspend.
> The only thing that prevents it today on platforms like the sc7180
> is the interconnect BW votes, which we certainly don't need when
> the system is in suspend. So in the suspend handler mark them as
> ACTIVE_ONLY (0x3) and on resume switch them back to the ALWAYS tag (0x7)
>
> Signed-off-by: Rajendra Nayak <[email protected]>

Tested-by: Matthias Kaehlcke <[email protected]>

2020-07-10 12:42:20

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] tty: serial: qcom-geni-serial: Drop the icc bw votes in suspend for console

On Thu, Jul 09, 2020 at 03:07:00PM +0530, Rajendra Nayak wrote:
> When using the geni-serial as console, its important to be
> able to hit the lowest possible power state in suspend,
> even with no_console_suspend.
> The only thing that prevents it today on platforms like the sc7180
> is the interconnect BW votes, which we certainly don't need when
> the system is in suspend. So in the suspend handler mark them as
> ACTIVE_ONLY (0x3) and on resume switch them back to the ALWAYS tag (0x7)
>
> Signed-off-by: Rajendra Nayak <[email protected]>
> ---
> Note: Patch applies on qcom for-next, which has the ICC support patches.
> The tag macros are currently not exported and hence the hardcoded values.
> Perhaps if and when https://patchwork.kernel.org/patch/11619705/ lands I
> can refresh this patch to use the macros.

Feel free to take this patch through that tree.

Acked-by: Greg Kroah-Hartman <[email protected]>

2020-07-14 00:54:14

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH] tty: serial: qcom-geni-serial: Drop the icc bw votes in suspend for console

Rajendra,

On Fri, Jul 10, 2020 at 5:41 AM Greg KH <[email protected]> wrote:
>
> On Thu, Jul 09, 2020 at 03:07:00PM +0530, Rajendra Nayak wrote:
> > When using the geni-serial as console, its important to be
> > able to hit the lowest possible power state in suspend,
> > even with no_console_suspend.
> > The only thing that prevents it today on platforms like the sc7180
> > is the interconnect BW votes, which we certainly don't need when
> > the system is in suspend. So in the suspend handler mark them as
> > ACTIVE_ONLY (0x3) and on resume switch them back to the ALWAYS tag (0x7)
> >
> > Signed-off-by: Rajendra Nayak <[email protected]>
> > ---
> > Note: Patch applies on qcom for-next, which has the ICC support patches.
> > The tag macros are currently not exported and hence the hardcoded values.
> > Perhaps if and when https://patchwork.kernel.org/patch/11619705/ lands I
> > can refresh this patch to use the macros.
>
> Feel free to take this patch through that tree.
>
> Acked-by: Greg Kroah-Hartman <[email protected]>

Could you post a v2? Bjorn landed some of my geni serial patches to
make console / kgdb work better, but then I realized that your patch
no longer applies cleanly. It's trivial to resolve, so hopefully a v2
should be quick and then Bjorn can land?

Thanks!

-Doug

2020-07-14 05:36:13

by Rajendra Nayak

[permalink] [raw]
Subject: Re: [PATCH] tty: serial: qcom-geni-serial: Drop the icc bw votes in suspend for console


On 7/14/2020 6:23 AM, Doug Anderson wrote:
> Rajendra,
>
> On Fri, Jul 10, 2020 at 5:41 AM Greg KH <[email protected]> wrote:
>>
>> On Thu, Jul 09, 2020 at 03:07:00PM +0530, Rajendra Nayak wrote:
>>> When using the geni-serial as console, its important to be
>>> able to hit the lowest possible power state in suspend,
>>> even with no_console_suspend.
>>> The only thing that prevents it today on platforms like the sc7180
>>> is the interconnect BW votes, which we certainly don't need when
>>> the system is in suspend. So in the suspend handler mark them as
>>> ACTIVE_ONLY (0x3) and on resume switch them back to the ALWAYS tag (0x7)
>>>
>>> Signed-off-by: Rajendra Nayak <[email protected]>
>>> ---
>>> Note: Patch applies on qcom for-next, which has the ICC support patches.
>>> The tag macros are currently not exported and hence the hardcoded values.
>>> Perhaps if and when https://patchwork.kernel.org/patch/11619705/ lands I
>>> can refresh this patch to use the macros.
>>
>> Feel free to take this patch through that tree.
>>
>> Acked-by: Greg Kroah-Hartman <[email protected]>
>
> Could you post a v2? Bjorn landed some of my geni serial patches to
> make console / kgdb work better, but then I realized that your patch
> no longer applies cleanly. It's trivial to resolve, so hopefully a v2
> should be quick and then Bjorn can land?

Thanks, just rebased and posted the v2.

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation