Re: [PATCH 4/4] drm/rcar-du: dsi: Implement DSI command support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/12/25 4:36 PM, Tomi Valkeinen wrote:

Hello Tomi,

On 08/06/2025 17:24, Marek Vasut wrote:
Implement support for DSI command transfer mode. Transmission of both Short

I constantly kept reading "DSI command mode support". So I was quite
confused for a bit =). Maybe avoid the use of "mode" with "DSI command".

I dropped the "mode" in V3.

[...]

+static ssize_t rcar_mipi_dsi_host_tx_transfer(struct mipi_dsi_host *host,
+					      const struct mipi_dsi_msg *msg,
+					      bool is_rx_xfer)
+{
+	const bool is_tx_long = mipi_dsi_packet_format_is_long(msg->type);
+	struct rcar_mipi_dsi *dsi = host_to_rcar_mipi_dsi(host);
+	struct mipi_dsi_packet packet;
+	u8 payload[16] = { 0 };
+	u32 status;
+	int ret;
+
+	ret = mipi_dsi_create_packet(&packet, msg);
+	if (ret)
+		return ret;
+
+	/* Configure LP or HS command transfer. */
+	rcar_mipi_dsi_write(dsi, TXCMSETR, (msg->flags & MIPI_DSI_MSG_USE_LPM) ?
+					   TXCMSETR_SPDTYP : 0);

There's no runtime PM in the driver, and the clocks are enabled
externally... So I think we just assume that the IP is running here?

Correct.

[...]

+	/* Start the transfer, RX with BTA, TX without BTA */
+	if (is_rx_xfer) {
+		rcar_mipi_dsi_write(dsi, TXCMCR, TXCMCR_BTAREQ);
+
+		ret = read_poll_timeout(rcar_mipi_dsi_read, status,
+					(status & RXPSR_BTAREQEND),
+					2000, 10000, false, dsi, RXPSR);
+	} else {
+		rcar_mipi_dsi_write(dsi, TXCMCR, TXCMCR_TXREQ);
+
+		ret = read_poll_timeout(rcar_mipi_dsi_read, status,
+					(status & TXCMSR_TXREQEND),
+					2000, 10000, false, dsi, TXCMSR);
+	}

Did you check the timeout is big enough? With LP and BTA... Well, it's
only 16 bytes at max. Maybe it's fine. Again, just a note. =)

I did check it with the only hardware I had available which needs this command mode so far, the RPi Display 2 using ILI9881 DSI-to-TCON .

Does this work when the video stream is on?

That is untested, the ILI9881 only uses command mode during initialization, before it switches video mode on.

If yes, then it might take
much longer until the command can be transferred.

Do you know the upper limit , is that one or two frame times ?

If not maybe the
function should return an error if the video stream is enabled.

I haven't seen any drivers special casing that, I'd prefer to increase the timeouts. For V3, I'll update the timeout to 50ms , which is about 3 frame times.

What do these read_poll_timeouts wait, exactly? The first one waits
until the data is sent, and BTA has been done? And the latter waits only
for the data to be sent? Hmm, no... The first must wait until the
peripheral has replied, as there's no wait in the
rcar_mipi_dsi_host_rx_transfer()...

First one is transmit+BTA+receive , second one is only transmit .

It would be nice to have a short comment what the wait is for.

Will do in V3.

[...]

+	ret = read_poll_timeout(rcar_mipi_dsi_read, status,
+				status & PPIDL0SR_STPST,
+				2000, 10000, false, dsi, PPIDL0SR);
+	if (ret < 0) {
+		dev_err(dsi->dev, "Command RX STPST timeout (0x%08x)\n", status);
+		return ret;
+	}

Same here, it's not very clear what the waits are for. Aren't we done
already after the tx function finished?

First one waits for bus handover to host processor to complete, the second one (STPST) waits for data lane to enter LP11 stop state .

+
+	return 0;
+}
+
+static ssize_t rcar_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
+					   const struct mipi_dsi_msg *msg)
+{
+	const bool is_rx_xfer = (msg->flags & MIPI_DSI_MSG_REQ_ACK) || msg->rx_len;
+	struct rcar_mipi_dsi *dsi = host_to_rcar_mipi_dsi(host);
+	int ret;
+
+	if (msg->tx_len > 16 || msg->rx_len > 16) {
+		/* ToDo: Implement Memory on AXI bus command mode. */
+		dev_warn(dsi->dev,
+			 "Register-based command mode supports only up to 16 Bytes long payload\n");
+		return -EOPNOTSUPP;
+	}
+
+	ret = rcar_mipi_dsi_host_tx_transfer(host, msg, is_rx_xfer);
+
+	/* If TX transfer succeeded and this transfer has RX part. */
+	if (ret >= 0 && is_rx_xfer) {
+		ret = rcar_mipi_dsi_host_rx_transfer(host, msg);
+		if (ret)
+			return ret;
+
+		ret = msg->rx_len;
+	}
+
+	/* Wait a bit between commands */
+	usleep_range(1000, 2000);

Why wait and wait a bit between what?
The consecutive command transmission was unreliable unless there was a slight delay between the consecutive commands. Hence this delay.




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux