linux-kernel - Re: [PATCH v3 2/3] media: mediatek: vcodec: support extended h264 decode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <90fe898c-0352-46da-aee3-898cdf2b5d26@collabora.com>
Date: Thu, 7 Nov 2024 11:52:49 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Yunfei Dong <yunfei.dong@...iatek.com>,
 Nícolas F . R . A . Prado <nfraprado@...labora.com>,
 Sebastian Fricke <sebastian.fricke@...labora.com>,
 Nicolas Dufresne <nicolas.dufresne@...labora.com>,
 Hans Verkuil <hverkuil-cisco@...all.nl>,
 Benjamin Gaignard <benjamin.gaignard@...labora.com>,
 Nathan Hebert <nhebert@...omium.org>
Cc: Hsin-Yi Wang <hsinyi@...omium.org>, Chen-Yu Tsai <wenst@...omium.org>,
 Fritz Koenig <frkoenig@...omium.org>, Daniel Vetter <daniel@...ll.ch>,
 Steve Cho <stevecho@...omium.org>, linux-media@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
 Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH v3 2/3] media: mediatek: vcodec: support extended h264
 decode

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@...iatek.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