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@xxxxxx> >> --- >> 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; > >
