Re: [PATCH v5 5/9] soundwire: qcom: update register read/write routine

2021-03-30 Thread Vinod Koul
On 26-03-21, 06:39, Srinivas Kandagatla wrote:

> + swrm->reg_write(swrm, SWRM_CMD_FIFO_WR_CMD, val);
> +
> + /* version 1.3 or less */
> + if (swrm->version <= 0x0103)
> + usleep_range(150, 155);
> +
> + if (cmd_id == SWR_BROADCAST_CMD_ID) {
> + /*
> +  * sleep for 10ms for MSM soundwire variant to allow broadcast
> +  * command to complete.
> +  */
> + ret = wait_for_completion_timeout(>broadcast,
> +   msecs_to_jiffies(TIMEOUT_MS));
> + if (!ret)
> + ret = SDW_CMD_IGNORED;
> + else
> + ret = SDW_CMD_OK;
>  
> - if (!ret) {
> - ret = SDW_CMD_IGNORED;
> - goto err;
>   } else {
>   ret = SDW_CMD_OK;
>   }

Maybe add a comment here that we dont get status so write is assumed to
be OK

-- 
~Vinod


[PATCH v5 5/9] soundwire: qcom: update register read/write routine

2021-03-26 Thread Srinivas Kandagatla
In the existing code every soundwire register read and register write
are kinda blocked. Each of these are using a special command id that
generates interrupt after it successfully finishes. This is really
overhead, limiting and not really necessary unless we are doing
something special.

We can simply read/write the fifo that should also give exactly
what we need! This will also allow to read/write registers in
interrupt context, which was not possible with the special
command approach.

With previous approach number of interrupts generated
after enumeration are around 130:
$ cat /proc/interrupts  | grep soundwire
 21: 130 0 0 0 0 0 0 0 GICv3 234 Edge  soundwire

after this patch they are just 3 interrupts
$ cat /proc/interrupts  | grep soundwire
 21: 3 0 0 0 0 0 0 0 GICv3 234 Edge  soundwire

This has significantly not only reduced interrupting CPU during enumeration
but also during streaming!

Signed-off-by: Srinivas Kandagatla 
Reviewed-by: Pierre-Louis Bossart 
---
 drivers/soundwire/qcom.c | 178 ++-
 1 file changed, 99 insertions(+), 79 deletions(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 2bcb4362f0e0..0cbd611fb8c6 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -38,11 +38,13 @@
 #define SWRM_CMD_FIFO_WR_CMD   0x300
 #define SWRM_CMD_FIFO_RD_CMD   0x304
 #define SWRM_CMD_FIFO_CMD  0x308
+#define SWRM_CMD_FIFO_FLUSH0x1
 #define SWRM_CMD_FIFO_STATUS   0x30C
 #define SWRM_CMD_FIFO_CFG_ADDR 0x314
 #define SWRM_CONTINUE_EXEC_ON_CMD_IGNORE   BIT(31)
 #define SWRM_RD_WR_CMD_RETRIES 0x7
 #define SWRM_CMD_FIFO_RD_FIFO_ADDR 0x318
+#define SWRM_RD_FIFO_CMD_ID_MASK   GENMASK(11, 8)
 #define SWRM_ENUMERATOR_CFG_ADDR   0x500
 #define SWRM_MCP_FRAME_CTRL_BANK_ADDR(m)   (0x101C + 0x40 * (m))
 #define SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_BMSK GENMASK(2, 0)
@@ -78,13 +80,16 @@
 #define SWRM_SPECIAL_CMD_ID0xF
 #define MAX_FREQ_NUM   1
 #define TIMEOUT_MS (2 * HZ)
-#define QCOM_SWRM_MAX_RD_LEN   0xf
+#define QCOM_SWRM_MAX_RD_LEN   0x1
 #define QCOM_SDW_MAX_PORTS 14
 #define DEFAULT_CLK_FREQ   960
 #define SWRM_MAX_DAIS  0xF
 #define SWR_INVALID_PARAM 0xFF
 #define SWR_HSTOP_MAX_VAL 0xF
 #define SWR_HSTART_MIN_VAL 0x0
+#define SWR_BROADCAST_CMD_ID0x0F
+#define SWR_MAX_CMD_ID 14
+#define MAX_FIFO_RD_RETRY 3
 
 struct qcom_swrm_port_config {
u8 si;
@@ -103,10 +108,8 @@ struct qcom_swrm_ctrl {
struct device *dev;
struct regmap *regmap;
void __iomem *mmio;
-   struct completion *comp;
+   struct completion broadcast;
struct work_struct slave_work;
-   /* read/write lock */
-   spinlock_t comp_lock;
/* Port alloc/free lock */
struct mutex port_lock;
struct clk *hclk;
@@ -120,6 +123,8 @@ struct qcom_swrm_ctrl {
int rows_index;
unsigned long dout_port_mask;
unsigned long din_port_mask;
+   u8 rcmd_id;
+   u8 wcmd_id;
struct qcom_swrm_port_config pconfig[QCOM_SDW_MAX_PORTS];
struct sdw_stream_runtime *sruntime[SWRM_MAX_DAIS];
enum sdw_slave_status status[SDW_MAX_DEVICES];
@@ -198,77 +203,105 @@ static int qcom_swrm_cpu_reg_write(struct qcom_swrm_ctrl 
*ctrl, int reg,
return SDW_CMD_OK;
 }
 
-static int qcom_swrm_cmd_fifo_wr_cmd(struct qcom_swrm_ctrl *ctrl, u8 cmd_data,
-u8 dev_addr, u16 reg_addr)
+static u32 swrm_get_packed_reg_val(u8 *cmd_id, u8 cmd_data,
+  u8 dev_addr, u16 reg_addr)
 {
-   DECLARE_COMPLETION_ONSTACK(comp);
-   unsigned long flags;
u32 val;
-   int ret;
-
-   spin_lock_irqsave(>comp_lock, flags);
-   ctrl->comp = 
-   spin_unlock_irqrestore(>comp_lock, flags);
-   val = SWRM_REG_VAL_PACK(cmd_data, dev_addr,
-   SWRM_SPECIAL_CMD_ID, reg_addr);
-   ret = ctrl->reg_write(ctrl, SWRM_CMD_FIFO_WR_CMD, val);
-   if (ret)
-   goto err;
-
-   ret = wait_for_completion_timeout(ctrl->comp,
- msecs_to_jiffies(TIMEOUT_MS));
+   u8 id = *cmd_id;
 
-   if (!ret)
-   ret = SDW_CMD_IGNORED;
-   else
-   ret = SDW_CMD_OK;
-err:
-   spin_lock_irqsave(>comp_lock, flags);
-   ctrl->comp = NULL;
-   spin_unlock_irqrestore(>comp_lock, flags);
+   if (id != SWR_BROADCAST_CMD_ID) {
+   if (id < SWR_MAX_CMD_ID)
+   id += 1;
+   else
+   id = 0;
+   *cmd_id = id;
+   }
+