On 5/12/2023 4:02 PM, Bryan O'Donoghue wrote:
> On 02/05/2023 17:37, Bryan O'Donoghue wrote:
>> On 14/04/2023 11: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)) {
>>> + 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
>>
>> I agree that a Fixes should be added.
>>
>> Fixes: bfee75f73c37 ("media: venus: venc: add support for
>> V4L2_CID_MPEG_VIDEO_H264_8X8_TRANSFORM control")
>>
>> When sending out your V2, please remember to cc -> Hans Verkuil
>> <hverkuil-cisco@xxxxxxxxx>
>>
>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>
>
> Hey Martin.
>
> I tried verifying the before/after of your patch last night on db410c as @
> https://git.codelinaro.org/bryan.odonoghue/kernel/-/tree/linux-next-23-05-11-venus-check
>
> I don't see any difference with h264 playback with or without your patch.
>
> Could you share a command to verify the bug against ?
Probably try an encode session.
-Vikash
>
> ---
> bod
