Re: [PATCH v2 0/6] ARM: dts: sun8i: v3s: Enable video decoder

[Hans Verkuil]

On 16/11/2020 13:07, Martin Cerveny wrote:
> Hello.
> 
> On Mon, 16 Nov 2020, Martin Cerveny wrote:
>> On Mon, 16 Nov 2020, Hans Verkuil wrote:
>>> On 15/11/2020 19:59, Martin Cerveny wrote:
>>>> On Thu, 5 Nov 2020, Hans Verkuil wrote:
>>>>> On 12/09/2020 16:30, Martin Cerveny wrote:
>>>>>> First patch extends cedrus capability to all decoders
>>>>>> because V3s missing MPEG2 decoder.
>>>>>>
>>>>>> Next two patches add system control node (SRAM C1) and
>>>>>> next three patches add support for Cedrus VPU.
>>>>>>
>>>>>> Tested on "Lichee Zero" V3s platform with testing LCD patch
>>>>>> ( https://github.com/mcerveny/linux/tree/v3s_videocodec_v4 )
>>>>>> and V4L2 raw API testing utility
>>>>>> ( https://github.com/mcerveny/v4l2-request-test ):
>>>>>> - enabled LCD (DRM dual VI and sigle UI planes)
>>>>>> - added RGB panel
>>>>>> - enabled PWM
>>>>>>
>>>>>> There is low memory on V3s (64MB) and maximum must be available to CMA:
>>>>>> - CONFIG_CMA_SIZE_MBYTES=28
>>>>>> - add swap to swapout other processes
>>>>>> - decrease buffers in v4l2-request-test (.buffers_count from 16 to 6)
>>>>>>
>>>>>> Only H.264 decoder working - MPEG and H.265 unsupported by V3s,
>>>>>> JPEG/MJPEG still unimplemented, encoder unimplemented
>>>>>
>>>>> When I tried to merged these patches I got merge conflicts.
>>>>>
>>>>> Possibly due to other 5.10 changes, but certainly because of conflicts
>>>>> with patches from Jernej:
>>>>>
>>>>> https://patchwork.linuxtv.org/project/linux-media/patch/20200825173523.1289379-4-jernej.skrabec@xxxxxxxx/
>>>>> https://patchwork.linuxtv.org/project/linux-media/patch/20200825173523.1289379-5-jernej.skrabec@xxxxxxxx/
>>>>>
>>>>> I've merged Jerne's patches and posted a PR for that:
>>>>> https://patchwork.linuxtv.org/project/linux-media/patch/f3b8e5e2-5f0e-fb6f-e5b2-7f44f7e365e7@xxxxxxxxx/
>>>>>
>>>>> Can you rebase your patches on top of my branch that contains Jernej's 
>>>>> patches?
>>>>>
>>>>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=for-v5.11e
>>>>>
>>>>> Once my PR is merged into the media_tree master I can take your rebased
>>>>> patches.
>>>>
>>>> I updated patches:
>>>> https://github.com/mcerveny/linux/tree/media_tree_for-v5.11e
>>>>
>>>> BUT, commit (555 commits) for v5.10-1
>>>> https://github.com/torvalds/linux/commit/fd5c32d80884268a381ed0e67cccef0b3d37750b
>>>> disrupts usability of Cedrus H.264 (at least for my Allwinner V3s):
>>>>
>>>> 1) colors are disrupted
>>>>
>>>> There are missing some initialization now.
>>>>
>>>> If I use "5.9" compatible code
>>>> (last bisect good point 
>>>> https://github.com/torvalds/linux/commit/647412daeb454b6dad12a6c6961ab90aac9e5d29 
>>>> )
>>>> then reboot (not power-off!) and use new code
>>>> ( https://github.com/mcerveny/linux/tree/media_tree_for-v5.11e )
>>>> and colors are OK.
>>>
>>> Could this or the next issue be related to this bug fix?
>>>
>>> https://git.linuxtv.org/media_tree.git/commit/?h=fixes&id=9ac924b98728c3733c91c6c59fc410827d0da49f
>>>
>>> That fix isn't yet in our master tree.
>>
>> Confirmed. It recovers colors !
>>
>>>>
>>>> 2) decoding of complex streams fails
>>>>
>>>> ( https://github.com/mcerveny/v4l2-request-test/tree/v5.10 )
>>>> - bbb-h264-all-i-32 - OK
>>>> - bbb-h264-32 - bad from frame 5
>>>> - bbb-h264-high-32 - bad from frame 6
>>>
>>> I think cedrus devs need to take a look at these issues.
>>
>> Maybe something wrong in my testing code,
>> problematic commit swapped some variables between structures :-(
>>
>> I try to investigate more, regards Martin
> 
> Yes. I found new format in ref_pic_list.
> I updated test application. All H.264 tests are working now.
> 
> https://github.com/mcerveny/v4l2-request-test
> 
> Is it necessary to release new "Patch V3" (rebased) series from 
> https://github.com/mcerveny/linux/tree/media_tree_for-v5.11e or not ?

Yes, please! That would be much appreciated.

Regards,

	Hans

> 
> Regards, Martin
> 
>>> Regards,
>>>
>>> 	Hans
>>>
>>>>
>>>> best regards,
>>>> Martin
>>>>
>>>>>> Changes since v1:
>>>>>> - patch 0005 rename
>>>>>> - added testing description
>>>>>>
>>>>>> Martin Cerveny (6):
>>>>>>   media: cedrus: Register all codecs as capability
>>>>>>   dt-bindings: sram: allwinner,sun4i-a10-system-control: Add V3s
>>>>>>     compatibles
>>>>>>   ARM: dts: sun8i: v3s: Add node for system control
>>>>>>   media: cedrus: Add support for V3s
>>>>>>   dt-bindings: media: cedrus: Add V3s compatible
>>>>>>   ARM: dts: sun8i: v3s: Add video engine node
>>>>>>
>>>>>>  .../allwinner,sun4i-a10-video-engine.yaml     |  1 +
>>>>>>  .../allwinner,sun4i-a10-system-control.yaml   |  6 ++++
>>>>>>  arch/arm/boot/dts/sun8i-v3s.dtsi              | 33 +++++++++++++++++++
>>>>>>  drivers/staging/media/sunxi/cedrus/cedrus.c   | 28 +++++++++++++++-
>>>>>>  drivers/staging/media/sunxi/cedrus/cedrus.h   |  2 ++
>>>>>>  .../staging/media/sunxi/cedrus/cedrus_video.c |  2 ++
>>>>>>  6 files changed, 71 insertions(+), 1 deletion(-)
>>>>>>
>>>>>
>>>
>>