Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
MIME-Version: 1.0
References: <20231023043751.17114-1-jason-jh.lin@mediatek.com> <20231023043751.17114-4-jason-jh.lin@mediatek.com>
In-Reply-To: <20231023043751.17114-4-jason-jh.lin@mediatek.com>
From:   Fei Shao <fshao@chromium.org>
Date:   Mon, 23 Oct 2023 16:26:19 +0800
Message-ID: <CAC=S1njjd+-oEgmBeWJGQC0PsbOdJ5eLLw0Mf35HOjA8UHStTw@mail.gmail.com>
Subject: Re: [PATCH v2 3/9] soc: mailbox: Add cmdq_pkt_logic_command to
 support math operation
To:     "Jason-JH.Lin" <jason-jh.lin@mediatek.com>
Cc:     Jassi Brar <jassisinghbrar@gmail.com>,
        Chun-Kuang Hu <chunkuang.hu@kernel.org>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org,
        Project_Global_Chrome_Upstream_Group@mediatek.com,
        Jeffrey Kardatzke <jkardatzke@google.com>,
        Jason-ch Chen <jason-ch.chen@mediatek.com>,
        Johnson Wang <johnson.wang@mediatek.com>,
        Singo Chang <singo.chang@mediatek.com>,
        Nancy Lin <nancy.lin@mediatek.com>,
        Shawn Sung <shawn.sung@mediatek.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Jason,

On Mon, Oct 23, 2023 at 12:39=E2=80=AFPM Jason-JH.Lin <jason-jh.lin@mediate=
k.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/mediate=
k/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 =3D 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 subs=
ys,
>  }
>  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 =3D { {0} };
> +       u32 left_idx_value;
> +       u32 right_idx_value;
> +
> +       if (!left_operand || !right_operand)
> +               return -EINVAL;
> +
> +       left_idx_value =3D CMDQ_OPERAND_GET_IDX_VALUE(left_operand);
> +       right_idx_value =3D CMDQ_OPERAND_GET_IDX_VALUE(right_operand);
> +       inst.op =3D CMDQ_CODE_LOGIC;
> +       inst.dst_t =3D CMDQ_REG_TYPE;
> +       inst.src_t =3D CMDQ_OPERAND_TYPE(left_operand);
> +       inst.arg_c_t =3D CMDQ_OPERAND_TYPE(right_operand);
> +       inst.sop =3D s_op;
> +       inst.arg_c =3D right_idx_value;
> +       inst.src_reg =3D left_idx_value;
> +       inst.reg_dst =3D 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 =3D {};
> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/me=
diatek/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