Re: [PATCH 2/2] clk: meson: g12a: add MIPI DSI Host Pixel Clock

2020-11-26 Thread Neil Armstrong
On 25/11/2020 18:32, Jerome Brunet wrote:
> 
> On Mon 23 Nov 2020 at 17:38, Neil Armstrong  wrote:
> 
>> This adds the MIPI DSI Host Pixel Clock, unlike AXG, the pixel clock can be 
>> different
>> from the VPU ENCL output clock to feed the DSI Host controller with a 
>> different clock rate.
>>
>> Signed-off-by: Neil Armstrong 
> 
> Series looks good.
> 2 minor comments below 
> 
>> ---
>>  drivers/clk/meson/g12a.c | 72 
>>  drivers/clk/meson/g12a.h |  3 +-
>>  2 files changed, 74 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
>> index 3cb8196c8e29..3dedf8408405 100644
>> --- a/drivers/clk/meson/g12a.c
>> +++ b/drivers/clk/meson/g12a.c
>> @@ -3658,6 +3658,66 @@ static struct clk_regmap g12a_hdmi_tx = {
>>  },
>>  };
>>  
>> +/* MIPI DSI Host Clocks */
>> +
>> +static const struct clk_hw *g12a_mipi_dsi_pxclk_parent_hws[] = {
>> +_vid_pll.hw,
>> +_gp0_pll.hw,
>> +_hifi_pll.hw,
>> +_mpll1.hw,
>> +_fclk_div2.hw,
>> +_fclk_div2p5.hw,
>> +_fclk_div3.hw,
>> +_fclk_div7.hw,
>> +};
>> +
>> +static struct clk_regmap g12a_mipi_dsi_pxclk_sel = {
>> +.data = &(struct clk_regmap_mux_data){
>> +.offset = HHI_MIPIDSI_PHY_CLK_CNTL,
>> +.mask = 0x7,
>> +.shift = 12,
>> +.flags = CLK_MUX_ROUND_CLOSEST,
>> +},
>> +.hw.init = &(struct clk_init_data){
>> +.name = "mipi_dsi_pxclk_sel",
>> +.ops = _regmap_mux_ops,
>> +.parent_hws = g12a_mipi_dsi_pxclk_parent_hws,
>> +.num_parents = ARRAY_SIZE(g12a_mipi_dsi_pxclk_parent_hws),
>> +.flags = CLK_SET_RATE_PARENT,
> 
> The id of the mux is exposed which seems to hint the mux will be
> manually controller but CLK_SET_RATE_NO_REPARENT is not set. Is this on
> purpose ?

You're right, it should be CLK_SET_RATE_NO_REPARENT here since we need to
control the source of the clock.

> 
>> +},
>> +};
>> +
>> +static struct clk_regmap g12a_mipi_dsi_pxclk_div = {
>> +.data = &(struct clk_regmap_div_data){
>> +.offset = HHI_MIPIDSI_PHY_CLK_CNTL,
>> +.shift = 0,
>> +.width = 7,
>> +},
>> +.hw.init = &(struct clk_init_data){
>> +.name = "mipi_dsi_pxclk_div",
>> +.ops = _regmap_divider_ops,
>> +.parent_hws = (const struct clk_hw *[]) {
>> +_mipi_dsi_pxclk_sel.hw },
> 
> Alignment here is weird compared to the reset of the file

ok

> 
>> +.num_parents = 1,
>> +.flags = CLK_SET_RATE_PARENT,
>> +},
>> +};
>> +
>> +static struct clk_regmap g12a_mipi_dsi_pxclk = {
>> +.data = &(struct clk_regmap_gate_data){
>> +.offset = HHI_MIPIDSI_PHY_CLK_CNTL,
>> +.bit_idx = 8,
>> +},
>> +.hw.init = &(struct clk_init_data) {
>> +.name = "mipi_dsi_pxclk",
>> +.ops = _regmap_gate_ops,
>> +.parent_hws = (const struct clk_hw *[]) {
>> +_mipi_dsi_pxclk_div.hw },
>> +.num_parents = 1,
>> +.flags = CLK_SET_RATE_PARENT,
>> +},
>> +};
>> +
>>  /* HDMI Clocks */
>>  
>>  static const struct clk_parent_data g12a_hdmi_parent_data[] = {
>> @@ -4403,6 +4463,9 @@ static struct clk_hw_onecell_data g12a_hw_onecell_data 
>> = {
>>  [CLKID_SPICC1_SCLK_SEL] = _spicc1_sclk_sel.hw,
>>  [CLKID_SPICC1_SCLK_DIV] = _spicc1_sclk_div.hw,
>>  [CLKID_SPICC1_SCLK] = _spicc1_sclk.hw,
>> +[CLKID_MIPI_DSI_PXCLK_SEL]  = _mipi_dsi_pxclk_sel.hw,
>> +[CLKID_MIPI_DSI_PXCLK_DIV]  = _mipi_dsi_pxclk_div.hw,
>> +[CLKID_MIPI_DSI_PXCLK]  = _mipi_dsi_pxclk.hw,
>>  [NR_CLKS]   = NULL,
>>  },
>>  .num = NR_CLKS,
>> @@ -4658,6 +4721,9 @@ static struct clk_hw_onecell_data g12b_hw_onecell_data 
>> = {
>>  [CLKID_SPICC1_SCLK_SEL] = _spicc1_sclk_sel.hw,
>>  [CLKID_SPICC1_SCLK_DIV] = _spicc1_sclk_div.hw,
>>  [CLKID_SPICC1_SCLK] = _spicc1_sclk.hw,
>> +[CLKID_MIPI_DSI_PXCLK_SEL]  = _mipi_dsi_pxclk_sel.hw,
>> +[CLKID_MIPI_DSI_PXCLK_DIV]  = _mipi_dsi_pxclk_div.hw,
>> +[CLKID_MIPI_DSI_PXCLK]  = _mipi_dsi_pxclk.hw,
>>  [NR_CLKS]   = NULL,
>>  },
>>  .num = NR_CLKS,
>> @@ -4904,6 +4970,9 @@ static struct clk_hw_onecell_data sm1_hw_onecell_data 
>> = {
>>  [CLKID_NNA_CORE_CLK_SEL]= _nna_core_clk_sel.hw,
>>  [CLKID_NNA_CORE_CLK_DIV]= _nna_core_clk_div.hw,
>>  [CLKID_NNA_CORE_CLK]= _nna_core_clk.hw,
>> +[CLKID_MIPI_DSI_PXCLK_SEL]  = _mipi_dsi_pxclk_sel.hw,
>> +[CLKID_MIPI_DSI_PXCLK_DIV]  = _mipi_dsi_pxclk_div.hw,
>> +[CLKID_MIPI_DSI_PXCLK]   

Re: [PATCH 2/2] clk: meson: g12a: add MIPI DSI Host Pixel Clock

2020-11-25 Thread Jerome Brunet


On Mon 23 Nov 2020 at 17:38, Neil Armstrong  wrote:

> This adds the MIPI DSI Host Pixel Clock, unlike AXG, the pixel clock can be 
> different
> from the VPU ENCL output clock to feed the DSI Host controller with a 
> different clock rate.
>
> Signed-off-by: Neil Armstrong 

Series looks good.
2 minor comments below 

> ---
>  drivers/clk/meson/g12a.c | 72 
>  drivers/clk/meson/g12a.h |  3 +-
>  2 files changed, 74 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
> index 3cb8196c8e29..3dedf8408405 100644
> --- a/drivers/clk/meson/g12a.c
> +++ b/drivers/clk/meson/g12a.c
> @@ -3658,6 +3658,66 @@ static struct clk_regmap g12a_hdmi_tx = {
>   },
>  };
>  
> +/* MIPI DSI Host Clocks */
> +
> +static const struct clk_hw *g12a_mipi_dsi_pxclk_parent_hws[] = {
> + _vid_pll.hw,
> + _gp0_pll.hw,
> + _hifi_pll.hw,
> + _mpll1.hw,
> + _fclk_div2.hw,
> + _fclk_div2p5.hw,
> + _fclk_div3.hw,
> + _fclk_div7.hw,
> +};
> +
> +static struct clk_regmap g12a_mipi_dsi_pxclk_sel = {
> + .data = &(struct clk_regmap_mux_data){
> + .offset = HHI_MIPIDSI_PHY_CLK_CNTL,
> + .mask = 0x7,
> + .shift = 12,
> + .flags = CLK_MUX_ROUND_CLOSEST,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "mipi_dsi_pxclk_sel",
> + .ops = _regmap_mux_ops,
> + .parent_hws = g12a_mipi_dsi_pxclk_parent_hws,
> + .num_parents = ARRAY_SIZE(g12a_mipi_dsi_pxclk_parent_hws),
> + .flags = CLK_SET_RATE_PARENT,

The id of the mux is exposed which seems to hint the mux will be
manually controller but CLK_SET_RATE_NO_REPARENT is not set. Is this on
purpose ?

> + },
> +};
> +
> +static struct clk_regmap g12a_mipi_dsi_pxclk_div = {
> + .data = &(struct clk_regmap_div_data){
> + .offset = HHI_MIPIDSI_PHY_CLK_CNTL,
> + .shift = 0,
> + .width = 7,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "mipi_dsi_pxclk_div",
> + .ops = _regmap_divider_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + _mipi_dsi_pxclk_sel.hw },

Alignment here is weird compared to the reset of the file

> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + },
> +};
> +
> +static struct clk_regmap g12a_mipi_dsi_pxclk = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = HHI_MIPIDSI_PHY_CLK_CNTL,
> + .bit_idx = 8,
> + },
> + .hw.init = &(struct clk_init_data) {
> + .name = "mipi_dsi_pxclk",
> + .ops = _regmap_gate_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + _mipi_dsi_pxclk_div.hw },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + },
> +};
> +
>  /* HDMI Clocks */
>  
>  static const struct clk_parent_data g12a_hdmi_parent_data[] = {
> @@ -4403,6 +4463,9 @@ static struct clk_hw_onecell_data g12a_hw_onecell_data 
> = {
>   [CLKID_SPICC1_SCLK_SEL] = _spicc1_sclk_sel.hw,
>   [CLKID_SPICC1_SCLK_DIV] = _spicc1_sclk_div.hw,
>   [CLKID_SPICC1_SCLK] = _spicc1_sclk.hw,
> + [CLKID_MIPI_DSI_PXCLK_SEL]  = _mipi_dsi_pxclk_sel.hw,
> + [CLKID_MIPI_DSI_PXCLK_DIV]  = _mipi_dsi_pxclk_div.hw,
> + [CLKID_MIPI_DSI_PXCLK]  = _mipi_dsi_pxclk.hw,
>   [NR_CLKS]   = NULL,
>   },
>   .num = NR_CLKS,
> @@ -4658,6 +4721,9 @@ static struct clk_hw_onecell_data g12b_hw_onecell_data 
> = {
>   [CLKID_SPICC1_SCLK_SEL] = _spicc1_sclk_sel.hw,
>   [CLKID_SPICC1_SCLK_DIV] = _spicc1_sclk_div.hw,
>   [CLKID_SPICC1_SCLK] = _spicc1_sclk.hw,
> + [CLKID_MIPI_DSI_PXCLK_SEL]  = _mipi_dsi_pxclk_sel.hw,
> + [CLKID_MIPI_DSI_PXCLK_DIV]  = _mipi_dsi_pxclk_div.hw,
> + [CLKID_MIPI_DSI_PXCLK]  = _mipi_dsi_pxclk.hw,
>   [NR_CLKS]   = NULL,
>   },
>   .num = NR_CLKS,
> @@ -4904,6 +4970,9 @@ static struct clk_hw_onecell_data sm1_hw_onecell_data = 
> {
>   [CLKID_NNA_CORE_CLK_SEL]= _nna_core_clk_sel.hw,
>   [CLKID_NNA_CORE_CLK_DIV]= _nna_core_clk_div.hw,
>   [CLKID_NNA_CORE_CLK]= _nna_core_clk.hw,
> + [CLKID_MIPI_DSI_PXCLK_SEL]  = _mipi_dsi_pxclk_sel.hw,
> + [CLKID_MIPI_DSI_PXCLK_DIV]  = _mipi_dsi_pxclk_div.hw,
> + [CLKID_MIPI_DSI_PXCLK]  = _mipi_dsi_pxclk.hw,
>   [NR_CLKS]   = NULL,
>   },
>   .num = NR_CLKS,
> @@ -5151,6 +5220,9 @@ static struct clk_regmap *const g12a_clk_regmaps[] = {
>   _nna_core_clk_sel,

[PATCH 2/2] clk: meson: g12a: add MIPI DSI Host Pixel Clock

2020-11-23 Thread Neil Armstrong
This adds the MIPI DSI Host Pixel Clock, unlike AXG, the pixel clock can be 
different
from the VPU ENCL output clock to feed the DSI Host controller with a different 
clock rate.

Signed-off-by: Neil Armstrong 
---
 drivers/clk/meson/g12a.c | 72 
 drivers/clk/meson/g12a.h |  3 +-
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
index 3cb8196c8e29..3dedf8408405 100644
--- a/drivers/clk/meson/g12a.c
+++ b/drivers/clk/meson/g12a.c
@@ -3658,6 +3658,66 @@ static struct clk_regmap g12a_hdmi_tx = {
},
 };
 
+/* MIPI DSI Host Clocks */
+
+static const struct clk_hw *g12a_mipi_dsi_pxclk_parent_hws[] = {
+   _vid_pll.hw,
+   _gp0_pll.hw,
+   _hifi_pll.hw,
+   _mpll1.hw,
+   _fclk_div2.hw,
+   _fclk_div2p5.hw,
+   _fclk_div3.hw,
+   _fclk_div7.hw,
+};
+
+static struct clk_regmap g12a_mipi_dsi_pxclk_sel = {
+   .data = &(struct clk_regmap_mux_data){
+   .offset = HHI_MIPIDSI_PHY_CLK_CNTL,
+   .mask = 0x7,
+   .shift = 12,
+   .flags = CLK_MUX_ROUND_CLOSEST,
+   },
+   .hw.init = &(struct clk_init_data){
+   .name = "mipi_dsi_pxclk_sel",
+   .ops = _regmap_mux_ops,
+   .parent_hws = g12a_mipi_dsi_pxclk_parent_hws,
+   .num_parents = ARRAY_SIZE(g12a_mipi_dsi_pxclk_parent_hws),
+   .flags = CLK_SET_RATE_PARENT,
+   },
+};
+
+static struct clk_regmap g12a_mipi_dsi_pxclk_div = {
+   .data = &(struct clk_regmap_div_data){
+   .offset = HHI_MIPIDSI_PHY_CLK_CNTL,
+   .shift = 0,
+   .width = 7,
+   },
+   .hw.init = &(struct clk_init_data){
+   .name = "mipi_dsi_pxclk_div",
+   .ops = _regmap_divider_ops,
+   .parent_hws = (const struct clk_hw *[]) {
+   _mipi_dsi_pxclk_sel.hw },
+   .num_parents = 1,
+   .flags = CLK_SET_RATE_PARENT,
+   },
+};
+
+static struct clk_regmap g12a_mipi_dsi_pxclk = {
+   .data = &(struct clk_regmap_gate_data){
+   .offset = HHI_MIPIDSI_PHY_CLK_CNTL,
+   .bit_idx = 8,
+   },
+   .hw.init = &(struct clk_init_data) {
+   .name = "mipi_dsi_pxclk",
+   .ops = _regmap_gate_ops,
+   .parent_hws = (const struct clk_hw *[]) {
+   _mipi_dsi_pxclk_div.hw },
+   .num_parents = 1,
+   .flags = CLK_SET_RATE_PARENT,
+   },
+};
+
 /* HDMI Clocks */
 
 static const struct clk_parent_data g12a_hdmi_parent_data[] = {
@@ -4403,6 +4463,9 @@ static struct clk_hw_onecell_data g12a_hw_onecell_data = {
[CLKID_SPICC1_SCLK_SEL] = _spicc1_sclk_sel.hw,
[CLKID_SPICC1_SCLK_DIV] = _spicc1_sclk_div.hw,
[CLKID_SPICC1_SCLK] = _spicc1_sclk.hw,
+   [CLKID_MIPI_DSI_PXCLK_SEL]  = _mipi_dsi_pxclk_sel.hw,
+   [CLKID_MIPI_DSI_PXCLK_DIV]  = _mipi_dsi_pxclk_div.hw,
+   [CLKID_MIPI_DSI_PXCLK]  = _mipi_dsi_pxclk.hw,
[NR_CLKS]   = NULL,
},
.num = NR_CLKS,
@@ -4658,6 +4721,9 @@ static struct clk_hw_onecell_data g12b_hw_onecell_data = {
[CLKID_SPICC1_SCLK_SEL] = _spicc1_sclk_sel.hw,
[CLKID_SPICC1_SCLK_DIV] = _spicc1_sclk_div.hw,
[CLKID_SPICC1_SCLK] = _spicc1_sclk.hw,
+   [CLKID_MIPI_DSI_PXCLK_SEL]  = _mipi_dsi_pxclk_sel.hw,
+   [CLKID_MIPI_DSI_PXCLK_DIV]  = _mipi_dsi_pxclk_div.hw,
+   [CLKID_MIPI_DSI_PXCLK]  = _mipi_dsi_pxclk.hw,
[NR_CLKS]   = NULL,
},
.num = NR_CLKS,
@@ -4904,6 +4970,9 @@ static struct clk_hw_onecell_data sm1_hw_onecell_data = {
[CLKID_NNA_CORE_CLK_SEL]= _nna_core_clk_sel.hw,
[CLKID_NNA_CORE_CLK_DIV]= _nna_core_clk_div.hw,
[CLKID_NNA_CORE_CLK]= _nna_core_clk.hw,
+   [CLKID_MIPI_DSI_PXCLK_SEL]  = _mipi_dsi_pxclk_sel.hw,
+   [CLKID_MIPI_DSI_PXCLK_DIV]  = _mipi_dsi_pxclk_div.hw,
+   [CLKID_MIPI_DSI_PXCLK]  = _mipi_dsi_pxclk.hw,
[NR_CLKS]   = NULL,
},
.num = NR_CLKS,
@@ -5151,6 +5220,9 @@ static struct clk_regmap *const g12a_clk_regmaps[] = {
_nna_core_clk_sel,
_nna_core_clk_div,
_nna_core_clk,
+   _mipi_dsi_pxclk_sel,
+   _mipi_dsi_pxclk_div,
+   _mipi_dsi_pxclk,
 };
 
 static const struct reg_sequence g12a_init_regs[] = {
diff --git a/drivers/clk/meson/g12a.h b/drivers/clk/meson/g12a.h
index 69b6a69549c7..a97613df38b3 100644
--- a/drivers/clk/meson/g12a.h
+++ b/drivers/clk/meson/g12a.h
@@ -264,8 +264,9 @@
 #define