Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Message-ID: <cf25a3cc-6411-45f5-bc7a-6b69cf28c860@collabora.com>
Date:   Tue, 24 Oct 2023 11:12:45 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 14/18] drm/mediatek: dpi: add support for dpi clock
Content-Language: en-US
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>,
        =?UTF-8?Q?Uwe_Kleine-K=C3=B6nig?= <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>
References: <20231023-display-support-v1-0-5c860ed5c33b@baylibre.com>
 <20231023-display-support-v1-14-5c860ed5c33b@baylibre.com>
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>
In-Reply-To: <20231023-display-support-v1-14-5c860ed5c33b@baylibre.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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