[PATCH] soundwire: qcom: cache SCP_ADDRPAGE1/2 to preserve MBQ atomicity

[Srinivas Kandagatla]

qcom_swrm_xfer_msg() reprograms SCP_ADDRPAGE1/2 before every paged
transfer, even when the paging window hasn't changed. Besides the
wasted bus transactions, this breaks the atomicity that SoundWire
Classic Multi-Byte Quantity (MBQ) writes rely on: the kernel's MBQ
regmap emits an MBQ pair as two sdw_write_no_pm() calls to bit-13
aliases sharing the same page, and the intervening PAGE writes
invalidate the MBQ high-byte staging on Peripherals that observe
them.  On WCD9378 FU42 Q7.8 Channel Volume this drops the MSB byte
silently.

Per SDCA v1.1 sections 12.2.5 and 12.2.8.2.1, MBQ pairs are expected
to be atomic on the wire.  Cache the last-programmed page values per
Slave, skip the FIFO write when they match, and invalidate on
re-attach since SCP_ADDRPAGE1/2 reset to defaults on re-enumeration.


Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxxxxxxxx>
Cc: Faiz Nabi Kuchay <fkuchay@xxxxxxxxxxxxxxxx>
---

Same pattern exists in intel/cadence and amd.
It makes sense to move this cache into the SoundWire core at some point
if everyone agrees with this approch. Buf for now the patch is focused
on qcom controller.

--srini

 drivers/soundwire/qcom.c | 38 ++++++++++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 55678a30cd4a..35ffffd541bd 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -221,6 +221,9 @@ struct qcom_swrm_ctrl {
 	u32 slave_status;
 	u32 wr_fifo_depth;
 	bool clock_stop_not_supported;
+	/* Per-Slave SCP_ADDRPAGE1/2 shadow; -1 = unknown. */
+	s16 page1_cache[SDW_MAX_DEVICES + 1];
+	s16 page2_cache[SDW_MAX_DEVICES + 1];
 };
 
 struct qcom_swrm_data {
@@ -630,6 +633,10 @@ static void qcom_swrm_set_slave_dev_num(struct sdw_bus *bus,
 		mutex_lock(&bus->bus_lock);
 		set_bit(devnum, bus->assigned);
 		mutex_unlock(&bus->bus_lock);
+
+		/* Re-attach resets SCP_ADDRPAGE1/2 to defaults; invalidate. */
+		ctrl->page1_cache[devnum] = -1;
+		ctrl->page2_cache[devnum] = -1;
 	}
 }
 
@@ -976,17 +983,25 @@ static enum sdw_command_response qcom_swrm_xfer_msg(struct sdw_bus *bus,
 	int ret, i, len;
 
 	if (msg->page) {
-		ret = qcom_swrm_cmd_fifo_wr_cmd(ctrl, msg->addr_page1,
-						msg->dev_num,
-						SDW_SCP_ADDRPAGE1);
-		if (ret)
-			return ret;
+		if (ctrl->page1_cache[msg->dev_num] != msg->addr_page1) {
+			ret = qcom_swrm_cmd_fifo_wr_cmd(ctrl, msg->addr_page1,
+							msg->dev_num,
+							SDW_SCP_ADDRPAGE1);
+			if (ret)
+				return ret;
 
-		ret = qcom_swrm_cmd_fifo_wr_cmd(ctrl, msg->addr_page2,
-						msg->dev_num,
-						SDW_SCP_ADDRPAGE2);
-		if (ret)
-			return ret;
+			ctrl->page1_cache[msg->dev_num] = msg->addr_page1;
+		}
+
+		if (ctrl->page2_cache[msg->dev_num] != msg->addr_page2) {
+			ret = qcom_swrm_cmd_fifo_wr_cmd(ctrl, msg->addr_page2,
+							msg->dev_num,
+							SDW_SCP_ADDRPAGE2);
+			if (ret)
+				return ret;
+
+			ctrl->page2_cache[msg->dev_num] = msg->addr_page2;
+		}
 	}
 
 	if (msg->flags == SDW_MSG_FLAG_READ) {
@@ -1561,6 +1576,9 @@ static int qcom_swrm_probe(struct platform_device *pdev)
 	if (!ctrl)
 		return -ENOMEM;
 
+	memset(ctrl->page1_cache, 0xff, sizeof(ctrl->page1_cache));
+	memset(ctrl->page2_cache, 0xff, sizeof(ctrl->page2_cache));
+
 	data = of_device_get_match_data(dev);
 	ctrl->max_reg = data->max_reg;
 	ctrl->reg_layout = data->reg_layout;
-- 
2.53.0