| Message ID | 20240622110929.3115714-8-a-bhatia1@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: cdns-dsi: Fix the color-shift issue | expand |
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. Your patch seems to always clear the FIFO, not only for zero-parameter commands. Is that a problem (I don't think so, but...)? Also, is the direct_cmd_wrdat written at all when sending zero-parameter dcs command? Tomi > > Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.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;
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@ti.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; > >
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;
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 Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> --- drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 3 +++ 1 file changed, 3 insertions(+)