mbox series

[v3,0/6] media: mediatek: vcodec: support h264 extend vsi

Message ID 20241107074603.31998-1-yunfei.dong@mediatek.com
Headers show
Series media: mediatek: vcodec: support h264 extend vsi | expand

Message

Yunfei Dong Nov. 7, 2024, 7:45 a.m. UTC 
The working buffer address start and end are calculated in kernel
side currently, the address end can't be calculated if the driver
only getting the address file handle, not the real physical address.
Need to send the extended vsi to firmware to calculate the address
end.

Re-construct some interface and add configuration to support extend
and non extend driver at the same time. Needn't to parse nal info for
extended architecture.
---
This patch series depends on:
[1] https://patchwork.kernel.org/project/linux-mediatek/cover/20241012064333.27269-1-yunfei.dong@mediatek.com

---
compared with v2:
- squash patch 2/3/4 together
- re-write commit message for patch 1

compared with v1:
- combine some pathes together for patch 2
- re-write patch 4
---
Yunfei Dong (3):
  media: mediatek: vcodec: remove vsi operation in common interface
  media: mediatek: vcodec: support extended h264 decode
  media: mediatek: vcodec: add description for vsi struct

 .../vcodec/decoder/mtk_vcodec_dec_drv.h       |   2 +
 .../decoder/vdec/vdec_h264_req_multi_if.c     | 505 +++++++++++++++++-
 2 files changed, 482 insertions(+), 25 deletions(-)

Comments

AngeloGioacchino Del Regno Nov. 7, 2024, 10:52 a.m. UTC | #1 
Il 07/11/24 08:45, Yunfei Dong ha scritto:
> The address end of working buffer can't be calculated directly with buffer
> size in kernel for some special architecture. Adding new extend vsi_ex to
> calculate the address end in firmware.
> Adding capability to separate extend and non extend driver for different
> platform.
> At last, hardware can parse the syntax to get nal information in firmware
> for extend architecture, needn't to parse it again in kernel.
> 
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> ---
>   .../vcodec/decoder/mtk_vcodec_dec_drv.h       |   2 +
>   .../decoder/vdec/vdec_h264_req_multi_if.c     | 487 +++++++++++++++++-
>   2 files changed, 472 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
> index 886fa385e2e6..1e697bc810b0 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
> @@ -17,6 +17,7 @@
>   
>   #define IS_VDEC_LAT_ARCH(hw_arch) ((hw_arch) >= MTK_VDEC_LAT_SINGLE_CORE)
>   #define IS_VDEC_INNER_RACING(capability) ((capability) & MTK_VCODEC_INNER_RACING)
> +#define IS_VDEC_SUPPORT_EX(capability) ((capability) & MTK_VDEC_IS_SUPPORT_EX)
>   
>   enum mtk_vcodec_dec_chip_name {
>   	MTK_VDEC_INVAL = 0,
> @@ -42,6 +43,7 @@ enum mtk_vdec_format_types {
>   	MTK_VDEC_FORMAT_HEVC_FRAME = 0x1000,
>   	MTK_VCODEC_INNER_RACING = 0x20000,
>   	MTK_VDEC_IS_SUPPORT_10BIT = 0x40000,
> +	MTK_VDEC_IS_SUPPORT_EX = 0x80000,
>   };
>   
>   /*
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
> index 851a8490b828..d0aecd9621d9 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c

..snip..

>   	inst->vsi_ctx.dec.y_fb_dma = y_fb_dma;
> @@ -816,8 +1260,17 @@ static int vdec_h264_slice_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
>   	if (!h_vdec)
>   		return -EINVAL;
>   
> -	if (inst->ctx->dev->vdec_pdata->hw_arch == MTK_VDEC_PURE_SINGLE_CORE)
> -		ret = vdec_h264_slice_single_decode(h_vdec, bs, unused, res_chg);
> +	if (inst->ctx->dev->vdec_pdata->hw_arch == MTK_VDEC_PURE_SINGLE_CORE) {
> +		if (IS_VDEC_SUPPORT_EX(inst->ctx->dev->dec_capability))
> +			ret = vdec_h264_slice_single_decode_ex(h_vdec, bs, unused, res_chg);

I wonder if we can use function pointers here, as I feel like vcodec is becoming
a bit "full of branches here and there"...

The rough idea is:

/* there, or somewhere that's called only once in the driver lifetime anyway */
static int vdec_h264_slice_init(.....)
{
	........

	if (hw_arch == MTK_VDEC_PURE_SINGLE_CORE) {
		if (inst->ctx->dev->dec_capability & MTK_VDEC_IS_SUPPORT_EX)
			inst->decode = vdec_h264_slice_single_decode_ex;
		else
			inst->decode = vdec_h264_slice_single_decode;
	}  else {
		if (inst->ctx->dev->dec_capability & MTK_VDEC_IS_SUPPORT_EX)
			inst->decode = vdec_h264_slice_lat_decode_ex;
		else
			inst->decode = vdec_h264_slice_lat_decode;
	}

	......
}

static int vdec_h264_slice_decode(...)
{
	if (!inst)
		return -EINVAL;

	return inst->decode( .... )
}

...less branches during decoding *of each frame* :-)

Cheers,
Angelo