* [PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface
@ 2022-07-30 9:17 ` Akhil P Oommen
0 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-07-30 9:17 UTC (permalink / raw)
To: freedreno, dri-devel, linux-arm-msm, Rob Clark, Bjorn Andersson,
Stephen Boyd
Cc: devicetree, Akhil P Oommen, Michael Turquette, Konrad Dybcio,
Douglas Anderson, Rob Herring, linux-kernel, Stephen Boyd,
Andy Gross, Krzysztof Kozlowski, linux-clk
Some clients like adreno gpu driver would like to ensure that its gdsc
is collapsed at hardware during a gpu reset sequence. This is because it
has a votable gdsc which could be ON due to a vote from another subsystem
like tz, hyp etc or due to an internal hardware signal. To allow
this, gpucc driver can expose an interface to the client driver using
reset framework. Using this the client driver can trigger a polling within
the gdsc driver.
This series is rebased on top of linus's master branch.
Related discussion: https://patchwork.freedesktop.org/patch/493144/
Akhil P Oommen (5):
dt-bindings: clk: qcom: Support gpu cx gdsc reset
clk: qcom: Allow custom reset ops
clk: qcom: gpucc-sc7280: Add cx collapse reset support
clk: qcom: gdsc: Add a reset op to poll gdsc collapse
arm64: dts: qcom: sc7280: Add Reset support for gpu
arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++
drivers/clk/qcom/gdsc.c | 23 +++++++++++++++++++----
drivers/clk/qcom/gdsc.h | 7 +++++++
drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
drivers/clk/qcom/reset.c | 6 ++++++
drivers/clk/qcom/reset.h | 2 ++
include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++
7 files changed, 46 insertions(+), 4 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 1/5] dt-bindings: clk: qcom: Support gpu cx gdsc reset
2022-07-30 9:17 ` Akhil P Oommen
@ 2022-07-30 9:17 ` Akhil P Oommen
-1 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-07-30 9:17 UTC (permalink / raw)
To: freedreno, dri-devel, linux-arm-msm, Rob Clark, Bjorn Andersson,
Stephen Boyd
Cc: Douglas Anderson, Akhil P Oommen, Andy Gross, Konrad Dybcio,
Krzysztof Kozlowski, Michael Turquette, Rob Herring, Stephen Boyd,
devicetree, linux-clk, linux-kernel
Add necessary definitions in gpucc bindings to ensure gpu cx gdsc collapse
through 'reset' framework for SC7280.
Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/dt-bindings/clock/qcom,gpucc-sc7280.h b/include/dt-bindings/clock/qcom,gpucc-sc7280.h
index 669b23b..843a31b 100644
--- a/include/dt-bindings/clock/qcom,gpucc-sc7280.h
+++ b/include/dt-bindings/clock/qcom,gpucc-sc7280.h
@@ -32,4 +32,7 @@
#define GPU_CC_CX_GDSC 0
#define GPU_CC_GX_GDSC 1
+/* GPU_CC reset IDs */
+#define GPU_CX_COLLAPSE 0
+
#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 1/5] dt-bindings: clk: qcom: Support gpu cx gdsc reset
@ 2022-07-30 9:17 ` Akhil P Oommen
0 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-07-30 9:17 UTC (permalink / raw)
To: freedreno, dri-devel, linux-arm-msm, Rob Clark, Bjorn Andersson,
Stephen Boyd
Cc: devicetree, Akhil P Oommen, Michael Turquette, Konrad Dybcio,
Douglas Anderson, Rob Herring, linux-kernel, Stephen Boyd,
Andy Gross, Krzysztof Kozlowski, linux-clk
Add necessary definitions in gpucc bindings to ensure gpu cx gdsc collapse
through 'reset' framework for SC7280.
Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/dt-bindings/clock/qcom,gpucc-sc7280.h b/include/dt-bindings/clock/qcom,gpucc-sc7280.h
index 669b23b..843a31b 100644
--- a/include/dt-bindings/clock/qcom,gpucc-sc7280.h
+++ b/include/dt-bindings/clock/qcom,gpucc-sc7280.h
@@ -32,4 +32,7 @@
#define GPU_CC_CX_GDSC 0
#define GPU_CC_GX_GDSC 1
+/* GPU_CC reset IDs */
+#define GPU_CX_COLLAPSE 0
+
#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH 1/5] dt-bindings: clk: qcom: Support gpu cx gdsc reset
2022-07-30 9:17 ` Akhil P Oommen
@ 2022-08-03 6:37 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 36+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-03 6:37 UTC (permalink / raw)
To: Akhil P Oommen, freedreno, dri-devel, linux-arm-msm, Rob Clark,
Bjorn Andersson, Stephen Boyd
Cc: devicetree, Stephen Boyd, linux-kernel, Michael Turquette,
Konrad Dybcio, Andy Gross, Douglas Anderson, Rob Herring,
Krzysztof Kozlowski, linux-clk
On 30/07/2022 11:17, Akhil P Oommen wrote:
> Add necessary definitions in gpucc bindings to ensure gpu cx gdsc collapse
> through 'reset' framework for SC7280.
>
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---
Assuming discussion in cover letter sorts out:
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/5] dt-bindings: clk: qcom: Support gpu cx gdsc reset
@ 2022-08-03 6:37 ` Krzysztof Kozlowski
0 siblings, 0 replies; 36+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-03 6:37 UTC (permalink / raw)
To: Akhil P Oommen, freedreno, dri-devel, linux-arm-msm, Rob Clark,
Bjorn Andersson, Stephen Boyd
Cc: Douglas Anderson, Andy Gross, Konrad Dybcio, Krzysztof Kozlowski,
Michael Turquette, Rob Herring, Stephen Boyd, devicetree,
linux-clk, linux-kernel
On 30/07/2022 11:17, Akhil P Oommen wrote:
> Add necessary definitions in gpucc bindings to ensure gpu cx gdsc collapse
> through 'reset' framework for SC7280.
>
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---
Assuming discussion in cover letter sorts out:
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 2/5] clk: qcom: Allow custom reset ops
2022-07-30 9:17 ` Akhil P Oommen
@ 2022-07-30 9:17 ` Akhil P Oommen
-1 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-07-30 9:17 UTC (permalink / raw)
To: freedreno, dri-devel, linux-arm-msm, Rob Clark, Bjorn Andersson,
Stephen Boyd
Cc: Douglas Anderson, Akhil P Oommen, Andy Gross, Konrad Dybcio,
Michael Turquette, Philipp Zabel, Stephen Boyd, linux-clk,
linux-kernel
Add support to allow soc specific clk drivers to specify a custom reset
operation. A consumer-driver of the reset framework can call
"reset_control_reset()" api to trigger this.
Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
drivers/clk/qcom/reset.c | 6 ++++++
drivers/clk/qcom/reset.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/clk/qcom/reset.c b/drivers/clk/qcom/reset.c
index 819d194..4782bf1 100644
--- a/drivers/clk/qcom/reset.c
+++ b/drivers/clk/qcom/reset.c
@@ -13,6 +13,12 @@
static int qcom_reset(struct reset_controller_dev *rcdev, unsigned long id)
{
+ struct qcom_reset_controller *rst = to_qcom_reset_controller(rcdev);
+ const struct qcom_reset_map *map = &rst->reset_map[id];
+
+ if (map->op)
+ return map->op(map);
+
rcdev->ops->assert(rcdev, id);
udelay(1);
rcdev->ops->deassert(rcdev, id);
diff --git a/drivers/clk/qcom/reset.h b/drivers/clk/qcom/reset.h
index 2a08b5e..295deeb 100644
--- a/drivers/clk/qcom/reset.h
+++ b/drivers/clk/qcom/reset.h
@@ -11,6 +11,8 @@
struct qcom_reset_map {
unsigned int reg;
u8 bit;
+ int (*op)(const struct qcom_reset_map *map);
+ void *priv;
};
struct regmap;
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH 2/5] clk: qcom: Allow custom reset ops
@ 2022-07-30 9:17 ` Akhil P Oommen
0 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-07-30 9:17 UTC (permalink / raw)
To: freedreno, dri-devel, linux-arm-msm, Rob Clark, Bjorn Andersson,
Stephen Boyd
Cc: Akhil P Oommen, Michael Turquette, Konrad Dybcio,
Douglas Anderson, linux-kernel, Stephen Boyd, Andy Gross,
linux-clk
Add support to allow soc specific clk drivers to specify a custom reset
operation. A consumer-driver of the reset framework can call
"reset_control_reset()" api to trigger this.
Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
drivers/clk/qcom/reset.c | 6 ++++++
drivers/clk/qcom/reset.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/clk/qcom/reset.c b/drivers/clk/qcom/reset.c
index 819d194..4782bf1 100644
--- a/drivers/clk/qcom/reset.c
+++ b/drivers/clk/qcom/reset.c
@@ -13,6 +13,12 @@
static int qcom_reset(struct reset_controller_dev *rcdev, unsigned long id)
{
+ struct qcom_reset_controller *rst = to_qcom_reset_controller(rcdev);
+ const struct qcom_reset_map *map = &rst->reset_map[id];
+
+ if (map->op)
+ return map->op(map);
+
rcdev->ops->assert(rcdev, id);
udelay(1);
rcdev->ops->deassert(rcdev, id);
diff --git a/drivers/clk/qcom/reset.h b/drivers/clk/qcom/reset.h
index 2a08b5e..295deeb 100644
--- a/drivers/clk/qcom/reset.h
+++ b/drivers/clk/qcom/reset.h
@@ -11,6 +11,8 @@
struct qcom_reset_map {
unsigned int reg;
u8 bit;
+ int (*op)(const struct qcom_reset_map *map);
+ void *priv;
};
struct regmap;
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 2/5] clk: qcom: Allow custom reset ops
2022-07-30 9:17 ` Akhil P Oommen
(?)
@ 2022-07-30 13:10 ` kernel test robot
2022-08-01 15:20 ` Akhil P Oommen
-1 siblings, 1 reply; 36+ messages in thread
From: kernel test robot @ 2022-07-30 13:10 UTC (permalink / raw)
To: Akhil P Oommen, freedreno, dri-devel, linux-arm-msm, Rob Clark,
Bjorn Andersson, Stephen Boyd
Cc: kbuild-all, Akhil P Oommen, Michael Turquette, Konrad Dybcio,
Douglas Anderson, linux-kernel, Andy Gross, linux-clk
Hi Akhil,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on clk/clk-next]
[also build test WARNING on robh/for-next drm-misc/drm-misc-next drm-tip/drm-tip linus/master v5.19-rc8 next-20220728]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Akhil-P-Oommen/clk-qcom-Support-gdsc-collapse-polling-using-reset-inteface/20220730-171922
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
config: ia64-randconfig-r031-20220729 (https://download.01.org/0day-ci/archive/20220730/202207302137.mPbHPaHz-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/971a03493e9854ff4a227ee4d80b533997959891
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Akhil-P-Oommen/clk-qcom-Support-gdsc-collapse-polling-using-reset-inteface/20220730-171922
git checkout 971a03493e9854ff4a227ee4d80b533997959891
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/clk/qcom/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/clk/qcom/reset.c: In function 'qcom_reset':
>> drivers/clk/qcom/reset.c:17:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
17 | const struct qcom_reset_map *map = &rst->reset_map[id];
| ^~~~~
vim +17 drivers/clk/qcom/reset.c
13
14 static int qcom_reset(struct reset_controller_dev *rcdev, unsigned long id)
15 {
16 struct qcom_reset_controller *rst = to_qcom_reset_controller(rcdev);
> 17 const struct qcom_reset_map *map = &rst->reset_map[id];
18
19 if (map->op)
20 return map->op(map);
21
22 rcdev->ops->assert(rcdev, id);
23 udelay(1);
24 rcdev->ops->deassert(rcdev, id);
25 return 0;
26 }
27
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Freedreno] [PATCH 2/5] clk: qcom: Allow custom reset ops
2022-07-30 13:10 ` kernel test robot
2022-08-01 15:20 ` Akhil P Oommen
@ 2022-08-01 15:20 ` Akhil P Oommen
0 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-08-01 15:20 UTC (permalink / raw)
To: kernel test robot, freedreno, dri-devel, linux-arm-msm, Rob Clark,
Bjorn Andersson, Stephen Boyd
Cc: kbuild-all, Michael Turquette, Konrad Dybcio, Douglas Anderson,
linux-kernel, Andy Gross, linux-clk
On 7/30/2022 6:40 PM, kernel test robot wrote:
> Hi Akhil,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on clk/clk-next]
> [also build test WARNING on robh/for-next drm-misc/drm-misc-next drm-tip/drm-tip linus/master v5.19-rc8 next-20220728]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Akhil-P-Oommen/clk-qcom-Support-gdsc-collapse-polling-using-reset-inteface/20220730-171922
> base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
> config: ia64-randconfig-r031-20220729 (https://download.01.org/0day-ci/archive/20220730/202207302137.mPbHPaHz-lkp@intel.com/config)
> compiler: ia64-linux-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/971a03493e9854ff4a227ee4d80b533997959891
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Akhil-P-Oommen/clk-qcom-Support-gdsc-collapse-polling-using-reset-inteface/20220730-171922
> git checkout 971a03493e9854ff4a227ee4d80b533997959891
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/clk/qcom/
>
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> drivers/clk/qcom/reset.c: In function 'qcom_reset':
>>> drivers/clk/qcom/reset.c:17:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
> 17 | const struct qcom_reset_map *map = &rst->reset_map[id];
> | ^~~~~
>
>
> vim +17 drivers/clk/qcom/reset.c
>
> 13
> 14 static int qcom_reset(struct reset_controller_dev *rcdev, unsigned long id)
> 15 {
> 16 struct qcom_reset_controller *rst = to_qcom_reset_controller(rcdev);
> > 17 const struct qcom_reset_map *map = &rst->reset_map[id];
> 18
> 19 if (map->op)
> 20 return map->op(map);
> 21
> 22 rcdev->ops->assert(rcdev, id);
> 23 udelay(1);
> 24 rcdev->ops->deassert(rcdev, id);
> 25 return 0;
> 26 }
> 27
>
Will fix this and send another version of this patch. Please let me know
if there is any feedback to the whole series.
-Akhil.
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Freedreno] [PATCH 2/5] clk: qcom: Allow custom reset ops
@ 2022-08-01 15:20 ` Akhil P Oommen
0 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-08-01 15:20 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2856 bytes --]
On 7/30/2022 6:40 PM, kernel test robot wrote:
> Hi Akhil,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on clk/clk-next]
> [also build test WARNING on robh/for-next drm-misc/drm-misc-next drm-tip/drm-tip linus/master v5.19-rc8 next-20220728]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Akhil-P-Oommen/clk-qcom-Support-gdsc-collapse-polling-using-reset-inteface/20220730-171922
> base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
> config: ia64-randconfig-r031-20220729 (https://download.01.org/0day-ci/archive/20220730/202207302137.mPbHPaHz-lkp(a)intel.com/config)
> compiler: ia64-linux-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/971a03493e9854ff4a227ee4d80b533997959891
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Akhil-P-Oommen/clk-qcom-Support-gdsc-collapse-polling-using-reset-inteface/20220730-171922
> git checkout 971a03493e9854ff4a227ee4d80b533997959891
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/clk/qcom/
>
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> drivers/clk/qcom/reset.c: In function 'qcom_reset':
>>> drivers/clk/qcom/reset.c:17:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
> 17 | const struct qcom_reset_map *map = &rst->reset_map[id];
> | ^~~~~
>
>
> vim +17 drivers/clk/qcom/reset.c
>
> 13
> 14 static int qcom_reset(struct reset_controller_dev *rcdev, unsigned long id)
> 15 {
> 16 struct qcom_reset_controller *rst = to_qcom_reset_controller(rcdev);
> > 17 const struct qcom_reset_map *map = &rst->reset_map[id];
> 18
> 19 if (map->op)
> 20 return map->op(map);
> 21
> 22 rcdev->ops->assert(rcdev, id);
> 23 udelay(1);
> 24 rcdev->ops->deassert(rcdev, id);
> 25 return 0;
> 26 }
> 27
>
Will fix this and send another version of this patch. Please let me know
if there is any feedback to the whole series.
-Akhil.
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Freedreno] [PATCH 2/5] clk: qcom: Allow custom reset ops
@ 2022-08-01 15:20 ` Akhil P Oommen
0 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-08-01 15:20 UTC (permalink / raw)
To: kernel test robot, freedreno, dri-devel, linux-arm-msm, Rob Clark,
Bjorn Andersson, Stephen Boyd
Cc: kbuild-all, Michael Turquette, Konrad Dybcio, Douglas Anderson,
linux-kernel, Andy Gross, linux-clk
On 7/30/2022 6:40 PM, kernel test robot wrote:
> Hi Akhil,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on clk/clk-next]
> [also build test WARNING on robh/for-next drm-misc/drm-misc-next drm-tip/drm-tip linus/master v5.19-rc8 next-20220728]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Akhil-P-Oommen/clk-qcom-Support-gdsc-collapse-polling-using-reset-inteface/20220730-171922
> base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
> config: ia64-randconfig-r031-20220729 (https://download.01.org/0day-ci/archive/20220730/202207302137.mPbHPaHz-lkp@intel.com/config)
> compiler: ia64-linux-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/971a03493e9854ff4a227ee4d80b533997959891
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Akhil-P-Oommen/clk-qcom-Support-gdsc-collapse-polling-using-reset-inteface/20220730-171922
> git checkout 971a03493e9854ff4a227ee4d80b533997959891
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/clk/qcom/
>
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> drivers/clk/qcom/reset.c: In function 'qcom_reset':
>>> drivers/clk/qcom/reset.c:17:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
> 17 | const struct qcom_reset_map *map = &rst->reset_map[id];
> | ^~~~~
>
>
> vim +17 drivers/clk/qcom/reset.c
>
> 13
> 14 static int qcom_reset(struct reset_controller_dev *rcdev, unsigned long id)
> 15 {
> 16 struct qcom_reset_controller *rst = to_qcom_reset_controller(rcdev);
> > 17 const struct qcom_reset_map *map = &rst->reset_map[id];
> 18
> 19 if (map->op)
> 20 return map->op(map);
> 21
> 22 rcdev->ops->assert(rcdev, id);
> 23 udelay(1);
> 24 rcdev->ops->deassert(rcdev, id);
> 25 return 0;
> 26 }
> 27
>
Will fix this and send another version of this patch. Please let me know
if there is any feedback to the whole series.
-Akhil.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/5] clk: qcom: Allow custom reset ops
2022-07-30 9:17 ` Akhil P Oommen
@ 2022-08-02 7:06 ` Dmitry Baryshkov
-1 siblings, 0 replies; 36+ messages in thread
From: Dmitry Baryshkov @ 2022-08-02 7:06 UTC (permalink / raw)
To: Akhil P Oommen, freedreno, dri-devel, linux-arm-msm, Rob Clark,
Bjorn Andersson, Stephen Boyd
Cc: Douglas Anderson, Andy Gross, Konrad Dybcio, Michael Turquette,
Philipp Zabel, Stephen Boyd, linux-clk, linux-kernel
On 30/07/2022 12:17, Akhil P Oommen wrote:
> Add support to allow soc specific clk drivers to specify a custom reset
> operation. A consumer-driver of the reset framework can call
> "reset_control_reset()" api to trigger this.
>
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---
>
> drivers/clk/qcom/reset.c | 6 ++++++
> drivers/clk/qcom/reset.h | 2 ++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/clk/qcom/reset.c b/drivers/clk/qcom/reset.c
> index 819d194..4782bf1 100644
> --- a/drivers/clk/qcom/reset.c
> +++ b/drivers/clk/qcom/reset.c
> @@ -13,6 +13,12 @@
>
> static int qcom_reset(struct reset_controller_dev *rcdev, unsigned long id)
> {
> + struct qcom_reset_controller *rst = to_qcom_reset_controller(rcdev);
> + const struct qcom_reset_map *map = &rst->reset_map[id];
> +
> + if (map->op)
> + return map->op(map);
This looks like a hack. For example, assert() and deassert() would still
follow the usual pattern of updating the bits. Please at least make them
return -EOPNOTSUP if map->op is defined.
A slightly better solution would be to make qcom_reset implementation
optional (and depending on desc->num_resets being greater than 0). Then
you can register your own reset controller implementation from the gpucc
driver.
> +
> rcdev->ops->assert(rcdev, id);
> udelay(1);
> rcdev->ops->deassert(rcdev, id);
> diff --git a/drivers/clk/qcom/reset.h b/drivers/clk/qcom/reset.h
> index 2a08b5e..295deeb 100644
> --- a/drivers/clk/qcom/reset.h
> +++ b/drivers/clk/qcom/reset.h
> @@ -11,6 +11,8 @@
> struct qcom_reset_map {
> unsigned int reg;
> u8 bit;
> + int (*op)(const struct qcom_reset_map *map);
> + void *priv;
> };
>
> struct regmap;
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 2/5] clk: qcom: Allow custom reset ops
@ 2022-08-02 7:06 ` Dmitry Baryshkov
0 siblings, 0 replies; 36+ messages in thread
From: Dmitry Baryshkov @ 2022-08-02 7:06 UTC (permalink / raw)
To: Akhil P Oommen, freedreno, dri-devel, linux-arm-msm, Rob Clark,
Bjorn Andersson, Stephen Boyd
Cc: Stephen Boyd, Michael Turquette, Konrad Dybcio, Douglas Anderson,
linux-kernel, Andy Gross, linux-clk
On 30/07/2022 12:17, Akhil P Oommen wrote:
> Add support to allow soc specific clk drivers to specify a custom reset
> operation. A consumer-driver of the reset framework can call
> "reset_control_reset()" api to trigger this.
>
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---
>
> drivers/clk/qcom/reset.c | 6 ++++++
> drivers/clk/qcom/reset.h | 2 ++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/clk/qcom/reset.c b/drivers/clk/qcom/reset.c
> index 819d194..4782bf1 100644
> --- a/drivers/clk/qcom/reset.c
> +++ b/drivers/clk/qcom/reset.c
> @@ -13,6 +13,12 @@
>
> static int qcom_reset(struct reset_controller_dev *rcdev, unsigned long id)
> {
> + struct qcom_reset_controller *rst = to_qcom_reset_controller(rcdev);
> + const struct qcom_reset_map *map = &rst->reset_map[id];
> +
> + if (map->op)
> + return map->op(map);
This looks like a hack. For example, assert() and deassert() would still
follow the usual pattern of updating the bits. Please at least make them
return -EOPNOTSUP if map->op is defined.
A slightly better solution would be to make qcom_reset implementation
optional (and depending on desc->num_resets being greater than 0). Then
you can register your own reset controller implementation from the gpucc
driver.
> +
> rcdev->ops->assert(rcdev, id);
> udelay(1);
> rcdev->ops->deassert(rcdev, id);
> diff --git a/drivers/clk/qcom/reset.h b/drivers/clk/qcom/reset.h
> index 2a08b5e..295deeb 100644
> --- a/drivers/clk/qcom/reset.h
> +++ b/drivers/clk/qcom/reset.h
> @@ -11,6 +11,8 @@
> struct qcom_reset_map {
> unsigned int reg;
> u8 bit;
> + int (*op)(const struct qcom_reset_map *map);
> + void *priv;
> };
>
> struct regmap;
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 3/5] clk: qcom: gpucc-sc7280: Add cx collapse reset support
2022-07-30 9:17 ` Akhil P Oommen
@ 2022-07-30 9:17 ` Akhil P Oommen
-1 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-07-30 9:17 UTC (permalink / raw)
To: freedreno, dri-devel, linux-arm-msm, Rob Clark, Bjorn Andersson,
Stephen Boyd
Cc: Douglas Anderson, Akhil P Oommen, Andy Gross, Konrad Dybcio,
Michael Turquette, Stephen Boyd, linux-clk, linux-kernel
Allow a consumer driver to poll for cx gdsc collapse through Reset
framework.
Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/clk/qcom/gpucc-sc7280.c b/drivers/clk/qcom/gpucc-sc7280.c
index 9a832f2..f5df51d 100644
--- a/drivers/clk/qcom/gpucc-sc7280.c
+++ b/drivers/clk/qcom/gpucc-sc7280.c
@@ -433,12 +433,18 @@ static const struct regmap_config gpu_cc_sc7280_regmap_config = {
.fast_io = true,
};
+static const struct qcom_reset_map gpucc_sc7280_resets[] = {
+ [GPU_CX_COLLAPSE] = { .op = gdsc_wait_for_collapse, .priv = &cx_gdsc },
+};
+
static const struct qcom_cc_desc gpu_cc_sc7280_desc = {
.config = &gpu_cc_sc7280_regmap_config,
.clks = gpu_cc_sc7280_clocks,
.num_clks = ARRAY_SIZE(gpu_cc_sc7280_clocks),
.gdscs = gpu_cc_sc7180_gdscs,
.num_gdscs = ARRAY_SIZE(gpu_cc_sc7180_gdscs),
+ .resets = gpucc_sc7280_resets,
+ .num_resets = ARRAY_SIZE(gpucc_sc7280_resets),
};
static const struct of_device_id gpu_cc_sc7280_match_table[] = {
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH 3/5] clk: qcom: gpucc-sc7280: Add cx collapse reset support
@ 2022-07-30 9:17 ` Akhil P Oommen
0 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-07-30 9:17 UTC (permalink / raw)
To: freedreno, dri-devel, linux-arm-msm, Rob Clark, Bjorn Andersson,
Stephen Boyd
Cc: Akhil P Oommen, Michael Turquette, Konrad Dybcio,
Douglas Anderson, linux-kernel, Stephen Boyd, Andy Gross,
linux-clk
Allow a consumer driver to poll for cx gdsc collapse through Reset
framework.
Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/clk/qcom/gpucc-sc7280.c b/drivers/clk/qcom/gpucc-sc7280.c
index 9a832f2..f5df51d 100644
--- a/drivers/clk/qcom/gpucc-sc7280.c
+++ b/drivers/clk/qcom/gpucc-sc7280.c
@@ -433,12 +433,18 @@ static const struct regmap_config gpu_cc_sc7280_regmap_config = {
.fast_io = true,
};
+static const struct qcom_reset_map gpucc_sc7280_resets[] = {
+ [GPU_CX_COLLAPSE] = { .op = gdsc_wait_for_collapse, .priv = &cx_gdsc },
+};
+
static const struct qcom_cc_desc gpu_cc_sc7280_desc = {
.config = &gpu_cc_sc7280_regmap_config,
.clks = gpu_cc_sc7280_clocks,
.num_clks = ARRAY_SIZE(gpu_cc_sc7280_clocks),
.gdscs = gpu_cc_sc7180_gdscs,
.num_gdscs = ARRAY_SIZE(gpu_cc_sc7180_gdscs),
+ .resets = gpucc_sc7280_resets,
+ .num_resets = ARRAY_SIZE(gpucc_sc7280_resets),
};
static const struct of_device_id gpu_cc_sc7280_match_table[] = {
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 3/5] clk: qcom: gpucc-sc7280: Add cx collapse reset support
2022-07-30 9:17 ` Akhil P Oommen
@ 2022-08-02 7:02 ` Dmitry Baryshkov
-1 siblings, 0 replies; 36+ messages in thread
From: Dmitry Baryshkov @ 2022-08-02 7:02 UTC (permalink / raw)
To: Akhil P Oommen, freedreno, dri-devel, linux-arm-msm, Rob Clark,
Bjorn Andersson, Stephen Boyd
Cc: Stephen Boyd, Michael Turquette, Konrad Dybcio, Douglas Anderson,
linux-kernel, Andy Gross, linux-clk
On 30/07/2022 12:17, Akhil P Oommen wrote:
> Allow a consumer driver to poll for cx gdsc collapse through Reset
> framework.
>
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---
>
> drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/clk/qcom/gpucc-sc7280.c b/drivers/clk/qcom/gpucc-sc7280.c
> index 9a832f2..f5df51d 100644
> --- a/drivers/clk/qcom/gpucc-sc7280.c
> +++ b/drivers/clk/qcom/gpucc-sc7280.c
> @@ -433,12 +433,18 @@ static const struct regmap_config gpu_cc_sc7280_regmap_config = {
> .fast_io = true,
> };
>
> +static const struct qcom_reset_map gpucc_sc7280_resets[] = {
> + [GPU_CX_COLLAPSE] = { .op = gdsc_wait_for_collapse, .priv = &cx_gdsc },
This will break bisectability. Please swap this one and the patch 4.
> +};
> +
> static const struct qcom_cc_desc gpu_cc_sc7280_desc = {
> .config = &gpu_cc_sc7280_regmap_config,
> .clks = gpu_cc_sc7280_clocks,
> .num_clks = ARRAY_SIZE(gpu_cc_sc7280_clocks),
> .gdscs = gpu_cc_sc7180_gdscs,
> .num_gdscs = ARRAY_SIZE(gpu_cc_sc7180_gdscs),
> + .resets = gpucc_sc7280_resets,
> + .num_resets = ARRAY_SIZE(gpucc_sc7280_resets),
> };
>
> static const struct of_device_id gpu_cc_sc7280_match_table[] = {
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 3/5] clk: qcom: gpucc-sc7280: Add cx collapse reset support
@ 2022-08-02 7:02 ` Dmitry Baryshkov
0 siblings, 0 replies; 36+ messages in thread
From: Dmitry Baryshkov @ 2022-08-02 7:02 UTC (permalink / raw)
To: Akhil P Oommen, freedreno, dri-devel, linux-arm-msm, Rob Clark,
Bjorn Andersson, Stephen Boyd
Cc: Douglas Anderson, Andy Gross, Konrad Dybcio, Michael Turquette,
Stephen Boyd, linux-clk, linux-kernel
On 30/07/2022 12:17, Akhil P Oommen wrote:
> Allow a consumer driver to poll for cx gdsc collapse through Reset
> framework.
>
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---
>
> drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/clk/qcom/gpucc-sc7280.c b/drivers/clk/qcom/gpucc-sc7280.c
> index 9a832f2..f5df51d 100644
> --- a/drivers/clk/qcom/gpucc-sc7280.c
> +++ b/drivers/clk/qcom/gpucc-sc7280.c
> @@ -433,12 +433,18 @@ static const struct regmap_config gpu_cc_sc7280_regmap_config = {
> .fast_io = true,
> };
>
> +static const struct qcom_reset_map gpucc_sc7280_resets[] = {
> + [GPU_CX_COLLAPSE] = { .op = gdsc_wait_for_collapse, .priv = &cx_gdsc },
This will break bisectability. Please swap this one and the patch 4.
> +};
> +
> static const struct qcom_cc_desc gpu_cc_sc7280_desc = {
> .config = &gpu_cc_sc7280_regmap_config,
> .clks = gpu_cc_sc7280_clocks,
> .num_clks = ARRAY_SIZE(gpu_cc_sc7280_clocks),
> .gdscs = gpu_cc_sc7180_gdscs,
> .num_gdscs = ARRAY_SIZE(gpu_cc_sc7180_gdscs),
> + .resets = gpucc_sc7280_resets,
> + .num_resets = ARRAY_SIZE(gpucc_sc7280_resets),
> };
>
> static const struct of_device_id gpu_cc_sc7280_match_table[] = {
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/5] clk: qcom: gpucc-sc7280: Add cx collapse reset support
2022-07-30 9:17 ` Akhil P Oommen
@ 2022-08-02 7:15 ` Dmitry Baryshkov
-1 siblings, 0 replies; 36+ messages in thread
From: Dmitry Baryshkov @ 2022-08-02 7:15 UTC (permalink / raw)
To: Akhil P Oommen, freedreno, dri-devel, linux-arm-msm, Rob Clark,
Bjorn Andersson, Stephen Boyd
Cc: Douglas Anderson, Andy Gross, Konrad Dybcio, Michael Turquette,
Stephen Boyd, linux-clk, linux-kernel
On 30/07/2022 12:17, Akhil P Oommen wrote:
> Allow a consumer driver to poll for cx gdsc collapse through Reset
> framework.
>
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---
>
> drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/clk/qcom/gpucc-sc7280.c b/drivers/clk/qcom/gpucc-sc7280.c
> index 9a832f2..f5df51d 100644
> --- a/drivers/clk/qcom/gpucc-sc7280.c
> +++ b/drivers/clk/qcom/gpucc-sc7280.c
> @@ -433,12 +433,18 @@ static const struct regmap_config gpu_cc_sc7280_regmap_config = {
> .fast_io = true,
> };
>
> +static const struct qcom_reset_map gpucc_sc7280_resets[] = {
> + [GPU_CX_COLLAPSE] = { .op = gdsc_wait_for_collapse, .priv = &cx_gdsc },
> +};
> +
> static const struct qcom_cc_desc gpu_cc_sc7280_desc = {
> .config = &gpu_cc_sc7280_regmap_config,
> .clks = gpu_cc_sc7280_clocks,
> .num_clks = ARRAY_SIZE(gpu_cc_sc7280_clocks),
> .gdscs = gpu_cc_sc7180_gdscs,
> .num_gdscs = ARRAY_SIZE(gpu_cc_sc7180_gdscs),
> + .resets = gpucc_sc7280_resets,
> + .num_resets = ARRAY_SIZE(gpucc_sc7280_resets),
An implementation question. Do we have to poll for the GDSC on platforms
like sm8150/sm8250 which have the plain BCR resets?
> };
>
> static const struct of_device_id gpu_cc_sc7280_match_table[] = {
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 3/5] clk: qcom: gpucc-sc7280: Add cx collapse reset support
@ 2022-08-02 7:15 ` Dmitry Baryshkov
0 siblings, 0 replies; 36+ messages in thread
From: Dmitry Baryshkov @ 2022-08-02 7:15 UTC (permalink / raw)
To: Akhil P Oommen, freedreno, dri-devel, linux-arm-msm, Rob Clark,
Bjorn Andersson, Stephen Boyd
Cc: Stephen Boyd, Michael Turquette, Konrad Dybcio, Douglas Anderson,
linux-kernel, Andy Gross, linux-clk
On 30/07/2022 12:17, Akhil P Oommen wrote:
> Allow a consumer driver to poll for cx gdsc collapse through Reset
> framework.
>
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---
>
> drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/clk/qcom/gpucc-sc7280.c b/drivers/clk/qcom/gpucc-sc7280.c
> index 9a832f2..f5df51d 100644
> --- a/drivers/clk/qcom/gpucc-sc7280.c
> +++ b/drivers/clk/qcom/gpucc-sc7280.c
> @@ -433,12 +433,18 @@ static const struct regmap_config gpu_cc_sc7280_regmap_config = {
> .fast_io = true,
> };
>
> +static const struct qcom_reset_map gpucc_sc7280_resets[] = {
> + [GPU_CX_COLLAPSE] = { .op = gdsc_wait_for_collapse, .priv = &cx_gdsc },
> +};
> +
> static const struct qcom_cc_desc gpu_cc_sc7280_desc = {
> .config = &gpu_cc_sc7280_regmap_config,
> .clks = gpu_cc_sc7280_clocks,
> .num_clks = ARRAY_SIZE(gpu_cc_sc7280_clocks),
> .gdscs = gpu_cc_sc7180_gdscs,
> .num_gdscs = ARRAY_SIZE(gpu_cc_sc7180_gdscs),
> + .resets = gpucc_sc7280_resets,
> + .num_resets = ARRAY_SIZE(gpucc_sc7280_resets),
An implementation question. Do we have to poll for the GDSC on platforms
like sm8150/sm8250 which have the plain BCR resets?
> };
>
> static const struct of_device_id gpu_cc_sc7280_match_table[] = {
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 3/5] clk: qcom: gpucc-sc7280: Add cx collapse reset support
2022-08-02 7:15 ` Dmitry Baryshkov
@ 2022-08-03 10:20 ` Akhil P Oommen
-1 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-08-03 10:20 UTC (permalink / raw)
To: Dmitry Baryshkov, freedreno, dri-devel, linux-arm-msm, Rob Clark,
Bjorn Andersson, Stephen Boyd
Cc: Stephen Boyd, Michael Turquette, Konrad Dybcio, Douglas Anderson,
linux-kernel, Andy Gross, linux-clk
On 8/2/2022 12:45 PM, Dmitry Baryshkov wrote:
> On 30/07/2022 12:17, Akhil P Oommen wrote:
>> Allow a consumer driver to poll for cx gdsc collapse through Reset
>> framework.
>>
>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
>> ---
>>
>> drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/gpucc-sc7280.c
>> b/drivers/clk/qcom/gpucc-sc7280.c
>> index 9a832f2..f5df51d 100644
>> --- a/drivers/clk/qcom/gpucc-sc7280.c
>> +++ b/drivers/clk/qcom/gpucc-sc7280.c
>> @@ -433,12 +433,18 @@ static const struct regmap_config
>> gpu_cc_sc7280_regmap_config = {
>> .fast_io = true,
>> };
>> +static const struct qcom_reset_map gpucc_sc7280_resets[] = {
>> + [GPU_CX_COLLAPSE] = { .op = gdsc_wait_for_collapse, .priv =
>> &cx_gdsc },
>> +};
>> +
>> static const struct qcom_cc_desc gpu_cc_sc7280_desc = {
>> .config = &gpu_cc_sc7280_regmap_config,
>> .clks = gpu_cc_sc7280_clocks,
>> .num_clks = ARRAY_SIZE(gpu_cc_sc7280_clocks),
>> .gdscs = gpu_cc_sc7180_gdscs,
>> .num_gdscs = ARRAY_SIZE(gpu_cc_sc7180_gdscs),
>> + .resets = gpucc_sc7280_resets,
>> + .num_resets = ARRAY_SIZE(gpucc_sc7280_resets),
>
> An implementation question. Do we have to poll for the GDSC on
> platforms like sm8150/sm8250 which have the plain BCR resets?
Yes.
Thanks for all the feedback, Dmitry. I will post another version of this
series.
-Akhil.
>
>> };
>> static const struct of_device_id gpu_cc_sc7280_match_table[] = {
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 3/5] clk: qcom: gpucc-sc7280: Add cx collapse reset support
@ 2022-08-03 10:20 ` Akhil P Oommen
0 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-08-03 10:20 UTC (permalink / raw)
To: Dmitry Baryshkov, freedreno, dri-devel, linux-arm-msm, Rob Clark,
Bjorn Andersson, Stephen Boyd
Cc: Douglas Anderson, Andy Gross, Konrad Dybcio, Michael Turquette,
Stephen Boyd, linux-clk, linux-kernel
On 8/2/2022 12:45 PM, Dmitry Baryshkov wrote:
> On 30/07/2022 12:17, Akhil P Oommen wrote:
>> Allow a consumer driver to poll for cx gdsc collapse through Reset
>> framework.
>>
>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
>> ---
>>
>> drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/gpucc-sc7280.c
>> b/drivers/clk/qcom/gpucc-sc7280.c
>> index 9a832f2..f5df51d 100644
>> --- a/drivers/clk/qcom/gpucc-sc7280.c
>> +++ b/drivers/clk/qcom/gpucc-sc7280.c
>> @@ -433,12 +433,18 @@ static const struct regmap_config
>> gpu_cc_sc7280_regmap_config = {
>> .fast_io = true,
>> };
>> +static const struct qcom_reset_map gpucc_sc7280_resets[] = {
>> + [GPU_CX_COLLAPSE] = { .op = gdsc_wait_for_collapse, .priv =
>> &cx_gdsc },
>> +};
>> +
>> static const struct qcom_cc_desc gpu_cc_sc7280_desc = {
>> .config = &gpu_cc_sc7280_regmap_config,
>> .clks = gpu_cc_sc7280_clocks,
>> .num_clks = ARRAY_SIZE(gpu_cc_sc7280_clocks),
>> .gdscs = gpu_cc_sc7180_gdscs,
>> .num_gdscs = ARRAY_SIZE(gpu_cc_sc7180_gdscs),
>> + .resets = gpucc_sc7280_resets,
>> + .num_resets = ARRAY_SIZE(gpucc_sc7280_resets),
>
> An implementation question. Do we have to poll for the GDSC on
> platforms like sm8150/sm8250 which have the plain BCR resets?
Yes.
Thanks for all the feedback, Dmitry. I will post another version of this
series.
-Akhil.
>
>> };
>> static const struct of_device_id gpu_cc_sc7280_match_table[] = {
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 4/5] clk: qcom: gdsc: Add a reset op to poll gdsc collapse
2022-07-30 9:17 ` Akhil P Oommen
@ 2022-07-30 9:17 ` Akhil P Oommen
-1 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-07-30 9:17 UTC (permalink / raw)
To: freedreno, dri-devel, linux-arm-msm, Rob Clark, Bjorn Andersson,
Stephen Boyd
Cc: Douglas Anderson, Akhil P Oommen, Andy Gross, Konrad Dybcio,
Michael Turquette, Stephen Boyd, linux-clk, linux-kernel
Add a reset op compatible function to poll for gdsc collapse.
Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
drivers/clk/qcom/gdsc.c | 23 +++++++++++++++++++----
drivers/clk/qcom/gdsc.h | 7 +++++++
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 44520ef..0c9f648 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -17,6 +17,7 @@
#include <linux/reset-controller.h>
#include <linux/slab.h>
#include "gdsc.h"
+#include "reset.h"
#define PWR_ON_MASK BIT(31)
#define EN_REST_WAIT_MASK GENMASK_ULL(23, 20)
@@ -116,7 +117,8 @@ static int gdsc_hwctrl(struct gdsc *sc, bool en)
return regmap_update_bits(sc->regmap, sc->gdscr, HW_CONTROL_MASK, val);
}
-static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status)
+static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status,
+ s64 timeout_us, unsigned int interval_ms)
{
ktime_t start;
@@ -124,7 +126,9 @@ static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status)
do {
if (gdsc_check_status(sc, status))
return 0;
- } while (ktime_us_delta(ktime_get(), start) < TIMEOUT_US);
+ if (interval_ms)
+ msleep(interval_ms);
+ } while (ktime_us_delta(ktime_get(), start) < timeout_us);
if (gdsc_check_status(sc, status))
return 0;
@@ -172,7 +176,7 @@ static int gdsc_toggle_logic(struct gdsc *sc, enum gdsc_status status)
udelay(1);
}
- ret = gdsc_poll_status(sc, status);
+ ret = gdsc_poll_status(sc, status, TIMEOUT_US, 0);
WARN(ret, "%s status stuck at 'o%s'", sc->pd.name, status ? "ff" : "n");
if (!ret && status == GDSC_OFF && sc->rsupply) {
@@ -343,7 +347,7 @@ static int _gdsc_disable(struct gdsc *sc)
*/
udelay(1);
- ret = gdsc_poll_status(sc, GDSC_ON);
+ ret = gdsc_poll_status(sc, GDSC_ON, TIMEOUT_US, 0);
if (ret)
return ret;
}
@@ -565,3 +569,14 @@ int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
return 0;
}
EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
+
+int gdsc_wait_for_collapse(const struct qcom_reset_map *map)
+{
+ struct gdsc *sc = map->priv;
+ int ret;
+
+ ret = gdsc_poll_status(sc, GDSC_OFF, 500000, 5);
+ WARN(ret, "%s status stuck at 'on'", sc->pd.name);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(gdsc_wait_for_collapse);
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index ad313d7..b76b6b6 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -12,6 +12,7 @@
struct regmap;
struct regulator;
struct reset_controller_dev;
+struct qcom_reset_map;
/**
* struct gdsc - Globally Distributed Switch Controller
@@ -79,6 +80,7 @@ int gdsc_register(struct gdsc_desc *desc, struct reset_controller_dev *,
struct regmap *);
void gdsc_unregister(struct gdsc_desc *desc);
int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain);
+int gdsc_wait_for_collapse(const struct qcom_reset_map *map);
#else
static inline int gdsc_register(struct gdsc_desc *desc,
struct reset_controller_dev *rcdev,
@@ -88,5 +90,10 @@ static inline int gdsc_register(struct gdsc_desc *desc,
}
static inline void gdsc_unregister(struct gdsc_desc *desc) {};
+
+static int gdsc_wait_for_collapse(const struct qcom_reset_map *map)
+{
+ return -ENOSYS;
+}
#endif /* CONFIG_QCOM_GDSC */
#endif /* __QCOM_GDSC_H__ */
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH 4/5] clk: qcom: gdsc: Add a reset op to poll gdsc collapse
@ 2022-07-30 9:17 ` Akhil P Oommen
0 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-07-30 9:17 UTC (permalink / raw)
To: freedreno, dri-devel, linux-arm-msm, Rob Clark, Bjorn Andersson,
Stephen Boyd
Cc: Akhil P Oommen, Michael Turquette, Konrad Dybcio,
Douglas Anderson, linux-kernel, Stephen Boyd, Andy Gross,
linux-clk
Add a reset op compatible function to poll for gdsc collapse.
Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
drivers/clk/qcom/gdsc.c | 23 +++++++++++++++++++----
drivers/clk/qcom/gdsc.h | 7 +++++++
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 44520ef..0c9f648 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -17,6 +17,7 @@
#include <linux/reset-controller.h>
#include <linux/slab.h>
#include "gdsc.h"
+#include "reset.h"
#define PWR_ON_MASK BIT(31)
#define EN_REST_WAIT_MASK GENMASK_ULL(23, 20)
@@ -116,7 +117,8 @@ static int gdsc_hwctrl(struct gdsc *sc, bool en)
return regmap_update_bits(sc->regmap, sc->gdscr, HW_CONTROL_MASK, val);
}
-static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status)
+static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status,
+ s64 timeout_us, unsigned int interval_ms)
{
ktime_t start;
@@ -124,7 +126,9 @@ static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status)
do {
if (gdsc_check_status(sc, status))
return 0;
- } while (ktime_us_delta(ktime_get(), start) < TIMEOUT_US);
+ if (interval_ms)
+ msleep(interval_ms);
+ } while (ktime_us_delta(ktime_get(), start) < timeout_us);
if (gdsc_check_status(sc, status))
return 0;
@@ -172,7 +176,7 @@ static int gdsc_toggle_logic(struct gdsc *sc, enum gdsc_status status)
udelay(1);
}
- ret = gdsc_poll_status(sc, status);
+ ret = gdsc_poll_status(sc, status, TIMEOUT_US, 0);
WARN(ret, "%s status stuck at 'o%s'", sc->pd.name, status ? "ff" : "n");
if (!ret && status == GDSC_OFF && sc->rsupply) {
@@ -343,7 +347,7 @@ static int _gdsc_disable(struct gdsc *sc)
*/
udelay(1);
- ret = gdsc_poll_status(sc, GDSC_ON);
+ ret = gdsc_poll_status(sc, GDSC_ON, TIMEOUT_US, 0);
if (ret)
return ret;
}
@@ -565,3 +569,14 @@ int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
return 0;
}
EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
+
+int gdsc_wait_for_collapse(const struct qcom_reset_map *map)
+{
+ struct gdsc *sc = map->priv;
+ int ret;
+
+ ret = gdsc_poll_status(sc, GDSC_OFF, 500000, 5);
+ WARN(ret, "%s status stuck at 'on'", sc->pd.name);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(gdsc_wait_for_collapse);
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index ad313d7..b76b6b6 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -12,6 +12,7 @@
struct regmap;
struct regulator;
struct reset_controller_dev;
+struct qcom_reset_map;
/**
* struct gdsc - Globally Distributed Switch Controller
@@ -79,6 +80,7 @@ int gdsc_register(struct gdsc_desc *desc, struct reset_controller_dev *,
struct regmap *);
void gdsc_unregister(struct gdsc_desc *desc);
int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain);
+int gdsc_wait_for_collapse(const struct qcom_reset_map *map);
#else
static inline int gdsc_register(struct gdsc_desc *desc,
struct reset_controller_dev *rcdev,
@@ -88,5 +90,10 @@ static inline int gdsc_register(struct gdsc_desc *desc,
}
static inline void gdsc_unregister(struct gdsc_desc *desc) {};
+
+static int gdsc_wait_for_collapse(const struct qcom_reset_map *map)
+{
+ return -ENOSYS;
+}
#endif /* CONFIG_QCOM_GDSC */
#endif /* __QCOM_GDSC_H__ */
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 5/5] arm64: dts: qcom: sc7280: Add Reset support for gpu
2022-07-30 9:17 ` Akhil P Oommen
@ 2022-07-30 9:17 ` Akhil P Oommen
-1 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-07-30 9:17 UTC (permalink / raw)
To: freedreno, dri-devel, linux-arm-msm, Rob Clark, Bjorn Andersson,
Stephen Boyd
Cc: Douglas Anderson, Akhil P Oommen, Andy Gross, Konrad Dybcio,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel
Add support for Reset using GPUCC driver for GPU. This helps to ensure
that GPU state is reset by making sure that CX head switch is collapsed.
Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index e66fc67..f5257d6 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2243,6 +2243,9 @@
nvmem-cells = <&gpu_speed_bin>;
nvmem-cell-names = "speed_bin";
+ resets = <&gpucc GPU_CX_COLLAPSE>;
+ reset-names = "cx_collapse";
+
gpu_opp_table: opp-table {
compatible = "operating-points-v2";
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH 5/5] arm64: dts: qcom: sc7280: Add Reset support for gpu
@ 2022-07-30 9:17 ` Akhil P Oommen
0 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-07-30 9:17 UTC (permalink / raw)
To: freedreno, dri-devel, linux-arm-msm, Rob Clark, Bjorn Andersson,
Stephen Boyd
Cc: devicetree, Akhil P Oommen, linux-kernel, Konrad Dybcio,
Andy Gross, Douglas Anderson, Rob Herring, Krzysztof Kozlowski
Add support for Reset using GPUCC driver for GPU. This helps to ensure
that GPU state is reset by making sure that CX head switch is collapsed.
Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index e66fc67..f5257d6 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2243,6 +2243,9 @@
nvmem-cells = <&gpu_speed_bin>;
nvmem-cell-names = "speed_bin";
+ resets = <&gpucc GPU_CX_COLLAPSE>;
+ reset-names = "cx_collapse";
+
gpu_opp_table: opp-table {
compatible = "operating-points-v2";
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface
2022-07-30 9:17 ` Akhil P Oommen
@ 2022-08-02 7:02 ` Dmitry Baryshkov
-1 siblings, 0 replies; 36+ messages in thread
From: Dmitry Baryshkov @ 2022-08-02 7:02 UTC (permalink / raw)
To: Akhil P Oommen, freedreno, dri-devel, linux-arm-msm, Rob Clark,
Bjorn Andersson, Stephen Boyd
Cc: devicetree, Krzysztof Kozlowski, Stephen Boyd, Michael Turquette,
Konrad Dybcio, Douglas Anderson, Rob Herring, linux-kernel,
Andy Gross, linux-clk
On 30/07/2022 12:17, Akhil P Oommen wrote:
>
> Some clients like adreno gpu driver would like to ensure that its gdsc
> is collapsed at hardware during a gpu reset sequence. This is because it
> has a votable gdsc which could be ON due to a vote from another subsystem
> like tz, hyp etc or due to an internal hardware signal.
If this is votable, do we have any guarantee that the gdsc will collapse
at all? How can we proceed if it did not collapse?
> To allow
> this, gpucc driver can expose an interface to the client driver using
> reset framework. Using this the client driver can trigger a polling within
> the gdsc driver.
Trigger the polling made me think initially that we will actually
trigger something in the HW. Instead the client uses reset framework to
poll for the gdsc to be reset.
>
> This series is rebased on top of linus's master branch.
>
> Related discussion: https://patchwork.freedesktop.org/patch/493144/
>
>
> Akhil P Oommen (5):
> dt-bindings: clk: qcom: Support gpu cx gdsc reset
> clk: qcom: Allow custom reset ops
> clk: qcom: gpucc-sc7280: Add cx collapse reset support
> clk: qcom: gdsc: Add a reset op to poll gdsc collapse
> arm64: dts: qcom: sc7280: Add Reset support for gpu
>
> arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++
> drivers/clk/qcom/gdsc.c | 23 +++++++++++++++++++----
> drivers/clk/qcom/gdsc.h | 7 +++++++
> drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
> drivers/clk/qcom/reset.c | 6 ++++++
> drivers/clk/qcom/reset.h | 2 ++
> include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++
> 7 files changed, 46 insertions(+), 4 deletions(-)
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface
@ 2022-08-02 7:02 ` Dmitry Baryshkov
0 siblings, 0 replies; 36+ messages in thread
From: Dmitry Baryshkov @ 2022-08-02 7:02 UTC (permalink / raw)
To: Akhil P Oommen, freedreno, dri-devel, linux-arm-msm, Rob Clark,
Bjorn Andersson, Stephen Boyd
Cc: Douglas Anderson, Andy Gross, Konrad Dybcio, Krzysztof Kozlowski,
Michael Turquette, Philipp Zabel, Rob Herring, Stephen Boyd,
devicetree, linux-clk, linux-kernel
On 30/07/2022 12:17, Akhil P Oommen wrote:
>
> Some clients like adreno gpu driver would like to ensure that its gdsc
> is collapsed at hardware during a gpu reset sequence. This is because it
> has a votable gdsc which could be ON due to a vote from another subsystem
> like tz, hyp etc or due to an internal hardware signal.
If this is votable, do we have any guarantee that the gdsc will collapse
at all? How can we proceed if it did not collapse?
> To allow
> this, gpucc driver can expose an interface to the client driver using
> reset framework. Using this the client driver can trigger a polling within
> the gdsc driver.
Trigger the polling made me think initially that we will actually
trigger something in the HW. Instead the client uses reset framework to
poll for the gdsc to be reset.
>
> This series is rebased on top of linus's master branch.
>
> Related discussion: https://patchwork.freedesktop.org/patch/493144/
>
>
> Akhil P Oommen (5):
> dt-bindings: clk: qcom: Support gpu cx gdsc reset
> clk: qcom: Allow custom reset ops
> clk: qcom: gpucc-sc7280: Add cx collapse reset support
> clk: qcom: gdsc: Add a reset op to poll gdsc collapse
> arm64: dts: qcom: sc7280: Add Reset support for gpu
>
> arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++
> drivers/clk/qcom/gdsc.c | 23 +++++++++++++++++++----
> drivers/clk/qcom/gdsc.h | 7 +++++++
> drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
> drivers/clk/qcom/reset.c | 6 ++++++
> drivers/clk/qcom/reset.h | 2 ++
> include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++
> 7 files changed, 46 insertions(+), 4 deletions(-)
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface
2022-08-02 7:02 ` Dmitry Baryshkov
@ 2022-08-02 18:32 ` Rob Clark
-1 siblings, 0 replies; 36+ messages in thread
From: Rob Clark @ 2022-08-02 18:32 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: devicetree, Stephen Boyd, Akhil P Oommen, linux-arm-msm,
Michael Turquette, Konrad Dybcio, Douglas Anderson, dri-devel,
Stephen Boyd, Rob Herring, Andy Gross, Krzysztof Kozlowski,
Bjorn Andersson, freedreno, linux-clk, linux-kernel
On Tue, Aug 2, 2022 at 12:02 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On 30/07/2022 12:17, Akhil P Oommen wrote:
> >
> > Some clients like adreno gpu driver would like to ensure that its gdsc
> > is collapsed at hardware during a gpu reset sequence. This is because it
> > has a votable gdsc which could be ON due to a vote from another subsystem
> > like tz, hyp etc or due to an internal hardware signal.
>
> If this is votable, do we have any guarantee that the gdsc will collapse
> at all? How can we proceed if it did not collapse?
Other potential votes should be transient. But I guess we eventually
need to timeout and give up. At which point we are no worse off than
before.
But hmm, we aren't using RBBM_SW_RESET_CMD for sw reset like we have
on previous generations? That does seem a bit odd. Looks like kgsl
does use it.
BR,
-R
> > To allow
> > this, gpucc driver can expose an interface to the client driver using
> > reset framework. Using this the client driver can trigger a polling within
> > the gdsc driver.
>
> Trigger the polling made me think initially that we will actually
> trigger something in the HW. Instead the client uses reset framework to
> poll for the gdsc to be reset.
>
> >
> > This series is rebased on top of linus's master branch.
> >
> > Related discussion: https://patchwork.freedesktop.org/patch/493144/
> >
> >
> > Akhil P Oommen (5):
> > dt-bindings: clk: qcom: Support gpu cx gdsc reset
> > clk: qcom: Allow custom reset ops
> > clk: qcom: gpucc-sc7280: Add cx collapse reset support
> > clk: qcom: gdsc: Add a reset op to poll gdsc collapse
> > arm64: dts: qcom: sc7280: Add Reset support for gpu
> >
> > arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++
> > drivers/clk/qcom/gdsc.c | 23 +++++++++++++++++++----
> > drivers/clk/qcom/gdsc.h | 7 +++++++
> > drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
> > drivers/clk/qcom/reset.c | 6 ++++++
> > drivers/clk/qcom/reset.h | 2 ++
> > include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++
> > 7 files changed, 46 insertions(+), 4 deletions(-)
> >
>
>
> --
> With best wishes
> Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface
@ 2022-08-02 18:32 ` Rob Clark
0 siblings, 0 replies; 36+ messages in thread
From: Rob Clark @ 2022-08-02 18:32 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Akhil P Oommen, freedreno, dri-devel, linux-arm-msm,
Bjorn Andersson, Stephen Boyd, Douglas Anderson, Andy Gross,
Konrad Dybcio, Krzysztof Kozlowski, Michael Turquette,
Philipp Zabel, Rob Herring, Stephen Boyd, devicetree, linux-clk,
linux-kernel
On Tue, Aug 2, 2022 at 12:02 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On 30/07/2022 12:17, Akhil P Oommen wrote:
> >
> > Some clients like adreno gpu driver would like to ensure that its gdsc
> > is collapsed at hardware during a gpu reset sequence. This is because it
> > has a votable gdsc which could be ON due to a vote from another subsystem
> > like tz, hyp etc or due to an internal hardware signal.
>
> If this is votable, do we have any guarantee that the gdsc will collapse
> at all? How can we proceed if it did not collapse?
Other potential votes should be transient. But I guess we eventually
need to timeout and give up. At which point we are no worse off than
before.
But hmm, we aren't using RBBM_SW_RESET_CMD for sw reset like we have
on previous generations? That does seem a bit odd. Looks like kgsl
does use it.
BR,
-R
> > To allow
> > this, gpucc driver can expose an interface to the client driver using
> > reset framework. Using this the client driver can trigger a polling within
> > the gdsc driver.
>
> Trigger the polling made me think initially that we will actually
> trigger something in the HW. Instead the client uses reset framework to
> poll for the gdsc to be reset.
>
> >
> > This series is rebased on top of linus's master branch.
> >
> > Related discussion: https://patchwork.freedesktop.org/patch/493144/
> >
> >
> > Akhil P Oommen (5):
> > dt-bindings: clk: qcom: Support gpu cx gdsc reset
> > clk: qcom: Allow custom reset ops
> > clk: qcom: gpucc-sc7280: Add cx collapse reset support
> > clk: qcom: gdsc: Add a reset op to poll gdsc collapse
> > arm64: dts: qcom: sc7280: Add Reset support for gpu
> >
> > arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++
> > drivers/clk/qcom/gdsc.c | 23 +++++++++++++++++++----
> > drivers/clk/qcom/gdsc.h | 7 +++++++
> > drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
> > drivers/clk/qcom/reset.c | 6 ++++++
> > drivers/clk/qcom/reset.h | 2 ++
> > include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++
> > 7 files changed, 46 insertions(+), 4 deletions(-)
> >
>
>
> --
> With best wishes
> Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface
2022-08-02 18:32 ` Rob Clark
@ 2022-08-03 10:01 ` Akhil P Oommen
-1 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-08-03 10:01 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov
Cc: devicetree, Stephen Boyd, linux-arm-msm, Michael Turquette,
Konrad Dybcio, Douglas Anderson, dri-devel, Stephen Boyd,
Rob Herring, Andy Gross, Krzysztof Kozlowski, Bjorn Andersson,
freedreno, linux-clk, linux-kernel
On 8/3/2022 12:02 AM, Rob Clark wrote:
> On Tue, Aug 2, 2022 at 12:02 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
>> On 30/07/2022 12:17, Akhil P Oommen wrote:
>>> Some clients like adreno gpu driver would like to ensure that its gdsc
>>> is collapsed at hardware during a gpu reset sequence. This is because it
>>> has a votable gdsc which could be ON due to a vote from another subsystem
>>> like tz, hyp etc or due to an internal hardware signal.
>> If this is votable, do we have any guarantee that the gdsc will collapse
>> at all? How can we proceed if it did not collapse?
> Other potential votes should be transient. But I guess we eventually
> need to timeout and give up. At which point we are no worse off than
> before.
>
> But hmm, we aren't using RBBM_SW_RESET_CMD for sw reset like we have
> on previous generations? That does seem a bit odd. Looks like kgsl
> does use it.
>
> BR,
> -R
Like Rob mentioned there could be transient votes from other
clients/subsystem. It could be even stuck ON when hardware is in bad
shape in some very rare cases. For the worst case scenario, I have added
a timeout (500msec) in the gdsc reset op.
I have added the Soft reset in [1]. But this resets only the core gpu
blocks, not everything. For eg. GMU.
[1] [PATCH v3 7/8] drm/msm/a6xx: Improve gpu recovery sequence
>
>>> To allow
>>> this, gpucc driver can expose an interface to the client driver using
>>> reset framework. Using this the client driver can trigger a polling within
>>> the gdsc driver.
>> Trigger the polling made me think initially that we will actually
>> trigger something in the HW. Instead the client uses reset framework to
>> poll for the gdsc to be reset.
Yes. I should replace 'trigger' with 'start' here.
-Akhil.
>>
>>> This series is rebased on top of linus's master branch.
>>>
>>> Related discussion: https://patchwork.freedesktop.org/patch/493144/
>>>
>>>
>>> Akhil P Oommen (5):
>>> dt-bindings: clk: qcom: Support gpu cx gdsc reset
>>> clk: qcom: Allow custom reset ops
>>> clk: qcom: gpucc-sc7280: Add cx collapse reset support
>>> clk: qcom: gdsc: Add a reset op to poll gdsc collapse
>>> arm64: dts: qcom: sc7280: Add Reset support for gpu
>>>
>>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++
>>> drivers/clk/qcom/gdsc.c | 23 +++++++++++++++++++----
>>> drivers/clk/qcom/gdsc.h | 7 +++++++
>>> drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
>>> drivers/clk/qcom/reset.c | 6 ++++++
>>> drivers/clk/qcom/reset.h | 2 ++
>>> include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++
>>> 7 files changed, 46 insertions(+), 4 deletions(-)
>>>
>>
>> --
>> With best wishes
>> Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface
@ 2022-08-03 10:01 ` Akhil P Oommen
0 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-08-03 10:01 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov
Cc: freedreno, dri-devel, linux-arm-msm, Bjorn Andersson,
Stephen Boyd, Douglas Anderson, Andy Gross, Konrad Dybcio,
Krzysztof Kozlowski, Michael Turquette, Philipp Zabel,
Rob Herring, Stephen Boyd, devicetree, linux-clk, linux-kernel
On 8/3/2022 12:02 AM, Rob Clark wrote:
> On Tue, Aug 2, 2022 at 12:02 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
>> On 30/07/2022 12:17, Akhil P Oommen wrote:
>>> Some clients like adreno gpu driver would like to ensure that its gdsc
>>> is collapsed at hardware during a gpu reset sequence. This is because it
>>> has a votable gdsc which could be ON due to a vote from another subsystem
>>> like tz, hyp etc or due to an internal hardware signal.
>> If this is votable, do we have any guarantee that the gdsc will collapse
>> at all? How can we proceed if it did not collapse?
> Other potential votes should be transient. But I guess we eventually
> need to timeout and give up. At which point we are no worse off than
> before.
>
> But hmm, we aren't using RBBM_SW_RESET_CMD for sw reset like we have
> on previous generations? That does seem a bit odd. Looks like kgsl
> does use it.
>
> BR,
> -R
Like Rob mentioned there could be transient votes from other
clients/subsystem. It could be even stuck ON when hardware is in bad
shape in some very rare cases. For the worst case scenario, I have added
a timeout (500msec) in the gdsc reset op.
I have added the Soft reset in [1]. But this resets only the core gpu
blocks, not everything. For eg. GMU.
[1] [PATCH v3 7/8] drm/msm/a6xx: Improve gpu recovery sequence
>
>>> To allow
>>> this, gpucc driver can expose an interface to the client driver using
>>> reset framework. Using this the client driver can trigger a polling within
>>> the gdsc driver.
>> Trigger the polling made me think initially that we will actually
>> trigger something in the HW. Instead the client uses reset framework to
>> poll for the gdsc to be reset.
Yes. I should replace 'trigger' with 'start' here.
-Akhil.
>>
>>> This series is rebased on top of linus's master branch.
>>>
>>> Related discussion: https://patchwork.freedesktop.org/patch/493144/
>>>
>>>
>>> Akhil P Oommen (5):
>>> dt-bindings: clk: qcom: Support gpu cx gdsc reset
>>> clk: qcom: Allow custom reset ops
>>> clk: qcom: gpucc-sc7280: Add cx collapse reset support
>>> clk: qcom: gdsc: Add a reset op to poll gdsc collapse
>>> arm64: dts: qcom: sc7280: Add Reset support for gpu
>>>
>>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++
>>> drivers/clk/qcom/gdsc.c | 23 +++++++++++++++++++----
>>> drivers/clk/qcom/gdsc.h | 7 +++++++
>>> drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
>>> drivers/clk/qcom/reset.c | 6 ++++++
>>> drivers/clk/qcom/reset.h | 2 ++
>>> include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++
>>> 7 files changed, 46 insertions(+), 4 deletions(-)
>>>
>>
>> --
>> With best wishes
>> Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface
2022-07-30 9:17 ` Akhil P Oommen
@ 2022-08-09 21:05 ` Bjorn Andersson
-1 siblings, 0 replies; 36+ messages in thread
From: Bjorn Andersson @ 2022-08-09 21:05 UTC (permalink / raw)
To: Akhil P Oommen
Cc: devicetree, Stephen Boyd, linux-arm-msm, Michael Turquette,
Konrad Dybcio, Douglas Anderson, dri-devel, Stephen Boyd,
Rob Herring, Andy Gross, Krzysztof Kozlowski, freedreno,
linux-clk, linux-kernel
On Sat 30 Jul 04:17 CDT 2022, Akhil P Oommen wrote:
>
> Some clients like adreno gpu driver would like to ensure that its gdsc
> is collapsed at hardware during a gpu reset sequence. This is because it
> has a votable gdsc which could be ON due to a vote from another subsystem
> like tz, hyp etc or due to an internal hardware signal. To allow
> this, gpucc driver can expose an interface to the client driver using
> reset framework. Using this the client driver can trigger a polling within
> the gdsc driver.
>
> This series is rebased on top of linus's master branch.
>
> Related discussion: https://patchwork.freedesktop.org/patch/493144/
>
Forgive me if I'm assuming too much, but isn't this an extension of:
85a3d920d30a ("clk: qcom: Add a dummy enable function for GX gdsc")
With the additional requirement that disable should really ensure that
the GDSC is turned off?
Regards,
Bjorn
>
> Akhil P Oommen (5):
> dt-bindings: clk: qcom: Support gpu cx gdsc reset
> clk: qcom: Allow custom reset ops
> clk: qcom: gpucc-sc7280: Add cx collapse reset support
> clk: qcom: gdsc: Add a reset op to poll gdsc collapse
> arm64: dts: qcom: sc7280: Add Reset support for gpu
>
> arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++
> drivers/clk/qcom/gdsc.c | 23 +++++++++++++++++++----
> drivers/clk/qcom/gdsc.h | 7 +++++++
> drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
> drivers/clk/qcom/reset.c | 6 ++++++
> drivers/clk/qcom/reset.h | 2 ++
> include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++
> 7 files changed, 46 insertions(+), 4 deletions(-)
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface
@ 2022-08-09 21:05 ` Bjorn Andersson
0 siblings, 0 replies; 36+ messages in thread
From: Bjorn Andersson @ 2022-08-09 21:05 UTC (permalink / raw)
To: Akhil P Oommen
Cc: freedreno, dri-devel, linux-arm-msm, Rob Clark, Stephen Boyd,
Douglas Anderson, Andy Gross, Konrad Dybcio, Krzysztof Kozlowski,
Michael Turquette, Philipp Zabel, Rob Herring, Stephen Boyd,
devicetree, linux-clk, linux-kernel
On Sat 30 Jul 04:17 CDT 2022, Akhil P Oommen wrote:
>
> Some clients like adreno gpu driver would like to ensure that its gdsc
> is collapsed at hardware during a gpu reset sequence. This is because it
> has a votable gdsc which could be ON due to a vote from another subsystem
> like tz, hyp etc or due to an internal hardware signal. To allow
> this, gpucc driver can expose an interface to the client driver using
> reset framework. Using this the client driver can trigger a polling within
> the gdsc driver.
>
> This series is rebased on top of linus's master branch.
>
> Related discussion: https://patchwork.freedesktop.org/patch/493144/
>
Forgive me if I'm assuming too much, but isn't this an extension of:
85a3d920d30a ("clk: qcom: Add a dummy enable function for GX gdsc")
With the additional requirement that disable should really ensure that
the GDSC is turned off?
Regards,
Bjorn
>
> Akhil P Oommen (5):
> dt-bindings: clk: qcom: Support gpu cx gdsc reset
> clk: qcom: Allow custom reset ops
> clk: qcom: gpucc-sc7280: Add cx collapse reset support
> clk: qcom: gdsc: Add a reset op to poll gdsc collapse
> arm64: dts: qcom: sc7280: Add Reset support for gpu
>
> arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++
> drivers/clk/qcom/gdsc.c | 23 +++++++++++++++++++----
> drivers/clk/qcom/gdsc.h | 7 +++++++
> drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
> drivers/clk/qcom/reset.c | 6 ++++++
> drivers/clk/qcom/reset.h | 2 ++
> include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++
> 7 files changed, 46 insertions(+), 4 deletions(-)
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface
2022-08-09 21:05 ` Bjorn Andersson
@ 2022-08-11 11:15 ` Akhil P Oommen
-1 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-08-11 11:15 UTC (permalink / raw)
To: Bjorn Andersson
Cc: freedreno, dri-devel, linux-arm-msm, Rob Clark, Stephen Boyd,
Douglas Anderson, Andy Gross, Konrad Dybcio, Krzysztof Kozlowski,
Michael Turquette, Philipp Zabel, Rob Herring, Stephen Boyd,
devicetree, linux-clk, linux-kernel
On 8/10/2022 2:35 AM, Bjorn Andersson wrote:
> On Sat 30 Jul 04:17 CDT 2022, Akhil P Oommen wrote:
>
>> Some clients like adreno gpu driver would like to ensure that its gdsc
>> is collapsed at hardware during a gpu reset sequence. This is because it
>> has a votable gdsc which could be ON due to a vote from another subsystem
>> like tz, hyp etc or due to an internal hardware signal. To allow
>> this, gpucc driver can expose an interface to the client driver using
>> reset framework. Using this the client driver can trigger a polling within
>> the gdsc driver.
>>
>> This series is rebased on top of linus's master branch.
>>
>> Related discussion: https://patchwork.freedesktop.org/patch/493144/
>>
> Forgive me if I'm assuming too much, but isn't this an extension of:
>
> 85a3d920d30a ("clk: qcom: Add a dummy enable function for GX gdsc")
>
> With the additional requirement that disable should really ensure that
> the GDSC is turned off?
Also, gpu driver needs a way to ensure cx gdsc was collapsed at least
once before it goes ahead with re-init.
Btw, the patch you mentioned is about gx gdsc in gpucc which is supposed
to be owned by gmu (except when it is in bad shape). But the current
series is about cx gdsc which is shared with other subsystems/drivers.
-Akhil.
>
> Regards,
> Bjorn
>
>> Akhil P Oommen (5):
>> dt-bindings: clk: qcom: Support gpu cx gdsc reset
>> clk: qcom: Allow custom reset ops
>> clk: qcom: gpucc-sc7280: Add cx collapse reset support
>> clk: qcom: gdsc: Add a reset op to poll gdsc collapse
>> arm64: dts: qcom: sc7280: Add Reset support for gpu
>>
>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++
>> drivers/clk/qcom/gdsc.c | 23 +++++++++++++++++++----
>> drivers/clk/qcom/gdsc.h | 7 +++++++
>> drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
>> drivers/clk/qcom/reset.c | 6 ++++++
>> drivers/clk/qcom/reset.h | 2 ++
>> include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++
>> 7 files changed, 46 insertions(+), 4 deletions(-)
>>
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface
@ 2022-08-11 11:15 ` Akhil P Oommen
0 siblings, 0 replies; 36+ messages in thread
From: Akhil P Oommen @ 2022-08-11 11:15 UTC (permalink / raw)
To: Bjorn Andersson
Cc: devicetree, Stephen Boyd, linux-arm-msm, Michael Turquette,
Konrad Dybcio, Douglas Anderson, dri-devel, Stephen Boyd,
Rob Herring, Andy Gross, Krzysztof Kozlowski, freedreno,
linux-clk, linux-kernel
On 8/10/2022 2:35 AM, Bjorn Andersson wrote:
> On Sat 30 Jul 04:17 CDT 2022, Akhil P Oommen wrote:
>
>> Some clients like adreno gpu driver would like to ensure that its gdsc
>> is collapsed at hardware during a gpu reset sequence. This is because it
>> has a votable gdsc which could be ON due to a vote from another subsystem
>> like tz, hyp etc or due to an internal hardware signal. To allow
>> this, gpucc driver can expose an interface to the client driver using
>> reset framework. Using this the client driver can trigger a polling within
>> the gdsc driver.
>>
>> This series is rebased on top of linus's master branch.
>>
>> Related discussion: https://patchwork.freedesktop.org/patch/493144/
>>
> Forgive me if I'm assuming too much, but isn't this an extension of:
>
> 85a3d920d30a ("clk: qcom: Add a dummy enable function for GX gdsc")
>
> With the additional requirement that disable should really ensure that
> the GDSC is turned off?
Also, gpu driver needs a way to ensure cx gdsc was collapsed at least
once before it goes ahead with re-init.
Btw, the patch you mentioned is about gx gdsc in gpucc which is supposed
to be owned by gmu (except when it is in bad shape). But the current
series is about cx gdsc which is shared with other subsystems/drivers.
-Akhil.
>
> Regards,
> Bjorn
>
>> Akhil P Oommen (5):
>> dt-bindings: clk: qcom: Support gpu cx gdsc reset
>> clk: qcom: Allow custom reset ops
>> clk: qcom: gpucc-sc7280: Add cx collapse reset support
>> clk: qcom: gdsc: Add a reset op to poll gdsc collapse
>> arm64: dts: qcom: sc7280: Add Reset support for gpu
>>
>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++
>> drivers/clk/qcom/gdsc.c | 23 +++++++++++++++++++----
>> drivers/clk/qcom/gdsc.h | 7 +++++++
>> drivers/clk/qcom/gpucc-sc7280.c | 6 ++++++
>> drivers/clk/qcom/reset.c | 6 ++++++
>> drivers/clk/qcom/reset.h | 2 ++
>> include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++
>> 7 files changed, 46 insertions(+), 4 deletions(-)
>>
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 36+ messages in thread