SM8150 and SM8250 videocc drivers

These two bindings are almost identical, so combine them into one. This will make it easier to add the sm8150 and sm8250 videocc bindings.

Signed-off-by Jonathan Marek <[email protected]>

---
 .../bindings/clock/qcom,sc7180-videocc.yaml   | 65 -------------------
 ...,sdm845-videocc.yaml => qcom,videocc.yaml} | 14 ++--
 2 files changed, 9 insertions(+), 70 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/qcom,sc7180-videocc.yaml
 rename Documentation/devicetree/bindings/clock/{qcom,sdm845-videocc.yaml => qcom,videocc.yaml} (76%)

Documentation/devicetree/bindings/clock/qcom,videocc.yaml renamed from Documentation/devicetree/bindings/clock/qcom,sdm845-videocc.yaml +2 -2

similarity index 76%
rename from Documentation/devicetree/bindings/clock/qcom,sdm845-videocc.yaml
rename to Documentation/devicetree/bindings/clock/qcom,videocc.yaml
index f7a0cf53d5f0..17666425476f 100644
@@ -1,23 +1,27 @@
 # SPDX-License-Identifier: GPL-2.0-only
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/clock/qcom,sdm845-videocc.yaml#
+$id: http://devicetree.org/schemas/clock/qcom,videocc.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Qualcomm Video Clock & Reset Controller Binding for SDM845
+title: Qualcomm Video Clock & Reset Controller Binding
 
 maintainers:
   - Taniya Das <[email protected]>

Documentation/devicetree/bindings/clock/qcom,videocc.yaml renamed from Documentation/devicetree/bindings/clock/qcom,sdm845-videocc.yaml +0 -0

   clocks:
     items:

Signature · 1 line

2.26.1

Add device tree bindings for video clock controller for SM8150 SoCs.

Signed-off-by Jonathan Marek <[email protected]>

---
 .../bindings/clock/qcom,videocc.yaml          |  4 ++-
 .../dt-bindings/clock/qcom,videocc-sm8150.h   | 25 +++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/clock/qcom,videocc-sm8150.h

Add device tree bindings for video clock controller for SM8150 SoCs.

Signed-off-by Jonathan Marek <[email protected]>

---

Add device tree bindings for video clock controller for SM8250 SoCs.

Signed-off-by Jonathan Marek <[email protected]>

---
 .../bindings/clock/qcom,videocc.yaml          |  8 +++-
 .../dt-bindings/clock/qcom,videocc-sm8250.h   | 42 +++++++++++++++++++
 2 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/clock/qcom,videocc-sm8250.h

Documentation/devicetree/bindings/clock/qcom,videocc.yaml +0 -0

index d04f5bd28dde..66a6066ae353 100644

Add support for the video clock controller found on SM8250 based devices.

Derived from the downstream driver.

Signed-off-by Jonathan Marek <[email protected]>

---
 drivers/clk/qcom/Kconfig          |   9 +
 drivers/clk/qcom/Makefile         |   1 +
 drivers/clk/qcom/videocc-sm8250.c | 518 ++++++++++++++++++++++++++++++
 3 files changed, 528 insertions(+)
 create mode 100644 drivers/clk/qcom/videocc-sm8250.c

drivers/clk/qcom/Kconfig +9 -0

index 40d7ee9886c9..95efa38211d5 100644
@@ -453,6 +453,15 @@ config SM_VIDEOCC_8150
          Say Y if you want to support video devices and functionality such as
          video encode and decode.
 
+config SM_VIDEOCC_8250
+       tristate "SM8250 Video Clock Controller"
+       select SDM_GCC_8250
+       select QCOM_GDSC
+       help
+         Support for the video clock controller on SM8250 devices.
+         Say Y if you want to support video devices and functionality such as
+         video encode and decode.
+
 config SPMI_PMIC_CLKDIV
        tristate "SPMI PMIC clkdiv Support"
        depends on SPMI || COMPILE_TEST

drivers/clk/qcom/Makefile +1 -0

index 6f4c580d2728..55fb20800b66 100644
@@ -69,6 +69,7 @@ obj-$(CONFIG_SM_GCC_8250) += gcc-sm8250.o
 obj-$(CONFIG_SM_GPUCC_8150) += gpucc-sm8150.o
 obj-$(CONFIG_SM_GPUCC_8250) += gpucc-sm8250.o
 obj-$(CONFIG_SM_VIDEOCC_8150) += videocc-sm8150.o
+obj-$(CONFIG_SM_VIDEOCC_8250) += videocc-sm8250.o
 obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
 obj-$(CONFIG_KPSS_XCC) += kpss-xcc.o
 obj-$(CONFIG_QCOM_HFPLL) += hfpll.o

drivers/clk/qcom/videocc-sm8250.c new file +12 -0

new file mode 100644
index 000000000000..a814d10945c4
@@ -0,0 +1,518 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/clock/qcom,videocc-sm8250.h>
+

drivers/clk/qcom/videocc-sm8250.c new file +16 -0

+static struct clk_rcg2 video_cc_ahb_clk_src = {
+       .cmd_rcgr = 0xbd4,
+       .mnd_width = 0,
+       .hid_width = 5,
+       .parent_map = video_cc_parent_map_0,
+       .freq_tbl = ftbl_video_cc_ahb_clk_src,
+       .clkr.hw.init = &(struct clk_init_data){
+               .name = "video_cc_ahb_clk_src",
+               .parent_data = video_cc_parent_data_0,
+               .num_parents = ARRAY_SIZE(video_cc_parent_data_0),
+               .flags = CLK_SET_RATE_PARENT,
+               .ops = &clk_rcg2_shared_ops,
+       },
+};
+
+static struct clk_rcg2 video_cc_xo_clk_src = {

drivers/clk/qcom/videocc-sm8250.c new file +7 -0

+               .ops = &clk_rcg2_ops,
+       },
+};
+
+static struct clk_branch video_cc_ahb_clk = {
+       .halt_reg = 0xe58,
+       .halt_check = BRANCH_HALT,

drivers/clk/qcom/videocc-sm8250.c new file +15 -0

+       .clkr = {
+               .enable_reg = 0xd34,
+               .enable_mask = BIT(0),
+               .hw.init = &(struct clk_init_data){
+                       .name = "video_cc_mvs0_clk",
+                       .parent_data = &(const struct clk_parent_data){
+                               .hw = &video_cc_mvs0_div_clk_src.clkr.hw,
+                       },
+                       .num_parents = 1,
+                       .flags = CLK_SET_RATE_PARENT,
+                       .ops = &clk_branch2_ops,
+               },
+       },
+};
+

drivers/clk/qcom/videocc-sm8250.c new file +4 -0

+
+static struct clk_branch video_cc_xo_clk = {
+       .halt_reg = 0xeec,
+       .halt_check = BRANCH_HALT,

drivers/clk/qcom/videocc-sm8250.c new file +5 -0

+                       .ops = &clk_branch2_ops,
+               },
+       },
+};
+

Quoting Jonathan Marek (2020-09-03 20:09:54)

Quoted 62 lines

Add support for the video clock controller found on SM8250 based devices.

Derived from the downstream driver.

Signed-off-by Jonathan Marek <[email protected]>

---
  drivers/clk/qcom/Kconfig          |   9 +
  drivers/clk/qcom/Makefile         |   1 +
  drivers/clk/qcom/videocc-sm8250.c | 518 ++++++++++++++++++++++++++++++
  3 files changed, 528 insertions(+)

create mode 100644 drivers/clk/qcom/videocc-sm8250.c

drivers/clk/qcom/Kconfig +9 -0

index 40d7ee9886c9..95efa38211d5 100644
@@ -453,6 +453,15 @@ config SM_VIDEOCC_8150
           Say Y if you want to support video devices and functionality such as
           video encode and decode.
 
+config SM_VIDEOCC_8250
+       tristate "SM8250 Video Clock Controller"
+       select SDM_GCC_8250
+       select QCOM_GDSC
+       help
+         Support for the video clock controller on SM8250 devices.
+         Say Y if you want to support video devices and functionality such as
+         video encode and decode.
+
  config SPMI_PMIC_CLKDIV
         tristate "SPMI PMIC clkdiv Support"
         depends on SPMI || COMPILE_TEST

drivers/clk/qcom/Makefile +1 -0

index 6f4c580d2728..55fb20800b66 100644
@@ -69,6 +69,7 @@ obj-$(CONFIG_SM_GCC_8250) += gcc-sm8250.o
  obj-$(CONFIG_SM_GPUCC_8150) += gpucc-sm8150.o
  obj-$(CONFIG_SM_GPUCC_8250) += gpucc-sm8250.o
  obj-$(CONFIG_SM_VIDEOCC_8150) += videocc-sm8150.o
+obj-$(CONFIG_SM_VIDEOCC_8250) += videocc-sm8250.o
  obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
  obj-$(CONFIG_KPSS_XCC) += kpss-xcc.o
  obj-$(CONFIG_QCOM_HFPLL) += hfpll.o

drivers/clk/qcom/videocc-sm8250.c new file +12 -0

new file mode 100644
index 000000000000..a814d10945c4
@@ -0,0 +1,518 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/clock/qcom,videocc-sm8250.h>
+

[...]

Quoted 16 lines

drivers/clk/qcom/videocc-sm8250.c new file +16 -0

+static struct clk_rcg2 video_cc_ahb_clk_src = {
+       .cmd_rcgr = 0xbd4,
+       .mnd_width = 0,
+       .hid_width = 5,
+       .parent_map = video_cc_parent_map_0,
+       .freq_tbl = ftbl_video_cc_ahb_clk_src,
+       .clkr.hw.init = &(struct clk_init_data){
+               .name = "video_cc_ahb_clk_src",
+               .parent_data = video_cc_parent_data_0,
+               .num_parents = ARRAY_SIZE(video_cc_parent_data_0),
+               .flags = CLK_SET_RATE_PARENT,
+               .ops = &clk_rcg2_shared_ops,
+       },
+};
+
+static struct clk_rcg2 video_cc_xo_clk_src = {

drivers/clk/qcom/videocc-sm8250.c line 255

Quoted 10 lines

drivers/clk/qcom/videocc-sm8250.c new file +10 -0

+       .cmd_rcgr = 0xecc,
+       .mnd_width = 0,
+       .hid_width = 5,
+       .parent_map = video_cc_parent_map_0,
+       .freq_tbl = ftbl_video_cc_ahb_clk_src,
+       .clkr.hw.init = &(struct clk_init_data){
+               .name = "video_cc_xo_clk_src",
+               .parent_data = video_cc_parent_data_0,
+               .num_parents = ARRAY_SIZE(video_cc_parent_data_0),
+               .flags = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT,

Similar critical clk comment, see below.

Quoted 7 lines

drivers/clk/qcom/videocc-sm8250.c new file +7 -0

+               .ops = &clk_rcg2_ops,
+       },
+};
+
+static struct clk_branch video_cc_ahb_clk = {
+       .halt_reg = 0xe58,
+       .halt_check = BRANCH_HALT,

drivers/clk/qcom/videocc-sm8250.c line 272

Quoted 10 lines

drivers/clk/qcom/videocc-sm8250.c new file +10 -0

+       .clkr = {
+               .enable_reg = 0xe58,
+               .enable_mask = BIT(0),
+               .hw.init = &(struct clk_init_data){
+                       .name = "video_cc_ahb_clk",
+                       .parent_data = &(const struct clk_parent_data){
+                               .hw = &video_cc_ahb_clk_src.clkr.hw,
+                       },
+                       .num_parents = 1,
+                       .flags = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT,

Similar critical clk comment, see below.

drivers/clk/qcom/videocc-sm8250.c line 280

Quoted 8 lines

drivers/clk/qcom/videocc-sm8250.c new file +8 -0

+                       .ops = &clk_branch2_ops,
+               },
+       },
+};
+
+static struct clk_branch video_cc_mvs0_clk = {
+       .halt_reg = 0xd34,
+       .halt_check = BRANCH_HALT_SKIP, /* TODO: hw gated ? */

Is this resolved?

Quoted 15 lines

drivers/clk/qcom/videocc-sm8250.c new file +15 -0

+       .clkr = {
+               .enable_reg = 0xd34,
+               .enable_mask = BIT(0),
+               .hw.init = &(struct clk_init_data){
+                       .name = "video_cc_mvs0_clk",
+                       .parent_data = &(const struct clk_parent_data){
+                               .hw = &video_cc_mvs0_div_clk_src.clkr.hw,
+                       },
+                       .num_parents = 1,
+                       .flags = CLK_SET_RATE_PARENT,
+                       .ops = &clk_branch2_ops,
+               },
+       },
+};
+

[...]

Quoted 4 lines

drivers/clk/qcom/videocc-sm8250.c new file +4 -0

+
+static struct clk_branch video_cc_xo_clk = {
+       .halt_reg = 0xeec,
+       .halt_check = BRANCH_HALT,

drivers/clk/qcom/videocc-sm8250.c line 380

Quoted 10 lines

drivers/clk/qcom/videocc-sm8250.c new file +10 -0

+       .clkr = {
+               .enable_reg = 0xeec,
+               .enable_mask = BIT(0),
+               .hw.init = &(struct clk_init_data){
+                       .name = "video_cc_xo_clk",
+                       .parent_data = &(const struct clk_parent_data){
+                               .hw = &video_cc_xo_clk_src.clkr.hw,
+                       },
+                       .num_parents = 1,
+                       .flags = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT,

Please add a coment why it's critical. If no consumer of this clk exists it's preferred to move the enabling to probe and not waste memory modeling it in software.

Quoted 5 lines

drivers/clk/qcom/videocc-sm8250.c new file +5 -0

+                       .ops = &clk_branch2_ops,
+               },
+       },
+};
+

Quoted 13 lines

Quoting Jonathan Marek (2020-09-03 20:09:54)

drivers/clk/qcom/videocc-sm8250.c line 280

Quoted 8 lines

drivers/clk/qcom/videocc-sm8250.c new file +8 -0

+                       .ops = &clk_branch2_ops,
+               },
+       },
+};
+
+static struct clk_branch video_cc_mvs0_clk = {
+       .halt_reg = 0xd34,
+       .halt_check = BRANCH_HALT_SKIP, /* TODO: hw gated ? */

Is this resolved?

Downstream has this clock as BRANCH_HALT_VOTED, but with the upstream venus driver (with patches to enable sm8250), that results in a "video_cc_mvs0_clk status stuck at 'off" error. AFAIK venus enables/disables this clock on its own (venus still works without touching this clock), but I didn't want to remove this in case it might be needed. I removed these clocks in the v3 I just sent.

Quoting Jonathan Marek (2020-09-23 09:07:16)

Quoted 15 lines · Stephen Boyd

Quoted 13 lines

Quoting Jonathan Marek (2020-09-03 20:09:54)

drivers/clk/qcom/videocc-sm8250.c line 280

Quoted 8 lines

drivers/clk/qcom/videocc-sm8250.c new file +8 -0

+                       .ops = &clk_branch2_ops,
+               },
+       },
+};
+
+static struct clk_branch video_cc_mvs0_clk = {
+       .halt_reg = 0xd34,
+       .halt_check = BRANCH_HALT_SKIP, /* TODO: hw gated ? */

Is this resolved?

Quoted 7 lines

Downstream has this clock as BRANCH_HALT_VOTED, but with the upstream venus driver (with patches to enable sm8250), that results in a "video_cc_mvs0_clk status stuck at 'off" error. AFAIK venus enables/disables this clock on its own (venus still works without touching this clock), but I didn't want to remove this in case it might be needed. I removed these clocks in the v3 I just sent.

Hmm. Does downstream use these clks? There have been some clk stuck problems with venus recently that were attributed to improperly enabling clks before enabling interconnects and power domains. Maybe it's the same problem.

Quoted 24 lines

Quoting Jonathan Marek (2020-09-23 09:07:16)

Quoted 15 lines · Stephen Boyd

Quoted 13 lines

Quoting Jonathan Marek (2020-09-03 20:09:54)

drivers/clk/qcom/videocc-sm8250.c line 280

Quoted 8 lines

drivers/clk/qcom/videocc-sm8250.c new file +8 -0

+                       .ops = &clk_branch2_ops,
+               },
+       },
+};
+
+static struct clk_branch video_cc_mvs0_clk = {
+       .halt_reg = 0xd34,
+       .halt_check = BRANCH_HALT_SKIP, /* TODO: hw gated ? */

Is this resolved?

Quoted 7 lines

Downstream has this clock as BRANCH_HALT_VOTED, but with the upstream venus driver (with patches to enable sm8250), that results in a "video_cc_mvs0_clk status stuck at 'off" error. AFAIK venus enables/disables this clock on its own (venus still works without touching this clock), but I didn't want to remove this in case it might be needed. I removed these clocks in the v3 I just sent.

Quoted 5 lines

Hmm. Does downstream use these clks? There have been some clk stuck problems with venus recently that were attributed to improperly enabling clks before enabling interconnects and power domains. Maybe it's the same problem.

Yes, downstream uses these clks.

The "stuck" problem still happens if GSDCS/interconnects are always on, and like I mentioned, venus works even with these clocks completely removed.

I think venus controls these clocks (and downstream just happens to try enabling it at a point where venus has already enabled it?). I'm not too sure about this, it might have something to do with the GDSC having the HW_CTRL flag too..