dri-devel Archive mirror
 help / color / mirror / Atom feed
From: Alex Bee <knaerzche@gmail.com>
To: "Heiko Stübner" <heiko@sntech.de>, "Andy Yan" <andy.yan@rock-chips.com>
Cc: dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-clk@vger.kernel.org, sebastian.reichel@collabora.com,
	Sandy Huang <hjc@rock-chips.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Daniel Vetter <daniel@ffwll.ch>,
	Maxime Ripard <mripard@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>, Rob Herring <robh@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@gmail.com>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>
Subject: Re: [PATCH v2 0/7] Add DSI support for RK3128
Date: Thu, 9 May 2024 14:43:34 +0200	[thread overview]
Message-ID: <1190cfb6-e2d1-4910-ad57-f7566343ff19@gmail.com> (raw)
In-Reply-To: <38423821.XM6RcZxFsP@diego>

Hi Heiko

Am 09.05.24 um 14:21 schrieb Heiko Stübner:
> Hi Alex,
>
> Am Donnerstag, 9. Mai 2024, 14:07:08 CEST schrieb Alex Bee:
>> This series aims to add support for the DesignWare MIPI DSI controller and
>> the Innoslicon D-PHY found in RK3128 SoCs. The code additions are rather
>> tiny: It only need some code in the Rockchip dw-mipi-dsi glue layer for
>> this SoC, add support for an additional clock and do some changes in the
>> SoC's clock driver. Support for the phy was already added when the
>> Innosilicon D-PHY driver was initially submitted. I tested it with a
>> 800x1280 DSI panel where all 4 lanes that are supported are used.
>>
>> changes in v2:
>>    To improve power-efficiency when the DSI controller is not in use, I
>>    dropped the patch which made hclk_vio_h2p a critical clock and instead
>>    added support for an AHB clock to the DSI controller driver and updated
>>    the bindings and the addition to the SoC DT accordingly.
> The naming already suggests that hclk_vio_h2p is not a clock-part of
> the actual dsi controller, but more an internal thing inside the clock
> controller.
>
> At least naming and perceived functionality would suggest a chain of
> 	hclk_vio -> hclk_vio_h2p -> pclk_mipi
I personally wouldn't give to much on naming when it comes to Rockchip
CRUs. Actually looking at "Fig. 2-5 Chip Clock Architecture Diagram 4" of
RK312x its:


... -> hclk_vio

                    -> hclk_h2p (clock in question)
                    -> pclk_mipi (DSI APB clock)
                    -> hclk_rga
                    -> hclk_vop
                    ....

Also there is no other display output path (HDMI, LVDS) which requires this
clock to be enabled. They all work when it's disabled. That really makes me
think it's just the AHB clock line for the DSI controller. Maybe Andy can
share some details?

Alex

> In any case, I really don't see hclk_vio_h2p to be in the realm of the
> actual DSI controller, but more a part of clock-controller / interconnect.
> Similar to the NIU clocks for the interconnect.
>
> rk3588 actually tries to implement this already and while the
> gate-link clocks are described as "recent", I think this definitly the same
> concept used a most/all older Rockchip SoCs, just nobody cared about that
> till now ;-) [0] .
>
> So TL;DR I'd really prefer to not leak CRU-details into the DSI controller.
>
>
> Heiko
>
> [0] Which reminds me that I should look at Sebastian's make GATE-LINK
> actually-work-patch.
>
>
>
>

  reply	other threads:[~2024-05-09 12:43 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-09 12:07 [PATCH v2 0/7] Add DSI support for RK3128 Alex Bee
2024-05-09 12:07 ` [PATCH v2 1/7] dt-bindings: display: rockchip, dw-mipi-dsi: Document RK3128 DSI Alex Bee
2024-05-09 12:07 ` [PATCH v2 2/7] dt-bindings: clock: rk3128: Add PCLK_MIPIPHY Alex Bee
2024-05-09 12:07 ` [PATCH v2 3/7] clk: rockchip: rk3128: Export PCLK_MIPIPHY Alex Bee
2024-05-09 12:07 ` [PATCH v2 4/7] drm/rockchip: dsi: Support optional AHB clock Alex Bee
2024-05-09 12:07 ` [PATCH v2 5/7] drm/rockchip: dsi: Add support for RK3128 Alex Bee
2024-05-09 12:07 ` [PATCH v2 6/7] ARM: dts: rockchip: Add D-PHY " Alex Bee
2024-05-09 12:07 ` [PATCH v2 7/7] ARM: dts: rockchip: Add DSI " Alex Bee
2024-05-09 12:21 ` [PATCH v2 0/7] Add DSI support " Heiko Stübner
2024-05-09 12:43   ` Alex Bee [this message]
2024-05-09 13:12     ` Alex Bee

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=1190cfb6-e2d1-4910-ad57-f7566343ff19@gmail.com \
    --to=knaerzche@gmail.com \
    --cc=airlied@gmail.com \
    --cc=andy.yan@rock-chips.com \
    --cc=conor+dt@kernel.org \
    --cc=daniel@ffwll.ch \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=heiko@sntech.de \
    --cc=hjc@rock-chips.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=robh@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=sebastian.reichel@collabora.com \
    --cc=tzimmermann@suse.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).