linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH 1/4] dt-bindings: iommu: rockchip: Fix rk3588 variant
       [not found] ` <20240320173736.2720778-2-linkmauve@linkmauve.fr>
@ 2024-03-20 19:15   ` Sebastian Reichel
  2024-03-21  8:14   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 11+ messages in thread
From: Sebastian Reichel @ 2024-03-20 19:15 UTC (permalink / raw)
  To: Emmanuel Gil Peyrot
  Cc: linux-kernel, Ezequiel Garcia, Philipp Zabel,
	Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Heiko Stuebner, Joerg Roedel, Will Deacon,
	Robin Murphy, Cristian Ciocaltea, Dragan Simic, Shreeya Patel,
	Chris Morgan, Andy Yan, Nicolas Frattaroli, linux-media,
	linux-rockchip, devicetree, linux-arm-kernel, iommu

[-- Attachment #1: Type: text/plain, Size: 2175 bytes --]

Hello Emmanuel,

On Wed, Mar 20, 2024 at 06:37:30PM +0100, Emmanuel Gil Peyrot wrote:
> The documentation got added in f8aa519976b38e67aae02d2db3e2998513305e80,
> but it hasn’t been added to the driver so it was unused.
> 
> Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
> ---

Everything is fine with f8aa519976b38e67aae02d2db3e2998513305e80
(well the patch description could be better :)) and this patch is
just wrong. The documentation explicitly adds the combination of
rk3588-iommu with rk3568-iommu as fallback. The idea is, that the
driver handles it just like an rk3568 iommu. If some differences
are found in the future, the driver can switch to handle the more
specific compatible without any DT changes (which is ABI).

I suggest watching this presentation:

https://www.youtube.com/watch?v=6iguKSJJfxo

Greetings,

-- Sebastian

>  arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 2 +-
>  drivers/iommu/rockchip-iommu.c            | 3 +++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> index 87b83c87bd55..2a23b4dc36e4 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> @@ -704,7 +704,7 @@ vp3: port@3 {
>  	};
>  
>  	vop_mmu: iommu@fdd97e00 {
> -		compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
> +		compatible = "rockchip,rk3588-iommu";
>  		reg = <0x0 0xfdd97e00 0x0 0x100>, <0x0 0xfdd97f00 0x0 0x100>;
>  		interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH 0>;
>  		clocks = <&cru ACLK_VOP>, <&cru HCLK_VOP>;
> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index da79d9f4cf63..da0e93c139d1 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -1361,6 +1361,9 @@ static const struct of_device_id rk_iommu_dt_ids[] = {
>  	{	.compatible = "rockchip,rk3568-iommu",
>  		.data = &iommu_data_ops_v2,
>  	},
> +	{	.compatible = "rockchip,rk3588-iommu",
> +		.data = &iommu_data_ops_v2,
> +	},
>  	{ /* sentinel */ }
>  };
>  
> -- 
> 2.44.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] media: dt-binding: media: Document rk3588’s vepu121
       [not found] ` <20240320173736.2720778-3-linkmauve@linkmauve.fr>
@ 2024-03-20 20:16   ` Sebastian Reichel
  2024-03-21  8:14   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 11+ messages in thread
From: Sebastian Reichel @ 2024-03-20 20:16 UTC (permalink / raw)
  To: Emmanuel Gil Peyrot
  Cc: linux-kernel, Ezequiel Garcia, Philipp Zabel,
	Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Heiko Stuebner, Joerg Roedel, Will Deacon,
	Robin Murphy, Cristian Ciocaltea, Dragan Simic, Shreeya Patel,
	Chris Morgan, Andy Yan, Nicolas Frattaroli, linux-media,
	linux-rockchip, devicetree, linux-arm-kernel, iommu

[-- Attachment #1: Type: text/plain, Size: 1444 bytes --]

Hi,

On Wed, Mar 20, 2024 at 06:37:31PM +0100, Emmanuel Gil Peyrot wrote:
> This encoder-only device is present four times on this SoC, and should
> support everything the rk3568 vepu supports (so JPEG, H.264 and VP8
> encoding).
> 
> Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
> ---
>  .../devicetree/bindings/media/rockchip,rk3568-vepu.yaml          | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml b/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
> index 9d90d8d0565a..947ad699cc5e 100644
> --- a/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
> +++ b/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
> @@ -17,6 +17,7 @@ properties:
>    compatible:
>      enum:
>        - rockchip,rk3568-vepu
> +      - rockchip,rk3588-vepu121

Looks like they are fully compatible. In that case it's better to
use a fallback compatible (i.e. like the iommu binding), which does
not need any drivers changes. So binding should be like this:

compatible:
  oneOf:
    - const: rockchip,rk3568-vepu
    - items:
      - enum:
          - rockchip,rk3588-vepu121
      - const: rockchip,rk3568-vepu

Then in DT (i.e. the following patch) you use

compatible = "rockchip,rk3588-vepu121", "rockchip,rk3568-vepu";

And patch 4/4 can be dropped.

Greetings,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4] dt-bindings: iommu: rockchip: Fix rk3588 variant
       [not found] ` <20240320173736.2720778-2-linkmauve@linkmauve.fr>
  2024-03-20 19:15   ` [PATCH 1/4] dt-bindings: iommu: rockchip: Fix rk3588 variant Sebastian Reichel
@ 2024-03-21  8:14   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2024-03-21  8:14 UTC (permalink / raw)
  To: Emmanuel Gil Peyrot, linux-kernel
  Cc: Ezequiel Garcia, Philipp Zabel, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
	Joerg Roedel, Will Deacon, Robin Murphy, Sebastian Reichel,
	Cristian Ciocaltea, Dragan Simic, Shreeya Patel, Chris Morgan,
	Andy Yan, Nicolas Frattaroli, linux-media, linux-rockchip,
	devicetree, linux-arm-kernel, iommu

On 20/03/2024 18:37, Emmanuel Gil Peyrot wrote:
> The documentation got added in f8aa519976b38e67aae02d2db3e2998513305e80,

Please use commit SHA () syntax (see submitting patches).

> but it hasn’t been added to the driver so it was unused.

Eh? That's not how this works.

> 
> Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
> ---
>  arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 2 +-

That's DTS, not bindings.

>  drivers/iommu/rockchip-iommu.c            | 3 +++

Driver code cannot be combined with DTS in one patch.

>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> index 87b83c87bd55..2a23b4dc36e4 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> @@ -704,7 +704,7 @@ vp3: port@3 {
>  	};
>  
>  	vop_mmu: iommu@fdd97e00 {
> -		compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
> +		compatible = "rockchip,rk3588-iommu";

NAK.


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] media: dt-binding: media: Document rk3588’s vepu121
       [not found] ` <20240320173736.2720778-3-linkmauve@linkmauve.fr>
  2024-03-20 20:16   ` [PATCH 2/4] media: dt-binding: media: Document rk3588’s vepu121 Sebastian Reichel
@ 2024-03-21  8:14   ` Krzysztof Kozlowski
  2024-03-21  8:47     ` Heiko Stübner
  1 sibling, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2024-03-21  8:14 UTC (permalink / raw)
  To: Emmanuel Gil Peyrot, linux-kernel
  Cc: Ezequiel Garcia, Philipp Zabel, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
	Joerg Roedel, Will Deacon, Robin Murphy, Sebastian Reichel,
	Cristian Ciocaltea, Dragan Simic, Shreeya Patel, Chris Morgan,
	Andy Yan, Nicolas Frattaroli, linux-media, linux-rockchip,
	devicetree, linux-arm-kernel, iommu

On 20/03/2024 18:37, Emmanuel Gil Peyrot wrote:
> This encoder-only device is present four times on this SoC, and should
> support everything the rk3568 vepu supports (so JPEG, H.264 and VP8
> encoding).
> 
> Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
> ---
>  .../devicetree/bindings/media/rockchip,rk3568-vepu.yaml          | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml b/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
> index 9d90d8d0565a..947ad699cc5e 100644
> --- a/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
> +++ b/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
> @@ -17,6 +17,7 @@ properties:
>    compatible:
>      enum:
>        - rockchip,rk3568-vepu
> +      - rockchip,rk3588-vepu121

What is 121?

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] arm64: dts: rockchip: Add VEPU121 to rk3588
       [not found] ` <20240320173736.2720778-4-linkmauve@linkmauve.fr>
@ 2024-03-21  8:15   ` Krzysztof Kozlowski
  2024-03-27 12:40     ` Link Mauve
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2024-03-21  8:15 UTC (permalink / raw)
  To: Emmanuel Gil Peyrot, linux-kernel
  Cc: Ezequiel Garcia, Philipp Zabel, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
	Joerg Roedel, Will Deacon, Robin Murphy, Sebastian Reichel,
	Cristian Ciocaltea, Dragan Simic, Shreeya Patel, Chris Morgan,
	Andy Yan, Nicolas Frattaroli, linux-media, linux-rockchip,
	devicetree, linux-arm-kernel, iommu

On 20/03/2024 18:37, Emmanuel Gil Peyrot wrote:
> The TRM (version 1.0 page 385) lists five VEPU121 cores, but only four
> interrupts are listed (on page 24), so I’ve only enabled four of them
> for now.
> 
> Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
> ---
>  arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 80 +++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> index 2a23b4dc36e4..fe77b56ac9a0 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> @@ -2488,6 +2488,86 @@ gpio4: gpio@fec50000 {
>  		};
>  	};
>  
> +	jpeg_enc0: video-codec@fdba0000 {
> +		compatible = "rockchip,rk3588-vepu121";
> +		reg = <0x0 0xfdba0000 0x0 0x800>;
> +		interrupts = <GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH 0>;
> +		clocks = <&cru ACLK_JPEG_ENCODER0>, <&cru HCLK_JPEG_ENCODER0>;
> +		clock-names = "aclk", "hclk";
> +		iommus = <&jpeg_enc0_mmu>;
> +		power-domains = <&power RK3588_PD_VDPU>;
> +	};
> +
> +	jpeg_enc0_mmu: iommu@fdba0800 {
> +		compatible = "rockchip,rk3588-iommu";

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] media: dt-binding: media: Document rk3588’s vepu121
  2024-03-21  8:14   ` Krzysztof Kozlowski
@ 2024-03-21  8:47     ` Heiko Stübner
  2024-03-21  9:19       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 11+ messages in thread
From: Heiko Stübner @ 2024-03-21  8:47 UTC (permalink / raw)
  To: Emmanuel Gil Peyrot, linux-kernel, Krzysztof Kozlowski
  Cc: Ezequiel Garcia, Philipp Zabel, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joerg Roedel,
	Will Deacon, Robin Murphy, Sebastian Reichel, Cristian Ciocaltea,
	Dragan Simic, Shreeya Patel, Chris Morgan, Andy Yan,
	Nicolas Frattaroli, linux-media, linux-rockchip, devicetree,
	linux-arm-kernel, iommu

Am Donnerstag, 21. März 2024, 09:14:51 CET schrieb Krzysztof Kozlowski:
> On 20/03/2024 18:37, Emmanuel Gil Peyrot wrote:
> > This encoder-only device is present four times on this SoC, and should
> > support everything the rk3568 vepu supports (so JPEG, H.264 and VP8
> > encoding).
> > 
> > Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
> > ---
> >  .../devicetree/bindings/media/rockchip,rk3568-vepu.yaml          | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml b/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
> > index 9d90d8d0565a..947ad699cc5e 100644
> > --- a/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
> > +++ b/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
> > @@ -17,6 +17,7 @@ properties:
> >    compatible:
> >      enum:
> >        - rockchip,rk3568-vepu
> > +      - rockchip,rk3588-vepu121
> 
> What is 121?

That is the strange naming of the ip block inside the soc.

I.e. the rk3588 TRM lists a number of different video encoders and decoders:
- VDPU121 is decoding h.263 and mpeg1,2,4
- VDPU381 is decoding h.265, h.264 and some more
- VDPU720 is decoding jpeg
- VDPU981 decodes AV1
- VEPU121 is the jpeg encoder above
- VEPU580 encodes h.264 and h.265

Each of those are separate IP blocks with their own io-memory, their own
interrupts and their own iommus, etc.


Heiko



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] media: dt-binding: media: Document rk3588’s vepu121
  2024-03-21  8:47     ` Heiko Stübner
@ 2024-03-21  9:19       ` Krzysztof Kozlowski
  2024-03-21  9:32         ` Heiko Stübner
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2024-03-21  9:19 UTC (permalink / raw)
  To: Heiko Stübner, Emmanuel Gil Peyrot, linux-kernel
  Cc: Ezequiel Garcia, Philipp Zabel, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joerg Roedel,
	Will Deacon, Robin Murphy, Sebastian Reichel, Cristian Ciocaltea,
	Dragan Simic, Shreeya Patel, Chris Morgan, Andy Yan,
	Nicolas Frattaroli, linux-media, linux-rockchip, devicetree,
	linux-arm-kernel, iommu

On 21/03/2024 09:47, Heiko Stübner wrote:
>>>      enum:
>>>        - rockchip,rk3568-vepu
>>> +      - rockchip,rk3588-vepu121
>>
>> What is 121?
> 
> That is the strange naming of the ip block inside the soc.
> 
> I.e. the rk3588 TRM lists a number of different video encoders and decoders:
> - VDPU121 is decoding h.263 and mpeg1,2,4
> - VDPU381 is decoding h.265, h.264 and some more
> - VDPU720 is decoding jpeg
> - VDPU981 decodes AV1
> - VEPU121 is the jpeg encoder above
> - VEPU580 encodes h.264 and h.265
> 
> Each of those are separate IP blocks with their own io-memory, their own
> interrupts and their own iommus, etc.

Thanks for explanation. Short introduction in commit msg would be nice
(e.g. VEPU121, one of two VEPU encoders). OTOH, why not documenting all
of them? Bindings are supposed to be as complete as possible.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] media: dt-binding: media: Document rk3588’s vepu121
  2024-03-21  9:19       ` Krzysztof Kozlowski
@ 2024-03-21  9:32         ` Heiko Stübner
  2024-03-22 14:57           ` Nicolas Dufresne
  0 siblings, 1 reply; 11+ messages in thread
From: Heiko Stübner @ 2024-03-21  9:32 UTC (permalink / raw)
  To: Emmanuel Gil Peyrot, linux-kernel, Krzysztof Kozlowski
  Cc: Ezequiel Garcia, Philipp Zabel, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joerg Roedel,
	Will Deacon, Robin Murphy, Sebastian Reichel, Cristian Ciocaltea,
	Dragan Simic, Shreeya Patel, Chris Morgan, Andy Yan,
	Nicolas Frattaroli, linux-media, linux-rockchip, devicetree,
	linux-arm-kernel, iommu

Am Donnerstag, 21. März 2024, 10:19:54 CET schrieb Krzysztof Kozlowski:
> On 21/03/2024 09:47, Heiko Stübner wrote:
> >>>      enum:
> >>>        - rockchip,rk3568-vepu
> >>> +      - rockchip,rk3588-vepu121
> >>
> >> What is 121?
> > 
> > That is the strange naming of the ip block inside the soc.
> > 
> > I.e. the rk3588 TRM lists a number of different video encoders and decoders:
> > - VDPU121 is decoding h.263 and mpeg1,2,4
> > - VDPU381 is decoding h.265, h.264 and some more
> > - VDPU720 is decoding jpeg
> > - VDPU981 decodes AV1
> > - VEPU121 is the jpeg encoder above
> > - VEPU580 encodes h.264 and h.265
> > 
> > Each of those are separate IP blocks with their own io-memory, their own
> > interrupts and their own iommus, etc.
> 
> Thanks for explanation. Short introduction in commit msg would be nice
> (e.g. VEPU121, one of two VEPU encoders). OTOH, why not documenting all
> of them? Bindings are supposed to be as complete as possible.

We have a concurrent series for the vdpu121 running at
  https://lore.kernel.org/all/20240316071100.2419369-1-liujianfeng1994@gmail.com

I think not all of those encoders/decoders are based on the Hantro IP
or at least at the moment we don't know this yet.
Hence people adding compatibles for the blocks they have actually
managed to run on their hardware.

Bindings are supposed to be as complete as possible, but revising a wrong
binding later is very hard. And the whole media part is full of binary libraries
in the vendor kernel and has not the best documentation.

So I guess people are just cautious ;-)


Heiko



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] media: dt-binding: media: Document rk3588’s vepu121
  2024-03-21  9:32         ` Heiko Stübner
@ 2024-03-22 14:57           ` Nicolas Dufresne
  0 siblings, 0 replies; 11+ messages in thread
From: Nicolas Dufresne @ 2024-03-22 14:57 UTC (permalink / raw)
  To: Heiko Stübner, Emmanuel Gil Peyrot, linux-kernel,
	Krzysztof Kozlowski
  Cc: Ezequiel Garcia, Philipp Zabel, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joerg Roedel,
	Will Deacon, Robin Murphy, Sebastian Reichel, Cristian Ciocaltea,
	Dragan Simic, Shreeya Patel, Chris Morgan, Andy Yan,
	Nicolas Frattaroli, linux-media, linux-rockchip, devicetree,
	linux-arm-kernel, iommu

Le jeudi 21 mars 2024 à 10:32 +0100, Heiko Stübner a écrit :
> Am Donnerstag, 21. März 2024, 10:19:54 CET schrieb Krzysztof Kozlowski:
> > On 21/03/2024 09:47, Heiko Stübner wrote:
> > > > >      enum:
> > > > >        - rockchip,rk3568-vepu
> > > > > +      - rockchip,rk3588-vepu121
> > > > 
> > > > What is 121?
> > > 
> > > That is the strange naming of the ip block inside the soc.
> > > 
> > > I.e. the rk3588 TRM lists a number of different video encoders and decoders:
> > > - VDPU121 is decoding h.263 and mpeg1,2,4
> > > - VDPU381 is decoding h.265, h.264 and some more
> > > - VDPU720 is decoding jpeg
> > > - VDPU981 decodes AV1
> > > - VEPU121 is the jpeg encoder above
> > > - VEPU580 encodes h.264 and h.265
> > > 
> > > Each of those are separate IP blocks with their own io-memory, their own
> > > interrupts and their own iommus, etc.
> > 
> > Thanks for explanation. Short introduction in commit msg would be nice
> > (e.g. VEPU121, one of two VEPU encoders). OTOH, why not documenting all
> > of them? Bindings are supposed to be as complete as possible.
> 
> We have a concurrent series for the vdpu121 running at
>   https://lore.kernel.org/all/20240316071100.2419369-1-liujianfeng1994@gmail.com
> 
> I think not all of those encoders/decoders are based on the Hantro IP
> or at least at the moment we don't know this yet.
> Hence people adding compatibles for the blocks they have actually
> managed to run on their hardware.

Correct, on top of this legacy core, only VDPU981 (AV1 decode) is a chip from
Hantro / Verisilicon. Everything else looks like their own design and derived
from their original rkvdec codec pack. We didn't name this AV1 decoder after VSI
brand as we didn't have the proper version information available, but we suspect
that is variant of their VSI9000D cores. In short, we tried to avoid documenting
our speculation, even if we believe we are right.

> 
> Bindings are supposed to be as complete as possible, but revising a wrong
> binding later is very hard. And the whole media part is full of binary libraries
> in the vendor kernel and has not the best documentation.

I agree with this, but I must give to RK that despite the lack of documentation,
their CODEC software is fully open-source and blob free on this platform.

> 
> So I guess people are just cautious ;-)
> 

exactly!

Nicolas


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] arm64: dts: rockchip: Add VEPU121 to rk3588
  2024-03-21  8:15   ` [PATCH 3/4] arm64: dts: rockchip: Add VEPU121 to rk3588 Krzysztof Kozlowski
@ 2024-03-27 12:40     ` Link Mauve
  2024-03-27 13:09       ` Sebastian Reichel
  0 siblings, 1 reply; 11+ messages in thread
From: Link Mauve @ 2024-03-27 12:40 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Emmanuel Gil Peyrot, linux-kernel, Ezequiel Garcia,
	Philipp Zabel, Mauro Carvalho Chehab, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, Joerg Roedel,
	Will Deacon, Robin Murphy, Sebastian Reichel, Cristian Ciocaltea,
	Dragan Simic, Shreeya Patel, Chris Morgan, Andy Yan,
	Nicolas Frattaroli, linux-media, linux-rockchip, devicetree,
	linux-arm-kernel, iommu

[-- Attachment #1: Type: text/plain, Size: 1945 bytes --]

On Thu, Mar 21, 2024 at 09:15:38AM +0100, Krzysztof Kozlowski wrote:
> On 20/03/2024 18:37, Emmanuel Gil Peyrot wrote:
> > The TRM (version 1.0 page 385) lists five VEPU121 cores, but only four
> > interrupts are listed (on page 24), so I’ve only enabled four of them
> > for now.
> > 
> > Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 80 +++++++++++++++++++++++
> >  1 file changed, 80 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> > index 2a23b4dc36e4..fe77b56ac9a0 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> > @@ -2488,6 +2488,86 @@ gpio4: gpio@fec50000 {
> >  		};
> >  	};
> >  
> > +	jpeg_enc0: video-codec@fdba0000 {
> > +		compatible = "rockchip,rk3588-vepu121";
> > +		reg = <0x0 0xfdba0000 0x0 0x800>;
> > +		interrupts = <GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH 0>;
> > +		clocks = <&cru ACLK_JPEG_ENCODER0>, <&cru HCLK_JPEG_ENCODER0>;
> > +		clock-names = "aclk", "hclk";
> > +		iommus = <&jpeg_enc0_mmu>;
> > +		power-domains = <&power RK3588_PD_VDPU>;
> > +	};
> > +
> > +	jpeg_enc0_mmu: iommu@fdba0800 {
> > +		compatible = "rockchip,rk3588-iommu";
> 
> It does not look like you tested the DTS against bindings. Please run
> `make dtbs_check W=1` (see
> Documentation/devicetree/bindings/writing-schema.rst or
> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> for instructions).

Even on master I get an exception about this unresolvable file:
referencing.exceptions.Unresolvable: cache-controller.yaml#

Yet it seems to be present in only three files, all of them unrelated to
the rockchip board I’m interested in (it seems), so I’m not sure what to
do about that.

The full stack trace is attached.

> 
> Best regards,
> Krzysztof
> 

-- 
Link Mauve

[-- Attachment #2: CHECK_DTBS=y W=1 --]
[-- Type: text/plain, Size: 4155 bytes --]

% make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CHECK_DTBS=y W=1 rockchip/rk3588-rock-5b.dtb
make[1]: Entering directory '/home/linkmauve/dev/linux'
  DTC_CHK arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtb
Traceback (most recent call last):
  File "/usr/lib/python3.11/site-packages/referencing/_core.py", line 417, in get_or_retrieve
    resource = registry._retrieve(uri)
               ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/jsonschema/validators.py", line 111, in _warn_for_remote_retrieve
    request = Request(uri, headers=headers)  # noqa: S310
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/urllib/request.py", line 322, in __init__
    self.full_url = url
    ^^^^^^^^^^^^^
  File "/usr/lib/python3.11/urllib/request.py", line 348, in full_url
    self._parse()
  File "/usr/lib/python3.11/urllib/request.py", line 377, in _parse
    raise ValueError("unknown url type: %r" % self.full_url)
ValueError: unknown url type: 'cache-controller.yaml'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/lib/python3.11/site-packages/referencing/_core.py", line 667, in lookup
    retrieved = self._registry.get_or_retrieve(uri)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/referencing/_core.py", line 424, in get_or_retrieve
    raise exceptions.Unretrievable(ref=uri) from error
referencing.exceptions.Unretrievable: 'cache-controller.yaml'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/bin/dt-validate", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/usr/lib/python3.11/site-packages/dtschema/dtb_validate.py", line 144, in main
    sg.check_dtb(filename)
  File "/usr/lib/python3.11/site-packages/dtschema/dtb_validate.py", line 89, in check_dtb
    self.check_subtree(dt, subtree, False, "/", "/", filename)
  File "/usr/lib/python3.11/site-packages/dtschema/dtb_validate.py", line 82, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/lib/python3.11/site-packages/dtschema/dtb_validate.py", line 82, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/lib/python3.11/site-packages/dtschema/dtb_validate.py", line 77, in check_subtree
    self.check_node(tree, subtree, disabled, nodename, fullname, filename)
  File "/usr/lib/python3.11/site-packages/dtschema/dtb_validate.py", line 33, in check_node
    for error in self.validator.iter_errors(node, filter=match_schema_file):
  File "/usr/lib/python3.11/site-packages/dtschema/validator.py", line 413, in iter_errors
    for error in self.DtValidator(sch,
  File "/usr/lib/python3.11/site-packages/jsonschema/validators.py", line 371, in iter_errors
    for error in errors:
  File "/usr/lib/python3.11/site-packages/jsonschema/_keywords.py", line 386, in if_
    yield from validator.descend(instance, then, schema_path="then")
  File "/usr/lib/python3.11/site-packages/jsonschema/validators.py", line 419, in descend
    for error in errors:
  File "/usr/lib/python3.11/site-packages/jsonschema/_legacy_keywords.py", line 423, in unevaluatedProperties_draft2019
    evaluated_keys = find_evaluated_property_keys_by_schema(
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/jsonschema/_legacy_keywords.py", line 399, in find_evaluated_property_keys_by_schema
    evaluated_keys += find_evaluated_property_keys_by_schema(
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/jsonschema/_legacy_keywords.py", line 342, in find_evaluated_property_keys_by_schema
    resolved = validator._resolver.lookup(ref)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/referencing/_core.py", line 671, in lookup
    raise exceptions.Unresolvable(ref=ref) from error
referencing.exceptions.Unresolvable: cache-controller.yaml#
make[1]: Leaving directory '/home/linkmauve/dev/linux'

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] arm64: dts: rockchip: Add VEPU121 to rk3588
  2024-03-27 12:40     ` Link Mauve
@ 2024-03-27 13:09       ` Sebastian Reichel
  0 siblings, 0 replies; 11+ messages in thread
From: Sebastian Reichel @ 2024-03-27 13:09 UTC (permalink / raw)
  To: Link Mauve
  Cc: Krzysztof Kozlowski, linux-kernel, Ezequiel Garcia,
	Philipp Zabel, Mauro Carvalho Chehab, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, Joerg Roedel,
	Will Deacon, Robin Murphy, Cristian Ciocaltea, Dragan Simic,
	Shreeya Patel, Chris Morgan, Andy Yan, Nicolas Frattaroli,
	linux-media, linux-rockchip, devicetree, linux-arm-kernel, iommu

[-- Attachment #1: Type: text/plain, Size: 2253 bytes --]

Hi,

On Wed, Mar 27, 2024 at 01:40:15PM +0100, Link Mauve wrote:
> On Thu, Mar 21, 2024 at 09:15:38AM +0100, Krzysztof Kozlowski wrote:
> > On 20/03/2024 18:37, Emmanuel Gil Peyrot wrote:
> > > The TRM (version 1.0 page 385) lists five VEPU121 cores, but only four
> > > interrupts are listed (on page 24), so I’ve only enabled four of them
> > > for now.
> > > 
> > > Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
> > > ---
> > >  arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 80 +++++++++++++++++++++++
> > >  1 file changed, 80 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> > > index 2a23b4dc36e4..fe77b56ac9a0 100644
> > > --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> > > @@ -2488,6 +2488,86 @@ gpio4: gpio@fec50000 {
> > >  		};
> > >  	};
> > >  
> > > +	jpeg_enc0: video-codec@fdba0000 {
> > > +		compatible = "rockchip,rk3588-vepu121";
> > > +		reg = <0x0 0xfdba0000 0x0 0x800>;
> > > +		interrupts = <GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH 0>;
> > > +		clocks = <&cru ACLK_JPEG_ENCODER0>, <&cru HCLK_JPEG_ENCODER0>;
> > > +		clock-names = "aclk", "hclk";
> > > +		iommus = <&jpeg_enc0_mmu>;
> > > +		power-domains = <&power RK3588_PD_VDPU>;
> > > +	};
> > > +
> > > +	jpeg_enc0_mmu: iommu@fdba0800 {
> > > +		compatible = "rockchip,rk3588-iommu";
> > 
> > It does not look like you tested the DTS against bindings. Please run
> > `make dtbs_check W=1` (see
> > Documentation/devicetree/bindings/writing-schema.rst or
> > https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> > for instructions).
> 
> Even on master I get an exception about this unresolvable file:
> referencing.exceptions.Unresolvable: cache-controller.yaml#
> 
> Yet it seems to be present in only three files, all of them unrelated to
> the rockchip board I’m interested in (it seems), so I’m not sure what to
> do about that.

The trace looked like you tried using dt-schema with jsonschema
version 4.18+, which is known broken:

https://github.com/devicetree-org/dt-schema/issues/109

Greetings,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-03-27 13:09 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20240320173736.2720778-1-linkmauve@linkmauve.fr>
     [not found] ` <20240320173736.2720778-2-linkmauve@linkmauve.fr>
2024-03-20 19:15   ` [PATCH 1/4] dt-bindings: iommu: rockchip: Fix rk3588 variant Sebastian Reichel
2024-03-21  8:14   ` Krzysztof Kozlowski
     [not found] ` <20240320173736.2720778-3-linkmauve@linkmauve.fr>
2024-03-20 20:16   ` [PATCH 2/4] media: dt-binding: media: Document rk3588’s vepu121 Sebastian Reichel
2024-03-21  8:14   ` Krzysztof Kozlowski
2024-03-21  8:47     ` Heiko Stübner
2024-03-21  9:19       ` Krzysztof Kozlowski
2024-03-21  9:32         ` Heiko Stübner
2024-03-22 14:57           ` Nicolas Dufresne
     [not found] ` <20240320173736.2720778-4-linkmauve@linkmauve.fr>
2024-03-21  8:15   ` [PATCH 3/4] arm64: dts: rockchip: Add VEPU121 to rk3588 Krzysztof Kozlowski
2024-03-27 12:40     ` Link Mauve
2024-03-27 13:09       ` Sebastian Reichel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).