Re: [PATCH v2 1/3] drm/vc4: dsi: Don't return error reports as read data

[Dave Stevenson]

Hi Henry

On Mon, 19 Oct 2026 at 18:54, Henry Zhang <henryzhang62@xxxxxxxxx> wrote:
>
> When a DSI peripheral detects an error, for example contention during
> the bus turnaround, it may answer a read request with an Acknowledge
> and Error Report short packet instead of the read response.
>
> vc4_dsi_host_transfer() does not check the type of the received packet
> (there is a FINISHME for it). It copies the two bytes of the error
> report into the caller's buffer and returns success, so the caller
> gets the error bits as if they were register contents. Packets
> received with a checksum error, an uncorrectable ECC error or
> incomplete are passed on the same way.
>
> Return -EIO in these cases so that callers can tell a failed read from
> data, and retry if they want to.
>
> Signed-off-by: Henry Zhang <henryzhang62@xxxxxxxxx>

Thanks for the patch.

Reviewed-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>

> ---
>
> Notes:
>     v2: no changes.
>
>  drivers/gpu/drm/vc4/vc4_dsi.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
> index 5ed3c35..5452b72 100644
> --- a/drivers/gpu/drm/vc4/vc4_dsi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dsi.c
> @@ -39,6 +39,8 @@
>  #include <drm/drm_print.h>
>  #include <drm/drm_probe_helper.h>
>
> +#include <video/mipi_display.h>
> +
>  #include "vc4_drv.h"
>  #include "vc4_regs.h"
>
> @@ -1299,6 +1301,24 @@ static ssize_t vc4_dsi_host_transfer(struct mipi_dsi_host *host,
>                 u32 rxpkt1h = DSI_PORT_READ(RXPKT1H);
>                 u8 *msg_rx = msg->rx_buf;
>
> +               /*
> +                * A peripheral that detected an error, e.g. contention during
> +                * the bus turnaround, may answer with an Acknowledge and Error
> +                * Report instead of the read response. Don't hand that, or a
> +                * packet received with errors, to the caller as read data.
> +                * Bits 7:6 of the Data ID are the virtual channel.
> +                */
> +               if ((rxpkt1h & (DSI_RXPKT1H_CRC_ERR | DSI_RXPKT1H_DET_ERR |
> +                               DSI_RXPKT1H_INCOMP_PKT)) ||
> +                   (!(rxpkt1h & DSI_RXPKT1H_PKT_TYPE_LONG) &&
> +                    (VC4_GET_FIELD(rxpkt1h, DSI_RXPKT1H_DT_LP_CMD) & 0x3f) ==
> +                    MIPI_DSI_RX_ACKNOWLEDGE_AND_ERROR_REPORT)) {
> +                       drm_dbg_driver(drm, "DSI read rejected, RXPKT1H 0x%08x\n",
> +                                      rxpkt1h);
> +                       ret = -EIO;
> +                       goto reset_fifo_and_return;
> +               }
> +
>                 if (rxpkt1h & DSI_RXPKT1H_PKT_TYPE_LONG) {
>                         u32 rxlen = VC4_GET_FIELD(rxpkt1h,
>                                                   DSI_RXPKT1H_BC_PARAM);
> @@ -1313,8 +1333,6 @@ static ssize_t vc4_dsi_host_transfer(struct mipi_dsi_host *host,
>                         for (i = 0; i < msg->rx_len; i++)
>                                 msg_rx[i] = DSI_READ(DSI1_RXPKT_FIFO);
>                 } else {
> -                       /* FINISHME: Handle AWER */
> -
>                         msg_rx[0] = VC4_GET_FIELD(rxpkt1h,
>                                                   DSI_RXPKT1H_SHORT_0);
>                         if (msg->rx_len > 1) {
> --
> 2.47.3
>