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@xxxxxxxxx> 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.
