Re: [PATCH v2 0/7] media: venus: Add QCM2290 support with AR50_LITE core

[Jorge Ramirez <jorge.ramirez@oss.qualcomm.com>]

On 19/06/25 15:29:02, Bryan O'Donoghue wrote:
> On 19/06/2025 15:20, Jorge Ramirez-Ortiz wrote:
> > Changes since v1:
> > - Add IS_HFI macro usage
> > - Move schema patch to top
> > - Fix commit messages
> > 
> > This patch series adds support for the Venus video decoder/encoder block
> > present on the Qualcomm QCM2290.
> > 
> > The QCM2290 integrates an AR50_LITE core, a low-power implementation of
> > Venus supporting H.264, HEVC (H.265), and VP9 decoding.
> > 
> > The series includes:
> >    • DT binding schema for qcom,qcm2290-venus
> >    • SoC integration via qcm2290.dtsi
> >    • Resource table definitions and frequency scaling
> >    • Platform capability registration for the AR50_LITE core
> > 
> > Decoding was verified on the QCOM RB1 platform using GStreamer with V4L2-based
> > decode plugins. The following pipelines were used for playback 1280x720 and
> > 1920x1080 H.264, HVEC and VP9 videos from https://www.elecard.com/videos.
> > 
> > [H.264]
> > gst-launch-1.0 filesrc location=videos/xxxxx.mp4 \
> >    ! qtdemux name=demux demux.video_0 ! queue ! h264parse ! v4l2h264dec \
> >    ! videoconvert ! autovideosink
> > 
> > [H.265]
> > gst-launch-1.0 filesrc location=videos/xxxxx.mp4 \
> >    ! qtdemux name=demux demux.video_0 ! queue ! h265parse ! v4l2h265dec \
> >    ! videoconvert ! autovideosink
> > 
> > [VP9]
> > gst-launch-1.0 filesrc location=videos/xxxxx.webm \
> >    ! matroskademus ! queue ! v4l2vp8dec \
> >    ! videoconvert ! autovideosink
> > 
> > ---
> > 
> > Jorge Ramirez-Ortiz (7):
> >    dt-bindings: media: venus: Add qcm2290 dt schema
> >    media: venus: helpers: add IS_VPU() and IS_HFI() macros
> >    media: venus: use IS_HFI() macro for multi-version check
> >    media: venus: vdec: AR50_LITE video core support
> >    media: venus: hfi_plat_v6_lite: Populate decode capabilities
> >    media: venus: core: Add qcm2290 DT compatible and resource data
> >    arm64: dts: qcom: qcm2290: Add venus video node
> > 
> >   .../bindings/media/qcom,qcm2290-venus.yaml    | 117 ++++++++++++++
> >   arch/arm64/boot/dts/qcom/qcm2290.dtsi         |  45 ++++++
> >   drivers/media/platform/qcom/venus/Makefile    |   2 +-
> >   drivers/media/platform/qcom/venus/core.c      |  51 +++++-
> >   drivers/media/platform/qcom/venus/core.h      |  28 +++-
> >   drivers/media/platform/qcom/venus/firmware.c  |   8 +-
> >   drivers/media/platform/qcom/venus/helpers.c   |  81 ++++++++++
> >   drivers/media/platform/qcom/venus/helpers.h   |   2 +
> >   .../media/platform/qcom/venus/hfi_helper.h    |  10 +-
> >   .../media/platform/qcom/venus/hfi_platform.c  |   2 +
> >   .../media/platform/qcom/venus/hfi_platform.h  |   1 +
> >   .../qcom/venus/hfi_platform_v6_lite.c         | 148 ++++++++++++++++++
> >   drivers/media/platform/qcom/venus/hfi_venus.c |  19 ++-
> >   .../media/platform/qcom/venus/pm_helpers.c    |   1 +
> >   drivers/media/platform/qcom/venus/vdec.c      |  17 +-
> >   15 files changed, 504 insertions(+), 28 deletions(-)
> >   create mode 100644 Documentation/devicetree/bindings/media/qcom,qcm2290-venus.yaml
> >   create mode 100644 drivers/media/platform/qcom/venus/hfi_platform_v6_lite.c
> > 
> 
> Please fix the following.
> 
> ~/my-environment/bin/checkset.sh
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #16:
> new file mode 100644
> 
> total: 0 errors, 1 warnings, 0 checks, 117 lines checked
> 
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or
> --fix-inplace.
> 
> 0001-dt-bindings-media-venus-Add-qcm2290-dt-schema.patch has style problems,
> please review.
> 
> NOTE: If any of the errors are false positives, please report
>       them to the maintainer, see CHECKPATCH in MAINTAINERS.
> CHECK: Macro argument reuse 'core' - possible side-effects?
> #33: FILE: drivers/media/platform/qcom/venus/core.h:537:
> +#define MATCH_2(CMD, core, a, b) \
> +	(IS_##CMD##_MATCH(core, a) || IS_##CMD##_MATCH(core, b))
> 
> CHECK: Macro argument reuse 'core' - possible side-effects?
> #35: FILE: drivers/media/platform/qcom/venus/core.h:539:
> +#define MATCH_3(CMD, core, a, b, c) \
> +	(IS_##CMD##_MATCH(core, a) || IS_##CMD##_MATCH(core, b) || \
> +	IS_##CMD##_MATCH(core, c))
> 
> CHECK: Macro argument reuse 'core' - possible side-effects?
> #38: FILE: drivers/media/platform/qcom/venus/core.h:542:
> +#define MATCH_4(CMD, core, a, b, c, d) \
> +	(IS_##CMD##_MATCH(core, a) || IS_##CMD##_MATCH(core, b) || \
> +	IS_##CMD##_MATCH(core, c) || IS_##CMD##_MATCH(core, d))

um ok, I'll rename core, makes sense.

> 
> WARNING: Argument '_1' is not used in function-like macro
> #42: FILE: drivers/media/platform/qcom/venus/core.h:546:
> +#define GET_MACRO(_1, _2, _3, _4, NAME, ...) NAME /* _1-_4 are ignored */
> 
> WARNING: Argument '_2' is not used in function-like macro
> #42: FILE: drivers/media/platform/qcom/venus/core.h:546:
> +#define GET_MACRO(_1, _2, _3, _4, NAME, ...) NAME /* _1-_4 are ignored */
> 
> WARNING: Argument '_3' is not used in function-like macro
> #42: FILE: drivers/media/platform/qcom/venus/core.h:546:
> +#define GET_MACRO(_1, _2, _3, _4, NAME, ...) NAME /* _1-_4 are ignored */
> 
> WARNING: Argument '_4' is not used in function-like macro
> #42: FILE: drivers/media/platform/qcom/venus/core.h:546:
> +#define GET_MACRO(_1, _2, _3, _4, NAME, ...) NAME /* _1-_4 are ignored */


about the warnings there is nothing that can be done - those parameters
as per the comments are necessarily ignored - that is the way the macro
is supposed to work.

can we not live with them? otherwise I cant do it in a nice way.

> 
> total: 0 errors, 4 warnings, 3 checks, 29 lines checked
> 
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or
> --fix-inplace.
> 
> 0002-media-venus-helpers-add-IS_VPU-and-IS_HFI-macros.patch has style
> problems, please review.
> 
> NOTE: If any of the errors are false positives, please report
>       them to the maintainer, see CHECKPATCH in MAINTAINERS.
> total: 0 errors, 0 warnings, 0 checks, 16 lines checked
> 
> 0003-media-venus-use-IS_HFI-macro-for-multi-version-check.patch has no
> obvious style problems and is ready for submission.
> CHECK: Macro argument reuse 'core' - possible side-effects?
> #79: FILE: drivers/media/platform/qcom/venus/core.h:525:
> +#define IS_V6(core)     (((core)->res->hfi_version == HFI_VERSION_6XX) || \
> +			 ((core)->res->hfi_version == HFI_VERSION_6XX_LITE))
> 
> CHECK: Blank lines aren't necessary before a close brace '}'
> #153: FILE: drivers/media/platform/qcom/venus/helpers.c:254:
> +
> +}

ok, will remove core from macros

> 
> CHECK: Unnecessary parentheses around 'buftype != HFI_BUFFER_INPUT'
> #215: FILE: drivers/media/platform/qcom/venus/helpers.c:709:
> +		if ((buftype != HFI_BUFFER_INPUT) &&
> +		    (buftype != HFI_BUFFER_OUTPUT) &&
> +		    (buftype != HFI_BUFFER_OUTPUT2))
> 
> CHECK: Unnecessary parentheses around 'buftype != HFI_BUFFER_OUTPUT'
> #215: FILE: drivers/media/platform/qcom/venus/helpers.c:709:
> +		if ((buftype != HFI_BUFFER_INPUT) &&
> +		    (buftype != HFI_BUFFER_OUTPUT) &&
> +		    (buftype != HFI_BUFFER_OUTPUT2))
> 
> CHECK: Unnecessary parentheses around 'buftype != HFI_BUFFER_OUTPUT2'
> #215: FILE: drivers/media/platform/qcom/venus/helpers.c:709:
> +		if ((buftype != HFI_BUFFER_INPUT) &&
> +		    (buftype != HFI_BUFFER_OUTPUT) &&
> +		    (buftype != HFI_BUFFER_OUTPUT2))
>

yeah, ok

> total: 0 errors, 0 warnings, 5 checks, 314 lines checked
> 
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or
> --fix-inplace.
> 
> 0004-media-venus-vdec-AR50_LITE-video-core-support.patch has style problems,
> please review.
> 
> NOTE: If any of the errors are false positives, please report
>       them to the maintainer, see CHECKPATCH in MAINTAINERS.
> 0004-media-venus-vdec-AR50_LITE-video-core-support.patch:64: bufferes ==>
> buffers, buffered
> 0004-media-venus-vdec-AR50_LITE-video-core-support.patch:65: bufferes ==>
> buffers, buffered
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #57:
> new file mode 100644
> 
> total: 0 errors, 1 warnings, 0 checks, 171 lines checked
> 
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or
> --fix-inplace.
> 
> 0005-media-venus-hfi_plat_v6_lite-Populate-decode-capabil.patch has style
> problems, please review.
> 
> NOTE: If any of the errors are false positives, please report
>       them to the maintainer, see CHECKPATCH in MAINTAINERS.
> total: 0 errors, 0 warnings, 0 checks, 52 lines checked
> 
> 0006-media-venus-core-Add-qcm2290-DT-compatible-and-resou.patch has no
> obvious style problems and is ready for submission.
> total: 0 errors, 0 warnings, 0 checks, 51 lines checked
> 
> 0007-arm64-dts-qcom-qcm2290-Add-venus-video-node.patch has no obvious style
> problems and is ready for submission.
>  ~/Development/worktree/merges/linux-6.17-media-platform-qcom-next
> (linux-6.17-media-platform-qcom-next*) [03:28:03]
> deckard$
>

ah thanks, was just going to ask

> deckard$ cat ~/my-environment/bin/checkset.sh
> #!/bin/sh
> if [ -z "$KERNELPATH" ]
> then
> 	echo "Setting KERNELPATH to this directory which is probably what you want"
> 	echo "Please set KERNELPATH when running this utility to over-ride";
> 	echo "KERNELPATH=`pwd` ~/my-environment/bin/checkset.sh"
> 	KERNELPATH=`pwd`
> fi
> for file in *.patch;
> do $KERNELPATH/scripts/checkpatch.pl --strict $file;
> codespell $file;
> done
> 
> ---
> bod