clk: qcom: gdsc: Add support to update GDSC transition delay
Subsystems (3)
PATCH v2 1/2 clk: qcom: gdsc: Add support to update GDSC transition delay
GDSCs have multiple transition delays which are used for the GDSC FSM states. Older targets/designs required these values to be updated from gdsc code to certain default values for the FSM state to work as expected. But on the newer targets/designs the values updated from the GDSC driver can hamper the FSM state to not work as expected.
On SC7180 we observe black screens because the gdsc is being enabled/disabled very rapidly and the GDSC FSM state does not work as expected. This is due to the fact that the GDSC reset value is being updated from SW.
Thus add support to update the transition delay from the clock controller gdscs as required.
---
[v2]
* Add 3 transition delays and update the default values in case of non-zero value.
* Update the delays from mdss gdsc in the corresponding display clock controllers.
drivers/clk/qcom/gdsc.c | 26 +++++++++++++++++++++----- drivers/clk/qcom/gdsc.h | 8 +++++++- 2 files changed, 28 insertions(+), 6 deletions(-)
drivers/clk/qcom/gdsc.c +21 -5
@@ -1,6 +1,6 @@// SPDX-License-Identifier: GPL-2.0-only /* -* Copyright (c) 2015, 2017-2018, The Linux Foundation. All rights reserved. +* Copyright (c) 2015, 2017-2018, 2022, The Linux Foundation. All rights reserved. */ #include <linux/bitops.h>
@@ -35,9 +35,14 @@#define CFG_GDSCR_OFFSET 0x4/* Wait 2^n CXO cycles between all states. Here, n=2 (4 cycles). */ -#define EN_REST_WAIT_VAL (0x2 << 20) -#define EN_FEW_WAIT_VAL (0x8 << 16) -#define CLK_DIS_WAIT_VAL (0x2 << 12) +#define EN_REST_WAIT_VAL 0x2 +#define EN_FEW_WAIT_VAL 0x8 +#define CLK_DIS_WAIT_VAL 0x2 + +/* Transition delay shifts */ +#define EN_REST_WAIT_SHIFT 20 +#define EN_FEW_WAIT_SHIFT 16 +#define CLK_DIS_WAIT_SHIFT 12#define RETAIN_MEM BIT(14)#define RETAIN_PERIPH BIT(13)
@@ -380,7 +385,18 @@ static int gdsc_init(struct gdsc *sc)*/ mask =HW_CONTROL_MASK |SW_OVERRIDE_MASK |EN_REST_WAIT_MASK |EN_FEW_WAIT_MASK |CLK_DIS_WAIT_MASK ; - val =EN_REST_WAIT_VAL |EN_FEW_WAIT_VAL |CLK_DIS_WAIT_VAL ; + +if (!sc->en_rest_wait_val) + sc->en_rest_wait_val =EN_REST_WAIT_VAL ; +if (!sc->en_few_wait_val) + sc->en_few_wait_val =EN_FEW_WAIT_VAL ; +if (!sc->clk_dis_wait_val) + sc->clk_dis_wait_val =CLK_DIS_WAIT_VAL ; + + val = sc->en_rest_wait_val <<EN_REST_WAIT_SHIFT | + sc->en_few_wait_val <<EN_FEW_WAIT_SHIFT | + sc->clk_dis_wait_val <<CLK_DIS_WAIT_SHIFT ; + ret =regmap_update_bits (sc->regmap, sc->gdscr, mask, val);if (ret)return ret;
drivers/clk/qcom/gdsc.h +7 -1
@@ -1,6 +1,6 @@/* SPDX-License-Identifier: GPL-2.0-only */ /* -* Copyright (c) 2015, 2017-2018, The Linux Foundation. All rights reserved. +* Copyright (c) 2015, 2017-2018, 2022, The Linux Foundation. All rights reserved. */ #ifndef __QCOM_GDSC_H__
@@ -22,6 +22,9 @@ struct reset_controller_dev;* @cxcs: offsets of branch registers to toggle mem/periph bits in * @cxc_count: number of @cxcs * @pwrsts: Possible powerdomain power states +* @en_rest_wait_val: transition delay value for receiving enr ack signal +* @en_few_wait_val: transition delay value for receiving enf ack signal +* @clk_dis_wait_val: transition delay value for halting clock * @resets: ids of resets associated with this gdsc * @reset_count: number of @resets * @rcdev: reset controller
@@ -36,6 +39,9 @@ struct gdsc {unsigned int clamp_io_ctrl;unsigned int *cxcs;unsigned int cxc_count; +unsigned int en_rest_wait_val; +unsigned int en_few_wait_val; +unsigned int clk_dis_wait_val;const u8 pwrsts;/* Powerdomain allowable state bitfields */ #define PWRSTS_OFF BIT(0)
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
PATCH v2 2/2 clk: qcom: dispcc: Update the transition delay for MDSS GDSC
On SC7180 we observe black screens because the gdsc is being enabled/disabled very rapidly and the GDSC FSM state does not work as expected. This is due to the fact that the GDSC reset value is being updated from SW.
The recommended transition delay for mdss core gdsc updated for SC7180/SC7280/SM8250.
--- drivers/clk/qcom/dispcc-sc7180.c | 5 ++++- drivers/clk/qcom/dispcc-sc7280.c | 5 ++++- drivers/clk/qcom/dispcc-sm8250.c | 5 ++++- 3 files changed, 12 insertions(+), 3 deletions(-)
drivers/clk/qcom/dispcc-sc7180.c +4 -1
@@ -1,6 +1,6 @@// SPDX-License-Identifier: GPL-2.0-only /* -* Copyright (c) 2019, The Linux Foundation. All rights reserved. +* Copyright (c) 2019, 2022, The Linux Foundation. All rights reserved. */ #include <linux/clk-provider.h>
@@ -625,6 +625,9 @@ static struct clk_branch disp_cc_mdss_vsync_clk = {static struct gdsc mdss_gdsc = { .gdscr =0x3000 , + .en_rest_wait_val =0x2 , + .en_few_wait_val =0x2 , + .clk_dis_wait_val =0xF , .pd = { .name ="mdss_gdsc" , },
drivers/clk/qcom/dispcc-sc7280.c +4 -1
@@ -1,6 +1,6 @@// SPDX-License-Identifier: GPL-2.0-only /* -* Copyright (c) 2021, The Linux Foundation. All rights reserved. +* Copyright (c) 2021-2022, The Linux Foundation. All rights reserved. */ #include <linux/clk-provider.h>
@@ -787,6 +787,9 @@ static struct clk_branch disp_cc_sleep_clk = {static struct gdsc disp_cc_mdss_core_gdsc = { .gdscr =0x1004 , + .en_rest_wait_val =0x2 , + .en_few_wait_val =0x2 , + .clk_dis_wait_val =0xF , .pd = { .name ="disp_cc_mdss_core_gdsc" , },
drivers/clk/qcom/dispcc-sm8250.c +4 -1
@@ -1,6 +1,6 @@// SPDX-License-Identifier: GPL-2.0 /* -* Copyright (c) 2018-2020, The Linux Foundation. All rights reserved. +* Copyright (c) 2018-2020, 2022, The Linux Foundation. All rights reserved. */ #include <linux/clk-provider.h>
@@ -1126,6 +1126,9 @@ static struct clk_branch disp_cc_mdss_vsync_clk = {static struct gdsc mdss_gdsc = { .gdscr =0x3000 , + .en_rest_wait_val =0x2 , + .en_few_wait_val =0x2 , + .clk_dis_wait_val =0xF , .pd = { .name ="mdss_gdsc" , },
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
Re: [v2 2/2] clk: qcom: dispcc: Update the transition delay for MDSS GDSC
Quoting Taniya Das (2022-02-23 10:56:06)
Quoted 5 lines
On SC7180 we observe black screens because the gdsc is being enabled/disabled very rapidly and the GDSC FSM state does not work as expected. This is due to the fact that the GDSC reset value is being updated from SW.
Quoted 8 lines
The recommended transition delay for mdss core gdsc updated for SC7180/SC7280/SM8250.
Fixes dd3d06622138 ("clk: qcom: Add display clock controller driver for SC7180")Fixes 1a00c962f9cd ("clk: qcom: Add display clock controller driver for SC7280")Fixes 80a18f4a8567 ("clk: qcom: Add display clock controller driver for SM8150 and SM8250")Signed-off-by Taniya Das <[email protected]>---
Applied to clk-fixes
Stephen Boyd
maintainerQuoting Taniya Das (2022-02-23 10:56:05)
Quoted 11 lines
GDSCs have multiple transition delays which are used for the GDSC FSM states. Older targets/designs required these values to be updated from gdsc code to certain default values for the FSM state to work as expected. But on the newer targets/designs the values updated from the GDSC driver can hamper the FSM state to not work as expected.
On SC7180 we observe black screens because the gdsc is being enabled/disabled very rapidly and the GDSC FSM state does not work as expected. This is due to the fact that the GDSC reset value is being updated from SW.
Quoted 6 lines
Thus add support to update the transition delay from the clock controller gdscs as required.
Fixes 45dd0e55317cc ("clk: qcom: Add support for GDSCs)Signed-off-by Taniya Das <[email protected]>---
Looks like I need to apply this for clk-fixes. Please keep Bjorn on Cc next time for awareness.
Quoting Taniya Das (2022-02-23 10:56:05)
Quoted 11 lines
GDSCs have multiple transition delays which are used for the GDSC FSM states. Older targets/designs required these values to be updated from gdsc code to certain default values for the FSM state to work as expected. But on the newer targets/designs the values updated from the GDSC driver can hamper the FSM state to not work as expected.
On SC7180 we observe black screens because the gdsc is being enabled/disabled very rapidly and the GDSC FSM state does not work as expected. This is due to the fact that the GDSC reset value is being updated from SW.
Quoted 5 lines
Thus add support to update the transition delay from the clock controller gdscs as required.
Fixes 45dd0e55317cc ("clk: qcom: Add support for GDSCs)Signed-off-by Taniya Das <[email protected]>
Applied to clk-fixes with Bjorn's reviewed-by. One note below, please make this improvement.
Quoted 23 lines
drivers/clk/qcom/gdsc.h +4
-1@@ -1,6 +1,6 @@/* SPDX-License-Identifier: GPL-2.0-only */ /* -* Copyright (c) 2015, 2017-2018, The Linux Foundation. All rights reserved. +* Copyright (c) 2015, 2017-2018, 2022, The Linux Foundation. All rights reserved. */ #ifndef __QCOM_GDSC_H__@@ -22,6 +22,9 @@ struct reset_controller_dev;* @cxcs: offsets of branch registers to toggle mem/periph bits in * @cxc_count: number of @cxcs * @pwrsts: Possible powerdomain power states +* @en_rest_wait_val: transition delay value for receiving enr ack signal +* @en_few_wait_val: transition delay value for receiving enf ack signal +* @clk_dis_wait_val: transition delay value for halting clock * @resets: ids of resets associated with this gdsc * @reset_count: number of @resets * @rcdev: reset controller
drivers/clk/qcom/gdsc.h struct gdsc { · line 44
Quoted 6 lines
drivers/clk/qcom/gdsc.h +3
-0@@ -36,6 +39,9 @@ struct gdsc {unsigned int clamp_io_ctrl;unsigned int *cxcs;unsigned int cxc_count; +unsigned int en_rest_wait_val; +unsigned int en_few_wait_val; +unsigned int clk_dis_wait_val;
Bjorn pointed out the usage of unsigned int is too big. These are 4-bits wide fields in the hardware.
We should pack these into a u16 and then shift it and write it into place if it is non-zero. That means the driver author has to know all the values, but that sounds OK to me given that they're already changing something from the hardware defaults. This will save space in the vmlinux for however many gdscs there are declared. We should have a macro for this too so we can pack all the values together and then just write it out directly without having to know the shifts and stuff.
#define GDSC_WAIT_VALS(en_rest, en_few, clk_dis)
This series was applied to qcom/linux.git (for-next) by Stephen Boyd <[email protected]>:
Hello:
This series was applied to qcom/linux.git (for-next) by Stephen Boyd <[email protected]>:
Quoted 6 lines · On Thu, 24 Feb 2022 00:26:05 +0530 you wrote:
GDSCs have multiple transition delays which are used for the GDSC FSM states. Older targets/designs required these values to be updated from gdsc code to certain default values for the FSM state to work as expected. But on the newer targets/designs the values updated from the GDSC driver can hamper the FSM state to not work as expected.
Quoted 6 lines
On SC7180 we observe black screens because the gdsc is being enabled/disabled very rapidly and the GDSC FSM state does not work as expected. This is due to the fact that the GDSC reset value is being updated from SW.
[...]
Here is the summary with links:
- [v2,1/2] clk: qcom: gdsc: Add support to update GDSC transition delay
https://git.kernel.org/qcom/c/4e7c4d3652f9 - [v2,2/2] clk: qcom: dispcc: Update the transition delay for MDSS GDSC https://git.kernel.org/qcom/c/6e6fec3f961c
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html