Re: [PATCH] media: venus: only set H264_TRANSFORM_8X8 on supported hfi versions

[Stanimir Varbanov <stanimir.k.varbanov@gmail.com>]

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@noisolation.com>
> 
> ---
> 
> 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;

         /* FOLLOWING PROPERTIES ARE NOT IMPLEMENTED IN CORE YET */

> +			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 (inst->fmt_cap->pixfmt == V4L2_PIX_FMT_H264 ||
> --
> 2.34.1

-- 
regards,
Stan