On 5/3/2023 11:23 AM, Stanimir Varbanov wrote:
Hi,
On 3.05.23 г. 7:25 ч., Vikash Garodia wrote:
On 5/3/2023 2:22 AM, Stanimir Varbanov wrote:
On 14.04.23 г. 13:12 ч., Martin Dørum wrote:
Setting the H264_TRANSFORM_8X8 property only works on HFI versions
=4xx. The code used to unconditionally set the property in
venc_set_properties, which meant that initializing the encoder would
always fail unless the hfi_version was >=4xx.
This patch changes venc_set_properties to only set the
H264_TRANSFORM_8X8 property if the hfi version is >=4xx.
Signed-off-by: Martin Dørum <dorum@xxxxxxxxxxxxxxx>
---
I have an APQ8016-based board. Before this patch, the Venus driver
would simply fail with EINVAL when trying to request buffers
(VIDIOC_REQBUFS). With this patch, encoding works
(tested using gstreamer's v4l2h264enc).
drivers/media/platform/qcom/venus/venc.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/venc.c
b/drivers/media/platform/qcom/venus/venc.c
index cdb12546c4fa..b3df805a8c9c 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -672,16 +672,17 @@ static int venc_set_properties(struct
venus_inst *inst)
if (ret)
return ret;
- ptype = HFI_PROPERTY_PARAM_VENC_H264_TRANSFORM_8X8;
- h264_transform.enable_type = 0;
- if (ctr->profile.h264 == V4L2_MPEG_VIDEO_H264_PROFILE_HIGH ||
- ctr->profile.h264 ==
V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_HIGH)
- h264_transform.enable_type = ctr->h264_8x8_transform;
-
- ret = hfi_session_set_property(inst, ptype, &h264_transform);
- if (ret)
- return ret;
-
+ if (!IS_V1(inst->core) && !IS_V3(inst->core)) {
Instead of doing these checks here you could do:
diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c
b/drivers/media/platform/qcom/venus/hfi_cmds.c
index bc3f8ff05840..2453e5c3d244 100644
--- a/drivers/media/platform/qcom/venus/hfi_cmds.c
+++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
@@ -1064,6 +1064,7 @@ static int pkt_session_set_property_1x(struct
hfi_session_set_property_pkt *pkt,
break;
}
case HFI_PROPERTY_PARAM_VENC_HDR10_PQ_SEI:
+ case HFI_PROPERTY_PARAM_VENC_H264_TRANSFORM_8X8:
return -ENOTSUPP;
This may still deinit the session from [1] based on failure return
value.
[1]
https://elixir.bootlin.com/linux/v6.3/source/drivers/media/platform/qcom/venus/venc.c#L963
No, it will not fail because of:
https://elixir.bootlin.com/linux/v6.3/source/drivers/media/platform/qcom/venus/hfi_venus.c#L1426
Thats correct, I missed to notice the explicit handling for -ENOTSUPP.
Above suggestion is better than keeping verison checks and
would also avoid deinit.
