linux-kernel - Re: [Patch v2 04/15] media: s5p-mfc: Add initial support for MFCv12

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Yx+yHPFhLbaaSCAw@intel.intel>
Date:   Tue, 13 Sep 2022 00:26:36 +0200
From:   Andi Shyti <andi@...zian.org>
To:     Smitha T Murthy <smitha.t@...sung.com>
Cc:     linux-arm-kernel@...ts.infradead.org, linux-media@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        m.szyprowski@...sung.com, andrzej.hajda@...el.com,
        mchehab@...nel.org, hverkuil-cisco@...all.nl,
        ezequiel@...guardiasur.com.ar, jernej.skrabec@...il.com,
        benjamin.gaignard@...labora.com, stanimir.varbanov@...aro.org,
        dillon.minfei@...il.com, david.plowman@...pberrypi.com,
        mark.rutland@....com, robh+dt@...nel.org, krzk+dt@...nel.org,
        andi@...zian.org, alim.akhtar@...sung.com,
        aswani.reddy@...sung.com, pankaj.dubey@...sung.com,
        linux-fsd@...la.com, aakarsh.jain@...sung.com
Subject: Re: [Patch v2 04/15] media: s5p-mfc: Add initial support for MFCv12

Hi Smitha,

just a fast look here

[...]

> +/* Encoder buffer size for MFCv12 */
> +#define ENC_V120_BASE_SIZE(x, y) \
> +	(((x + 3) * (y + 3) * 8) \
> +	+ (((y * 64) + 2304) * (x + 7) / 8))

do we need any overflow control here?

[...]

>  #define MFC_V7_BIT	BIT(2)
>  #define MFC_V8_BIT	BIT(3)
>  #define MFC_V10_BIT	BIT(5)
> +#define MFC_V12_BIT	BIT(7)
>  
>  #define MFC_V5PLUS_BITS		(MFC_V5_BIT | MFC_V6_BIT | MFC_V7_BIT | \
> -					MFC_V8_BIT | MFC_V10_BIT)
> +					MFC_V8_BIT | MFC_V10_BIT | MFC_V12_BIT)
>  #define MFC_V6PLUS_BITS		(MFC_V6_BIT | MFC_V7_BIT | MFC_V8_BIT | \
> -					MFC_V10_BIT)
> -#define MFC_V7PLUS_BITS		(MFC_V7_BIT | MFC_V8_BIT | MFC_V10_BIT)
> +					MFC_V10_BIT | MFC_V12_BIT)
> +#define MFC_V7PLUS_BITS		(MFC_V7_BIT | MFC_V8_BIT | MFC_V10_BIT | \
> +					MFC_V12_BIT)

why not MFC_V10PLUS_BITS ?

[...]

> @@ -146,7 +146,7 @@ static struct s5p_mfc_fmt formats[] = {
>  		.codec_mode	= S5P_FIMV_CODEC_HEVC_DEC,
>  		.type		= MFC_FMT_DEC,
>  		.num_planes	= 1,
> -		.versions	= MFC_V10_BIT,
> +		.versions	= MFC_V10_BIT | MFC_V12_BIT,

we could just yse MFC_V10PLUS_BITS here

[...]

> +		if (IS_MFCV12(dev)) {
> +			lcu_width = S5P_MFC_LCU_WIDTH(ctx->img_width);
> +			lcu_height = S5P_MFC_LCU_HEIGHT(ctx->img_height);
> +			if (ctx->codec_mode == S5P_FIMV_CODEC_HEVC_ENC &&
> +								ctx->is_10bit) {
> +				ctx->luma_dpb_size =
> +					width64 * height32 +
> +					ALIGN(DIV_ROUND_UP(lcu_width * 32, 4),
> +							16) * height32 + 128;
> +				if (ctx->is_422) {
> +					ctx->chroma_dpb_size =
> +						ctx->luma_dpb_size;
> +				} else {
> +					ctx->chroma_dpb_size =
> +						width64 * height32 / 2 +
> +						ALIGN(DIV_ROUND_UP(lcu_width *
> +						32, 4), 16) * height32 / 2 + 128;
> +				}

You don't need the brackets here.

> +			} else if (ctx->codec_mode == S5P_FIMV_CODEC_VP9_ENC &&
> +					ctx->is_10bit) {
> +				ctx->luma_dpb_size =
> +					ALIGN(ctx->img_width * 2, 128) *
> +					height32 + 64;
> +				ctx->chroma_dpb_size =
> +					ALIGN(ctx->img_width * 2, 128) *
> +					height32 / 2 + 64;
> +			} else {
> +				ctx->luma_dpb_size =
> +					width64 * height32 + 64;
> +				if (ctx->is_422) {
> +					ctx->chroma_dpb_size =
> +						ctx->luma_dpb_size;
> +				} else {
> +					ctx->chroma_dpb_size =
> +						width64 * height32 / 2 + 64;
> +				}

You don't need the brackets here.

[...]

Andi