Re: [PATCH v6 3/8] soc: mediatek: cmdq: Add cmdq_pkt_logic_command to support math operation

AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx> · Mon, 24 Jun 2024 13:39:28 +0200

Il 28/05/24 17:52, Jason-JH Lin (林睿祥) ha scritto:
Hi Angelo,

On Tue, 2024-05-28 at 12:24 +0200, AngeloGioacchino Del Regno wrote:
Il 26/05/24 01:08, Jason-JH.Lin ha scritto:
Add cmdq_pkt_logic_command to support math operation.

cmdq_pkt_logic_command can append logic command to the CMDQ packet,
ask GCE to execute a arithmetic calculate instruction,
such as add, subtract, multiply, AND, OR and NOT, etc.

Note that all arithmetic instructions are unsigned calculations.
If there are any overflows, GCE will sent the invalid IRQ to notify
CMDQ driver.

Signed-off-by: Jason-JH.Lin <jason-jh.lin@xxxxxxxxxxxx>
Signed-off-by: Hsiao Chien Sung <shawn.sung@xxxxxxxxxxxx>
---
   drivers/soc/mediatek/mtk-cmdq-helper.c | 36
++++++++++++++++++++++
   include/linux/soc/mediatek/mtk-cmdq.h  | 42
++++++++++++++++++++++++++
   2 files changed, 78 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 046522664dc1..42fae05f61a8 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -15,10 +15,19 @@
   /* dedicate the last GPR_R15 to assign the register address to be
poll */
   #define CMDQ_POLL_ADDR_GPR	(15)
   #define CMDQ_EOC_IRQ_EN		BIT(0)
+#define CMDQ_IMMEDIATE_VALUE	0
   #define CMDQ_REG_TYPE		1
   #define CMDQ_JUMP_RELATIVE	0
   #define CMDQ_JUMP_ABSOLUTE	1
   
+#define CMDQ_OPERAND_GET_IDX_VALUE(operand) \
+	({ \
+		struct cmdq_operand *op = operand; \
+		op->reg ? op->idx : op->value; \
+	})

I think this CMDQ_OPERAND_GET_IDX_VALUE would be better expressed as
a (inline?)
function instead...


Yes, I can change them to static inline function to pass their type
directly.

static inline u16 cmdq_operand_get_idx_value(struct cmdq_operand *op)
{
	return op->reg ? op->idx : op->value;
}

and maybe the same for the other definition too..

static inline u16 cmdq_operand_type(struct cmdq_operand *op)
{
	return op->reg ? CMDQ_REG_TYPE : CMDQ_IMMEDIATE_VALUE;
}

but definitely the first one is what I don't like much, the second
one can pass.


You mean '#define CMDQ_OPERAND_GET_IDX_VALUE()' is the first one or
'static inline u16 cmdq_operand_get_idx_value()' is the first one?


I'm sorry, your reply slipped through for some reason and I've read it just now.

I mean I don't like the macros (#define CMDQ_OPERAND_GET_IDX_VALUE(..) and
CMDQ_OPERAND_TYPE(..)) :-)

Please use static inline functions and resend; if you can do that this week, I
can pick the commit before I send the PR.

P.S.: And you're right, re-reading my own reply, it was a bit ambiguous, so
I'm again sorry for that, will be clearer next time.

Thanks,
Angelo

Regards,
Jason-JH.Lin

Cheers,
Angelo

+#define CMDQ_OPERAND_TYPE(operand) \
+	((operand)->reg ? CMDQ_REG_TYPE : CMDQ_IMMEDIATE_VALUE)
+
   struct cmdq_instruction {
   	union {
   		u32 value;
@@ -461,6 +470,33 @@ int cmdq_pkt_poll_addr(struct cmdq_pkt *pkt,
dma_addr_t addr, u32 value, u32 mas
   }
   EXPORT_SYMBOL(cmdq_pkt_poll_addr);
   
+int cmdq_pkt_logic_command(struct cmdq_pkt *pkt, u16
result_reg_idx,
+			   struct cmdq_operand *left_operand,
+			   enum cmdq_logic_op s_op,
+			   struct cmdq_operand *right_operand)
+{
+	struct cmdq_instruction inst = { {0} };
+	u32 left_idx_value;
+	u32 right_idx_value;
+
+	if (!left_operand || !right_operand || s_op >= CMDQ_LOGIC_MAX)
+		return -EINVAL;
+
+	left_idx_value = CMDQ_OPERAND_GET_IDX_VALUE(left_operand);
+	right_idx_value = CMDQ_OPERAND_GET_IDX_VALUE(right_operand);
+	inst.op = CMDQ_CODE_LOGIC;
+	inst.dst_t = CMDQ_REG_TYPE;
+	inst.src_t = CMDQ_OPERAND_TYPE(left_operand);
+	inst.arg_c_t = CMDQ_OPERAND_TYPE(right_operand);
+	inst.sop = s_op;
+	inst.reg_dst = result_reg_idx;
+	inst.src_reg = left_idx_value;
+	inst.arg_c = right_idx_value;
+
+	return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_logic_command);
+
   int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
   {
   	struct cmdq_instruction inst = {};
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index d4a8e34505e6..5bee6f7fc400 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -25,6 +25,31 @@
   
   struct cmdq_pkt;
   
+enum cmdq_logic_op {
+	CMDQ_LOGIC_ASSIGN = 0,
+	CMDQ_LOGIC_ADD = 1,
+	CMDQ_LOGIC_SUBTRACT = 2,
+	CMDQ_LOGIC_MULTIPLY = 3,
+	CMDQ_LOGIC_XOR = 8,
+	CMDQ_LOGIC_NOT = 9,
+	CMDQ_LOGIC_OR = 10,
+	CMDQ_LOGIC_AND = 11,
+	CMDQ_LOGIC_LEFT_SHIFT = 12,
+	CMDQ_LOGIC_RIGHT_SHIFT = 13,
+	CMDQ_LOGIC_MAX,
+};
+
+struct cmdq_operand {
+	/* register type */
+	bool reg;
+	union {
+		/* index */
+		u16 idx;
+		/* value */
+		u16 value;
+	};
+};
+
   struct cmdq_client_reg {
   	u8 subsys;
   	u16 offset;
@@ -272,6 +297,23 @@ int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8
subsys,
   int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
   		       u16 offset, u32 value, u32 mask);
   
+/**
+ * cmdq_pkt_logic_command() - Append logic command to the CMDQ
packet, ask GCE to
+ *		          execute an instruction that store the result
of logic operation
+ *		          with left and right operand into
result_reg_idx.
+ * @pkt:		the CMDQ packet
+ * @result_reg_idx:	SPR index that store operation result
of left_operand and right_operand
+ * @left_operand:	left operand
+ * @s_op:		the logic operator enum
+ * @right_operand:	right operand
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_logic_command(struct cmdq_pkt *pkt, u16
result_reg_idx,
+			   struct cmdq_operand *left_operand,
+			   enum cmdq_logic_op s_op,
+			   struct cmdq_operand *right_operand);
+
   /**
    * cmdq_pkt_assign() - Append logic assign command to the CMDQ
packet, ask GCE
    *		       to execute an instruction that set a
constant value into













