Subject: [PATCH v2 0/2] MediaTek clocks: Support mux indices list and 8195 DP

Changes in v2:
- Rebased on next-20231018

This series adds support to specify custom parent indices for MediaTek
MUX clocks, necessary to avoid setting the same parent PLL for MT8195's
top_dp and top_edp clocks, solving DP+eDP concurrent output issues.

No fixes tags are provided as the clk-mux commit introduces new logic
and the actual MT8195 fix depends on that.

This commit was tested on the Acer Tomato Chromebook (MT8195) with
dual concurrent display outputs (internal eDP panel and TypeC->DP->HDMI
adapter connected to Samsung UE40JU6400 4k TV); resolution switch on
DP was also tested; eDP output is not paused and internal display keeps
working as expected.


After feedback on v1 of this series (effectively the same as v2, except
for the rebase), I looked for an alternative solution with using
clk_set_rate_exclusive() as proposed. Couldn't find any, as that still
doesn't give any guarantee about selecting the same PLL that the driver
was manipulating before setting the right divider MUX, hence that still
wouldn't work correctly.

For more context, please look at the conversation at [1].

Cheers!

[1] https://lore.kernel.org/linux-arm-kernel/[email protected]

AngeloGioacchino Del Regno (2):
clk: mediatek: clk-mux: Support custom parent indices for muxes
clk: mediatek: mt8195-topckgen: Refactor parents for top_dp/edp muxes

drivers/clk/mediatek/clk-mt8195-topckgen.c | 22 +++++++----
drivers/clk/mediatek/clk-mux.c | 14 +++++++
drivers/clk/mediatek/clk-mux.h | 43 ++++++++++++++++++++--
3 files changed, 67 insertions(+), 12 deletions(-)

--
2.42.0


Subject: [PATCH v2 2/2] clk: mediatek: mt8195-topckgen: Refactor parents for top_dp/edp muxes

The top_dp and top_edp muxes can be both parented to either TVDPLL1
or TVDPLL2, two identically specced PLLs for the specific purpose of
giving out pixel clock: this becomes a problem when the MediaTek
DisplayPort Interface (DPI) driver tries to set the pixel clock rate.

In the usecase of two simultaneous outputs (using two controllers),
it was seen that one of the displays would sometimes display garbled
output (if any at all) and this was because:
- top_edp was set to TVDPLL1, outputting X GHz
- top_dp was set to TVDPLL2, outputting Y GHz
- mtk_dpi calls clk_set_rate(top_edp, Z GHz)
- top_dp is switched to TVDPLL1
- TVDPLL1 changes its rate, top_edp outputs the wrong rate.
- eDP display is garbled

To solve this issue, remove all TVDPLL1 parents from `top_dp` and
all TVDPLL2 parents from `top_edp`, plus, necessarily switch both
clocks to use the new MUX_GATE_CLR_SET_UPD_INDEXED() macro to be
able to use the right bit index for the new parents list.

Reviewed-by: Alexandre Mergnat <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/clk/mediatek/clk-mt8195-topckgen.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt8195-topckgen.c b/drivers/clk/mediatek/clk-mt8195-topckgen.c
index 5c426a1c94c7..4529255376e9 100644
--- a/drivers/clk/mediatek/clk-mt8195-topckgen.c
+++ b/drivers/clk/mediatek/clk-mt8195-topckgen.c
@@ -417,15 +417,21 @@ static const char * const pwrmcu_parents[] = {

static const char * const dp_parents[] = {
"clk26m",
- "tvdpll1_d2",
"tvdpll2_d2",
- "tvdpll1_d4",
"tvdpll2_d4",
- "tvdpll1_d8",
"tvdpll2_d8",
- "tvdpll1_d16",
"tvdpll2_d16"
};
+static const u8 dp_parents_idx[] = { 0, 2, 4, 6, 8 };
+
+static const char * const edp_parents[] = {
+ "clk26m",
+ "tvdpll1_d2",
+ "tvdpll1_d4",
+ "tvdpll1_d8",
+ "tvdpll1_d16"
+};
+static const u8 edp_parents_idx[] = { 0, 1, 3, 5, 7 };

static const char * const disp_pwm_parents[] = {
"clk26m",
@@ -957,11 +963,11 @@ static const struct mtk_mux top_mtk_muxes[] = {
MUX_GATE_CLR_SET_UPD_FLAGS(CLK_TOP_PWRMCU, "top_pwrmcu",
pwrmcu_parents, 0x08C, 0x090, 0x094, 16, 3, 23, 0x08, 6,
CLK_IS_CRITICAL | CLK_SET_RATE_PARENT),
- MUX_GATE_CLR_SET_UPD(CLK_TOP_DP, "top_dp",
- dp_parents, 0x08C, 0x090, 0x094, 24, 4, 31, 0x08, 7),
+ MUX_GATE_CLR_SET_UPD_INDEXED(CLK_TOP_DP, "top_dp",
+ dp_parents, dp_parents_idx, 0x08C, 0x090, 0x094, 24, 4, 31, 0x08, 7),
/* CLK_CFG_10 */
- MUX_GATE_CLR_SET_UPD(CLK_TOP_EDP, "top_edp",
- dp_parents, 0x098, 0x09C, 0x0A0, 0, 4, 7, 0x08, 8),
+ MUX_GATE_CLR_SET_UPD_INDEXED(CLK_TOP_EDP, "top_edp",
+ edp_parents, edp_parents_idx, 0x098, 0x09C, 0x0A0, 0, 4, 7, 0x08, 8),
MUX_GATE_CLR_SET_UPD(CLK_TOP_DPI, "top_dpi",
dp_parents, 0x098, 0x09C, 0x0A0, 8, 4, 15, 0x08, 9),
MUX_GATE_CLR_SET_UPD(CLK_TOP_DISP_PWM0, "top_disp_pwm0",
--
2.42.0

Subject: [PATCH v2 1/2] clk: mediatek: clk-mux: Support custom parent indices for muxes

Add support for customized parent indices for MediaTek muxes: this is
necessary for the case in which we want to exclude some clocks from
a mux's parent clocks list, where the exclusions are not from the
very bottom of the list but either in the middle or the beginning.

Example:
- MUX1 (all parents)
- parent1; idx=0
- parent2; idx=1
- parent3; idx=2

- MUX1 (wanted parents)
- parent1; idx=0
- parent3; idx=2

To achieve that add a `parent_index` array pointer to struct mtk_mux,
then in .set_parent(), .get_parent() callbacks check if this array
was populated and eventually get the index from that.

Also, to avoid updating all clock drivers for all SoCs, rename the
"main" macro to __GATE_CLR_SET_UPD_FLAGS (so, `__` was added) and
add the new member to it; furthermore, GATE_CLK_SET_UPD_FLAGS has
been reintroduced as being fully compatible with the older version.

The new parent_index can be specified with the new `_INDEXED`
variants of the MUX_GATE_CLR_SET_UPD_xxxx macros.

Reviewed-by: Alexandre Mergnat <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/clk/mediatek/clk-mux.c | 14 +++++++++++
drivers/clk/mediatek/clk-mux.h | 43 ++++++++++++++++++++++++++++++----
2 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
index c93bc7f926e5..60990296450b 100644
--- a/drivers/clk/mediatek/clk-mux.c
+++ b/drivers/clk/mediatek/clk-mux.c
@@ -89,6 +89,17 @@ static u8 mtk_clk_mux_get_parent(struct clk_hw *hw)
regmap_read(mux->regmap, mux->data->mux_ofs, &val);
val = (val >> mux->data->mux_shift) & mask;

+ if (mux->data->parent_index) {
+ int i;
+
+ for (i = 0; i < mux->data->num_parents; i++)
+ if (mux->data->parent_index[i] == val)
+ return i;
+
+ /* Not found: return an impossible index to generate error */
+ return mux->data->num_parents + 1;
+ }
+
return val;
}

@@ -104,6 +115,9 @@ static int mtk_clk_mux_set_parent_setclr_lock(struct clk_hw *hw, u8 index)
else
__acquire(mux->lock);

+ if (mux->data->parent_index)
+ index = mux->data->parent_index[index];
+
regmap_read(mux->regmap, mux->data->mux_ofs, &orig);
val = (orig & ~(mask << mux->data->mux_shift))
| (index << mux->data->mux_shift);
diff --git a/drivers/clk/mediatek/clk-mux.h b/drivers/clk/mediatek/clk-mux.h
index 7ecb963b0ec6..943ad1d7ce4b 100644
--- a/drivers/clk/mediatek/clk-mux.h
+++ b/drivers/clk/mediatek/clk-mux.h
@@ -21,6 +21,7 @@ struct mtk_mux {
int id;
const char *name;
const char * const *parent_names;
+ const u8 *parent_index;
unsigned int flags;

u32 mux_ofs;
@@ -37,9 +38,10 @@ struct mtk_mux {
signed char num_parents;
};

-#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \
- _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
- _gate, _upd_ofs, _upd, _flags, _ops) { \
+#define __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _paridx, \
+ _num_parents, _mux_ofs, _mux_set_ofs, \
+ _mux_clr_ofs, _shift, _width, _gate, _upd_ofs, \
+ _upd, _flags, _ops) { \
.id = _id, \
.name = _name, \
.mux_ofs = _mux_ofs, \
@@ -51,11 +53,28 @@ struct mtk_mux {
.gate_shift = _gate, \
.upd_shift = _upd, \
.parent_names = _parents, \
- .num_parents = ARRAY_SIZE(_parents), \
+ .parent_index = _paridx, \
+ .num_parents = _num_parents, \
.flags = _flags, \
.ops = &_ops, \
}

+#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \
+ _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
+ _gate, _upd_ofs, _upd, _flags, _ops) \
+ __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, \
+ NULL, ARRAY_SIZE(_parents), _mux_ofs, \
+ _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
+ _gate, _upd_ofs, _upd, _flags, _ops) \
+
+#define GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, _paridx, \
+ _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift, \
+ _width, _gate, _upd_ofs, _upd, _flags, _ops) \
+ __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, \
+ _paridx, ARRAY_SIZE(_paridx), _mux_ofs, \
+ _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
+ _gate, _upd_ofs, _upd, _flags, _ops) \
+
extern const struct clk_ops mtk_mux_clr_set_upd_ops;
extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;

@@ -67,6 +86,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
_gate, _upd_ofs, _upd, _flags, \
mtk_mux_gate_clr_set_upd_ops)

+#define MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, \
+ _paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs, \
+ _shift, _width, _gate, _upd_ofs, _upd, _flags) \
+ GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, \
+ _paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs, \
+ _shift, _width, _gate, _upd_ofs, _upd, _flags, \
+ mtk_mux_gate_clr_set_upd_ops)
+
#define MUX_GATE_CLR_SET_UPD(_id, _name, _parents, _mux_ofs, \
_mux_set_ofs, _mux_clr_ofs, _shift, _width, \
_gate, _upd_ofs, _upd) \
@@ -75,6 +102,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
_width, _gate, _upd_ofs, _upd, \
CLK_SET_RATE_PARENT)

+#define MUX_GATE_CLR_SET_UPD_INDEXED(_id, _name, _parents, _paridx, \
+ _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift, \
+ _width, _gate, _upd_ofs, _upd) \
+ MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, \
+ _parents, _paridx, _mux_ofs, _mux_set_ofs, \
+ _mux_clr_ofs, _shift, _width, _gate, _upd_ofs, \
+ _upd, CLK_SET_RATE_PARENT)
+
#define MUX_CLR_SET_UPD(_id, _name, _parents, _mux_ofs, \
_mux_set_ofs, _mux_clr_ofs, _shift, _width, \
_upd_ofs, _upd) \
--
2.42.0

Subject: Re: [PATCH v2 1/2] clk: mediatek: clk-mux: Support custom parent indices for muxes

Hi Angelo,

On Wed, Oct 18, 2023 at 12:35:45PM +0200, AngeloGioacchino Del Regno wrote:
> Add support for customized parent indices for MediaTek muxes: this is
> necessary for the case in which we want to exclude some clocks from
> a mux's parent clocks list, where the exclusions are not from the
> very bottom of the list but either in the middle or the beginning.
>
> Example:
> - MUX1 (all parents)
> - parent1; idx=0
> - parent2; idx=1
> - parent3; idx=2
>
> - MUX1 (wanted parents)
> - parent1; idx=0
> - parent3; idx=2
>
> To achieve that add a `parent_index` array pointer to struct mtk_mux,
> then in .set_parent(), .get_parent() callbacks check if this array
> was populated and eventually get the index from that.
>
> Also, to avoid updating all clock drivers for all SoCs, rename the
> "main" macro to __GATE_CLR_SET_UPD_FLAGS (so, `__` was added) and
> add the new member to it; furthermore, GATE_CLK_SET_UPD_FLAGS has
> been reintroduced as being fully compatible with the older version.
>
> The new parent_index can be specified with the new `_INDEXED`
> variants of the MUX_GATE_CLR_SET_UPD_xxxx macros.
>
> Reviewed-by: Alexandre Mergnat <[email protected]>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
> ---
> drivers/clk/mediatek/clk-mux.c | 14 +++++++++++
> drivers/clk/mediatek/clk-mux.h | 43 ++++++++++++++++++++++++++++++----
> 2 files changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
> index c93bc7f926e5..60990296450b 100644
> --- a/drivers/clk/mediatek/clk-mux.c
> +++ b/drivers/clk/mediatek/clk-mux.c
> @@ -89,6 +89,17 @@ static u8 mtk_clk_mux_get_parent(struct clk_hw *hw)
> regmap_read(mux->regmap, mux->data->mux_ofs, &val);
> val = (val >> mux->data->mux_shift) & mask;
>
> + if (mux->data->parent_index) {
> + int i;
> +
> + for (i = 0; i < mux->data->num_parents; i++)
> + if (mux->data->parent_index[i] == val)
> + return i;
> +
> + /* Not found: return an impossible index to generate error */
> + return mux->data->num_parents + 1;
> + }
> +
> return val;
> }
>
> @@ -104,6 +115,9 @@ static int mtk_clk_mux_set_parent_setclr_lock(struct clk_hw *hw, u8 index)
> else
> __acquire(mux->lock);
>
> + if (mux->data->parent_index)
> + index = mux->data->parent_index[index];
> +
> regmap_read(mux->regmap, mux->data->mux_ofs, &orig);
> val = (orig & ~(mask << mux->data->mux_shift))
> | (index << mux->data->mux_shift);
> diff --git a/drivers/clk/mediatek/clk-mux.h b/drivers/clk/mediatek/clk-mux.h
> index 7ecb963b0ec6..943ad1d7ce4b 100644
> --- a/drivers/clk/mediatek/clk-mux.h
> +++ b/drivers/clk/mediatek/clk-mux.h
> @@ -21,6 +21,7 @@ struct mtk_mux {
> int id;
> const char *name;
> const char * const *parent_names;
> + const u8 *parent_index;
> unsigned int flags;

I think at some point it would be nice to have a documentation of these
fields.

>
> u32 mux_ofs;
> @@ -37,9 +38,10 @@ struct mtk_mux {
> signed char num_parents;
> };
>
> -#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \
> - _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> - _gate, _upd_ofs, _upd, _flags, _ops) { \
> +#define __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _paridx, \
> + _num_parents, _mux_ofs, _mux_set_ofs, \
> + _mux_clr_ofs, _shift, _width, _gate, _upd_ofs, \
> + _upd, _flags, _ops) { \
> .id = _id, \
> .name = _name, \
> .mux_ofs = _mux_ofs, \
> @@ -51,11 +53,28 @@ struct mtk_mux {
> .gate_shift = _gate, \
> .upd_shift = _upd, \
> .parent_names = _parents, \
> - .num_parents = ARRAY_SIZE(_parents), \
> + .parent_index = _paridx, \
> + .num_parents = _num_parents, \

I was wondering why you moved the ARRAY_SIZE() to the outer macros and
add another argument to the already huge list of arguments? I couldn't
find a use-case for this in the patches you sent.

Best,
Markus

> .flags = _flags, \
> .ops = &_ops, \
> }
>
> +#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \
> + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> + _gate, _upd_ofs, _upd, _flags, _ops) \
> + __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, \
> + NULL, ARRAY_SIZE(_parents), _mux_ofs, \
> + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> + _gate, _upd_ofs, _upd, _flags, _ops) \
> +
> +#define GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, _paridx, \
> + _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift, \
> + _width, _gate, _upd_ofs, _upd, _flags, _ops) \
> + __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, \
> + _paridx, ARRAY_SIZE(_paridx), _mux_ofs, \
> + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> + _gate, _upd_ofs, _upd, _flags, _ops) \
> +
> extern const struct clk_ops mtk_mux_clr_set_upd_ops;
> extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
>
> @@ -67,6 +86,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
> _gate, _upd_ofs, _upd, _flags, \
> mtk_mux_gate_clr_set_upd_ops)
>
> +#define MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, \
> + _paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs, \
> + _shift, _width, _gate, _upd_ofs, _upd, _flags) \
> + GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, \
> + _paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs, \
> + _shift, _width, _gate, _upd_ofs, _upd, _flags, \
> + mtk_mux_gate_clr_set_upd_ops)
> +
> #define MUX_GATE_CLR_SET_UPD(_id, _name, _parents, _mux_ofs, \
> _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> _gate, _upd_ofs, _upd) \
> @@ -75,6 +102,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
> _width, _gate, _upd_ofs, _upd, \
> CLK_SET_RATE_PARENT)
>
> +#define MUX_GATE_CLR_SET_UPD_INDEXED(_id, _name, _parents, _paridx, \
> + _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift, \
> + _width, _gate, _upd_ofs, _upd) \
> + MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, \
> + _parents, _paridx, _mux_ofs, _mux_set_ofs, \
> + _mux_clr_ofs, _shift, _width, _gate, _upd_ofs, \
> + _upd, CLK_SET_RATE_PARENT)
> +
> #define MUX_CLR_SET_UPD(_id, _name, _parents, _mux_ofs, \
> _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> _upd_ofs, _upd) \
> --
> 2.42.0
>

2023-10-23 06:54:45

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] clk: mediatek: clk-mux: Support custom parent indices for muxes

On Mon, Oct 23, 2023 at 2:44 PM Markus Schneider-Pargmann
<[email protected]> wrote:
>
> Hi Angelo,
>
> On Wed, Oct 18, 2023 at 12:35:45PM +0200, AngeloGioacchino Del Regno wrote:
> > Add support for customized parent indices for MediaTek muxes: this is
> > necessary for the case in which we want to exclude some clocks from
> > a mux's parent clocks list, where the exclusions are not from the
> > very bottom of the list but either in the middle or the beginning.
> >
> > Example:
> > - MUX1 (all parents)
> > - parent1; idx=0
> > - parent2; idx=1
> > - parent3; idx=2
> >
> > - MUX1 (wanted parents)
> > - parent1; idx=0
> > - parent3; idx=2
> >
> > To achieve that add a `parent_index` array pointer to struct mtk_mux,
> > then in .set_parent(), .get_parent() callbacks check if this array
> > was populated and eventually get the index from that.
> >
> > Also, to avoid updating all clock drivers for all SoCs, rename the
> > "main" macro to __GATE_CLR_SET_UPD_FLAGS (so, `__` was added) and
> > add the new member to it; furthermore, GATE_CLK_SET_UPD_FLAGS has
> > been reintroduced as being fully compatible with the older version.
> >
> > The new parent_index can be specified with the new `_INDEXED`
> > variants of the MUX_GATE_CLR_SET_UPD_xxxx macros.
> >
> > Reviewed-by: Alexandre Mergnat <[email protected]>
> > Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
> > ---
> > drivers/clk/mediatek/clk-mux.c | 14 +++++++++++
> > drivers/clk/mediatek/clk-mux.h | 43 ++++++++++++++++++++++++++++++----
> > 2 files changed, 53 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
> > index c93bc7f926e5..60990296450b 100644
> > --- a/drivers/clk/mediatek/clk-mux.c
> > +++ b/drivers/clk/mediatek/clk-mux.c
> > @@ -89,6 +89,17 @@ static u8 mtk_clk_mux_get_parent(struct clk_hw *hw)
> > regmap_read(mux->regmap, mux->data->mux_ofs, &val);
> > val = (val >> mux->data->mux_shift) & mask;
> >
> > + if (mux->data->parent_index) {
> > + int i;
> > +
> > + for (i = 0; i < mux->data->num_parents; i++)
> > + if (mux->data->parent_index[i] == val)
> > + return i;
> > +
> > + /* Not found: return an impossible index to generate error */
> > + return mux->data->num_parents + 1;
> > + }
> > +
> > return val;
> > }
> >
> > @@ -104,6 +115,9 @@ static int mtk_clk_mux_set_parent_setclr_lock(struct clk_hw *hw, u8 index)
> > else
> > __acquire(mux->lock);
> >
> > + if (mux->data->parent_index)
> > + index = mux->data->parent_index[index];
> > +
> > regmap_read(mux->regmap, mux->data->mux_ofs, &orig);
> > val = (orig & ~(mask << mux->data->mux_shift))
> > | (index << mux->data->mux_shift);
> > diff --git a/drivers/clk/mediatek/clk-mux.h b/drivers/clk/mediatek/clk-mux.h
> > index 7ecb963b0ec6..943ad1d7ce4b 100644
> > --- a/drivers/clk/mediatek/clk-mux.h
> > +++ b/drivers/clk/mediatek/clk-mux.h
> > @@ -21,6 +21,7 @@ struct mtk_mux {
> > int id;
> > const char *name;
> > const char * const *parent_names;
> > + const u8 *parent_index;
> > unsigned int flags;
>
> I think at some point it would be nice to have a documentation of these
> fields.
>
> >
> > u32 mux_ofs;
> > @@ -37,9 +38,10 @@ struct mtk_mux {
> > signed char num_parents;
> > };
> >
> > -#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \
> > - _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> > - _gate, _upd_ofs, _upd, _flags, _ops) { \
> > +#define __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _paridx, \
> > + _num_parents, _mux_ofs, _mux_set_ofs, \
> > + _mux_clr_ofs, _shift, _width, _gate, _upd_ofs, \
> > + _upd, _flags, _ops) { \
> > .id = _id, \
> > .name = _name, \
> > .mux_ofs = _mux_ofs, \
> > @@ -51,11 +53,28 @@ struct mtk_mux {
> > .gate_shift = _gate, \
> > .upd_shift = _upd, \
> > .parent_names = _parents, \
> > - .num_parents = ARRAY_SIZE(_parents), \
> > + .parent_index = _paridx, \
> > + .num_parents = _num_parents, \
>
> I was wondering why you moved the ARRAY_SIZE() to the outer macros and
> add another argument to the already huge list of arguments? I couldn't
> find a use-case for this in the patches you sent.

This is the base macro that all the other ones are based on, so it has
the complete list of arguments. The other ones use this and provide
default values for the arguments they don't need.

ChenYu

> Best,
> Markus
>
> > .flags = _flags, \
> > .ops = &_ops, \
> > }
> >
> > +#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \
> > + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> > + _gate, _upd_ofs, _upd, _flags, _ops) \
> > + __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, \
> > + NULL, ARRAY_SIZE(_parents), _mux_ofs, \
> > + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> > + _gate, _upd_ofs, _upd, _flags, _ops) \
> > +
> > +#define GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, _paridx, \
> > + _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift, \
> > + _width, _gate, _upd_ofs, _upd, _flags, _ops) \
> > + __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, \
> > + _paridx, ARRAY_SIZE(_paridx), _mux_ofs, \
> > + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> > + _gate, _upd_ofs, _upd, _flags, _ops) \
> > +
> > extern const struct clk_ops mtk_mux_clr_set_upd_ops;
> > extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
> >
> > @@ -67,6 +86,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
> > _gate, _upd_ofs, _upd, _flags, \
> > mtk_mux_gate_clr_set_upd_ops)
> >
> > +#define MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, \
> > + _paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs, \
> > + _shift, _width, _gate, _upd_ofs, _upd, _flags) \
> > + GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, \
> > + _paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs, \
> > + _shift, _width, _gate, _upd_ofs, _upd, _flags, \
> > + mtk_mux_gate_clr_set_upd_ops)
> > +
> > #define MUX_GATE_CLR_SET_UPD(_id, _name, _parents, _mux_ofs, \
> > _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> > _gate, _upd_ofs, _upd) \
> > @@ -75,6 +102,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
> > _width, _gate, _upd_ofs, _upd, \
> > CLK_SET_RATE_PARENT)
> >
> > +#define MUX_GATE_CLR_SET_UPD_INDEXED(_id, _name, _parents, _paridx, \
> > + _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift, \
> > + _width, _gate, _upd_ofs, _upd) \
> > + MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, \
> > + _parents, _paridx, _mux_ofs, _mux_set_ofs, \
> > + _mux_clr_ofs, _shift, _width, _gate, _upd_ofs, \
> > + _upd, CLK_SET_RATE_PARENT)
> > +
> > #define MUX_CLR_SET_UPD(_id, _name, _parents, _mux_ofs, \
> > _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> > _upd_ofs, _upd) \
> > --
> > 2.42.0
> >
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2023-10-23 06:58:11

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] clk: mediatek: clk-mux: Support custom parent indices for muxes

On Wed, Oct 18, 2023 at 6:36 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Add support for customized parent indices for MediaTek muxes: this is
> necessary for the case in which we want to exclude some clocks from
> a mux's parent clocks list, where the exclusions are not from the
> very bottom of the list but either in the middle or the beginning.
>
> Example:
> - MUX1 (all parents)
> - parent1; idx=0
> - parent2; idx=1
> - parent3; idx=2
>
> - MUX1 (wanted parents)
> - parent1; idx=0
> - parent3; idx=2
>
> To achieve that add a `parent_index` array pointer to struct mtk_mux,
> then in .set_parent(), .get_parent() callbacks check if this array
> was populated and eventually get the index from that.
>
> Also, to avoid updating all clock drivers for all SoCs, rename the
> "main" macro to __GATE_CLR_SET_UPD_FLAGS (so, `__` was added) and
> add the new member to it; furthermore, GATE_CLK_SET_UPD_FLAGS has
> been reintroduced as being fully compatible with the older version.
>
> The new parent_index can be specified with the new `_INDEXED`
> variants of the MUX_GATE_CLR_SET_UPD_xxxx macros.
>
> Reviewed-by: Alexandre Mergnat <[email protected]>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
> ---
> drivers/clk/mediatek/clk-mux.c | 14 +++++++++++
> drivers/clk/mediatek/clk-mux.h | 43 ++++++++++++++++++++++++++++++----
> 2 files changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
> index c93bc7f926e5..60990296450b 100644
> --- a/drivers/clk/mediatek/clk-mux.c
> +++ b/drivers/clk/mediatek/clk-mux.c
> @@ -89,6 +89,17 @@ static u8 mtk_clk_mux_get_parent(struct clk_hw *hw)
> regmap_read(mux->regmap, mux->data->mux_ofs, &val);
> val = (val >> mux->data->mux_shift) & mask;
>
> + if (mux->data->parent_index) {
> + int i;
> +
> + for (i = 0; i < mux->data->num_parents; i++)
> + if (mux->data->parent_index[i] == val)
> + return i;
> +
> + /* Not found: return an impossible index to generate error */
> + return mux->data->num_parents + 1;
> + }
> +
> return val;
> }
>
> @@ -104,6 +115,9 @@ static int mtk_clk_mux_set_parent_setclr_lock(struct clk_hw *hw, u8 index)
> else
> __acquire(mux->lock);
>
> + if (mux->data->parent_index)
> + index = mux->data->parent_index[index];
> +
> regmap_read(mux->regmap, mux->data->mux_ofs, &orig);
> val = (orig & ~(mask << mux->data->mux_shift))
> | (index << mux->data->mux_shift);
> diff --git a/drivers/clk/mediatek/clk-mux.h b/drivers/clk/mediatek/clk-mux.h
> index 7ecb963b0ec6..943ad1d7ce4b 100644
> --- a/drivers/clk/mediatek/clk-mux.h
> +++ b/drivers/clk/mediatek/clk-mux.h
> @@ -21,6 +21,7 @@ struct mtk_mux {
> int id;
> const char *name;
> const char * const *parent_names;
> + const u8 *parent_index;
> unsigned int flags;
>
> u32 mux_ofs;
> @@ -37,9 +38,10 @@ struct mtk_mux {
> signed char num_parents;
> };
>
> -#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \
> - _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> - _gate, _upd_ofs, _upd, _flags, _ops) { \
> +#define __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _paridx, \
> + _num_parents, _mux_ofs, _mux_set_ofs, \
> + _mux_clr_ofs, _shift, _width, _gate, _upd_ofs, \
> + _upd, _flags, _ops) { \

Nit: I would take this opportunity to fix up the alignment of the lines
here, as well as for the new macros. Also you needn't wrap the lines at
80 characters. Instead I would wrap where it makes more sense, like group
arguments for the same type (parents / mux / gate / update ...) on the
same line if possible.

Otherwise,

Reviewed-by: Chen-Yu Tsai <[email protected]>

> .id = _id, \
> .name = _name, \
> .mux_ofs = _mux_ofs, \
> @@ -51,11 +53,28 @@ struct mtk_mux {
> .gate_shift = _gate, \
> .upd_shift = _upd, \
> .parent_names = _parents, \
> - .num_parents = ARRAY_SIZE(_parents), \
> + .parent_index = _paridx, \
> + .num_parents = _num_parents, \
> .flags = _flags, \
> .ops = &_ops, \
> }
>
> +#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \
> + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> + _gate, _upd_ofs, _upd, _flags, _ops) \
> + __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, \
> + NULL, ARRAY_SIZE(_parents), _mux_ofs, \
> + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> + _gate, _upd_ofs, _upd, _flags, _ops) \
> +
> +#define GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, _paridx, \
> + _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift, \
> + _width, _gate, _upd_ofs, _upd, _flags, _ops) \
> + __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, \
> + _paridx, ARRAY_SIZE(_paridx), _mux_ofs, \
> + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> + _gate, _upd_ofs, _upd, _flags, _ops) \
> +
> extern const struct clk_ops mtk_mux_clr_set_upd_ops;
> extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
>
> @@ -67,6 +86,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
> _gate, _upd_ofs, _upd, _flags, \
> mtk_mux_gate_clr_set_upd_ops)
>
> +#define MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, \
> + _paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs, \
> + _shift, _width, _gate, _upd_ofs, _upd, _flags) \
> + GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, \
> + _paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs, \
> + _shift, _width, _gate, _upd_ofs, _upd, _flags, \
> + mtk_mux_gate_clr_set_upd_ops)
> +
> #define MUX_GATE_CLR_SET_UPD(_id, _name, _parents, _mux_ofs, \
> _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> _gate, _upd_ofs, _upd) \
> @@ -75,6 +102,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
> _width, _gate, _upd_ofs, _upd, \
> CLK_SET_RATE_PARENT)
>
> +#define MUX_GATE_CLR_SET_UPD_INDEXED(_id, _name, _parents, _paridx, \
> + _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift, \
> + _width, _gate, _upd_ofs, _upd) \
> + MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, \
> + _parents, _paridx, _mux_ofs, _mux_set_ofs, \
> + _mux_clr_ofs, _shift, _width, _gate, _upd_ofs, \
> + _upd, CLK_SET_RATE_PARENT)
> +
> #define MUX_CLR_SET_UPD(_id, _name, _parents, _mux_ofs, \
> _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> _upd_ofs, _upd) \
> --
> 2.42.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2023-10-23 06:59:10

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] clk: mediatek: clk-mux: Support custom parent indices for muxes

On Mon, Oct 23, 2023 at 2:57 PM Chen-Yu Tsai <[email protected]> wrote:
>
> On Wed, Oct 18, 2023 at 6:36 PM AngeloGioacchino Del Regno
> <[email protected]> wrote:
> >
> > Add support for customized parent indices for MediaTek muxes: this is
> > necessary for the case in which we want to exclude some clocks from
> > a mux's parent clocks list, where the exclusions are not from the
> > very bottom of the list but either in the middle or the beginning.
> >
> > Example:
> > - MUX1 (all parents)
> > - parent1; idx=0
> > - parent2; idx=1
> > - parent3; idx=2
> >
> > - MUX1 (wanted parents)
> > - parent1; idx=0
> > - parent3; idx=2
> >
> > To achieve that add a `parent_index` array pointer to struct mtk_mux,
> > then in .set_parent(), .get_parent() callbacks check if this array
> > was populated and eventually get the index from that.
> >
> > Also, to avoid updating all clock drivers for all SoCs, rename the
> > "main" macro to __GATE_CLR_SET_UPD_FLAGS (so, `__` was added) and
> > add the new member to it; furthermore, GATE_CLK_SET_UPD_FLAGS has
> > been reintroduced as being fully compatible with the older version.
> >
> > The new parent_index can be specified with the new `_INDEXED`
> > variants of the MUX_GATE_CLR_SET_UPD_xxxx macros.
> >
> > Reviewed-by: Alexandre Mergnat <[email protected]>
> > Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
> > ---
> > drivers/clk/mediatek/clk-mux.c | 14 +++++++++++
> > drivers/clk/mediatek/clk-mux.h | 43 ++++++++++++++++++++++++++++++----
> > 2 files changed, 53 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
> > index c93bc7f926e5..60990296450b 100644
> > --- a/drivers/clk/mediatek/clk-mux.c
> > +++ b/drivers/clk/mediatek/clk-mux.c
> > @@ -89,6 +89,17 @@ static u8 mtk_clk_mux_get_parent(struct clk_hw *hw)
> > regmap_read(mux->regmap, mux->data->mux_ofs, &val);
> > val = (val >> mux->data->mux_shift) & mask;
> >
> > + if (mux->data->parent_index) {
> > + int i;
> > +
> > + for (i = 0; i < mux->data->num_parents; i++)
> > + if (mux->data->parent_index[i] == val)
> > + return i;
> > +
> > + /* Not found: return an impossible index to generate error */
> > + return mux->data->num_parents + 1;
> > + }
> > +
> > return val;
> > }
> >
> > @@ -104,6 +115,9 @@ static int mtk_clk_mux_set_parent_setclr_lock(struct clk_hw *hw, u8 index)
> > else
> > __acquire(mux->lock);
> >
> > + if (mux->data->parent_index)
> > + index = mux->data->parent_index[index];
> > +
> > regmap_read(mux->regmap, mux->data->mux_ofs, &orig);
> > val = (orig & ~(mask << mux->data->mux_shift))
> > | (index << mux->data->mux_shift);
> > diff --git a/drivers/clk/mediatek/clk-mux.h b/drivers/clk/mediatek/clk-mux.h
> > index 7ecb963b0ec6..943ad1d7ce4b 100644
> > --- a/drivers/clk/mediatek/clk-mux.h
> > +++ b/drivers/clk/mediatek/clk-mux.h
> > @@ -21,6 +21,7 @@ struct mtk_mux {
> > int id;
> > const char *name;
> > const char * const *parent_names;
> > + const u8 *parent_index;
> > unsigned int flags;
> >
> > u32 mux_ofs;
> > @@ -37,9 +38,10 @@ struct mtk_mux {
> > signed char num_parents;
> > };
> >
> > -#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \
> > - _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> > - _gate, _upd_ofs, _upd, _flags, _ops) { \
> > +#define __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _paridx, \
> > + _num_parents, _mux_ofs, _mux_set_ofs, \
> > + _mux_clr_ofs, _shift, _width, _gate, _upd_ofs, \
> > + _upd, _flags, _ops) { \
>
> Nit: I would take this opportunity to fix up the alignment of the lines
> here, as well as for the new macros. Also you needn't wrap the lines at
> 80 characters. Instead I would wrap where it makes more sense, like group
> arguments for the same type (parents / mux / gate / update ...) on the
> same line if possible.
>
> Otherwise,
>
> Reviewed-by: Chen-Yu Tsai <[email protected]>

And of course I meant to send this from my chromium.org address...

Reviewed-by: Chen-Yu Tsai <[email protected]>

> > .id = _id, \
> > .name = _name, \
> > .mux_ofs = _mux_ofs, \
> > @@ -51,11 +53,28 @@ struct mtk_mux {
> > .gate_shift = _gate, \
> > .upd_shift = _upd, \
> > .parent_names = _parents, \
> > - .num_parents = ARRAY_SIZE(_parents), \
> > + .parent_index = _paridx, \
> > + .num_parents = _num_parents, \
> > .flags = _flags, \
> > .ops = &_ops, \
> > }
> >
> > +#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \
> > + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> > + _gate, _upd_ofs, _upd, _flags, _ops) \
> > + __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, \
> > + NULL, ARRAY_SIZE(_parents), _mux_ofs, \
> > + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> > + _gate, _upd_ofs, _upd, _flags, _ops) \
> > +
> > +#define GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, _paridx, \
> > + _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift, \
> > + _width, _gate, _upd_ofs, _upd, _flags, _ops) \
> > + __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, \
> > + _paridx, ARRAY_SIZE(_paridx), _mux_ofs, \
> > + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> > + _gate, _upd_ofs, _upd, _flags, _ops) \
> > +
> > extern const struct clk_ops mtk_mux_clr_set_upd_ops;
> > extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
> >
> > @@ -67,6 +86,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
> > _gate, _upd_ofs, _upd, _flags, \
> > mtk_mux_gate_clr_set_upd_ops)
> >
> > +#define MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, \
> > + _paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs, \
> > + _shift, _width, _gate, _upd_ofs, _upd, _flags) \
> > + GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, \
> > + _paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs, \
> > + _shift, _width, _gate, _upd_ofs, _upd, _flags, \
> > + mtk_mux_gate_clr_set_upd_ops)
> > +
> > #define MUX_GATE_CLR_SET_UPD(_id, _name, _parents, _mux_ofs, \
> > _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> > _gate, _upd_ofs, _upd) \
> > @@ -75,6 +102,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
> > _width, _gate, _upd_ofs, _upd, \
> > CLK_SET_RATE_PARENT)
> >
> > +#define MUX_GATE_CLR_SET_UPD_INDEXED(_id, _name, _parents, _paridx, \
> > + _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift, \
> > + _width, _gate, _upd_ofs, _upd) \
> > + MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, \
> > + _parents, _paridx, _mux_ofs, _mux_set_ofs, \
> > + _mux_clr_ofs, _shift, _width, _gate, _upd_ofs, \
> > + _upd, CLK_SET_RATE_PARENT)
> > +
> > #define MUX_CLR_SET_UPD(_id, _name, _parents, _mux_ofs, \
> > _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> > _upd_ofs, _upd) \
> > --
> > 2.42.0
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2023-10-23 07:00:56

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] clk: mediatek: mt8195-topckgen: Refactor parents for top_dp/edp muxes

On Wed, Oct 18, 2023 at 6:35 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> The top_dp and top_edp muxes can be both parented to either TVDPLL1
> or TVDPLL2, two identically specced PLLs for the specific purpose of
> giving out pixel clock: this becomes a problem when the MediaTek
> DisplayPort Interface (DPI) driver tries to set the pixel clock rate.
>
> In the usecase of two simultaneous outputs (using two controllers),
> it was seen that one of the displays would sometimes display garbled
> output (if any at all) and this was because:
> - top_edp was set to TVDPLL1, outputting X GHz
> - top_dp was set to TVDPLL2, outputting Y GHz
> - mtk_dpi calls clk_set_rate(top_edp, Z GHz)
> - top_dp is switched to TVDPLL1
> - TVDPLL1 changes its rate, top_edp outputs the wrong rate.
> - eDP display is garbled
>
> To solve this issue, remove all TVDPLL1 parents from `top_dp` and
> all TVDPLL2 parents from `top_edp`, plus, necessarily switch both
> clocks to use the new MUX_GATE_CLR_SET_UPD_INDEXED() macro to be
> able to use the right bit index for the new parents list.
>
> Reviewed-by: Alexandre Mergnat <[email protected]>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
> ---
> drivers/clk/mediatek/clk-mt8195-topckgen.c | 22 ++++++++++++++--------
> 1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/mediatek/clk-mt8195-topckgen.c b/drivers/clk/mediatek/clk-mt8195-topckgen.c
> index 5c426a1c94c7..4529255376e9 100644
> --- a/drivers/clk/mediatek/clk-mt8195-topckgen.c
> +++ b/drivers/clk/mediatek/clk-mt8195-topckgen.c
> @@ -417,15 +417,21 @@ static const char * const pwrmcu_parents[] = {


Nit: I'd add a comment saying this list isn't the full list for reasons.

Reviewed-by: Chen-Yu Tsai <[email protected]>

> static const char * const dp_parents[] = {
> "clk26m",
> - "tvdpll1_d2",
> "tvdpll2_d2",
> - "tvdpll1_d4",
> "tvdpll2_d4",
> - "tvdpll1_d8",
> "tvdpll2_d8",
> - "tvdpll1_d16",
> "tvdpll2_d16"
> };
> +static const u8 dp_parents_idx[] = { 0, 2, 4, 6, 8 };
> +
> +static const char * const edp_parents[] = {
> + "clk26m",
> + "tvdpll1_d2",
> + "tvdpll1_d4",
> + "tvdpll1_d8",
> + "tvdpll1_d16"
> +};
> +static const u8 edp_parents_idx[] = { 0, 1, 3, 5, 7 };
>
> static const char * const disp_pwm_parents[] = {
> "clk26m",
> @@ -957,11 +963,11 @@ static const struct mtk_mux top_mtk_muxes[] = {
> MUX_GATE_CLR_SET_UPD_FLAGS(CLK_TOP_PWRMCU, "top_pwrmcu",
> pwrmcu_parents, 0x08C, 0x090, 0x094, 16, 3, 23, 0x08, 6,
> CLK_IS_CRITICAL | CLK_SET_RATE_PARENT),
> - MUX_GATE_CLR_SET_UPD(CLK_TOP_DP, "top_dp",
> - dp_parents, 0x08C, 0x090, 0x094, 24, 4, 31, 0x08, 7),
> + MUX_GATE_CLR_SET_UPD_INDEXED(CLK_TOP_DP, "top_dp",
> + dp_parents, dp_parents_idx, 0x08C, 0x090, 0x094, 24, 4, 31, 0x08, 7),
> /* CLK_CFG_10 */
> - MUX_GATE_CLR_SET_UPD(CLK_TOP_EDP, "top_edp",
> - dp_parents, 0x098, 0x09C, 0x0A0, 0, 4, 7, 0x08, 8),
> + MUX_GATE_CLR_SET_UPD_INDEXED(CLK_TOP_EDP, "top_edp",
> + edp_parents, edp_parents_idx, 0x098, 0x09C, 0x0A0, 0, 4, 7, 0x08, 8),
> MUX_GATE_CLR_SET_UPD(CLK_TOP_DPI, "top_dpi",
> dp_parents, 0x098, 0x09C, 0x0A0, 8, 4, 15, 0x08, 9),
> MUX_GATE_CLR_SET_UPD(CLK_TOP_DISP_PWM0, "top_disp_pwm0",
> --
> 2.42.0
>