[PATCH 0/4] media: iris: Fix DMA coherency, power-off ordering, and frame interval issues

[PATCH 0/4] media: iris: Fix DMA coherency, power-off ordering, and frame interval issues

[Vishnu Reddy]

This series fixes a set of issues found in the Qualcomm iris video
driver and the venus/kodiak devicetree bindings:

- The venus devicetree nodes for sc7280-based bindings and the kodiak
  platform do not declare dma-coherent, which allows the CPU and the
  video hardware/controller to see stale or inconsistent data in DMA
  buffers they share. This causes hardware faults on input and
  corruption of captured output when the client dumps it, observed
  while testing with some higher resolution clips.

- iris_vpu_power_off_hw() disables the power domain before disabling
  the associated clocks, reversing the correct power-down order and
  risking clock-controller access after its power domain is already
  removed.

- iris_enum_frameintervals() advertised frame intervals as
  V4L2_FRMIVAL_TYPE_STEPWISE with a fixed step derived from the
  maximum FPS, which excluded valid framerates that aren't exact
  divisors of the maximum and broke GStreamer caps negotiation for
  those framerates. Switching to V4L2_FRMIVAL_TYPE_CONTINUOUS fixes
  this.

Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
---
Vishnu Reddy (4):
      dt-bindings: media: qcom,sc7280-venus: Add dma-coherent property
      arm64: dts: qcom: sc7280: Add dma-coherent property into venus node
      media: iris: Fix power-off ordering to disable power domain after clocks
      media: iris: Fix frame interval enumeration for non-divisor framerates

 Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml | 5 +++++
 arch/arm64/boot/dts/qcom/kodiak.dtsi                           | 2 ++
 drivers/media/platform/qcom/iris/iris_vidc.c                   | 4 ++--
 drivers/media/platform/qcom/iris/iris_vpu_common.c             | 2 +-
 4 files changed, 10 insertions(+), 3 deletions(-)
---
base-commit: 415606a7be939835db9b0d6b711887586646346d
change-id: 20260801-iris-fixes-dma-pseq-fint-8345b2d67e3d

Best regards,
--  
Vishnu Reddy <busanna.reddy@oss.qualcomm.com>

[PATCH 1/4] dt-bindings: media: qcom,sc7280-venus: Add dma-coherent property

[Vishnu Reddy]

While testing with some higher resolution clips, the venus hardware
triggers a fault due to wrong input data being received. Corruption
was also observed in the captured output when the client dumped it
to a file.

On debugging, this was traced to the venus node not declaring
dma-coherent. As a result, DMA buffers shared between the CPU and the
venus video hardware/controller are not guaranteed to be I/O coherent:
CPU writes to an input buffer can remain in CPU caches without being
visible to the video hardware when it reads the same buffer, so the
hardware receives input data that does not match what the CPU wrote.
Likewise, on the capture path, data written by the video hardware to
the output buffer may not be visible to the CPU, so the client reads
stale or partial data, resulting in corruption.

Add the dma-coherent property to the venus node so that DMA buffers
shared between the CPU and the video hardware and controller remain
coherent.

Fixes: 37613aee2179 ("arm64: dts: qcom: sc7280: Add venus DT node")
Cc: stable@vger.kernel.org
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml b/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml
index 9725fcb761dc..cc31f3ba7e7e 100644
--- a/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml
@@ -42,6 +42,8 @@ properties:
       - const: vcodec_core
       - const: vcodec_bus
 
+  dma-coherent: true
+
   iommus:
     maxItems: 1
 
@@ -85,6 +87,7 @@ properties:
 
 required:
   - compatible
+  - dma-coherent
   - power-domain-names
   - iommus
 
@@ -119,6 +122,8 @@ examples:
                         <&mmss_noc MASTER_VIDEO_P0 0 &mc_virt SLAVE_EBI1 0>;
         interconnect-names = "cpu-cfg", "video-mem";
 
+        dma-coherent;
+
         iommus = <&apps_smmu 0x2180 0x20>;
 
         memory-region = <&video_mem>;

-- 
2.34.1

Re: [PATCH 1/4] dt-bindings: media: qcom,sc7280-venus: Add dma-coherent property

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch updates the YAML schema to require the 'dma-coherent' property but fails to update the device tree source, breaking DT validation and leaving the runtime bug unfixed.
--

--- Patch [1]: [PATCH 1/4] dt-bindings: media: qcom,sc7280-venus: Add dma-coherent property ---
commit 73ea504069776873329a245bdcc0241e9976a3c3
Author: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>

dt-bindings: media: qcom,sc7280-venus: Add dma-coherent property

This commit adds the dma-coherent property to the venus node binding schema
to address I/O coherency issues between the CPU and the video hardware.

> diff --git a/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml b/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml
> index 9725fcb761dc0..cc31f3ba7e7e0 100644
> --- a/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,sc7280-venus.yaml

[ ... ]

> @@ -85,6 +87,7 @@ properties:
>  
>  required:
>    - compatible
> +  - dma-coherent

[Severity: Medium]
Will adding this to the required list break make dtbs_check for existing
device trees? 

Since the corresponding dts updates don't appear to be included in this patch,
the schema validation might fail because the dma-coherent property is missing
from the actual device tree source. 

Additionally, without updating the dts files used to build the DTB, will the
runtime data corruption issue described in the commit message persist because
the hardware configuration remains unchanged?

>    - power-domain-names
>    - iommus

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801-iris-fixes-dma-pseq-fint-v1-0-aba0cb22f6ab@oss.qualcomm.com?part=1

[PATCH 2/4] arm64: dts: qcom: sc7280: Add dma-coherent property into venus node

[Vishnu Reddy]

While testing with some higher resolution clips, the venus hardware
triggers a fault due to wrong input data being received. Corruption
was also observed in the captured output when the client dumped it
to a file.

On debugging, this was traced to the venus node not declaring
dma-coherent. As a result, DMA buffers shared between the CPU and the
venus video hardware/controller are not guaranteed to be I/O coherent:
CPU writes to an input buffer can remain in CPU caches without being
visible to the video hardware when it reads the same buffer, so the
hardware receives input data that does not match what the CPU wrote.
Likewise, on the capture path, data written by the video hardware to
the output buffer may not be visible to the CPU, so the client reads
stale or partial data, resulting in corruption.

Add the dma-coherent property to the venus node so that DMA buffers
shared between the CPU and the video hardware and controller remain
coherent.

Fixes: 37613aee2179 ("arm64: dts: qcom: sc7280: Add venus DT node")
Cc: stable@vger.kernel.org
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/kodiak.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/kodiak.dtsi b/arch/arm64/boot/dts/qcom/kodiak.dtsi
index f74fa66f1a67..98184760b4bf 100644
--- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
+++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
@@ -5024,6 +5024,8 @@ venus: video-codec@aa00000 {
 					<&mmss_noc MASTER_VIDEO_P0 0 &mc_virt SLAVE_EBI1 0>;
 			interconnect-names = "cpu-cfg", "video-mem";
 
+			dma-coherent;
+
 			iommus = <&apps_smmu 0x2180 0x20>;
 			memory-region = <&video_mem>;
 

-- 
2.34.1

[PATCH 3/4] media: iris: Fix power-off ordering to disable power domain after clocks

[Vishnu Reddy]

In iris_vpu_power_off_hw(), iris_disable_power_domains() was called
before the associated clocks (IRIS_BSE_HW_CLK, IRIS_HW_AHB_CLK,
IRIS_HW_CLK) were disabled and unprepared. This reverses the correct
power-down sequence: with the power domain already removed, the
subsequent clk_disable_unprepare() calls end up operating on
clock-controller hardware that is no longer powered, which can hang
or behave unpredictably.

Reorder the calls so iris_disable_power_domains() runs after all
three clocks are disabled, ensuring clocks are always turned off
while their power domain is still active, and mirroring the reverse
of the power-on sequence.

Fixes: bb8a95aa038e ("media: iris: implement power management")
Cc: stable@vger.kernel.org
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
---
 drivers/media/platform/qcom/iris/iris_vpu_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c
index e4847c107709..3cd493b06d84 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c
@@ -223,10 +223,10 @@ int iris_vpu_power_off_controller(struct iris_core *core)
 void iris_vpu_power_off_hw(struct iris_core *core)
 {
 	dev_pm_genpd_set_hwmode(core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN], false);
-	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
 	iris_disable_unprepare_clock(core, IRIS_BSE_HW_CLK);
 	iris_disable_unprepare_clock(core, IRIS_HW_AHB_CLK);
 	iris_disable_unprepare_clock(core, IRIS_HW_CLK);
+	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
 }
 
 void iris_vpu_power_off(struct iris_core *core)

-- 
2.34.1

Re: [PATCH 3/4] media: iris: Fix power-off ordering to disable power domain after clocks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Use-After-Free of `iris_core` when user-space holds V4L2 device open during driver unbind.
- [High] Incomplete fix for power-off hardware hangs in nested VPU3x functions.
- [High] Use-After-Free due to missing cancellation of `sys_error_handler` delayed work.
- [High] AB-BA deadlock between `disable_irq()` and `core->lock`.
- [High] Hardware hang due to premature OPP rate drop nullifying the patch's clock ordering fix.
- [High] Hardware resets are triggered on unclocked and unpowered domains.
--

--- Patch [3]: [PATCH 3/4] media: iris: Fix power-off ordering to disable power domain after clocks ---
commit 21b9f8f2d0a12300785d96896a5ff556ccf1532a
Author: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>

media: iris: Fix power-off ordering to disable power domain after clocks

This patch reorders the power-off sequence in iris_vpu_power_off_hw() to
ensure that clocks are disabled before removing the power domain. This avoids
hardware hangs that occur when disabling clock controllers without power.

Cc: stable@vger.kernel.org

> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c
> index e4847c1077097..3cd493b06d84c 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c
> @@ -223,10 +223,10 @@ int iris_vpu_power_off_controller(struct iris_core *core)
>  void iris_vpu_power_off_hw(struct iris_core *core)
>  {
>  	dev_pm_genpd_set_hwmode(core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN], false);
> -	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
>  	iris_disable_unprepare_clock(core, IRIS_BSE_HW_CLK);
>  	iris_disable_unprepare_clock(core, IRIS_HW_AHB_CLK);
>  	iris_disable_unprepare_clock(core, IRIS_HW_CLK);
> +	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
>  }
>  
>  void iris_vpu_power_off(struct iris_core *core)
>  {
>  	iris_opp_set_rate(core->dev, 0);

[Severity: High]
This isn't a bug introduced by this patch, but does calling
iris_opp_set_rate() drop the OPP performance state and voltage before
power_off_hw() disables the active hardware clocks?

If high-frequency clocks run with insufficient voltage before being disabled,
could this negate the structural fix and cause the same hardware hang during
suspension or teardown?

>  	core->iris_platform_data->vpu_ops->power_off_hw(core);
>  	core->iris_platform_data->vpu_ops->power_off_controller(core);
>  	iris_unset_icc_bw(core);
>  
>  	if (!iris_vpu_watchdog(core, core->intr_status))
>  		disable_irq(core->irq);

[Severity: High]
This is a pre-existing issue, but does disable_irq() synchronize and wait for
the threaded IRQ handler to complete while holding core->lock?

If iris_core_deinit() holds core->lock and calls iris_vpu_power_off():

iris_core_deinit()
    mutex_lock(&core->lock);
    iris_vpu_power_off(core);
        disable_irq(core->irq);

And concurrently, a hardware interrupt fires and wakes the threaded handler:

iris_hfi_isr_handler()
    mutex_lock(&core->lock);

Will this cause a deadlock between the interrupt handler and
iris_core_deinit()?

>  }

While reviewing this, I noticed similar pre-existing issues in other areas of
the driver:

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a similar pattern in
iris_vpu3x.c where the power domain is disabled before clocks:

iris_vpu33_power_off_controller() {
...
disable_power:
	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]);
	iris_disable_unprepare_clock(core, IRIS_AXI_CLK);
...
}

Does this leave the same reverse-ordering problem where IRIS_CTRL_POWER_DOMAIN
is disabled before IRIS_AXI_CLK is unprepared?

Similarly, in iris_vpu35_power_off_hw():

iris_vpu35_power_off_hw() {
...
	iris_vpu33_power_off_hardware(core);

	iris_disable_unprepare_clock(core, IRIS_HW_FREERUN_CLK);
	iris_disable_unprepare_clock(core, IRIS_AXI_CLK);
...
}

Does disabling the clocks after iris_vpu33_power_off_hardware() has already
removed the hardware power domain result in the same hardware hang?

[Severity: High]
This is a pre-existing issue, but in iris_vpu_common.c, does triggering
reset_control_bulk_reset() after the clocks and power domain have already been
disabled violate hardware constraints and risk a bus lockup?

iris_vpu35_vpu4x_power_off_controller() {
...
disable_power:
	iris_disable_unprepare_clock(core, IRIS_CTRL_CLK);
...
	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]);

	reset_control_bulk_reset(clk_rst_tbl_size, core->resets);
...
}

[Severity: Critical]
This isn't a bug introduced by this patch, but in iris_probe.c, if user-space
holds a file descriptor open on the video node during driver unbind, will
devres free the core structure while the video_device stays alive?

iris_remove() {
...
	iris_core_deinit(core);
	video_unregister_device(core->vdev_dec);
	video_unregister_device(core->vdev_enc);
	v4l2_device_unregister(&core->v4l2_dev);
	mutex_destroy(&core->lock);
...
}

If the file descriptor is subsequently closed, iris_remove_session() is
invoked:

iris_remove_session() {
...
	struct iris_core *core = inst->core;
	struct iris_inst *iter, *temp;

	mutex_lock(&core->lock);
...
}

Will this result in a use-after-free when attempting to acquire core->lock?

[Severity: High]
This is a pre-existing issue, but in iris_remove(), is there a missing call
to cancel_delayed_work_sync(&core->sys_error_handler)?

If devres frees core, and the delayed work subsequently runs
iris_sys_error_handler(), could this also trigger a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801-iris-fixes-dma-pseq-fint-v1-0-aba0cb22f6ab@oss.qualcomm.com?part=3

[PATCH 4/4] media: iris: Fix frame interval enumeration for non-divisor framerates

[Vishnu Reddy]

iris_enum_frameintervals() advertised frame intervals using
V4L2_FRMIVAL_TYPE_STEPWISE with step=1/MAXIMUM_FPS where MAXIMUM_FPS
is 480. This caused client to enumerate only framerates of the form
MAXIMUM_FPS/n (where n is a positive integer), restricting support to
exact divisors of MAXIMUM_FPS (e.g., 480, 240, 160, 120, 96, 80, 60,
30, 24, 1).

Framerates that are not exact divisors of MAXIMUM_FPS, such as 29 fps,
25 fps, were excluded from the enumerated list. There is no hardware
restriction to framerates that are exact divisors of MAXIMUM_FPS. This
caused GStreamer caps negotiation to fail with an "internal data
stream error" when encoding content at such framerates.

Fix this by using V4L2_FRMIVAL_TYPE_CONTINUOUS. With CONTINUOUS type,
GStreamer creates a continuous framerate range [1, max_fps], allowing
any integer framerate within the range to pass caps negotiation. The
step field is set to 1/1 as required by the V4L2 specification for
continuous frame intervals.

Fixes: a6882431a138 ("media: iris: Add support for ENUM_FRAMESIZES/FRAMEINTERVALS for encoder")
Cc: stable@vger.kernel.org
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
---
 drivers/media/platform/qcom/iris/iris_vidc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c
index fcbc60016bee..8f20cb281a93 100644
--- a/drivers/media/platform/qcom/iris/iris_vidc.c
+++ b/drivers/media/platform/qcom/iris/iris_vidc.c
@@ -438,14 +438,14 @@ static int iris_enum_frameintervals(struct file *filp, void *fh,
 	mbpf = NUM_MBS_PER_FRAME(fival->height, fival->width);
 	fps = DIV_ROUND_UP(core->iris_platform_data->max_core_mbps, mbpf);
 
-	fival->type = V4L2_FRMIVAL_TYPE_STEPWISE;
+	fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
 	fival->stepwise.min.numerator = 1;
 	fival->stepwise.min.denominator =
 			min_t(u32, fps, MAXIMUM_FPS);
 	fival->stepwise.max.numerator = 1;
 	fival->stepwise.max.denominator = 1;
 	fival->stepwise.step.numerator = 1;
-	fival->stepwise.step.denominator = MAXIMUM_FPS;
+	fival->stepwise.step.denominator = 1;
 
 	return 0;
 }

-- 
2.34.1