Message ID | 20211207015446.1250854-1-aford173@gmail.com (mailing list archive) |
---|---|
Headers |
Received: from vger.kernel.org ([23.128.96.18]) by www.linuxtv.org with esmtp (Exim 4.92) (envelope-from <linux-media-owner@vger.kernel.org>) id 1muPh8-009rsm-GJ; Tue, 07 Dec 2021 01:55:02 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236621AbhLGB63 (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Mon, 6 Dec 2021 20:58:29 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39210 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233991AbhLGB63 (ORCPT <rfc822;linux-media@vger.kernel.org>); Mon, 6 Dec 2021 20:58:29 -0500 Received: from mail-io1-xd2e.google.com (mail-io1-xd2e.google.com [IPv6:2607:f8b0:4864:20::d2e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9769DC061746; Mon, 6 Dec 2021 17:54:59 -0800 (PST) Received: by mail-io1-xd2e.google.com with SMTP id x10so15211098ioj.9; Mon, 06 Dec 2021 17:54:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=GG9IZeKUXqHbwgRlEUez+S6LOG7iNmAE27aQpf7wARQ=; b=WqIK77xBg4jom+nkZAVdSaO+QIg13np6F3MKrEpOJxlgBj0edb47bwPLekDQTHcKeK FA6d4WHrbDFppB6kaDqVZOMsV1vBjPQVh+b4LV0tRsPi9oMulqUiJH5AW3HVYjxnKDGs uiJfjNrUOd9TGOE79/VgAXaRCZsG4oJoFZvJtzTzigoTIhct5DRYBLL89/UYpa3oWGnb 2cRrKUPPdp9rsOuSJ89vNpJXKg8+bnCPY4SW7GDNgMJ+6s5AeDatpN1Qz0elwnESIzuU dL9ubYC0HXa/RCrG9fznbfdPuUPwHGAkPD0ahJB7VYnU5Yo6PtQwfb/ObYmEDfcGR4lg +VQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=GG9IZeKUXqHbwgRlEUez+S6LOG7iNmAE27aQpf7wARQ=; b=D3cjzvFW9eFkUvJJRXMe1uv3m9wmV8d/SkPCNXSax4i4LCLC6Kz5KyHPX39eVSYGL8 K8T07oS7x2/FQS1FV/xWrG7O5LHuvNlHiPMt7owztPiBXcdvDu7HN62DRfaOzMhpeHyG YsLM926c9W5b/16QCt+sFwly9M1yTIsu4iZuz24cHn78AW4aSxuiuUQbtW1MIyyGfiX5 rcquMZTTHx0Ud9VqE7c1nPSti0kijj2Y/lOSjl5hpfJmIywXD8LcdDm8rfma0re1F9zb 0raWYqpNK7FJ1N5YEI+XsgKRhst/pVx85X79bsvBEtB2SbCozZJ6KezuOgJgargeGxA7 BfXA== X-Gm-Message-State: AOAM532hZuu/CibvZETmZgH9atW/zi6sw0K32NPVqGn4HRrr0WrbXaQ7 z5A/0TZOQcxOXlSps6y95s/S16sIJsPSOGN6 X-Google-Smtp-Source: ABdhPJxXzleMgy7efHrDCU1qKOzpEcVeClDkZTXS6ODGap4b87zAOAQznzyyx/BOUpKMQ3mjTanv4w== X-Received: by 2002:a6b:c881:: with SMTP id y123mr36840733iof.53.1638842098503; Mon, 06 Dec 2021 17:54:58 -0800 (PST) Received: from aford-IdeaCentre-A730.lan ([2601:448:8400:9e8:6db8:c9e4:6f6d:f38e]) by smtp.gmail.com with ESMTPSA id j21sm8623639ila.6.2021.12.06.17.54.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Dec 2021 17:54:58 -0800 (PST) From: Adam Ford <aford173@gmail.com> To: linux-media@vger.kernel.org Cc: cphealy@gmail.com, benjamin.gaignard@collabora.com, hverkuil@xs4all.nl, Adam Ford <aford173@gmail.com>, Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>, Philipp Zabel <p.zabel@pengutronix.de>, Mauro Carvalho Chehab <mchehab@kernel.org>, Rob Herring <robh+dt@kernel.org>, Shawn Guo <shawnguo@kernel.org>, Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix Kernel Team <kernel@pengutronix.de>, Fabio Estevam <festevam@gmail.com>, NXP Linux Team <linux-imx@nxp.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Lucas Stach <l.stach@pengutronix.de>, linux-rockchip@lists.infradead.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-staging@lists.linux.dev Subject: [RFC V2 0/6] media: Hantro: Split iMX8MQ VPU into G1 and G2 with blk-ctrl support Date: Mon, 6 Dec 2021 19:54:39 -0600 Message-Id: <20211207015446.1250854-1-aford173@gmail.com> X-Mailer: git-send-email 2.32.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-LSpam-Score: -2.5 (--) X-LSpam-Report: No, score=-2.5 required=5.0 tests=BAYES_00=-1.9,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,FREEMAIL_FORGED_FROMDOMAIN=0.001,FREEMAIL_FROM=0.001,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1 autolearn=ham autolearn_force=no |
Series |
media: Hantro: Split iMX8MQ VPU into G1 and G2 with blk-ctrl support
|
|
Message
Adam Ford
Dec. 7, 2021, 1:54 a.m. UTC
Currently, the VPU in the i.MQ8MQ is appearing as one codec, but in reality, it's two IP blocks called G1 and G2. There is initialization code in VPU code to pull some clocks, resets and other features which has been integrated into the vpu-blk-ctrl for the i.MX8M Mini and a similar method can be used to make the VPU codec's operate as stand-alone cores without having to know the details of each other or the quirks unique to the i.MX8MQ, so the remaining code can be left more generic. This series was started by Lucas Stach with one by Benjamin Gaignard. Most patches have been modified slightly by me. It's in an RFC state because I wasn't sure how to best handle the signatures and wasn't sure if I could base it off the branch I did. Since the g-streamer and media trees are in a constant state of change, this series is based on git://linuxtv.org/hverkuil/media_tree.git for-v5.17e The downstream code from NXP shows the G1 and G2 clocks running at 600MHz, but between the TRM and the datasheet, there is some discrepancy. Because the NXP reference code used 600MHz, that is what was chosen here. Users who need to adjust their G1 and G2 clocks can do so in their board files. Fluster Results: ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0 Ran 90/135 tests successfully in 61.966 secs ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0 Ran 55/61 tests successfully in 7.660 secs ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0 Ran 144/303 tests successfully in 162.665 secs Changes log: V2: Make vpu-blk-ctrl enable G2 clock when enabling fuses. Remove syscon from device tree and binding example Added modified nxp,imx8mq-vpu.yaml from Benjamin Gaignard Adam Ford (2): media: hantro: split i.MX8MQ G1 and G2 code arm64: dts: imx8mq: Split i.MX8MQ G1 and G2 with vpu-blk-ctrl Benjamin Gaignard (1): dt-bindings: media: nxp,imx8mq-vpu: Update the bindings for G2 support Lucas Stach (3): dt-bindings: power: imx8mq: add defines for VPU blk-ctrl domains dt-bindings: soc: add binding for i.MX8MQ VPU blk-ctrl soc: imx: imx8m-blk-ctrl: add i.MX8MQ VPU blk-ctrl .../bindings/media/nxp,imx8mq-vpu.yaml | 58 +++++---- .../soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml | 71 +++++++++++ arch/arm64/boot/dts/freescale/imx8mq.dtsi | 69 ++++++---- drivers/soc/imx/imx8m-blk-ctrl.c | 68 +++++++++- drivers/staging/media/hantro/hantro_drv.c | 4 +- drivers/staging/media/hantro/hantro_hw.h | 2 +- drivers/staging/media/hantro/imx8m_vpu_hw.c | 119 +++--------------- include/dt-bindings/power/imx8mq-power.h | 3 + 8 files changed, 237 insertions(+), 157 deletions(-) create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml base-commit: d1888b0bfd2ddef2e8a81505ffa200b92cc32e0c
Comments
Le 07/12/2021 à 02:54, Adam Ford a écrit : > Currently, the VPU in the i.MQ8MQ is appearing as one codec, but in > reality, it's two IP blocks called G1 and G2. There is initialization > code in VPU code to pull some clocks, resets and other features which > has been integrated into the vpu-blk-ctrl for the i.MX8M Mini and a > similar method can be used to make the VPU codec's operate as > stand-alone cores without having to know the details of each other > or the quirks unique to the i.MX8MQ, so the remaining code can be > left more generic. > > This series was started by Lucas Stach with one by Benjamin Gaignard. > Most patches have been modified slightly by me. It's in an RFC state > because I wasn't sure how to best handle the signatures and wasn't sure > if I could base it off the branch I did. > > Since the g-streamer and media trees are in a constant state of > change, this series is based on > > git://linuxtv.org/hverkuil/media_tree.git for-v5.17e > > The downstream code from NXP shows the G1 and G2 clocks running > at 600MHz, but between the TRM and the datasheet, there is some > discrepancy. Because the NXP reference code used 600MHz, that is > what was chosen here. Users who need to adjust their G1 and G2 > clocks can do so in their board files. Hi Adam, Thanks for your patches, I have been able to reproduce VP9 results on my side (Fluster 147/303). In past I have notice spurious errors when using 600MHz clock on HEVC decode but not with 300MHz. Regards, Benjamin > > Fluster Results: > > ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0 > Ran 90/135 tests successfully in 61.966 secs > > ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0 > Ran 55/61 tests successfully in 7.660 secs > > > ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0 > Ran 144/303 tests successfully in 162.665 secs > > Changes log: > > V2: Make vpu-blk-ctrl enable G2 clock when enabling fuses. > Remove syscon from device tree and binding example > Added modified nxp,imx8mq-vpu.yaml from Benjamin Gaignard > > Adam Ford (2): > media: hantro: split i.MX8MQ G1 and G2 code > arm64: dts: imx8mq: Split i.MX8MQ G1 and G2 with vpu-blk-ctrl > > Benjamin Gaignard (1): > dt-bindings: media: nxp,imx8mq-vpu: Update the bindings for G2 support > > Lucas Stach (3): > dt-bindings: power: imx8mq: add defines for VPU blk-ctrl domains > dt-bindings: soc: add binding for i.MX8MQ VPU blk-ctrl > soc: imx: imx8m-blk-ctrl: add i.MX8MQ VPU blk-ctrl > > .../bindings/media/nxp,imx8mq-vpu.yaml | 58 +++++---- > .../soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml | 71 +++++++++++ > arch/arm64/boot/dts/freescale/imx8mq.dtsi | 69 ++++++---- > drivers/soc/imx/imx8m-blk-ctrl.c | 68 +++++++++- > drivers/staging/media/hantro/hantro_drv.c | 4 +- > drivers/staging/media/hantro/hantro_hw.h | 2 +- > drivers/staging/media/hantro/imx8m_vpu_hw.c | 119 +++--------------- > include/dt-bindings/power/imx8mq-power.h | 3 + > 8 files changed, 237 insertions(+), 157 deletions(-) > create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml > > > base-commit: d1888b0bfd2ddef2e8a81505ffa200b92cc32e0c
Am Mittwoch, dem 08.12.2021 um 10:32 +0100 schrieb Benjamin Gaignard: > Le 07/12/2021 à 02:54, Adam Ford a écrit : > > > Currently, the VPU in the i.MQ8MQ is appearing as one codec, but in > > reality, it's two IP blocks called G1 and G2. There is initialization > > code in VPU code to pull some clocks, resets and other features which > > has been integrated into the vpu-blk-ctrl for the i.MX8M Mini and a > > similar method can be used to make the VPU codec's operate as > > stand-alone cores without having to know the details of each other > > or the quirks unique to the i.MX8MQ, so the remaining code can be > > left more generic. > > > > This series was started by Lucas Stach with one by Benjamin Gaignard. > > Most patches have been modified slightly by me. It's in an RFC state > > because I wasn't sure how to best handle the signatures and wasn't sure > > if I could base it off the branch I did. > > > > Since the g-streamer and media trees are in a constant state of > > change, this series is based on > > > > git://linuxtv.org/hverkuil/media_tree.git for-v5.17e > > > > The downstream code from NXP shows the G1 and G2 clocks running > > at 600MHz, but between the TRM and the datasheet, there is some > > discrepancy. Because the NXP reference code used 600MHz, that is > > what was chosen here. Users who need to adjust their G1 and G2 > > clocks can do so in their board files. > > Hi Adam, > > Thanks for your patches, I have been able to reproduce VP9 results on my side (Fluster 147/303). > In past I have notice spurious errors when using 600MHz clock on HEVC decode but not with 300MHz. The max supported G2 clock frequency is 660MHz but needs a higher voltage. The maximum supported frequency at the default 0.9V is 550MHz. We should not configure the clocks for the higher than that, as long as there is no support in the VPU driver to scale the voltage along with the frequency. Same as with the GPU we should stick to base frequency levels for the nominal operating mode. Regards, Lucas > > Regards, > Benjamin > > > > > Fluster Results: > > > > ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0 > > Ran 90/135 tests successfully in 61.966 secs > > > > ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0 > > Ran 55/61 tests successfully in 7.660 secs > > > > > > ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0 > > Ran 144/303 tests successfully in 162.665 secs > > > > Changes log: > > > > V2: Make vpu-blk-ctrl enable G2 clock when enabling fuses. > > Remove syscon from device tree and binding example > > Added modified nxp,imx8mq-vpu.yaml from Benjamin Gaignard > > > > Adam Ford (2): > > media: hantro: split i.MX8MQ G1 and G2 code > > arm64: dts: imx8mq: Split i.MX8MQ G1 and G2 with vpu-blk-ctrl > > > > Benjamin Gaignard (1): > > dt-bindings: media: nxp,imx8mq-vpu: Update the bindings for G2 support > > > > Lucas Stach (3): > > dt-bindings: power: imx8mq: add defines for VPU blk-ctrl domains > > dt-bindings: soc: add binding for i.MX8MQ VPU blk-ctrl > > soc: imx: imx8m-blk-ctrl: add i.MX8MQ VPU blk-ctrl > > > > .../bindings/media/nxp,imx8mq-vpu.yaml | 58 +++++---- > > .../soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml | 71 +++++++++++ > > arch/arm64/boot/dts/freescale/imx8mq.dtsi | 69 ++++++---- > > drivers/soc/imx/imx8m-blk-ctrl.c | 68 +++++++++- > > drivers/staging/media/hantro/hantro_drv.c | 4 +- > > drivers/staging/media/hantro/hantro_hw.h | 2 +- > > drivers/staging/media/hantro/imx8m_vpu_hw.c | 119 +++--------------- > > include/dt-bindings/power/imx8mq-power.h | 3 + > > 8 files changed, 237 insertions(+), 157 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml > > > > > > base-commit: d1888b0bfd2ddef2e8a81505ffa200b92cc32e0c
Le 08/12/2021 à 11:32, Lucas Stach a écrit : > Am Mittwoch, dem 08.12.2021 um 10:32 +0100 schrieb Benjamin Gaignard: >> Le 07/12/2021 à 02:54, Adam Ford a écrit : >> >>> Currently, the VPU in the i.MQ8MQ is appearing as one codec, but in >>> reality, it's two IP blocks called G1 and G2. There is initialization >>> code in VPU code to pull some clocks, resets and other features which >>> has been integrated into the vpu-blk-ctrl for the i.MX8M Mini and a >>> similar method can be used to make the VPU codec's operate as >>> stand-alone cores without having to know the details of each other >>> or the quirks unique to the i.MX8MQ, so the remaining code can be >>> left more generic. >>> >>> This series was started by Lucas Stach with one by Benjamin Gaignard. >>> Most patches have been modified slightly by me. It's in an RFC state >>> because I wasn't sure how to best handle the signatures and wasn't sure >>> if I could base it off the branch I did. >>> >>> Since the g-streamer and media trees are in a constant state of >>> change, this series is based on >>> >>> git://linuxtv.org/hverkuil/media_tree.git for-v5.17e >>> >>> The downstream code from NXP shows the G1 and G2 clocks running >>> at 600MHz, but between the TRM and the datasheet, there is some >>> discrepancy. Because the NXP reference code used 600MHz, that is >>> what was chosen here. Users who need to adjust their G1 and G2 >>> clocks can do so in their board files. >> Hi Adam, >> >> Thanks for your patches, I have been able to reproduce VP9 results on my side (Fluster 147/303). >> In past I have notice spurious errors when using 600MHz clock on HEVC decode but not with 300MHz. The results for Fluster HEVC are 77/147 so no regressions :-) Regards, Benjamin >> The max supported G2 clock frequency is 660MHz but needs a higher >> voltage. The maximum supported frequency at the default 0.9V is >> 550MHz. We should not configure the clocks for the higher than that, as >> long as there is no support in the VPU driver to scale the voltage >> along with the frequency. Same as with the GPU we should stick to base >> frequency levels for the nominal operating mode. >> >> Regards, >> Lucas >> >> Regards, >> Benjamin >> >>> Fluster Results: >>> >>> ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0 >>> Ran 90/135 tests successfully in 61.966 secs >>> >>> ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0 >>> Ran 55/61 tests successfully in 7.660 secs >>> >>> >>> ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0 >>> Ran 144/303 tests successfully in 162.665 secs >>> >>> Changes log: >>> >>> V2: Make vpu-blk-ctrl enable G2 clock when enabling fuses. >>> Remove syscon from device tree and binding example >>> Added modified nxp,imx8mq-vpu.yaml from Benjamin Gaignard >>> >>> Adam Ford (2): >>> media: hantro: split i.MX8MQ G1 and G2 code >>> arm64: dts: imx8mq: Split i.MX8MQ G1 and G2 with vpu-blk-ctrl >>> >>> Benjamin Gaignard (1): >>> dt-bindings: media: nxp,imx8mq-vpu: Update the bindings for G2 support >>> >>> Lucas Stach (3): >>> dt-bindings: power: imx8mq: add defines for VPU blk-ctrl domains >>> dt-bindings: soc: add binding for i.MX8MQ VPU blk-ctrl >>> soc: imx: imx8m-blk-ctrl: add i.MX8MQ VPU blk-ctrl >>> >>> .../bindings/media/nxp,imx8mq-vpu.yaml | 58 +++++---- >>> .../soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml | 71 +++++++++++ >>> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 69 ++++++---- >>> drivers/soc/imx/imx8m-blk-ctrl.c | 68 +++++++++- >>> drivers/staging/media/hantro/hantro_drv.c | 4 +- >>> drivers/staging/media/hantro/hantro_hw.h | 2 +- >>> drivers/staging/media/hantro/imx8m_vpu_hw.c | 119 +++--------------- >>> include/dt-bindings/power/imx8mq-power.h | 3 + >>> 8 files changed, 237 insertions(+), 157 deletions(-) >>> create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml >>> >>> >>> base-commit: d1888b0bfd2ddef2e8a81505ffa200b92cc32e0c >
On Wed, Dec 8, 2021 at 7:36 AM Benjamin Gaignard <benjamin.gaignard@collabora.com> wrote: > > > Le 08/12/2021 à 11:32, Lucas Stach a écrit : > > Am Mittwoch, dem 08.12.2021 um 10:32 +0100 schrieb Benjamin Gaignard: > >> Le 07/12/2021 à 02:54, Adam Ford a écrit : > >> > >>> Currently, the VPU in the i.MQ8MQ is appearing as one codec, but in > >>> reality, it's two IP blocks called G1 and G2. There is initialization > >>> code in VPU code to pull some clocks, resets and other features which > >>> has been integrated into the vpu-blk-ctrl for the i.MX8M Mini and a > >>> similar method can be used to make the VPU codec's operate as > >>> stand-alone cores without having to know the details of each other > >>> or the quirks unique to the i.MX8MQ, so the remaining code can be > >>> left more generic. > >>> > >>> This series was started by Lucas Stach with one by Benjamin Gaignard. > >>> Most patches have been modified slightly by me. It's in an RFC state > >>> because I wasn't sure how to best handle the signatures and wasn't sure > >>> if I could base it off the branch I did. > >>> > >>> Since the g-streamer and media trees are in a constant state of > >>> change, this series is based on > >>> > >>> git://linuxtv.org/hverkuil/media_tree.git for-v5.17e > >>> > >>> The downstream code from NXP shows the G1 and G2 clocks running > >>> at 600MHz, but between the TRM and the datasheet, there is some > >>> discrepancy. Because the NXP reference code used 600MHz, that is > >>> what was chosen here. Users who need to adjust their G1 and G2 > >>> clocks can do so in their board files. > >> Hi Adam, > >> > >> Thanks for your patches, I have been able to reproduce VP9 results on my side (Fluster 147/303). > >> In past I have notice spurious errors when using 600MHz clock on HEVC decode but not with 300MHz. > > The results for Fluster HEVC are 77/147 so no regressions :-) > > Regards, > Benjamin > > >> The max supported G2 clock frequency is 660MHz but needs a higher > >> voltage. The maximum supported frequency at the default 0.9V is > >> 550MHz. We should not configure the clocks for the higher than that, as > >> long as there is no support in the VPU driver to scale the voltage > >> along with the frequency. Same as with the GPU we should stick to base > >> frequency levels for the nominal operating mode. Lucas, After reviewing the comments from Benjamin, I re-ran the VP9 tests with the G2 running at 300MHz,and the number of passing VP9 tests increased to 148 from 144 with an increase of time to 250.502 secs from 162.665 secs. While the datasheet reads that the G2 can run faster, the i.MX 8M Dual/8M QuadLite/8M Quad Applications Processors Reference Manual, Rev. 3.1, 06/2021, table 5-1 shows the VPU_G2_CLK_ROOT has a max clock of 300MHz. I might be inclined to agree with Benjamin on the 300MHz and let people who want to push their hardware overwrite the default clocks since it increases functionality. I wonder if someone from NXP can comment adam > >> > >> Regards, > >> Lucas > >> > >> Regards, > >> Benjamin > >> > >>> Fluster Results: > >>> > >>> ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0 > >>> Ran 90/135 tests successfully in 61.966 secs > >>> > >>> ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0 > >>> Ran 55/61 tests successfully in 7.660 secs > >>> > >>> > >>> ./fluster.py run -d GStreamer-VP9-V4L2SL-Gst1.0 > >>> Ran 144/303 tests successfully in 162.665 secs > >>> > >>> Changes log: > >>> > >>> V2: Make vpu-blk-ctrl enable G2 clock when enabling fuses. > >>> Remove syscon from device tree and binding example > >>> Added modified nxp,imx8mq-vpu.yaml from Benjamin Gaignard > >>> > >>> Adam Ford (2): > >>> media: hantro: split i.MX8MQ G1 and G2 code > >>> arm64: dts: imx8mq: Split i.MX8MQ G1 and G2 with vpu-blk-ctrl > >>> > >>> Benjamin Gaignard (1): > >>> dt-bindings: media: nxp,imx8mq-vpu: Update the bindings for G2 support > >>> > >>> Lucas Stach (3): > >>> dt-bindings: power: imx8mq: add defines for VPU blk-ctrl domains > >>> dt-bindings: soc: add binding for i.MX8MQ VPU blk-ctrl > >>> soc: imx: imx8m-blk-ctrl: add i.MX8MQ VPU blk-ctrl > >>> > >>> .../bindings/media/nxp,imx8mq-vpu.yaml | 58 +++++---- > >>> .../soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml | 71 +++++++++++ > >>> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 69 ++++++---- > >>> drivers/soc/imx/imx8m-blk-ctrl.c | 68 +++++++++- > >>> drivers/staging/media/hantro/hantro_drv.c | 4 +- > >>> drivers/staging/media/hantro/hantro_hw.h | 2 +- > >>> drivers/staging/media/hantro/imx8m_vpu_hw.c | 119 +++--------------- > >>> include/dt-bindings/power/imx8mq-power.h | 3 + > >>> 8 files changed, 237 insertions(+), 157 deletions(-) > >>> create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mq-vpu-blk-ctrl.yaml > >>> > >>> > >>> base-commit: d1888b0bfd2ddef2e8a81505ffa200b92cc32e0c > >
Am Mittwoch, dem 08.12.2021 um 09:13 -0600 schrieb Adam Ford: > On Wed, Dec 8, 2021 at 7:36 AM Benjamin Gaignard > <benjamin.gaignard@collabora.com> wrote: > > > > > > Le 08/12/2021 à 11:32, Lucas Stach a écrit : > > > Am Mittwoch, dem 08.12.2021 um 10:32 +0100 schrieb Benjamin Gaignard: > > > > Le 07/12/2021 à 02:54, Adam Ford a écrit : > > > > > > > > > Currently, the VPU in the i.MQ8MQ is appearing as one codec, but in > > > > > reality, it's two IP blocks called G1 and G2. There is initialization > > > > > code in VPU code to pull some clocks, resets and other features which > > > > > has been integrated into the vpu-blk-ctrl for the i.MX8M Mini and a > > > > > similar method can be used to make the VPU codec's operate as > > > > > stand-alone cores without having to know the details of each other > > > > > or the quirks unique to the i.MX8MQ, so the remaining code can be > > > > > left more generic. > > > > > > > > > > This series was started by Lucas Stach with one by Benjamin Gaignard. > > > > > Most patches have been modified slightly by me. It's in an RFC state > > > > > because I wasn't sure how to best handle the signatures and wasn't sure > > > > > if I could base it off the branch I did. > > > > > > > > > > Since the g-streamer and media trees are in a constant state of > > > > > change, this series is based on > > > > > > > > > > git://linuxtv.org/hverkuil/media_tree.git for-v5.17e > > > > > > > > > > The downstream code from NXP shows the G1 and G2 clocks running > > > > > at 600MHz, but between the TRM and the datasheet, there is some > > > > > discrepancy. Because the NXP reference code used 600MHz, that is > > > > > what was chosen here. Users who need to adjust their G1 and G2 > > > > > clocks can do so in their board files. > > > > Hi Adam, > > > > > > > > Thanks for your patches, I have been able to reproduce VP9 results on my side (Fluster 147/303). > > > > In past I have notice spurious errors when using 600MHz clock on HEVC decode but not with 300MHz. > > > > The results for Fluster HEVC are 77/147 so no regressions :-) > > > > Regards, > > Benjamin > > > > > > The max supported G2 clock frequency is 660MHz but needs a higher > > > > voltage. The maximum supported frequency at the default 0.9V is > > > > 550MHz. We should not configure the clocks for the higher than that, as > > > > long as there is no support in the VPU driver to scale the voltage > > > > along with the frequency. Same as with the GPU we should stick to base > > > > frequency levels for the nominal operating mode. > > Lucas, > > After reviewing the comments from Benjamin, I re-ran the VP9 tests > with the G2 running at 300MHz,and the number of passing VP9 tests > increased to 148 from 144 with an increase of time to 250.502 secs > from 162.665 secs. > > While the datasheet reads that the G2 can run faster, the i.MX 8M > Dual/8M QuadLite/8M Quad Applications Processors Reference Manual, > Rev. 3.1, 06/2021, table 5-1 shows the VPU_G2_CLK_ROOT has a max > clock of 300MHz. I might be inclined to agree with Benjamin on the > 300MHz and let people who want to push their hardware overwrite the > default clocks since it increases functionality. > I'm quite surprised that the G2 codec should have such a much lower max frequency compared with the G1. While the table from the RM hasn't been obviously incorrect for any other clock, I'm still inclined to believe the frequencies stated in the datasheet. > I wonder if someone from NXP can comment > Yea, not sure how to proceed here. 300MHz is the safe bet, but we are leaving quite some performance on the table if the datasheet is correct. Without some help from NXP it's probably hard to validate which max frequency statement is correct. It doesn't really help that the NXP downstream kernel seems to drive the G2 at 600MHz, which is neither the 550MHz nominal mode max, nor the 660MHz overdrive mode max. Regards, Lucas
On Wed, Dec 8, 2021 at 7:39 AM Lucas Stach <l.stach@pengutronix.de> wrote: > > Am Mittwoch, dem 08.12.2021 um 09:13 -0600 schrieb Adam Ford: > > On Wed, Dec 8, 2021 at 7:36 AM Benjamin Gaignard > > <benjamin.gaignard@collabora.com> wrote: > > > > > > > > > Le 08/12/2021 à 11:32, Lucas Stach a écrit : > > > > Am Mittwoch, dem 08.12.2021 um 10:32 +0100 schrieb Benjamin Gaignard: > > > > > Le 07/12/2021 à 02:54, Adam Ford a écrit : > > > > > > > > > > > Currently, the VPU in the i.MQ8MQ is appearing as one codec, but in > > > > > > reality, it's two IP blocks called G1 and G2. There is initialization > > > > > > code in VPU code to pull some clocks, resets and other features which > > > > > > has been integrated into the vpu-blk-ctrl for the i.MX8M Mini and a > > > > > > similar method can be used to make the VPU codec's operate as > > > > > > stand-alone cores without having to know the details of each other > > > > > > or the quirks unique to the i.MX8MQ, so the remaining code can be > > > > > > left more generic. > > > > > > > > > > > > This series was started by Lucas Stach with one by Benjamin Gaignard. > > > > > > Most patches have been modified slightly by me. It's in an RFC state > > > > > > because I wasn't sure how to best handle the signatures and wasn't sure > > > > > > if I could base it off the branch I did. > > > > > > > > > > > > Since the g-streamer and media trees are in a constant state of > > > > > > change, this series is based on > > > > > > > > > > > > git://linuxtv.org/hverkuil/media_tree.git for-v5.17e > > > > > > > > > > > > The downstream code from NXP shows the G1 and G2 clocks running > > > > > > at 600MHz, but between the TRM and the datasheet, there is some > > > > > > discrepancy. Because the NXP reference code used 600MHz, that is > > > > > > what was chosen here. Users who need to adjust their G1 and G2 > > > > > > clocks can do so in their board files. > > > > > Hi Adam, > > > > > > > > > > Thanks for your patches, I have been able to reproduce VP9 results on my side (Fluster 147/303). > > > > > In past I have notice spurious errors when using 600MHz clock on HEVC decode but not with 300MHz. > > > > > > The results for Fluster HEVC are 77/147 so no regressions :-) > > > > > > Regards, > > > Benjamin > > > > > > > > The max supported G2 clock frequency is 660MHz but needs a higher > > > > > voltage. The maximum supported frequency at the default 0.9V is > > > > > 550MHz. We should not configure the clocks for the higher than that, as > > > > > long as there is no support in the VPU driver to scale the voltage > > > > > along with the frequency. Same as with the GPU we should stick to base > > > > > frequency levels for the nominal operating mode. > > > > Lucas, > > > > After reviewing the comments from Benjamin, I re-ran the VP9 tests > > with the G2 running at 300MHz,and the number of passing VP9 tests > > increased to 148 from 144 with an increase of time to 250.502 secs > > from 162.665 secs. > > > > While the datasheet reads that the G2 can run faster, the i.MX 8M > > Dual/8M QuadLite/8M Quad Applications Processors Reference Manual, > > Rev. 3.1, 06/2021, table 5-1 shows the VPU_G2_CLK_ROOT has a max > > clock of 300MHz. I might be inclined to agree with Benjamin on the > > 300MHz and let people who want to push their hardware overwrite the > > default clocks since it increases functionality. > > > I'm quite surprised that the G2 codec should have such a much lower max > frequency compared with the G1. While the table from the RM hasn't been > obviously incorrect for any other clock, I'm still inclined to believe > the frequencies stated in the datasheet. > > > I wonder if someone from NXP can comment > > > Yea, not sure how to proceed here. 300MHz is the safe bet, but we are > leaving quite some performance on the table if the datasheet is > correct. Without some help from NXP it's probably hard to validate > which max frequency statement is correct. It doesn't really help that > the NXP downstream kernel seems to drive the G2 at 600MHz, which is > neither the 550MHz nominal mode max, nor the 660MHz overdrive mode max. According to the NXP i.MX8MQ docs, "The G2 decoder conforms to the HEVC Main/Main 10 profiles and can decode streams up to level 5.1." If I'm reading the levels correctly, level 5.1 = 4,096×2,160@60.0. If the decoder can achieve this throughput at 300MHz, then there should be no reason to go higher? My speculation though is that when run at 300MHz, the decoder is not able to decode this fast. Adam, can you determine the throughput at the different frequencies to see if 300MHz can support 4K60 decode? > > Regards, > Lucas >