linux-kernel - Re: [PATCH 7/8] media: iris: Introduce vpu ops for vpu4 with necessary hooks

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: <3355306e-4059-4af5-8865-3b5335356382@oss.qualcomm.com>
Date: Thu, 25 Sep 2025 11:18:44 +0200
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Vikash Garodia <vikash.garodia@....qualcomm.com>,
        Dikshita Agarwal <dikshita.agarwal@....qualcomm.com>,
        Abhinav Kumar <abhinav.kumar@...ux.dev>,
        Bryan O'Donoghue <bod@...nel.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
 <conor+dt@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>
Cc: linux-arm-msm@...r.kernel.org, linux-media@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Vishnu Reddy <quic_bvisredd@...cinc.com>
Subject: Re: [PATCH 7/8] media: iris: Introduce vpu ops for vpu4 with
 necessary hooks

On 9/25/25 1:14 AM, Vikash Garodia wrote:
> Add power sequence for vpu4 by reusing from previous generation wherever
> possible. Hook up vpu4 op with vpu4 specific implemtation or resue from
> earlier generation wherever feasible, like clock calculation in this
> case.
> 
> Co-developed-by: Vishnu Reddy <quic_bvisredd@...cinc.com>
> Signed-off-by: Vishnu Reddy <quic_bvisredd@...cinc.com>
> Signed-off-by: Vikash Garodia <vikash.garodia@....qualcomm.com>
> ---

[...]

> +#include <linux/iopoll.h>
> +#include <linux/reset.h>
> +#include "iris_instance.h"
> +#include "iris_vpu_common.h"
> +#include "iris_vpu_register_defines.h"
> +
> +#define WRAPPER_EFUSE_MONITOR			(WRAPPER_BASE_OFFS + 0x08)
> +#define AON_WRAPPER_MVP_NOC_RESET_SYNCRST	(AON_MVP_NOC_RESET + 0x08)
> +#define CPU_CS_APV_BRIDGE_SYNC_RESET		(CPU_BASE_OFFS + 0x174)
> +#define DISABLE_VIDEO_APV_BIT			BIT(27)
> +#define DISABLE_VIDEO_VPP1_BIT			BIT(28)
> +#define DISABLE_VIDEO_VPP0_BIT			BIT(29)
> +#define CORE_CLK_HALT				BIT(0)
> +#define APV_CLK_HALT				BIT(1)
> +#define CORE_PWR_ON				BIT(1)
> +
> +static int iris_vpu4x_genpd_set_hwmode(struct iris_core *core, bool hw_mode)
> +{
> +	u32 value = readl(core->reg_base + WRAPPER_EFUSE_MONITOR);

I think this could use some explanations.

I'll go ahead and assume that the eFuse tells us that parts of the
IP are disables (hopefully not all three at once.. we shouldn't
advertise the v4l2 device then, probably)

You read back the fuse register a lot, even though I presume it's not
supposed to change at runtime. How about we add:

bool vpp0_fused_off
bool vpp1_fused_off
bool apv_fused_off

instead?

[...]

> +	if (!(value & DISABLE_VIDEO_VPP0_BIT)) {
> +		ret = iris_enable_power_domains(core, core->pmdomain_tbl->pd_devs
> +						[IRIS_VPP0_HW_POWER_DOMAIN]);

Maybe the iris_en/disable_foo functions could get a wrapper like:

int iris_enable_power_domains_if(core, pd_devs[IRIS_VPP0_HW_POWER_DOMAIN],
				 !foo->vpp0_fused_off)

I'm not super sure about it, but that's something to consider

[...]

> +	readl_poll_timeout(core->reg_base + VCODEC_SS_IDLE_STATUSN, value,
> +			   value & 0x7103, 2000, 20000);

That's a nice magic number.. but what does it mean?

[...]

> +	writel(0x070103, core->reg_base + AON_WRAPPER_MVP_NOC_RESET_REQ);
> +	readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_RESET_ACK,
> +			   value, value == 0x070103, 200, 2000);

That's a slightly different magic number, but it's oddly similar to
the one above

Konrad