Re: [PATCH v2 5/5] clk: qcom: add video clock controller driver for SM8250

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/22/20 2:46 PM, Stephen Boyd wrote:
Quoting Jonathan Marek (2020-09-03 20:09:54)
Add support for the video clock controller found on SM8250 based devices.

Derived from the downstream driver.

Signed-off-by: Jonathan Marek <jonathan@xxxxxxxx>
---
  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

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 40d7ee9886c9..95efa38211d5 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -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
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 6f4c580d2728..55fb20800b66 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -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
diff --git a/drivers/clk/qcom/videocc-sm8250.c b/drivers/clk/qcom/videocc-sm8250.c
new file mode 100644
index 000000000000..a814d10945c4
--- /dev/null
+++ b/drivers/clk/qcom/videocc-sm8250.c
@@ -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>
+
[...]
+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 = {
+       .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.

+               .ops = &clk_rcg2_ops,
+       },
+};
+
+static struct clk_branch video_cc_ahb_clk = {
+       .halt_reg = 0xe58,
+       .halt_check = BRANCH_HALT,
+       .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.

+                       .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.

+       .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,
+               },
+       },
+};
+
[...]
+
+static struct clk_branch video_cc_xo_clk = {
+       .halt_reg = 0xeec,
+       .halt_check = BRANCH_HALT,
+       .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.

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



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux