Re: [PATCH v5 0/5] Add support for video hardware codec of STMicroelectronics STM32 SoC series

[Alex Bee <knaerzche@gmail.com>]

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
>>>>>