Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <6a386ff3-2af6-a544-674f-7c60d4d89e4e@linaro.org>
Date:   Fri, 31 Mar 2023 11:06:44 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.1
From:   Neil Armstrong <neil.armstrong@linaro.org>
Reply-To: neil.armstrong@linaro.org
Subject: Re: [PATCH v1 2/2] drm/bridge: ti-sn65dsi83: Add DSI video mode
 configuration
Content-Language: en-US
To:     Francesco Dolcini <francesco@dolcini.it>,
        Andrzej Hajda <andrzej.hajda@intel.com>,
        Robert Foss <rfoss@kernel.org>,
        Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
        Jonas Karlman <jonas@kwiboo.se>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        dri-devel@lists.freedesktop.org, Marek Vasut <marex@denx.de>
Cc:     Francesco Dolcini <francesco.dolcini@toradex.com>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>, linux-kernel@vger.kernel.org
References: <20230330101752.429804-1-francesco@dolcini.it>
 <20230330101752.429804-3-francesco@dolcini.it>
Organization: Linaro Developer Services
In-Reply-To: <20230330101752.429804-3-francesco@dolcini.it>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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