| Message ID | 20231023043751.17114-4-jason-jh.lin@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add CMDQ secure driver for SVP | expand |
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/ > > cmdq_pkt_logic_command can append logic command to the CMDQ packet, > ask GCE to execute a arithematic calculate instruction, s/arithematic/arithmetic/ > 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. > 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. > +#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`. Regards, Fei
On Mon, Oct 23, 2023 at 4:26 PM Fei Shao <fshao@chromium.org> wrote: > > Hi Jason, > > On Mon, Oct 23, 2023 at 12:39 PM Jason-JH.Lin <jason-jh.lin@mediatek.com> wrote: <snip> > > 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`. I overlooked the CMDQ_CODE_LOGIC line, so cmdq_logic_type makes more sense. Regards, Fei
Il 23/10/23 06:37, Jason-JH.Lin ha scritto: > Add cmdq_pkt_logic_command to support match operation. > > cmdq_pkt_logic_command can append logic command to the CMDQ packet, > ask GCE to execute a arithematic calculate instruction, > such as add, subtract, multiply, AND, OR and NOT, etc. > > Note that all instructions just accept unsigned calculate. > 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> Please always run `git log` on the file that you're changing to properly format the commit title. In this case, this should be: soc: mediatek: cmdq: ..... > --- > 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; \ > + }) > +#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) I would also check that the requested logic operation is actually implemented: if (!left_operand || !right_operand || s_op >= CMDQ_LOGIC_MAX) return -EINVAL; > + 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 { Please no capital letters in enumeration names. Besides, it's clearer if you named this enum cmdq_logic_operation { or 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, You should also add a `CMDQ_LOGIC_MAX,` here to use it in the code for validation. > +}; > + > +struct cmdq_operand { > + /* register type */ > + bool reg; > + union { > + /* index */ > + u16 idx; > + /* value */ > + u16 value; > + }; > +}; > + > struct cmdq_client_reg { > u8 subsys; > u16 offset; > @@ -244,6 +268,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 > + * @s_op: the logic operator enum > + * @result_reg_idx: SPR index that store operation result of left_operand and right_operand > + * @left_operand: left operand > + * @right_operand: right operand > + * I think that there's a way to dramatically increase human readability of calls to this function: being this a request to perform calculations, it would be way easier to read it like an actual calculation :-) cmdq_pkt_logic_command(pkt, result, op_x, CMDQ_LOGIC_ADD, op_y); At least in my head, this easily resembles "result = op_x + op_y". I therefore propose to change this to: int cmdq_pkt_logic_command(struct cmdq_pkt *pkt, u16 result_reg_idx, struct cmdq_operand *left_operand, enum cmdq_logic_operation s_op, struct cmdq_operand *right_operand); > + * Return: 0 for success; else the error code is returned > + */ > +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); > + > /** > * cmdq_pkt_assign() - Append logic assign command to the CMDQ packet, ask GCE > * to execute an instruction that set a constant value into Regards, Angelo
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
Hi Angelo, Thanks for the reviews. On Mon, 2023-10-23 at 11:50 +0200, AngeloGioacchino Del Regno wrote: > Il 23/10/23 06:37, Jason-JH.Lin ha scritto: > > Add cmdq_pkt_logic_command to support match operation. > > > > cmdq_pkt_logic_command can append logic command to the CMDQ packet, > > ask GCE to execute a arithematic calculate instruction, > > such as add, subtract, multiply, AND, OR and NOT, etc. > > > > Note that all instructions just accept unsigned calculate. > > 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> > > Please always run `git log` on the file that you're changing to > properly format the > commit title. > > In this case, this should be: > > soc: mediatek: cmdq: ..... I think it happended because I deleted the `\n` in the first line of the title when I review these patches in `git send-email` cmd. I won't do that in the next version. Thanks. > > > --- > > 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; \ > > + }) > > +#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) > > I would also check that the requested logic operation is actually > implemented: > > if (!left_operand || !right_operand || s_op >= CMDQ_LOGIC_MAX) > return -EINVAL; > OK, I'll add this check. > > + 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 { > > Please no capital letters in enumeration names. > Besides, it's clearer if you named this > > enum cmdq_logic_operation { > > or > > enum cmdq_logic_op { OK, I'll change to this one. Thanks. > > > + 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, > > You should also add a `CMDQ_LOGIC_MAX,` here to use it in the code > for validation. OK, I'll add this. Thanks. > > > +}; > > + > > +struct cmdq_operand { > > + /* register type */ > > + bool reg; > > + union { > > + /* index */ > > + u16 idx; > > + /* value */ > > + u16 value; > > + }; > > +}; > > + > > struct cmdq_client_reg { > > u8 subsys; > > u16 offset; > > @@ -244,6 +268,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 > > + * @s_op: the logic operator enum > > + * @result_reg_idx: SPR index that store operation result > > of left_operand and right_operand > > + * @left_operand: left operand > > + * @right_operand: right operand > > + * > > I think that there's a way to dramatically increase human readability > of calls to > this function: being this a request to perform calculations, it would > be way easier > to read it like an actual calculation :-) > > cmdq_pkt_logic_command(pkt, result, op_x, CMDQ_LOGIC_ADD, op_y); > > At least in my head, this easily resembles "result = op_x + op_y". > > I therefore propose to change this to: > > int cmdq_pkt_logic_command(struct cmdq_pkt *pkt, u16 result_reg_idx, > struct cmdq_operand *left_operand, > enum cmdq_logic_operation s_op, > struct cmdq_operand *right_operand); > OK, I think it's a good idea. I'll change that. Thanks! Regards, Jason-JH.Lin > > + * Return: 0 for success; else the error code is returned > > + */ > > +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); > > + > > /** > > * cmdq_pkt_assign() - Append logic assign command to the CMDQ > > packet, ask GCE > > * to execute an instruction that set a > > constant value into > > > Regards, > Angelo >
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; \ + }) +#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 { + 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, +}; + +struct cmdq_operand { + /* register type */ + bool reg; + union { + /* index */ + u16 idx; + /* value */ + u16 value; + }; +}; + struct cmdq_client_reg { u8 subsys; u16 offset; @@ -244,6 +268,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 + * @s_op: the logic operator enum + * @result_reg_idx: SPR index that store operation result of left_operand and right_operand + * @left_operand: left operand + * @right_operand: right operand + * + * Return: 0 for success; else the error code is returned + */ +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); + /** * cmdq_pkt_assign() - Append logic assign command to the CMDQ packet, ask GCE * to execute an instruction that set a constant value into
Add cmdq_pkt_logic_command to support match operation. cmdq_pkt_logic_command can append logic command to the CMDQ packet, ask GCE to execute a arithematic calculate instruction, such as add, subtract, multiply, AND, OR and NOT, etc. Note that all instructions just accept unsigned calculate. 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(+)