[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <d6e864d9-53ea-44d3-832c-55a6e58ac6d3@ti.com>
Date: Thu, 17 Apr 2025 02:40:43 +0530
From: Jayesh Choudhary <j-choudhary@...com>
To: Doug Anderson <dianders@...omium.org>
CC: <andrzej.hajda@...el.com>, <neil.armstrong@...aro.org>, <rfoss@...nel.org>,
<Laurent.pinchart@...asonboard.com>, <dri-devel@...ts.freedesktop.org>,
<jonas@...boo.se>, <jernej.skrabec@...il.com>,
<maarten.lankhorst@...ux.intel.com>, <mripard@...nel.org>,
<tzimmermann@...e.de>, <airlied@...il.com>, <simona@...ll.ch>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/bridge: ti-sn65dsi86: Add necessary DSI flags
Hello Doug,
On 13/04/25 07:22, Doug Anderson wrote:
> Hi,
>
> On Fri, Apr 11, 2025 at 2:23 AM Jayesh Choudhary <j-choudhary@...com> wrote:
>>
>> Enable NO_EOT and SYNC flags for DSI to use VIDEO_SYNC_PULSE_MODE
>> with EOT disabled.
>
> Any chance you could add some details to this commit message? Your
> subject says that these flags are "necessary", but people have been
> using this driver successfully for many years now. Why did these flags
> suddenly become necessary and why were things working before?
>
> I'm not saying that we shouldn't use these flags, just trying to
> understand. I actually don't know a ton about these details in MIPI,
> so it would help me :-).
>
Definitely.
I will add more details for the commit message.
For more context here, I was working with cadence dsi driver for TI
SoCs. So to be more accurate, this is required for CDNS_DSI
I observed other bridges like lt-9211, where I have seen such flags
being set for dsi-controller by vendors.
>
>> Signed-off-by: Jayesh Choudhary <j-choudhary@...com>
>> ---
>> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> index f72675766e01..8e9a7eb927da 100644
>> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> @@ -707,7 +707,8 @@ static int ti_sn_attach_host(struct auxiliary_device *adev, struct ti_sn65dsi86
>> /* TODO: setting to 4 MIPI lanes always for now */
>> dsi->lanes = 4;
>> dsi->format = MIPI_DSI_FMT_RGB888;
>> - dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
>> + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_NO_EOT_PACKET |
>> + MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
>
> FWIW, I can confirm that on my board the screen still seems to light
> up after this change. ...so I'd be OK w/
>
> Tested-by: Douglas Anderson <dianders@...omium.org>
>
> ...before giving a Reviewed-by I'd want a description that helps me
> understand it better.
>
> -Doug
Powered by blists - more mailing lists