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

[Devarsh Thakkar <devarsht@ti.com>]

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@toradex.com>
>>> Tested-by: Jayesh Choudhary <j-choudhary@ti.com> >>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

Reviewed-by: Devarsh Thakkar <devarsht@ti.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
>>>
>