All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Heiko Stübner" <heiko@sntech.de>
To: cristian.ciocaltea@collabora.com,
	Jianfeng Liu <liujianfeng1994@gmail.com>
Cc: airlied@gmail.com, andy.yan@rock-chips.com, conor+dt@kernel.org,
	devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org,
	hjc@rock-chips.com, kernel@collabora.com, krzk+dt@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org,
	liujianfeng1994@gmail.com, maarten.lankhorst@linux.intel.com,
	mripard@kernel.org, robh@kernel.org, simona@ffwll.ch,
	tzimmermann@suse.de
Subject: Re: [PATCH 3/4] arm64: dts: rockchip: Add HDMI1 PHY PLL clock source to VOP2 on RK3588
Date: Tue, 18 Feb 2025 11:00:57 +0100	[thread overview]
Message-ID: <1919367.CQOukoFCf9@diego> (raw)
In-Reply-To: <20250218095216.1253498-1-liujianfeng1994@gmail.com>

Am Dienstag, 18. Februar 2025, 10:52:16 MEZ schrieb Jianfeng Liu:
> Hi Cristian,
> 
> No matter one or two hdmi ports the rk3588 boards have, most of
> devicetrees in mainline kernel only have hdmi0 supported. After applying
> this patch their hdmi0 support is broken.
> 
> So I recommend moving the vop clk part to board level devicetree.
> Then support of hdmi0 won't be broken, and board maintainers can add
> HDMI1 PHY PLL clk when they are adding hdmi1 support. I can add support
> for orangepi 5 max and armsom w3 for reference by other developers.

better, fix the VOP2 driver - both for the existing hdmi0 + this hdmi1
please.

I.e. the clock is optional, and the error you are seeing comes from the

       vop2->pll_hdmiphy1 = devm_clk_get_optional(vop2->dev, "pll_hdmiphy1");
       if (IS_ERR(vop2->pll_hdmiphy1)) {
               drm_err(vop2->drm, "failed to get pll_hdmiphy1\n");
               return PTR_ERR(vop2->pll_hdmiphy1);
       }

part. clk_get_optional is supposed to return NULL when clock-retrieval
causes a ENOENT error. Seemingly going to a clock controller in a disabled
node returns a different error?

So I guess step1, check what error is actually returned.
Step2 check if clk_get_optional need to be adapted or alternatively
catch the error in the vop2 and set the clock to NULL ourself in that case.


hdptxphy0 + hdpxphy1 _are_ valid supplies for the vop, so their reference
should be in the soc-dtsi and the kernel code should just figure things out
correctly. Wiggling with clocks in each board will cause headaches down
the road.


Heiko




WARNING: multiple messages have this Message-ID (diff)
From: "Heiko Stübner" <heiko@sntech.de>
To: cristian.ciocaltea@collabora.com,
	Jianfeng Liu <liujianfeng1994@gmail.com>
Cc: airlied@gmail.com, andy.yan@rock-chips.com, conor+dt@kernel.org,
	devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org,
	hjc@rock-chips.com, kernel@collabora.com, krzk+dt@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org,
	liujianfeng1994@gmail.com, maarten.lankhorst@linux.intel.com,
	mripard@kernel.org, robh@kernel.org, simona@ffwll.ch,
	tzimmermann@suse.de
Subject: Re: [PATCH 3/4] arm64: dts: rockchip: Add HDMI1 PHY PLL clock source to VOP2 on RK3588
Date: Tue, 18 Feb 2025 11:00:57 +0100	[thread overview]
Message-ID: <1919367.CQOukoFCf9@diego> (raw)
In-Reply-To: <20250218095216.1253498-1-liujianfeng1994@gmail.com>

Am Dienstag, 18. Februar 2025, 10:52:16 MEZ schrieb Jianfeng Liu:
> Hi Cristian,
> 
> No matter one or two hdmi ports the rk3588 boards have, most of
> devicetrees in mainline kernel only have hdmi0 supported. After applying
> this patch their hdmi0 support is broken.
> 
> So I recommend moving the vop clk part to board level devicetree.
> Then support of hdmi0 won't be broken, and board maintainers can add
> HDMI1 PHY PLL clk when they are adding hdmi1 support. I can add support
> for orangepi 5 max and armsom w3 for reference by other developers.

better, fix the VOP2 driver - both for the existing hdmi0 + this hdmi1
please.

I.e. the clock is optional, and the error you are seeing comes from the

       vop2->pll_hdmiphy1 = devm_clk_get_optional(vop2->dev, "pll_hdmiphy1");
       if (IS_ERR(vop2->pll_hdmiphy1)) {
               drm_err(vop2->drm, "failed to get pll_hdmiphy1\n");
               return PTR_ERR(vop2->pll_hdmiphy1);
       }

part. clk_get_optional is supposed to return NULL when clock-retrieval
causes a ENOENT error. Seemingly going to a clock controller in a disabled
node returns a different error?

So I guess step1, check what error is actually returned.
Step2 check if clk_get_optional need to be adapted or alternatively
catch the error in the vop2 and set the clock to NULL ourself in that case.


hdptxphy0 + hdpxphy1 _are_ valid supplies for the vop, so their reference
should be in the soc-dtsi and the kernel code should just figure things out
correctly. Wiggling with clocks in each board will cause headaches down
the road.


Heiko



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

  reply	other threads:[~2025-02-18 10:14 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-15  0:55 [PATCH 0/4] Improve Rockchip VOP2 display modes handling on RK3588 HDMI1 Cristian Ciocaltea
2025-02-15  0:55 ` Cristian Ciocaltea
2025-02-15  0:55 ` [PATCH 1/4] drm/rockchip: vop2: Improve " Cristian Ciocaltea
2025-02-15  0:55   ` Cristian Ciocaltea
2025-02-15  0:55 ` [PATCH 2/4] arm64: dts: rockchip: Enable HDMI1 PHY clk provider on RK3588 Cristian Ciocaltea
2025-02-15  0:55   ` Cristian Ciocaltea
2025-02-15  0:55 ` [PATCH 3/4] arm64: dts: rockchip: Add HDMI1 PHY PLL clock source to VOP2 " Cristian Ciocaltea
2025-02-15  0:55   ` Cristian Ciocaltea
2025-02-17  2:44   ` Jianfeng Liu
2025-02-17  2:44     ` Jianfeng Liu
2025-02-17 14:33     ` [PATCH " Heiko Stübner
2025-02-17 14:33       ` Heiko Stübner
2025-02-17 23:33       ` Cristian Ciocaltea
2025-02-17 23:33         ` Cristian Ciocaltea
2025-02-18  3:38         ` Jianfeng Liu
2025-02-18  3:38           ` Jianfeng Liu
2025-02-18  9:52         ` Jianfeng Liu
2025-02-18  9:52           ` Jianfeng Liu
2025-02-18 10:00           ` Heiko Stübner [this message]
2025-02-18 10:00             ` Heiko Stübner
2025-02-18 12:17             ` Jianfeng Liu
2025-02-18 12:17               ` Jianfeng Liu
2025-02-18 14:13               ` Sebastian Reichel
2025-02-18 14:13                 ` Sebastian Reichel
2025-02-18 14:53                 ` Heiko Stübner
2025-02-18 14:53                   ` Heiko Stübner
2025-02-18 16:05                   ` Sebastian Reichel
2025-02-18 16:05                     ` Sebastian Reichel
2025-02-18 23:40                     ` Cristian Ciocaltea
2025-02-18 23:40                       ` Cristian Ciocaltea
2025-02-22  6:10                   ` Johannes Erdfelt
2025-02-22  6:10                     ` Johannes Erdfelt
2025-02-15  0:55 ` [PATCH 4/4] arm64: dts: rockchip: Enable HDMI1 on rk3588-evb1 Cristian Ciocaltea
2025-02-15  0:55   ` Cristian Ciocaltea

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=1919367.CQOukoFCf9@diego \
    --to=heiko@sntech.de \
    --cc=airlied@gmail.com \
    --cc=andy.yan@rock-chips.com \
    --cc=conor+dt@kernel.org \
    --cc=cristian.ciocaltea@collabora.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hjc@rock-chips.com \
    --cc=kernel@collabora.com \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=liujianfeng1994@gmail.com \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=robh@kernel.org \
    --cc=simona@ffwll.ch \
    --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 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.