Re: [PATCH 04/10] media: mtk-vcodec: venc: handle firmware version field

Tiffany Lin
2020-06-16T09:04:38Z

On Wed, 2020-05-20 at 17:27 +0900, Alexandre Courbot wrote:
> Firmwares for encoders newer than MT8173 will include an ABI version
> number in their initialization ack message. Add the capacity to manage
> it and make initialization fail if the firmware ABI is of a version that
> we don't support.
>
> For MT8173, this ABI version field is reserved and thus undefined ; thus
> ignore it on this chip. There should only be one firmware version available
> for it anyway.
>

Acked-by: Tiffany Lin <tiffany.lin@mediatek.com>

> Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
> ---
> .../platform/mtk-vcodec/mtk_vcodec_drv.h | 8 ++++++
> .../platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 1 +
> .../media/platform/mtk-vcodec/venc_ipi_msg.h | 9 ++++---
> .../media/platform/mtk-vcodec/venc_vpu_if.c | 27 ++++++++++++++++---
> 4 files changed, 38 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> index 45c8adfc6a0c..e7b155e7432e 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> @@ -300,13 +300,21 @@ struct mtk_vcodec_ctx {
>
> };
>
> +enum mtk_chip {
> + MTK_MT8173,
> +};
> +
> /**
> * struct mtk_vcodec_enc_pdata - compatible data for each IC
> *
> + * @chip: chip this encoder is compatible with
> + *
> * @uses_ext: whether the encoder uses the extended firmware messaging format
> * @has_lt_irq: whether the encoder uses the LT irq
> */
> struct mtk_vcodec_enc_pdata {
> + enum mtk_chip chip;
> +
> bool uses_ext;
> bool has_lt_irq;
> };
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
> index 922bc8883811..c1365209263e 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
> @@ -378,6 +378,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> }
>
> static const struct mtk_vcodec_enc_pdata mt8173_pdata = {
> + .chip = MTK_MT8173,
> .has_lt_irq = true,
> };
>
> diff --git a/drivers/media/platform/mtk-vcodec/venc_ipi_msg.h b/drivers/media/platform/mtk-vcodec/venc_ipi_msg.h
> index 4cafbf92d9cd..31a3c76f7d0d 100644
> --- a/drivers/media/platform/mtk-vcodec/venc_ipi_msg.h
> +++ b/drivers/media/platform/mtk-vcodec/venc_ipi_msg.h
> @@ -131,16 +131,17 @@ struct venc_vpu_ipi_msg_common {
> * @venc_inst: AP encoder instance (struct venc_vp8_inst/venc_h264_inst *)
> * @vpu_inst_addr: VPU encoder instance addr
> * (struct venc_vp8_vsi/venc_h264_vsi *)
> - * @reserved: reserved for future use. vpu is running in 32bit. Without
> - * this reserved field, if kernel run in 64bit. this struct size
> - * will be different between kernel and vpu
> + * @venc_abi_version: ABI version of the firmware. Kernel can use it to
> + * ensure that it is compatible with the firmware.
> + * For MT8173 the value of this field is undefined and
> + * should not be used.
> */
> struct venc_vpu_ipi_msg_init {
> uint32_t msg_id;
> uint32_t status;
> uint64_t venc_inst;
> uint32_t vpu_inst_addr;
> - uint32_t reserved;
> + uint32_t venc_abi_version;
> };
>
> /**
> diff --git a/drivers/media/platform/mtk-vcodec/venc_vpu_if.c b/drivers/media/platform/mtk-vcodec/venc_vpu_if.c
> index 6c77bf025172..472503701003 100644
> --- a/drivers/media/platform/mtk-vcodec/venc_vpu_if.c
> +++ b/drivers/media/platform/mtk-vcodec/venc_vpu_if.c
> @@ -4,6 +4,7 @@
> * Author: PoChun Lin <pochun.lin@mediatek.com>
> */
>
> +#include "mtk_vcodec_drv.h"
> #include "mtk_vcodec_fw.h"
> #include "venc_ipi_msg.h"
> #include "venc_vpu_if.h"
> @@ -15,6 +16,23 @@ static void handle_enc_init_msg(struct venc_vpu_inst *vpu, const void *data)
> vpu->inst_addr = msg->vpu_inst_addr;
> vpu->vsi = mtk_vcodec_fw_map_dm_addr(vpu->ctx->dev->fw_handler,
> msg->vpu_inst_addr);
> +
> + /* Firmware version field value is unspecified on MT8173. */
> + if (vpu->ctx->dev->venc_pdata->chip == MTK_MT8173)
> + return;
> +
> + /* Check firmware version. */
> + mtk_vcodec_debug(vpu, "firmware version: 0x%x\n",
> + msg->venc_abi_version);
> + switch (msg->venc_abi_version) {
> + case 1:
> + break;
> + default:
> + mtk_vcodec_err(vpu, "unhandled firmware version 0x%x\n",
> + msg->venc_abi_version);
> + vpu->failure = 1;
> + break;
> + }
> }
>
> static void handle_enc_encode_msg(struct venc_vpu_inst *vpu, const void *data)
> @@ -35,6 +53,11 @@ static void vpu_enc_ipi_handler(void *data, unsigned int len, void *priv)
> mtk_vcodec_debug(vpu, "msg_id %x inst %p status %d",
> msg->msg_id, vpu, msg->status);
>
> + vpu->signaled = 1;
> + vpu->failure = (msg->status != VENC_IPI_MSG_STATUS_OK);
> + if (vpu->failure)
> + goto failure;
> +
> switch (msg->msg_id) {
> case VPU_IPIMSG_ENC_INIT_DONE:
> handle_enc_init_msg(vpu, data);
> @@ -51,9 +74,7 @@ static void vpu_enc_ipi_handler(void *data, unsigned int len, void *priv)
> break;
> }
>
> - vpu->signaled = 1;
> - vpu->failure = (msg->status != VENC_IPI_MSG_STATUS_OK);
> -
> +failure:
> mtk_vcodec_debug_leave(vpu);
> }
>