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

[Tiffany Lin <tiffany.lin@mediatek.com>]

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);
>  }
>