Messages in this thread | | | From | Jason-JH Lin (林睿祥) <> | Subject | Re: [PATCH v2 3/9] soc: mailbox: Add cmdq_pkt_logic_command to support math operation | Date | Tue, 24 Oct 2023 16:59:13 +0000 |
| |
Hi Fei,
Thanks for the reviews.
On Mon, 2023-10-23 at 16:26 +0800, Fei Shao wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > Hi Jason, > > On Mon, Oct 23, 2023 at 12:39 PM Jason-JH.Lin < > jason-jh.lin@mediatek.com> wrote: > > > > Add cmdq_pkt_logic_command to support match operation. > s/match/math/ > I'll fix this typo. Thanks.
> > > > cmdq_pkt_logic_command can append logic command to the CMDQ packet, > > ask GCE to execute a arithematic calculate instruction, > s/arithematic/arithmetic/ > I'll fix this typo. Thanks.
> > such as add, subtract, multiply, AND, OR and NOT, etc. > > > > Note that all instructions just accept unsigned calculate. > s/calculate/calculation/ > > > Or I'd rephrase it as: > Note that all arithmetic instructions are unsigned calculations. > OK, I'll rephrase as yours. Thanks!
> > 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> > > --- > > drivers/soc/mediatek/mtk-cmdq-helper.c | 36 ++++++++++++++++++++++ > > include/linux/soc/mediatek/mtk-cmdq.h | 41 > ++++++++++++++++++++++++++ > > 2 files changed, 77 insertions(+) > > > > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c > b/drivers/soc/mediatek/mtk-cmdq-helper.c > > index b0cd071c4719..5194d66dfc0a 100644 > > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c > > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c > > @@ -13,9 +13,18 @@ > > #define CMDQ_WRITE_ENABLE_MASK BIT(0) > > #define CMDQ_POLL_ENABLE_MASK BIT(0) > > #define CMDQ_EOC_IRQ_EN BIT(0) > > +#define CMDQ_IMMEDIATE_VALUE 0 > > #define CMDQ_REG_TYPE 1 > > #define CMDQ_JUMP_RELATIVE 1 > > > > +#define CMDQ_OPERAND_GET_IDX_VALUE(operand) \ > > + ({ \ > > + struct cmdq_operand *op = operand; \ > > + op->reg ? op->idx : op->value; \ > > + }) > `((operand)->reg ? (operand)->idx : (operand)->value)` fits in one > line. >
I had use that way, but it will get this CHECK warning: CHECK: Macro argument reuse '_stat' - possible side-effects?
So I change to current way.
> > +#define CMDQ_OPERAND_TYPE(operand) \ > > + ((operand)->reg ? CMDQ_REG_TYPE : CMDQ_IMMEDIATE_VALUE) > > + > > struct cmdq_instruction { > > union { > > u32 value; > > @@ -380,6 +389,33 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, > u8 subsys, > > } > > EXPORT_SYMBOL(cmdq_pkt_poll_mask); > > > > +int cmdq_pkt_logic_command(struct cmdq_pkt *pkt, enum > CMDQ_LOGIC_ENUM s_op, > > + u16 result_reg_idx, > > + struct cmdq_operand *left_operand, > > + struct cmdq_operand *right_operand) > > +{ > > + struct cmdq_instruction inst = { {0} }; > > + u32 left_idx_value; > > + u32 right_idx_value; > > + > > + if (!left_operand || !right_operand) > > + 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.arg_c = right_idx_value; > > + inst.src_reg = left_idx_value; > > + inst.reg_dst = result_reg_idx; > > + > > + 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 a253c001c861..ea4fadfb5443 100644 > > --- a/include/linux/soc/mediatek/mtk-cmdq.h > > +++ b/include/linux/soc/mediatek/mtk-cmdq.h > > @@ -26,6 +26,30 @@ > > > > struct cmdq_pkt; > > > > +enum CMDQ_LOGIC_ENUM { > Use lower case, and perhaps use `cmdq_logic_type` or > `cmdq_operator_type`. > Thanks! I think I would change to `cmdq_logic_op`.
Regards, Jason-JH.Lin
> Regards, > Fei
| |