Message ID | 20231221084723.2152034-1-hugues.fruchet@foss.st.com (mailing list archive) |
---|---|
Headers |
Received: from sv.mirrors.kernel.org ([139.178.88.99]) by www.linuxtv.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from <linux-media+bounces-2838-patchwork=linuxtv.org@vger.kernel.org>) id 1rGEjb-001p5Z-Kn for patchwork@linuxtv.org; Thu, 21 Dec 2023 08:48:52 +0000 Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id C469C28758F for <patchwork@linuxtv.org>; Thu, 21 Dec 2023 08:48:47 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id E25932206A; Thu, 21 Dec 2023 08:48:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=foss.st.com header.i=@foss.st.com header.b="cxN0BXhC" X-Original-To: linux-media@vger.kernel.org Received: from mx07-00178001.pphosted.com (mx08-00178001.pphosted.com [91.207.212.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F23A01A70A; Thu, 21 Dec 2023 08:48:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=foss.st.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=foss.st.com Received: from pps.filterd (m0046661.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.17.1.22/8.17.1.22) with ESMTP id 3BL24G1s012155; Thu, 21 Dec 2023 09:47:28 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foss.st.com; h= from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding:content-type; s=selector1; bh=jeRT+eK 7Vhl0LiS2NYUaPuSsz9Ja8QWgixI33nTKj3w=; b=cxN0BXhC6kFkwbBZlqXoMQG AMpG8S781Rpd6LlI084zqO6s9DQLxjy3ttzt/j9GgY48m8+ATwmJZIPSOj+vcwyw FhCuS3s+J3FK8DAvXPraZdQdY7aY/0sWMagGXXWK0fjmezQiakF7ha+iUIK5DRrT J9TJQlaEoQauvcUA5d8pybCEFfxBWdq17iCI/YrZ4IOauHi7G5ofHQ/y+AN6LwAu AHs3pDHp1WcX2QRe6Y9KLQ2M1bh559xNIIvg4+J4B77nPAk/DPn57JCBa1nxPy/V 4LbeRIgnTxqT47Uw6jTfdPD+8I4qIyLaH1DMoAkTZAJzLXZFJzkwuv/ogPmIKJQ= = Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by mx07-00178001.pphosted.com (PPS) with ESMTPS id 3v13nhnxu6-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 21 Dec 2023 09:47:28 +0100 (CET) Received: from euls16034.sgp.st.com (euls16034.sgp.st.com [10.75.44.20]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 82424100066; Thu, 21 Dec 2023 09:47:25 +0100 (CET) Received: from Webmail-eu.st.com (shfdag1node1.st.com [10.75.129.69]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id 72D1B22A6D2; Thu, 21 Dec 2023 09:47:25 +0100 (CET) Received: from localhost (10.201.20.120) by SHFDAG1NODE1.st.com (10.75.129.69) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27; Thu, 21 Dec 2023 09:47:25 +0100 From: Hugues Fruchet <hugues.fruchet@foss.st.com> To: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>, Philipp Zabel <p.zabel@pengutronix.de>, Andrzej Pietrasiewicz <andrzej.p@collabora.com>, Nicolas Dufresne <nicolas.dufresne@collabora.com>, Sakari Ailus <sakari.ailus@linux.intel.com>, Benjamin Gaignard <benjamin.gaignard@collabora.com>, Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>, Daniel Almeida <daniel.almeida@collabora.com>, Benjamin Mugnier <benjamin.mugnier@foss.st.com>, Heiko Stuebner <heiko@sntech.de>, Mauro Carvalho Chehab <mchehab@kernel.org>, Hans Verkuil <hverkuil@xs4all.nl>, <linux-media@vger.kernel.org>, Maxime Coquelin <mcoquelin.stm32@gmail.com>, Alexandre Torgue <alexandre.torgue@foss.st.com>, <linux-stm32@st-md-mailman.stormreply.com>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, <devicetree@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linux-kernel@vger.kernel.org>, <linux-rockchip@lists.infradead.org> CC: Hugues Fruchet <hugues.fruchet@foss.st.com>, Marco Felsch <m.felsch@pengutronix.de>, Adam Ford <aford173@gmail.com> Subject: [PATCH v5 0/5] Add support for video hardware codec of STMicroelectronics STM32 SoC series Date: Thu, 21 Dec 2023 09:47:18 +0100 Message-ID: <20231221084723.2152034-1-hugues.fruchet@foss.st.com> X-Mailer: git-send-email 2.25.1 Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: <linux-media.vger.kernel.org> List-Subscribe: <mailto:linux-media+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-media+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-ClientProxiedBy: EQNCAS1NODE3.st.com (10.75.129.80) To SHFDAG1NODE1.st.com (10.75.129.69) X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-12-21_04,2023-12-20_01,2023-05-22_02 X-LSpam-Score: -4.8 (----) X-LSpam-Report: No, score=-4.8 required=5.0 tests=BAYES_00=-1.9,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1,RCVD_IN_DNSWL_MED=-2.3 autolearn=ham autolearn_force=no |
Series |
Add support for video hardware codec of STMicroelectronics STM32 SoC series
|
|
Message
Hugues Fruchet
Dec. 21, 2023, 8:47 a.m. UTC
This patchset introduces support for VDEC video hardware decoder and VENC video hardware encoder of STMicroelectronics STM32MP25 SoC series. This initial support implements H264 decoding, VP8 decoding and JPEG encoding. This has been tested on STM32MP257F-EV1 evaluation board. =========== = history = =========== version 5: - Precise that video decoding as been successfully tested up to full HD - Add Nicolas Dufresne reviewed-by version 4: - Fix comments from Nicolas about dropping encoder raw steps version 3: - Fix remarks from Krzysztof Kozlowski: - drop "items", we keep simple enum in such case - drop second example - it is the same as the first - Drop unused node labels as suggested by Conor Dooley - Revisit min/max resolutions as suggested by Nicolas Dufresne version 2: - Fix remarks from Krzysztof Kozlowski on v1: - single video-codec binding for both VDEC/VENC - get rid of "-names" - use of generic node name "video-codec" version 1: - Initial submission Hugues Fruchet (5): dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs media: hantro: add support for STM32MP25 VDEC media: hantro: add support for STM32MP25 VENC arm64: dts: st: add video decoder support to stm32mp255 arm64: dts: st: add video encoder support to stm32mp255 .../media/st,stm32mp25-video-codec.yaml | 50 ++++++++ arch/arm64/boot/dts/st/stm32mp251.dtsi | 12 ++ arch/arm64/boot/dts/st/stm32mp255.dtsi | 17 +++ drivers/media/platform/verisilicon/Kconfig | 14 ++- drivers/media/platform/verisilicon/Makefile | 4 + .../media/platform/verisilicon/hantro_drv.c | 4 + .../media/platform/verisilicon/hantro_hw.h | 2 + .../platform/verisilicon/stm32mp25_vdec_hw.c | 92 ++++++++++++++ .../platform/verisilicon/stm32mp25_venc_hw.c | 115 ++++++++++++++++++ 9 files changed, 307 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml create mode 100644 drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c create mode 100644 drivers/media/platform/verisilicon/stm32mp25_venc_hw.c
Comments
Hi Hugues, Hi Nicolas, is there any specific reason I'm not understanding / seeing why this is added in two seperate vdec* / venc* files and not a single vpu* file? Is it only for the seperate clocks (-names) / irqs (-names) / callbacks? Those are defined per variant and perfectly fit in a single file holding one vdec and one venc variant. Alex Am 21.12.23 um 09:47 schrieb Hugues Fruchet: > This patchset introduces support for VDEC video hardware decoder > and VENC video hardware encoder of STMicroelectronics STM32MP25 > SoC series. > > This initial support implements H264 decoding, VP8 decoding and > JPEG encoding. > > This has been tested on STM32MP257F-EV1 evaluation board. > > =========== > = history = > =========== > version 5: > - Precise that video decoding as been successfully tested up to full HD > - Add Nicolas Dufresne reviewed-by > > version 4: > - Fix comments from Nicolas about dropping encoder raw steps > > version 3: > - Fix remarks from Krzysztof Kozlowski: > - drop "items", we keep simple enum in such case > - drop second example - it is the same as the first > - Drop unused node labels as suggested by Conor Dooley > - Revisit min/max resolutions as suggested by Nicolas Dufresne > > version 2: > - Fix remarks from Krzysztof Kozlowski on v1: > - single video-codec binding for both VDEC/VENC > - get rid of "-names" > - use of generic node name "video-codec" > > version 1: > - Initial submission > > Hugues Fruchet (5): > dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs > media: hantro: add support for STM32MP25 VDEC > media: hantro: add support for STM32MP25 VENC > arm64: dts: st: add video decoder support to stm32mp255 > arm64: dts: st: add video encoder support to stm32mp255 > > .../media/st,stm32mp25-video-codec.yaml | 50 ++++++++ > arch/arm64/boot/dts/st/stm32mp251.dtsi | 12 ++ > arch/arm64/boot/dts/st/stm32mp255.dtsi | 17 +++ > drivers/media/platform/verisilicon/Kconfig | 14 ++- > drivers/media/platform/verisilicon/Makefile | 4 + > .../media/platform/verisilicon/hantro_drv.c | 4 + > .../media/platform/verisilicon/hantro_hw.h | 2 + > .../platform/verisilicon/stm32mp25_vdec_hw.c | 92 ++++++++++++++ > .../platform/verisilicon/stm32mp25_venc_hw.c | 115 ++++++++++++++++++ > 9 files changed, 307 insertions(+), 3 deletions(-) > create mode 100644 Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml > create mode 100644 drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c > create mode 100644 drivers/media/platform/verisilicon/stm32mp25_venc_hw.c >
Hi Alex, This is because VDEC and VENC are two separated IPs with their own hardware resources and no links between both. On future SoCs, VDEC can ship on its own, same for VENC. Hoping that this clarify. Best regards, Hugues. On 12/21/23 13:40, Alex Bee wrote: > Hi Hugues, Hi Nicolas, > > is there any specific reason I'm not understanding / seeing why this is > added in two seperate vdec* / venc* files and not a single vpu* file? Is > it only for the seperate clocks (-names) / irqs (-names) / callbacks? > Those are defined per variant and perfectly fit in a single file holding > one vdec and one venc variant. > > Alex > > Am 21.12.23 um 09:47 schrieb Hugues Fruchet: >> This patchset introduces support for VDEC video hardware decoder >> and VENC video hardware encoder of STMicroelectronics STM32MP25 >> SoC series. >> >> This initial support implements H264 decoding, VP8 decoding and >> JPEG encoding. >> >> This has been tested on STM32MP257F-EV1 evaluation board. >> >> =========== >> = history = >> =========== >> version 5: >> - Precise that video decoding as been successfully tested up to >> full HD >> - Add Nicolas Dufresne reviewed-by >> >> version 4: >> - Fix comments from Nicolas about dropping encoder raw steps >> >> version 3: >> - Fix remarks from Krzysztof Kozlowski: >> - drop "items", we keep simple enum in such case >> - drop second example - it is the same as the first >> - Drop unused node labels as suggested by Conor Dooley >> - Revisit min/max resolutions as suggested by Nicolas Dufresne >> >> version 2: >> - Fix remarks from Krzysztof Kozlowski on v1: >> - single video-codec binding for both VDEC/VENC >> - get rid of "-names" >> - use of generic node name "video-codec" >> >> version 1: >> - Initial submission >> >> Hugues Fruchet (5): >> dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs >> media: hantro: add support for STM32MP25 VDEC >> media: hantro: add support for STM32MP25 VENC >> arm64: dts: st: add video decoder support to stm32mp255 >> arm64: dts: st: add video encoder support to stm32mp255 >> >> .../media/st,stm32mp25-video-codec.yaml | 50 ++++++++ >> arch/arm64/boot/dts/st/stm32mp251.dtsi | 12 ++ >> arch/arm64/boot/dts/st/stm32mp255.dtsi | 17 +++ >> drivers/media/platform/verisilicon/Kconfig | 14 ++- >> drivers/media/platform/verisilicon/Makefile | 4 + >> .../media/platform/verisilicon/hantro_drv.c | 4 + >> .../media/platform/verisilicon/hantro_hw.h | 2 + >> .../platform/verisilicon/stm32mp25_vdec_hw.c | 92 ++++++++++++++ >> .../platform/verisilicon/stm32mp25_venc_hw.c | 115 ++++++++++++++++++ >> 9 files changed, 307 insertions(+), 3 deletions(-) >> create mode 100644 >> Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml >> create mode 100644 >> drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c >> create mode 100644 >> drivers/media/platform/verisilicon/stm32mp25_venc_hw.c >> >
Hi Hugues, Am 21.12.23 um 14:08 schrieb Hugues FRUCHET: > Hi Alex, > > This is because VDEC and VENC are two separated IPs with their own > hardware resources and no links between both. > On future SoCs, VDEC can ship on its own, same for VENC. > I think that's what the driver is/was designed for :) I don't think there _has_ to be a link between variants in the same file. For Rockchip we only had the issue that there _is_ a link (shared resources) between encoder and decoder and they had (for that reason) to be defined has a _single_ variant. And there is no reason you can ship decoder and encoder seperated when you have two variants (with different compatibles). For Rockchip and iMX those files are even containing variants for completly different generations / different SoCs. I had to cleanup this mess for Rockchip once - and it was no fun :) Anyways: It's up to the maintainers I guess - I just wanted to ask if I missunderstand something here. Greetings, Alex > Hoping that this clarify. > > Best regards, > Hugues. > > On 12/21/23 13:40, Alex Bee wrote: >> Hi Hugues, Hi Nicolas, >> >> is there any specific reason I'm not understanding / seeing why this >> is added in two seperate vdec* / venc* files and not a single vpu* >> file? Is it only for the seperate clocks (-names) / irqs (-names) / >> callbacks? Those are defined per variant and perfectly fit in a >> single file holding one vdec and one venc variant. >> >> Alex >> >> Am 21.12.23 um 09:47 schrieb Hugues Fruchet: >>> This patchset introduces support for VDEC video hardware decoder >>> and VENC video hardware encoder of STMicroelectronics STM32MP25 >>> SoC series. >>> >>> This initial support implements H264 decoding, VP8 decoding and >>> JPEG encoding. >>> >>> This has been tested on STM32MP257F-EV1 evaluation board. >>> >>> =========== >>> = history = >>> =========== >>> version 5: >>> - Precise that video decoding as been successfully tested up to >>> full HD >>> - Add Nicolas Dufresne reviewed-by >>> >>> version 4: >>> - Fix comments from Nicolas about dropping encoder raw steps >>> >>> version 3: >>> - Fix remarks from Krzysztof Kozlowski: >>> - drop "items", we keep simple enum in such case >>> - drop second example - it is the same as the first >>> - Drop unused node labels as suggested by Conor Dooley >>> - Revisit min/max resolutions as suggested by Nicolas Dufresne >>> >>> version 2: >>> - Fix remarks from Krzysztof Kozlowski on v1: >>> - single video-codec binding for both VDEC/VENC >>> - get rid of "-names" >>> - use of generic node name "video-codec" >>> >>> version 1: >>> - Initial submission >>> >>> Hugues Fruchet (5): >>> dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs >>> media: hantro: add support for STM32MP25 VDEC >>> media: hantro: add support for STM32MP25 VENC >>> arm64: dts: st: add video decoder support to stm32mp255 >>> arm64: dts: st: add video encoder support to stm32mp255 >>> >>> .../media/st,stm32mp25-video-codec.yaml | 50 ++++++++ >>> arch/arm64/boot/dts/st/stm32mp251.dtsi | 12 ++ >>> arch/arm64/boot/dts/st/stm32mp255.dtsi | 17 +++ >>> drivers/media/platform/verisilicon/Kconfig | 14 ++- >>> drivers/media/platform/verisilicon/Makefile | 4 + >>> .../media/platform/verisilicon/hantro_drv.c | 4 + >>> .../media/platform/verisilicon/hantro_hw.h | 2 + >>> .../platform/verisilicon/stm32mp25_vdec_hw.c | 92 ++++++++++++++ >>> .../platform/verisilicon/stm32mp25_venc_hw.c | 115 >>> ++++++++++++++++++ >>> 9 files changed, 307 insertions(+), 3 deletions(-) >>> create mode 100644 >>> Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml >>> create mode 100644 >>> drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c >>> create mode 100644 >>> drivers/media/platform/verisilicon/stm32mp25_venc_hw.c >>> >>
On Thu, Dec 21, 2023 at 7:31 AM Alex Bee <knaerzche@gmail.com> wrote: > > Hi Hugues, > > Am 21.12.23 um 14:08 schrieb Hugues FRUCHET: > > Hi Alex, > > > > This is because VDEC and VENC are two separated IPs with their own > > hardware resources and no links between both. > > On future SoCs, VDEC can ship on its own, same for VENC. > > > I think that's what the driver is/was designed for :) > > I don't think there _has_ to be a link between variants in the same file. > For Rockchip we only had the issue that there _is_ a link (shared > resources) between encoder and decoder and they had (for that reason) to be > defined has a _single_ variant. And there is no reason you can ship decoder > and encoder seperated when you have two variants (with different > compatibles). > For Rockchip and iMX those files are even containing variants for completly > different generations / different SoCs. I had to cleanup this mess for The i.MX8M Mini and Plus have different power domains for encoder and decoders as well as different clocks. Keeping them separate would almost be necessary. adam > Rockchip once - and it was no fun :) Anyways: It's up to the maintainers I > guess - I just wanted to ask if I missunderstand something here. > > Greetings, > > Alex > > > Hoping that this clarify. > > > > Best regards, > > Hugues. > > > > On 12/21/23 13:40, Alex Bee wrote: > >> Hi Hugues, Hi Nicolas, > >> > >> is there any specific reason I'm not understanding / seeing why this > >> is added in two seperate vdec* / venc* files and not a single vpu* > >> file? Is it only for the seperate clocks (-names) / irqs (-names) / > >> callbacks? Those are defined per variant and perfectly fit in a > >> single file holding one vdec and one venc variant. > >> > >> Alex > >> > >> Am 21.12.23 um 09:47 schrieb Hugues Fruchet: > >>> This patchset introduces support for VDEC video hardware decoder > >>> and VENC video hardware encoder of STMicroelectronics STM32MP25 > >>> SoC series. > >>> > >>> This initial support implements H264 decoding, VP8 decoding and > >>> JPEG encoding. > >>> > >>> This has been tested on STM32MP257F-EV1 evaluation board. > >>> > >>> =========== > >>> = history = > >>> =========== > >>> version 5: > >>> - Precise that video decoding as been successfully tested up to > >>> full HD > >>> - Add Nicolas Dufresne reviewed-by > >>> > >>> version 4: > >>> - Fix comments from Nicolas about dropping encoder raw steps > >>> > >>> version 3: > >>> - Fix remarks from Krzysztof Kozlowski: > >>> - drop "items", we keep simple enum in such case > >>> - drop second example - it is the same as the first > >>> - Drop unused node labels as suggested by Conor Dooley > >>> - Revisit min/max resolutions as suggested by Nicolas Dufresne > >>> > >>> version 2: > >>> - Fix remarks from Krzysztof Kozlowski on v1: > >>> - single video-codec binding for both VDEC/VENC > >>> - get rid of "-names" > >>> - use of generic node name "video-codec" > >>> > >>> version 1: > >>> - Initial submission > >>> > >>> Hugues Fruchet (5): > >>> dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs > >>> media: hantro: add support for STM32MP25 VDEC > >>> media: hantro: add support for STM32MP25 VENC > >>> arm64: dts: st: add video decoder support to stm32mp255 > >>> arm64: dts: st: add video encoder support to stm32mp255 > >>> > >>> .../media/st,stm32mp25-video-codec.yaml | 50 ++++++++ > >>> arch/arm64/boot/dts/st/stm32mp251.dtsi | 12 ++ > >>> arch/arm64/boot/dts/st/stm32mp255.dtsi | 17 +++ > >>> drivers/media/platform/verisilicon/Kconfig | 14 ++- > >>> drivers/media/platform/verisilicon/Makefile | 4 + > >>> .../media/platform/verisilicon/hantro_drv.c | 4 + > >>> .../media/platform/verisilicon/hantro_hw.h | 2 + > >>> .../platform/verisilicon/stm32mp25_vdec_hw.c | 92 ++++++++++++++ > >>> .../platform/verisilicon/stm32mp25_venc_hw.c | 115 > >>> ++++++++++++++++++ > >>> 9 files changed, 307 insertions(+), 3 deletions(-) > >>> create mode 100644 > >>> Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml > >>> create mode 100644 > >>> drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c > >>> create mode 100644 > >>> drivers/media/platform/verisilicon/stm32mp25_venc_hw.c > >>> > >>
Am 21.12.23 um 14:38 schrieb Adam Ford: > On Thu, Dec 21, 2023 at 7:31 AM Alex Bee <knaerzche@gmail.com> wrote: >> Hi Hugues, >> >> Am 21.12.23 um 14:08 schrieb Hugues FRUCHET: >>> Hi Alex, >>> >>> This is because VDEC and VENC are two separated IPs with their own >>> hardware resources and no links between both. >>> On future SoCs, VDEC can ship on its own, same for VENC. >>> >> I think that's what the driver is/was designed for :) >> >> I don't think there _has_ to be a link between variants in the same file. >> For Rockchip we only had the issue that there _is_ a link (shared >> resources) between encoder and decoder and they had (for that reason) to be >> defined has a _single_ variant. And there is no reason you can ship decoder >> and encoder seperated when you have two variants (with different >> compatibles). >> For Rockchip and iMX those files are even containing variants for completly >> different generations / different SoCs. I had to cleanup this mess for > The i.MX8M Mini and Plus have different power domains for encoder and > decoders as well as different clocks. Keeping them separate would > almost be necessary. I guess there is missunderstanding: I didn't say the two STM variants should be merged in one variant, but the two variants should be within the same _file_, like the other platforms are doing :) > adam > >> Rockchip once - and it was no fun :) Anyways: It's up to the maintainers I >> guess - I just wanted to ask if I missunderstand something here. >> >> Greetings, >> >> Alex >> >>> Hoping that this clarify. >>> >>> Best regards, >>> Hugues. >>> >>> On 12/21/23 13:40, Alex Bee wrote: >>>> Hi Hugues, Hi Nicolas, >>>> >>>> is there any specific reason I'm not understanding / seeing why this >>>> is added in two seperate vdec* / venc* files and not a single vpu* >>>> file? Is it only for the seperate clocks (-names) / irqs (-names) / >>>> callbacks? Those are defined per variant and perfectly fit in a >>>> single file holding one vdec and one venc variant. >>>> >>>> Alex >>>> >>>> Am 21.12.23 um 09:47 schrieb Hugues Fruchet: >>>>> This patchset introduces support for VDEC video hardware decoder >>>>> and VENC video hardware encoder of STMicroelectronics STM32MP25 >>>>> SoC series. >>>>> >>>>> This initial support implements H264 decoding, VP8 decoding and >>>>> JPEG encoding. >>>>> >>>>> This has been tested on STM32MP257F-EV1 evaluation board. >>>>> >>>>> =========== >>>>> = history = >>>>> =========== >>>>> version 5: >>>>> - Precise that video decoding as been successfully tested up to >>>>> full HD >>>>> - Add Nicolas Dufresne reviewed-by >>>>> >>>>> version 4: >>>>> - Fix comments from Nicolas about dropping encoder raw steps >>>>> >>>>> version 3: >>>>> - Fix remarks from Krzysztof Kozlowski: >>>>> - drop "items", we keep simple enum in such case >>>>> - drop second example - it is the same as the first >>>>> - Drop unused node labels as suggested by Conor Dooley >>>>> - Revisit min/max resolutions as suggested by Nicolas Dufresne >>>>> >>>>> version 2: >>>>> - Fix remarks from Krzysztof Kozlowski on v1: >>>>> - single video-codec binding for both VDEC/VENC >>>>> - get rid of "-names" >>>>> - use of generic node name "video-codec" >>>>> >>>>> version 1: >>>>> - Initial submission >>>>> >>>>> Hugues Fruchet (5): >>>>> dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs >>>>> media: hantro: add support for STM32MP25 VDEC >>>>> media: hantro: add support for STM32MP25 VENC >>>>> arm64: dts: st: add video decoder support to stm32mp255 >>>>> arm64: dts: st: add video encoder support to stm32mp255 >>>>> >>>>> .../media/st,stm32mp25-video-codec.yaml | 50 ++++++++ >>>>> arch/arm64/boot/dts/st/stm32mp251.dtsi | 12 ++ >>>>> arch/arm64/boot/dts/st/stm32mp255.dtsi | 17 +++ >>>>> drivers/media/platform/verisilicon/Kconfig | 14 ++- >>>>> drivers/media/platform/verisilicon/Makefile | 4 + >>>>> .../media/platform/verisilicon/hantro_drv.c | 4 + >>>>> .../media/platform/verisilicon/hantro_hw.h | 2 + >>>>> .../platform/verisilicon/stm32mp25_vdec_hw.c | 92 ++++++++++++++ >>>>> .../platform/verisilicon/stm32mp25_venc_hw.c | 115 >>>>> ++++++++++++++++++ >>>>> 9 files changed, 307 insertions(+), 3 deletions(-) >>>>> create mode 100644 >>>>> Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml >>>>> create mode 100644 >>>>> drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c >>>>> create mode 100644 >>>>> drivers/media/platform/verisilicon/stm32mp25_venc_hw.c >>>>>
Hi Alex, On 12/21/23 14:46, Alex Bee wrote: > > Am 21.12.23 um 14:38 schrieb Adam Ford: >> On Thu, Dec 21, 2023 at 7:31 AM Alex Bee <knaerzche@gmail.com> wrote: >>> Hi Hugues, >>> >>> Am 21.12.23 um 14:08 schrieb Hugues FRUCHET: >>>> Hi Alex, >>>> >>>> This is because VDEC and VENC are two separated IPs with their own >>>> hardware resources and no links between both. >>>> On future SoCs, VDEC can ship on its own, same for VENC. >>>> >>> I think that's what the driver is/was designed for :) >>> >>> I don't think there _has_ to be a link between variants in the same >>> file. >>> For Rockchip we only had the issue that there _is_ a link (shared >>> resources) between encoder and decoder and they had (for that reason) >>> to be >>> defined has a _single_ variant. And there is no reason you can ship >>> decoder >>> and encoder seperated when you have two variants (with different >>> compatibles). >>> For Rockchip and iMX those files are even containing variants for >>> completly >>> different generations / different SoCs. I had to cleanup this mess for >> The i.MX8M Mini and Plus have different power domains for encoder and >> decoders as well as different clocks. Keeping them separate would >> almost be necessary. > I guess there is missunderstanding: I didn't say the two STM variants > should be merged in one variant, but the two variants should be within the > same _file_, like the other platforms are doing :) I have two separated hardware: VDEC and VENC, not a single block like "VPU" for example. So what name should have this file ? Other platforms had a common file because there was a common block embedding both decoder and encoder, sometimes with links/dependencies between both. SAMA5D4 has only a decoder, only a single file called "_vdec_hw.c"... so it is quite logical for me to have one file per independent IP. >> adam >> >>> Rockchip once - and it was no fun :) Anyways: It's up to the >>> maintainers I >>> guess - I just wanted to ask if I missunderstand something here. >>> >>> Greetings, >>> >>> Alex >>> >>>> Hoping that this clarify. >>>> >>>> Best regards, >>>> Hugues. >>>> >>>> On 12/21/23 13:40, Alex Bee wrote: >>>>> Hi Hugues, Hi Nicolas, >>>>> >>>>> is there any specific reason I'm not understanding / seeing why this >>>>> is added in two seperate vdec* / venc* files and not a single vpu* >>>>> file? Is it only for the seperate clocks (-names) / irqs (-names) / >>>>> callbacks? Those are defined per variant and perfectly fit in a >>>>> single file holding one vdec and one venc variant. >>>>> >>>>> Alex >>>>> >>>>> Am 21.12.23 um 09:47 schrieb Hugues Fruchet: >>>>>> This patchset introduces support for VDEC video hardware decoder >>>>>> and VENC video hardware encoder of STMicroelectronics STM32MP25 >>>>>> SoC series. >>>>>> >>>>>> This initial support implements H264 decoding, VP8 decoding and >>>>>> JPEG encoding. >>>>>> >>>>>> This has been tested on STM32MP257F-EV1 evaluation board. >>>>>> >>>>>> =========== >>>>>> = history = >>>>>> =========== >>>>>> version 5: >>>>>> - Precise that video decoding as been successfully tested up to >>>>>> full HD >>>>>> - Add Nicolas Dufresne reviewed-by >>>>>> >>>>>> version 4: >>>>>> - Fix comments from Nicolas about dropping encoder raw steps >>>>>> >>>>>> version 3: >>>>>> - Fix remarks from Krzysztof Kozlowski: >>>>>> - drop "items", we keep simple enum in such case >>>>>> - drop second example - it is the same as the first >>>>>> - Drop unused node labels as suggested by Conor Dooley >>>>>> - Revisit min/max resolutions as suggested by Nicolas Dufresne >>>>>> >>>>>> version 2: >>>>>> - Fix remarks from Krzysztof Kozlowski on v1: >>>>>> - single video-codec binding for both VDEC/VENC >>>>>> - get rid of "-names" >>>>>> - use of generic node name "video-codec" >>>>>> >>>>>> version 1: >>>>>> - Initial submission >>>>>> >>>>>> Hugues Fruchet (5): >>>>>> dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs >>>>>> media: hantro: add support for STM32MP25 VDEC >>>>>> media: hantro: add support for STM32MP25 VENC >>>>>> arm64: dts: st: add video decoder support to stm32mp255 >>>>>> arm64: dts: st: add video encoder support to stm32mp255 >>>>>> >>>>>> .../media/st,stm32mp25-video-codec.yaml | 50 ++++++++ >>>>>> arch/arm64/boot/dts/st/stm32mp251.dtsi | 12 ++ >>>>>> arch/arm64/boot/dts/st/stm32mp255.dtsi | 17 +++ >>>>>> drivers/media/platform/verisilicon/Kconfig | 14 ++- >>>>>> drivers/media/platform/verisilicon/Makefile | 4 + >>>>>> .../media/platform/verisilicon/hantro_drv.c | 4 + >>>>>> .../media/platform/verisilicon/hantro_hw.h | 2 + >>>>>> .../platform/verisilicon/stm32mp25_vdec_hw.c | 92 ++++++++++++++ >>>>>> .../platform/verisilicon/stm32mp25_venc_hw.c | 115 >>>>>> ++++++++++++++++++ >>>>>> 9 files changed, 307 insertions(+), 3 deletions(-) >>>>>> create mode 100644 >>>>>> Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml >>>>>> create mode 100644 >>>>>> drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c >>>>>> create mode 100644 >>>>>> drivers/media/platform/verisilicon/stm32mp25_venc_hw.c >>>>>> Best regards, Hugues.
Hi Hugues, Am 21.12.23 um 14:55 schrieb Hugues FRUCHET: > Hi Alex, > > On 12/21/23 14:46, Alex Bee wrote: >> >> Am 21.12.23 um 14:38 schrieb Adam Ford: >>> On Thu, Dec 21, 2023 at 7:31 AM Alex Bee <knaerzche@gmail.com> wrote: >>>> Hi Hugues, >>>> >>>> Am 21.12.23 um 14:08 schrieb Hugues FRUCHET: >>>>> Hi Alex, >>>>> >>>>> This is because VDEC and VENC are two separated IPs with their own >>>>> hardware resources and no links between both. >>>>> On future SoCs, VDEC can ship on its own, same for VENC. >>>>> >>>> I think that's what the driver is/was designed for :) >>>> >>>> I don't think there _has_ to be a link between variants in the >>>> same file. >>>> For Rockchip we only had the issue that there _is_ a link (shared >>>> resources) between encoder and decoder and they had (for that >>>> reason) to be >>>> defined has a _single_ variant. And there is no reason you can ship >>>> decoder >>>> and encoder seperated when you have two variants (with different >>>> compatibles). >>>> For Rockchip and iMX those files are even containing variants for >>>> completly >>>> different generations / different SoCs. I had to cleanup this mess for >>> The i.MX8M Mini and Plus have different power domains for encoder and >>> decoders as well as different clocks. Keeping them separate would >>> almost be necessary. >> I guess there is missunderstanding: I didn't say the two STM variants >> should be merged in one variant, but the two variants should be >> within the >> same _file_, like the other platforms are doing :) > > I have two separated hardware: VDEC and VENC, not a single block like > "VPU" for example. So what name should have this file ? > Other platforms had a common file because there was a common block > embedding both decoder and encoder, sometimes with links/dependencies > between both. > SAMA5D4 has only a decoder, only a single file called "_vdec_hw.c"... > so it is quite logical for me to have one file per independent IP. > Maybe - but that's not way the driver is currently organzied. rockchip_vpu_hw.c also holds variants which have only have a decoder and also some which only have a encoder. So "vpu" is quite neutral, I guess. It doesn't say anything if it belongs to encoder or decoder. When I was adding the RK3066 variant a I was even asked to add a explicit comment, why this integration can't be splitted in encoder and decoder variant. We were having a a lot of these split-ups in the early days of hantro driver and it was no fun to clean them up. Alex >>> adam >>> >>>> Rockchip once - and it was no fun :) Anyways: It's up to the >>>> maintainers I >>>> guess - I just wanted to ask if I missunderstand something here. >>>> >>>> Greetings, >>>> >>>> Alex >>>> >>>>> Hoping that this clarify. >>>>> >>>>> Best regards, >>>>> Hugues. >>>>> >>>>> On 12/21/23 13:40, Alex Bee wrote: >>>>>> Hi Hugues, Hi Nicolas, >>>>>> >>>>>> is there any specific reason I'm not understanding / seeing why this >>>>>> is added in two seperate vdec* / venc* files and not a single vpu* >>>>>> file? Is it only for the seperate clocks (-names) / irqs (-names) / >>>>>> callbacks? Those are defined per variant and perfectly fit in a >>>>>> single file holding one vdec and one venc variant. >>>>>> >>>>>> Alex >>>>>> >>>>>> Am 21.12.23 um 09:47 schrieb Hugues Fruchet: >>>>>>> This patchset introduces support for VDEC video hardware decoder >>>>>>> and VENC video hardware encoder of STMicroelectronics STM32MP25 >>>>>>> SoC series. >>>>>>> >>>>>>> This initial support implements H264 decoding, VP8 decoding and >>>>>>> JPEG encoding. >>>>>>> >>>>>>> This has been tested on STM32MP257F-EV1 evaluation board. >>>>>>> >>>>>>> =========== >>>>>>> = history = >>>>>>> =========== >>>>>>> version 5: >>>>>>> - Precise that video decoding as been successfully tested >>>>>>> up to >>>>>>> full HD >>>>>>> - Add Nicolas Dufresne reviewed-by >>>>>>> >>>>>>> version 4: >>>>>>> - Fix comments from Nicolas about dropping encoder raw steps >>>>>>> >>>>>>> version 3: >>>>>>> - Fix remarks from Krzysztof Kozlowski: >>>>>>> - drop "items", we keep simple enum in such case >>>>>>> - drop second example - it is the same as the first >>>>>>> - Drop unused node labels as suggested by Conor Dooley >>>>>>> - Revisit min/max resolutions as suggested by Nicolas Dufresne >>>>>>> >>>>>>> version 2: >>>>>>> - Fix remarks from Krzysztof Kozlowski on v1: >>>>>>> - single video-codec binding for both VDEC/VENC >>>>>>> - get rid of "-names" >>>>>>> - use of generic node name "video-codec" >>>>>>> >>>>>>> version 1: >>>>>>> - Initial submission >>>>>>> >>>>>>> Hugues Fruchet (5): >>>>>>> dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs >>>>>>> media: hantro: add support for STM32MP25 VDEC >>>>>>> media: hantro: add support for STM32MP25 VENC >>>>>>> arm64: dts: st: add video decoder support to stm32mp255 >>>>>>> arm64: dts: st: add video encoder support to stm32mp255 >>>>>>> >>>>>>> .../media/st,stm32mp25-video-codec.yaml | 50 ++++++++ >>>>>>> arch/arm64/boot/dts/st/stm32mp251.dtsi | 12 ++ >>>>>>> arch/arm64/boot/dts/st/stm32mp255.dtsi | 17 +++ >>>>>>> drivers/media/platform/verisilicon/Kconfig | 14 ++- >>>>>>> drivers/media/platform/verisilicon/Makefile | 4 + >>>>>>> .../media/platform/verisilicon/hantro_drv.c | 4 + >>>>>>> .../media/platform/verisilicon/hantro_hw.h | 2 + >>>>>>> .../platform/verisilicon/stm32mp25_vdec_hw.c | 92 >>>>>>> ++++++++++++++ >>>>>>> .../platform/verisilicon/stm32mp25_venc_hw.c | 115 >>>>>>> ++++++++++++++++++ >>>>>>> 9 files changed, 307 insertions(+), 3 deletions(-) >>>>>>> create mode 100644 >>>>>>> Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml >>>>>>> >>>>>>> create mode 100644 >>>>>>> drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c >>>>>>> create mode 100644 >>>>>>> drivers/media/platform/verisilicon/stm32mp25_venc_hw.c >>>>>>> > > Best regards, > Hugues.
Hi Alex, v6 sent with all the variants in a single file. Best regards, Hugues. On 12/21/23 14:31, Alex Bee wrote: > Hi Hugues, > > Am 21.12.23 um 14:08 schrieb Hugues FRUCHET: >> Hi Alex, >> >> This is because VDEC and VENC are two separated IPs with their own >> hardware resources and no links between both. >> On future SoCs, VDEC can ship on its own, same for VENC. >> > I think that's what the driver is/was designed for :) > > I don't think there _has_ to be a link between variants in the same file. > For Rockchip we only had the issue that there _is_ a link (shared > resources) between encoder and decoder and they had (for that reason) to be > defined has a _single_ variant. And there is no reason you can ship decoder > and encoder seperated when you have two variants (with different > compatibles). > For Rockchip and iMX those files are even containing variants for completly > different generations / different SoCs. I had to cleanup this mess for > Rockchip once - and it was no fun :) Anyways: It's up to the maintainers I > guess - I just wanted to ask if I missunderstand something here. > > Greetings, > > Alex > >> Hoping that this clarify. >> >> Best regards, >> Hugues. >> >> On 12/21/23 13:40, Alex Bee wrote: >>> Hi Hugues, Hi Nicolas, >>> >>> is there any specific reason I'm not understanding / seeing why this >>> is added in two seperate vdec* / venc* files and not a single vpu* >>> file? Is it only for the seperate clocks (-names) / irqs (-names) / >>> callbacks? Those are defined per variant and perfectly fit in a >>> single file holding one vdec and one venc variant. >>> >>> Alex >>> >>> Am 21.12.23 um 09:47 schrieb Hugues Fruchet: >>>> This patchset introduces support for VDEC video hardware decoder >>>> and VENC video hardware encoder of STMicroelectronics STM32MP25 >>>> SoC series. >>>> >>>> This initial support implements H264 decoding, VP8 decoding and >>>> JPEG encoding. >>>> >>>> This has been tested on STM32MP257F-EV1 evaluation board. >>>> >>>> =========== >>>> = history = >>>> =========== >>>> version 5: >>>> - Precise that video decoding as been successfully tested up to >>>> full HD >>>> - Add Nicolas Dufresne reviewed-by >>>> >>>> version 4: >>>> - Fix comments from Nicolas about dropping encoder raw steps >>>> >>>> version 3: >>>> - Fix remarks from Krzysztof Kozlowski: >>>> - drop "items", we keep simple enum in such case >>>> - drop second example - it is the same as the first >>>> - Drop unused node labels as suggested by Conor Dooley >>>> - Revisit min/max resolutions as suggested by Nicolas Dufresne >>>> >>>> version 2: >>>> - Fix remarks from Krzysztof Kozlowski on v1: >>>> - single video-codec binding for both VDEC/VENC >>>> - get rid of "-names" >>>> - use of generic node name "video-codec" >>>> >>>> version 1: >>>> - Initial submission >>>> >>>> Hugues Fruchet (5): >>>> dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs >>>> media: hantro: add support for STM32MP25 VDEC >>>> media: hantro: add support for STM32MP25 VENC >>>> arm64: dts: st: add video decoder support to stm32mp255 >>>> arm64: dts: st: add video encoder support to stm32mp255 >>>> >>>> .../media/st,stm32mp25-video-codec.yaml | 50 ++++++++ >>>> arch/arm64/boot/dts/st/stm32mp251.dtsi | 12 ++ >>>> arch/arm64/boot/dts/st/stm32mp255.dtsi | 17 +++ >>>> drivers/media/platform/verisilicon/Kconfig | 14 ++- >>>> drivers/media/platform/verisilicon/Makefile | 4 + >>>> .../media/platform/verisilicon/hantro_drv.c | 4 + >>>> .../media/platform/verisilicon/hantro_hw.h | 2 + >>>> .../platform/verisilicon/stm32mp25_vdec_hw.c | 92 ++++++++++++++ >>>> .../platform/verisilicon/stm32mp25_venc_hw.c | 115 >>>> ++++++++++++++++++ >>>> 9 files changed, 307 insertions(+), 3 deletions(-) >>>> create mode 100644 >>>> Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml >>>> create mode 100644 >>>> drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c >>>> create mode 100644 >>>> drivers/media/platform/verisilicon/stm32mp25_venc_hw.c >>>> >>>