Re: [PATCH v4 09/16] media: iris: Add framework support for AR50_LITE video core

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The VPU clock frequency calculation ignores the actual stream frame rate and uses a hardcoded 30 FPS.
- [High] Calculation of VSP frequency causes severe 32-bit integer overflows on 32-bit architectures, leading to under-provisioned clocks.
--

commit 117c5206f4fac821d317f597cfe56c462e1556c1
Author: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>

media: iris: Add framework support for AR50_LITE video core

Add power sequence for ar5lt core.
Add register handling for ar50lt by hooking up vpu op with ar50lt
specific implemtation or resue from earlier generation wherever
feasible.

> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_ar50lt.c b/drivers/media/platform/qcom/iris/iris_vpu_ar50lt.c
> new file mode 100644
> index 0000000000000..1af20b067c032
> --- /dev/null
> +++ b/drivers/media/platform/qcom/iris/iris_vpu_ar50lt.c
[ ... ]
> +static u64 iris_vpu_ar50lt_calc_freq(struct iris_inst *inst, size_t data_size)
> +{
> +	struct platform_inst_caps *caps = inst->core->iris_platform_data->inst_caps;
> +	struct v4l2_format *inp_f = inst->fmt_src;
> +	u32 mbs_per_second, mbpf, height, width;
> +	unsigned long vpp_freq, vsp_freq;
> +	u32 fps = DEFAULT_FPS;

[Severity: High]
Does this hardcode the frame rate to 30 FPS instead of using the actual
stream frame rate from the session (e.g., inst->frame_rate)? For
high-framerate streams, this might under-provision the VPU clocks and lead
to dropped frames or hardware stalls.

> +
> +	width = max(inp_f->fmt.pix_mp.width, inst->crop.width);
> +	height = max(inp_f->fmt.pix_mp.height, inst->crop.height);
> +
> +	mbpf = NUM_MBS_PER_FRAME(height, width);
> +	mbs_per_second = mbpf * fps;
> +
> +	vpp_freq = mbs_per_second * caps->mb_cycles_vpp;
> +
> +	/* 21 / 20 is overhead factor */
> +	vpp_freq += vpp_freq / 20;
> +	vsp_freq = mbs_per_second * caps->mb_cycles_vsp;
> +
> +	/* 10 / 7 is overhead factor */
> +	vsp_freq += ((fps * data_size * 8) * 10) / 7;

[Severity: High]
Can this calculation overflow on 32-bit architectures? Since size_t is 32-bit
on these platforms, the intermediate product could exceed the 32-bit unsigned
maximum before the division by 7, resulting in a wrap-around and an incorrectly
low requested clock frequency.

> +
> +	return max(vpp_freq, vsp_freq);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-iris-ar50lt-v4-0-0abfb74d5b3c@oss.qualcomm.com?part=9