Message-ID: <aJ0QJVnafFf/S0EF@trex>
Date: Thu, 14 Aug 2025 00:22:29 +0200
From: Jorge Ramirez <jorge.ramirez@....qualcomm.com>
To: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
Cc: Jorge Ramirez-Ortiz <jorge.ramirez@....qualcomm.com>,
        quic_vgarodia@...cinc.com, quic_dikshita@...cinc.com,
        krzk+dt@...nel.org, konradybcio@...nel.org,
        dmitry.baryshkov@....qualcomm.com, mchehab@...nel.org, robh@...nel.org,
        andersson@...nel.org, linux-arm-msm@...r.kernel.org,
        linux-media@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v9 0/8] media: venus: Add QCM2290 support with AR50_LITE
 core

On 13/08/25 23:18:04, Bryan O'Donoghue wrote:
> On 08/08/2025 09:52, Jorge Ramirez-Ortiz wrote:
> > Changes since v8:
> >    v8: https://lore.kernel.org/all/20250805064430.782201-1-jorge.ramirez@oss.qualcomm.com/
> > -  patch 1/8: Add qcm2290 dt schema
> >    - no change
> > - patch 2/8: media: venus: Define minimum valid firmware version
> >    - dev_err on failure
> > - patch 3/8: Add framework to support for AR50 LITE video core
> >    - pass core rather than the lite property to the hfi platform
> >    - remove warnings
> > - patch 4/8: HFI platform V4 capabilities
> >    -no change
> > - patch 5/8: Sort dt_match alphabetically
> >    - new
> > - patch 6/8: Add DT compatible and capabilities
> >    - sort dt_match alphabetically
> > - patch 7/8: Add Venus video node for 2290
> >    - Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
> > - patch 8/8: Enable Venus on 2210 RB1 board
> >    - Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
> > 
> > Changes since v7:
> >    v7: https://lore.kernel.org/all/20250715204749.2189875-1-jorge.ramirez@oss.qualcomm.com/
> > -  patch 1/7: Add qcm2290 dt schema
> >    - extend commit message to cover IOMMU order, remove iommus minItems, alphabetic order for includes
> >    - added myself as the file maintainer
> > -  patch 2/7: media: venus: Define minimum valid firmware version
> >    - Replaces previous patch that was able to distinguish encoder vs decoder firmware version support
> >    - Now it is an all or nothing implementation (the firmware either support enc and dec or it doesn't)
> > - patch 3/7: Add Support for AR50 LITE video core
> >    - remove EOS support for older firmwares since we will drop support for fw < 6.0.55
> > - patch 4/7: HFI platform V4 capabilities
> >    - remove some hardcoded values and some ternary operations
> >    - added reviewed by Bryan O'Donoghue <bryan.odonoghue@...aro.org>
> > - patch 5/7: Add DT compatible and capabilities
> >    - only min_fw now required since we dont support encode/decode indepedently after 2/7
> >    - added reviewed by Dikshita Agarwal <quic_dikshita@...cinc.com>
> > - patch 6/7: Add Venus video node for 2290
> >    - no change
> > - patch 7/7: Enable Venus on 2210 RB1 board
> >    - fix alphabetical order
> > 
> > Changes since v6:
> >    v6: https://lore.kernel.org/all/20250708180530.1384330-1-jorge.ramirez@oss.qualcomm.com/
> > - patch 1/7: Add qcm2290 dt schema
> >    - no change
> > - patch 2/7: Conditionally register video nodes
> >    - fixed potential NULL dereference on handling firmware errors when the encoder is not present.
> > - patch 3/7: Add Support for AR50 LITE video core
> >    - applied Dikshita Agarwal review comments: do not handle no-tz case, fix WD mask
> >    - replaced EOS buffers for a work around (firmware should not access the eos buffers)
> >       renamed venus_helper_alloc_eos_buf to venus_helper_prepare_eos_data
> > - patch 4/7: HFI platform V4 capabilities
> >    - removed 10 bit encoder support as per Dikshita Agarwal review
> > - patch 5/7: Add DT compatible and capabilities
> >    - removed frequency table as per Konrad Dybcio review (driver now uses opp)
> >    - fixed bw_tables for the supported loads (remove 10 bit support)
> > - patch 6/7: Add Venus video node for 2290
> >    - no change
> > - patch 7/7: Enable Venus on 2210 RB1 board
> >    - no change
> > 
> > Changes since v5:
> >    Two additional new patches added to the set to support video encoding.
> > 
> > - patch 1/7: Add qcm2290 dt schema
> >    - added reviewed by: Bryan O'Donoghue
> >    - added reviewed by: Krzysztof Kozlowski
> > - patch 2/7: Conditionally register video nodes
> >    - new functionality required to support AR50_LITE
> > - patch 3/7: Add Support for AR50 LITE video core
> >    - completely rewritten - lite platforms are extensions of previous HFI_XXX
> > - patch 4/7: HFI platform V4 capabilities
> >    - use HFI 4XX instead of HFI 6XX as per review
> > - patch 5/7: Add DT compatible and capabilities
> >    - added reviewed by: Bryan O'Donoghue
> >    - enabled new option min_fw_version
> > - patch 6/7: Add Venus video node for 2290
> >    - added reviewed by: Konrad Dybcio
> >    - removed status property
> > - patch 7/7: Enable Venus on 2210 RB1 board
> >    - new
> > 
> > Changes since v4:
> > - patch 1/5:
> >    - added reviewed by: Krzysztof Kozlowski
> >    - updated example section
> > - patch 2/5:
> >    - added reviewed by: Bryan O'Donoghue
> >    - Fixed Co-developed-by order
> > - patch 3/5:
> > - patch 4/5:
> >   - removed encode-node (userspace exposure)
> >   - fixed Co-developed-by order
> > - patch 5/5:
> >   - fixed venus register region
> >   - power-domain-names: one per line
> >   - clock-names: one per line
> >   - fixed interconnect tags
> >   - empty line before subnode
> >   - enable the venus node
> > 
> > Changes since v3:
> > - Fixed schema commit subject.
> > 
> > Changes since v2:
> > - Removed IS_HFI/IS_VPU macros
> > - checkpatch.pl --strict fixes:
> >    - convert macro to static inline to avoid argument reuse side effect
> > 
> > Changes since v1:
> > - Added IS_HFI macro usage
> > - Moved schema patch to top
> > - Fixed commit messages
> > 
> > This patch series adds support for the Venus video decoder/encoder block
> > present on the Qualcomm QCM2290.
> > 
> > This series functionally depends on the following patch:
> > 
> >    clk: qcom: gcc-qcm2290: Set HW_CTRL_TRIGGER for video GDSC
> >    https://lore.kernel.org/all/20250613102245.782511-1-loic.poulain@oss.qualcomm.com/
> > 
> > Please ensure the above patch is applied before this series.
> > 
> > The QCM2290 integrates an AR50_LITE core, a low-power implementation of
> > Venus supporting H.264, HEVC (H.265), and VP9 decoding as well as H.264
> > and H.265 encoding.
> > 
> > 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 block.
> > 
> > Limitations:
> >    - The hardware does not support concurrent operation at FullHD.
> >    - Encoding is not supported in firmware version before 6.0.54
> > 
> > Firmware:
> >    - This code has been tested with 6.0.52, 6.0.53, 6.0.54-PRERELEASE
> > 
> > Some basic Tests:
> > 
> > Decoding and Encoding were 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, HEVC and VP9 videos from https://www.elecard.com/videos.
> > 
> > Decoding:
> > [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 \
> >    ! matroskademux ! queue ! v4l2vp9dec \
> >    ! videoconvert ! autovideosink
> > 
> > Encoding:
> > [H.264]
> > gst-launch-1.0 videotestsrc num-buffers=30 \
> >    ! video/x-raw,width=1280,height=720,framerate=30/1 \
> >    ! v4l2h264enc ! h264parse ! mp4mux ! filesink location=/tmp/output.mp4
> > 
> > [H.265]
> > gst-launch-1.0 videotestsrc num-buffers=30 \
> >    ! video/x-raw,width=1920,height=1080,framerate=30/1 \
> >    ! v4l2h265enc ! h265parse ! mp4mux ! filesink location=/tmp/output.mp4
> > 
> > Concurrency:
> > gst-launch-1.0 -v videotestsrc num-buffers=1000 \
> >    ! video/x-raw,format=NV12,width=1280,height=720,framerate=30/1 \
> >    ! v4l2h264enc capture-io-mode=4 output-io-mode=2 ! h264parse \
> >    ! v4l2h264dec capture-io-mode=4 output-io-mode=2 ! videoconvert \
> >    ! autovideosink
> > 
> > [Fluster]
> > The H.264 decoder was also tested using the Fluster test suite
> > (version: v0.4.0-12-g33566abd0964).
> >   Target: GStreamer-H.264-V4L2-Gst1.0, Test Suite: JVT-AVC_V1
> >   Result: 126/135 tests passed
> >   Failures:
> >   FM1_BT_B, FM1_FT_E, FM2_SVA_C, BA3_SVA_C, SP1_BT_A,
> >   SP2_BT_B, MR6_BT_B, MR7_BT_B, MR8_BT_B
> > 
> > ---
> > 
> > Jorge Ramirez-Ortiz (8):
> >    media: dt-bindings: venus: Add qcm2290 dt schema
> >    media: venus: Define minimum valid firmware version
> >    media: venus: Add framework support for AR50_LITE video core
> >    media: venus: hfi_plat_v4: Add capabilities for the 4XX lite core
> >    media: venus: core: Sort dt_match alphabetically.
> >    media: venus: core: Add qcm2290 DT compatible and resource data
> >    arm64: dts: qcom: qcm2290: Add Venus video node
> >    arm64: dts: qcom: qrb2210-rb1: Enable Venus
> > 
> >   .../bindings/media/qcom,qcm2290-venus.yaml    | 130 ++++++++++++
> >   arch/arm64/boot/dts/qcom/qcm2290.dtsi         |  55 +++++
> >   arch/arm64/boot/dts/qcom/qrb2210-rb1.dts      |   4 +
> >   drivers/media/platform/qcom/venus/core.c      | 109 +++++++---
> >   drivers/media/platform/qcom/venus/core.h      |  18 +-
> >   drivers/media/platform/qcom/venus/firmware.c  |  30 +++
> >   drivers/media/platform/qcom/venus/firmware.h  |   2 +
> >   drivers/media/platform/qcom/venus/helpers.c   |  12 +-
> >   .../media/platform/qcom/venus/hfi_parser.c    |   2 +-
> >   .../media/platform/qcom/venus/hfi_platform.c  |  23 ++-
> >   .../media/platform/qcom/venus/hfi_platform.h  |  34 ++--
> >   .../platform/qcom/venus/hfi_platform_v4.c     | 188 ++++++++++++++++--
> >   .../platform/qcom/venus/hfi_platform_v6.c     |  33 ++-
> >   drivers/media/platform/qcom/venus/hfi_venus.c |  25 ++-
> >   .../media/platform/qcom/venus/hfi_venus_io.h  |   4 +
> >   .../media/platform/qcom/venus/pm_helpers.c    |   2 +-
> >   16 files changed, 582 insertions(+), 89 deletions(-)
> >   create mode 100644 Documentation/devicetree/bindings/media/qcom,qcm2290-venus.yaml
> > 
> 
> Trying to apply your series and getting a few or more than one splat anyway,
> on media-comitters/next
> 
> Could you fix this series and test up as a V10 ?

sure, let me rebase and retest.

FYI when I posted I applied it on top


    media: rkvdec: Unstage the driver
    
    The TODO list for unstaging being empty, the driver can now be moved to the
    main media folder.
    
    Also add myself as maintainer.
    
    Signed-off-by: Detlev Casanova <detlev.casanova@...labora.com>
    Reviewed-by: Nicolas Dufresne <nicolas.dufresne@...labora.com>
    Signed-off-by: Nicolas Dufresne <nicolas.dufresne@...labora.com>
    Signed-off-by: Hans Verkuil <hverkuil@...all.nl>


> 
> https://gitlab.freedesktop.org/linux-media/users/bodonoghue/-/commits/b4/bod-media-committers-next-platform-qcom-6.17-rc1
> 
> ---
> bod
> 
> 

Powered by blists - more mailing lists