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

[Hugues FRUCHET <hugues.fruchet@foss.st.com>]

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