Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] AXI command transfer payloads are incorrectly capped at 128 bytes for High Speed (HS) transfers, truncating payloads and causing a compilation error. - [High] Long TX commands (>16 bytes) that request an ACK or expect RX data bypass AXI mode, falling back to register mode where they are silently truncated to 16 bytes. --
--- Patch [1]: [PATCH v2] drm/rcar-du: dsi: Implement DSI command TX using AXI memory access --- commit be1d10f7046189958d60166cb7834225e599f379 Author: Marek Vasut <[email protected]> drm/rcar-du: dsi: Implement DSI command TX using AXI memory access This commit implements support for DSI command TX using AXI memory access mode for the rcar-du driver. It extends the packet transfer length up to 128 bytes in LP mode and 1024 bytes in HS mode, falling back to register access mode for smaller packets. > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c > b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c > index 6e46d6d99f3c0..8a380cbe72bbb 100644 > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c [ ... ] > @@ -977,6 +981,7 @@ static ssize_t rcar_mipi_dsi_host_tx_transfer(struct > mipi_dsi_host *host, > bool is_rx_xfer) > { > const bool is_tx_long = mipi_dsi_packet_format_is_long(msg->type); > + const bool is_tx_axi = !is_rx_xfer && is_tx_long && (msg->tx_len > 16); [Severity: High] If a long TX command (greater than 16 bytes) requests an ACK or expects RX data, is_rx_xfer will be true, making is_tx_axi false. Will this cause the transfer to fall back to register mode where the payload is silently truncated to 16 bytes, while the hardware header is programmed with the full msg->tx_len? > struct rcar_mipi_dsi *dsi = host_to_rcar_mipi_dsi(host); > struct mipi_dsi_packet packet; [ ... ] > @@ -1011,7 +1021,10 @@ static ssize_t rcar_mipi_dsi_host_tx_transfer(struct > mipi_dsi_host *host, > TXCMPHDR_DATA1(packet.header[2]) | > TXCMPHDR_DATA0(packet.header[1])); > > - if (is_tx_long) { > + if (is_tx_axi) { > + memcpy(dsi->cmd_axi_cpu, packet.payload, > + min(msg->tx_len, 128)); [Severity: High] Does this hardcoded 128-byte limit truncate High Speed (HS) transfers, which are allowed to be up to 1024 bytes long according to the newly added limits in rcar_mipi_dsi_host_transfer()? Additionally, does using min() here cause a compilation warning or error due to a type mismatch between msg->tx_len (size_t) and 128 (signed int)? > + } else if (is_tx_long) { > memcpy(payload, packet.payload, > min(msg->tx_len, sizeof(payload))); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
