linux-kernel - Re: [PATCH v4 17/17] drm/bridge: cdns-dsi: Don't fail on MIPI_DSI_MODE_VIDEO

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] [day] [month] [year] [list]
Message-ID: <187d6072-55b1-4402-a2b5-82afb45a3345@ti.com>
Date: Thu, 17 Jul 2025 19:11:25 +0530
From: Devarsh Thakkar <devarsht@...com>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
CC: <dri-devel@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>,
        <linux-phy@...ts.infradead.org>,
        Francesco Dolcini <francesco@...cini.it>,
        Aradhya Bhatia <aradhya.bhatia@...ux.dev>,
        Parth Pancholi
	<parth.pancholi@...adex.com>,
        Jyri Sarha <jyri.sarha@....fi>,
        Maarten
 Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard
	<mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie
	<airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
        Vinod Koul
	<vkoul@...nel.org>,
        Kishon Vijay Abraham I <kishon@...nel.org>,
        Andrzej Hajda
	<andrzej.hajda@...el.com>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Robert
 Foss <rfoss@...nel.org>,
        Laurent Pinchart
	<Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej
 Skrabec <jernej.skrabec@...il.com>,
        Jayesh Choudhary <j-choudhary@...com>,
        Dmitry Baryshkov <lumag@...nel.org>
Subject: Re: [PATCH v4 17/17] drm/bridge: cdns-dsi: Don't fail on
 MIPI_DSI_MODE_VIDEO_BURST

Hi Tomi,

On 17/07/25 15:59, Tomi Valkeinen wrote:
> Hi,
> 
> On 17/07/2025 12:36, Devarsh Thakkar wrote:
>> Hi Tomi
>>
>> Thanks for the patch.
>>
>> On 18/06/25 15:29, Tomi Valkeinen wrote:
>>> While the cdns-dsi does not support DSI burst mode, the burst mode is
>>> essentially DSI event mode with more versatile clocking and timings.
>> I don't fully agree with this statement, DSI burst mode and DSI event
>> mode are two different things having separate requirements. DSI burst
>> mode maps to MIPI_DSI_MODE_VIDEO_BURST. I don't see a separate flag for
>> event mode but I guess,
> 
> Well, what does DSI burst mode mean? Signal-wise it's the same as DSI
> event mode. "burst" just means that the DSI TX is allowed to send the
> data much faster than the pixel clock. But there's no strict requirement
> that it _must_ be faster.
> 
> So, DSI burst mode is basically DSI event mode with more freedom on the
> timings. Thus, afaics, DSI TX in DSI event mode should always work if
> the DSI RX expects DSI burst mode.
> 

Yes this is true, although there are subtle differences between event 
mode and burst mode and I guess that's why DSI specification 8.11.1 
Transmission Packet Sequences mentions for video mode lists DSI event 
mode sequence as differently then burst mode w.r.t packet sequence viz 
listing 3 different sequences i.e. non-burst with sync pulse, non-burst 
with sync event and burst mode. But seems like we do not have 3 separate 
flags for each of those.

And I see most drivers using MIPI_DSI_MODE_VIDEO both in context of 
selecting video mode as opposed to command mode and also subtly 
inferring it as event mode in case MIPI_DSI_MODE_VIDEO_BURST and 
MIPI_DSI_MODE_VIDEO_PULSE are not set. So there is no accurate way to 
differentiate between the three or for the bridge driver to enforce 
burst mode over event mode and I guess decision is left to host driver.

So, I think this patch looks fine considering these aspects.


>>> Thus cdns-dsi doesn't need to fail if the DSI peripheral driver requests
>>> MIPI_DSI_MODE_VIDEO_BURST.
>>
>> MIPI_DSI_MODE_VIDEO_BURST is currently not supported by the cadence DSI
>> host driver, so only if DSI peripheral driver is saying that burst mode
>> is the only one it supports in that case only we should fail.
>>
>>>
>>> In my particular use case, this allows the use of ti-sn65dsi83 driver.
>>>
>>> Tested-by: Parth Pancholi <parth.pancholi@...adex.com>
>>> Tested-by: Jayesh Choudhary <j-choudhary@...com> >>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>

Reviewed-by: Devarsh Thakkar <devarsht@...com>

Regards
Devarsh

>>> ---
>>>    drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 4 ----
>>>    1 file changed, 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c b/drivers/
>>> gpu/drm/bridge/cadence/cdns-dsi-core.c
>>> index 114d883c65dc..09b289f0fcbf 100644
>>> --- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
>>> +++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
>>> @@ -1052,10 +1052,6 @@ static int cdns_dsi_attach(struct mipi_dsi_host
>>> *host,
>>>        if (output->dev)
>>>            return -EBUSY;
>>>    -    /* We do not support burst mode yet. */
>>> -    if (dev->mode_flags & MIPI_DSI_MODE_VIDEO_BURST)
>>> -        return -ENOTSUPP;
>>> -
>>
>> Removing this check also gives a false impression that burst mode is
>> supported by the driver and can also lead to failures too in case device
>> is only supporting burst mode.
>>
>> I think it makes sense to fail only if burst mode is the only one being
>> supported by the device, something like below should work I believe,
>>
>> if (dev->mode_flags & MIPI_DSI_MODE_VIDEO_BURST ==
>> MIPI_DSI_MODE_VIDEO_BURST)
>>          return -ENOTSUPP;
>>
>> Regards
>> Devarsh
>>
>>>        /*
>>>         * The host <-> device link might be described using an OF-graph
>>>         * representation, in this case we extract the device of_node from
>>>
>