mbox series

[v12,0/9] Add HANTRO G2/HEVC decoder support for IMX8MQ

Message ID 20210526124516.2039434-1-benjamin.gaignard@collabora.com (mailing list archive)
Headers show
Series Add HANTRO G2/HEVC decoder support for IMX8MQ | expand

Message

Benjamin Gaignard May 26, 2021, 12:45 p.m. UTC
The IMX8MQ got two VPUs but until now only G1 has been enabled.
This series aim to add the second VPU (aka G2) and provide basic 
HEVC decoding support.

To be able to decode HEVC it is needed to add/update some of the
structures in the uapi. In addition of them one HANTRO dedicated
control is required to inform the driver of the number of bits to skip
at the beginning of the slice header.
The hardware require to allocate few auxiliary buffers to store the
references frame or tile size data.

The driver has been tested with fluster test suite stream.
For example with this command: ./fluster.py run -ts JCT-VC-HEVC_V1 -d GStreamer-H.265-V4L2SL-Gst1.0

version 12:
 - Change macro to avoid the final ';'
 - Made arrays static and const
 - Distinguish G2 generic fields (i.e these who could be reused for VP9)
   from HEVC dedicated fields.

version 11:
 - Rebased on media_stage.
 - Fix minor typo/remarks.

version 10:
 - Shorter version of the previous series without ctrl block patches
   and no DT modifications.
   The scope of this series is limited to HEVC support.

version 9:
 - Corrections in commits messages.
 - Define the dedicated control in hevc-controls.h
 - Add note in documentation.
 - Change max value of the dedicated control.
 - Rebased on media_tree/master branch.

version 8:
 - Add reviewed-by and ack-by tags 
 - Fix the warnings reported by kernel test robot
 - Only patch 9 (adding dedicated control), patch 11 (HEVC support) and
   patch 13 (DT changes) are still missing of review/ack tag.

version 7:
 - Remove 'q' from syscon phandle name to make usable for iMX8MM too.
   Update the bindings documentation.
 - Add review/ack tags.
 - Rebase on top of media_tree/master
 - Be more accurate when computing the size of the memory needed motion
   vectors.
 - Explain why the all clocks need to set in the both DT node.

version 6:
 - fix the errors reported by kernel test robot

version 5:
 - use syscon instead of VPU reset driver.
 - Do not break kernel/DT backward compatibility.
 - Add documentation for dedicated Hantro control.
 - Fix the remarks done by Ezequeil (typo, comments, unused function)
 - Run v4l2-compliance without errors (see below).
 - Do not add field to distinguish version, check postproc reg instead

version 4:
- Split the changes in hevc controls in 2 commits to make them easier to
  review.
- Change hantro_codec_ops run() prototype to return errors   
- Hantro v4l2 dedicated control is now only an integer
- rebase on top of VPU reset changes posted here:
  https://www.spinics.net/lists/arm-kernel/msg878440.html
- Various fix from previous remarks
- Limit the modifications in API to what the driver needs

version 3:
- Fix typo in Hantro v4l2 dedicated control
- Add documentation for the new structures and fields
- Rebased on top of media_tree for-linus-5.12-rc1 tag

version 2:
- remove all change related to scaling
- squash commits to a coherent split
- be more verbose about the added fields
- fix the comments done by Ezequiel about dma_alloc_coherent usage
- fix Dan's comments about control copy, reverse the test logic
in tile_buffer_reallocate, rework some goto and return cases.
- be more verbose about why I change the bindings
- remove all sign-off expect mime since it is confusing
- remove useless clocks in VPUs nodes

Benjamin Gaignard (9):
  media: hevc: Add fields and flags for hevc PPS
  media: hevc: Add decode params control
  media: hantro: change hantro_codec_ops run prototype to return errors
  media: hantro: Define HEVC codec profiles and supported features
  media: hantro: Only use postproc when post processed formats are
    defined
  media: uapi: Add a control for HANTRO driver
  media: hantro: handle V4L2_PIX_FMT_HEVC_SLICE control
  media: hantro: Introduce G2/HEVC decoder
  media: hantro: IMX8M: add variant for G2/HEVC codec

 .../userspace-api/media/drivers/hantro.rst    |  19 +
 .../userspace-api/media/drivers/index.rst     |   1 +
 .../media/v4l/ext-ctrls-codec.rst             | 108 +++-
 .../media/v4l/vidioc-queryctrl.rst            |   6 +
 drivers/media/v4l2-core/v4l2-ctrls-core.c     |  21 +-
 drivers/media/v4l2-core/v4l2-ctrls-defs.c     |   4 +
 drivers/staging/media/hantro/Makefile         |   2 +
 drivers/staging/media/hantro/hantro.h         |  13 +-
 drivers/staging/media/hantro/hantro_drv.c     |  99 ++-
 .../staging/media/hantro/hantro_g1_h264_dec.c |  10 +-
 .../media/hantro/hantro_g1_mpeg2_dec.c        |   4 +-
 .../staging/media/hantro/hantro_g1_vp8_dec.c  |   6 +-
 .../staging/media/hantro/hantro_g2_hevc_dec.c | 587 ++++++++++++++++++
 drivers/staging/media/hantro/hantro_g2_regs.h | 206 ++++++
 .../staging/media/hantro/hantro_h1_jpeg_enc.c |   4 +-
 drivers/staging/media/hantro/hantro_hevc.c    | 327 ++++++++++
 drivers/staging/media/hantro/hantro_hw.h      |  69 +-
 .../staging/media/hantro/hantro_postproc.c    |  14 +
 drivers/staging/media/hantro/hantro_v4l2.c    |   5 +-
 drivers/staging/media/hantro/imx8m_vpu_hw.c   |  96 ++-
 .../media/hantro/rk3399_vpu_hw_jpeg_enc.c     |   4 +-
 .../media/hantro/rk3399_vpu_hw_mpeg2_dec.c    |   4 +-
 .../media/hantro/rk3399_vpu_hw_vp8_dec.c      |   6 +-
 drivers/staging/media/sunxi/cedrus/cedrus.c   |   6 +
 drivers/staging/media/sunxi/cedrus/cedrus.h   |   1 +
 .../staging/media/sunxi/cedrus/cedrus_dec.c   |   2 +
 .../staging/media/sunxi/cedrus/cedrus_h265.c  |  12 +-
 include/media/hevc-ctrls.h                    |  46 +-
 28 files changed, 1613 insertions(+), 69 deletions(-)
 create mode 100644 Documentation/userspace-api/media/drivers/hantro.rst
 create mode 100644 drivers/staging/media/hantro/hantro_g2_hevc_dec.c
 create mode 100644 drivers/staging/media/hantro/hantro_g2_regs.h
 create mode 100644 drivers/staging/media/hantro/hantro_hevc.c

Comments

Benjamin Gaignard May 27, 2021, 1:36 p.m. UTC | #1
Le 26/05/2021 à 14:45, Benjamin Gaignard a écrit :
> The IMX8MQ got two VPUs but until now only G1 has been enabled.
> This series aim to add the second VPU (aka G2) and provide basic
> HEVC decoding support.
>
> To be able to decode HEVC it is needed to add/update some of the
> structures in the uapi. In addition of them one HANTRO dedicated
> control is required to inform the driver of the number of bits to skip
> at the beginning of the slice header.
> The hardware require to allocate few auxiliary buffers to store the
> references frame or tile size data.
>
> The driver has been tested with fluster test suite stream.
> For example with this command: ./fluster.py run -ts JCT-VC-HEVC_V1 -d GStreamer-H.265-V4L2SL-Gst1.0
>
> version 12:
>   - Change macro to avoid the final ';'
>   - Made arrays static and const
>   - Distinguish G2 generic fields (i.e these who could be reused for VP9)
>     from HEVC dedicated fields.

Hi Hans,

Could you merge this series ?
The documentation for the compatible string will come when we will reach a conclusion
about the power domain like describe in this thread:
https://lore.kernel.org/linux-media/20210407073534.376722-1-benjamin.gaignard@collabora.com/T/#m6bd571fee94b1b1fa5c5d3fa6315ae5c9f4c8fad

Regards,
Benjamin

> version 11:
>   - Rebased on media_stage.
>   - Fix minor typo/remarks.
>
> version 10:
>   - Shorter version of the previous series without ctrl block patches
>     and no DT modifications.
>     The scope of this series is limited to HEVC support.
>
> version 9:
>   - Corrections in commits messages.
>   - Define the dedicated control in hevc-controls.h
>   - Add note in documentation.
>   - Change max value of the dedicated control.
>   - Rebased on media_tree/master branch.
>
> version 8:
>   - Add reviewed-by and ack-by tags
>   - Fix the warnings reported by kernel test robot
>   - Only patch 9 (adding dedicated control), patch 11 (HEVC support) and
>     patch 13 (DT changes) are still missing of review/ack tag.
>
> version 7:
>   - Remove 'q' from syscon phandle name to make usable for iMX8MM too.
>     Update the bindings documentation.
>   - Add review/ack tags.
>   - Rebase on top of media_tree/master
>   - Be more accurate when computing the size of the memory needed motion
>     vectors.
>   - Explain why the all clocks need to set in the both DT node.
>
> version 6:
>   - fix the errors reported by kernel test robot
>
> version 5:
>   - use syscon instead of VPU reset driver.
>   - Do not break kernel/DT backward compatibility.
>   - Add documentation for dedicated Hantro control.
>   - Fix the remarks done by Ezequeil (typo, comments, unused function)
>   - Run v4l2-compliance without errors (see below).
>   - Do not add field to distinguish version, check postproc reg instead
>
> version 4:
> - Split the changes in hevc controls in 2 commits to make them easier to
>    review.
> - Change hantro_codec_ops run() prototype to return errors
> - Hantro v4l2 dedicated control is now only an integer
> - rebase on top of VPU reset changes posted here:
>    https://www.spinics.net/lists/arm-kernel/msg878440.html
> - Various fix from previous remarks
> - Limit the modifications in API to what the driver needs
>
> version 3:
> - Fix typo in Hantro v4l2 dedicated control
> - Add documentation for the new structures and fields
> - Rebased on top of media_tree for-linus-5.12-rc1 tag
>
> version 2:
> - remove all change related to scaling
> - squash commits to a coherent split
> - be more verbose about the added fields
> - fix the comments done by Ezequiel about dma_alloc_coherent usage
> - fix Dan's comments about control copy, reverse the test logic
> in tile_buffer_reallocate, rework some goto and return cases.
> - be more verbose about why I change the bindings
> - remove all sign-off expect mime since it is confusing
> - remove useless clocks in VPUs nodes
>
> Benjamin Gaignard (9):
>    media: hevc: Add fields and flags for hevc PPS
>    media: hevc: Add decode params control
>    media: hantro: change hantro_codec_ops run prototype to return errors
>    media: hantro: Define HEVC codec profiles and supported features
>    media: hantro: Only use postproc when post processed formats are
>      defined
>    media: uapi: Add a control for HANTRO driver
>    media: hantro: handle V4L2_PIX_FMT_HEVC_SLICE control
>    media: hantro: Introduce G2/HEVC decoder
>    media: hantro: IMX8M: add variant for G2/HEVC codec
>
>   .../userspace-api/media/drivers/hantro.rst    |  19 +
>   .../userspace-api/media/drivers/index.rst     |   1 +
>   .../media/v4l/ext-ctrls-codec.rst             | 108 +++-
>   .../media/v4l/vidioc-queryctrl.rst            |   6 +
>   drivers/media/v4l2-core/v4l2-ctrls-core.c     |  21 +-
>   drivers/media/v4l2-core/v4l2-ctrls-defs.c     |   4 +
>   drivers/staging/media/hantro/Makefile         |   2 +
>   drivers/staging/media/hantro/hantro.h         |  13 +-
>   drivers/staging/media/hantro/hantro_drv.c     |  99 ++-
>   .../staging/media/hantro/hantro_g1_h264_dec.c |  10 +-
>   .../media/hantro/hantro_g1_mpeg2_dec.c        |   4 +-
>   .../staging/media/hantro/hantro_g1_vp8_dec.c  |   6 +-
>   .../staging/media/hantro/hantro_g2_hevc_dec.c | 587 ++++++++++++++++++
>   drivers/staging/media/hantro/hantro_g2_regs.h | 206 ++++++
>   .../staging/media/hantro/hantro_h1_jpeg_enc.c |   4 +-
>   drivers/staging/media/hantro/hantro_hevc.c    | 327 ++++++++++
>   drivers/staging/media/hantro/hantro_hw.h      |  69 +-
>   .../staging/media/hantro/hantro_postproc.c    |  14 +
>   drivers/staging/media/hantro/hantro_v4l2.c    |   5 +-
>   drivers/staging/media/hantro/imx8m_vpu_hw.c   |  96 ++-
>   .../media/hantro/rk3399_vpu_hw_jpeg_enc.c     |   4 +-
>   .../media/hantro/rk3399_vpu_hw_mpeg2_dec.c    |   4 +-
>   .../media/hantro/rk3399_vpu_hw_vp8_dec.c      |   6 +-
>   drivers/staging/media/sunxi/cedrus/cedrus.c   |   6 +
>   drivers/staging/media/sunxi/cedrus/cedrus.h   |   1 +
>   .../staging/media/sunxi/cedrus/cedrus_dec.c   |   2 +
>   .../staging/media/sunxi/cedrus/cedrus_h265.c  |  12 +-
>   include/media/hevc-ctrls.h                    |  46 +-
>   28 files changed, 1613 insertions(+), 69 deletions(-)
>   create mode 100644 Documentation/userspace-api/media/drivers/hantro.rst
>   create mode 100644 drivers/staging/media/hantro/hantro_g2_hevc_dec.c
>   create mode 100644 drivers/staging/media/hantro/hantro_g2_regs.h
>   create mode 100644 drivers/staging/media/hantro/hantro_hevc.c
>
Hans Verkuil June 3, 2021, 9:19 a.m. UTC | #2
Hi Benjamin,

On 26/05/2021 14:45, Benjamin Gaignard wrote:
> The IMX8MQ got two VPUs but until now only G1 has been enabled.
> This series aim to add the second VPU (aka G2) and provide basic 
> HEVC decoding support.
> 
> To be able to decode HEVC it is needed to add/update some of the
> structures in the uapi. In addition of them one HANTRO dedicated
> control is required to inform the driver of the number of bits to skip
> at the beginning of the slice header.
> The hardware require to allocate few auxiliary buffers to store the
> references frame or tile size data.
> 
> The driver has been tested with fluster test suite stream.
> For example with this command: ./fluster.py run -ts JCT-VC-HEVC_V1 -d GStreamer-H.265-V4L2SL-Gst1.0
> 
> version 12:
>  - Change macro to avoid the final ';'
>  - Made arrays static and const
>  - Distinguish G2 generic fields (i.e these who could be reused for VP9)
>    from HEVC dedicated fields.

I ran the latest checkpatch/smatch/sparse etc. over this series and got a lot
of issues.

The trickiest first:

I get many reports for the register definitions in hantro_g2_regs.h:

drivers/staging/media/hantro/hantro_g2_regs.h:16:33: warning: 'g2_strm_start_offset' defined but not used [-Wunused-const-variable=]

I think it is a bad idea to declare variables in a header, and that's really
what is causing this.

I think it should be possible to rework this to defines such as:

#define G2_DEC_REG(b, s, m) \
        (const struct hantro_reg) { \
                .base = G2_SWREG(b), \
                .shift = s, \
                .mask = m, \
        }

#define g2_strm_start_offset G2_DEC_REG(259, 0, 0xffffffff)

This allows you to do:

hantro_reg_write(vpu, &g2_strm_start_offset, 0);

without having to declare static variables in a header.

Other warnings:

From the compiler:

linux-git-arm-multi: WARNINGS

drivers/staging/media/hantro/hantro_g2_hevc_dec.c: In function 'hantro_write_addr':
drivers/staging/media/hantro/hantro_g2_hevc_dec.c:23:24: warning: right shift count >= width of type [-Wshift-count-overflow]
   23 |  vdpu_write(vpu, (addr >> 32) & 0xffffffff, offset + 4);
      |                        ^~

Note: this builds on a 32-bit arm!

From smatch:

drivers/staging/media/hantro/hantro_hevc.c:228 tile_buffer_reallocate() error: double free of 'hevc_dec->tile_sao.cpu'
drivers/staging/media/hantro/hantro_hevc.c:234 tile_buffer_reallocate() error: double free of 'hevc_dec->tile_bsd.cpu'

From kerneldoc:

drivers/staging/media/hantro/hantro_hw.h:136: warning: Function parameter or member 'ref_bufs' not described in 'hantro_hevc_dec_hw_ctx'
drivers/staging/media/hantro/hantro_hw.h:136: warning: Function parameter or member 'ref_bufs_poc' not described in 'hantro_hevc_dec_hw_ctx'
drivers/staging/media/hantro/hantro_hw.h:136: warning: Function parameter or member 'ref_bufs_used' not described in 'hantro_hevc_dec_hw_ctx'
drivers/staging/media/hantro/hantro_hw.h:136: warning: Function parameter or member 'num_tile_cols_allocated' not described in
'hantro_hevc_dec_hw_ctx'

Regards,

	Hans

> 
> version 11:
>  - Rebased on media_stage.
>  - Fix minor typo/remarks.
> 
> version 10:
>  - Shorter version of the previous series without ctrl block patches
>    and no DT modifications.
>    The scope of this series is limited to HEVC support.
> 
> version 9:
>  - Corrections in commits messages.
>  - Define the dedicated control in hevc-controls.h
>  - Add note in documentation.
>  - Change max value of the dedicated control.
>  - Rebased on media_tree/master branch.
> 
> version 8:
>  - Add reviewed-by and ack-by tags 
>  - Fix the warnings reported by kernel test robot
>  - Only patch 9 (adding dedicated control), patch 11 (HEVC support) and
>    patch 13 (DT changes) are still missing of review/ack tag.
> 
> version 7:
>  - Remove 'q' from syscon phandle name to make usable for iMX8MM too.
>    Update the bindings documentation.
>  - Add review/ack tags.
>  - Rebase on top of media_tree/master
>  - Be more accurate when computing the size of the memory needed motion
>    vectors.
>  - Explain why the all clocks need to set in the both DT node.
> 
> version 6:
>  - fix the errors reported by kernel test robot
> 
> version 5:
>  - use syscon instead of VPU reset driver.
>  - Do not break kernel/DT backward compatibility.
>  - Add documentation for dedicated Hantro control.
>  - Fix the remarks done by Ezequeil (typo, comments, unused function)
>  - Run v4l2-compliance without errors (see below).
>  - Do not add field to distinguish version, check postproc reg instead
> 
> version 4:
> - Split the changes in hevc controls in 2 commits to make them easier to
>   review.
> - Change hantro_codec_ops run() prototype to return errors   
> - Hantro v4l2 dedicated control is now only an integer
> - rebase on top of VPU reset changes posted here:
>   https://www.spinics.net/lists/arm-kernel/msg878440.html
> - Various fix from previous remarks
> - Limit the modifications in API to what the driver needs
> 
> version 3:
> - Fix typo in Hantro v4l2 dedicated control
> - Add documentation for the new structures and fields
> - Rebased on top of media_tree for-linus-5.12-rc1 tag
> 
> version 2:
> - remove all change related to scaling
> - squash commits to a coherent split
> - be more verbose about the added fields
> - fix the comments done by Ezequiel about dma_alloc_coherent usage
> - fix Dan's comments about control copy, reverse the test logic
> in tile_buffer_reallocate, rework some goto and return cases.
> - be more verbose about why I change the bindings
> - remove all sign-off expect mime since it is confusing
> - remove useless clocks in VPUs nodes
> 
> Benjamin Gaignard (9):
>   media: hevc: Add fields and flags for hevc PPS
>   media: hevc: Add decode params control
>   media: hantro: change hantro_codec_ops run prototype to return errors
>   media: hantro: Define HEVC codec profiles and supported features
>   media: hantro: Only use postproc when post processed formats are
>     defined
>   media: uapi: Add a control for HANTRO driver
>   media: hantro: handle V4L2_PIX_FMT_HEVC_SLICE control
>   media: hantro: Introduce G2/HEVC decoder
>   media: hantro: IMX8M: add variant for G2/HEVC codec
> 
>  .../userspace-api/media/drivers/hantro.rst    |  19 +
>  .../userspace-api/media/drivers/index.rst     |   1 +
>  .../media/v4l/ext-ctrls-codec.rst             | 108 +++-
>  .../media/v4l/vidioc-queryctrl.rst            |   6 +
>  drivers/media/v4l2-core/v4l2-ctrls-core.c     |  21 +-
>  drivers/media/v4l2-core/v4l2-ctrls-defs.c     |   4 +
>  drivers/staging/media/hantro/Makefile         |   2 +
>  drivers/staging/media/hantro/hantro.h         |  13 +-
>  drivers/staging/media/hantro/hantro_drv.c     |  99 ++-
>  .../staging/media/hantro/hantro_g1_h264_dec.c |  10 +-
>  .../media/hantro/hantro_g1_mpeg2_dec.c        |   4 +-
>  .../staging/media/hantro/hantro_g1_vp8_dec.c  |   6 +-
>  .../staging/media/hantro/hantro_g2_hevc_dec.c | 587 ++++++++++++++++++
>  drivers/staging/media/hantro/hantro_g2_regs.h | 206 ++++++
>  .../staging/media/hantro/hantro_h1_jpeg_enc.c |   4 +-
>  drivers/staging/media/hantro/hantro_hevc.c    | 327 ++++++++++
>  drivers/staging/media/hantro/hantro_hw.h      |  69 +-
>  .../staging/media/hantro/hantro_postproc.c    |  14 +
>  drivers/staging/media/hantro/hantro_v4l2.c    |   5 +-
>  drivers/staging/media/hantro/imx8m_vpu_hw.c   |  96 ++-
>  .../media/hantro/rk3399_vpu_hw_jpeg_enc.c     |   4 +-
>  .../media/hantro/rk3399_vpu_hw_mpeg2_dec.c    |   4 +-
>  .../media/hantro/rk3399_vpu_hw_vp8_dec.c      |   6 +-
>  drivers/staging/media/sunxi/cedrus/cedrus.c   |   6 +
>  drivers/staging/media/sunxi/cedrus/cedrus.h   |   1 +
>  .../staging/media/sunxi/cedrus/cedrus_dec.c   |   2 +
>  .../staging/media/sunxi/cedrus/cedrus_h265.c  |  12 +-
>  include/media/hevc-ctrls.h                    |  46 +-
>  28 files changed, 1613 insertions(+), 69 deletions(-)
>  create mode 100644 Documentation/userspace-api/media/drivers/hantro.rst
>  create mode 100644 drivers/staging/media/hantro/hantro_g2_hevc_dec.c
>  create mode 100644 drivers/staging/media/hantro/hantro_g2_regs.h
>  create mode 100644 drivers/staging/media/hantro/hantro_hevc.c
>
Benjamin Gaignard June 3, 2021, 11:39 a.m. UTC | #3
Le 03/06/2021 à 11:19, Hans Verkuil a écrit :
> Hi Benjamin,
>
> On 26/05/2021 14:45, Benjamin Gaignard wrote:
>> The IMX8MQ got two VPUs but until now only G1 has been enabled.
>> This series aim to add the second VPU (aka G2) and provide basic
>> HEVC decoding support.
>>
>> To be able to decode HEVC it is needed to add/update some of the
>> structures in the uapi. In addition of them one HANTRO dedicated
>> control is required to inform the driver of the number of bits to skip
>> at the beginning of the slice header.
>> The hardware require to allocate few auxiliary buffers to store the
>> references frame or tile size data.
>>
>> The driver has been tested with fluster test suite stream.
>> For example with this command: ./fluster.py run -ts JCT-VC-HEVC_V1 -d GStreamer-H.265-V4L2SL-Gst1.0
>>
>> version 12:
>>   - Change macro to avoid the final ';'
>>   - Made arrays static and const
>>   - Distinguish G2 generic fields (i.e these who could be reused for VP9)
>>     from HEVC dedicated fields.
> I ran the latest checkpatch/smatch/sparse etc. over this series and got a lot
> of issues.
>
> The trickiest first:
>
> I get many reports for the register definitions in hantro_g2_regs.h:
>
> drivers/staging/media/hantro/hantro_g2_regs.h:16:33: warning: 'g2_strm_start_offset' defined but not used [-Wunused-const-variable=]
>
> I think it is a bad idea to declare variables in a header, and that's really
> what is causing this.
>
> I think it should be possible to rework this to defines such as:
>
> #define G2_DEC_REG(b, s, m) \
>          (const struct hantro_reg) { \
>                  .base = G2_SWREG(b), \
>                  .shift = s, \
>                  .mask = m, \
>          }

checkpatch complains about the macro to be complex so I have to add parentheses
but that will be in v12

> #define g2_strm_start_offset G2_DEC_REG(259, 0, 0xffffffff)
>
> This allows you to do:
>
> hantro_reg_write(vpu, &g2_strm_start_offset, 0);
>
> without having to declare static variables in a header.
>
> Other warnings:
>
>  From the compiler:
>
> linux-git-arm-multi: WARNINGS
>
> drivers/staging/media/hantro/hantro_g2_hevc_dec.c: In function 'hantro_write_addr':
> drivers/staging/media/hantro/hantro_g2_hevc_dec.c:23:24: warning: right shift count >= width of type [-Wshift-count-overflow]
>     23 |  vdpu_write(vpu, (addr >> 32) & 0xffffffff, offset + 4);
>        |                        ^~
>
> Note: this builds on a 32-bit arm!
>
>  From smatch:
>
> drivers/staging/media/hantro/hantro_hevc.c:228 tile_buffer_reallocate() error: double free of 'hevc_dec->tile_sao.cpu'
> drivers/staging/media/hantro/hantro_hevc.c:234 tile_buffer_reallocate() error: double free of 'hevc_dec->tile_bsd.cpu'

That will be fixed in v12

>
>  From kerneldoc:
>
> drivers/staging/media/hantro/hantro_hw.h:136: warning: Function parameter or member 'ref_bufs' not described in 'hantro_hevc_dec_hw_ctx'
> drivers/staging/media/hantro/hantro_hw.h:136: warning: Function parameter or member 'ref_bufs_poc' not described in 'hantro_hevc_dec_hw_ctx'
> drivers/staging/media/hantro/hantro_hw.h:136: warning: Function parameter or member 'ref_bufs_used' not described in 'hantro_hevc_dec_hw_ctx'
> drivers/staging/media/hantro/hantro_hw.h:136: warning: Function parameter or member 'num_tile_cols_allocated' not described in
> 'hantro_hevc_dec_hw_ctx'

I fix this and also remove the description for not existing fields.

Thanks for your review.
Benjamin

>
> Regards,
>
> 	Hans
>
>> version 11:
>>   - Rebased on media_stage.
>>   - Fix minor typo/remarks.
>>
>> version 10:
>>   - Shorter version of the previous series without ctrl block patches
>>     and no DT modifications.
>>     The scope of this series is limited to HEVC support.
>>
>> version 9:
>>   - Corrections in commits messages.
>>   - Define the dedicated control in hevc-controls.h
>>   - Add note in documentation.
>>   - Change max value of the dedicated control.
>>   - Rebased on media_tree/master branch.
>>
>> version 8:
>>   - Add reviewed-by and ack-by tags
>>   - Fix the warnings reported by kernel test robot
>>   - Only patch 9 (adding dedicated control), patch 11 (HEVC support) and
>>     patch 13 (DT changes) are still missing of review/ack tag.
>>
>> version 7:
>>   - Remove 'q' from syscon phandle name to make usable for iMX8MM too.
>>     Update the bindings documentation.
>>   - Add review/ack tags.
>>   - Rebase on top of media_tree/master
>>   - Be more accurate when computing the size of the memory needed motion
>>     vectors.
>>   - Explain why the all clocks need to set in the both DT node.
>>
>> version 6:
>>   - fix the errors reported by kernel test robot
>>
>> version 5:
>>   - use syscon instead of VPU reset driver.
>>   - Do not break kernel/DT backward compatibility.
>>   - Add documentation for dedicated Hantro control.
>>   - Fix the remarks done by Ezequeil (typo, comments, unused function)
>>   - Run v4l2-compliance without errors (see below).
>>   - Do not add field to distinguish version, check postproc reg instead
>>
>> version 4:
>> - Split the changes in hevc controls in 2 commits to make them easier to
>>    review.
>> - Change hantro_codec_ops run() prototype to return errors
>> - Hantro v4l2 dedicated control is now only an integer
>> - rebase on top of VPU reset changes posted here:
>>    https://www.spinics.net/lists/arm-kernel/msg878440.html
>> - Various fix from previous remarks
>> - Limit the modifications in API to what the driver needs
>>
>> version 3:
>> - Fix typo in Hantro v4l2 dedicated control
>> - Add documentation for the new structures and fields
>> - Rebased on top of media_tree for-linus-5.12-rc1 tag
>>
>> version 2:
>> - remove all change related to scaling
>> - squash commits to a coherent split
>> - be more verbose about the added fields
>> - fix the comments done by Ezequiel about dma_alloc_coherent usage
>> - fix Dan's comments about control copy, reverse the test logic
>> in tile_buffer_reallocate, rework some goto and return cases.
>> - be more verbose about why I change the bindings
>> - remove all sign-off expect mime since it is confusing
>> - remove useless clocks in VPUs nodes
>>
>> Benjamin Gaignard (9):
>>    media: hevc: Add fields and flags for hevc PPS
>>    media: hevc: Add decode params control
>>    media: hantro: change hantro_codec_ops run prototype to return errors
>>    media: hantro: Define HEVC codec profiles and supported features
>>    media: hantro: Only use postproc when post processed formats are
>>      defined
>>    media: uapi: Add a control for HANTRO driver
>>    media: hantro: handle V4L2_PIX_FMT_HEVC_SLICE control
>>    media: hantro: Introduce G2/HEVC decoder
>>    media: hantro: IMX8M: add variant for G2/HEVC codec
>>
>>   .../userspace-api/media/drivers/hantro.rst    |  19 +
>>   .../userspace-api/media/drivers/index.rst     |   1 +
>>   .../media/v4l/ext-ctrls-codec.rst             | 108 +++-
>>   .../media/v4l/vidioc-queryctrl.rst            |   6 +
>>   drivers/media/v4l2-core/v4l2-ctrls-core.c     |  21 +-
>>   drivers/media/v4l2-core/v4l2-ctrls-defs.c     |   4 +
>>   drivers/staging/media/hantro/Makefile         |   2 +
>>   drivers/staging/media/hantro/hantro.h         |  13 +-
>>   drivers/staging/media/hantro/hantro_drv.c     |  99 ++-
>>   .../staging/media/hantro/hantro_g1_h264_dec.c |  10 +-
>>   .../media/hantro/hantro_g1_mpeg2_dec.c        |   4 +-
>>   .../staging/media/hantro/hantro_g1_vp8_dec.c  |   6 +-
>>   .../staging/media/hantro/hantro_g2_hevc_dec.c | 587 ++++++++++++++++++
>>   drivers/staging/media/hantro/hantro_g2_regs.h | 206 ++++++
>>   .../staging/media/hantro/hantro_h1_jpeg_enc.c |   4 +-
>>   drivers/staging/media/hantro/hantro_hevc.c    | 327 ++++++++++
>>   drivers/staging/media/hantro/hantro_hw.h      |  69 +-
>>   .../staging/media/hantro/hantro_postproc.c    |  14 +
>>   drivers/staging/media/hantro/hantro_v4l2.c    |   5 +-
>>   drivers/staging/media/hantro/imx8m_vpu_hw.c   |  96 ++-
>>   .../media/hantro/rk3399_vpu_hw_jpeg_enc.c     |   4 +-
>>   .../media/hantro/rk3399_vpu_hw_mpeg2_dec.c    |   4 +-
>>   .../media/hantro/rk3399_vpu_hw_vp8_dec.c      |   6 +-
>>   drivers/staging/media/sunxi/cedrus/cedrus.c   |   6 +
>>   drivers/staging/media/sunxi/cedrus/cedrus.h   |   1 +
>>   .../staging/media/sunxi/cedrus/cedrus_dec.c   |   2 +
>>   .../staging/media/sunxi/cedrus/cedrus_h265.c  |  12 +-
>>   include/media/hevc-ctrls.h                    |  46 +-
>>   28 files changed, 1613 insertions(+), 69 deletions(-)
>>   create mode 100644 Documentation/userspace-api/media/drivers/hantro.rst
>>   create mode 100644 drivers/staging/media/hantro/hantro_g2_hevc_dec.c
>>   create mode 100644 drivers/staging/media/hantro/hantro_g2_regs.h
>>   create mode 100644 drivers/staging/media/hantro/hantro_hevc.c
>>