[0/9] media: rkvdec: Add HEVC backend

  This series add a HEVC backend to the Rockchip Video Decoder driver.

A version of this HEVC backend has been in use by the LibreELEC distro
for the past 3+ years [1]. It was initially created based on a copy of
the H264 backend, unstable HEVC uAPI controls and a cabac table + scaling
matrix functions shamelessly copied 1:1 from the Rockchip mpp library.

It has since then been extended to use the stable HEVC uAPI controls and
improved opon e.g. to include support for rk3288 and fix decoding issues
by Alex Bee and Nicolas Dufresne.

The version submitted in this series is based on the code currently used
by the LibreELEC distro, excluding hard/soft reset, and with cabac table
and scaling matrix functions picked from Sebastian Fricke prior series
to add a HEVC backend [2].

Big thanks to Alex Bee, Nicolas Dufresne and Sebastian Fricke for making
this series possible!

Patch 1 add the new HEVC backend.
Patch 2-3 add variants support to the driver.
Patch 4 add support for a rk3288 variant.
Patch 5 add a rk3328 variant to work around hw quirks.
Patch 6-7 add device tree node for rk3288.
Patch 8-9 extend vdec node reg size to include cache/perf registers.

This was tested on a ROCK Pi 4 (RK3399) and Rock64 (RK3328):

  v4l2-compliance 1.24.1, 64 bits, 64-bit time_t
  ...
  Total for rkvdec device /dev/video1: 46, Succeeded: 46, Failed: 0, Warnings: 0

  Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
  ...
  Ran 135/147 tests successfully

  Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
  ...
  Ran 9/9 tests successfully

And on a TinkerBoard (RK3288):

  v4l2-compliance 1.24.1, 32 bits, 32-bit time_t
  ...
  Total for rkvdec device /dev/video3: 47, Succeeded: 47, Failed: 0, Warnings: 0

  Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
  ...
  Ran 137/147 tests successfully

  Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
  ...
  Ran 9/9 tests successfully

This series depend on the following series:
- media: rkvdec: Add H.264 High 10 and 4:2:2 profile support [3]

To fully runtime test this series you need above series and ffmpeg
patches from [4], this series and its depends is also available at [5].

Full summary of fluster run can be found at [6].

[1] https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/patches/linux/default/linux-2000-v4l2-wip-rkvdec-hevc.patch
[2] https://lore.kernel.org/linux-media/20230101-patch-series-v2-6-2-rc1-v2-0-fa1897efac14@collabora.com/
[3] https://lore.kernel.org/linux-media/20231105165521.3592037-1-jonas@kwiboo.se/
[4] https://github.com/Kwiboo/FFmpeg/commits/v4l2-request-n6.1-dev/
[5] https://github.com/Kwiboo/linux-rockchip/commits/linuxtv-rkvdec-hevc-v1/
[6] https://gist.github.com/Kwiboo/4c0ed87774dede44ce6838451a1ec93d

Regards,
Jonas

Alex Bee (5):
  media: rkvdec: Add variants support
  media: rkvdec: Add RK3288 variant
  media: rkvdec: Disable QoS for HEVC and VP9 on RK3328
  ARM: dts: rockchip: Add vdec node for RK3288
  arm64: dts: rockchip: Expand reg size of vdec node for RK3399

Jonas Karlman (4):
  media: rkvdec: Add HEVC backend
  media: rkvdec: Implement capability filtering
  media: dt-bindings: rockchip,vdec: Add RK3288 compatible
  arm64: dts: rockchip: Expand reg size of vdec node for RK3328

 .../bindings/media/rockchip,vdec.yaml         |    4 +-
 arch/arm/boot/dts/rockchip/rk3288.dtsi        |   17 +-
 arch/arm64/boot/dts/rockchip/rk3328.dtsi      |    2 +-
 arch/arm64/boot/dts/rockchip/rk3399.dtsi      |    6 +-
 drivers/staging/media/rkvdec/Makefile         |    2 +-
 drivers/staging/media/rkvdec/TODO             |    7 -
 .../staging/media/rkvdec/rkvdec-hevc-data.c   | 1848 +++++++++++++++++
 drivers/staging/media/rkvdec/rkvdec-hevc.c    |  823 ++++++++
 drivers/staging/media/rkvdec/rkvdec-regs.h    |    3 +
 drivers/staging/media/rkvdec/rkvdec-vp9.c     |   10 +
 drivers/staging/media/rkvdec/rkvdec.c         |  180 +-
 drivers/staging/media/rkvdec/rkvdec.h         |   15 +
 12 files changed, 2886 insertions(+), 31 deletions(-)
 create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc-data.c
 create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc.c
  

Hi Jonas,

Am Montag, 6. November 2023, 00:36:07 CET schrieb Jonas Karlman:
> This series add a HEVC backend to the Rockchip Video Decoder driver.
> 
> A version of this HEVC backend has been in use by the LibreELEC distro
> for the past 3+ years [1]. It was initially created based on a copy of
> the H264 backend, unstable HEVC uAPI controls and a cabac table + scaling
> matrix functions shamelessly copied 1:1 from the Rockchip mpp library.
> 
> It has since then been extended to use the stable HEVC uAPI controls and
> improved opon e.g. to include support for rk3288 and fix decoding issues
> by Alex Bee and Nicolas Dufresne.
> 
> The version submitted in this series is based on the code currently used
> by the LibreELEC distro, excluding hard/soft reset, and with cabac table
> and scaling matrix functions picked from Sebastian Fricke prior series
> to add a HEVC backend [2].
> 
> Big thanks to Alex Bee, Nicolas Dufresne and Sebastian Fricke for making
> this series possible!
> 
> Patch 1 add the new HEVC backend.
> Patch 2-3 add variants support to the driver.
> Patch 4 add support for a rk3288 variant.
> Patch 5 add a rk3328 variant to work around hw quirks.
> Patch 6-7 add device tree node for rk3288.
> Patch 8-9 extend vdec node reg size to include cache/perf registers.

thanks a lot for working on this.

Looking at the rkvdec TODO file, isn't the hvec support the only thing
keeping the driver in staging? So with this support using stable hevc
uapi, shouldn't the driver then also move out of staging after this
series is applied?

Heiko


> This was tested on a ROCK Pi 4 (RK3399) and Rock64 (RK3328):
> 
>   v4l2-compliance 1.24.1, 64 bits, 64-bit time_t
>   ...
>   Total for rkvdec device /dev/video1: 46, Succeeded: 46, Failed: 0, Warnings: 0
> 
>   Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
>   ...
>   Ran 135/147 tests successfully
> 
>   Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
>   ...
>   Ran 9/9 tests successfully
> 
> And on a TinkerBoard (RK3288):
> 
>   v4l2-compliance 1.24.1, 32 bits, 32-bit time_t
>   ...
>   Total for rkvdec device /dev/video3: 47, Succeeded: 47, Failed: 0, Warnings: 0
> 
>   Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
>   ...
>   Ran 137/147 tests successfully
> 
>   Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
>   ...
>   Ran 9/9 tests successfully
> 
> This series depend on the following series:
> - media: rkvdec: Add H.264 High 10 and 4:2:2 profile support [3]
> 
> To fully runtime test this series you need above series and ffmpeg
> patches from [4], this series and its depends is also available at [5].
> 
> Full summary of fluster run can be found at [6].
> 
> [1] https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/patches/linux/default/linux-2000-v4l2-wip-rkvdec-hevc.patch
> [2] https://lore.kernel.org/linux-media/20230101-patch-series-v2-6-2-rc1-v2-0-fa1897efac14@collabora.com/
> [3] https://lore.kernel.org/linux-media/20231105165521.3592037-1-jonas@kwiboo.se/
> [4] https://github.com/Kwiboo/FFmpeg/commits/v4l2-request-n6.1-dev/
> [5] https://github.com/Kwiboo/linux-rockchip/commits/linuxtv-rkvdec-hevc-v1/
> [6] https://gist.github.com/Kwiboo/4c0ed87774dede44ce6838451a1ec93d
> 
> Regards,
> Jonas
> 
> Alex Bee (5):
>   media: rkvdec: Add variants support
>   media: rkvdec: Add RK3288 variant
>   media: rkvdec: Disable QoS for HEVC and VP9 on RK3328
>   ARM: dts: rockchip: Add vdec node for RK3288
>   arm64: dts: rockchip: Expand reg size of vdec node for RK3399
> 
> Jonas Karlman (4):
>   media: rkvdec: Add HEVC backend
>   media: rkvdec: Implement capability filtering
>   media: dt-bindings: rockchip,vdec: Add RK3288 compatible
>   arm64: dts: rockchip: Expand reg size of vdec node for RK3328
> 
>  .../bindings/media/rockchip,vdec.yaml         |    4 +-
>  arch/arm/boot/dts/rockchip/rk3288.dtsi        |   17 +-
>  arch/arm64/boot/dts/rockchip/rk3328.dtsi      |    2 +-
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi      |    6 +-
>  drivers/staging/media/rkvdec/Makefile         |    2 +-
>  drivers/staging/media/rkvdec/TODO             |    7 -
>  .../staging/media/rkvdec/rkvdec-hevc-data.c   | 1848 +++++++++++++++++
>  drivers/staging/media/rkvdec/rkvdec-hevc.c    |  823 ++++++++
>  drivers/staging/media/rkvdec/rkvdec-regs.h    |    3 +
>  drivers/staging/media/rkvdec/rkvdec-vp9.c     |   10 +
>  drivers/staging/media/rkvdec/rkvdec.c         |  180 +-
>  drivers/staging/media/rkvdec/rkvdec.h         |   15 +
>  12 files changed, 2886 insertions(+), 31 deletions(-)
>  create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc-data.c
>  create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc.c
> 
>
  

On Sun, 5 Nov 2023 23:36:07 +0000, Jonas Karlman wrote:
> This series add a HEVC backend to the Rockchip Video Decoder driver.
> 
> A version of this HEVC backend has been in use by the LibreELEC distro
> for the past 3+ years [1]. It was initially created based on a copy of
> the H264 backend, unstable HEVC uAPI controls and a cabac table + scaling
> matrix functions shamelessly copied 1:1 from the Rockchip mpp library.
> 
> [...]

Applied as fix for 6.7-rc, thanks!

[8/9] arm64: dts: rockchip: Expand reg size of vdec node for RK3328
      commit: 667a89fe8383de1be44a8cdf22716448f84ee510
[9/9] arm64: dts: rockchip: Expand reg size of vdec node for RK3399
      commit: a31d776cccb98e0763dadce023ad5caa1a6dce2c

Expanding the range up to the start of the MMU should always be the
correct way, so this doesn't need the rest of the series :-) .


Best regards,
  

Hi Jonas,

On 06/11/2023 00:36, Jonas Karlman wrote:
> This series add a HEVC backend to the Rockchip Video Decoder driver.
> 
> A version of this HEVC backend has been in use by the LibreELEC distro
> for the past 3+ years [1]. It was initially created based on a copy of
> the H264 backend, unstable HEVC uAPI controls and a cabac table + scaling
> matrix functions shamelessly copied 1:1 from the Rockchip mpp library.
> 
> It has since then been extended to use the stable HEVC uAPI controls and
> improved opon e.g. to include support for rk3288 and fix decoding issues
> by Alex Bee and Nicolas Dufresne.
> 
> The version submitted in this series is based on the code currently used
> by the LibreELEC distro, excluding hard/soft reset, and with cabac table
> and scaling matrix functions picked from Sebastian Fricke prior series
> to add a HEVC backend [2].
> 
> Big thanks to Alex Bee, Nicolas Dufresne and Sebastian Fricke for making
> this series possible!

I ran this series through smatch and found these two issues:

drivers/staging/media/rkvdec/rkvdec-hevc.c: In function 'transpose_and_flatten_matrices':
drivers/staging/media/rkvdec/rkvdec-hevc.c:429:83: warning: variable 'new_value' set but not used [-Wunused-but-set-variable]
  429 |         int i, j, row, x_offset, matrix_offset, rot_index, y_offset, matrix_size, new_value;
      |                                                                                   ^~~~~~~~~
drivers/staging/media/rkvdec/rkvdec-hevc.c:756 rkvdec_hevc_run_preamble() error: we previously assumed 'ctrl' could be null (see line 755)

Also, this series drops the HEVC part from the TODO file, but
I wonder if the last remaining item is still valid:

* Evaluate introducing a helper to consolidate duplicated
  code in rkvdec_request_validate and cedrus_request_validate.
  The helper needs to the driver private data associated with
  the videobuf2 queue, from a media request.

It doesn't look like there is much duplicate code at all. It is certainly not
something that prevents this driver from moving out of staging.

Regards,

	Hans

> 
> Patch 1 add the new HEVC backend.
> Patch 2-3 add variants support to the driver.
> Patch 4 add support for a rk3288 variant.
> Patch 5 add a rk3328 variant to work around hw quirks.
> Patch 6-7 add device tree node for rk3288.
> Patch 8-9 extend vdec node reg size to include cache/perf registers.
> 
> This was tested on a ROCK Pi 4 (RK3399) and Rock64 (RK3328):
> 
>   v4l2-compliance 1.24.1, 64 bits, 64-bit time_t
>   ...
>   Total for rkvdec device /dev/video1: 46, Succeeded: 46, Failed: 0, Warnings: 0
> 
>   Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
>   ...
>   Ran 135/147 tests successfully
> 
>   Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
>   ...
>   Ran 9/9 tests successfully
> 
> And on a TinkerBoard (RK3288):
> 
>   v4l2-compliance 1.24.1, 32 bits, 32-bit time_t
>   ...
>   Total for rkvdec device /dev/video3: 47, Succeeded: 47, Failed: 0, Warnings: 0
> 
>   Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
>   ...
>   Ran 137/147 tests successfully
> 
>   Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
>   ...
>   Ran 9/9 tests successfully
> 
> This series depend on the following series:
> - media: rkvdec: Add H.264 High 10 and 4:2:2 profile support [3]
> 
> To fully runtime test this series you need above series and ffmpeg
> patches from [4], this series and its depends is also available at [5].
> 
> Full summary of fluster run can be found at [6].
> 
> [1] https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/patches/linux/default/linux-2000-v4l2-wip-rkvdec-hevc.patch
> [2] https://lore.kernel.org/linux-media/20230101-patch-series-v2-6-2-rc1-v2-0-fa1897efac14@collabora.com/
> [3] https://lore.kernel.org/linux-media/20231105165521.3592037-1-jonas@kwiboo.se/
> [4] https://github.com/Kwiboo/FFmpeg/commits/v4l2-request-n6.1-dev/
> [5] https://github.com/Kwiboo/linux-rockchip/commits/linuxtv-rkvdec-hevc-v1/
> [6] https://gist.github.com/Kwiboo/4c0ed87774dede44ce6838451a1ec93d
> 
> Regards,
> Jonas
> 
> Alex Bee (5):
>   media: rkvdec: Add variants support
>   media: rkvdec: Add RK3288 variant
>   media: rkvdec: Disable QoS for HEVC and VP9 on RK3328
>   ARM: dts: rockchip: Add vdec node for RK3288
>   arm64: dts: rockchip: Expand reg size of vdec node for RK3399
> 
> Jonas Karlman (4):
>   media: rkvdec: Add HEVC backend
>   media: rkvdec: Implement capability filtering
>   media: dt-bindings: rockchip,vdec: Add RK3288 compatible
>   arm64: dts: rockchip: Expand reg size of vdec node for RK3328
> 
>  .../bindings/media/rockchip,vdec.yaml         |    4 +-
>  arch/arm/boot/dts/rockchip/rk3288.dtsi        |   17 +-
>  arch/arm64/boot/dts/rockchip/rk3328.dtsi      |    2 +-
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi      |    6 +-
>  drivers/staging/media/rkvdec/Makefile         |    2 +-
>  drivers/staging/media/rkvdec/TODO             |    7 -
>  .../staging/media/rkvdec/rkvdec-hevc-data.c   | 1848 +++++++++++++++++
>  drivers/staging/media/rkvdec/rkvdec-hevc.c    |  823 ++++++++
>  drivers/staging/media/rkvdec/rkvdec-regs.h    |    3 +
>  drivers/staging/media/rkvdec/rkvdec-vp9.c     |   10 +
>  drivers/staging/media/rkvdec/rkvdec.c         |  180 +-
>  drivers/staging/media/rkvdec/rkvdec.h         |   15 +
>  12 files changed, 2886 insertions(+), 31 deletions(-)
>  create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc-data.c
>  create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc.c
>
  

Hi Heiko,

On 2023-11-06 10:22, Heiko Stübner wrote:
> Hi Jonas,
> 
> Am Montag, 6. November 2023, 00:36:07 CET schrieb Jonas Karlman:
>> This series add a HEVC backend to the Rockchip Video Decoder driver.
>>
>> A version of this HEVC backend has been in use by the LibreELEC distro
>> for the past 3+ years [1]. It was initially created based on a copy of
>> the H264 backend, unstable HEVC uAPI controls and a cabac table + scaling
>> matrix functions shamelessly copied 1:1 from the Rockchip mpp library.
>>
>> It has since then been extended to use the stable HEVC uAPI controls and
>> improved opon e.g. to include support for rk3288 and fix decoding issues
>> by Alex Bee and Nicolas Dufresne.
>>
>> The version submitted in this series is based on the code currently used
>> by the LibreELEC distro, excluding hard/soft reset, and with cabac table
>> and scaling matrix functions picked from Sebastian Fricke prior series
>> to add a HEVC backend [2].
>>
>> Big thanks to Alex Bee, Nicolas Dufresne and Sebastian Fricke for making
>> this series possible!
>>
>> Patch 1 add the new HEVC backend.
>> Patch 2-3 add variants support to the driver.
>> Patch 4 add support for a rk3288 variant.
>> Patch 5 add a rk3328 variant to work around hw quirks.
>> Patch 6-7 add device tree node for rk3288.
>> Patch 8-9 extend vdec node reg size to include cache/perf registers.
> 
> thanks a lot for working on this.
> 
> Looking at the rkvdec TODO file, isn't the hvec support the only thing
> keeping the driver in staging? So with this support using stable hevc
> uapi, shouldn't the driver then also move out of staging after this
> series is applied?

Yes, I agree, this driver should be moved out of staging now that uapi
is stable. Something for a follow-up series :-)

Regards,
Jonas

> 
> Heiko
> 
> 
>> This was tested on a ROCK Pi 4 (RK3399) and Rock64 (RK3328):
>>
>>   v4l2-compliance 1.24.1, 64 bits, 64-bit time_t
>>   ...
>>   Total for rkvdec device /dev/video1: 46, Succeeded: 46, Failed: 0, Warnings: 0
>>
>>   Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
>>   ...
>>   Ran 135/147 tests successfully
>>
>>   Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
>>   ...
>>   Ran 9/9 tests successfully
>>
>> And on a TinkerBoard (RK3288):
>>
>>   v4l2-compliance 1.24.1, 32 bits, 32-bit time_t
>>   ...
>>   Total for rkvdec device /dev/video3: 47, Succeeded: 47, Failed: 0, Warnings: 0
>>
>>   Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
>>   ...
>>   Ran 137/147 tests successfully
>>
>>   Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
>>   ...
>>   Ran 9/9 tests successfully
>>
>> This series depend on the following series:
>> - media: rkvdec: Add H.264 High 10 and 4:2:2 profile support [3]
>>
>> To fully runtime test this series you need above series and ffmpeg
>> patches from [4], this series and its depends is also available at [5].
>>
>> Full summary of fluster run can be found at [6].
>>
>> [1] https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/patches/linux/default/linux-2000-v4l2-wip-rkvdec-hevc.patch
>> [2] https://lore.kernel.org/linux-media/20230101-patch-series-v2-6-2-rc1-v2-0-fa1897efac14@collabora.com/
>> [3] https://lore.kernel.org/linux-media/20231105165521.3592037-1-jonas@kwiboo.se/
>> [4] https://github.com/Kwiboo/FFmpeg/commits/v4l2-request-n6.1-dev/
>> [5] https://github.com/Kwiboo/linux-rockchip/commits/linuxtv-rkvdec-hevc-v1/
>> [6] https://gist.github.com/Kwiboo/4c0ed87774dede44ce6838451a1ec93d
>>
>> Regards,
>> Jonas
>>
>> Alex Bee (5):
>>   media: rkvdec: Add variants support
>>   media: rkvdec: Add RK3288 variant
>>   media: rkvdec: Disable QoS for HEVC and VP9 on RK3328
>>   ARM: dts: rockchip: Add vdec node for RK3288
>>   arm64: dts: rockchip: Expand reg size of vdec node for RK3399
>>
>> Jonas Karlman (4):
>>   media: rkvdec: Add HEVC backend
>>   media: rkvdec: Implement capability filtering
>>   media: dt-bindings: rockchip,vdec: Add RK3288 compatible
>>   arm64: dts: rockchip: Expand reg size of vdec node for RK3328
>>
>>  .../bindings/media/rockchip,vdec.yaml         |    4 +-
>>  arch/arm/boot/dts/rockchip/rk3288.dtsi        |   17 +-
>>  arch/arm64/boot/dts/rockchip/rk3328.dtsi      |    2 +-
>>  arch/arm64/boot/dts/rockchip/rk3399.dtsi      |    6 +-
>>  drivers/staging/media/rkvdec/Makefile         |    2 +-
>>  drivers/staging/media/rkvdec/TODO             |    7 -
>>  .../staging/media/rkvdec/rkvdec-hevc-data.c   | 1848 +++++++++++++++++
>>  drivers/staging/media/rkvdec/rkvdec-hevc.c    |  823 ++++++++
>>  drivers/staging/media/rkvdec/rkvdec-regs.h    |    3 +
>>  drivers/staging/media/rkvdec/rkvdec-vp9.c     |   10 +
>>  drivers/staging/media/rkvdec/rkvdec.c         |  180 +-
>>  drivers/staging/media/rkvdec/rkvdec.h         |   15 +
>>  12 files changed, 2886 insertions(+), 31 deletions(-)
>>  create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc-data.c
>>  create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc.c
>>
>>
> 
> 
> 
>
  

Hi Hans,

On 2023-11-07 14:49, Hans Verkuil wrote:
> Hi Jonas,
> 
> On 06/11/2023 00:36, Jonas Karlman wrote:
>> This series add a HEVC backend to the Rockchip Video Decoder driver.
>>
>> A version of this HEVC backend has been in use by the LibreELEC distro
>> for the past 3+ years [1]. It was initially created based on a copy of
>> the H264 backend, unstable HEVC uAPI controls and a cabac table + scaling
>> matrix functions shamelessly copied 1:1 from the Rockchip mpp library.
>>
>> It has since then been extended to use the stable HEVC uAPI controls and
>> improved opon e.g. to include support for rk3288 and fix decoding issues
>> by Alex Bee and Nicolas Dufresne.
>>
>> The version submitted in this series is based on the code currently used
>> by the LibreELEC distro, excluding hard/soft reset, and with cabac table
>> and scaling matrix functions picked from Sebastian Fricke prior series
>> to add a HEVC backend [2].
>>
>> Big thanks to Alex Bee, Nicolas Dufresne and Sebastian Fricke for making
>> this series possible!
> 
> I ran this series through smatch and found these two issues:
> 
> drivers/staging/media/rkvdec/rkvdec-hevc.c: In function 'transpose_and_flatten_matrices':
> drivers/staging/media/rkvdec/rkvdec-hevc.c:429:83: warning: variable 'new_value' set but not used [-Wunused-but-set-variable]
>   429 |         int i, j, row, x_offset, matrix_offset, rot_index, y_offset, matrix_size, new_value;
>       |                                                                                   ^~~~~~~~~
> drivers/staging/media/rkvdec/rkvdec-hevc.c:756 rkvdec_hevc_run_preamble() error: we previously assumed 'ctrl' could be null (see line 755)

Thanks, will fix in v2.

> 
> Also, this series drops the HEVC part from the TODO file, but
> I wonder if the last remaining item is still valid:
> 
> * Evaluate introducing a helper to consolidate duplicated
>   code in rkvdec_request_validate and cedrus_request_validate.
>   The helper needs to the driver private data associated with
>   the videobuf2 queue, from a media request.
> 
> It doesn't look like there is much duplicate code at all. It is certainly not
> something that prevents this driver from moving out of staging.

I agree, if this is still valid it is not something that should prevent
this driver from moving out of staging.

There is however one remaining feature/issue that is not listed in TODO.

In certain situations the hw block may need a reset after there has been
a decoding error, especially after a hevc decoding error. Decoding can
typically be re-started with successful result after a short pm
autosuspend timeout.

We do have a soft/hard reset implementation in LibreELEC-distro,
excluded it from this series because reset is typically not needed, and
I think the reset code need some rework to be upstream ready.

Would missing hard reset support be a reason for keeping this driver
in staging?

Regards,
Jonas

> 
> Regards,
> 
> 	Hans
> 
>>
>> Patch 1 add the new HEVC backend.
>> Patch 2-3 add variants support to the driver.
>> Patch 4 add support for a rk3288 variant.
>> Patch 5 add a rk3328 variant to work around hw quirks.
>> Patch 6-7 add device tree node for rk3288.
>> Patch 8-9 extend vdec node reg size to include cache/perf registers.
>>
>> This was tested on a ROCK Pi 4 (RK3399) and Rock64 (RK3328):
>>
>>   v4l2-compliance 1.24.1, 64 bits, 64-bit time_t
>>   ...
>>   Total for rkvdec device /dev/video1: 46, Succeeded: 46, Failed: 0, Warnings: 0
>>
>>   Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
>>   ...
>>   Ran 135/147 tests successfully
>>
>>   Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
>>   ...
>>   Ran 9/9 tests successfully
>>
>> And on a TinkerBoard (RK3288):
>>
>>   v4l2-compliance 1.24.1, 32 bits, 32-bit time_t
>>   ...
>>   Total for rkvdec device /dev/video3: 47, Succeeded: 47, Failed: 0, Warnings: 0
>>
>>   Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
>>   ...
>>   Ran 137/147 tests successfully
>>
>>   Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
>>   ...
>>   Ran 9/9 tests successfully
>>
>> This series depend on the following series:
>> - media: rkvdec: Add H.264 High 10 and 4:2:2 profile support [3]
>>
>> To fully runtime test this series you need above series and ffmpeg
>> patches from [4], this series and its depends is also available at [5].
>>
>> Full summary of fluster run can be found at [6].
>>
>> [1] https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/patches/linux/default/linux-2000-v4l2-wip-rkvdec-hevc.patch
>> [2] https://lore.kernel.org/linux-media/20230101-patch-series-v2-6-2-rc1-v2-0-fa1897efac14@collabora.com/
>> [3] https://lore.kernel.org/linux-media/20231105165521.3592037-1-jonas@kwiboo.se/
>> [4] https://github.com/Kwiboo/FFmpeg/commits/v4l2-request-n6.1-dev/
>> [5] https://github.com/Kwiboo/linux-rockchip/commits/linuxtv-rkvdec-hevc-v1/
>> [6] https://gist.github.com/Kwiboo/4c0ed87774dede44ce6838451a1ec93d
>>
>> Regards,
>> Jonas
>>
>> Alex Bee (5):
>>   media: rkvdec: Add variants support
>>   media: rkvdec: Add RK3288 variant
>>   media: rkvdec: Disable QoS for HEVC and VP9 on RK3328
>>   ARM: dts: rockchip: Add vdec node for RK3288
>>   arm64: dts: rockchip: Expand reg size of vdec node for RK3399
>>
>> Jonas Karlman (4):
>>   media: rkvdec: Add HEVC backend
>>   media: rkvdec: Implement capability filtering
>>   media: dt-bindings: rockchip,vdec: Add RK3288 compatible
>>   arm64: dts: rockchip: Expand reg size of vdec node for RK3328
>>
>>  .../bindings/media/rockchip,vdec.yaml         |    4 +-
>>  arch/arm/boot/dts/rockchip/rk3288.dtsi        |   17 +-
>>  arch/arm64/boot/dts/rockchip/rk3328.dtsi      |    2 +-
>>  arch/arm64/boot/dts/rockchip/rk3399.dtsi      |    6 +-
>>  drivers/staging/media/rkvdec/Makefile         |    2 +-
>>  drivers/staging/media/rkvdec/TODO             |    7 -
>>  .../staging/media/rkvdec/rkvdec-hevc-data.c   | 1848 +++++++++++++++++
>>  drivers/staging/media/rkvdec/rkvdec-hevc.c    |  823 ++++++++
>>  drivers/staging/media/rkvdec/rkvdec-regs.h    |    3 +
>>  drivers/staging/media/rkvdec/rkvdec-vp9.c     |   10 +
>>  drivers/staging/media/rkvdec/rkvdec.c         |  180 +-
>>  drivers/staging/media/rkvdec/rkvdec.h         |   15 +
>>  12 files changed, 2886 insertions(+), 31 deletions(-)
>>  create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc-data.c
>>  create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc.c
>>
>
  

Hi Jonas,

Le dimanche 05 novembre 2023 à 23:36 +0000, Jonas Karlman a écrit :
> This series add a HEVC backend to the Rockchip Video Decoder driver.
> 
> A version of this HEVC backend has been in use by the LibreELEC distro
> for the past 3+ years [1]. It was initially created based on a copy of
> the H264 backend, unstable HEVC uAPI controls and a cabac table + scaling
> matrix functions shamelessly copied 1:1 from the Rockchip mpp library.
> 
> It has since then been extended to use the stable HEVC uAPI controls and
> improved opon e.g. to include support for rk3288 and fix decoding issues
> by Alex Bee and Nicolas Dufresne.
> 
> The version submitted in this series is based on the code currently used
> by the LibreELEC distro, excluding hard/soft reset, and with cabac table
> and scaling matrix functions picked from Sebastian Fricke prior series
> to add a HEVC backend [2].
> 
> Big thanks to Alex Bee, Nicolas Dufresne and Sebastian Fricke for making
> this series possible!
> 
> Patch 1 add the new HEVC backend.
> Patch 2-3 add variants support to the driver.
> Patch 4 add support for a rk3288 variant.
> Patch 5 add a rk3328 variant to work around hw quirks.
> Patch 6-7 add device tree node for rk3288.
> Patch 8-9 extend vdec node reg size to include cache/perf registers.
> 
> This was tested on a ROCK Pi 4 (RK3399) and Rock64 (RK3328):
> 
>   v4l2-compliance 1.24.1, 64 bits, 64-bit time_t
>   ...
>   Total for rkvdec device /dev/video1: 46, Succeeded: 46, Failed: 0, Warnings: 0
> 
>   Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
>   ...
>   Ran 135/147 tests successfully

Just tested, same score on GStreamer. Failing are:

    - DBLK_D_VIXS_2
    - DSLICE_A_HHI_5
    - EXT_A_ericsson_4
    - PICSIZE* (all 4, resolution reason)
    - SAODBLK_A_MainConcept_4
    - SAODBLK_B_MainConcept_4
    - TSUNEQBD_A_MAIN10_Technicolor_2
    - WPP_D_ericsson_MAIN_2

To be noted that TSUNEQBD_A_MAIN10_Technicolor_2 has 10bit luma, and
9bit chroma. Works on Hantro G2, but I just tried waving your check, 
and the results did not match the MD5. This is of low important though,
I have never seen such a stream in the wild and MTK VCODEC also does
not support it.

> 
>   Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
>   ...
>   Ran 9/9 tests successfully

Fails with GStreamer, but most likely a GStreamer issue, I'll have a
look later. JCT-VC-RExt and JCT-VC-SCC also fails. I know of a fact
that we did not implement SCC in the uAPI (and it may not be supported
by this hardware anyway).

Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> 
> And on a TinkerBoard (RK3288):
> 
>   v4l2-compliance 1.24.1, 32 bits, 32-bit time_t
>   ...
>   Total for rkvdec device /dev/video3: 47, Succeeded: 47, Failed: 0, Warnings: 0
> 
>   Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
>   ...
>   Ran 137/147 tests successfully
> 
>   Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
>   ...
>   Ran 9/9 tests successfully
> 
> This series depend on the following series:
> - media: rkvdec: Add H.264 High 10 and 4:2:2 profile support [3]
> 
> To fully runtime test this series you need above series and ffmpeg
> patches from [4], this series and its depends is also available at [5].
> 
> Full summary of fluster run can be found at [6].
> 
> [1] https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/patches/linux/default/linux-2000-v4l2-wip-rkvdec-hevc.patch
> [2] https://lore.kernel.org/linux-media/20230101-patch-series-v2-6-2-rc1-v2-0-fa1897efac14@collabora.com/
> [3] https://lore.kernel.org/linux-media/20231105165521.3592037-1-jonas@kwiboo.se/
> [4] https://github.com/Kwiboo/FFmpeg/commits/v4l2-request-n6.1-dev/
> [5] https://github.com/Kwiboo/linux-rockchip/commits/linuxtv-rkvdec-hevc-v1/
> [6] https://gist.github.com/Kwiboo/4c0ed87774dede44ce6838451a1ec93d
> 
> Regards,
> Jonas
> 
> Alex Bee (5):
>   media: rkvdec: Add variants support
>   media: rkvdec: Add RK3288 variant
>   media: rkvdec: Disable QoS for HEVC and VP9 on RK3328
>   ARM: dts: rockchip: Add vdec node for RK3288
>   arm64: dts: rockchip: Expand reg size of vdec node for RK3399
> 
> Jonas Karlman (4):
>   media: rkvdec: Add HEVC backend
>   media: rkvdec: Implement capability filtering
>   media: dt-bindings: rockchip,vdec: Add RK3288 compatible
>   arm64: dts: rockchip: Expand reg size of vdec node for RK3328
> 
>  .../bindings/media/rockchip,vdec.yaml         |    4 +-
>  arch/arm/boot/dts/rockchip/rk3288.dtsi        |   17 +-
>  arch/arm64/boot/dts/rockchip/rk3328.dtsi      |    2 +-
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi      |    6 +-
>  drivers/staging/media/rkvdec/Makefile         |    2 +-
>  drivers/staging/media/rkvdec/TODO             |    7 -
>  .../staging/media/rkvdec/rkvdec-hevc-data.c   | 1848 +++++++++++++++++
>  drivers/staging/media/rkvdec/rkvdec-hevc.c    |  823 ++++++++
>  drivers/staging/media/rkvdec/rkvdec-regs.h    |    3 +
>  drivers/staging/media/rkvdec/rkvdec-vp9.c     |   10 +
>  drivers/staging/media/rkvdec/rkvdec.c         |  180 +-
>  drivers/staging/media/rkvdec/rkvdec.h         |   15 +
>  12 files changed, 2886 insertions(+), 31 deletions(-)
>  create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc-data.c
>  create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc.c
>
  

On 09/11/2023 18:44, Jonas Karlman wrote:
> Hi Hans,
> 
> On 2023-11-07 14:49, Hans Verkuil wrote:
>> Hi Jonas,
>>
>> On 06/11/2023 00:36, Jonas Karlman wrote:
>>> This series add a HEVC backend to the Rockchip Video Decoder driver.
>>>
>>> A version of this HEVC backend has been in use by the LibreELEC distro
>>> for the past 3+ years [1]. It was initially created based on a copy of
>>> the H264 backend, unstable HEVC uAPI controls and a cabac table + scaling
>>> matrix functions shamelessly copied 1:1 from the Rockchip mpp library.
>>>
>>> It has since then been extended to use the stable HEVC uAPI controls and
>>> improved opon e.g. to include support for rk3288 and fix decoding issues
>>> by Alex Bee and Nicolas Dufresne.
>>>
>>> The version submitted in this series is based on the code currently used
>>> by the LibreELEC distro, excluding hard/soft reset, and with cabac table
>>> and scaling matrix functions picked from Sebastian Fricke prior series
>>> to add a HEVC backend [2].
>>>
>>> Big thanks to Alex Bee, Nicolas Dufresne and Sebastian Fricke for making
>>> this series possible!
>>
>> I ran this series through smatch and found these two issues:
>>
>> drivers/staging/media/rkvdec/rkvdec-hevc.c: In function 'transpose_and_flatten_matrices':
>> drivers/staging/media/rkvdec/rkvdec-hevc.c:429:83: warning: variable 'new_value' set but not used [-Wunused-but-set-variable]
>>   429 |         int i, j, row, x_offset, matrix_offset, rot_index, y_offset, matrix_size, new_value;
>>       |                                                                                   ^~~~~~~~~
>> drivers/staging/media/rkvdec/rkvdec-hevc.c:756 rkvdec_hevc_run_preamble() error: we previously assumed 'ctrl' could be null (see line 755)
> 
> Thanks, will fix in v2.
> 
>>
>> Also, this series drops the HEVC part from the TODO file, but
>> I wonder if the last remaining item is still valid:
>>
>> * Evaluate introducing a helper to consolidate duplicated
>>   code in rkvdec_request_validate and cedrus_request_validate.
>>   The helper needs to the driver private data associated with
>>   the videobuf2 queue, from a media request.
>>
>> It doesn't look like there is much duplicate code at all. It is certainly not
>> something that prevents this driver from moving out of staging.
> 
> I agree, if this is still valid it is not something that should prevent
> this driver from moving out of staging.
> 
> There is however one remaining feature/issue that is not listed in TODO.
> 
> In certain situations the hw block may need a reset after there has been
> a decoding error, especially after a hevc decoding error. Decoding can
> typically be re-started with successful result after a short pm
> autosuspend timeout.
> 
> We do have a soft/hard reset implementation in LibreELEC-distro,
> excluded it from this series because reset is typically not needed, and
> I think the reset code need some rework to be upstream ready.
> 
> Would missing hard reset support be a reason for keeping this driver
> in staging?

I think Nicolas would be better placed to comment on that. If it is not
considered a blocker, shouldn't there at least be a patch adding comments
at the relevant place describing this issue? That way it is at least
documented as a known issue.

Regards,

	Hans

> 
> Regards,
> Jonas
> 
>>
>> Regards,
>>
>> 	Hans
>>
>>>
>>> Patch 1 add the new HEVC backend.
>>> Patch 2-3 add variants support to the driver.
>>> Patch 4 add support for a rk3288 variant.
>>> Patch 5 add a rk3328 variant to work around hw quirks.
>>> Patch 6-7 add device tree node for rk3288.
>>> Patch 8-9 extend vdec node reg size to include cache/perf registers.
>>>
>>> This was tested on a ROCK Pi 4 (RK3399) and Rock64 (RK3328):
>>>
>>>   v4l2-compliance 1.24.1, 64 bits, 64-bit time_t
>>>   ...
>>>   Total for rkvdec device /dev/video1: 46, Succeeded: 46, Failed: 0, Warnings: 0
>>>
>>>   Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
>>>   ...
>>>   Ran 135/147 tests successfully
>>>
>>>   Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
>>>   ...
>>>   Ran 9/9 tests successfully
>>>
>>> And on a TinkerBoard (RK3288):
>>>
>>>   v4l2-compliance 1.24.1, 32 bits, 32-bit time_t
>>>   ...
>>>   Total for rkvdec device /dev/video3: 47, Succeeded: 47, Failed: 0, Warnings: 0
>>>
>>>   Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
>>>   ...
>>>   Ran 137/147 tests successfully
>>>
>>>   Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
>>>   ...
>>>   Ran 9/9 tests successfully
>>>
>>> This series depend on the following series:
>>> - media: rkvdec: Add H.264 High 10 and 4:2:2 profile support [3]
>>>
>>> To fully runtime test this series you need above series and ffmpeg
>>> patches from [4], this series and its depends is also available at [5].
>>>
>>> Full summary of fluster run can be found at [6].
>>>
>>> [1] https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/patches/linux/default/linux-2000-v4l2-wip-rkvdec-hevc.patch
>>> [2] https://lore.kernel.org/linux-media/20230101-patch-series-v2-6-2-rc1-v2-0-fa1897efac14@collabora.com/
>>> [3] https://lore.kernel.org/linux-media/20231105165521.3592037-1-jonas@kwiboo.se/
>>> [4] https://github.com/Kwiboo/FFmpeg/commits/v4l2-request-n6.1-dev/
>>> [5] https://github.com/Kwiboo/linux-rockchip/commits/linuxtv-rkvdec-hevc-v1/
>>> [6] https://gist.github.com/Kwiboo/4c0ed87774dede44ce6838451a1ec93d
>>>
>>> Regards,
>>> Jonas
>>>
>>> Alex Bee (5):
>>>   media: rkvdec: Add variants support
>>>   media: rkvdec: Add RK3288 variant
>>>   media: rkvdec: Disable QoS for HEVC and VP9 on RK3328
>>>   ARM: dts: rockchip: Add vdec node for RK3288
>>>   arm64: dts: rockchip: Expand reg size of vdec node for RK3399
>>>
>>> Jonas Karlman (4):
>>>   media: rkvdec: Add HEVC backend
>>>   media: rkvdec: Implement capability filtering
>>>   media: dt-bindings: rockchip,vdec: Add RK3288 compatible
>>>   arm64: dts: rockchip: Expand reg size of vdec node for RK3328
>>>
>>>  .../bindings/media/rockchip,vdec.yaml         |    4 +-
>>>  arch/arm/boot/dts/rockchip/rk3288.dtsi        |   17 +-
>>>  arch/arm64/boot/dts/rockchip/rk3328.dtsi      |    2 +-
>>>  arch/arm64/boot/dts/rockchip/rk3399.dtsi      |    6 +-
>>>  drivers/staging/media/rkvdec/Makefile         |    2 +-
>>>  drivers/staging/media/rkvdec/TODO             |    7 -
>>>  .../staging/media/rkvdec/rkvdec-hevc-data.c   | 1848 +++++++++++++++++
>>>  drivers/staging/media/rkvdec/rkvdec-hevc.c    |  823 ++++++++
>>>  drivers/staging/media/rkvdec/rkvdec-regs.h    |    3 +
>>>  drivers/staging/media/rkvdec/rkvdec-vp9.c     |   10 +
>>>  drivers/staging/media/rkvdec/rkvdec.c         |  180 +-
>>>  drivers/staging/media/rkvdec/rkvdec.h         |   15 +
>>>  12 files changed, 2886 insertions(+), 31 deletions(-)
>>>  create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc-data.c
>>>  create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc.c
>>>
>>
>
  

On Wed, Nov 22, 2023 at 11:23 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> On 09/11/2023 18:44, Jonas Karlman wrote:
> > Hi Hans,
> >
> > On 2023-11-07 14:49, Hans Verkuil wrote:
> >> Hi Jonas,
> >>
> >> On 06/11/2023 00:36, Jonas Karlman wrote:
> >>> This series add a HEVC backend to the Rockchip Video Decoder driver.
> >>>
> >>> A version of this HEVC backend has been in use by the LibreELEC distro
> >>> for the past 3+ years [1]. It was initially created based on a copy of
> >>> the H264 backend, unstable HEVC uAPI controls and a cabac table + scaling
> >>> matrix functions shamelessly copied 1:1 from the Rockchip mpp library.
> >>>
> >>> It has since then been extended to use the stable HEVC uAPI controls and
> >>> improved opon e.g. to include support for rk3288 and fix decoding issues
> >>> by Alex Bee and Nicolas Dufresne.
> >>>
> >>> The version submitted in this series is based on the code currently used
> >>> by the LibreELEC distro, excluding hard/soft reset, and with cabac table
> >>> and scaling matrix functions picked from Sebastian Fricke prior series
> >>> to add a HEVC backend [2].
> >>>
> >>> Big thanks to Alex Bee, Nicolas Dufresne and Sebastian Fricke for making
> >>> this series possible!
> >>
> >> I ran this series through smatch and found these two issues:
> >>
> >> drivers/staging/media/rkvdec/rkvdec-hevc.c: In function 'transpose_and_flatten_matrices':
> >> drivers/staging/media/rkvdec/rkvdec-hevc.c:429:83: warning: variable 'new_value' set but not used [-Wunused-but-set-variable]
> >>   429 |         int i, j, row, x_offset, matrix_offset, rot_index, y_offset, matrix_size, new_value;
> >>       |                                                                                   ^~~~~~~~~
> >> drivers/staging/media/rkvdec/rkvdec-hevc.c:756 rkvdec_hevc_run_preamble() error: we previously assumed 'ctrl' could be null (see line 755)
> >
> > Thanks, will fix in v2.
> >
> >>
> >> Also, this series drops the HEVC part from the TODO file, but
> >> I wonder if the last remaining item is still valid:
> >>
> >> * Evaluate introducing a helper to consolidate duplicated
> >>   code in rkvdec_request_validate and cedrus_request_validate.
> >>   The helper needs to the driver private data associated with
> >>   the videobuf2 queue, from a media request.
> >>
> >> It doesn't look like there is much duplicate code at all. It is certainly not
> >> something that prevents this driver from moving out of staging.
> >
> > I agree, if this is still valid it is not something that should prevent
> > this driver from moving out of staging.
> >
> > There is however one remaining feature/issue that is not listed in TODO.
> >
> > In certain situations the hw block may need a reset after there has been
> > a decoding error, especially after a hevc decoding error. Decoding can
> > typically be re-started with successful result after a short pm
> > autosuspend timeout.
> >
> > We do have a soft/hard reset implementation in LibreELEC-distro,
> > excluded it from this series because reset is typically not needed, and
> > I think the reset code need some rework to be upstream ready.
> >
> > Would missing hard reset support be a reason for keeping this driver
> > in staging?
>
> I think Nicolas would be better placed to comment on that. If it is not
> considered a blocker, shouldn't there at least be a patch adding comments
> at the relevant place describing this issue? That way it is at least
> documented as a known issue.

FWIW Hantro on i.MX8MM doesn't either.

ChenYu

> Regards,
>
>         Hans
>
> >
> > Regards,
> > Jonas
> >
> >>
> >> Regards,
> >>
> >>      Hans
> >>
> >>>
> >>> Patch 1 add the new HEVC backend.
> >>> Patch 2-3 add variants support to the driver.
> >>> Patch 4 add support for a rk3288 variant.
> >>> Patch 5 add a rk3328 variant to work around hw quirks.
> >>> Patch 6-7 add device tree node for rk3288.
> >>> Patch 8-9 extend vdec node reg size to include cache/perf registers.
> >>>
> >>> This was tested on a ROCK Pi 4 (RK3399) and Rock64 (RK3328):
> >>>
> >>>   v4l2-compliance 1.24.1, 64 bits, 64-bit time_t
> >>>   ...
> >>>   Total for rkvdec device /dev/video1: 46, Succeeded: 46, Failed: 0, Warnings: 0
> >>>
> >>>   Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
> >>>   ...
> >>>   Ran 135/147 tests successfully
> >>>
> >>>   Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
> >>>   ...
> >>>   Ran 9/9 tests successfully
> >>>
> >>> And on a TinkerBoard (RK3288):
> >>>
> >>>   v4l2-compliance 1.24.1, 32 bits, 32-bit time_t
> >>>   ...
> >>>   Total for rkvdec device /dev/video3: 47, Succeeded: 47, Failed: 0, Warnings: 0
> >>>
> >>>   Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
> >>>   ...
> >>>   Ran 137/147 tests successfully
> >>>
> >>>   Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
> >>>   ...
> >>>   Ran 9/9 tests successfully
> >>>
> >>> This series depend on the following series:
> >>> - media: rkvdec: Add H.264 High 10 and 4:2:2 profile support [3]
> >>>
> >>> To fully runtime test this series you need above series and ffmpeg
> >>> patches from [4], this series and its depends is also available at [5].
> >>>
> >>> Full summary of fluster run can be found at [6].
> >>>
> >>> [1] https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/patches/linux/default/linux-2000-v4l2-wip-rkvdec-hevc.patch
> >>> [2] https://lore.kernel.org/linux-media/20230101-patch-series-v2-6-2-rc1-v2-0-fa1897efac14@collabora.com/
> >>> [3] https://lore.kernel.org/linux-media/20231105165521.3592037-1-jonas@kwiboo.se/
> >>> [4] https://github.com/Kwiboo/FFmpeg/commits/v4l2-request-n6.1-dev/
> >>> [5] https://github.com/Kwiboo/linux-rockchip/commits/linuxtv-rkvdec-hevc-v1/
> >>> [6] https://gist.github.com/Kwiboo/4c0ed87774dede44ce6838451a1ec93d
> >>>
> >>> Regards,
> >>> Jonas
> >>>
> >>> Alex Bee (5):
> >>>   media: rkvdec: Add variants support
> >>>   media: rkvdec: Add RK3288 variant
> >>>   media: rkvdec: Disable QoS for HEVC and VP9 on RK3328
> >>>   ARM: dts: rockchip: Add vdec node for RK3288
> >>>   arm64: dts: rockchip: Expand reg size of vdec node for RK3399
> >>>
> >>> Jonas Karlman (4):
> >>>   media: rkvdec: Add HEVC backend
> >>>   media: rkvdec: Implement capability filtering
> >>>   media: dt-bindings: rockchip,vdec: Add RK3288 compatible
> >>>   arm64: dts: rockchip: Expand reg size of vdec node for RK3328
> >>>
> >>>  .../bindings/media/rockchip,vdec.yaml         |    4 +-
> >>>  arch/arm/boot/dts/rockchip/rk3288.dtsi        |   17 +-
> >>>  arch/arm64/boot/dts/rockchip/rk3328.dtsi      |    2 +-
> >>>  arch/arm64/boot/dts/rockchip/rk3399.dtsi      |    6 +-
> >>>  drivers/staging/media/rkvdec/Makefile         |    2 +-
> >>>  drivers/staging/media/rkvdec/TODO             |    7 -
> >>>  .../staging/media/rkvdec/rkvdec-hevc-data.c   | 1848 +++++++++++++++++
> >>>  drivers/staging/media/rkvdec/rkvdec-hevc.c    |  823 ++++++++
> >>>  drivers/staging/media/rkvdec/rkvdec-regs.h    |    3 +
> >>>  drivers/staging/media/rkvdec/rkvdec-vp9.c     |   10 +
> >>>  drivers/staging/media/rkvdec/rkvdec.c         |  180 +-
> >>>  drivers/staging/media/rkvdec/rkvdec.h         |   15 +
> >>>  12 files changed, 2886 insertions(+), 31 deletions(-)
> >>>  create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc-data.c
> >>>  create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc.c
> >>>
> >>
> >
>
>
  

Le mercredi 22 novembre 2023 à 16:23 +0100, Hans Verkuil a écrit :
> On 09/11/2023 18:44, Jonas Karlman wrote:
> > Hi Hans,
> > 
> > On 2023-11-07 14:49, Hans Verkuil wrote:
> > > Hi Jonas,
> > > 
> > > On 06/11/2023 00:36, Jonas Karlman wrote:
> > > > This series add a HEVC backend to the Rockchip Video Decoder driver.
> > > > 
> > > > A version of this HEVC backend has been in use by the LibreELEC distro
> > > > for the past 3+ years [1]. It was initially created based on a copy of
> > > > the H264 backend, unstable HEVC uAPI controls and a cabac table + scaling
> > > > matrix functions shamelessly copied 1:1 from the Rockchip mpp library.
> > > > 
> > > > It has since then been extended to use the stable HEVC uAPI controls and
> > > > improved opon e.g. to include support for rk3288 and fix decoding issues
> > > > by Alex Bee and Nicolas Dufresne.
> > > > 
> > > > The version submitted in this series is based on the code currently used
> > > > by the LibreELEC distro, excluding hard/soft reset, and with cabac table
> > > > and scaling matrix functions picked from Sebastian Fricke prior series
> > > > to add a HEVC backend [2].
> > > > 
> > > > Big thanks to Alex Bee, Nicolas Dufresne and Sebastian Fricke for making
> > > > this series possible!
> > > 
> > > I ran this series through smatch and found these two issues:
> > > 
> > > drivers/staging/media/rkvdec/rkvdec-hevc.c: In function 'transpose_and_flatten_matrices':
> > > drivers/staging/media/rkvdec/rkvdec-hevc.c:429:83: warning: variable 'new_value' set but not used [-Wunused-but-set-variable]
> > >   429 |         int i, j, row, x_offset, matrix_offset, rot_index, y_offset, matrix_size, new_value;
> > >       |                                                                                   ^~~~~~~~~
> > > drivers/staging/media/rkvdec/rkvdec-hevc.c:756 rkvdec_hevc_run_preamble() error: we previously assumed 'ctrl' could be null (see line 755)
> > 
> > Thanks, will fix in v2.
> > 
> > > 
> > > Also, this series drops the HEVC part from the TODO file, but
> > > I wonder if the last remaining item is still valid:
> > > 
> > > * Evaluate introducing a helper to consolidate duplicated
> > >   code in rkvdec_request_validate and cedrus_request_validate.
> > >   The helper needs to the driver private data associated with
> > >   the videobuf2 queue, from a media request.
> > > 
> > > It doesn't look like there is much duplicate code at all. It is certainly not
> > > something that prevents this driver from moving out of staging.
> > 
> > I agree, if this is still valid it is not something that should prevent
> > this driver from moving out of staging.
> > 
> > There is however one remaining feature/issue that is not listed in TODO.
> > 
> > In certain situations the hw block may need a reset after there has been
> > a decoding error, especially after a hevc decoding error. Decoding can
> > typically be re-started with successful result after a short pm
> > autosuspend timeout.
> > 
> > We do have a soft/hard reset implementation in LibreELEC-distro,
> > excluded it from this series because reset is typically not needed, and
> > I think the reset code need some rework to be upstream ready.
> > 
> > Would missing hard reset support be a reason for keeping this driver
> > in staging?
> 
> I think Nicolas would be better placed to comment on that. If it is not
> considered a blocker, shouldn't there at least be a patch adding comments
> at the relevant place describing this issue? That way it is at least
> documented as a known issue.

I *think* that Sebastian have identified how to detect when the self reset is
not sufficient (and I guess the same may exist in LibreELEC tree). As the
failure is not fatal, and it will recover if you let the codec idle for a
little, I don't have any argument to make this a hard blocker.

But before we unstated with this known issue, I'd like to see this well
documented, and ideally see a kernel warning whenever that state is reached.
From there, it will be easy to justify a more invasive change to reset the
entire block. It may also help identify when getting user reports of issues.

Nicolas

> 
> Regards,
> 
> 	Hans
> 
> > 
> > Regards,
> > Jonas
> > 
> > > 
> > > Regards,
> > > 
> > > 	Hans
> > > 
> > > > 
> > > > Patch 1 add the new HEVC backend.
> > > > Patch 2-3 add variants support to the driver.
> > > > Patch 4 add support for a rk3288 variant.
> > > > Patch 5 add a rk3328 variant to work around hw quirks.
> > > > Patch 6-7 add device tree node for rk3288.
> > > > Patch 8-9 extend vdec node reg size to include cache/perf registers.
> > > > 
> > > > This was tested on a ROCK Pi 4 (RK3399) and Rock64 (RK3328):
> > > > 
> > > >   v4l2-compliance 1.24.1, 64 bits, 64-bit time_t
> > > >   ...
> > > >   Total for rkvdec device /dev/video1: 46, Succeeded: 46, Failed: 0, Warnings: 0
> > > > 
> > > >   Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
> > > >   ...
> > > >   Ran 135/147 tests successfully
> > > > 
> > > >   Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
> > > >   ...
> > > >   Ran 9/9 tests successfully
> > > > 
> > > > And on a TinkerBoard (RK3288):
> > > > 
> > > >   v4l2-compliance 1.24.1, 32 bits, 32-bit time_t
> > > >   ...
> > > >   Total for rkvdec device /dev/video3: 47, Succeeded: 47, Failed: 0, Warnings: 0
> > > > 
> > > >   Running test suite JCT-VC-HEVC_V1 with decoder FFmpeg-H.265-V4L2-request
> > > >   ...
> > > >   Ran 137/147 tests successfully
> > > > 
> > > >   Running test suite JCT-VC-MV-HEVC with decoder FFmpeg-H.265-V4L2-request
> > > >   ...
> > > >   Ran 9/9 tests successfully
> > > > 
> > > > This series depend on the following series:
> > > > - media: rkvdec: Add H.264 High 10 and 4:2:2 profile support [3]
> > > > 
> > > > To fully runtime test this series you need above series and ffmpeg
> > > > patches from [4], this series and its depends is also available at [5].
> > > > 
> > > > Full summary of fluster run can be found at [6].
> > > > 
> > > > [1] https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/patches/linux/default/linux-2000-v4l2-wip-rkvdec-hevc.patch
> > > > [2] https://lore.kernel.org/linux-media/20230101-patch-series-v2-6-2-rc1-v2-0-fa1897efac14@collabora.com/
> > > > [3] https://lore.kernel.org/linux-media/20231105165521.3592037-1-jonas@kwiboo.se/
> > > > [4] https://github.com/Kwiboo/FFmpeg/commits/v4l2-request-n6.1-dev/
> > > > [5] https://github.com/Kwiboo/linux-rockchip/commits/linuxtv-rkvdec-hevc-v1/
> > > > [6] https://gist.github.com/Kwiboo/4c0ed87774dede44ce6838451a1ec93d
> > > > 
> > > > Regards,
> > > > Jonas
> > > > 
> > > > Alex Bee (5):
> > > >   media: rkvdec: Add variants support
> > > >   media: rkvdec: Add RK3288 variant
> > > >   media: rkvdec: Disable QoS for HEVC and VP9 on RK3328
> > > >   ARM: dts: rockchip: Add vdec node for RK3288
> > > >   arm64: dts: rockchip: Expand reg size of vdec node for RK3399
> > > > 
> > > > Jonas Karlman (4):
> > > >   media: rkvdec: Add HEVC backend
> > > >   media: rkvdec: Implement capability filtering
> > > >   media: dt-bindings: rockchip,vdec: Add RK3288 compatible
> > > >   arm64: dts: rockchip: Expand reg size of vdec node for RK3328
> > > > 
> > > >  .../bindings/media/rockchip,vdec.yaml         |    4 +-
> > > >  arch/arm/boot/dts/rockchip/rk3288.dtsi        |   17 +-
> > > >  arch/arm64/boot/dts/rockchip/rk3328.dtsi      |    2 +-
> > > >  arch/arm64/boot/dts/rockchip/rk3399.dtsi      |    6 +-
> > > >  drivers/staging/media/rkvdec/Makefile         |    2 +-
> > > >  drivers/staging/media/rkvdec/TODO             |    7 -
> > > >  .../staging/media/rkvdec/rkvdec-hevc-data.c   | 1848 +++++++++++++++++
> > > >  drivers/staging/media/rkvdec/rkvdec-hevc.c    |  823 ++++++++
> > > >  drivers/staging/media/rkvdec/rkvdec-regs.h    |    3 +
> > > >  drivers/staging/media/rkvdec/rkvdec-vp9.c     |   10 +
> > > >  drivers/staging/media/rkvdec/rkvdec.c         |  180 +-
> > > >  drivers/staging/media/rkvdec/rkvdec.h         |   15 +
> > > >  12 files changed, 2886 insertions(+), 31 deletions(-)
> > > >  create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc-data.c
> > > >  create mode 100644 drivers/staging/media/rkvdec/rkvdec-hevc.c
> > > > 
> > > 
> > 
> 
>