linux-kernel - Re: [PATCH v1 2/2] drm/bridge: ti-sn65dsi83: Add DSI video mode configuration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZCasOyfXOMTzzZ8s@francesco-nb.int.toradex.com>
Date:   Fri, 31 Mar 2023 11:47:39 +0200
From:   Francesco Dolcini <francesco@...cini.it>
To:     Neil Armstrong <neil.armstrong@...aro.org>
Cc:     Francesco Dolcini <francesco@...cini.it>,
        Andrzej Hajda <andrzej.hajda@...el.com>,
        Robert Foss <rfoss@...nel.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        dri-devel@...ts.freedesktop.org, Marek Vasut <marex@...x.de>,
        Francesco Dolcini <francesco.dolcini@...adex.com>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/2] drm/bridge: ti-sn65dsi83: Add DSI video mode
 configuration

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@...adex.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@...adex.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