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

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

[Vishnu Reddy]

This series addresses three independent issues in the iris video
driver and its devicetree bindings/nodes for SC7280-based platforms:

- 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, found that DMA buffers shared between the CPU and the
  venus video hardware/controller are not in sync. CPU writes to an
  input buffer can remain in CPU cache 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.
  This series adds explicit dma_sync*() calls in the driver for the
  input and output buffers, and declares the dma-coherent property on
  the venus node and its dt-binding, since the venus hardware on
  SC7280 is IO-coherent hardware that performs cache snooping between
  the CPU and the video hardware/controller. Together, these changes
  keep the CPU cache and the DMA buffers in sync.

- 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.

---
Changes in v2:
- Add dma_sync*() calls in driver (Rob Herring)
- Updated commit descriptions.
- Link to v1: https://lore.kernel.org/all/20260801-iris-fixes-dma-pseq-fint-v1-0-aba0cb22f6ab@oss.qualcomm.com

To: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
To: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
To: Abhinav Kumar <abhinav.kumar@linux.dev>
To: Bryan O'Donoghue <bod@kernel.org>
To: Mauro Carvalho Chehab <mchehab@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Stanimir Varbanov <stanimir.varbanov@linaro.org>
To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konradybcio@kernel.org>
To: Hans Verkuil <hverkuil@kernel.org>
To: Stefan Schmidt <stefan.schmidt@linaro.org>
To: Hans Verkuil <hverkuil+cisco@kernel.org>
To: Mansur Alisha Shaik <mansur@codeaurora.org>
Cc: linux-media@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>

---
Vishnu Reddy (5):
      media: iris: Add dma sync calls for input and output buffers
      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_buffer.c                 | 7 +++++++
 drivers/media/platform/qcom/iris/iris_vidc.c                   | 4 ++--
 drivers/media/platform/qcom/iris/iris_vpu_common.c             | 2 +-
 5 files changed, 17 insertions(+), 3 deletions(-)
---
base-commit: e6664f2b33db9b6811eb4cec109f06cb2b4f458d
change-id: 20260818-iris-fixes-dma-pseq-fint-707f14d69753

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

[PATCH v2 1/5] media: iris: Add dma sync calls for input and output buffers

[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, found that DMA buffers shared between the CPU and the
venus video hardware/controller are not in sync. CPU writes to an input
buffer can remain in CPU cache 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 dma sync API calls to synchronize the data between the CPU cache
and the venus hardware/controller.

Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
---
 drivers/media/platform/qcom/iris/iris_buffer.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
index eb8de60c1177..c7a664426187 100644
--- a/drivers/media/platform/qcom/iris/iris_buffer.c
+++ b/drivers/media/platform/qcom/iris/iris_buffer.c
@@ -585,6 +585,13 @@ int iris_queue_buffer(struct iris_inst *inst, struct iris_buffer *buf)
 	const struct iris_hfi_session_ops *hfi_ops = inst->hfi_session_ops;
 	int ret;
 
+	if (buf->type == BUF_INPUT)
+		dma_sync_single_for_device(inst->core->dev, buf->device_addr,
+					   buf->data_size, DMA_TO_DEVICE);
+	else if (buf->type == BUF_OUTPUT)
+		dma_sync_single_for_cpu(inst->core->dev, buf->device_addr,
+					buf->data_size, DMA_FROM_DEVICE);
+
 	ret = hfi_ops->session_queue_buf(inst, buf);
 	if (ret)
 		return ret;

-- 
2.34.1

[PATCH v2 2/5] dt-bindings: media: qcom,sc7280-venus: Add dma-coherent property

[Vishnu Reddy]

The venus video hardware on SC7280 is IO-coherent hardware: even though
the driver does dma_sync*() calls for the input and output buffers, the
venus node still needs to declare dma-coherent so that the DMA mapping
layer relies on this hardware level snooping to keep the CPU cache and
the DMA buffers in sync. This avoids the unnecessary cache clean or
invalidate operations performed by the dma_sync*() calls, which are not
required once hardware level snooping is enabled via dma-coherent.

Add the dma-coherent property to the venus node to describe this
hardware capability.

Fixes: 37613aee2179 ("arm64: dts: qcom: sc7280: Add venus DT node")
Cc: stable@vger.kernel.org
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.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

[PATCH v2 3/5] arm64: dts: qcom: sc7280: Add dma-coherent property into venus node

[Vishnu Reddy]

The venus video hardware on SC7280 is IO-coherent hardware: even though
the driver does dma_sync*() calls for the input and output buffers, the
venus node still needs to declare dma-coherent so that the DMA mapping
layer relies on this hardware level snooping to keep the CPU cache and
the DMA buffers in sync. This avoids the unnecessary cache clean or
invalidate operations performed by the dma_sync*() calls, which are not
required once hardware level snooping is enabled via dma-coherent.

Add the dma-coherent property to the venus node to describe this
hardware capability.

Fixes: 37613aee2179 ("arm64: dts: qcom: sc7280: Add venus DT node")
Cc: stable@vger.kernel.org
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.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 f2da3706d5c8..b8f5705ebfc8 100644
--- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
+++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
@@ -5025,6 +5025,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 v2 4/5] 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
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.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

[PATCH v2 5/5] 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
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.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

Re: [PATCH v2 1/5] media: iris: Add dma sync calls for input and output buffers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Calling `dma_sync_single_*` on an IOVA managed by videobuf2-dma-contig violates the DMA API and can cause severe memory corruption for non-contiguous DMA-BUF imports.
- [High] The DMA synchronization logic for output buffers is fundamentally broken: it synchronizes the CPU cache before the hardware writes, uses the wrong API direction, specifies a zero size, and omits the necessary sync upon completion.
--

--- Patch [1]: [PATCH v2 1/5] media: iris: Add dma sync calls for input and output buffers ---
commit ee7c5670aeeb217b4b298594d79282fac01b4135
Author: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>

media: iris: Add dma sync calls for input and output buffers

This commit adds DMA synchronization calls in iris_queue_buffer() to fix
data corruption and hardware faults. It uses dma_sync_single_for_device for
input buffers and dma_sync_single_for_cpu for output buffers.

> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
> index eb8de60c1177f..c7a664426187d 100644
> --- a/drivers/media/platform/qcom/iris/iris_buffer.c
> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
> @@ -585,6 +585,13 @@ int iris_queue_buffer(struct iris_inst *inst, struct iris_buffer *buf)
>  	const struct iris_hfi_session_ops *hfi_ops = inst->hfi_session_ops;
>  	int ret;
>  
> +	if (buf->type == BUF_INPUT)
> +		dma_sync_single_for_device(inst->core->dev, buf->device_addr,
> +					   buf->data_size, DMA_TO_DEVICE);

[Severity: Critical]
Does this violate the DMA API by passing a videobuf2-managed IOVA to
dma_sync_single_for_device()?

For imported scatter-gather DMA-BUFs, the IOVA might be contiguous but backed
by physically discontiguous pages. Calling a single-mapped sync on the IOVA 
translates the start IOVA to a single physical address and performs cache 
maintenance on a linear physical address range. Can this corrupt arbitrary 
physical memory while leaving the actual backing pages unsynchronized?

> +	else if (buf->type == BUF_OUTPUT)
> +		dma_sync_single_for_cpu(inst->core->dev, buf->device_addr,
> +					buf->data_size, DMA_FROM_DEVICE);
> +

[Severity: High]
Is the synchronization direction correct for a queued capture buffer?

This calls dma_sync_single_for_cpu() before the hardware writes to the 
buffer, claiming CPU ownership instead of giving it to the device with 
dma_sync_single_for_device(..., DMA_FROM_DEVICE). 

Additionally, buf->data_size for a queued empty capture buffer is typically
zero. Does this result in syncing zero bytes?

Finally, is there a missing dma_sync_single_for_cpu() in the completion path
(such as iris_vb2_buffer_done()) after the hardware finishes writing and 
buf->data_size is populated? Without it, the CPU might read speculative or 
stale data.

>  	ret = hfi_ops->session_queue_buf(inst, buf);
>  	if (ret)
>  		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818-iris-fixes-dma-pseq-fint-v2-0-8dc8c4b3880e@oss.qualcomm.com?part=1

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

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch fixes the clock/power domain disable sequence in `iris_vpu_power_off_hw()` but misses identical sequence bugs in `iris_vpu35_power_off_hw()` and `iris_vpu33_power_off_controller()`, leaving them to operate on unpowered clock controllers.
--

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

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

This commit fixes the power-off ordering in iris_vpu_power_off_hw() to
ensure clocks are disabled before the associated power domain is removed,
preventing unpredictable behavior or hardware hangs.

> 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]);
>  }

[Severity: High]
This is a pre-existing issue and was not introduced by this patch, but does
this same ordering problem exist in
drivers/media/platform/qcom/iris/iris_vpu3x.c?

Looking at iris_vpu33_power_off_controller(), IRIS_CTRL_POWER_DOMAIN is
removed before disabling IRIS_AXI_CLK:

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

Similarly, in iris_vpu35_power_off_hw(), iris_vpu33_power_off_hardware()
removes the domain before IRIS_HW_FREERUN_CLK and IRIS_AXI_CLK are disabled:

	iris_vpu33_power_off_hardware(core);

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

Should these paths also be updated to prevent operating on unpowered clock
controllers and causing system hangs on device teardown?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818-iris-fixes-dma-pseq-fint-v2-0-8dc8c4b3880e@oss.qualcomm.com?part=4