2016-11-18 12:28:45

by Sricharan R

[permalink] [raw]
Subject: [PATCH V2 0/2] clk: qcom: gdsc: Add support for gdscs with HW control

This series adds support for gdscs(powerdomains) that can be configured
in hw controlled mode. So they are turned 'ON' based on needs dynamically,
helping to save power. Also updated the venus video ip's gdsc/clock
data to put them in hw control.

V2:
Dropped patch#3 [1] as it was concluded that the patch was effectively
masking the fact the clocks were not getting turned on when the gdsc
is put in hwctrl. With some change in sequence from venus core, masking
is not needed and so patch needs to handled in venus driver.

Fixed a comment in patch#1 to check the return value for gdsc_hwctrl

[1] https://www.mail-archive.com/[email protected]/msg1270922.html

Rajendra Nayak (1):
clk: qcom: gdsc: Add support for gdscs with HW control

Sricharan R (1):
clk: qcom: Put venus core0/1 gdscs to hw control mode

drivers/clk/qcom/gdsc.c | 19 +++++++++++++++++++
drivers/clk/qcom/gdsc.h | 1 +
drivers/clk/qcom/mmcc-msm8996.c | 2 ++
3 files changed, 22 insertions(+)

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


2016-11-18 12:29:05

by Sricharan R

[permalink] [raw]
Subject: [PATCH V2 1/2] clk: qcom: gdsc: Add support for gdscs with HW control

From: Rajendra Nayak <[email protected]>

Some GDSCs might support a HW control mode, where in the power
domain (gdsc) is brought in and out of low power state (while
unsued) without any SW assistance, saving power.
Such GDSCs can be configured in a HW control mode when powered on
until they are explicitly requested to be powered off by software.

Signed-off-by: Rajendra Nayak <[email protected]>
Signed-off-by: Sricharan R <[email protected]>
---
[V2] Fixed to take care of the return value of gdsc_hwctrl

drivers/clk/qcom/gdsc.c | 19 +++++++++++++++++++
drivers/clk/qcom/gdsc.h | 1 +
2 files changed, 20 insertions(+)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index f12d7b2..57c7c1b 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -55,6 +55,13 @@ static int gdsc_is_enabled(struct gdsc *sc, unsigned int reg)
return !!(val & PWR_ON_MASK);
}

+static int gdsc_hwctrl(struct gdsc *sc, bool en)
+{
+ u32 val = en ? HW_CONTROL_MASK : 0;
+
+ return regmap_update_bits(sc->regmap, sc->gdscr, HW_CONTROL_MASK, val);
+}
+
static int gdsc_toggle_logic(struct gdsc *sc, bool en)
{
int ret;
@@ -164,16 +171,28 @@ static int gdsc_enable(struct generic_pm_domain *domain)
*/
udelay(1);

+ /* Turn on HW trigger mode if supported */
+ if (sc->flags & HW_CTRL)
+ return gdsc_hwctrl(sc, true);
+
return 0;
}

static int gdsc_disable(struct generic_pm_domain *domain)
{
struct gdsc *sc = domain_to_gdsc(domain);
+ int ret;

if (sc->pwrsts == PWRSTS_ON)
return gdsc_assert_reset(sc);

+ /* Turn off HW trigger mode if supported */
+ if (sc->flags & HW_CTRL) {
+ ret = gdsc_hwctrl(sc, false);
+ if (ret < 0)
+ return ret;
+ }
+
if (sc->pwrsts & PWRSTS_OFF)
gdsc_clear_mem_on(sc);

diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index 3bf497c..b1f30f8 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -50,6 +50,7 @@ struct gdsc {
#define PWRSTS_RET_ON (PWRSTS_RET | PWRSTS_ON)
const u8 flags;
#define VOTABLE BIT(0)
+#define HW_CTRL BIT(1)
struct reset_controller_dev *rcdev;
unsigned int *resets;
unsigned int reset_count;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

2016-11-18 12:29:12

by Sricharan R

[permalink] [raw]
Subject: [PATCH V2 2/2] clk: qcom: Put venus core0/1 gdscs to hw control mode

The venus video ip's internal core blocks are under the
control of the firmware and their powerdomains needs to be
'ON' only when used by the firmware. So putting it into
hw controlled mode lets this to happen, otherwise the firmware
hangs checking for this.

Signed-off-by: Sricharan R <[email protected]>
---
drivers/clk/qcom/mmcc-msm8996.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/mmcc-msm8996.c b/drivers/clk/qcom/mmcc-msm8996.c
index ca97e11..41aabe3 100644
--- a/drivers/clk/qcom/mmcc-msm8996.c
+++ b/drivers/clk/qcom/mmcc-msm8996.c
@@ -2945,6 +2945,7 @@ enum {
.name = "venus_core0",
},
.pwrsts = PWRSTS_OFF_ON,
+ .flags = HW_CTRL,
};

static struct gdsc venus_core1_gdsc = {
@@ -2955,6 +2956,7 @@ enum {
.name = "venus_core1",
},
.pwrsts = PWRSTS_OFF_ON,
+ .flags = HW_CTRL,
};

static struct gdsc camss_gdsc = {
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

2016-11-18 14:49:30

by Stanimir Varbanov

[permalink] [raw]
Subject: Re: [PATCH V2 0/2] clk: qcom: gdsc: Add support for gdscs with HW control

Hi,

On 11/18/2016 02:28 PM, Sricharan R wrote:
> This series adds support for gdscs(powerdomains) that can be configured
> in hw controlled mode. So they are turned 'ON' based on needs dynamically,
> helping to save power. Also updated the venus video ip's gdsc/clock
> data to put them in hw control.
>
> V2:
> Dropped patch#3 [1] as it was concluded that the patch was effectively
> masking the fact the clocks were not getting turned on when the gdsc
> is put in hwctrl. With some change in sequence from venus core, masking
> is not needed and so patch needs to handled in venus driver.

Which sequence should be changed in venus driver?

>
> Fixed a comment in patch#1 to check the return value for gdsc_hwctrl
>
> [1] https://www.mail-archive.com/[email protected]/msg1270922.html
>
> Rajendra Nayak (1):
> clk: qcom: gdsc: Add support for gdscs with HW control
>
> Sricharan R (1):
> clk: qcom: Put venus core0/1 gdscs to hw control mode
>
> drivers/clk/qcom/gdsc.c | 19 +++++++++++++++++++
> drivers/clk/qcom/gdsc.h | 1 +
> drivers/clk/qcom/mmcc-msm8996.c | 2 ++
> 3 files changed, 22 insertions(+)
>

2016-11-18 18:11:01

by Sricharan R

[permalink] [raw]
Subject: RE: [PATCH V2 0/2] clk: qcom: gdsc: Add support for gdscs with HW control

Hi Stan,

>Hi,
>
>On 11/18/2016 02:28 PM, Sricharan R wrote:
>> This series adds support for gdscs(powerdomains) that can be configured
>> in hw controlled mode. So they are turned 'ON' based on needs dynamically,
>> helping to save power. Also updated the venus video ip's gdsc/clock
>> data to put them in hw control.
>>
>> V2:
>> Dropped patch#3 [1] as it was concluded that the patch was effectively
>> masking the fact the clocks were not getting turned on when the gdsc
>> is put in hwctrl. With some change in sequence from venus core, masking
>> is not needed and so patch needs to handled in venus driver.
>
>Which sequence should be changed in venus driver?
>
Ya wanted to discuss this with you on the venus thread, but let me put it here.

So while enabling the hw control bit for the venus subcores 0/1 gdscs and turning
on the subcore 0/1 clks, we saw that unless the
VENUS_WRAPPER_VENUS0_MMCC_VDEC_VCODEC_POWER_CONTROL
register is programmed to '0'(reset value is 1), the subcores domain/
clocks do not turn on. So this means that the,

1) venus driver should turn on all clocks except the subcore clocks.
2) Program VENUS_WRAPPER_VENUS0_MMCC_VDEC_VCODEC_POWER_CONTROL to
'0' to turn on sub domains.
3) Turn on subcore clocks (cbc) and verify their running status using clk_enable
4) Program VENUS_WRAPPER_VENUS0_MMCC_VDEC_VCODEC_POWER_CONTROL to
'1' again to turn off subdomain/clocks and let the firmware turn it on when required.

Note that in my previous patch set, i was skipping the check to verify the subcore clocks
'running status' previously, assuming that it can't be done while the gdsc is in hwctrl, but
that was not right.

Regards,
Sricharan


2016-11-24 00:48:57

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] clk: qcom: gdsc: Add support for gdscs with HW control

On 11/18, Sricharan R wrote:
> From: Rajendra Nayak <[email protected]>
>
> Some GDSCs might support a HW control mode, where in the power
> domain (gdsc) is brought in and out of low power state (while
> unsued) without any SW assistance, saving power.
> Such GDSCs can be configured in a HW control mode when powered on
> until they are explicitly requested to be powered off by software.
>
> Signed-off-by: Rajendra Nayak <[email protected]>
> Signed-off-by: Sricharan R <[email protected]>
> ---

Applied to clk-next

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