linux-kernel - Re: [PATCH v4 07/11] drm/bridge: cdns-dsi: Reset the DCS write FIFO

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: <3aa27f67-02d5-4bd4-b45e-9d4753a333f8@ti.com>
Date: Thu, 11 Jul 2024 13:00:44 +0530
From: Aradhya Bhatia <a-bhatia1@...com>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>,
        Dmitry Baryshkov
	<dmitry.baryshkov@...aro.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>,
        Maarten
 Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard
	<mripard@...nel.org>, Jyri Sarha <jyri.sarha@....fi>,
        Thomas Zimmermann
	<tzimmermann@...e.de>,
        David Airlie <airlied@...il.com>, Daniel Vetter
	<daniel@...ll.ch>
CC: DRI Development List <dri-devel@...ts.freedesktop.org>,
        Linux Kernel List
	<linux-kernel@...r.kernel.org>,
        Dominik Haller <d.haller@...tec.de>, Sam
 Ravnborg <sam@...nborg.org>,
        Thierry Reding <treding@...dia.com>,
        Kieran
 Bingham <kieran.bingham+renesas@...asonboard.com>,
        Nishanth Menon
	<nm@...com>, Vignesh Raghavendra <vigneshr@...com>,
        Praneeth Bajjuri
	<praneeth@...com>, Udit Kumar <u-kumar1@...com>,
        Devarsh Thakkar
	<devarsht@...com>,
        Jayesh Choudhary <j-choudhary@...com>, Jai Luthra
	<j-luthra@...com>
Subject: Re: [PATCH v4 07/11] drm/bridge: cdns-dsi: Reset the DCS write FIFO



On 26/06/24 16:33, Tomi Valkeinen wrote:
> On 22/06/2024 14:09, Aradhya Bhatia wrote:
>> If any normal DCS write command has already been transmitted prior to
>> transmitting any Zero-Parameter DCS command, then it is necessary to
>> clear the TX FIFO by resetting it. Otherwise, the FIFO points to another
>> location, and the DCS command transmits unnecessary data causing the
>> panel to not work[0].
>>
>> Allow the DCS Write FIFO in the cdns-dsi controller to reset as a rule,
>> before any DCS packet is transmitted to the DSI peripheral.
>>
>> [0]: Section 12.6.5.7.5.2: "Command Mode Settings" in TDA4VM Technical
>>       Reference Manual: https://www.ti.com/lit/zip/spruil1
> 
> Hmm so if I read the doc right, it says: if sending zero-parameter dcs
> command, clear the FIFO and write zero to direct_cmd_wrdat.

That's right.

> 
> Your patch seems to always clear the FIFO, not only for zero-parameter
> commands. Is that a problem (I don't think so, but...)?
> 

My patch does clear the FIFO every time.

While there is no documentation that says its harmless, I have tested
the patches with RPi Panel (which doesn't seem to have any
zero-parameter commands in the driver) - and so far it seems to have
worked fine.


> Also, is the direct_cmd_wrdat written at all when sending zero-parameter
> dcs command?
> 

At the moment, no.

Apparently there are 2 types of "Zero parameter" commands.

There is,

a) "MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM" - which has absolutely no
parameter that needs to be sent, and there is,

b) "MIPI_DSI_DCS_SHORT_WRITE" - which has a 1-byte command value that
needs to be transmitted.

(Macros referred from mipi_display.h)

In the J721E TRM[0], there is a table[1]  which classifies the
"MIPI_DSI_DCS_SHORT_WRITE" - the command with 1-byte command parameter -
as a "zero parameter" command.

For a "MIPI_DSI_DCS_SHORT_WRITE" command, we are still writing the
1-byte command data into the FIFO.

However, in the other section which talks about resetting the FIFO[2],
it is mentioned that, for "zero parameter" commands, the FIFO needs to
be reset and then 0x00 needs to be written to the FIFO.

The second step cannot be done for "MIPI_DSI_DCS_SHORT_WRITE" commands
because we want to write the 1 byte command parameter instead of 0x00
into the FIFO.

So, the only logical conclusion is that, the FIFO reset is only required
for _truly_ zero parameter commands, which is the
"MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM" command.

So, I am planning to change this patch to do 2 things, under the
condition that there are absolutely no data bytes that require
transmission.

a. Reset the FIFO.
b. Write 0x00 to the FIFO.


Regards
Aradhya

[0]: J721E TRM: https://www.ti.com/lit/zip/spruil1
[1]: Table: 12-1933: "DSI Main Settings Register Description".
[2]: Section 12.6.5.7.5.2: "Command Mode Settings"


> 
>>
>> Signed-off-by: Aradhya Bhatia <a-bhatia1@...com>
>> ---
>>   drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
>> b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
>> index 126e4bccd868..cad0c1478ef0 100644
>> --- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
>> +++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
>> @@ -1018,6 +1018,9 @@ static ssize_t cdns_dsi_transfer(struct
>> mipi_dsi_host *host,
>>         cdns_dsi_init_link(dsi);
>>   +    /* Reset the DCS Write FIFO */
>> +    writel(0x00, dsi->regs + DIRECT_CMD_FIFO_RST);
>> >>       ret = mipi_dsi_create_packet(&packet, msg);
>>       if (ret)
>>           goto out;
> 
>