mbox series

[RFC,v1,00/11] Qcom SM8350 DispCC & VideoCC

Message ID 20210616141107.291430-1-robert.foss@linaro.org
Headers show
Series Qcom SM8350 DispCC & VideoCC | expand

Message

Robert Foss June 16, 2021, 2:10 p.m. UTC 
Do not merge, this series has yet to be properly tested. Work is in
progress for sm8350 display driver support, which will test this series
properly.

This series implements display clock controller (dispcc) & video
clock controller (videocc) support for the Qcom SM8350 SOC.

In order to support these new clock controllers, some changes to the
alpha plls are required. These changes add support to the Lucid 5LPE PLLs.

Robert Foss (11):
  clk: qcom: common: Add runtime init/suspend/resume
  clk: qcom: rcg2: Add support for flags
  clk: qcom: clk-alpha-pll: Fix typo in comment
  clk: qcom: clk-alpha-pll: Add configuration support for LUCID 5LPE
  dt-bindings: clock: Add QCOM SM8350 display clock bindings
  clk: qcom: Add display clock controller driver for SM8350
  dt-bindings: clock: Add SM8350 QCOM video clock bindings
  clk: qcom: Add video clock controller driver for SM8350
  arm64: dts: qcom: sm8350: Power up dispcc & videocc on sm8350 by MMCX
    regulator
  arm64: dts: qcom: sm8350: Add videocc DT node
  arm64: dts: qcom: sm8350: Add dispcc DT node

 .../bindings/clock/qcom,dispcc-sm8x50.yaml    |    6 +-
 .../bindings/clock/qcom,videocc.yaml          |    2 +
 arch/arm64/boot/dts/qcom/sm8350.dtsi          |   46 +
 drivers/clk/qcom/Kconfig                      |   18 +
 drivers/clk/qcom/Makefile                     |    2 +
 drivers/clk/qcom/clk-alpha-pll.c              |    5 +-
 drivers/clk/qcom/clk-alpha-pll.h              |    5 +
 drivers/clk/qcom/clk-rcg.h                    |    4 +
 drivers/clk/qcom/clk-rcg2.c                   |    3 +
 drivers/clk/qcom/common.c                     |   92 ++
 drivers/clk/qcom/common.h                     |    6 +
 drivers/clk/qcom/dispcc-sm8350.c              | 1402 +++++++++++++++++
 drivers/clk/qcom/videocc-sm8350.c             |  593 +++++++
 .../dt-bindings/clock/qcom,dispcc-sm8350.h    |   77 +
 .../dt-bindings/clock/qcom,videocc-sm8350.h   |   44 +
 15 files changed, 2302 insertions(+), 3 deletions(-)
 create mode 100644 drivers/clk/qcom/dispcc-sm8350.c
 create mode 100644 drivers/clk/qcom/videocc-sm8350.c
 create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm8350.h
 create mode 100644 include/dt-bindings/clock/qcom,videocc-sm8350.h

-- 
2.30.2

Comments

Konrad Dybcio June 16, 2021, 3:33 p.m. UTC | #1 
On 16.06.2021 16:10, Robert Foss wrote:
> These changes are ported from the downstream driver, and are used on SM8350

> for CAMCC, DISPCC, GCC, GPUCC & VIDEOCC.

>

> Signed-off-by: Robert Foss <robert.foss@linaro.org>

> ---

>  drivers/clk/qcom/clk-rcg.h  | 4 ++++

>  drivers/clk/qcom/clk-rcg2.c | 3 +++

>  2 files changed, 7 insertions(+)

>

> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h

> index 99efcc7f8d88..a1f05281d950 100644

> --- a/drivers/clk/qcom/clk-rcg.h

> +++ b/drivers/clk/qcom/clk-rcg.h

> @@ -149,6 +149,10 @@ struct clk_rcg2 {

>  	const struct freq_tbl	*freq_tbl;

>  	struct clk_regmap	clkr;

>  	u8			cfg_off;

> +	u8			flags;

> +#define FORCE_ENABLE_RCG	BIT(0)

> +#define HW_CLK_CTRL_MODE	BIT(1)

> +#define DFS_SUPPORT		BIT(2)

>  };

>  

>  #define to_clk_rcg2(_hw) container_of(to_clk_regmap(_hw), struct clk_rcg2, clkr)

> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c

> index 42f13a2d1cc1..ed2c9b6659cc 100644

> --- a/drivers/clk/qcom/clk-rcg2.c

> +++ b/drivers/clk/qcom/clk-rcg2.c

> @@ -295,6 +295,9 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)

>  	cfg |= rcg->parent_map[index].cfg << CFG_SRC_SEL_SHIFT;

>  	if (rcg->mnd_width && f->n && (f->m != f->n))

>  		cfg |= CFG_MODE_DUAL_EDGE;

> +	if (rcg->flags & HW_CLK_CTRL_MODE)

> +		cfg |= CFG_HW_CLK_CTRL_MASK;

> +

>  	return regmap_update_bits(rcg->clkr.regmap, RCG_CFG_OFFSET(rcg),

>  					mask, cfg);

>  }


What about code for handling other flags? If it's not a part of the series,

I don't think it makes sense to define them. Or perhaps you sent the

wrong revision?


Konrad
Dmitry Baryshkov June 16, 2021, 4:07 p.m. UTC | #2 
On 16/06/2021 17:10, Robert Foss wrote:
> These changes are ported from the downstream driver, and are used on SM8350
> for CAMCC, DISPCC, GCC, GPUCC & VIDEOCC.
> 
> Signed-off-by: Robert Foss <robert.foss@linaro.org>
> ---
>   drivers/clk/qcom/clk-rcg.h  | 4 ++++
>   drivers/clk/qcom/clk-rcg2.c | 3 +++
>   2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> index 99efcc7f8d88..a1f05281d950 100644
> --- a/drivers/clk/qcom/clk-rcg.h
> +++ b/drivers/clk/qcom/clk-rcg.h
> @@ -149,6 +149,10 @@ struct clk_rcg2 {
>   	const struct freq_tbl	*freq_tbl;
>   	struct clk_regmap	clkr;
>   	u8			cfg_off;
> +	u8			flags;
> +#define FORCE_ENABLE_RCG	BIT(0)
> +#define HW_CLK_CTRL_MODE	BIT(1)

Downstream also has these flags for 8250, but the upstream driver ended 
up not using them for the dispcc clocks. Could you please check that you 
realy need HW_CLK_CTRL for dispcc clocks?

> +#define DFS_SUPPORT		BIT(2)
>   };
>   
>   #define to_clk_rcg2(_hw) container_of(to_clk_regmap(_hw), struct clk_rcg2, clkr)
> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> index 42f13a2d1cc1..ed2c9b6659cc 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -295,6 +295,9 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
>   	cfg |= rcg->parent_map[index].cfg << CFG_SRC_SEL_SHIFT;
>   	if (rcg->mnd_width && f->n && (f->m != f->n))
>   		cfg |= CFG_MODE_DUAL_EDGE;
> +	if (rcg->flags & HW_CLK_CTRL_MODE)
> +		cfg |= CFG_HW_CLK_CTRL_MASK;
> +
>   	return regmap_update_bits(rcg->clkr.regmap, RCG_CFG_OFFSET(rcg),
>   					mask, cfg);
>   }
>
Robert Foss June 17, 2021, 7:58 a.m. UTC | #3 
On Wed, 16 Jun 2021 at 17:33, Konrad Dybcio
<konrad.dybcio@somainline.org> wrote:
>
>
> On 16.06.2021 16:10, Robert Foss wrote:
> > These changes are ported from the downstream driver, and are used on SM8350
> > for CAMCC, DISPCC, GCC, GPUCC & VIDEOCC.
> >
> > Signed-off-by: Robert Foss <robert.foss@linaro.org>
> > ---
> >  drivers/clk/qcom/clk-rcg.h  | 4 ++++
> >  drivers/clk/qcom/clk-rcg2.c | 3 +++
> >  2 files changed, 7 insertions(+)
> >
> > diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> > index 99efcc7f8d88..a1f05281d950 100644
> > --- a/drivers/clk/qcom/clk-rcg.h
> > +++ b/drivers/clk/qcom/clk-rcg.h
> > @@ -149,6 +149,10 @@ struct clk_rcg2 {
> >       const struct freq_tbl   *freq_tbl;
> >       struct clk_regmap       clkr;
> >       u8                      cfg_off;
> > +     u8                      flags;
> > +#define FORCE_ENABLE_RCG     BIT(0)
> > +#define HW_CLK_CTRL_MODE     BIT(1)
> > +#define DFS_SUPPORT          BIT(2)
> >  };
> >
> >  #define to_clk_rcg2(_hw) container_of(to_clk_regmap(_hw), struct clk_rcg2, clkr)
> > diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> > index 42f13a2d1cc1..ed2c9b6659cc 100644
> > --- a/drivers/clk/qcom/clk-rcg2.c
> > +++ b/drivers/clk/qcom/clk-rcg2.c
> > @@ -295,6 +295,9 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
> >       cfg |= rcg->parent_map[index].cfg << CFG_SRC_SEL_SHIFT;
> >       if (rcg->mnd_width && f->n && (f->m != f->n))
> >               cfg |= CFG_MODE_DUAL_EDGE;
> > +     if (rcg->flags & HW_CLK_CTRL_MODE)
> > +             cfg |= CFG_HW_CLK_CTRL_MASK;
> > +
> >       return regmap_update_bits(rcg->clkr.regmap, RCG_CFG_OFFSET(rcg),
> >                                       mask, cfg);
> >  }
>
> What about code for handling other flags? If it's not a part of the series,
>
> I don't think it makes sense to define them. Or perhaps you sent the
>
> wrong revision?

I opted to add all of the flags just to document them existing, but
only introducing the ones that will immediately be used is the better
way to go.
Robert Foss June 17, 2021, 1:37 p.m. UTC | #4 
On Wed, 16 Jun 2021 at 18:07, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
> > diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h

> > index 99efcc7f8d88..a1f05281d950 100644

> > --- a/drivers/clk/qcom/clk-rcg.h

> > +++ b/drivers/clk/qcom/clk-rcg.h

> > @@ -149,6 +149,10 @@ struct clk_rcg2 {

> >       const struct freq_tbl   *freq_tbl;

> >       struct clk_regmap       clkr;

> >       u8                      cfg_off;

> > +     u8                      flags;

> > +#define FORCE_ENABLE_RCG     BIT(0)

> > +#define HW_CLK_CTRL_MODE     BIT(1)

>

> Downstream also has these flags for 8250, but the upstream driver ended

> up not using them for the dispcc clocks. Could you please check that you

> realy need HW_CLK_CTRL for dispcc clocks?


HW_CLK_CTRL being flagged in dispcc causes the CFG_HW_CLK_CTRL flag to
be set in the RCG_CFG registers of dispcc.

This flag simply marks the clock as having hardware control enabled or disabled.

As for the question if it is really needed, I can't answer that since
no documentation or downstream comments explain the exact behaviour.
As far as I know the only way to figure out if it is required is
disabling the flag and checking for bugs. I did find this[1] patch,
which enabled HW_CLK_CTRL_MODE.

Should I err on the side of the downstream implementation, or try to
create a minimum functional driver based on the downstream driver?

[1] https://patchwork.kernel.org/project/linux-arm-msm/patch/1514877987-8082-2-git-send-email-anischal@codeaurora.org/