Re: [PATCH 10/10] venus: dec: make decoder compliant with stateful codec API
On 2019-01-21 16:50, mgot...@codeaurora.org wrote:
On 2019-01-17 21:50, Stanimir Varbanov wrote:
This refactored code for start/stop streaming vb2 operations and
adds a state machine handling similar to the one in stateful codec
API documentation. One major change is that now the HFI session is
started on STREAMON(OUTPUT) and stopped on REQBUF(OUTPUT,count=0),
during that time streamoff(cap,out) just flush buffers but doesn't
stop the session. The other major change is that now the capture
and output queues are completely separated.
Signed-off-by: Stanimir Varbanov
---
drivers/media/platform/qcom/venus/core.h| 20 +-
drivers/media/platform/qcom/venus/helpers.c | 23 +-
drivers/media/platform/qcom/venus/helpers.h | 5 +
drivers/media/platform/qcom/venus/vdec.c| 449
4 files changed, 389 insertions(+), 108 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/core.h
b/drivers/media/platform/qcom/venus/core.h
index 79c7e816c706..5a133c203455 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -218,6 +218,15 @@ struct venus_buffer {
#define to_venus_buffer(ptr) container_of(ptr, struct venus_buffer,
vb)
+#define DEC_STATE_UNINIT 0
+#define DEC_STATE_INIT 1
+#define DEC_STATE_CAPTURE_SETUP2
+#define DEC_STATE_STOPPED 3
+#define DEC_STATE_SEEK 4
+#define DEC_STATE_DRAIN5
+#define DEC_STATE_DECODING 6
+#define DEC_STATE_DRC 7
+
/**
* struct venus_inst - holds per instance paramerters
*
@@ -241,6 +250,10 @@ struct venus_buffer {
* @colorspace:current color space
* @quantization: current quantization
* @xfer_func: current xfer function
+ * @codec_state: current codec API state (see DEC/ENC_STATE_)
+ * @reconf_wait: wait queue for resolution change event
+ * @ten_bits: does new stream is 10bits depth
+ * @buf_count: used to count number number of buffers (reqbuf(0))
* @fps: holds current FPS
* @timeperframe: holds current time per frame structure
* @fmt_out: a reference to output format structure
@@ -255,8 +268,6 @@ struct venus_buffer {
* @opb_buftype: output picture buffer type
* @opb_fmt: output picture buffer raw format
* @reconfig: a flag raised by decoder when the stream resolution
changed
- * @reconfig_width:holds the new width
- * @reconfig_height: holds the new height
* @hfi_codec: current codec for this instance in HFI space
* @sequence_cap: a sequence counter for capture queue
* @sequence_out: a sequence counter for output queue
@@ -296,6 +307,9 @@ struct venus_inst {
u8 ycbcr_enc;
u8 quantization;
u8 xfer_func;
+ unsigned int codec_state;
+ wait_queue_head_t reconf_wait;
+ int buf_count;
u64 fps;
struct v4l2_fract timeperframe;
const struct venus_format *fmt_out;
@@ -310,8 +324,6 @@ struct venus_inst {
u32 opb_buftype;
u32 opb_fmt;
bool reconfig;
- u32 reconfig_width;
- u32 reconfig_height;
u32 hfi_codec;
u32 sequence_cap;
u32 sequence_out;
diff --git a/drivers/media/platform/qcom/venus/helpers.c
b/drivers/media/platform/qcom/venus/helpers.c
index 637ce7b82d94..25d8cceccae4 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -1030,16 +1030,15 @@ void venus_helper_vb2_buf_queue(struct
vb2_buffer *vb)
v4l2_m2m_buf_queue(m2m_ctx, vbuf);
- if (!(inst->streamon_out & inst->streamon_cap))
- goto unlock;
-
- ret = is_buf_refed(inst, vbuf);
- if (ret)
- goto unlock;
+ if (IS_OUT(vb->vb2_queue, inst) || IS_CAP(vb->vb2_queue, inst)) {
+ ret = is_buf_refed(inst, vbuf);
+ if (ret)
+ goto unlock;
- ret = session_process_buf(inst, vbuf);
- if (ret)
- return_buf_error(inst, vbuf);
+ ret = session_process_buf(inst, vbuf);
+ if (ret)
+ return_buf_error(inst, vbuf);
+ }
unlock:
mutex_unlock(>lock);
Hi Stan,
In case of encoder, we are queuing buffers only after both planes are
streamed on.
As we don’t have any reconfig event in case of encoder,
it’s better if we stick to the earlier implementation of queuing
buffers.
So I would recommend to add a check for the same in the below way :
diff --git a/drivers/media/platform/qcom/venus/helpers.c
b/drivers/media/platform/qcom/venus/helpers.c
index 25d8cce..cc490fe2 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -1029,6 +1029,8 @@ void venus_helper_vb2_buf_queue(struct vb2_buffer
*vb)
mutex_lock(>lock);
v4l2_m2m_buf_queue(m2m_ctx, vbuf);
+ if
Re: [PATCH 10/10] venus: dec: make decoder compliant with stateful codec API
On 2019-01-17 21:50, Stanimir Varbanov wrote:
This refactored code for start/stop streaming vb2 operations and
adds a state machine handling similar to the one in stateful codec
API documentation. One major change is that now the HFI session is
started on STREAMON(OUTPUT) and stopped on REQBUF(OUTPUT,count=0),
during that time streamoff(cap,out) just flush buffers but doesn't
stop the session. The other major change is that now the capture
and output queues are completely separated.
Signed-off-by: Stanimir Varbanov
---
drivers/media/platform/qcom/venus/core.h| 20 +-
drivers/media/platform/qcom/venus/helpers.c | 23 +-
drivers/media/platform/qcom/venus/helpers.h | 5 +
drivers/media/platform/qcom/venus/vdec.c| 449
4 files changed, 389 insertions(+), 108 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/core.h
b/drivers/media/platform/qcom/venus/core.h
index 79c7e816c706..5a133c203455 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -218,6 +218,15 @@ struct venus_buffer {
#define to_venus_buffer(ptr) container_of(ptr, struct venus_buffer,
vb)
+#define DEC_STATE_UNINIT 0
+#define DEC_STATE_INIT 1
+#define DEC_STATE_CAPTURE_SETUP2
+#define DEC_STATE_STOPPED 3
+#define DEC_STATE_SEEK 4
+#define DEC_STATE_DRAIN5
+#define DEC_STATE_DECODING 6
+#define DEC_STATE_DRC 7
+
/**
* struct venus_inst - holds per instance paramerters
*
@@ -241,6 +250,10 @@ struct venus_buffer {
* @colorspace:current color space
* @quantization: current quantization
* @xfer_func: current xfer function
+ * @codec_state: current codec API state (see DEC/ENC_STATE_)
+ * @reconf_wait: wait queue for resolution change event
+ * @ten_bits: does new stream is 10bits depth
+ * @buf_count: used to count number number of buffers (reqbuf(0))
* @fps: holds current FPS
* @timeperframe: holds current time per frame structure
* @fmt_out: a reference to output format structure
@@ -255,8 +268,6 @@ struct venus_buffer {
* @opb_buftype: output picture buffer type
* @opb_fmt: output picture buffer raw format
* @reconfig: a flag raised by decoder when the stream resolution
changed
- * @reconfig_width:holds the new width
- * @reconfig_height: holds the new height
* @hfi_codec: current codec for this instance in HFI space
* @sequence_cap: a sequence counter for capture queue
* @sequence_out: a sequence counter for output queue
@@ -296,6 +307,9 @@ struct venus_inst {
u8 ycbcr_enc;
u8 quantization;
u8 xfer_func;
+ unsigned int codec_state;
+ wait_queue_head_t reconf_wait;
+ int buf_count;
u64 fps;
struct v4l2_fract timeperframe;
const struct venus_format *fmt_out;
@@ -310,8 +324,6 @@ struct venus_inst {
u32 opb_buftype;
u32 opb_fmt;
bool reconfig;
- u32 reconfig_width;
- u32 reconfig_height;
u32 hfi_codec;
u32 sequence_cap;
u32 sequence_out;
diff --git a/drivers/media/platform/qcom/venus/helpers.c
b/drivers/media/platform/qcom/venus/helpers.c
index 637ce7b82d94..25d8cceccae4 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -1030,16 +1030,15 @@ void venus_helper_vb2_buf_queue(struct
vb2_buffer *vb)
v4l2_m2m_buf_queue(m2m_ctx, vbuf);
- if (!(inst->streamon_out & inst->streamon_cap))
- goto unlock;
-
- ret = is_buf_refed(inst, vbuf);
- if (ret)
- goto unlock;
+ if (IS_OUT(vb->vb2_queue, inst) || IS_CAP(vb->vb2_queue, inst)) {
+ ret = is_buf_refed(inst, vbuf);
+ if (ret)
+ goto unlock;
- ret = session_process_buf(inst, vbuf);
- if (ret)
- return_buf_error(inst, vbuf);
+ ret = session_process_buf(inst, vbuf);
+ if (ret)
+ return_buf_error(inst, vbuf);
+ }
unlock:
mutex_unlock(>lock);
Hi Stan,
In case of encoder, we are queuing buffers only after both planes are
streamed on.
As we don’t have any reconfig event in case of encoder,
it’s better if we stick to the earlier implementation of queuing
buffers.
So I would recommend to add a check for the same in the below way :
diff --git a/drivers/media/platform/qcom/venus/helpers.c
b/drivers/media/platform/qcom/venus/helpers.c
index 25d8cce..cc490fe2 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -1029,6 +1029,8 @@ void venus_helper_vb2_buf_queue(struct vb2_buffer
*vb)
mutex_lock(>lock);
v4l2_m2m_buf_queue(m2m_ctx, vbuf);
+ if (inst->session_type == VIDC_SESSION_TYPE_ENC &&
Re: [PATCH v4] arm64: dts: sdm845: add video nodes
On 2019-01-07 20:00, Malathi Gottam wrote:
This adds video nodes to sdm845 based on the examples
in the bindings.
Signed-off-by: Malathi Gottam
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 35
+++
1 file changed, 35 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index b72bdb0..df31d39 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -87,6 +87,11 @@
reg = <0 0x8620 0 0x2d0>;
no-map;
};
+
+ venus_region: memory@9580 {
+ reg = <0x0 0x9580 0x0 0x60>;
+ no-map;
+ };
};
cpus {
@@ -1403,5 +1408,35 @@
status = "disabled";
};
};
+
+ video-codec@aa0 {
+ compatible = "qcom,sdm845-venus";
+ reg = <0x0aa0 0xff000>;
+ interrupts = ;
+ power-domains = < VENUS_GDSC>;
+ clocks = < VIDEO_CC_VENUS_CTL_CORE_CLK>,
+< VIDEO_CC_VENUS_AHB_CLK>,
+< VIDEO_CC_VENUS_CTL_AXI_CLK>;
+ clock-names = "core", "iface", "bus";
+ iommus = <_smmu 0x10a0 0x8>,
+<_smmu 0x10b0 0x0>;
+ memory-region = <_region>;
+
+ video-core0 {
+ compatible = "venus-decoder";
+ clocks = < VIDEO_CC_VCODEC0_CORE_CLK>,
+< VIDEO_CC_VCODEC0_AXI_CLK>;
+ clock-names = "core", "bus";
+ power-domains = < VCODEC0_GDSC>;
+ };
+
+ video-core1 {
+ compatible = "venus-encoder";
+ clocks = < VIDEO_CC_VCODEC1_CORE_CLK>,
+< VIDEO_CC_VCODEC1_AXI_CLK>;
+ clock-names = "core", "bus";
+ power-domains = < VCODEC1_GDSC>;
+ };
+ };
};
};
This patch is no longer needed as the size mismatch patch has been
posted
https://patchwork.kernel.org/patch/10753645/
So please consider v3 as the latest one.
Thanks,
Malathi.
Re: [PATCH v2] arm64: dts: sdm845: add video nodes
On 2018-11-30 12:09, Alexandre Courbot wrote:
On Wed, Nov 28, 2018 at 10:12 PM Malathi Gottam
wrote:
This adds video nodes to sdm845 based on the examples
in the bindings.
Signed-off-by: Malathi Gottam
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 35
+++
1 file changed, 35 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 0c9a2aa..4c9d6a4 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -84,6 +84,11 @@
reg = <0 0x8620 0 0x2d0>;
no-map;
};
+
+ venus_region: memory@9580 {
+ reg = <0x0 0x9580 0x0 0x50>;
This patch depends on the firmware loader being fixed to not expect a
6MB area size. I think you can do it as follows:
For now, we are proceeding on adding node with memory region as 6MB.
Once we have the other patch for fixing firmware loader merged,
we can then try on recording the reserved area size.
I am posting updated patch with hard coded memory region.
Regards,
Malathi.
1) Record the reserved area size in the venus_core structure during
venus_load_fw(),
2) Use that area size in place of VENUS_FW_MEM_SIZE everywhere else in
the code.
That way we don't put any artificial limitation on the expected
firmware size, or on the reserved area in the DT matching a hardcoded
size.
Once you have this, the driver should work no matter what the size of
the reserved area is, provided the firmware first into it.
+ no-map;
+ };
};
cpus {
@@ -1103,5 +1108,35 @@
status = "disabled";
};
};
+
+ video-codec@aa0 {
+ compatible = "qcom,sdm845-venus";
+ reg = <0x0aa0 0xff000>;
+ interrupts = IRQ_TYPE_LEVEL_HIGH>;
+ power-domains = < VENUS_GDSC>;
+ clocks = <
VIDEO_CC_VENUS_CTL_CORE_CLK>,
+< VIDEO_CC_VENUS_AHB_CLK>,
+<
VIDEO_CC_VENUS_CTL_AXI_CLK>;
+ clock-names = "core", "iface", "bus";
+ iommus = <_smmu 0x10a0 0x8>,
+<_smmu 0x10b0 0x0>;
+ memory-region = <_region>;
+
+ video-core0 {
+ compatible = "venus-decoder";
+ clocks = <
VIDEO_CC_VCODEC0_CORE_CLK>,
+<
VIDEO_CC_VCODEC0_AXI_CLK>;
+ clock-names = "core", "bus";
+ power-domains = <
VCODEC0_GDSC>;
+ };
+
+ video-core1 {
+ compatible = "venus-encoder";
+ clocks = <
VIDEO_CC_VCODEC1_CORE_CLK>,
+<
VIDEO_CC_VCODEC1_AXI_CLK>;
+ clock-names = "core", "bus";
+ power-domains = <
VCODEC1_GDSC>;
+ };
We should probably have status = "disabled" here and enable this node
on a per-board basis?
Also, shouldn't we define the firmware subnode here too?
Cheers,
Alex.
--
1.9.1
