[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9a0817c2-4101-5c21-977d-77ac0d83a067@baylibre.com>
Date: Thu, 13 Jul 2023 15:22:41 +0200
From: Alexandre Mergnat <amergnat@...libre.com>
To: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>, sboyd@...nel.org
Cc: mturquette@...libre.com, matthias.bgg@...il.com,
wenst@...omium.org, msp@...libre.com, yangyingliang@...wei.com,
u.kleine-koenig@...gutronix.de, miles.chen@...iatek.com,
linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, kernel@...labora.com
Subject: Re: [PATCH 2/2] clk: mediatek: mt8195-topckgen: Refactor parents for
top_dp/edp muxes
On 13/07/2023 09:21, AngeloGioacchino Del Regno 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.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
> ---
> 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 81daa24cadde..abb3721f6e1b 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 };
AFAII your solution is to force a specific TVDPLLX for each display, and
it isn't dynamic.
Do you think it's possible to do that using the DTS ? I'm asking
because, IMHO, this kind of setup is more friendly/readable/flexible in
the DTS than hardcoded into the driver.
>
> 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",
--
Regards,
Alexandre
Powered by blists - more mailing lists