Date: Thu, 21 Dec 2023 14:46:56 +0100
From: Alex Bee <knaerzche@...il.com>
To: Adam Ford <aford173@...il.com>
Cc: Hugues FRUCHET <hugues.fruchet@...s.st.com>,
 Nicolas Dufresne <nicolas.dufresne@...labora.com>,
 Marco Felsch <m.felsch@...gutronix.de>,
 Philipp Zabel <p.zabel@...gutronix.de>,
 Andrzej Pietrasiewicz <andrzej.p@...labora.com>,
 Sakari Ailus <sakari.ailus@...ux.intel.com>,
 Benjamin Gaignard <benjamin.gaignard@...labora.com>,
 Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>,
 Benjamin Mugnier <benjamin.mugnier@...s.st.com>,
 Mauro Carvalho Chehab <mchehab@...nel.org>, linux-kernel@...r.kernel.org,
 Daniel Almeida <daniel.almeida@...labora.com>,
 Heiko Stuebner <heiko@...ech.de>, Hans Verkuil <hverkuil@...all.nl>,
 Rob Herring <robh+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 linux-stm32@...md-mailman.stormreply.com,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
 devicetree@...r.kernel.org, Maxime Coquelin <mcoquelin.stm32@...il.com>,
 linux-media@...r.kernel.org, Alexandre Torgue
 <alexandre.torgue@...s.st.com>,
 Ezequiel Garcia <ezequiel@...guardiasur.com.ar>
Subject: Re: [PATCH v5 0/5] Add support for video hardware codec of
 STMicroelectronics STM32 SoC series


Am 21.12.23 um 14:38 schrieb Adam Ford:
> On Thu, Dec 21, 2023 at 7:31 AM Alex Bee <knaerzche@...il.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
>>>>>

Powered by blists - more mailing lists