[RESEND PATCH 0/4] Fixup clocks for venus on sm8250

[RESEND PATCH 0/4] Fixup clocks for venus on sm8250

[Bryan O'Donoghue]

- Adds some missing indices to the videocc index
- Adds videocc clock tree hirearchy for msv0
- Adds a regulator to power the videocc consistent with downstream

Once done we can move on to enabling these clocks in the DTS and switch on
the venus.

Bryan O'Donoghue (4):
  dt-bindings: clock: Add missing SM8250 videoc clock indices
  clk: qcom: videocc: Add sm8250 VIDEO_CC_MVS0_DIV_CLK_SRC
  clk: qcom: videocc: Add sm8250 VIDEO_CC_MVS0_CLK
  clk: qcom: videocc: Add gdsc mmcx-reg supply hook

 drivers/clk/qcom/videocc-sm8250.c             | 39 +++++++++++++++++++
 .../dt-bindings/clock/qcom,videocc-sm8250.h   |  2 +
 2 files changed, 41 insertions(+)

-- 
2.29.2

[PATCH 1/4] dt-bindings: clock: Add missing SM8250 videoc clock indices

[Bryan O'Donoghue]

Two indexes need to be added to videocc-sm8250.h for venus to function
properly. Rather than adding the missing indexes when used we add them
separately here to keep checkpatch.pl happy.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 include/dt-bindings/clock/qcom,videocc-sm8250.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/dt-bindings/clock/qcom,videocc-sm8250.h b/include/dt-bindings/clock/qcom,videocc-sm8250.h
index 2b2b3867af25..8d321ac3b1fa 100644
--- a/include/dt-bindings/clock/qcom,videocc-sm8250.h
+++ b/include/dt-bindings/clock/qcom,videocc-sm8250.h
@@ -16,6 +16,8 @@
 #define VIDEO_CC_MVS1C_DIV2_DIV_CLK_SRC	6
 #define VIDEO_CC_PLL0			7
 #define VIDEO_CC_PLL1			8
+#define VIDEO_CC_MVS0_DIV_CLK_SRC	9
+#define VIDEO_CC_MVS0_CLK		10
 
 /* VIDEO_CC resets */
 #define VIDEO_CC_CVP_INTERFACE_BCR	0
-- 
2.29.2

[PATCH 2/4] clk: qcom: videocc: Add sm8250 VIDEO_CC_MVS0_DIV_CLK_SRC

[Bryan O'Donoghue]

This patch adds the missing video_cc_mvs0_div_clk_src entry to
videocc-sm8250 replicating in upstream the explicit entry for this clock in
downstream.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/clk/qcom/videocc-sm8250.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/clk/qcom/videocc-sm8250.c b/drivers/clk/qcom/videocc-sm8250.c
index 2797c61f5938..cc84963ced40 100644
--- a/drivers/clk/qcom/videocc-sm8250.c
+++ b/drivers/clk/qcom/videocc-sm8250.c
@@ -169,6 +169,21 @@ static struct clk_regmap_div video_cc_mvs0c_div2_div_clk_src = {
 	},
 };
 
+static struct clk_regmap_div video_cc_mvs0_div_clk_src = {
+	.reg = 0xd54,
+	.shift = 0,
+	.width = 2,
+	.clkr.hw.init = &(struct clk_init_data) {
+		.name = "video_cc_mvs0_div_clk_src",
+		.parent_data = &(const struct clk_parent_data){
+			.hw = &video_cc_mvs0_clk_src.clkr.hw,
+		},
+		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT,
+		.ops = &clk_regmap_div_ro_ops,
+	},
+};
+
 static struct clk_regmap_div video_cc_mvs1c_div2_div_clk_src = {
 	.reg = 0xcf4,
 	.shift = 0,
@@ -276,6 +291,7 @@ static struct gdsc mvs1_gdsc = {
 
 static struct clk_regmap *video_cc_sm8250_clocks[] = {
 	[VIDEO_CC_MVS0_CLK_SRC] = &video_cc_mvs0_clk_src.clkr,
+	[VIDEO_CC_MVS0_DIV_CLK_SRC] = &video_cc_mvs0_div_clk_src.clkr,
 	[VIDEO_CC_MVS0C_CLK] = &video_cc_mvs0c_clk.clkr,
 	[VIDEO_CC_MVS0C_DIV2_DIV_CLK_SRC] = &video_cc_mvs0c_div2_div_clk_src.clkr,
 	[VIDEO_CC_MVS1_CLK_SRC] = &video_cc_mvs1_clk_src.clkr,
-- 
2.29.2

[PATCH 3/4] clk: qcom: videocc: Add sm8250 VIDEO_CC_MVS0_CLK

[Bryan O'Donoghue]

This patch adds the missing video_cc_mvs0_div_clk entry to
videocc-sm8250 replicating in upstream the explicit entry for this clock in
downstream.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/clk/qcom/videocc-sm8250.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/clk/qcom/videocc-sm8250.c b/drivers/clk/qcom/videocc-sm8250.c
index cc84963ced40..3ade240bd49c 100644
--- a/drivers/clk/qcom/videocc-sm8250.c
+++ b/drivers/clk/qcom/videocc-sm8250.c
@@ -217,6 +217,24 @@ static struct clk_branch video_cc_mvs0c_clk = {
 	},
 };
 
+static struct clk_branch video_cc_mvs0_clk = {
+	.halt_reg = 0xd34,
+	.halt_check = BRANCH_HALT_VOTED,
+	.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_mvs1_div2_clk = {
 	.halt_reg = 0xdf4,
 	.halt_check = BRANCH_HALT_VOTED,
@@ -290,6 +308,7 @@ static struct gdsc mvs1_gdsc = {
 };
 
 static struct clk_regmap *video_cc_sm8250_clocks[] = {
+	[VIDEO_CC_MVS0_CLK] = &video_cc_mvs0_clk.clkr,
 	[VIDEO_CC_MVS0_CLK_SRC] = &video_cc_mvs0_clk_src.clkr,
 	[VIDEO_CC_MVS0_DIV_CLK_SRC] = &video_cc_mvs0_div_clk_src.clkr,
 	[VIDEO_CC_MVS0C_CLK] = &video_cc_mvs0c_clk.clkr,
-- 
2.29.2

[PATCH 4/4] clk: qcom: videocc: Add gdsc mmcx-reg supply hook

[Bryan O'Donoghue]

This commit adds a regulator supply hook to mmcx-reg missing from

- mvs0c_gdsc
- mvs1c_gdsc
- mvs0_gdsc
- mvs1_gdsc

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/clk/qcom/videocc-sm8250.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/qcom/videocc-sm8250.c b/drivers/clk/qcom/videocc-sm8250.c
index 3ade240bd49c..b0efadc19634 100644
--- a/drivers/clk/qcom/videocc-sm8250.c
+++ b/drivers/clk/qcom/videocc-sm8250.c
@@ -278,6 +278,7 @@ static struct gdsc mvs0c_gdsc = {
 	},
 	.flags = 0,
 	.pwrsts = PWRSTS_OFF_ON,
+	.supply = "mmcx",
 };
 
 static struct gdsc mvs1c_gdsc = {
@@ -287,6 +288,7 @@ static struct gdsc mvs1c_gdsc = {
 	},
 	.flags = 0,
 	.pwrsts = PWRSTS_OFF_ON,
+	.supply = "mmcx",
 };
 
 static struct gdsc mvs0_gdsc = {
@@ -296,6 +298,7 @@ static struct gdsc mvs0_gdsc = {
 	},
 	.flags = HW_CTRL,
 	.pwrsts = PWRSTS_OFF_ON,
+	.supply = "mmcx",
 };
 
 static struct gdsc mvs1_gdsc = {
@@ -305,6 +308,7 @@ static struct gdsc mvs1_gdsc = {
 	},
 	.flags = HW_CTRL,
 	.pwrsts = PWRSTS_OFF_ON,
+	.supply = "mmcx",
 };
 
 static struct clk_regmap *video_cc_sm8250_clocks[] = {
-- 
2.29.2

Re: [RESEND PATCH 0/4] Fixup clocks for venus on sm8250

[Dmitry Baryshkov]

Patch 3 should reference 'video_cc_mvs0_clk' rather than
'video_cc_mvs0_div_clk'  in the commit message.

With that fixed all patches are:
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Tested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

On Wed, 3 Feb 2021 at 14:53, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> - Adds some missing indices to the videocc index
> - Adds videocc clock tree hirearchy for msv0
> - Adds a regulator to power the videocc consistent with downstream
>
> Once done we can move on to enabling these clocks in the DTS and switch on
> the venus.
>
> Bryan O'Donoghue (4):
>   dt-bindings: clock: Add missing SM8250 videoc clock indices
>   clk: qcom: videocc: Add sm8250 VIDEO_CC_MVS0_DIV_CLK_SRC
>   clk: qcom: videocc: Add sm8250 VIDEO_CC_MVS0_CLK
>   clk: qcom: videocc: Add gdsc mmcx-reg supply hook
>
>  drivers/clk/qcom/videocc-sm8250.c             | 39 +++++++++++++++++++
>  .../dt-bindings/clock/qcom,videocc-sm8250.h   |  2 +
>  2 files changed, 41 insertions(+)
>
> --
> 2.29.2
>


-- 
With best wishes
Dmitry

Re: [RESEND PATCH 0/4] Fixup clocks for venus on sm8250

[Dmitry Baryshkov]

On Wed, 3 Feb 2021 at 14:53, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> - Adds some missing indices to the videocc index
> - Adds videocc clock tree hirearchy for msv0
> - Adds a regulator to power the videocc consistent with downstream
>
> Once done we can move on to enabling these clocks in the DTS and switch on
> the venus.
>
> Bryan O'Donoghue (4):
>   dt-bindings: clock: Add missing SM8250 videoc clock indices
>   clk: qcom: videocc: Add sm8250 VIDEO_CC_MVS0_DIV_CLK_SRC
>   clk: qcom: videocc: Add sm8250 VIDEO_CC_MVS0_CLK
>   clk: qcom: videocc: Add gdsc mmcx-reg supply hook

I remember now the old discussion about these two clocks
(https://lore.kernel.org/linux-arm-msm/160092826778.310579.12225989905897101118@swboyd.mtv.corp.google.com/).

Is the message. reported by Jonathan, solved by the mmcx-reg supply?
Also are these clocks necessary at all?


-- 
With best wishes
Dmitry

Re: [RESEND PATCH 0/4] Fixup clocks for venus on sm8250

[Bryan O'Donoghue]

On 03/02/2021 14:25, Dmitry Baryshkov wrote:
> On Wed, 3 Feb 2021 at 14:53, Bryan O'Donoghue
> <bryan.odonoghue@linaro.org> wrote:
>>
>> - Adds some missing indices to the videocc index
>> - Adds videocc clock tree hirearchy for msv0
>> - Adds a regulator to power the videocc consistent with downstream
>>
>> Once done we can move on to enabling these clocks in the DTS and switch on
>> the venus.
>>
>> Bryan O'Donoghue (4):
>>    dt-bindings: clock: Add missing SM8250 videoc clock indices
>>    clk: qcom: videocc: Add sm8250 VIDEO_CC_MVS0_DIV_CLK_SRC
>>    clk: qcom: videocc: Add sm8250 VIDEO_CC_MVS0_CLK
>>    clk: qcom: videocc: Add gdsc mmcx-reg supply hook
> 
> I remember now the old discussion about these two clocks
> (https://lore.kernel.org/linux-arm-msm/160092826778.310579.12225989905897101118@swboyd.mtv.corp.google.com/).
> 
> Is the message. reported by Jonathan, solved by the mmcx-reg supply?

The stuck problem is a bandwidth problem not a regulator problem, 
there's some internal clock dependency we don't have visibility of.

Both of these resolve,

Jonathan had a hack:
https://git.linaro.org/people/bryan.odonoghue/kernel.git/commit/?h=tracking-qcomlt-sm8250-09-11-20%2bvenus-flto-simple-test2&id=d4bea74282d14244127a1e97766b6108ec68e22f

Dikshita proposed this instead, which, we will send out soon:
https://git.linaro.org/people/bryan.odonoghue/kernel.git/commit/?h=tracking-qcomlt-sm8250-venus-integrated-sm8250&id=782202d611761e639af7e44d8344c0c098642b9f

> Also are these clocks necessary at all?

Eh well I'm not sure what happens if I try to dump the clocks but, this 
is an accurate representation of what is done downstream so..

---
bod

Re: [RESEND PATCH 0/4] Fixup clocks for venus on sm8250

[Dmitry Baryshkov]

On Wed, 3 Feb 2021 at 17:56, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 03/02/2021 14:25, Dmitry Baryshkov wrote:
> > On Wed, 3 Feb 2021 at 14:53, Bryan O'Donoghue
> > <bryan.odonoghue@linaro.org> wrote:
> >>
> >> - Adds some missing indices to the videocc index
> >> - Adds videocc clock tree hirearchy for msv0
> >> - Adds a regulator to power the videocc consistent with downstream
> >>
> >> Once done we can move on to enabling these clocks in the DTS and switch on
> >> the venus.
> >>
> >> Bryan O'Donoghue (4):
> >>    dt-bindings: clock: Add missing SM8250 videoc clock indices
> >>    clk: qcom: videocc: Add sm8250 VIDEO_CC_MVS0_DIV_CLK_SRC
> >>    clk: qcom: videocc: Add sm8250 VIDEO_CC_MVS0_CLK
> >>    clk: qcom: videocc: Add gdsc mmcx-reg supply hook
> >
> > I remember now the old discussion about these two clocks
> > (https://lore.kernel.org/linux-arm-msm/160092826778.310579.12225989905897101118@swboyd.mtv.corp.google.com/).
> >
> > Is the message. reported by Jonathan, solved by the mmcx-reg supply?
>
> The stuck problem is a bandwidth problem not a regulator problem,
> there's some internal clock dependency we don't have visibility of.
>
> Both of these resolve,
>
> Jonathan had a hack:
> https://git.linaro.org/people/bryan.odonoghue/kernel.git/commit/?h=tracking-qcomlt-sm8250-09-11-20%2bvenus-flto-simple-test2&id=d4bea74282d14244127a1e97766b6108ec68e22f
>
> Dikshita proposed this instead, which, we will send out soon:
> https://git.linaro.org/people/bryan.odonoghue/kernel.git/commit/?h=tracking-qcomlt-sm8250-venus-integrated-sm8250&id=782202d611761e639af7e44d8344c0c098642b9f
>
> > Also are these clocks necessary at all?
>
> Eh well I'm not sure what happens if I try to dump the clocks but, this
> is an accurate representation of what is done downstream so..

OK then.

-- 
With best wishes
Dmitry