Hi,
On 28/10/2025 05:39, Swamil Jain wrote:
> From: Jayesh Choudhary <[email protected]>
>
> After integrating OLDI support[0], it is necessary to identify which VP
> instances use OLDI, since the OLDI driver owns the video port clock
> (as a serial clock). Clock operations on these VPs must be delegated to
> the OLDI driver, not handled by the TIDSS driver. This issue also
> emerged in upstream discussions when DSI-related clock management was
> attempted in the TIDSS driver[1].
>
> To address this, add an 'is_ext_vp_clk' array to the 'tidss_device'
> structure, marking a VP as 'true' during 'tidss_oldi_init()' and as
> 'false' during 'tidss_oldi_deinit()'. TIDSS then uses 'is_ext_vp_clk'
> to skip clock validation checks in 'dispc_vp_mode_valid()' for VPs
> under OLDI control.
>
> Since OLDI uses the DSS VP clock directly as a serial interface and
> manages its own rate, mode validation should be implemented in the OLDI
> bridge's 'mode_valid' hook. This patch adds that logic, ensuring proper
> delegation and avoiding spurious clock handling in the TIDSS driver.
>
> [0]:
> https://lore.kernel.org/all/[email protected]/
> [1]: https://lore.kernel.org/all/[email protected]/
>
> Fixes: 7246e0929945 ("drm/tidss: Add OLDI bridge support")
> Tested-by: Michael Walle <[email protected]>
> Reviewed-by: Devarsh Thakkar <[email protected]>
> Signed-off-by: Jayesh Choudhary <[email protected]>
> Signed-off-by: Swamil Jain <[email protected]>
> ---
> drivers/gpu/drm/tidss/tidss_dispc.c | 2 ++
> drivers/gpu/drm/tidss/tidss_drv.h | 2 ++
> drivers/gpu/drm/tidss/tidss_oldi.c | 21 +++++++++++++++++++++
> 3 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c
> b/drivers/gpu/drm/tidss/tidss_dispc.c
> index 07731b02490f..0c3337a7b163 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
> @@ -1315,6 +1315,8 @@ static int check_pixel_clock(struct dispc_device *dispc,
> {
> unsigned long round_clock;
>
> + if (dispc->tidss->is_ext_vp_clk[hw_videoport])
> + return 0;
Add empty line here. Also, add a short comment what this check is about.
> round_clock = clk_round_rate(dispc->vp_clk[hw_videoport], clock);
> /*
> * To keep the check consistent with dispc_vp_set_clk_rate(), we
> diff --git a/drivers/gpu/drm/tidss/tidss_drv.h
> b/drivers/gpu/drm/tidss/tidss_drv.h
> index 84454a4855d1..e1c1f41d8b4b 100644
> --- a/drivers/gpu/drm/tidss/tidss_drv.h
> +++ b/drivers/gpu/drm/tidss/tidss_drv.h
> @@ -24,6 +24,8 @@ struct tidss_device {
>
> const struct dispc_features *feat;
> struct dispc_device *dispc;
> + bool is_ext_vp_clk[TIDSS_MAX_PORTS];
> +
>
> unsigned int num_crtcs;
> struct drm_crtc *crtcs[TIDSS_MAX_PORTS];
> diff --git a/drivers/gpu/drm/tidss/tidss_oldi.c
> b/drivers/gpu/drm/tidss/tidss_oldi.c
> index 7688251beba2..d1a5fdac93ff 100644
> --- a/drivers/gpu/drm/tidss/tidss_oldi.c
> +++ b/drivers/gpu/drm/tidss/tidss_oldi.c
> @@ -309,6 +309,24 @@ static u32 *tidss_oldi_atomic_get_input_bus_fmts(struct
> drm_bridge *bridge,
> return input_fmts;
> }
>
> +static enum drm_mode_status
> +tidss_oldi_mode_valid(struct drm_bridge *bridge,
> + const struct drm_display_info *info,
> + const struct drm_display_mode *mode)
> +{
> + struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
> + unsigned long round_clock;
> +
> + round_clock = clk_round_rate(oldi->serial, mode->clock * 7 * 1000);
> + /*
> + * To keep the check consistent with dispc_vp_set_clk_rate(),
> + * we use the same 5% check here.
> + */
> + if (dispc_pclk_diff(mode->clock * 7 * 1000, round_clock) > 5)
> + return -EINVAL;
Add an empty line here.
Reviewed-by: Tomi Valkeinen <[email protected]>
> + 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 +335,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,
> + .mode_valid = tidss_oldi_mode_valid,
> };
>
> static int get_oldi_mode(struct device_node *oldi_tx, int
> *companion_instance)
> @@ -430,6 +449,7 @@ void tidss_oldi_deinit(struct tidss_device *tidss)
> for (int i = 0; i < tidss->num_oldis; i++) {
> if (tidss->oldis[i]) {
> drm_bridge_remove(&tidss->oldis[i]->bridge);
> + tidss->is_ext_vp_clk[tidss->oldis[i]->parent_vp] =
> false;
> tidss->oldis[i] = NULL;
> }
> }
> @@ -580,6 +600,7 @@ int tidss_oldi_init(struct tidss_device *tidss)
> oldi->bridge.timings = &default_tidss_oldi_timings;
>
> tidss->oldis[tidss->num_oldis++] = oldi;
> + tidss->is_ext_vp_clk[oldi->parent_vp] = true;
> oldi->tidss = tidss;
>
> drm_bridge_add(&oldi->bridge);
