[PATCH 0/2] media: venus: vdec: fix drain and restart on SDM845

[PATCH 0/2] media: venus: vdec: fix drain and restart on SDM845

[David Heidelberg via B4 Relay]

Please consider this more as a bug-report than the proper fix.

On SDM845 (Venus v4) every V4L2_DEC_CMD_STOP kills the decoder
session: the firmware rejects the dummy 0xdeadb000 address of
the EOS buffer with HFI_ERR_SESSION_FATAL, both queues go into error
and userspace sees poll() fail at the end of every stream.

Patch 1 hands the firmware a real, mapped buffer for EOS, after which
the drain completes as designed: every frame is returned and the last
capture buffer is flagged.

Patch 2 fixes restarting after a seek when CAPTURE is streamed on before
OUTPUT, which left the decoder without any capture or DPB buffers.

Tested on Pixel 3 (blueline) with H.264: output is bit-exact against
ffmpeg's software decoder including the final frames, and drain,
DEC_CMD_START, seek with either queue order, and mid-stream seek all
return every frame followed by a LAST buffer.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
David Heidelberg (2):
      media: venus: vdec: signal EOS with a real buffer
      media: venus: vdec: Start capture after a seek that streams CAPTURE first

 drivers/media/platform/qcom/venus/core.h |  4 +++
 drivers/media/platform/qcom/venus/vdec.c | 51 +++++++++++++++++++++++++++++---
 2 files changed, 51 insertions(+), 4 deletions(-)
---
base-commit: 7079a12d7506b07fb53b54a664bfad5fa9b16d70
change-id: 20260928-venus-vdec-eos-9a451a4c0561

Best regards,
--  
David Heidelberg <david@ixit.cz>

[PATCH 1/2] media: venus: vdec: signal EOS with a real buffer

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

The decoder tells the firmware about the end of the stream with an empty
buffer command whose address is a dummy (0xdeadb000), because the host
has no input buffer to name. The SDM845 (Venus v4) firmware validates
that address and rejects it:

  IsBufferProtected(1087): VenusCoreCtrl: CP_UNKNOWN, buffer straddles
  CP & non-CP regions
  vDec_EmptyBuffer(1932): Invalidate buffer in ETB

and raises HFI_ERR_SESSION_FATAL, reported by the driver as

  qcom-venus aa00000.video-codec: session error: event id:1001 (deadb000)

Both queues are then put into an error state, so the drain never
completes and every V4L2_DEC_CMD_STOP ends the stream with an error:
GStreamer's poll() fails at end of file, ffmpeg aborts.

Allocate a small buffer the first time a decoder instance is drained and
use its device address for the EOS command, so the firmware gets an
address that is mapped and outside the protected regions. The firmware
then drains normally: all frames are returned and the last capture
buffer carries V4L2_BUF_FLAG_LAST. The NULL address used by old IRIS2
firmware is left alone.

Assisted-by: LLM
Fixes: beac82904a87 ("media: venus: make decoder compliant with stateful codec API")
Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/media/platform/qcom/venus/core.h |  4 ++++
 drivers/media/platform/qcom/venus/vdec.c | 31 ++++++++++++++++++++++++++++---
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 46705a6667762..136bbeb3da156 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -452,16 +452,18 @@ enum venus_inst_modes {
  * @hprop:	a union used as a holder by get property
  * @core_acquired:	the Core has been acquired
  * @bit_depth:		current bitstream bit-depth
  * @pic_struct:		bitstream progressive vs interlaced
  * @next_buf_last: a flag to mark next queued capture buffer as last
  * @drain_active:	Drain sequence is in progress
  * @flags:	bitmask flags describing current instance mode
  * @dpb_ids:	DPB buffer ID's
+ * @eos_va:	kernel cookie of the buffer used to signal decoder EOS
+ * @eos_da:	device address of the buffer used to signal decoder EOS
  */
 struct venus_inst {
 	struct list_head list;
 	struct mutex lock;
 	struct venus_core *core;
 	struct clock_data clk_data;
 	struct list_head dpbbufs;
 	struct list_head internalbufs;
@@ -523,16 +525,18 @@ struct venus_inst {
 	union hfi_get_property hprop;
 	unsigned int core_acquired: 1;
 	unsigned int bit_depth;
 	unsigned int pic_struct;
 	bool next_buf_last;
 	bool drain_active;
 	enum venus_inst_modes flags;
 	struct ida dpb_ids;
+	void *eos_va;
+	dma_addr_t eos_da;
 };
 
 #define IS_V1(core)	((core)->res->hfi_version == HFI_VERSION_1XX)
 #define IS_V3(core)	((core)->res->hfi_version == HFI_VERSION_3XX)
 #define IS_V4(core)	((core)->res->hfi_version == HFI_VERSION_4XX)
 #if (!IS_ENABLED(CONFIG_VIDEO_QCOM_IRIS))
 #define IS_V6(core)	((core)->res->hfi_version == HFI_VERSION_6XX)
 #else
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 6a43ea191da15..74591c7a820f5 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -1,31 +1,35 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
  * Copyright (C) 2017 Linaro Ltd.
  */
 #include <linux/clk.h>
+#include <linux/dma-mapping.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/sizes.h>
 #include <linux/slab.h>
 #include <media/v4l2-ioctl.h>
 #include <media/v4l2-event.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-mem2mem.h>
 #include <media/videobuf2-dma-contig.h>
 
 #include "hfi_venus_io.h"
 #include "hfi_parser.h"
 #include "core.h"
 #include "helpers.h"
 #include "vdec.h"
 #include "pm_helpers.h"
 
+#define VDEC_EOS_BUF_SIZE	SZ_4K
+
 /*
  * Three resons to keep MPLANE formats (despite that the number of planes
  * currently is one):
  * - the MPLANE formats allow only one plane to be used
  * - the downstream driver use MPLANE formats too
  * - future firmware versions could add support for >1 planes
  */
 static const struct venus_format vdec_formats[] = {
@@ -569,20 +573,38 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
 		fdata.buffer_type = HFI_BUFFER_INPUT;
 		fdata.flags |= HFI_BUFFERFLAG_EOS;
 
 		/* Send NULL EOS addr for only IRIS2 (SM8250),for firmware <= 1.0.87.
 		 * SC7280 also reports "1.0.<hash>" parsed as 1.0.0; restricting to IRIS2
 		 * avoids misapplying this quirk and breaking VP9 decode on SC7280.
 		 */
 
-		if (IS_IRIS2(inst->core) && is_fw_rev_or_older(inst->core, 1, 0, 87))
+		if (IS_IRIS2(inst->core) && is_fw_rev_or_older(inst->core, 1, 0, 87)) {
 			fdata.device_addr = 0;
-		else
-			fdata.device_addr = 0xdeadb000;
+		} else {
+			/*
+			 * Firmware may validate the address of the EOS buffer,
+			 * so hand it a real one rather than a made-up address.
+			 */
+			if (!inst->eos_va) {
+				inst->eos_va = dma_alloc_attrs(inst->core->dev,
+							       VDEC_EOS_BUF_SIZE,
+							       &inst->eos_da,
+							       GFP_KERNEL,
+							       DMA_ATTR_NO_KERNEL_MAPPING);
+				if (!inst->eos_va) {
+					ret = -ENOMEM;
+					goto unlock;
+				}
+			}
+
+			fdata.device_addr = inst->eos_da;
+			fdata.alloc_len = VDEC_EOS_BUF_SIZE;
+		}
 
 		ret = hfi_session_process_buf(inst, &fdata);
 
 		if (!ret && inst->codec_state == VENUS_DEC_STATE_DECODING) {
 			inst->codec_state = VENUS_DEC_STATE_DRAIN;
 			inst->drain_active = true;
 		}
 	} else if (cmd->cmd == V4L2_DEC_CMD_START &&
@@ -1758,16 +1780,19 @@ static int vdec_open(struct file *file)
 static int vdec_close(struct file *file)
 {
 	struct venus_inst *inst = to_inst(file);
 
 	vdec_pm_get(inst);
 	cancel_work_sync(&inst->delayed_process_work);
 	venus_close_common(inst, file);
 	ida_destroy(&inst->dpb_ids);
+	if (inst->eos_va)
+		dma_free_attrs(inst->core->dev, VDEC_EOS_BUF_SIZE, inst->eos_va,
+			       inst->eos_da, DMA_ATTR_NO_KERNEL_MAPPING);
 	vdec_pm_put(inst, false);
 
 	kfree(inst);
 	return 0;
 }
 
 static const struct v4l2_file_operations vdec_fops = {
 	.owner = THIS_MODULE,

-- 
2.55.0

Re: [PATCH 1/2] media: venus: vdec: signal EOS with a real buffer

[bod]

On 2026-09-28 23:40 +0200, David Heidelberg wrote:
> The decoder tells the firmware about the end of the stream with an empty
> buffer command whose address is a dummy (0xdeadb000), because the host
> has no input buffer to name. The SDM845 (Venus v4) firmware validates
> that address and rejects it:
> 
>   IsBufferProtected(1087): VenusCoreCtrl: CP_UNKNOWN, buffer straddles
>   CP & non-CP regions
>   vDec_EmptyBuffer(1932): Invalidate buffer in ETB
> 
> and raises HFI_ERR_SESSION_FATAL, reported by the driver as
> 
>   qcom-venus aa00000.video-codec: session error: event id:1001 (deadb000)
> 
> Both queues are then put into an error state, so the drain never
> completes and every V4L2_DEC_CMD_STOP ends the stream with an error:
> GStreamer's poll() fails at end of file, ffmpeg aborts.
> 
> Allocate a small buffer the first time a decoder instance is drained and
> use its device address for the EOS command, so the firmware gets an
> address that is mapped and outside the protected regions. The firmware
> then drains normally: all frames are returned and the last capture
> buffer carries V4L2_BUF_FLAG_LAST. The NULL address used by old IRIS2
> firmware is left alone.
> 
> Assisted-by: LLM
> Fixes: beac82904a87 ("media: venus: make decoder compliant with stateful codec API")
> Signed-off-by: David Heidelberg <david@ixit.cz>

The logic stacks up as I read it but, I want to see this a) work on sdm845
and b) not break on sm8250 before applying.


Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
bod

Re: [PATCH 1/2] media: venus: vdec: signal EOS with a real buffer

[David Heidelberg]

On 09/10/2026 11:15, bod@kernel.org wrote:
> On 2026-09-28 23:40 +0200, David Heidelberg wrote:
>> The decoder tells the firmware about the end of the stream with an empty
>> buffer command whose address is a dummy (0xdeadb000), because the host
>> has no input buffer to name. The SDM845 (Venus v4) firmware validates
>> that address and rejects it:
>>
>>    IsBufferProtected(1087): VenusCoreCtrl: CP_UNKNOWN, buffer straddles
>>    CP & non-CP regions
>>    vDec_EmptyBuffer(1932): Invalidate buffer in ETB
>>
>> and raises HFI_ERR_SESSION_FATAL, reported by the driver as
>>
>>    qcom-venus aa00000.video-codec: session error: event id:1001 (deadb000)
>>
>> Both queues are then put into an error state, so the drain never
>> completes and every V4L2_DEC_CMD_STOP ends the stream with an error:
>> GStreamer's poll() fails at end of file, ffmpeg aborts.
>>
>> Allocate a small buffer the first time a decoder instance is drained and
>> use its device address for the EOS command, so the firmware gets an
>> address that is mapped and outside the protected regions. The firmware
>> then drains normally: all frames are returned and the last capture
>> buffer carries V4L2_BUF_FLAG_LAST. The NULL address used by old IRIS2
>> firmware is left alone.
>>
>> Assisted-by: LLM
>> Fixes: beac82904a87 ("media: venus: make decoder compliant with stateful codec API")
>> Signed-off-by: David Heidelberg <david@ixit.cz>
> 
> The logic stacks up as I read it but, I want to see this a) work on sdm845
> and b) not break on sm8250 before applying.

On sdm845 it works great (tested multiple sdm845 phones with `livi --demo-h264` 
and loop enabled. I don't have sm8250, thus that's up to someone with the device :)

David

> 
> 
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> bod
> 

Re: [PATCH 1/2] media: venus: vdec: signal EOS with a real buffer

[Bryan O'Donoghue]

On 16/10/2026 14:34, David Heidelberg wrote:
> On 09/10/2026 11:15,bod@kernel.org wrote:
>> On 2026-09-28 23:40 +0200, David Heidelberg wrote:
>>> The decoder tells the firmware about the end of the stream with an empty
>>> buffer command whose address is a dummy (0xdeadb000), because the host
>>> has no input buffer to name. The SDM845 (Venus v4) firmware validates
>>> that address and rejects it:
>>>
>>>     IsBufferProtected(1087): VenusCoreCtrl: CP_UNKNOWN, buffer straddles
>>>     CP & non-CP regions
>>>     vDec_EmptyBuffer(1932): Invalidate buffer in ETB
>>>
>>> and raises HFI_ERR_SESSION_FATAL, reported by the driver as
>>>
>>>     qcom-venus aa00000.video-codec: session error: event id:1001 (deadb000)
>>>
>>> Both queues are then put into an error state, so the drain never
>>> completes and every V4L2_DEC_CMD_STOP ends the stream with an error:
>>> GStreamer's poll() fails at end of file, ffmpeg aborts.
>>>
>>> Allocate a small buffer the first time a decoder instance is drained and
>>> use its device address for the EOS command, so the firmware gets an
>>> address that is mapped and outside the protected regions. The firmware
>>> then drains normally: all frames are returned and the last capture
>>> buffer carries V4L2_BUF_FLAG_LAST. The NULL address used by old IRIS2
>>> firmware is left alone.
>>>
>>> Assisted-by: LLM
>>> Fixes: beac82904a87 ("media: venus: make decoder compliant with stateful codec API")
>>> Signed-off-by: David Heidelberg<david@ixit.cz>
>> The logic stacks up as I read it but, I want to see this a) work on sdm845
>> and b) not break on sm8250 before applying.
> On sdm845 it works great (tested multiple sdm845 phones with `livi --demo-h264`
> and loop enabled. I don't have sm8250, thus that's up to someone with the device 🙂
> 
> David

heh I do have it.

I will test ;)

---
bod

[PATCH 2/2] media: venus: vdec: Start capture after a seek that streams CAPTURE first

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

After both queues are streamed off, the decoder sits in
VENUS_DEC_STATE_SEEK. vdec_start_capture() does nothing while OUTPUT is
not streaming, and when OUTPUT is streamed on again, the SEEK path only
queues the DPB buffers if the capture side was already started. So an
application that streams on CAPTURE before OUTPUT gets a decoder that
never receives DPB or capture buffers: the firmware consumes the
bitstream and no frame is ever returned.

The stateful decoder interface does not require either order, and
GStreamer restarts with CAPTURE first. Finish the capture start from the
OUTPUT SEEK path when the CAPTURE queue is already streaming, the same
way vdec_start_capture() does in VENUS_DEC_STATE_DECODING.

Assisted-by: LLM
Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/media/platform/qcom/venus/vdec.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 74591c7a820f5..ae6c2fc65404b 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -1128,33 +1128,51 @@ static int vdec_start_capture(struct venus_inst *inst)
 free_dpb_bufs:
 	venus_helper_free_dpb_bufs(inst);
 err:
 	return ret;
 }
 
 static int vdec_start_output(struct venus_inst *inst)
 {
+	struct vb2_queue *dst_vq;
+	bool cap_waiting;
 	int ret;
 
 	if (inst->codec_state == VENUS_DEC_STATE_SEEK) {
 		ret = venus_helper_process_initial_out_bufs(inst);
 		if (ret)
 			return ret;
 
 		if (inst->next_buf_last) {
 			inst->codec_state = VENUS_DEC_STATE_DRC;
 		} else {
 			inst->codec_state = VENUS_DEC_STATE_DECODING;
 
-			if (inst->streamon_cap) {
+			/*
+			 * If CAPTURE was streamed on first, vdec_start_capture()
+			 * returned early because OUTPUT was not streaming, so
+			 * start the capture side from here.
+			 */
+			dst_vq = v4l2_m2m_get_dst_vq(inst->m2m_ctx);
+			cap_waiting = !inst->streamon_cap && vb2_is_streaming(dst_vq);
+
+			if (inst->streamon_cap || cap_waiting) {
 				ret = venus_helper_queue_dpb_bufs(inst);
 				if (ret)
 					return ret;
 			}
+
+			if (cap_waiting) {
+				ret = venus_helper_process_initial_cap_bufs(inst);
+				if (ret)
+					return ret;
+
+				inst->streamon_cap = 1;
+			}
 		}
 		goto done;
 	}
 
 	if (inst->codec_state == VENUS_DEC_STATE_INIT ||
 	    inst->codec_state == VENUS_DEC_STATE_CAPTURE_SETUP) {
 		ret = venus_helper_process_initial_out_bufs(inst);
 		goto done;

-- 
2.55.0

Re: [PATCH 2/2] media: venus: vdec: Start capture after a seek that streams CAPTURE first

[bod]

On 2026-09-28 23:40 +0200, David Heidelberg wrote:
> After both queues are streamed off, the decoder sits in
> VENUS_DEC_STATE_SEEK. vdec_start_capture() does nothing while OUTPUT is
> not streaming, and when OUTPUT is streamed on again, the SEEK path only
> queues the DPB buffers if the capture side was already started. So an
> application that streams on CAPTURE before OUTPUT gets a decoder that
> never receives DPB or capture buffers: the firmware consumes the
> bitstream and no frame is ever returned.
> 
> The stateful decoder interface does not require either order, and
> GStreamer restarts with CAPTURE first. Finish the capture start from the
> OUTPUT SEEK path when the CAPTURE queue is already streaming, the same
> way vdec_start_capture() does in VENUS_DEC_STATE_DECODING.
> 
> Assisted-by: LLM
> Signed-off-by: David Heidelberg <david@ixit.cz>

This one is certainly missing at least a Fixes:
---
bod