Re: [PATCH 4/7] media: iris: Switch to hardware mode after firmware boot

[Bryan O'Donoghue <bod@kernel.org>]

On 26/01/2026 12:25, Vikash Garodia wrote:
> Currently the driver switches the vcodec GDSC to hardware (HW) mode
> before firmware load and boot sequence. GDSC can be powered off,
> keeping in hw mode, thereby the vcodec registers programmed in TrustZone
> (TZ) carry default (reset) values.
> Move the transition to HW mode after firmware load and boot sequence.
> 
> The bug was exposed with driver configuring different stream ids to
> different devices via iommu-map. With registers carrying reset values,
> VPU would not generate desired stream-id, thereby leading to SMMU fault.
> 
> Fixes: dde659d37036 ("media: iris: Introduce vpu ops for vpu4 with necessary hooks")
> Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
> Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
> Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
> ---
>   drivers/media/platform/qcom/iris/iris_core.c       |  4 ++++
>   drivers/media/platform/qcom/iris/iris_hfi_common.c |  4 ++++
>   drivers/media/platform/qcom/iris/iris_vpu2.c       |  1 +
>   drivers/media/platform/qcom/iris/iris_vpu3x.c      |  9 +++-----
>   drivers/media/platform/qcom/iris/iris_vpu4x.c      | 24 ++++++++++++----------
>   drivers/media/platform/qcom/iris/iris_vpu_common.c | 16 +++++++++------
>   drivers/media/platform/qcom/iris/iris_vpu_common.h |  3 +++
>   7 files changed, 38 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_core.c b/drivers/media/platform/qcom/iris/iris_core.c
> index 8406c48d635b6eba0879396ce9f9ae2292743f09..dbaac01eb15a0e622e85635fddd29c1f7fc18662 100644
> --- a/drivers/media/platform/qcom/iris/iris_core.c
> +++ b/drivers/media/platform/qcom/iris/iris_core.c
> @@ -75,6 +75,10 @@ int iris_core_init(struct iris_core *core)
>   	if (ret)
>   		goto error_unload_fw;
> 
> +	ret = iris_vpu_switch_to_hwmode(core);
> +	if (ret)
> +		goto error_unload_fw;
> +
>   	ret = iris_hfi_core_init(core);
>   	if (ret)
>   		goto error_unload_fw;
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_common.c b/drivers/media/platform/qcom/iris/iris_hfi_common.c
> index 92112eb16c11048e28230a2926dfb46e3163aada..621c66593d88d47ef3438c98a07cb29421c4e375 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_common.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_common.c
> @@ -159,6 +159,10 @@ int iris_hfi_pm_resume(struct iris_core *core)
>   	if (ret)
>   		goto err_suspend_hw;
> 
> +	ret = iris_vpu_switch_to_hwmode(core);
> +	if (ret)
> +		goto err_suspend_hw;
> +
>   	ret = ops->sys_interframe_powercollapse(core);
>   	if (ret)
>   		goto err_suspend_hw;
> diff --git a/drivers/media/platform/qcom/iris/iris_vpu2.c b/drivers/media/platform/qcom/iris/iris_vpu2.c
> index 9c103a2e4e4eafee101a8a9b168fdc8ca76e277d..01ef40f3895743b3784464e2d5ba2de1aeca5a4a 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu2.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu2.c
> @@ -44,4 +44,5 @@ const struct vpu_ops iris_vpu2_ops = {
>   	.power_off_controller = iris_vpu_power_off_controller,
>   	.power_on_controller = iris_vpu_power_on_controller,
>   	.calc_freq = iris_vpu2_calc_freq,
> +	.set_hwmode = iris_vpu_set_hwmode,
>   };
> diff --git a/drivers/media/platform/qcom/iris/iris_vpu3x.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
> index fe4423b951b1e9e31d06dffc69d18071cc985731..3dad47be78b58f6cd5ed6f333b3376571a04dbf0 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu3x.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu3x.c
> @@ -234,14 +234,8 @@ static int iris_vpu35_power_on_hw(struct iris_core *core)
>   	if (ret)
>   		goto err_disable_hw_free_clk;
> 
> -	ret = dev_pm_genpd_set_hwmode(core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN], true);
> -	if (ret)
> -		goto err_disable_hw_clk;
> -
>   	return 0;
> 
> -err_disable_hw_clk:
> -	iris_disable_unprepare_clock(core, IRIS_HW_CLK);
>   err_disable_hw_free_clk:
>   	iris_disable_unprepare_clock(core, IRIS_HW_FREERUN_CLK);
>   err_disable_axi_clk:
> @@ -266,6 +260,7 @@ const struct vpu_ops iris_vpu3_ops = {
>   	.power_off_controller = iris_vpu_power_off_controller,
>   	.power_on_controller = iris_vpu_power_on_controller,
>   	.calc_freq = iris_vpu3x_vpu4x_calculate_frequency,
> +	.set_hwmode = iris_vpu_set_hwmode,
>   };
> 
>   const struct vpu_ops iris_vpu33_ops = {
> @@ -274,6 +269,7 @@ const struct vpu_ops iris_vpu33_ops = {
>   	.power_off_controller = iris_vpu33_power_off_controller,
>   	.power_on_controller = iris_vpu_power_on_controller,
>   	.calc_freq = iris_vpu3x_vpu4x_calculate_frequency,
> +	.set_hwmode = iris_vpu_set_hwmode,
>   };
> 
>   const struct vpu_ops iris_vpu35_ops = {
> @@ -283,4 +279,5 @@ const struct vpu_ops iris_vpu35_ops = {
>   	.power_on_controller = iris_vpu35_vpu4x_power_on_controller,
>   	.program_bootup_registers = iris_vpu35_vpu4x_program_bootup_registers,
>   	.calc_freq = iris_vpu3x_vpu4x_calculate_frequency,
> +	.set_hwmode = iris_vpu_set_hwmode,
>   };
> diff --git a/drivers/media/platform/qcom/iris/iris_vpu4x.c b/drivers/media/platform/qcom/iris/iris_vpu4x.c
> index a8db02ce5c5ec583c4027166b34ce51d3d683b4e..02e100a4045fced33d7a3545b632cc5f0955233f 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu4x.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu4x.c
> @@ -252,21 +252,10 @@ static int iris_vpu4x_power_on_hardware(struct iris_core *core)
>   		ret = iris_vpu4x_power_on_apv(core);
>   		if (ret)
>   			goto disable_hw_clocks;
> -
> -		iris_vpu4x_ahb_sync_reset_apv(core);
>   	}
> 
> -	iris_vpu4x_ahb_sync_reset_hardware(core);
> -
> -	ret = iris_vpu4x_genpd_set_hwmode(core, true, efuse_value);
> -	if (ret)
> -		goto disable_apv_power_domain;
> -
>   	return 0;
> 
> -disable_apv_power_domain:
> -	if (!(efuse_value & DISABLE_VIDEO_APV_BIT))
> -		iris_vpu4x_power_off_apv(core);
>   disable_hw_clocks:
>   	iris_vpu4x_disable_hardware_clocks(core, efuse_value);
>   disable_vpp1_power_domain:
> @@ -359,6 +348,18 @@ static void iris_vpu4x_power_off_hardware(struct iris_core *core)
>   	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
>   }
> 
> +static int iris_vpu4x_set_hwmode(struct iris_core *core)
> +{
> +	u32 efuse_value = readl(core->reg_base + WRAPPER_EFUSE_MONITOR);
> +
> +	if (!(efuse_value & DISABLE_VIDEO_APV_BIT))
> +		iris_vpu4x_ahb_sync_reset_apv(core);
> +
> +	iris_vpu4x_ahb_sync_reset_hardware(core);
> +
> +	return iris_vpu4x_genpd_set_hwmode(core, true, efuse_value);
I'd like to see something in the commit log about the efuse value. What 
is it, why does it appear etc.

Because just to be difficult you are not doing a 1:1 switch to hw_mode 
here, you're also introducing contingent logic.

---
bod