Re: [PATCH 06/10] drm/tidss: Add mechanism to detect DPI output

[Swamil Jain <s-jain1@ti.com>]

On 9/3/26 17:39, Tomi Valkeinen wrote:
> There are situations where the driver needs to know if the output is
> going to the DPI output or not. There is no trivial way to get this
> information, as there is no "DPI bridge". We can only find this out in
> reverse: check if the output is NOT DPI, and if that is negative, then
> it must be DPI.
> 
> At the moment we have two non-DPI outputs: DSI and OLDI. DSI always has
> "ti,j721e-dsi" DSI bridge connected to the DSI, so we can use that for
> checking. OLDI doesn't have a compatible property, but we can check if
> the DT node has "oldi-transmitters" node as a parent, and the dss node
> itself as a grand-parent. There's also a special case for OLDI, the
> AM62x OLDI, which uses an older design that requires a special case in
> the detection.
> 
> If the output is not connected to either of the above, it must be DPI.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---

Reviewed-by: Swamil Jain <s-jain1@ti.com>

>   drivers/gpu/drm/tidss/tidss_crtc.c  | 10 ++++--
>   drivers/gpu/drm/tidss/tidss_crtc.h  |  4 ++-
>   drivers/gpu/drm/tidss/tidss_dispc.c |  5 ++-
>   drivers/gpu/drm/tidss/tidss_dispc.h |  3 +-
>   drivers/gpu/drm/tidss/tidss_kms.c   | 70 ++++++++++++++++++++++++++++++++++++-
>   5 files changed, 85 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c b/drivers/gpu/drm/tidss/tidss_crtc.c
> index 400329aa2200..ae9c66831b6c 100644
> --- a/drivers/gpu/drm/tidss/tidss_crtc.c
> +++ b/drivers/gpu/drm/tidss/tidss_crtc.c
> @@ -192,7 +192,8 @@ static void tidss_crtc_atomic_flush(struct drm_crtc *crtc,
>   		return;
>   
>   	/* Write vp properties to HW if needed. */
> -	dispc_vp_setup(tidss->dispc, tcrtc->hw_videoport, crtc->state, false);
> +	dispc_vp_setup(tidss->dispc, tcrtc->hw_videoport, crtc->state, false,
> +		       tcrtc->dpi_output);
>   
>   	/* Update plane positions if needed. */
>   	tidss_crtc_position_planes(tidss, crtc, old_crtc_state, false);
> @@ -235,7 +236,8 @@ static void tidss_crtc_atomic_enable(struct drm_crtc *crtc,
>   	if (r != 0)
>   		return;
>   
> -	dispc_vp_setup(tidss->dispc, tcrtc->hw_videoport, crtc->state, true);
> +	dispc_vp_setup(tidss->dispc, tcrtc->hw_videoport, crtc->state, true,
> +		       tcrtc->dpi_output);
>   	tidss_crtc_position_planes(tidss, crtc, old_state, true);
>   
>   	/* Turn vertical blanking interrupt reporting on. */
> @@ -414,7 +416,8 @@ static const struct drm_crtc_funcs tidss_crtc_funcs = {
>   
>   struct tidss_crtc *tidss_crtc_create(struct tidss_device *tidss,
>   				     u32 hw_videoport,
> -				     struct drm_plane *primary)
> +				     struct drm_plane *primary,
> +				     bool dpi_output)
>   {
>   	struct tidss_crtc *tcrtc;
>   	struct drm_crtc *crtc;
> @@ -427,6 +430,7 @@ struct tidss_crtc *tidss_crtc_create(struct tidss_device *tidss,
>   		return ERR_PTR(-ENOMEM);
>   
>   	tcrtc->hw_videoport = hw_videoport;
> +	tcrtc->dpi_output = dpi_output;
>   	init_completion(&tcrtc->framedone_completion);
>   
>   	crtc =  &tcrtc->crtc;
> diff --git a/drivers/gpu/drm/tidss/tidss_crtc.h b/drivers/gpu/drm/tidss/tidss_crtc.h
> index 040d1205496b..65df220698f6 100644
> --- a/drivers/gpu/drm/tidss/tidss_crtc.h
> +++ b/drivers/gpu/drm/tidss/tidss_crtc.h
> @@ -20,6 +20,7 @@ struct tidss_crtc {
>   	struct drm_crtc crtc;
>   
>   	u32 hw_videoport;
> +	bool dpi_output;
>   
>   	struct drm_pending_vblank_event *event;
>   
> @@ -44,5 +45,6 @@ void tidss_crtc_error_irq(struct drm_crtc *crtc, u64 irqstatus);
>   
>   struct tidss_crtc *tidss_crtc_create(struct tidss_device *tidss,
>   				     u32 hw_videoport,
> -				     struct drm_plane *primary);
> +				     struct drm_plane *primary,
> +				     bool dpi_output);
>   #endif
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
> index 58d5eb033bdb..c21ac3f51720 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
> @@ -448,6 +448,7 @@ static const u16 *dispc_common_regmap;
>   
>   struct dss_vp_data {
>   	u32 *gamma_table;
> +	bool dpi_output;
>   };
>   
>   struct dispc_device {
> @@ -2770,8 +2771,10 @@ static void dispc_vp_set_color_mgmt(struct dispc_device *dispc,
>   }
>   
>   void dispc_vp_setup(struct dispc_device *dispc, u32 hw_videoport,
> -		    const struct drm_crtc_state *state, bool newmodeset)
> +		    const struct drm_crtc_state *state, bool newmodeset,
> +		    bool dpi_output)
>   {
> +	dispc->vp_data[hw_videoport].dpi_output = dpi_output;
>   	dispc_vp_set_default_color(dispc, hw_videoport, 0);
>   	dispc_vp_set_color_mgmt(dispc, hw_videoport, state, newmodeset);
>   }
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
> index 739d211d0018..6f53d554259c 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.h
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.h
> @@ -131,7 +131,8 @@ void dispc_vp_disable_clk(struct dispc_device *dispc, u32 hw_videoport);
>   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,
> -		    const struct drm_crtc_state *state, bool newmodeset);
> +		    const struct drm_crtc_state *state, bool newmodeset,
> +		    bool dpi_output);
>   
>   int dispc_runtime_suspend(struct dispc_device *dispc);
>   int dispc_runtime_resume(struct dispc_device *dispc);
> diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c
> index 70c14c3be10d..eefe2221bdde 100644
> --- a/drivers/gpu/drm/tidss/tidss_kms.c
> +++ b/drivers/gpu/drm/tidss/tidss_kms.c
> @@ -122,6 +122,68 @@ static const struct drm_mode_config_funcs mode_config_funcs = {
>   	.atomic_commit = drm_atomic_helper_commit,
>   };
>   
> +static const char * const tidss_internal_bridge_compatibles[] = {
> +	"ti,j721e-dsi",
> +};
> +
> +/*
> + * Detect whether the bridge is internal to the SoC or not.
> + * We detect this via two means:
> + * - If the bridge's of_node has a compatible, compare to known internal values.
> + * - If the bridge is a grand-child of DSS, and has "oldi-transmitters" parent.
> + */
> +static bool tidss_is_bridge_internal(struct tidss_device *tidss,
> +				     struct drm_bridge *bridge)
> +{
> +	struct device_node *parent, *grand_parent;
> +	bool is_internal;
> +
> +	if (WARN_ON(!bridge->of_node))
> +		return false;
> +
> +	for (unsigned int i = 0;
> +	     i < ARRAY_SIZE(tidss_internal_bridge_compatibles); ++i) {
> +		if (of_device_is_compatible(bridge->of_node,
> +					    tidss_internal_bridge_compatibles[i]))
> +			return true;
> +	}
> +
> +	parent = of_get_parent(bridge->of_node);
> +	grand_parent = of_get_parent(parent);
> +
> +	is_internal = parent && grand_parent &&
> +		      tidss->dev->of_node == grand_parent &&
> +		      of_node_name_eq(parent, "oldi-transmitters");
> +
> +	of_node_put(grand_parent);
> +	of_node_put(parent);
> +
> +	return is_internal;
> +}
> +
> +/*
> + * Detect whether the videoport's output goes to the DPI output or not. There is
> + * no "DPI bridge", so we have to find this out in reverse: check if the output
> + * is NOT DPI, and if that is negative, then it must be DPI.
> + */
> +static bool tidss_is_dpi_output(struct tidss_device *tidss, u32 hw_videoport,
> +				struct drm_bridge *bridge)
> +{
> +	/*
> +	 * On AM65x the OLDI TX is modeled as a part of the DSS itself, on VP0.
> +	 * On other SoCs OLDI TX is a separate bridge. As there is no DT node
> +	 * for the OLDI TX on AM65x, we cannot detect this from the DT data, and
> +	 * have to special-case it here.
> +	 */
> +	if (tidss->feat->subrev == DISPC_AM65X && hw_videoport == 0)
> +		return false;
> +
> +	if (!bridge)
> +		return true;
> +
> +	return !tidss_is_bridge_internal(tidss, bridge);
> +}
> +
>   static int tidss_dispc_modeset_init(struct tidss_device *tidss)
>   {
>   	struct device *dev = tidss->dev;
> @@ -133,6 +195,7 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
>   		u32 hw_videoport;
>   		struct drm_bridge *bridge;
>   		u32 enc_type;
> +		bool dpi_output;
>   	};
>   
>   	const struct dispc_features *feat = tidss->feat;
> @@ -149,6 +212,7 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
>   		struct drm_panel *panel;
>   		struct drm_bridge *bridge;
>   		u32 enc_type = DRM_MODE_ENCODER_NONE;
> +		bool dpi_output;
>   		int ret;
>   
>   		ret = drm_of_find_panel_or_bridge(dev->of_node, i, 0,
> @@ -160,6 +224,8 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
>   			return dev_err_probe(dev, ret, "port %d probe failed\n", i);
>   		}
>   
> +		dpi_output = tidss_is_dpi_output(tidss, i, bridge);
> +
>   		if (panel) {
>   			u32 conn_type;
>   			int ret;
> @@ -205,6 +271,7 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
>   		pipes[num_pipes].hw_videoport = i;
>   		pipes[num_pipes].bridge = bridge;
>   		pipes[num_pipes].enc_type = enc_type;
> +		pipes[num_pipes].dpi_output = dpi_output;
>   		num_pipes++;
>   	}
>   
> @@ -230,7 +297,8 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
>   		tidss->planes[tidss->num_planes++] = &tplane->plane;
>   
>   		tcrtc = tidss_crtc_create(tidss, pipes[i].hw_videoport,
> -					  &tplane->plane);
> +					  &tplane->plane,
> +					  pipes[i].dpi_output);
>   		if (IS_ERR(tcrtc)) {
>   			dev_err(tidss->dev, "crtc create failed\n");
>   			return PTR_ERR(tcrtc);
>