All of lore.kernel.org
 help / color / mirror / Atom feed
From: AngeloGioacchino Del Regno  <angelogioacchino.delregno@collabora.com>
To: amergnat@baylibre.com, "Chun-Kuang Hu" <chunkuang.hu@kernel.org>,
	"Philipp Zabel" <p.zabel@pengutronix.de>,
	"David Airlie" <airlied@gmail.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"Jitao Shi" <jitao.shi@mediatek.com>,
	"Xinlei Lee" <xinlei.lee@mediatek.com>,
	"CK Hu" <ck.hu@mediatek.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Will Deacon" <will@kernel.org>
Cc: dri-devel@lists.freedesktop.org,
	linux-mediatek@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-pwm@vger.kernel.org,
	Fabien Parent <fparent@baylibre.com>
Subject: Re: [PATCH 14/18] drm/mediatek: dpi: add support for dpi clock
Date: Tue, 24 Oct 2023 11:12:45 +0200	[thread overview]
Message-ID: <cf25a3cc-6411-45f5-bc7a-6b69cf28c860@collabora.com> (raw)
In-Reply-To: <20231023-display-support-v1-14-5c860ed5c33b@baylibre.com>

Il 23/10/23 16:40, amergnat@baylibre.com ha scritto:
> From: Fabien Parent <fparent@baylibre.com>
> 
> MT8365 requires an additional clock for DPI. Add support for that
> additional clock.
> 
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>

I'm not convinced that this is right... at all.

 From a fast check of the MT8365 DPI clocks, I can see that the DPI0 clock declares
parent VPLL_DPIX (a fixed clock), but nothing ever has VPLL_DPIX_EN (which is the
GATE clock, enabling output of DPIx VPLL?).

But then, there's even more: no clock ever references the CLK_TOP_DPI0_SEL nor the
CLK_TOP_DPI1_SEL gate, which is a PLL parent selector... in other platforms, that
is muxing through the TVDPLL, but on MT8365 that is LVDSPLL?!

I have many questions now:
* Two PLLs are apparently brought up, but which one is the right one?!
   * Is the LVDS PLL really used for DisplayPort? (dpi0_sel)
   * Is the VPLL_DPIx PLL used for DisplayPort instead? (dpi0_dpi0)
* Why is the LVDSTX_PXL clock using the same PLL as DPI0?!
   * Why is the VPLL_DPIx gate never enabled?
* Are you sure that CLK_MM_DPI0_DPI0's parent shouldn't be dpi0_sel instead?
* Where is DPI1 in this SoC? Why is there a dpi1_sel clock, but no MM clock
   for the DPI1 controller? Is there any DPI1 controller, even?!
   * Why is there a DPI1 MUX, if there's no DPI1 controller?!

Answering all those questions will lead you to the right change, which I believe
to be in the clock drivers, not here in mtk_dpi.c.

Cheers!
Angelo


WARNING: multiple messages have this Message-ID (diff)
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
To: amergnat@baylibre.com, "Chun-Kuang Hu" <chunkuang.hu@kernel.org>,
	"Philipp Zabel" <p.zabel@pengutronix.de>,
	"David Airlie" <airlied@gmail.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"Jitao Shi" <jitao.shi@mediatek.com>,
	"Xinlei Lee" <xinlei.lee@mediatek.com>,
	"CK Hu" <ck.hu@mediatek.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Will Deacon" <will@kernel.org>
Cc: dri-devel@lists.freedesktop.org,
	linux-mediatek@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-pwm@vger.kernel.org,
	Fabien Parent <fparent@baylibre.com>
Subject: Re: [PATCH 14/18] drm/mediatek: dpi: add support for dpi clock
Date: Tue, 24 Oct 2023 11:12:45 +0200	[thread overview]
Message-ID: <cf25a3cc-6411-45f5-bc7a-6b69cf28c860@collabora.com> (raw)
In-Reply-To: <20231023-display-support-v1-14-5c860ed5c33b@baylibre.com>

Il 23/10/23 16:40, amergnat@baylibre.com ha scritto:
> From: Fabien Parent <fparent@baylibre.com>
> 
> MT8365 requires an additional clock for DPI. Add support for that
> additional clock.
> 
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>

I'm not convinced that this is right... at all.

 From a fast check of the MT8365 DPI clocks, I can see that the DPI0 clock declares
parent VPLL_DPIX (a fixed clock), but nothing ever has VPLL_DPIX_EN (which is the
GATE clock, enabling output of DPIx VPLL?).

But then, there's even more: no clock ever references the CLK_TOP_DPI0_SEL nor the
CLK_TOP_DPI1_SEL gate, which is a PLL parent selector... in other platforms, that
is muxing through the TVDPLL, but on MT8365 that is LVDSPLL?!

I have many questions now:
* Two PLLs are apparently brought up, but which one is the right one?!
   * Is the LVDS PLL really used for DisplayPort? (dpi0_sel)
   * Is the VPLL_DPIx PLL used for DisplayPort instead? (dpi0_dpi0)
* Why is the LVDSTX_PXL clock using the same PLL as DPI0?!
   * Why is the VPLL_DPIx gate never enabled?
* Are you sure that CLK_MM_DPI0_DPI0's parent shouldn't be dpi0_sel instead?
* Where is DPI1 in this SoC? Why is there a dpi1_sel clock, but no MM clock
   for the DPI1 controller? Is there any DPI1 controller, even?!
   * Why is there a DPI1 MUX, if there's no DPI1 controller?!

Answering all those questions will lead you to the right change, which I believe
to be in the clock drivers, not here in mtk_dpi.c.

Cheers!
Angelo


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
To: amergnat@baylibre.com, "Chun-Kuang Hu" <chunkuang.hu@kernel.org>,
	"Philipp Zabel" <p.zabel@pengutronix.de>,
	"David Airlie" <airlied@gmail.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"Jitao Shi" <jitao.shi@mediatek.com>,
	"Xinlei Lee" <xinlei.lee@mediatek.com>,
	"CK Hu" <ck.hu@mediatek.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Will Deacon" <will@kernel.org>
Cc: devicetree@vger.kernel.org, linux-pwm@vger.kernel.org,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	Fabien Parent <fparent@baylibre.com>,
	linux-mediatek@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 14/18] drm/mediatek: dpi: add support for dpi clock
Date: Tue, 24 Oct 2023 11:12:45 +0200	[thread overview]
Message-ID: <cf25a3cc-6411-45f5-bc7a-6b69cf28c860@collabora.com> (raw)
In-Reply-To: <20231023-display-support-v1-14-5c860ed5c33b@baylibre.com>

Il 23/10/23 16:40, amergnat@baylibre.com ha scritto:
> From: Fabien Parent <fparent@baylibre.com>
> 
> MT8365 requires an additional clock for DPI. Add support for that
> additional clock.
> 
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>

I'm not convinced that this is right... at all.

 From a fast check of the MT8365 DPI clocks, I can see that the DPI0 clock declares
parent VPLL_DPIX (a fixed clock), but nothing ever has VPLL_DPIX_EN (which is the
GATE clock, enabling output of DPIx VPLL?).

But then, there's even more: no clock ever references the CLK_TOP_DPI0_SEL nor the
CLK_TOP_DPI1_SEL gate, which is a PLL parent selector... in other platforms, that
is muxing through the TVDPLL, but on MT8365 that is LVDSPLL?!

I have many questions now:
* Two PLLs are apparently brought up, but which one is the right one?!
   * Is the LVDS PLL really used for DisplayPort? (dpi0_sel)
   * Is the VPLL_DPIx PLL used for DisplayPort instead? (dpi0_dpi0)
* Why is the LVDSTX_PXL clock using the same PLL as DPI0?!
   * Why is the VPLL_DPIx gate never enabled?
* Are you sure that CLK_MM_DPI0_DPI0's parent shouldn't be dpi0_sel instead?
* Where is DPI1 in this SoC? Why is there a dpi1_sel clock, but no MM clock
   for the DPI1 controller? Is there any DPI1 controller, even?!
   * Why is there a DPI1 MUX, if there's no DPI1 controller?!

Answering all those questions will lead you to the right change, which I believe
to be in the clock drivers, not here in mtk_dpi.c.

Cheers!
Angelo


  reply	other threads:[~2023-10-24  9:12 UTC|newest]

Thread overview: 104+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-23 14:40 [PATCH 00/18] Add display support for the MT8365-EVK board Alexandre Mergnat
2023-10-23 14:40 ` Alexandre Mergnat
2023-10-23 14:40 ` Alexandre Mergnat
2023-10-23 14:40 ` [PATCH 01/18] dt-bindings: display: mediatek: aal: add binding for MT8365 SoC Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 17:03   ` Conor Dooley
2023-10-23 17:03     ` Conor Dooley
2023-10-23 17:03     ` Conor Dooley
2023-10-23 14:40 ` [PATCH 02/18] dt-bindings: display: mediatek: ccorr: " Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 17:31   ` Conor Dooley
2023-10-23 17:31     ` Conor Dooley
2023-10-23 17:31     ` Conor Dooley
2023-10-23 17:33     ` Conor Dooley
2023-10-23 17:33       ` Conor Dooley
2023-10-23 17:33       ` Conor Dooley
2023-10-23 14:40 ` [PATCH 03/18] dt-bindings: display: mediatek: color: " Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-24  9:40   ` Chen-Yu Tsai
2023-10-24  9:40     ` Chen-Yu Tsai
2023-10-24  9:40     ` Chen-Yu Tsai
2023-10-24  9:47     ` Alexandre Mergnat
2023-10-24  9:47       ` Alexandre Mergnat
2023-10-24  9:47       ` Alexandre Mergnat
2023-10-23 14:40 ` [PATCH 04/18] dt-bindings: display: mediatek: dither: " Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40 ` [PATCH 05/18] dt-bindings: display: mediatek: dsi: " Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-24 20:30   ` Rob Herring
2023-10-24 20:30     ` Rob Herring
2023-10-24 20:30     ` Rob Herring
2023-10-25  7:35     ` Alexandre Mergnat
2023-10-25  7:35       ` Alexandre Mergnat
2023-10-25  7:35       ` Alexandre Mergnat
2023-10-23 14:40 ` [PATCH 06/18] dt-bindings: display: mediatek: dpi: add power-domains property amergnat
2023-10-23 14:40   ` amergnat
2023-10-23 14:40   ` amergnat
2023-10-23 14:40 ` [PATCH 07/18] dt-bindings: display: mediatek: dpi: add binding for MT8365 amergnat
2023-10-23 14:40   ` amergnat
2023-10-23 14:40   ` amergnat
2023-10-23 14:40 ` [PATCH 08/18] dt-bindings: display: mediatek: gamma: add binding for MT8365 SoC Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40 ` [PATCH 09/18] dt-bindings: display: mediatek: ovl: " Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40 ` [PATCH 10/18] dt-bindings: display: mediatek: rdma: " Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40 ` [PATCH 11/18] dt-bindings: pwm: add power-domains property Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 17:38   ` Conor Dooley
2023-10-23 17:38     ` Conor Dooley
2023-10-23 17:38     ` Conor Dooley
2023-10-24  9:21     ` Alexandre Mergnat
2023-10-24  9:21       ` Alexandre Mergnat
2023-10-24  9:21       ` Alexandre Mergnat
2023-10-23 14:40 ` [PATCH 12/18] dt-bindings: pwm: add binding for mt8365 SoC Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 17:35   ` Conor Dooley
2023-10-23 17:35     ` Conor Dooley
2023-10-23 17:35     ` Conor Dooley
2023-10-23 21:44   ` Uwe Kleine-König
2023-10-23 21:44     ` Uwe Kleine-König
2023-10-23 21:44     ` Uwe Kleine-König
2023-12-06 17:38     ` Uwe Kleine-König
2023-12-06 17:38       ` Uwe Kleine-König
2023-12-06 17:38       ` Uwe Kleine-König
2023-10-24  9:16   ` AngeloGioacchino Del Regno
2023-10-24  9:16     ` AngeloGioacchino Del Regno
2023-10-24  9:16     ` AngeloGioacchino Del Regno
2023-10-23 14:40 ` [PATCH 13/18] drm/mediatek: dsi: Improves the DSI lane setup robustness Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40 ` [PATCH 14/18] drm/mediatek: dpi: add support for dpi clock amergnat
2023-10-23 14:40   ` amergnat
2023-10-23 14:40   ` amergnat
2023-10-24  9:12   ` AngeloGioacchino Del Regno [this message]
2023-10-24  9:12     ` AngeloGioacchino Del Regno
2023-10-24  9:12     ` AngeloGioacchino Del Regno
2024-04-16 14:53     ` Alexandre Mergnat
2024-04-16 14:53       ` Alexandre Mergnat
2023-10-23 14:40 ` [PATCH 15/18] drm/mediatek: add MT8365 SoC support amergnat
2023-10-23 14:40   ` amergnat
2023-10-23 14:40   ` amergnat
2023-10-24  9:20   ` AngeloGioacchino Del Regno
2023-10-24  9:20     ` AngeloGioacchino Del Regno
2023-10-24  9:20     ` AngeloGioacchino Del Regno
2023-10-23 14:40 ` [PATCH 16/18] arm64: defconfig: enable display connector support Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40 ` [PATCH 17/18] arm64: dts: mediatek: add display blocks support for the MT8365 SoC Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40 ` [PATCH 18/18] arm64: dts: mediatek: add display support for mt8365-evk Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat
2023-10-23 14:40   ` Alexandre Mergnat

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cf25a3cc-6411-45f5-bc7a-6b69cf28c860@collabora.com \
    --to=angelogioacchino.delregno@collabora.com \
    --cc=airlied@gmail.com \
    --cc=amergnat@baylibre.com \
    --cc=catalin.marinas@arm.com \
    --cc=chunkuang.hu@kernel.org \
    --cc=ck.hu@mediatek.com \
    --cc=conor+dt@kernel.org \
    --cc=daniel@ffwll.ch \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=fparent@baylibre.com \
    --cc=jitao.shi@mediatek.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=matthias.bgg@gmail.com \
    --cc=p.zabel@pengutronix.de \
    --cc=robh+dt@kernel.org \
    --cc=thierry.reding@gmail.com \
    --cc=u.kleine-koenig@pengutronix.de \
    --cc=will@kernel.org \
    --cc=xinlei.lee@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.