mbox series

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

Message ID 20210203115456.1072975-1-bryan.odonoghue@linaro.org (mailing list archive)
Headers show
Series Fixup clocks for venus on sm8250 | expand

Message

Bryan O'Donoghue Feb. 3, 2021, 11:54 a.m. UTC
- 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(+)

Comments

Dmitry Baryshkov Feb. 3, 2021, 2:16 p.m. UTC | #1
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
>
Dmitry Baryshkov Feb. 3, 2021, 2:25 p.m. UTC | #2
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?
Bryan O'Donoghue Feb. 3, 2021, 2:57 p.m. UTC | #3
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
Dmitry Baryshkov Feb. 4, 2021, 10:50 a.m. UTC | #4
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.