Since OLDI consumes DSS VP clock directly as serial clock, certain
checks cannot be performed in tidss driver which should be checked
in oldi driver. Add check for mode clock and set the curr_max_pclk
field for tidss in case the VP is OLDI.

Fixes: 7246e0929945 ("drm/tidss: Add OLDI bridge support")
Reviewed-by: Devarsh Thakkar <devarsht@ti.com>
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
---
 drivers/gpu/drm/tidss/tidss_oldi.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/tidss/tidss_oldi.c b/drivers/gpu/drm/tidss/tidss_oldi.c
index 63e07c8edeaa..486e7373531b 100644
--- a/drivers/gpu/drm/tidss/tidss_oldi.c
+++ b/drivers/gpu/drm/tidss/tidss_oldi.c
@@ -309,6 +309,29 @@ static u32 *tidss_oldi_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
 	return input_fmts;
 }
 
+static int tidss_oldi_atomic_check(struct drm_bridge *bridge,
+				   struct drm_bridge_state *bridge_state,
+				   struct drm_crtc_state *crtc_state,
+				   struct drm_connector_state *conn_state)
+{
+	struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
+	struct drm_display_mode *adjusted_mode;
+	unsigned long round_clock;
+
+	adjusted_mode = &crtc_state->adjusted_mode;
+
+	if (adjusted_mode->clock > oldi->tidss->curr_max_pclk[oldi->parent_vp]) {
+		round_clock = clk_round_rate(oldi->serial, adjusted_mode->clock * 7 * 1000);
+
+		if (dispc_pclk_diff(adjusted_mode->clock * 7 * 1000, round_clock) > 5)
+			return -EINVAL;
+
+		oldi->tidss->curr_max_pclk[oldi->parent_vp] = round_clock;
+	}
+
+	return 0;
+}
+
 static const struct drm_bridge_funcs tidss_oldi_bridge_funcs = {
 	.attach	= tidss_oldi_bridge_attach,
 	.atomic_pre_enable = tidss_oldi_atomic_pre_enable,
@@ -317,6 +340,7 @@ static const struct drm_bridge_funcs tidss_oldi_bridge_funcs = {
 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
 	.atomic_reset = drm_atomic_helper_bridge_reset,
+	.atomic_check = tidss_oldi_atomic_check,
 };
 
 static int get_oldi_mode(struct device_node *oldi_tx, int *companion_instance)
-- 
2.34.1

Hi,

On 04/07/2025 12:48, Jayesh Choudhary wrote:
> Since OLDI consumes DSS VP clock directly as serial clock, certain

I don't think that's true. Although depends what you mean with VP clock.
I think VP clock is whatever pclk goes into the DSS.

OLDI gets a serial clock from the PLL. That same clock also goes through
a divider, and then goes into DSS as the VP clock.

And the problem is, if I'm not mistaken, that the DSS can't
clk_set_rate()/clk_round_rate() to the VP clock, as the divider won't
propagate the rate change to the PLL. Another problem is that the OLDI
driver is controlling the clock, so DSS shouldn't.

As a side topic/question, I wonder if the divider should propagate the
rate change, and DSS should be in control of the clock...

> checks cannot be performed in tidss driver which should be checked

"certain checks"? It's not good to be very vague in the patch descriptions.

> in oldi driver. Add check for mode clock and set the curr_max_pclk
> field for tidss in case the VP is OLDI.

The check part makes sense after reading the description, but the
"curr_max_pclk" part doesn't. I think it needs a few extra words. But
also, the code is the same as in DSS, isn't it... Maybe OLDI can just
call check_pixel_clock() (or whatever it is named, perhaps with the
changes I mentioned in the previous mail).

 Tomi

> Fixes: 7246e0929945 ("drm/tidss: Add OLDI bridge support")
> Reviewed-by: Devarsh Thakkar <devarsht@ti.com>
> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
> ---
>  drivers/gpu/drm/tidss/tidss_oldi.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tidss/tidss_oldi.c b/drivers/gpu/drm/tidss/tidss_oldi.c
> index 63e07c8edeaa..486e7373531b 100644
> --- a/drivers/gpu/drm/tidss/tidss_oldi.c
> +++ b/drivers/gpu/drm/tidss/tidss_oldi.c
> @@ -309,6 +309,29 @@ static u32 *tidss_oldi_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
>  	return input_fmts;
>  }
>  
> +static int tidss_oldi_atomic_check(struct drm_bridge *bridge,
> +				   struct drm_bridge_state *bridge_state,
> +				   struct drm_crtc_state *crtc_state,
> +				   struct drm_connector_state *conn_state)
> +{
> +	struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
> +	struct drm_display_mode *adjusted_mode;
> +	unsigned long round_clock;
> +
> +	adjusted_mode = &crtc_state->adjusted_mode;
> +
> +	if (adjusted_mode->clock > oldi->tidss->curr_max_pclk[oldi->parent_vp]) {
> +		round_clock = clk_round_rate(oldi->serial, adjusted_mode->clock * 7 * 1000);
> +
> +		if (dispc_pclk_diff(adjusted_mode->clock * 7 * 1000, round_clock) > 5)
> +			return -EINVAL;
> +
> +		oldi->tidss->curr_max_pclk[oldi->parent_vp] = round_clock;
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct drm_bridge_funcs tidss_oldi_bridge_funcs = {
>  	.attach	= tidss_oldi_bridge_attach,
>  	.atomic_pre_enable = tidss_oldi_atomic_pre_enable,
> @@ -317,6 +340,7 @@ static const struct drm_bridge_funcs tidss_oldi_bridge_funcs = {
>  	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
>  	.atomic_reset = drm_atomic_helper_bridge_reset,
> +	.atomic_check = tidss_oldi_atomic_check,
>  };
>  
>  static int get_oldi_mode(struct device_node *oldi_tx, int *companion_instance)