diff mbox series

[v1,2/2] drm/bridge: ti-sn65dsi83: Add DSI video mode configuration

Message ID 20230330101752.429804-3-francesco@dolcini.it (mailing list archive)
State New, archived
Headers show
Series drm/bridge: ti-sn65dsi83: Add DSI video mode configuration | expand

Commit Message

Francesco Dolcini March 30, 2023, 10:17 a.m. UTC 
From: Francesco Dolcini <francesco.dolcini@toradex.com>

Enable configuration of the DSI video mode instead of hard-coding
MIPI_DSI_MODE_VIDEO_BURST. The bridge support any of burst-mode,
non-burst with sync event or non-burst with sync pulse

With this change is possible to use the bridge with host DSI
that do not support burst mode.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

Comments

Neil Armstrong March 31, 2023, 9:06 a.m. UTC | #1 
Hi,

On 30/03/2023 12:17, Francesco Dolcini wrote:
> From: Francesco Dolcini <francesco.dolcini@toradex.com>
> 
> Enable configuration of the DSI video mode instead of hard-coding
> MIPI_DSI_MODE_VIDEO_BURST. The bridge support any of burst-mode,
> non-burst with sync event or non-burst with sync pulse
> 
> With this change is possible to use the bridge with host DSI
> that do not support burst mode.
> 
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
>   drivers/gpu/drm/bridge/ti-sn65dsi83.c | 24 +++++++++++++++++++++++-
>   1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index 91ecfbe45bf9..144d0de0d278 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -145,6 +145,7 @@ struct sn65dsi83 {
>   	struct drm_bridge		*panel_bridge;
>   	struct gpio_desc		*enable_gpio;
>   	struct regulator		*vcc;
> +	u32				dsi_video_mode;
>   	bool				lvds_dual_link;
>   	bool				lvds_dual_link_even_odd_swap;
>   };
> @@ -603,6 +604,9 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
>   		return dev_err_probe(dev, PTR_ERR(ctx->vcc),
>   				     "Failed to get supply 'vcc'\n");
>   
> +	ctx->dsi_video_mode = 0;
> +	of_property_read_u32(dev->of_node, "dsi-video-mode", &ctx->dsi_video_mode);
> +
>   	return 0;
>   }
>   
> @@ -642,7 +646,25 @@ static int sn65dsi83_host_attach(struct sn65dsi83 *ctx)
>   
>   	dsi->lanes = dsi_lanes;
>   	dsi->format = MIPI_DSI_FMT_RGB888;
> -	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST;
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
> +
> +	switch (ctx->dsi_video_mode) {
> +	case 0:
> +		/* burst mode */
> +		dsi->mode_flags |= MIPI_DSI_MODE_VIDEO_BURST;
> +		break;
> +	case 1:
> +		/* non-burst mode with sync event */
> +		break;
> +	case 2:
> +		/* non-burst mode with sync pulse */
> +		dsi->mode_flags |= MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> +		break;
> +	default:
> +		dev_warn(dev, "invalid video mode %d\n", ctx->dsi_video_mode);
> +		break;
> +	}
> +
>   
>   	ret = devm_mipi_dsi_attach(dev, dsi);
>   	if (ret < 0) {

As I understand from DSI spec is that those are dynamic, so they are hardcoded in driver now
but they shouldn't be hardcoded in DT.

The problem is larger and we should add some sort of "supported features" we can get
from the DSI controller and set the flags accordingly because the features support accross the
DSI controllers isn't linear.

Neil
Francesco Dolcini March 31, 2023, 9:47 a.m. UTC | #2 
On Fri, Mar 31, 2023 at 11:06:44AM +0200, Neil Armstrong wrote:
> Hi,
> 
> On 30/03/2023 12:17, Francesco Dolcini wrote:
> > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> > 
> > Enable configuration of the DSI video mode instead of hard-coding
> > MIPI_DSI_MODE_VIDEO_BURST. The bridge support any of burst-mode,
> > non-burst with sync event or non-burst with sync pulse
> > 
> > With this change is possible to use the bridge with host DSI
> > that do not support burst mode.
> > 
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > ---
> >   drivers/gpu/drm/bridge/ti-sn65dsi83.c | 24 +++++++++++++++++++++++-
> >   1 file changed, 23 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > index 91ecfbe45bf9..144d0de0d278 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > @@ -145,6 +145,7 @@ struct sn65dsi83 {
> >   	struct drm_bridge		*panel_bridge;
> >   	struct gpio_desc		*enable_gpio;
> >   	struct regulator		*vcc;
> > +	u32				dsi_video_mode;
> >   	bool				lvds_dual_link;
> >   	bool				lvds_dual_link_even_odd_swap;
> >   };
> > @@ -603,6 +604,9 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
> >   		return dev_err_probe(dev, PTR_ERR(ctx->vcc),
> >   				     "Failed to get supply 'vcc'\n");
> > +	ctx->dsi_video_mode = 0;
> > +	of_property_read_u32(dev->of_node, "dsi-video-mode", &ctx->dsi_video_mode);
> > +
> >   	return 0;
> >   }
> > @@ -642,7 +646,25 @@ static int sn65dsi83_host_attach(struct sn65dsi83 *ctx)
> >   	dsi->lanes = dsi_lanes;
> >   	dsi->format = MIPI_DSI_FMT_RGB888;
> > -	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST;
> > +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
> > +
> > +	switch (ctx->dsi_video_mode) {
> > +	case 0:
> > +		/* burst mode */
> > +		dsi->mode_flags |= MIPI_DSI_MODE_VIDEO_BURST;
> > +		break;
> > +	case 1:
> > +		/* non-burst mode with sync event */
> > +		break;
> > +	case 2:
> > +		/* non-burst mode with sync pulse */
> > +		dsi->mode_flags |= MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> > +		break;
> > +	default:
> > +		dev_warn(dev, "invalid video mode %d\n", ctx->dsi_video_mode);
> > +		break;
> > +	}
> > +
> >   	ret = devm_mipi_dsi_attach(dev, dsi);
> >   	if (ret < 0) {
> 
> As I understand from DSI spec is that those are dynamic, so they are hardcoded in driver now
> but they shouldn't be hardcoded in DT.
> 
> The problem is larger and we should add some sort of "supported features" we can get
> from the DSI controller and set the flags accordingly because the features support accross the
> DSI controllers isn't linear.

I need to change this since I do have a bridge that does not support
DSI video burst mode, while the ti-sn65 supports really any mode.

The change here is the same as what is done for the raydium,rm67191
panel, see Documentation/devicetree/bindings/display/panel/raydium,rm67191.yaml
and drivers/gpu/drm/panel/panel-raydium-rm67191.c.

What's wrong to have this in the dts so that a panel can explicitly tell
which DSI video mode it expects?

Francesco
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 91ecfbe45bf9..144d0de0d278 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -145,6 +145,7 @@  struct sn65dsi83 {
 	struct drm_bridge		*panel_bridge;
 	struct gpio_desc		*enable_gpio;
 	struct regulator		*vcc;
+	u32				dsi_video_mode;
 	bool				lvds_dual_link;
 	bool				lvds_dual_link_even_odd_swap;
 };
@@ -603,6 +604,9 @@  static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
 		return dev_err_probe(dev, PTR_ERR(ctx->vcc),
 				     "Failed to get supply 'vcc'\n");
 
+	ctx->dsi_video_mode = 0;
+	of_property_read_u32(dev->of_node, "dsi-video-mode", &ctx->dsi_video_mode);
+
 	return 0;
 }
 
@@ -642,7 +646,25 @@  static int sn65dsi83_host_attach(struct sn65dsi83 *ctx)
 
 	dsi->lanes = dsi_lanes;
 	dsi->format = MIPI_DSI_FMT_RGB888;
-	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST;
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
+
+	switch (ctx->dsi_video_mode) {
+	case 0:
+		/* burst mode */
+		dsi->mode_flags |= MIPI_DSI_MODE_VIDEO_BURST;
+		break;
+	case 1:
+		/* non-burst mode with sync event */
+		break;
+	case 2:
+		/* non-burst mode with sync pulse */
+		dsi->mode_flags |= MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
+		break;
+	default:
+		dev_warn(dev, "invalid video mode %d\n", ctx->dsi_video_mode);
+		break;
+	}
+
 
 	ret = devm_mipi_dsi_attach(dev, dsi);
 	if (ret < 0) {