clk: meson: add the video decoder clocks

In preparation for the V4L2 M2M driver, add the clocks for VDEC_1 and VDEC_HEVC to gxbb.

Signed-off-by Maxime Jourdan <maxi.jourdan at wanadoo.fr <mailto:[email protected]>>

---

?drivers/clk/meson/gxbb.c? ? ? ? ? ? ? | 112 ++++++++++++++++++++++++++
?drivers/clk/meson/gxbb.h? ? ? ? ? ? ? |? ?4 +-
?include/dt-bindings/clock/gxbb-clkc.h |? ?4 +

?3 files changed, 119 insertions(+), 1 deletion(-)

drivers/clk/meson/gxbb.c +0 -0

index b1e4d9557610..f9d7ab9c924e 100644
@@ -1543,6 +1543,100 @@ static struct clk_regmap gxbb_vapb = {
 ?},
 ?};

?

drivers/clk/meson/gxbb.c +94 -0

+/* VDEC clocks */
+
+static const char * const gxbb_vdec_parent_names[] = {
+"fclk_div4", "fclk_div3", "fclk_div5", "fclk_div7"
+};
+
+static struct clk_regmap gxbb_vdec_1_sel = {
+.data = &(struct clk_regmap_mux_data){
+.offset = HHI_VDEC_CLK_CNTL,
+.mask = 0x3,
+.shift = 9,
+},
+.hw.init = &(struct clk_init_data){
+.name = "vdec_1_sel",
+.ops = &clk_regmap_mux_ops,
+.parent_names = gxbb_vdec_parent_names,
+.num_parents = ARRAY_SIZE(gxbb_vdec_parent_names),
+.flags = CLK_SET_RATE_NO_REPARENT,
+},
+};
+
+static struct clk_regmap gxbb_vdec_1_div = {
+.data = &(struct clk_regmap_div_data){
+.offset = HHI_VDEC_CLK_CNTL,
+.shift = 0,
+.width = 7,
+},
+.hw.init = &(struct clk_init_data){
+.name = "vdec_1_div",
+.ops = &clk_regmap_divider_ops,
+.parent_names = (const char *[]){ "vdec_1_sel" },
+.num_parents = 1,
+.flags = CLK_SET_RATE_PARENT,
+},
+};
+
+static struct clk_regmap gxbb_vdec_1 = {
+.data = &(struct clk_regmap_gate_data){
+.offset = HHI_VDEC_CLK_CNTL,
+.bit_idx = 8,
+},
+.hw.init = &(struct clk_init_data) {
+.name = "vdec_1",
+.ops = &clk_regmap_gate_ops,
+.parent_names = (const char *[]){ "vdec_1_div" },
+.num_parents = 1,
+.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+},
+};
+
+static struct clk_regmap gxbb_vdec_hevc_sel = {
+.data = &(struct clk_regmap_mux_data){
+.offset = HHI_VDEC2_CLK_CNTL,
+.mask = 0x3,
+.shift = 25,
+},
+.hw.init = &(struct clk_init_data){
+.name = "vdec_hevc_sel",
+.ops = &clk_regmap_mux_ops,
+.parent_names = gxbb_vdec_parent_names,
+.num_parents = ARRAY_SIZE(gxbb_vdec_parent_names),
+.flags = CLK_SET_RATE_NO_REPARENT,
+},
+};
+
+static struct clk_regmap gxbb_vdec_hevc_div = {
+.data = &(struct clk_regmap_div_data){
+.offset = HHI_VDEC2_CLK_CNTL,
+.shift = 16,
+.width = 7,
+},
+.hw.init = &(struct clk_init_data){
+.name = "vdec_hevc_div",
+.ops = &clk_regmap_divider_ops,
+.parent_names = (const char *[]){ "vdec_hevc_sel" },
+.num_parents = 1,
+.flags = CLK_SET_RATE_PARENT,
+},
+};
+
+static struct clk_regmap gxbb_vdec_hevc = {
+.data = &(struct clk_regmap_gate_data){
+.offset = HHI_VDEC2_CLK_CNTL,
+.bit_idx = 24,
+},
+.hw.init = &(struct clk_init_data) {
+.name = "vdec_hevc",
+.ops = &clk_regmap_gate_ops,
+.parent_names = (const char *[]){ "vdec_hevc_div" },
+.num_parents = 1,
+.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+},
+};
+

?/* Everything Else (EE) domain gates */
?static MESON_GATE(gxbb_ddr, HHI_GCLK_MPEG0, 0);
?static MESON_GATE(gxbb_dos, HHI_GCLK_MPEG0, 1);

drivers/clk/meson/gxbb.c +18 -0

@@ -1786,6 +1880,12 @@ static struct clk_hw_onecell_data gxbb_hw_onecell_data = {
 ?[CLKID_FCLK_DIV4_DIV]? ? = &gxbb_fclk_div4_div.hw,
 ?[CLKID_FCLK_DIV5_DIV]? ? = &gxbb_fclk_div5_div.hw,
 ?[CLKID_FCLK_DIV7_DIV]? ? = &gxbb_fclk_div7_div.hw,
+[CLKID_VDEC_1_SEL]? ? = &gxbb_vdec_1_sel.hw,
+[CLKID_VDEC_1_DIV]? ? = &gxbb_vdec_1_div.hw,
+[CLKID_VDEC_1]? ? = &gxbb_vdec_1.hw,
+[CLKID_VDEC_HEVC_SEL]? ? = &gxbb_vdec_hevc_sel.hw,
+[CLKID_VDEC_HEVC_DIV]? ? = &gxbb_vdec_hevc_div.hw,
+[CLKID_VDEC_HEVC]? ? = &gxbb_vdec_hevc.hw,
 ?[NR_CLKS]? ? = NULL,
 ?},
 ?.num = NR_CLKS,

@@ -1942,6 +2042,12 @@ static struct clk_hw_onecell_data gxl_hw_onecell_data = {
 ?[CLKID_FCLK_DIV4_DIV]? ? = &gxbb_fclk_div4_div.hw,
 ?[CLKID_FCLK_DIV5_DIV]? ? = &gxbb_fclk_div5_div.hw,
 ?[CLKID_FCLK_DIV7_DIV]? ? = &gxbb_fclk_div7_div.hw,
+[CLKID_VDEC_1_SEL]? ? = &gxbb_vdec_1_sel.hw,
+[CLKID_VDEC_1_DIV]? ? = &gxbb_vdec_1_div.hw,
+[CLKID_VDEC_1]? ? = &gxbb_vdec_1.hw,
+[CLKID_VDEC_HEVC_SEL]? ? = &gxbb_vdec_hevc_sel.hw,
+[CLKID_VDEC_HEVC_DIV]? ? = &gxbb_vdec_hevc_div.hw,
+[CLKID_VDEC_HEVC]? ? = &gxbb_vdec_hevc.hw,
 ?[NR_CLKS]? ? = NULL,
 ?},
 ?.num = NR_CLKS,

@@ -2100,6 +2206,12 @@ static struct clk_regmap *const gx_clk_regmaps[] = {
 ?&gxbb_fclk_div4,
 ?&gxbb_fclk_div5,
 ?&gxbb_fclk_div7,
+&gxbb_vdec_1_sel,
+&gxbb_vdec_1_div,
+&gxbb_vdec_1,
+&gxbb_vdec_hevc_sel,
+&gxbb_vdec_hevc_div,
+&gxbb_vdec_hevc,
 ?};

?
?struct clkc_data {

drivers/clk/meson/gxbb.h +0 -0

index 9febf3f03739..ae21d235355a 100644
@@ -204,8 +204,10 @@

?#define CLKID_FCLK_DIV4_DIV? 148
?#define CLKID_FCLK_DIV5_DIV? 149
?#define CLKID_FCLK_DIV7_DIV? 150

drivers/clk/meson/gxbb.h +2 -0

+#define CLKID_VDEC_1_DIV? 152
+#define CLKID_VDEC_HEVC_DIV? 155

?

drivers/clk/meson/gxbb.h +1 -1

-#define NR_CLKS? 151
+#define NR_CLKS? 157

?
?/* include the CLKIDs that have been made part of the DT binding */ ?#include <dt-bindings/clock/gxbb-clkc.h>

include/dt-bindings/clock/gxbb-clkc.h +0 -0

index 8ba99a5e3fd3..ae7f6be747e4 100644
@@ -125,5 +125,9 @@

?#define CLKID_VAPB_1138
?#define CLKID_VAPB_SEL139
?#define CLKID_VAPB140

include/dt-bindings/clock/gxbb-clkc.h +4 -0

+#define CLKID_VDEC_1_SEL151
+#define CLKID_VDEC_1153
+#define CLKID_VDEC_HEVC_SEL154
+#define CLKID_VDEC_HEVC156

?
?#endif /* __GXBB_CLKC_H */
--?
2.17.0

In preparation for the V4L2 M2M driver, add the clocks for VDEC_1 and VDEC_HEVC to gxbb.

Signed-off-by Maxime Jourdan <[email protected]>

---
 drivers/clk/meson/gxbb.c              | 112 ++++++++++++++++++++++++++
 drivers/clk/meson/gxbb.h              |   4 +-
 include/dt-bindings/clock/gxbb-clkc.h |   4 +
 3 files changed, 119 insertions(+), 1 deletion(-)

drivers/clk/meson/gxbb.c +8 -0

index b1e4d9557610..f9d7ab9c924e 100644
@@ -1543,6 +1543,100 @@ static struct clk_regmap gxbb_vapb = {
 	},
 };
 
+/* VDEC clocks */
+
+static const char * const gxbb_vdec_parent_names[] = {
+	"fclk_div4", "fclk_div3", "fclk_div5", "fclk_div7"
+};
+
+static struct clk_regmap gxbb_vdec_1_sel = {
+	.data = &(struct clk_regmap_mux_data){

drivers/clk/meson/gxbb.c +19 -0

+	},
+};
+
+static struct clk_regmap gxbb_vdec_1_div = {
+	.data = &(struct clk_regmap_div_data){
+		.offset = HHI_VDEC_CLK_CNTL,
+		.shift = 0,
+		.width = 7,
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "vdec_1_div",
+		.ops = &clk_regmap_divider_ops,
+		.parent_names = (const char *[]){ "vdec_1_sel" },
+		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_regmap gxbb_vdec_1 = {

drivers/clk/meson/gxbb.c +65 -0

+	},
+};
+
+static struct clk_regmap gxbb_vdec_hevc_sel = {
+	.data = &(struct clk_regmap_mux_data){
+		.offset = HHI_VDEC2_CLK_CNTL,
+		.mask = 0x3,
+		.shift = 25,
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "vdec_hevc_sel",
+		.ops = &clk_regmap_mux_ops,
+		.parent_names = gxbb_vdec_parent_names,
+		.num_parents = ARRAY_SIZE(gxbb_vdec_parent_names),
+		.flags = CLK_SET_RATE_NO_REPARENT,
+	},
+};
+
+static struct clk_regmap gxbb_vdec_hevc_div = {
+	.data = &(struct clk_regmap_div_data){
+		.offset = HHI_VDEC2_CLK_CNTL,
+		.shift = 16,
+		.width = 7,
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "vdec_hevc_div",
+		.ops = &clk_regmap_divider_ops,
+		.parent_names = (const char *[]){ "vdec_hevc_sel" },
+		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_regmap gxbb_vdec_hevc = {
+	.data = &(struct clk_regmap_gate_data){
+		.offset = HHI_VDEC2_CLK_CNTL,
+		.bit_idx = 24,
+	},
+	.hw.init = &(struct clk_init_data) {
+		.name = "vdec_hevc",
+		.ops = &clk_regmap_gate_ops,
+		.parent_names = (const char *[]){ "vdec_hevc_div" },
+		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+	},
+};
+
 /* Everything Else (EE) domain gates */
 static MESON_GATE(gxbb_ddr, HHI_GCLK_MPEG0, 0);
 static MESON_GATE(gxbb_dos, HHI_GCLK_MPEG0, 1);

@@ -1786,6 +1880,12 @@ static struct clk_hw_onecell_data gxbb_hw_onecell_data = {
 		[CLKID_FCLK_DIV4_DIV]	    = &gxbb_fclk_div4_div.hw,
 		[CLKID_FCLK_DIV5_DIV]	    = &gxbb_fclk_div5_div.hw,
 		[CLKID_FCLK_DIV7_DIV]	    = &gxbb_fclk_div7_div.hw,
+		[CLKID_VDEC_1_SEL]	    = &gxbb_vdec_1_sel.hw,
+		[CLKID_VDEC_1_DIV]	    = &gxbb_vdec_1_div.hw,
+		[CLKID_VDEC_1]		    = &gxbb_vdec_1.hw,
+		[CLKID_VDEC_HEVC_SEL]	    = &gxbb_vdec_hevc_sel.hw,
+		[CLKID_VDEC_HEVC_DIV]	    = &gxbb_vdec_hevc_div.hw,
+		[CLKID_VDEC_HEVC]	    = &gxbb_vdec_hevc.hw,
 		[NR_CLKS]		    = NULL,
 	},
 	.num = NR_CLKS,

@@ -1942,6 +2042,12 @@ static struct clk_hw_onecell_data gxl_hw_onecell_data = {
 		[CLKID_FCLK_DIV4_DIV]	    = &gxbb_fclk_div4_div.hw,
 		[CLKID_FCLK_DIV5_DIV]	    = &gxbb_fclk_div5_div.hw,
 		[CLKID_FCLK_DIV7_DIV]	    = &gxbb_fclk_div7_div.hw,
+		[CLKID_VDEC_1_SEL]	    = &gxbb_vdec_1_sel.hw,
+		[CLKID_VDEC_1_DIV]	    = &gxbb_vdec_1_div.hw,
+		[CLKID_VDEC_1]		    = &gxbb_vdec_1.hw,
+		[CLKID_VDEC_HEVC_SEL]	    = &gxbb_vdec_hevc_sel.hw,
+		[CLKID_VDEC_HEVC_DIV]	    = &gxbb_vdec_hevc_div.hw,
+		[CLKID_VDEC_HEVC]	    = &gxbb_vdec_hevc.hw,
 		[NR_CLKS]		    = NULL,
 	},
 	.num = NR_CLKS,

@@ -2100,6 +2206,12 @@ static struct clk_regmap *const gx_clk_regmaps[] = {
 	&gxbb_fclk_div4,
 	&gxbb_fclk_div5,
 	&gxbb_fclk_div7,
+	&gxbb_vdec_1_sel,
+	&gxbb_vdec_1_div,
+	&gxbb_vdec_1,
+	&gxbb_vdec_hevc_sel,
+	&gxbb_vdec_hevc_div,
+	&gxbb_vdec_hevc,
 };
 
 struct clkc_data {

drivers/clk/meson/gxbb.h +0 -0

index 9febf3f03739..ae21d235355a 100644

drivers/clk/meson/gxbb.h +0 -0

 /* include the CLKIDs that have been made part of the DT binding */
 #include <dt-bindings/clock/gxbb-clkc.h>

include/dt-bindings/clock/gxbb-clkc.h +4 -0

index 8ba99a5e3fd3..ae7f6be747e4 100644
@@ -125,5 +125,9 @@
 #define CLKID_VAPB_1		138
 #define CLKID_VAPB_SEL		139
 #define CLKID_VAPB		140
+#define CLKID_VDEC_1_SEL	151
+#define CLKID_VDEC_1		153
+#define CLKID_VDEC_HEVC_SEL	154
+#define CLKID_VDEC_HEVC	156
 
 #endif /* __GXBB_CLKC_H */

Signature · 1 line

2.17.0

Quoted 4 lines

In preparation for the V4L2 M2M driver, add the clocks for VDEC_1 and VDEC_HEVC to gxbb.

Signed-off-by Maxime Jourdan <[email protected]>

a few other comments, in addition to what neil pointed out.

Quoted 22 lines

---
 drivers/clk/meson/gxbb.c              | 112 ++++++++++++++++++++++++++
 drivers/clk/meson/gxbb.h              |   4 +-
 include/dt-bindings/clock/gxbb-clkc.h |   4 +
 3 files changed, 119 insertions(+), 1 deletion(-)

drivers/clk/meson/gxbb.c +8 -0

index b1e4d9557610..f9d7ab9c924e 100644
@@ -1543,6 +1543,100 @@ static struct clk_regmap gxbb_vapb = {
      },
 };
 
+/* VDEC clocks */
+
+static const char * const gxbb_vdec_parent_names[] = {
+     "fclk_div4", "fclk_div3", "fclk_div5", "fclk_div7"
+};
+
+static struct clk_regmap gxbb_vdec_1_sel = {
+     .data = &(struct clk_regmap_mux_data){

drivers/clk/meson/gxbb.c static struct clk_regmap gxbb_vapb = { · line 1563

Quoted 10 lines

drivers/clk/meson/gxbb.c +10 -0

+             .offset = HHI_VDEC_CLK_CNTL,
+             .mask = 0x3,
+             .shift = 9,
+     },
+     .hw.init = &(struct clk_init_data){
+             .name = "vdec_1_sel",
+             .ops = &clk_regmap_mux_ops,
+             .parent_names = gxbb_vdec_parent_names,
+             .num_parents = ARRAY_SIZE(gxbb_vdec_parent_names),
+             .flags = CLK_SET_RATE_NO_REPARENT,

Looks like you are/will be controlling the mux manually from your driver. Any particular reason for doing so ?

Looking at the possible parent, you may as well call clk_set_rate() on the leaf
clock with 500Mhz, 666Mhz, etc ... it would accomplish the same thing but :
* You would need to get only one clock in your driver
* You don't need to manage the topology of the clock tree in your driver, which
could be interresting if your driver ends up supporting more than GXBB.

Quoted 19 lines

drivers/clk/meson/gxbb.c +19 -0

+     },
+};
+
+static struct clk_regmap gxbb_vdec_1_div = {
+     .data = &(struct clk_regmap_div_data){
+             .offset = HHI_VDEC_CLK_CNTL,
+             .shift = 0,
+             .width = 7,
+     },
+     .hw.init = &(struct clk_init_data){
+             .name = "vdec_1_div",
+             .ops = &clk_regmap_divider_ops,
+             .parent_names = (const char *[]){ "vdec_1_sel" },
+             .num_parents = 1,
+             .flags = CLK_SET_RATE_PARENT,
+     },
+};
+
+static struct clk_regmap gxbb_vdec_1 = {

drivers/clk/meson/gxbb.c static struct clk_regmap gxbb_vapb = { · line 1592

Quoted 10 lines

drivers/clk/meson/gxbb.c +10 -0

+     .data = &(struct clk_regmap_gate_data){
+             .offset = HHI_VDEC_CLK_CNTL,
+             .bit_idx = 8,
+     },
+     .hw.init = &(struct clk_init_data) {
+             .name = "vdec_1",
+             .ops = &clk_regmap_gate_ops,
+             .parent_names = (const char *[]){ "vdec_1_div" },
+             .num_parents = 1,
+             .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,

Any particular reason for using CLK_IGNORE_UNUSED ?

You should only need CLK_IGNORE_UNUSED when the clock is left on by the bootloader, and you need it stay that way until the driver load.

It allows to skip the clk_disable_unused() mechanism of CCF but I don't think
the video decoder should require this, unless I missed something.

drivers/clk/meson/gxbb.c static struct clk_hw_onecell_data

Quoted 51 lines

drivers/clk/meson/gxbb.c +47 -0

+     },
+};
+
+static struct clk_regmap gxbb_vdec_hevc_sel = {
+     .data = &(struct clk_regmap_mux_data){
+             .offset = HHI_VDEC2_CLK_CNTL,
+             .mask = 0x3,
+             .shift = 25,
+     },
+     .hw.init = &(struct clk_init_data){
+             .name = "vdec_hevc_sel",
+             .ops = &clk_regmap_mux_ops,
+             .parent_names = gxbb_vdec_parent_names,
+             .num_parents = ARRAY_SIZE(gxbb_vdec_parent_names),
+             .flags = CLK_SET_RATE_NO_REPARENT,
+     },
+};
+
+static struct clk_regmap gxbb_vdec_hevc_div = {
+     .data = &(struct clk_regmap_div_data){
+             .offset = HHI_VDEC2_CLK_CNTL,
+             .shift = 16,
+             .width = 7,
+     },
+     .hw.init = &(struct clk_init_data){
+             .name = "vdec_hevc_div",
+             .ops = &clk_regmap_divider_ops,
+             .parent_names = (const char *[]){ "vdec_hevc_sel" },
+             .num_parents = 1,
+             .flags = CLK_SET_RATE_PARENT,
+     },
+};
+
+static struct clk_regmap gxbb_vdec_hevc = {
+     .data = &(struct clk_regmap_gate_data){
+             .offset = HHI_VDEC2_CLK_CNTL,
+             .bit_idx = 24,
+     },
+     .hw.init = &(struct clk_init_data) {
+             .name = "vdec_hevc",
+             .ops = &clk_regmap_gate_ops,
+             .parent_names = (const char *[]){ "vdec_hevc_div" },
+             .num_parents = 1,
+             .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+     },
+};
+
 /* Everything Else (EE) domain gates */
 static MESON_GATE(gxbb_ddr, HHI_GCLK_MPEG0, 0);
 static MESON_GATE(gxbb_dos, HHI_GCLK_MPEG0, 1);

@@ -1786,6 +1880,12 @@ static struct clk_hw_onecell_data

gxbb_hw_onecell_data = {

drivers/clk/meson/gxbb.c static struct clk_hw_onecell_data

Quoted 13 lines

drivers/clk/meson/gxbb.c +6 -0

              [CLKID_FCLK_DIV4_DIV]       = &gxbb_fclk_div4_div.hw,
              [CLKID_FCLK_DIV5_DIV]       = &gxbb_fclk_div5_div.hw,
              [CLKID_FCLK_DIV7_DIV]       = &gxbb_fclk_div7_div.hw,
+             [CLKID_VDEC_1_SEL]          = &gxbb_vdec_1_sel.hw,
+             [CLKID_VDEC_1_DIV]          = &gxbb_vdec_1_div.hw,
+             [CLKID_VDEC_1]              = &gxbb_vdec_1.hw,
+             [CLKID_VDEC_HEVC_SEL]       = &gxbb_vdec_hevc_sel.hw,
+             [CLKID_VDEC_HEVC_DIV]       = &gxbb_vdec_hevc_div.hw,
+             [CLKID_VDEC_HEVC]           = &gxbb_vdec_hevc.hw,
              [NR_CLKS]                   = NULL,
      },
      .num = NR_CLKS,

@@ -1942,6 +2042,12 @@ static struct clk_hw_onecell_data

gxl_hw_onecell_data = {

drivers/clk/meson/gxbb.c static struct clk_regmap *const gx_clk_regmaps[]

Quoted 13 lines

drivers/clk/meson/gxbb.c +6 -0

              [CLKID_FCLK_DIV4_DIV]       = &gxbb_fclk_div4_div.hw,
              [CLKID_FCLK_DIV5_DIV]       = &gxbb_fclk_div5_div.hw,
              [CLKID_FCLK_DIV7_DIV]       = &gxbb_fclk_div7_div.hw,
+             [CLKID_VDEC_1_SEL]          = &gxbb_vdec_1_sel.hw,
+             [CLKID_VDEC_1_DIV]          = &gxbb_vdec_1_div.hw,
+             [CLKID_VDEC_1]              = &gxbb_vdec_1.hw,
+             [CLKID_VDEC_HEVC_SEL]       = &gxbb_vdec_hevc_sel.hw,
+             [CLKID_VDEC_HEVC_DIV]       = &gxbb_vdec_hevc_div.hw,
+             [CLKID_VDEC_HEVC]           = &gxbb_vdec_hevc.hw,
              [NR_CLKS]                   = NULL,
      },
      .num = NR_CLKS,

@@ -2100,6 +2206,12 @@ static struct clk_regmap *const gx_clk_regmaps[]

= {

Quoted 17 lines

drivers/clk/meson/gxbb.c +6 -0

      &gxbb_fclk_div4,
      &gxbb_fclk_div5,
      &gxbb_fclk_div7,
+     &gxbb_vdec_1_sel,
+     &gxbb_vdec_1_div,
+     &gxbb_vdec_1,
+     &gxbb_vdec_hevc_sel,
+     &gxbb_vdec_hevc_div,
+     &gxbb_vdec_hevc,
 };
 
 struct clkc_data {

drivers/clk/meson/gxbb.h +0 -0

index 9febf3f03739..ae21d235355a 100644

drivers/clk/meson/gxbb.h line 211

Quoted 9 lines

drivers/clk/meson/gxbb.h +3 -1

@@ -204,8 +204,10 @@
 #define CLKID_FCLK_DIV4_DIV    148
 #define CLKID_FCLK_DIV5_DIV    149
 #define CLKID_FCLK_DIV7_DIV    150
+#define CLKID_VDEC_1_DIV       152
+#define CLKID_VDEC_HEVC_DIV    155
 
-#define NR_CLKS                        151
+#define NR_CLKS                        157
 

We prefer to get the DT binding part in a separate patch, it makes the platform
maintainer's life easier.

Review

Quoted 3 lines

drivers/clk/meson/gxbb.h +0 -0

 /* include the CLKIDs that have been made part of the DT binding */
 #include <dt-bindings/clock/gxbb-clkc.h>

(quoted diff) +0 -0

b/include/dt-bindings/clock/gxbb-clkc.h

Quoted 14 lines

index 8ba99a5e3fd3..ae7f6be747e4 100644

include/dt-bindings/clock/gxbb-clkc.h +4 -1

@@ -125,5 +125,9 @@
 #define CLKID_VAPB_1         138
 #define CLKID_VAPB_SEL               139
 #define CLKID_VAPB           140
+#define CLKID_VDEC_1_SEL     151
+#define CLKID_VDEC_1         153
+#define CLKID_VDEC_HEVC_SEL  154
+#define CLKID_VDEC_HEVC      156
 
 #endif /* __GXBB_CLKC_H */
--

Quoted 3 lines

2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi Neil, J?rome,

Thanks for the answers, I'll resubmit with proper formatting.

@J?rome: very valid points. The usage of the clk will indeed be through set_clk_rate on the leaf clock within the driver, so I don't need to expose the mux. If I want the mux automatically selected to provide the nearest target clock, should I keep CLK_SET_RATE_NO_REPARENT ?

CLK_IGNORE_UNUSED doesn't seem like a required property indeed for those clocks.

Maxime

2018-04-21 22:19 GMT+02:00 Jerome Brunet <[email protected]>:

Quoted 22 lines · Maxime Jourdan

Quoted 4 lines

In preparation for the V4L2 M2M driver, add the clocks for VDEC_1 and VDEC_HEVC to gxbb.

Signed-off-by Maxime Jourdan <[email protected]>

a few other comments, in addition to what neil pointed out.

Quoted 14 lines

---
 drivers/clk/meson/gxbb.c              | 112 ++++++++++++++++++++++++++
 drivers/clk/meson/gxbb.h              |   4 +-
 include/dt-bindings/clock/gxbb-clkc.h |   4 +
 3 files changed, 119 insertions(+), 1 deletion(-)

drivers/clk/meson/gxbb.c +0 -0

index b1e4d9557610..f9d7ab9c924e 100644
@@ -1543,6 +1543,100 @@ static struct clk_regmap gxbb_vapb = {
      },
 };
 

Quoted 11 lines

drivers/clk/meson/gxbb.c static struct clk_regmap gxbb_vapb = { · line 1556

Quoted 11 lines

drivers/clk/meson/gxbb.c +11 -0

+/* VDEC clocks */
+
+static const char * const gxbb_vdec_parent_names[] = {
+     "fclk_div4", "fclk_div3", "fclk_div5", "fclk_div7"
+};
+
+static struct clk_regmap gxbb_vdec_1_sel = {
+     .data = &(struct clk_regmap_mux_data){
+             .offset = HHI_VDEC_CLK_CNTL,
+             .mask = 0x3,
+             .shift = 9,

Quoted 7 lines

drivers/clk/meson/gxbb.c static struct clk_regmap gxbb_vapb = { · line 1563

Quoted 7 lines

drivers/clk/meson/gxbb.c +7 -0

+     },
+     .hw.init = &(struct clk_init_data){
+             .name = "vdec_1_sel",
+             .ops = &clk_regmap_mux_ops,
+             .parent_names = gxbb_vdec_parent_names,
+             .num_parents = ARRAY_SIZE(gxbb_vdec_parent_names),
+             .flags = CLK_SET_RATE_NO_REPARENT,

Quoted 180 lines

Looks like you are/will be controlling the mux manually from your driver. Any particular reason for doing so ?

Looking at the possible parent, you may as well call clk_set_rate() on the leaf clock with 500Mhz, 666Mhz, etc ... it would accomplish the same thing but :
* You would need to get only one clock in your driver
* You don't need to manage the topology of the clock tree in your driver, which could be interresting if your driver ends up supporting more than GXBB.

Quoted 19 lines

drivers/clk/meson/gxbb.c +19 -0

+     },
+};
+
+static struct clk_regmap gxbb_vdec_1_div = {
+     .data = &(struct clk_regmap_div_data){
+             .offset = HHI_VDEC_CLK_CNTL,
+             .shift = 0,
+             .width = 7,
+     },
+     .hw.init = &(struct clk_init_data){
+             .name = "vdec_1_div",
+             .ops = &clk_regmap_divider_ops,
+             .parent_names = (const char *[]){ "vdec_1_sel" },
+             .num_parents = 1,
+             .flags = CLK_SET_RATE_PARENT,
+     },
+};
+
+static struct clk_regmap gxbb_vdec_1 = {

drivers/clk/meson/gxbb.c static struct clk_regmap gxbb_vapb = { · line 1592

Quoted 10 lines

drivers/clk/meson/gxbb.c +10 -0

+     .data = &(struct clk_regmap_gate_data){
+             .offset = HHI_VDEC_CLK_CNTL,
+             .bit_idx = 8,
+     },
+     .hw.init = &(struct clk_init_data) {
+             .name = "vdec_1",
+             .ops = &clk_regmap_gate_ops,
+             .parent_names = (const char *[]){ "vdec_1_div" },
+             .num_parents = 1,
+             .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,

Any particular reason for using CLK_IGNORE_UNUSED ?

You should only need CLK_IGNORE_UNUSED when the clock is left on by the bootloader, and you need it stay that way until the driver load.

It allows to skip the clk_disable_unused() mechanism of CCF but I don't think the video decoder should require this, unless I missed something.

Quoted 94 lines

drivers/clk/meson/gxbb.c +65 -0

+     },
+};
+
+static struct clk_regmap gxbb_vdec_hevc_sel = {
+     .data = &(struct clk_regmap_mux_data){
+             .offset = HHI_VDEC2_CLK_CNTL,
+             .mask = 0x3,
+             .shift = 25,
+     },
+     .hw.init = &(struct clk_init_data){
+             .name = "vdec_hevc_sel",
+             .ops = &clk_regmap_mux_ops,
+             .parent_names = gxbb_vdec_parent_names,
+             .num_parents = ARRAY_SIZE(gxbb_vdec_parent_names),
+             .flags = CLK_SET_RATE_NO_REPARENT,
+     },
+};
+
+static struct clk_regmap gxbb_vdec_hevc_div = {
+     .data = &(struct clk_regmap_div_data){
+             .offset = HHI_VDEC2_CLK_CNTL,
+             .shift = 16,
+             .width = 7,
+     },
+     .hw.init = &(struct clk_init_data){
+             .name = "vdec_hevc_div",
+             .ops = &clk_regmap_divider_ops,
+             .parent_names = (const char *[]){ "vdec_hevc_sel" },
+             .num_parents = 1,
+             .flags = CLK_SET_RATE_PARENT,
+     },
+};
+
+static struct clk_regmap gxbb_vdec_hevc = {
+     .data = &(struct clk_regmap_gate_data){
+             .offset = HHI_VDEC2_CLK_CNTL,
+             .bit_idx = 24,
+     },
+     .hw.init = &(struct clk_init_data) {
+             .name = "vdec_hevc",
+             .ops = &clk_regmap_gate_ops,
+             .parent_names = (const char *[]){ "vdec_hevc_div" },
+             .num_parents = 1,
+             .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+     },
+};
+
 /* Everything Else (EE) domain gates */
 static MESON_GATE(gxbb_ddr, HHI_GCLK_MPEG0, 0);
 static MESON_GATE(gxbb_dos, HHI_GCLK_MPEG0, 1);

@@ -1786,6 +1880,12 @@ static struct clk_hw_onecell_data gxbb_hw_onecell_data = {
              [CLKID_FCLK_DIV4_DIV]       = &gxbb_fclk_div4_div.hw,
              [CLKID_FCLK_DIV5_DIV]       = &gxbb_fclk_div5_div.hw,
              [CLKID_FCLK_DIV7_DIV]       = &gxbb_fclk_div7_div.hw,
+             [CLKID_VDEC_1_SEL]          = &gxbb_vdec_1_sel.hw,
+             [CLKID_VDEC_1_DIV]          = &gxbb_vdec_1_div.hw,
+             [CLKID_VDEC_1]              = &gxbb_vdec_1.hw,
+             [CLKID_VDEC_HEVC_SEL]       = &gxbb_vdec_hevc_sel.hw,
+             [CLKID_VDEC_HEVC_DIV]       = &gxbb_vdec_hevc_div.hw,
+             [CLKID_VDEC_HEVC]           = &gxbb_vdec_hevc.hw,
              [NR_CLKS]                   = NULL,
      },
      .num = NR_CLKS,

@@ -1942,6 +2042,12 @@ static struct clk_hw_onecell_data gxl_hw_onecell_data = {
              [CLKID_FCLK_DIV4_DIV]       = &gxbb_fclk_div4_div.hw,
              [CLKID_FCLK_DIV5_DIV]       = &gxbb_fclk_div5_div.hw,
              [CLKID_FCLK_DIV7_DIV]       = &gxbb_fclk_div7_div.hw,
+             [CLKID_VDEC_1_SEL]          = &gxbb_vdec_1_sel.hw,
+             [CLKID_VDEC_1_DIV]          = &gxbb_vdec_1_div.hw,
+             [CLKID_VDEC_1]              = &gxbb_vdec_1.hw,
+             [CLKID_VDEC_HEVC_SEL]       = &gxbb_vdec_hevc_sel.hw,
+             [CLKID_VDEC_HEVC_DIV]       = &gxbb_vdec_hevc_div.hw,
+             [CLKID_VDEC_HEVC]           = &gxbb_vdec_hevc.hw,
              [NR_CLKS]                   = NULL,
      },
      .num = NR_CLKS,

@@ -2100,6 +2206,12 @@ static struct clk_regmap *const gx_clk_regmaps[] = {
      &gxbb_fclk_div4,
      &gxbb_fclk_div5,
      &gxbb_fclk_div7,
+     &gxbb_vdec_1_sel,
+     &gxbb_vdec_1_div,
+     &gxbb_vdec_1,
+     &gxbb_vdec_hevc_sel,
+     &gxbb_vdec_hevc_div,
+     &gxbb_vdec_hevc,
 };
 
 struct clkc_data {

drivers/clk/meson/gxbb.h +0 -0

index 9febf3f03739..ae21d235355a 100644

drivers/clk/meson/gxbb.h line 211

Quoted 9 lines

drivers/clk/meson/gxbb.h +3 -1

@@ -204,8 +204,10 @@
 #define CLKID_FCLK_DIV4_DIV    148
 #define CLKID_FCLK_DIV5_DIV    149
 #define CLKID_FCLK_DIV7_DIV    150
+#define CLKID_VDEC_1_DIV       152
+#define CLKID_VDEC_HEVC_DIV    155
 
-#define NR_CLKS                        151
+#define NR_CLKS                        157
 

We prefer to get the DT binding part in a separate patch, it makes the platform maintainer's life easier.

Quoted 20 lines

drivers/clk/meson/gxbb.h +0 -0

 /* include the CLKIDs that have been made part of the DT binding */
 #include <dt-bindings/clock/gxbb-clkc.h>

include/dt-bindings/clock/gxbb-clkc.h +4 -0

index 8ba99a5e3fd3..ae7f6be747e4 100644
@@ -125,5 +125,9 @@
 #define CLKID_VAPB_1         138
 #define CLKID_VAPB_SEL               139
 #define CLKID_VAPB           140
+#define CLKID_VDEC_1_SEL     151
+#define CLKID_VDEC_1         153
+#define CLKID_VDEC_HEVC_SEL  154
+#define CLKID_VDEC_HEVC      156
 
 #endif /* __GXBB_CLKC_H */

Signature · 1 line

2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html