| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20241217-uppish-sapphire-dinosaur-4c40a2@houat>
Date: Tue, 17 Dec 2024 15:07:47 +0100
From: Maxime Ripard <mripard@...nel.org>
To: Liu Ying <victor.liu@....com>
Cc: imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org,
dri-devel@...ts.freedesktop.org, shawnguo@...nel.org, s.hauer@...gutronix.de,
kernel@...gutronix.de, festevam@...il.com, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, catalin.marinas@....com, will@...nel.org, abelvesa@...nel.org,
peng.fan@....com, mturquette@...libre.com, sboyd@...nel.org,
andrzej.hajda@...el.com, neil.armstrong@...aro.org, rfoss@...nel.org,
Laurent.pinchart@...asonboard.com, jonas@...boo.se, jernej.skrabec@...il.com,
maarten.lankhorst@...ux.intel.com, tzimmermann@...e.de, airlied@...il.com, simona@...ll.ch,
quic_bjorande@...cinc.com, geert+renesas@...der.be, dmitry.baryshkov@...aro.org,
arnd@...db.de, nfraprado@...labora.com, marex@...x.de
Subject: Re: [PATCH v7 0/7] Add ITE IT6263 LVDS to HDMI converter support
Hi,
Thanks for the description, I have several questions here.
On Thu, Nov 14, 2024 at 02:57:52PM +0800, Liu Ying wrote:
> This patch series aims to add ITE IT6263 LVDS to HDMI converter on
> i.MX8MP EVK.
>
> Since IT6263 DT binding and driver were picked up from v5 and landed
> in drm-misc, this patch series contains patches almost all i.MX8MP
> SoC/platform specific.
>
> Patch 1 is a preparation patch to allow display mode of an existing
> panel to pass the added mode validation logic in patch 3.
>
> Patch 2 is a preparation patch to drop CLK_SET_RATE_PARENT flag for
> media_disp{1,2}_pix clocks. Patch 5 depends on patch 2.
>
> Patch 3 allows i.MX8MP LVDS Display Bridge(LDB) bridge driver to find
> the next non-panel bridge, that is the IT6263 in this case.
>
> Patch 4 adds mode validation logic to i.MX8MP LDB bridge driver against
> "ldb" clock so that it can filter out unsupported display modes read
> from EDID.
>
> Patch 5 adds mode validation logic to i.MX8MP LDB bridge driver against
> "pix" clock so that it can filter out display modes which are not
> supported by pixel clock tree.
>
> Patch 6 adds DT overlays to support NXP adapter cards[1][2] with IT6263
> populated.
>
> Patch 7 enables the IT6263 bridge driver in defconfig.
>
> Note that patch 3 and 4 depend on patch[3] in shawnguo/imx/fixes.
>
> Since this patch series is related to another one[4] authored by Marek,
> Maxime asked for a proper description[5] about the exact problem.
>
> Admittedly, it's a bit complicated. Here, I'm trying to do so and explain
> a bit more.
>
> [ Description ]
> It's a clock problem about shared i.MX8MP video PLL between MIPI DSI and
> LVDS display pipelines. The pipelines are driven by separate DRM driver
> instances, hence there is no way to negotiate a dynamically changeable
> PLL rate to satisfy both of them. The only solution is to assign a
> sensible/unchangeable clock rate for the PLL in DT.
>
> Admittedly, sys_pll3_out can be another clock source to derive pixel clock
> for i.MX8MP MIPI DSI display pipeline if a particalur i.MX8MP platform
> doesn't use audio(sys_pll3_out is supposed to derive audio AXI clock running
> at nominal 600MHz). However, for i.MX8MP platforms with audio features,
> the shared video PLL case has to be handled and it determines that the above
> solution(unchangeable PLL rate assigned in DT) has to be used no matter
> sys_pll3_out is for display or audio, because the separate DRM driver
> instances really don't know if they are sharing the video PLL or not.
>
> [[ i.MX8MP Display Hardware ]]
> i.MX8MP SoC supports three display pipelines:
>
> ----------------------------- ------------------------
> | imx8mp_media_disp_pix_sels | | imx8mp_media_ldb_sels |
> ----------------------------- ------------------------
> | osc_24m (fixed 24MHz) | | osc_24m (fixed 24MHz) |
> |*-video_pll1_out (video) | | sys_pll2_333m (sys) |
> | audio_pll2_out (audio) | | sys_pll2_100m (sys) |
> | audio_pll1_out (audio) | | -sys_pll1_800m (sys) |
> | -sys_pll1_800m (sys) | | -sys_pll2_1000m (sys) |
> | -sys_pll2_1000m (sys) | | clk_ext2 (external) |
> | sys_pll3_out (audio ?) | | audio_pll2_out (audio)|
> | clk_ext4 (external) | |*-video_pll1_out (video)|
> ----------------------------- ------------------------
> || |
> ----------------------------- ------------------------
> | media_disp{1,2}_pix | | media_ldb |
> ----------------------------- mux+div ------------------------ mux+div
> || |
> ----------------------------- ------------------------
> | media_disp{1,2}_pix_root_clk| | media_ldb_root_clk |
> ----------------------------- gate ------------------------ gate
> || | (LVDS serial clock)
> || V
> || (Disp2 Pclk) -------- ------------------
> | ------------> | LCDIF2 | -> | LDB | -> panel/bridge
> | -------- ------------------
> | (Disp1 Pclk) -------- ------------------
> -------------> | LCDIF1 | -> | Samsung MIPI DSI | -> panel/bridge
> -------- ------------------
> -------- ------------------ ----------
> | LCDIF3 | -> | Synopsys HDMI TX | -> | HDMI PHY |
> -------- ------------------ | + |
> ^ | PLL |
> | ----------
> | (Disp3 pclk) | |
> -------------------------------------- |
> V
> panel/bridge
>
> * video_pll1_out is supposed to be used by video outputs.
>
> - LCDIF2 + LDB can only use the *same* video_pll1_out, sys_pll1_800m or
> sys_pll2_1000m.
>
> [[ i.MX8MP Display Drivers ]]
> LCDIF: drivers/gpu/drm/mxsfb/lcdif_*.c
> Three LCDIFv3 display controllers are driven by three imx-lcdif DRM instances
> separately.
>
> LDB: drivers/gpu/drm/bridge/fsl-ldb.c
>
> Samsung MIPI DSI: drivers/gpu/drm/bridge/samsung-dsim.c
>
> Synopsys HDMI TX: drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>
> [[ Problem - Shared Video PLL Between Samsung MIPI DSI and LDB ]]
> osc_24m, audio_pll*, sys_pll* and clk_ext* are not for video outputs,
> because:
> a. Aparently, osc_24m runs at fixed 24MHz which is too low for most displays.
> b. Audio subsystem may consume all audio_pll*.
> c. sys_pll* are system clocks which are supposed to run at fixed typical
> rates, e.g., sys_pll2_1000m runs at 1000MHz.
> d. sys_pll3_out is supposed to derive audio AXI clock running at nominal
> 600MHz(i.MX8MP data sheet specifies the rate), see NXP downstream kernel:
> https://github.com/nxp-imx/linux-imx/blob/lf-6.6.y/arch/arm64/boot/dts/freescale/imx8mp-evk-ndm.dts#L19
> https://github.com/nxp-imx/linux-imx/blob/lf-6.6.y/arch/arm64/boot/dts/freescale/imx8mp-ddr4-evk.dts#L25
Is there any downside to using any of these clocks, aside from the fact
that their rate must not change?
Also, if they can't change their rate, why do they have
CLK_SET_RATE_PARENT (sys_pll* in particular) ?
> e. clk_ext* are external clocks without known capabilities.
>
> So, the only eligible video_pll1_out is supposed to be shared between LDB
> and Samsung MIPI DSI in the two separate display pipelines if sys_pll3_out
> is already used to derive the audio AXI clock.
>
> With the shared video_pll1_out, drivers for the two display pipelines cannot
> change the PLL clock rate in runtime, since the pipelines are driven by two
> DRM driver instances.
What is the typicall frequency on those pipelines? Could setting the PLL
high enough that any frequency required by any of these pipelines can be
accomodated through a divider work?
Something like you run the PLL at 594MHz, and then most HDMI frequencies
can be reached by a 1, 2 or 4 divider.
> [[ Solution ]]
> Assign the PLL clock source(s) and the PLL clock rate(s) in DT. Disallow
> display drivers to change the PLL clock source(s) or rate(s) in runtime
> including LCDIF driver and bridge drivers. With sensible PLL clock rate(s),
> typical display modes like 1920x1080@60 can be supported if external HDMI
> bridges are connected, and panel display modes can be too. Also the unneeded
> CLK_SET_RATE_PARENT flag can be dropped for media_disp{1,2}_pix clocks.
> If needed, bridge drivers just call clk_round_rate() to validate clocks so
> that unsupported display modes can be filtered out. Although the
> unchangeable PLL clock rate disallows more potential display modes, the
> solution works for single/dual/triple display pipelines(OFC, hardware designers
> should pick panel/bridge display devices carefully first by considering clock
> resources).
I think it's a reasonable idea, if not for the hardcode-it it DT stuff.
If we can manage to have a fixed setup work ok for all display use
cases, why would it be in DT? The clock driver seems like a much better
choice to me.
Maxime
Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)
Powered by blists - more mailing lists