On Tue, Sep 03, 2019 at 10:11:29AM +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.
> 
> Reported-by: kbuild test robot <[email protected]>
> Fixes: 5fc537bfd000 ("drm/mcde: Add new driver for ST-Ericsson MCDE")
> Reviewed-by: Stephan Gerhold <[email protected]>
> Signed-off-by: Linus Walleij <[email protected]>
> ---
> ChangeLog v2->v3:
> - Fix an error message to indicate reading error rather than
>   writing error.
> - Use the local variable for underflow print.
> - Collected Stephan's reviewed-by.
> 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..cd261c266f35 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");

It looks like you changed the wrong message.
This one is the "read timeout",

> +                     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 read timeout!\n");

and this one the "write timeout".
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to