On Sat, Aug 31, 2019 at 09:50:13AM +0200, Linus Walleij wrote:
There were bugs in the DSI transfer (read and write) function as it was only tested with displays ever needing a single byte to be written. Fixed it up and tested so we can now write messages of up to 16 bytes and read up to 4 bytes from the display.
Tested with a Sony ACX424AKP display: this display now self- identifies and can control backlight in command mode.
Cc: Stephan Gerhold stephan@gerhold.net Reported-by: kbuild test robot lkp@intel.com Fixes: 5fc537bfd000 ("drm/mcde: Add new driver for ST-Ericsson MCDE") Signed-off-by: Linus Walleij linus.walleij@linaro.org
ChangeLog v1->v2:
- Fix a print modifier for dev_err() found by the build robot.
drivers/gpu/drm/mcde/mcde_dsi.c | 70 ++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c index 07f7090d08b3..90659d190d78 100644 --- a/drivers/gpu/drm/mcde/mcde_dsi.c +++ b/drivers/gpu/drm/mcde/mcde_dsi.c @@ -178,22 +178,26 @@ static ssize_t mcde_dsi_host_transfer(struct mipi_dsi_host *host, const u32 loop_delay_us = 10; /* us */ const u8 *tx = msg->tx_buf; u32 loop_counter;
- size_t txlen;
- size_t txlen = msg->tx_len;
- size_t rxlen = msg->rx_len; u32 val; int ret; int i;
- txlen = msg->tx_len;
- if (txlen > 12) {
- if (txlen > 16) { dev_err(d->dev,
"dunno how to write more than 12 bytes yet\n");
"dunno how to write more than 16 bytes yet\n");return -EIO;}
if (rxlen > 4) {
dev_err(d->dev,"dunno how to read more than 4 bytes yet\n");return -EIO; }
dev_dbg(d->dev,
"message to channel %d, %zd bytes",msg->channel,txlen);
"message to channel %d, write %zd bytes read %zd bytes\n",msg->channel, txlen, rxlen);/* Command "nature" */ if (MCDE_DSI_HOST_IS_READ(msg->type))
@@ -210,9 +214,7 @@ static ssize_t mcde_dsi_host_transfer(struct mipi_dsi_host *host, if (mipi_dsi_packet_format_is_long(msg->type)) val |= DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_LONGNOTSHORT; val |= 0 << DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_ID_SHIFT;
- /* Add one to the length for the MIPI DCS command */
- val |= txlen
<< DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_SIZE_SHIFT;
- val |= txlen << DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_SIZE_SHIFT; val |= DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_LP_EN; val |= msg->type << DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_HEAD_SHIFT; writel(val, d->regs + DSI_DIRECT_CMD_MAIN_SETTINGS);
@@ -249,17 +251,36 @@ static ssize_t mcde_dsi_host_transfer(struct mipi_dsi_host *host, writel(1, d->regs + DSI_DIRECT_CMD_SEND);
loop_counter = 1000 * 1000 / loop_delay_us;
- while (!(readl(d->regs + DSI_DIRECT_CMD_STS) &
DSI_DIRECT_CMD_STS_WRITE_COMPLETED)&& --loop_counter)usleep_range(loop_delay_us, (loop_delay_us * 3) / 2);- if (!loop_counter) {
dev_err(d->dev, "DSI write timeout!\n");return -ETIME;
- if (MCDE_DSI_HOST_IS_READ(msg->type)) {
/* Read command */while (!(readl(d->regs + DSI_DIRECT_CMD_STS) &(DSI_DIRECT_CMD_STS_READ_COMPLETED |DSI_DIRECT_CMD_STS_READ_COMPLETED_WITH_ERR))&& --loop_counter)usleep_range(loop_delay_us, (loop_delay_us * 3) / 2);if (!loop_counter) {dev_err(d->dev, "DSI write timeout!\n");
"DSI read timeout!" might be more apppropriate here?
return -ETIME;}} else {
/* Writing only */while (!(readl(d->regs + DSI_DIRECT_CMD_STS) &DSI_DIRECT_CMD_STS_WRITE_COMPLETED)&& --loop_counter)usleep_range(loop_delay_us, (loop_delay_us * 3) / 2);if (!loop_counter) {dev_err(d->dev, "DSI write timeout!\n");return -ETIME;}}
val = readl(d->regs + DSI_DIRECT_CMD_STS);
if (val & DSI_DIRECT_CMD_STS_READ_COMPLETED_WITH_ERR) {
dev_err(d->dev, "read completed with error\n");writel(1, d->regs + DSI_DIRECT_CMD_RD_INIT);return -EIO;} if (val & DSI_DIRECT_CMD_STS_ACKNOWLEDGE_WITH_ERR_RECEIVED) { val >>= DSI_DIRECT_CMD_STS_ACK_VAL_SHIFT; dev_err(d->dev, "error during transmission: %04x\n",
@@ -269,10 +290,7 @@ static ssize_t mcde_dsi_host_transfer(struct mipi_dsi_host *host,
if (!MCDE_DSI_HOST_IS_READ(msg->type)) { /* Return number of bytes written */
if (mipi_dsi_packet_format_is_long(msg->type))ret = 4 + txlen;elseret = 4;
ret = txlen;@@ -282,7 +300,13 @@ static ssize_t mcde_dsi_host_transfer(struct mipi_dsi_host *host, rdsz = readl(d->regs + DSI_DIRECT_CMD_RD_PROPERTY); rdsz &= DSI_DIRECT_CMD_RD_PROPERTY_RD_SIZE_MASK; rddat = readl(d->regs + DSI_DIRECT_CMD_RDDAT);
for (i = 0; i < 4 && i < rdsz; i++)
if (rdsz < rxlen) {dev_err(d->dev, "read error, requested %zd got %d\n",msg->rx_len, rdsz);
Using rxlen instead of msg->rx_len would be more consistent with the if condition.
These are just minor nitpicks, so with or without changes: Reviewed-by: Stephan Gerhold stephan@gerhold.net