diff mbox series

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

Message ID 20240525230810.24623-4-jason-jh.lin@mediatek.com (mailing list archive)
State New
Headers show
Series Add CMDQ secure driver for SVP | expand

Commit Message

Jason-JH Lin (林睿祥) May 25, 2024, 11:08 p.m. UTC 
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@mediatek.com>
Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
---
 drivers/soc/mediatek/mtk-cmdq-helper.c | 36 ++++++++++++++++++++++
 include/linux/soc/mediatek/mtk-cmdq.h  | 42 ++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)

Comments

AngeloGioacchino Del Regno May 28, 2024, 10:24 a.m. UTC | #1 
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@mediatek.com>
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
>   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...

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.

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
Jason-JH Lin (林睿祥) May 28, 2024, 3:52 p.m. UTC | #2 
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@mediatek.com>
> > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> > ---
> >   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?

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
> 
> 
> 
>
AngeloGioacchino Del Regno June 24, 2024, 11:39 a.m. UTC | #3 
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@mediatek.com>
>>> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
>>> ---
>>>    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
>>
>>
>>
>>
Jason-JH Lin (林睿祥) June 25, 2024, 5:59 a.m. UTC | #4 
On Mon, 2024-06-24 at 13:39 +0200, AngeloGioacchino Del Regno wrote:
> 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@mediatek.com>
> > > > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> > > > ---
> > > >    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.
> 

No problem!
Since other patches in this series are still cooking, I'll resend this
patch separately. Thanks!

Regards,
Jason-JH.Lin

> 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
> > > 
> > > 
> > > 
> > > 
> 
>
diff mbox series

Patch

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; \
+	})
+#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