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

Jorge Ramirez
2025-06-19T14:38:45Z

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