Re: [PATCH v2 03/18] drm/tidss: Adjust the pclk based on the HW capabilities

["Michael Walle" <mwalle@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 4578 bytes --]

Hi Tomi,

While testing Aardhya's OLDI support patches [1], I've noticed that
the resulting LVDS clock is wrong if this patch is applied.

> In practice, with the current K3 SoCs, the display PLL is capable of
> producing very exact clocks, so most likely the rounded rate is the same
> as the original one.

This is now what I'm seeing. Most SoCs have that fixed clock thingy
for (some?) VPs, e.g. [2]. And clk_round_rate() will return the
fixed clock rate for this clock, which will then result in an LVDS
clock which is way off.

I'm testing on an AM67A (J722S) and I've backported some of the
patches as well as dtsi fragmets from downstream. Thus, it might be
as well the case that the fixed-factor-clock node is wrong here.
OTOH other K3 SoCs do this in mainline as well.

>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/gpu/drm/tidss/tidss_crtc.c  | 23 +++++++++++++++++++----
>  drivers/gpu/drm/tidss/tidss_dispc.c |  6 ++++++
>  drivers/gpu/drm/tidss/tidss_dispc.h |  2 ++
>  3 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c b/drivers/gpu/drm/tidss/tidss_crtc.c
> index 1604eca265ef..6c3967f70510 100644
> --- a/drivers/gpu/drm/tidss/tidss_crtc.c
> +++ b/drivers/gpu/drm/tidss/tidss_crtc.c
> @@ -91,7 +91,7 @@ static int tidss_crtc_atomic_check(struct drm_crtc *crtc,
>  	struct dispc_device *dispc = tidss->dispc;
>  	struct tidss_crtc *tcrtc = to_tidss_crtc(crtc);
>  	u32 hw_videoport = tcrtc->hw_videoport;
> -	const struct drm_display_mode *mode;
> +	struct drm_display_mode *adjusted_mode;
>  	enum drm_mode_status ok;
>  
>  	dev_dbg(ddev->dev, "%s\n", __func__);
> @@ -99,12 +99,27 @@ static int tidss_crtc_atomic_check(struct drm_crtc *crtc,
>  	if (!crtc_state->enable)
>  		return 0;
>  
> -	mode = &crtc_state->adjusted_mode;
> +	adjusted_mode = &crtc_state->adjusted_mode;

Here, adjusted_mode->clock is still the correct pixel clock.

> -	ok = dispc_vp_mode_valid(dispc, hw_videoport, mode);
> +	if (drm_atomic_crtc_needs_modeset(crtc_state)) {
> +		long rate;
> +
> +		rate = dispc_vp_round_clk_rate(tidss->dispc,
> +					       tcrtc->hw_videoport,
> +					       adjusted_mode->clock * 1000);
> +		if (rate < 0)
> +			return -EINVAL;
> +
> +		adjusted_mode->clock = rate / 1000;

While after this statement, adjusted_mode->clock is 300MHz in my
case (the VP1 clock seems to be 2.1GHz, divided by 7).

-michael

[1] https://lore.kernel.org/all/20250525151721.567042-1-aradhya.bhatia@linux.dev/
[2] https://elixir.bootlin.com/linux/v6.15/source/arch/arm64/boot/dts/ti/k3-am62.dtsi#L110

> +
> +		drm_mode_set_crtcinfo(adjusted_mode, 0);
> +	}
> +
> +	ok = dispc_vp_mode_valid(dispc, hw_videoport, adjusted_mode);
>  	if (ok != MODE_OK) {
>  		dev_dbg(ddev->dev, "%s: bad mode: %ux%u pclk %u kHz\n",
> -			__func__, mode->hdisplay, mode->vdisplay, mode->clock);
> +			__func__, adjusted_mode->hdisplay,
> +			adjusted_mode->vdisplay, adjusted_mode->clock); >  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
> index a5107f2732b1..3930fb7f03c2 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
> @@ -1318,6 +1318,12 @@ unsigned int dispc_pclk_diff(unsigned long rate, unsigned long real_rate)
>  	return (unsigned int)(abs(((rr - r) * 100) / r));
>  }
>  
> +long dispc_vp_round_clk_rate(struct dispc_device *dispc, u32 hw_videoport,
> +			     unsigned long rate)
> +{
> +	return clk_round_rate(dispc->vp_clk[hw_videoport], rate);
> +}
> +
>  int dispc_vp_set_clk_rate(struct dispc_device *dispc, u32 hw_videoport,
>  			  unsigned long rate)
>  {
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
> index c31b477a18b0..d4c335e918fb 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.h
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.h
> @@ -120,6 +120,8 @@ enum drm_mode_status dispc_vp_mode_valid(struct dispc_device *dispc,
>  					 const struct drm_display_mode *mode);
>  int dispc_vp_enable_clk(struct dispc_device *dispc, u32 hw_videoport);
>  void dispc_vp_disable_clk(struct dispc_device *dispc, u32 hw_videoport);
> +long dispc_vp_round_clk_rate(struct dispc_device *dispc, u32 hw_videoport,
> +			     unsigned long rate);
>  int dispc_vp_set_clk_rate(struct dispc_device *dispc, u32 hw_videoport,
>  			  unsigned long rate);
>  void dispc_vp_setup(struct dispc_device *dispc, u32 hw_videoport,


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy