Messages in this thread | ![/](/images/icornerl.gif) | | Date | Thu, 21 Dec 2023 14:55:54 +0100 | Subject | Re: [PATCH v5 0/5] Add support for video hardware codec of STMicroelectronics STM32 SoC series | From | 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.
>> 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.
| ![\](/images/icornerr.gif) |