linux-kernel - Re: [PATCH v2 3/5] drm/rockchip: vop2: Improve display modes handling on RK3588 HDMI0

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a4ex3s23r4k6wehyoaw3aylpcexfrclrxxykjpabhdfne2jgmu@ii6riiiga2zj>
Date: Wed, 11 Dec 2024 18:47:44 +0100
From: Maxime Ripard <mripard@...nel.org>
To: Heiko Stübner <heiko@...ech.de>
Cc: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>, 
	Sandy Huang <hjc@...k-chips.com>, Andy Yan <andy.yan@...k-chips.com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Thomas Zimmermann <tzimmermann@...e.de>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, kernel@...labora.com, 
	dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org, FUKAUMI Naoki <naoki@...xa.com>
Subject: Re: [PATCH v2 3/5] drm/rockchip: vop2: Improve display modes
 handling on RK3588 HDMI0

On Wed, Dec 11, 2024 at 06:23:03PM +0100, Heiko Stübner wrote:
> Am Mittwoch, 11. Dezember 2024, 18:07:57 CET schrieb Maxime Ripard:
> > On Wed, Dec 11, 2024 at 12:15:07PM +0200, Cristian Ciocaltea wrote:
> > > The RK3588 specific implementation is currently quite limited in terms
> > > of handling the full range of display modes supported by the connected
> > > screens, e.g. 2560x1440@...z, 2048x1152@...z, 1024x768@...z are just a
> > > few of them.
> > > 
> > > Additionally, it doesn't cope well with non-integer refresh rates like
> > > 59.94, 29.97, 23.98, etc.
> > > 
> > > Make use of HDMI0 PHY PLL as a more accurate DCLK source to handle
> > > all display modes up to 4K@...z.
> > > 
> > > Tested-by: FUKAUMI Naoki <naoki@...xa.com>
> > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
> > > ---
> > >  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 34 ++++++++++++++++++++++++++++
> > >  1 file changed, 34 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > index 8b2f53ffefdbf1cc8737b3a86e630a03a7fd9348..393fe6aa170aaee9663c4a6d98c1cd6a5ef79392 100644
> > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > @@ -158,6 +158,7 @@ struct vop2_video_port {
> > >  	struct drm_crtc crtc;
> > >  	struct vop2 *vop2;
> > >  	struct clk *dclk;
> > > +	struct clk *dclk_src;
> > >  	unsigned int id;
> > >  	const struct vop2_video_port_data *data;
> > >  
> > > @@ -212,6 +213,7 @@ struct vop2 {
> > >  	struct clk *hclk;
> > >  	struct clk *aclk;
> > >  	struct clk *pclk;
> > > +	struct clk *pll_hdmiphy0;
> > >  
> > >  	/* optional internal rgb encoder */
> > >  	struct rockchip_rgb *rgb;
> > > @@ -220,6 +222,8 @@ struct vop2 {
> > >  	struct vop2_win win[];
> > >  };
> > >  
> > > +#define VOP2_MAX_DCLK_RATE		600000 /* kHz */
> > > +
> > >  #define vop2_output_if_is_hdmi(x)	((x) == ROCKCHIP_VOP2_EP_HDMI0 || \
> > >  					 (x) == ROCKCHIP_VOP2_EP_HDMI1)
> > >  
> > > @@ -1033,6 +1037,9 @@ static void vop2_crtc_atomic_disable(struct drm_crtc *crtc,
> > >  
> > >  	vop2_crtc_disable_irq(vp, VP_INT_DSP_HOLD_VALID);
> > >  
> > > +	if (vp->dclk_src)
> > > +		clk_set_parent(vp->dclk, vp->dclk_src);
> > > +
> > >  	clk_disable_unprepare(vp->dclk);
> > >  
> > >  	vop2->enable_count--;
> > > @@ -2049,6 +2056,27 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
> > >  
> > >  	vop2_vp_write(vp, RK3568_VP_MIPI_CTRL, 0);
> > >  
> > > +	/*
> > > +	 * Switch to HDMI PHY PLL as DCLK source for display modes up
> > > +	 * to 4K@...z, if available, otherwise keep using the system CRU.
> > > +	 */
> > > +	if (vop2->pll_hdmiphy0 && mode->crtc_clock <= VOP2_MAX_DCLK_RATE) {
> > > +		drm_for_each_encoder_mask(encoder, crtc->dev, crtc_state->encoder_mask) {
> > > +			struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder);
> > > +
> > > +			if (rkencoder->crtc_endpoint_id == ROCKCHIP_VOP2_EP_HDMI0) {
> > > +				if (!vp->dclk_src)
> > > +					vp->dclk_src = clk_get_parent(vp->dclk);
> > > +
> > > +				ret = clk_set_parent(vp->dclk, vop2->pll_hdmiphy0);
> > > +				if (ret < 0)
> > > +					drm_warn(vop2->drm,
> > > +						 "Could not switch to HDMI0 PHY PLL: %d\n", ret);
> > > +				break;
> > > +			}
> > > +		}
> > > +	}
> > > +
> > 
> > It seems pretty fragile to do it at atomic_enable time, even more so
> > since you don't lock the parent either.
> > 
> > Any reason not to do it in the DRM or clock driver probe, and make sure
> > you never change the parent somehow?
> 
> On rk3588 we have 3 dclk_s and 2 hdmi controllers. Each video-port can
> use the clock generated from either the hdmi0phy or hdmi1phy, depending
> on which hdmi-controller it uses.
> 
> So you actually need to know which vpX will output to which hdmiY to then
> reparent that dclk to the hdmiphy output.

The Rockchip nomenclature isn't super obvious to me, sorry. Is there a
datasheet for this somewhere? Also, does this vpX -> HDMI-Y mapping need
to be dynamic?

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)