mbox series

[00/10] Add support for older Rockchip SoCs to V4L2 hantro and rkvdec drivers

Message ID 20210525152225.154302-1-knaerzche@gmail.com
Headers show
Series Add support for older Rockchip SoCs to V4L2 hantro and rkvdec drivers | expand

Message

Alex Bee May 25, 2021, 3:22 p.m. UTC
Hi list,

this series adds support for older Rockchip SoCs (RK3036, RK3066, RK3188
and RK322x) to the existing V4L2 video decoder/-encoder drivers - namely
hantro and rkvdec.
They can be used as-is or with very little modifications.

In preparation to that patches 1-3 add power-controller support for RK3036
and RK322x, since both drivers rely on pm. The drivers for them exist
already in the common Rockchip pm driver, they just haven't be added to
the device trees yet.

Thanks for your feedback,
Alex.

Alex Bee (10):
  ARM: dts: rockchip: add power controller for RK322x
  ARM: dts: rockchip: add power controller for RK3036
  dt-bindings: mfd: syscon: add Rockchip RK3036/RK3228 qos compatibles
  media: hantro: add support for Rockchip RK3066
  media: hantro: add support for Rockchip RK3036
  ARM: dts: rockchip: add vpu nodes for RK3066 and RK3188
  ARM: dts: rockchip: add vpu node for RK322x
  media: dt-bindings: media: rockchip-vpu: add new compatibles
  ARM: dts: rockchip: add vdec node for RK322x
  media: dt-bindings: media: rockchip-vdec: add RK3228 compatible

 .../bindings/media/rockchip,vdec.yaml         |  10 +-
 .../bindings/media/rockchip-vpu.yaml          |  33 +++-
 .../devicetree/bindings/mfd/syscon.yaml       |   2 +
 arch/arm/boot/dts/rk3036.dtsi                 |  51 ++++++
 arch/arm/boot/dts/rk3066a.dtsi                |   4 +
 arch/arm/boot/dts/rk3188.dtsi                 |   5 +
 arch/arm/boot/dts/rk322x.dtsi                 | 139 ++++++++++++++-
 arch/arm/boot/dts/rk3xxx.dtsi                 |  12 ++
 drivers/staging/media/hantro/hantro_drv.c     |   2 +
 drivers/staging/media/hantro/hantro_hw.h      |   2 +
 drivers/staging/media/hantro/rk3288_vpu_hw.c  | 165 ++++++++++++++++++
 11 files changed, 414 insertions(+), 11 deletions(-)


base-commit: 5d765451c2409e63563fa6a3e8005bd03ab9e82f

Comments

Heiko Stuebner May 25, 2021, 11:05 p.m. UTC | #1
Am Dienstag, 25. Mai 2021, 17:22:22 CEST schrieb Alex Bee:
> The VPU IP block of RK322x is the same as RK3399 has and the driver can
> be used as-is.
> 
> Add the respective nodes to the device tree.
> 
> Signed-off-by: Alex Bee <knaerzche@gmail.com>
> ---
>  arch/arm/boot/dts/rk322x.dtsi | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/rk322x.dtsi b/arch/arm/boot/dts/rk322x.dtsi
> index c8095ede7d7a..62d1113b7804 100644
> --- a/arch/arm/boot/dts/rk322x.dtsi
> +++ b/arch/arm/boot/dts/rk322x.dtsi
> @@ -611,6 +611,18 @@ gpu: gpu@20000000 {
>  		status = "disabled";
>  	};
>  
> +	vpu: video-codec@20020000 {
> +		compatible = "rockchip,rk3228-vpu", "rockchip,rk3399-vpu";
> +		reg = <0x20020000 0x800>;
> +		interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI  9 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-names = "vepu", "vdpu";
> +		clocks = <&cru ACLK_VPU>, <&cru HCLK_VPU>;
> +		clock-names = "aclk", "hclk";
> +		power-domains = <&power RK3228_PD_VPU>;
> +		iommus = <&vpu_mmu>;

NIT: [if you need to resend for other reasons] iommus before power-domains please

> +	};
> +
>  	vpu_mmu: iommu@20020800 {
>  		compatible = "rockchip,iommu";
>  		reg = <0x20020800 0x100>;
> @@ -619,7 +631,6 @@ vpu_mmu: iommu@20020800 {
>  		clock-names = "aclk", "iface";
>  		power-domains = <&power RK3228_PD_VPU>;
>  		#iommu-cells = <0>;
> -		status = "disabled";
>  	};
>  
>  	vdec_mmu: iommu@20030480 {
>
Ezequiel Garcia May 26, 2021, 10:28 a.m. UTC | #2
Hi Alex,

Thanks a lot for the patch.

On Tue, 2021-05-25 at 17:22 +0200, Alex Bee wrote:
> RK3036's VPU IP block is the same as RK3288 has, except that it doesn't
> have an encoder, decoding is supported up to 1920x1088 only and the axi
> clock can be set to 300 MHz max.
> 
> Add a new RK3036 variant which reflect this differences.
> 
> Signed-off-by: Alex Bee <knaerzche@gmail.com>
> ---
>  drivers/staging/media/hantro/hantro_drv.c    |  1 +
>  drivers/staging/media/hantro/hantro_hw.h     |  1 +
>  drivers/staging/media/hantro/rk3288_vpu_hw.c | 49 ++++++++++++++++++++
>  3 files changed, 51 insertions(+)
> 
> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> index 38ea7b24036e..4f3c08e85bb8 100644
> --- a/drivers/staging/media/hantro/hantro_drv.c
> +++ b/drivers/staging/media/hantro/hantro_drv.c
> @@ -490,6 +490,7 @@ static const struct of_device_id of_hantro_match[] = {
>         { .compatible = "rockchip,rk3328-vpu", .data = &rk3328_vpu_variant, },
>         { .compatible = "rockchip,rk3288-vpu", .data = &rk3288_vpu_variant, },
>         { .compatible = "rockchip,rk3066-vpu", .data = &rk3066_vpu_variant, },
> +       { .compatible = "rockchip,rk3036-vpu", .data = &rk3036_vpu_variant, },
>  #endif
>  #ifdef CONFIG_VIDEO_HANTRO_IMX8M
>         { .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },
> diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> index de2bc367a15a..d8d6b0d3c3b3 100644
> --- a/drivers/staging/media/hantro/hantro_hw.h
> +++ b/drivers/staging/media/hantro/hantro_hw.h
> @@ -164,6 +164,7 @@ extern const struct hantro_variant rk3399_vpu_variant;
>  extern const struct hantro_variant rk3328_vpu_variant;
>  extern const struct hantro_variant rk3288_vpu_variant;
>  extern const struct hantro_variant rk3066_vpu_variant;
> +extern const struct hantro_variant rk3036_vpu_variant;
>  extern const struct hantro_variant imx8mq_vpu_variant;
>  extern const struct hantro_variant sama5d4_vdec_variant;
>  
> diff --git a/drivers/staging/media/hantro/rk3288_vpu_hw.c b/drivers/staging/media/hantro/rk3288_vpu_hw.c
> index 29805c4bd92f..c4684df4e012 100644
> --- a/drivers/staging/media/hantro/rk3288_vpu_hw.c
> +++ b/drivers/staging/media/hantro/rk3288_vpu_hw.c
> @@ -174,6 +174,13 @@ static irqreturn_t rk3288_vepu_irq(int irq, void *dev_id)
>         return IRQ_HANDLED;
>  }
>  
> +static int rk3036_vpu_hw_init(struct hantro_dev *vpu)
> +{
> +       /* Bump ACLKs to max. possible freq. to improve performance. */
> +       clk_set_rate(vpu->clocks[0].clk, RK3066_ACLK_MAX_FREQ);
> +       return 0;
> +}
> +
>  static int rk3066_vpu_hw_init(struct hantro_dev *vpu)
>  {
>         /* Bump ACLKs to max. possible freq. to improve performance. */
> @@ -209,6 +216,27 @@ static void rk3288_vpu_enc_reset(struct hantro_ctx *ctx)
>  /*
>   * Supported codec ops.
>   */
> +static const struct hantro_codec_ops rk3036_vpu_codec_ops[] = {
> +       [HANTRO_MODE_H264_DEC] = {
> +               .run = hantro_g1_h264_dec_run,
> +               .reset = hantro_g1_reset,
> +               .init = hantro_h264_dec_init,
> +               .exit = hantro_h264_dec_exit,
> +       },
> +       [HANTRO_MODE_MPEG2_DEC] = {
> +               .run = hantro_g1_mpeg2_dec_run,
> +               .reset = hantro_g1_reset,
> +               .init = hantro_mpeg2_dec_init,
> +               .exit = hantro_mpeg2_dec_exit,
> +       },
> +       [HANTRO_MODE_VP8_DEC] = {
> +               .run = hantro_g1_vp8_dec_run,
> +               .reset = hantro_g1_reset,
> +               .init = hantro_vp8_dec_init,
> +               .exit = hantro_vp8_dec_exit,
> +       },
> +};
> +
>  static const struct hantro_codec_ops rk3066_vpu_codec_ops[] = {
>         [HANTRO_MODE_JPEG_ENC] = {
>                 .run = hantro_h1_jpeg_enc_run,
> @@ -269,6 +297,10 @@ static const struct hantro_codec_ops rk3288_vpu_codec_ops[] = {
>   * VPU variant.
>   */
>  
> +static const struct hantro_irq rk3036_irqs[] = {
> +       { "vdpu", hantro_g1_irq },
> +};
> +
>  static const struct hantro_irq rk3288_irqs[] = {
>         { "vepu", rk3288_vepu_irq },
>         { "vdpu", hantro_g1_irq },
> @@ -283,6 +315,23 @@ static const char * const rk3288_clk_names[] = {
>         "aclk", "hclk"
>  };
>  
> +const struct hantro_variant rk3036_vpu_variant = {
> +       .dec_offset = 0x400,

If it doesn't have an encoder, then you should just
use dec_offset = 0x0.

Thanks,
Ezequiel
Alex Bee May 26, 2021, 11:27 p.m. UTC | #3
Hi Ezequiel,

Am 26.05.21 um 12:28 schrieb Ezequiel Garcia:
> Hi Alex,
>
> Thanks a lot for the patch.
>
> On Tue, 2021-05-25 at 17:22 +0200, Alex Bee wrote:
>> RK3036's VPU IP block is the same as RK3288 has, except that it doesn't
>> have an encoder, decoding is supported up to 1920x1088 only and the axi
>> clock can be set to 300 MHz max.
>>
>> Add a new RK3036 variant which reflect this differences.
>>
>> Signed-off-by: Alex Bee <knaerzche@gmail.com>
>> ---
>>   drivers/staging/media/hantro/hantro_drv.c    |  1 +
>>   drivers/staging/media/hantro/hantro_hw.h     |  1 +
>>   drivers/staging/media/hantro/rk3288_vpu_hw.c | 49 ++++++++++++++++++++
>>   3 files changed, 51 insertions(+)
>>
>> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
>> index 38ea7b24036e..4f3c08e85bb8 100644
>> --- a/drivers/staging/media/hantro/hantro_drv.c
>> +++ b/drivers/staging/media/hantro/hantro_drv.c
>> @@ -490,6 +490,7 @@ static const struct of_device_id of_hantro_match[] = {
>>          { .compatible = "rockchip,rk3328-vpu", .data = &rk3328_vpu_variant, },
>>          { .compatible = "rockchip,rk3288-vpu", .data = &rk3288_vpu_variant, },
>>          { .compatible = "rockchip,rk3066-vpu", .data = &rk3066_vpu_variant, },
>> +       { .compatible = "rockchip,rk3036-vpu", .data = &rk3036_vpu_variant, },
>>   #endif
>>   #ifdef CONFIG_VIDEO_HANTRO_IMX8M
>>          { .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },
>> diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
>> index de2bc367a15a..d8d6b0d3c3b3 100644
>> --- a/drivers/staging/media/hantro/hantro_hw.h
>> +++ b/drivers/staging/media/hantro/hantro_hw.h
>> @@ -164,6 +164,7 @@ extern const struct hantro_variant rk3399_vpu_variant;
>>   extern const struct hantro_variant rk3328_vpu_variant;
>>   extern const struct hantro_variant rk3288_vpu_variant;
>>   extern const struct hantro_variant rk3066_vpu_variant;
>> +extern const struct hantro_variant rk3036_vpu_variant;
>>   extern const struct hantro_variant imx8mq_vpu_variant;
>>   extern const struct hantro_variant sama5d4_vdec_variant;
>>   
>> diff --git a/drivers/staging/media/hantro/rk3288_vpu_hw.c b/drivers/staging/media/hantro/rk3288_vpu_hw.c
>> index 29805c4bd92f..c4684df4e012 100644
>> --- a/drivers/staging/media/hantro/rk3288_vpu_hw.c
>> +++ b/drivers/staging/media/hantro/rk3288_vpu_hw.c
>> @@ -174,6 +174,13 @@ static irqreturn_t rk3288_vepu_irq(int irq, void *dev_id)
>>          return IRQ_HANDLED;
>>   }
>>   
>> +static int rk3036_vpu_hw_init(struct hantro_dev *vpu)
>> +{
>> +       /* Bump ACLKs to max. possible freq. to improve performance. */
>> +       clk_set_rate(vpu->clocks[0].clk, RK3066_ACLK_MAX_FREQ);
>> +       return 0;
>> +}
>> +
>>   static int rk3066_vpu_hw_init(struct hantro_dev *vpu)
>>   {
>>          /* Bump ACLKs to max. possible freq. to improve performance. */
>> @@ -209,6 +216,27 @@ static void rk3288_vpu_enc_reset(struct hantro_ctx *ctx)
>>   /*
>>    * Supported codec ops.
>>    */
>> +static const struct hantro_codec_ops rk3036_vpu_codec_ops[] = {
>> +       [HANTRO_MODE_H264_DEC] = {
>> +               .run = hantro_g1_h264_dec_run,
>> +               .reset = hantro_g1_reset,
>> +               .init = hantro_h264_dec_init,
>> +               .exit = hantro_h264_dec_exit,
>> +       },
>> +       [HANTRO_MODE_MPEG2_DEC] = {
>> +               .run = hantro_g1_mpeg2_dec_run,
>> +               .reset = hantro_g1_reset,
>> +               .init = hantro_mpeg2_dec_init,
>> +               .exit = hantro_mpeg2_dec_exit,
>> +       },
>> +       [HANTRO_MODE_VP8_DEC] = {
>> +               .run = hantro_g1_vp8_dec_run,
>> +               .reset = hantro_g1_reset,
>> +               .init = hantro_vp8_dec_init,
>> +               .exit = hantro_vp8_dec_exit,
>> +       },
>> +};
>> +
>>   static const struct hantro_codec_ops rk3066_vpu_codec_ops[] = {
>>          [HANTRO_MODE_JPEG_ENC] = {
>>                  .run = hantro_h1_jpeg_enc_run,
>> @@ -269,6 +297,10 @@ static const struct hantro_codec_ops rk3288_vpu_codec_ops[] = {
>>    * VPU variant.
>>    */
>>   
>> +static const struct hantro_irq rk3036_irqs[] = {
>> +       { "vdpu", hantro_g1_irq },
>> +};
>> +
>>   static const struct hantro_irq rk3288_irqs[] = {
>>          { "vepu", rk3288_vepu_irq },
>>          { "vdpu", hantro_g1_irq },
>> @@ -283,6 +315,23 @@ static const char * const rk3288_clk_names[] = {
>>          "aclk", "hclk"
>>   };
>>   
>> +const struct hantro_variant rk3036_vpu_variant = {
>> +       .dec_offset = 0x400,
> If it doesn't have an encoder, then you should just
> use dec_offset = 0x0.
>
> Thanks,
> Ezequiel
>
That would mean, I'd have to adapt the register offset in the device 
tree - I'd prefer to keep it in line with the TRM. Unless you insist, 
I'd like to keep it this way (It's , btw, the very same for RK3328).

Alex
Alex Bee May 26, 2021, 11:38 p.m. UTC | #4
Hi Heiko, Ezequiel, Rob and List,

thanks for your feedback.

Am 26.05.21 um 01:01 schrieb Heiko Stübner:
> Hi Alex,
>
> Am Dienstag, 25. Mai 2021, 17:22:15 CEST schrieb Alex Bee:
>> Hi list,
>>
>> this series adds support for older Rockchip SoCs (RK3036, RK3066, RK3188
>> and RK322x) to the existing V4L2 video decoder/-encoder drivers - namely
>> hantro and rkvdec.
>> They can be used as-is or with very little modifications.
>>
>> In preparation to that patches 1-3 add power-controller support for RK3036
>> and RK322x, since both drivers rely on pm. The drivers for them exist
>> already in the common Rockchip pm driver, they just haven't be added to
>> the device trees yet.
> on first glance, looks good. Just a small ordering nit, if you need to resend
> the series for other reasons:
>
> Please try to order patches like:
> (1) dt-binding - compatible addition
> (2) driver patches
> (3) devicetree node patches
>
> That makes it way easier to keep track of dependencies when glancing at
> the series. Like for patches 1+2, I need to wait for Lee to apply (or Ack) the
> binding addition in patch 3.
>
> Same for the hantro devicetree additions, that need to wait for both
> bindings (and driver) changes to get applied to the media tree.
>
> Thanks
> Heiko
>
>
>> Thanks for your feedback,
>> Alex.
>>
>> Alex Bee (10):
>>    ARM: dts: rockchip: add power controller for RK322x
>>    ARM: dts: rockchip: add power controller for RK3036
>>    dt-bindings: mfd: syscon: add Rockchip RK3036/RK3228 qos compatibles
>>    media: hantro: add support for Rockchip RK3066
>>    media: hantro: add support for Rockchip RK3036
>>    ARM: dts: rockchip: add vpu nodes for RK3066 and RK3188
>>    ARM: dts: rockchip: add vpu node for RK322x
>>    media: dt-bindings: media: rockchip-vpu: add new compatibles
>>    ARM: dts: rockchip: add vdec node for RK322x
>>    media: dt-bindings: media: rockchip-vdec: add RK3228 compatible
>>
>>   .../bindings/media/rockchip,vdec.yaml         |  10 +-
>>   .../bindings/media/rockchip-vpu.yaml          |  33 +++-
>>   .../devicetree/bindings/mfd/syscon.yaml       |   2 +
>>   arch/arm/boot/dts/rk3036.dtsi                 |  51 ++++++
>>   arch/arm/boot/dts/rk3066a.dtsi                |   4 +
>>   arch/arm/boot/dts/rk3188.dtsi                 |   5 +
>>   arch/arm/boot/dts/rk322x.dtsi                 | 139 ++++++++++++++-
>>   arch/arm/boot/dts/rk3xxx.dtsi                 |  12 ++
>>   drivers/staging/media/hantro/hantro_drv.c     |   2 +
>>   drivers/staging/media/hantro/hantro_hw.h      |   2 +
>>   drivers/staging/media/hantro/rk3288_vpu_hw.c  | 165 ++++++++++++++++++
>>   11 files changed, 414 insertions(+), 11 deletions(-)
>>
>>
>> base-commit: 5d765451c2409e63563fa6a3e8005bd03ab9e82f
>>
>
>
>
I'll address your comments in v2 - see individual patches for specific 
replies (if any).

Thanks,

Alex
Heiko Stuebner May 26, 2021, 11:58 p.m. UTC | #5
Am Donnerstag, 27. Mai 2021, 01:27:59 CEST schrieb Alex Bee:
> Hi Ezequiel,
> 
> Am 26.05.21 um 12:28 schrieb Ezequiel Garcia:
> > Hi Alex,
> >
> > Thanks a lot for the patch.
> >
> > On Tue, 2021-05-25 at 17:22 +0200, Alex Bee wrote:
> >> RK3036's VPU IP block is the same as RK3288 has, except that it doesn't
> >> have an encoder, decoding is supported up to 1920x1088 only and the axi
> >> clock can be set to 300 MHz max.
> >>
> >> Add a new RK3036 variant which reflect this differences.
> >>
> >> Signed-off-by: Alex Bee <knaerzche@gmail.com>
> >> ---
> >>   drivers/staging/media/hantro/hantro_drv.c    |  1 +
> >>   drivers/staging/media/hantro/hantro_hw.h     |  1 +
> >>   drivers/staging/media/hantro/rk3288_vpu_hw.c | 49 ++++++++++++++++++++
> >>   3 files changed, 51 insertions(+)
> >>
> >> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> >> index 38ea7b24036e..4f3c08e85bb8 100644
> >> --- a/drivers/staging/media/hantro/hantro_drv.c
> >> +++ b/drivers/staging/media/hantro/hantro_drv.c
> >> @@ -490,6 +490,7 @@ static const struct of_device_id of_hantro_match[] = {
> >>          { .compatible = "rockchip,rk3328-vpu", .data = &rk3328_vpu_variant, },
> >>          { .compatible = "rockchip,rk3288-vpu", .data = &rk3288_vpu_variant, },
> >>          { .compatible = "rockchip,rk3066-vpu", .data = &rk3066_vpu_variant, },
> >> +       { .compatible = "rockchip,rk3036-vpu", .data = &rk3036_vpu_variant, },
> >>   #endif
> >>   #ifdef CONFIG_VIDEO_HANTRO_IMX8M
> >>          { .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },
> >> diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> >> index de2bc367a15a..d8d6b0d3c3b3 100644
> >> --- a/drivers/staging/media/hantro/hantro_hw.h
> >> +++ b/drivers/staging/media/hantro/hantro_hw.h
> >> @@ -164,6 +164,7 @@ extern const struct hantro_variant rk3399_vpu_variant;
> >>   extern const struct hantro_variant rk3328_vpu_variant;
> >>   extern const struct hantro_variant rk3288_vpu_variant;
> >>   extern const struct hantro_variant rk3066_vpu_variant;
> >> +extern const struct hantro_variant rk3036_vpu_variant;
> >>   extern const struct hantro_variant imx8mq_vpu_variant;
> >>   extern const struct hantro_variant sama5d4_vdec_variant;
> >>   
> >> diff --git a/drivers/staging/media/hantro/rk3288_vpu_hw.c b/drivers/staging/media/hantro/rk3288_vpu_hw.c
> >> index 29805c4bd92f..c4684df4e012 100644
> >> --- a/drivers/staging/media/hantro/rk3288_vpu_hw.c
> >> +++ b/drivers/staging/media/hantro/rk3288_vpu_hw.c
> >> @@ -174,6 +174,13 @@ static irqreturn_t rk3288_vepu_irq(int irq, void *dev_id)
> >>          return IRQ_HANDLED;
> >>   }
> >>   
> >> +static int rk3036_vpu_hw_init(struct hantro_dev *vpu)
> >> +{
> >> +       /* Bump ACLKs to max. possible freq. to improve performance. */
> >> +       clk_set_rate(vpu->clocks[0].clk, RK3066_ACLK_MAX_FREQ);
> >> +       return 0;
> >> +}
> >> +
> >>   static int rk3066_vpu_hw_init(struct hantro_dev *vpu)
> >>   {
> >>          /* Bump ACLKs to max. possible freq. to improve performance. */
> >> @@ -209,6 +216,27 @@ static void rk3288_vpu_enc_reset(struct hantro_ctx *ctx)
> >>   /*
> >>    * Supported codec ops.
> >>    */
> >> +static const struct hantro_codec_ops rk3036_vpu_codec_ops[] = {
> >> +       [HANTRO_MODE_H264_DEC] = {
> >> +               .run = hantro_g1_h264_dec_run,
> >> +               .reset = hantro_g1_reset,
> >> +               .init = hantro_h264_dec_init,
> >> +               .exit = hantro_h264_dec_exit,
> >> +       },
> >> +       [HANTRO_MODE_MPEG2_DEC] = {
> >> +               .run = hantro_g1_mpeg2_dec_run,
> >> +               .reset = hantro_g1_reset,
> >> +               .init = hantro_mpeg2_dec_init,
> >> +               .exit = hantro_mpeg2_dec_exit,
> >> +       },
> >> +       [HANTRO_MODE_VP8_DEC] = {
> >> +               .run = hantro_g1_vp8_dec_run,
> >> +               .reset = hantro_g1_reset,
> >> +               .init = hantro_vp8_dec_init,
> >> +               .exit = hantro_vp8_dec_exit,
> >> +       },
> >> +};
> >> +
> >>   static const struct hantro_codec_ops rk3066_vpu_codec_ops[] = {
> >>          [HANTRO_MODE_JPEG_ENC] = {
> >>                  .run = hantro_h1_jpeg_enc_run,
> >> @@ -269,6 +297,10 @@ static const struct hantro_codec_ops rk3288_vpu_codec_ops[] = {
> >>    * VPU variant.
> >>    */
> >>   
> >> +static const struct hantro_irq rk3036_irqs[] = {
> >> +       { "vdpu", hantro_g1_irq },
> >> +};
> >> +
> >>   static const struct hantro_irq rk3288_irqs[] = {
> >>          { "vepu", rk3288_vepu_irq },
> >>          { "vdpu", hantro_g1_irq },
> >> @@ -283,6 +315,23 @@ static const char * const rk3288_clk_names[] = {
> >>          "aclk", "hclk"
> >>   };
> >>   
> >> +const struct hantro_variant rk3036_vpu_variant = {
> >> +       .dec_offset = 0x400,
> > If it doesn't have an encoder, then you should just
> > use dec_offset = 0x0.
> >
> > Thanks,
> > Ezequiel
> >
> That would mean, I'd have to adapt the register offset in the device 
> tree - I'd prefer to keep it in line with the TRM. Unless you insist, 
> I'd like to keep it this way (It's , btw, the very same for RK3328).

I'd agree with Alex ... ideally the devicetree should match the block
register area from the TRM not some internal offset.
[DT describes hardware etc etc ;-) ]

Heiko
Ezequiel Garcia May 27, 2021, 1:27 a.m. UTC | #6
On Thu, 2021-05-27 at 01:58 +0200, Heiko Stübner wrote:
> Am Donnerstag, 27. Mai 2021, 01:27:59 CEST schrieb Alex Bee:
> > Hi Ezequiel,
> > 
> > Am 26.05.21 um 12:28 schrieb Ezequiel Garcia:
> > > Hi Alex,
> > > 
> > > Thanks a lot for the patch.
> > > 
> > > On Tue, 2021-05-25 at 17:22 +0200, Alex Bee wrote:
> > > > RK3036's VPU IP block is the same as RK3288 has, except that it doesn't
> > > > have an encoder, decoding is supported up to 1920x1088 only and the axi
> > > > clock can be set to 300 MHz max.
> > > > 
> > > > Add a new RK3036 variant which reflect this differences.
> > > > 
> > > > Signed-off-by: Alex Bee <knaerzche@gmail.com>
> > > > ---
> > > >   drivers/staging/media/hantro/hantro_drv.c    |  1 +
> > > >   drivers/staging/media/hantro/hantro_hw.h     |  1 +
> > > >   drivers/staging/media/hantro/rk3288_vpu_hw.c | 49 ++++++++++++++++++++
> > > >   3 files changed, 51 insertions(+)
> > > > 
> > > > diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> > > > index 38ea7b24036e..4f3c08e85bb8 100644
> > > > --- a/drivers/staging/media/hantro/hantro_drv.c
> > > > +++ b/drivers/staging/media/hantro/hantro_drv.c
> > > > @@ -490,6 +490,7 @@ static const struct of_device_id of_hantro_match[] = {
> > > >          { .compatible = "rockchip,rk3328-vpu", .data = &rk3328_vpu_variant, },
> > > >          { .compatible = "rockchip,rk3288-vpu", .data = &rk3288_vpu_variant, },
> > > >          { .compatible = "rockchip,rk3066-vpu", .data = &rk3066_vpu_variant, },
> > > > +       { .compatible = "rockchip,rk3036-vpu", .data = &rk3036_vpu_variant, },
> > > >   #endif
> > > >   #ifdef CONFIG_VIDEO_HANTRO_IMX8M
> > > >          { .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },
> > > > diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> > > > index de2bc367a15a..d8d6b0d3c3b3 100644
> > > > --- a/drivers/staging/media/hantro/hantro_hw.h
> > > > +++ b/drivers/staging/media/hantro/hantro_hw.h
> > > > @@ -164,6 +164,7 @@ extern const struct hantro_variant rk3399_vpu_variant;
> > > >   extern const struct hantro_variant rk3328_vpu_variant;
> > > >   extern const struct hantro_variant rk3288_vpu_variant;
> > > >   extern const struct hantro_variant rk3066_vpu_variant;
> > > > +extern const struct hantro_variant rk3036_vpu_variant;
> > > >   extern const struct hantro_variant imx8mq_vpu_variant;
> > > >   extern const struct hantro_variant sama5d4_vdec_variant;
> > > >   
> > > > diff --git a/drivers/staging/media/hantro/rk3288_vpu_hw.c b/drivers/staging/media/hantro/rk3288_vpu_hw.c
> > > > index 29805c4bd92f..c4684df4e012 100644
> > > > --- a/drivers/staging/media/hantro/rk3288_vpu_hw.c
> > > > +++ b/drivers/staging/media/hantro/rk3288_vpu_hw.c
> > > > @@ -174,6 +174,13 @@ static irqreturn_t rk3288_vepu_irq(int irq, void *dev_id)
> > > >          return IRQ_HANDLED;
> > > >   }
> > > >   
> > > > +static int rk3036_vpu_hw_init(struct hantro_dev *vpu)
> > > > +{
> > > > +       /* Bump ACLKs to max. possible freq. to improve performance. */
> > > > +       clk_set_rate(vpu->clocks[0].clk, RK3066_ACLK_MAX_FREQ);
> > > > +       return 0;
> > > > +}
> > > > +
> > > >   static int rk3066_vpu_hw_init(struct hantro_dev *vpu)
> > > >   {
> > > >          /* Bump ACLKs to max. possible freq. to improve performance. */
> > > > @@ -209,6 +216,27 @@ static void rk3288_vpu_enc_reset(struct hantro_ctx *ctx)
> > > >   /*
> > > >    * Supported codec ops.
> > > >    */
> > > > +static const struct hantro_codec_ops rk3036_vpu_codec_ops[] = {
> > > > +       [HANTRO_MODE_H264_DEC] = {
> > > > +               .run = hantro_g1_h264_dec_run,
> > > > +               .reset = hantro_g1_reset,
> > > > +               .init = hantro_h264_dec_init,
> > > > +               .exit = hantro_h264_dec_exit,
> > > > +       },
> > > > +       [HANTRO_MODE_MPEG2_DEC] = {
> > > > +               .run = hantro_g1_mpeg2_dec_run,
> > > > +               .reset = hantro_g1_reset,
> > > > +               .init = hantro_mpeg2_dec_init,
> > > > +               .exit = hantro_mpeg2_dec_exit,
> > > > +       },
> > > > +       [HANTRO_MODE_VP8_DEC] = {
> > > > +               .run = hantro_g1_vp8_dec_run,
> > > > +               .reset = hantro_g1_reset,
> > > > +               .init = hantro_vp8_dec_init,
> > > > +               .exit = hantro_vp8_dec_exit,
> > > > +       },
> > > > +};
> > > > +
> > > >   static const struct hantro_codec_ops rk3066_vpu_codec_ops[] = {
> > > >          [HANTRO_MODE_JPEG_ENC] = {
> > > >                  .run = hantro_h1_jpeg_enc_run,
> > > > @@ -269,6 +297,10 @@ static const struct hantro_codec_ops rk3288_vpu_codec_ops[] = {
> > > >    * VPU variant.
> > > >    */
> > > >   
> > > > +static const struct hantro_irq rk3036_irqs[] = {
> > > > +       { "vdpu", hantro_g1_irq },
> > > > +};
> > > > +
> > > >   static const struct hantro_irq rk3288_irqs[] = {
> > > >          { "vepu", rk3288_vepu_irq },
> > > >          { "vdpu", hantro_g1_irq },
> > > > @@ -283,6 +315,23 @@ static const char * const rk3288_clk_names[] = {
> > > >          "aclk", "hclk"
> > > >   };
> > > >   
> > > > +const struct hantro_variant rk3036_vpu_variant = {
> > > > +       .dec_offset = 0x400,
> > > If it doesn't have an encoder, then you should just
> > > use dec_offset = 0x0.
> > > 
> > > Thanks,
> > > Ezequiel
> > > 
> > That would mean, I'd have to adapt the register offset in the device 
> > tree - I'd prefer to keep it in line with the TRM. Unless you insist, 
> > I'd like to keep it this way (It's , btw, the very same for RK3328).
> 
> I'd agree with Alex ... ideally the devicetree should match the block
> register area from the TRM not some internal offset.
> [DT describes hardware etc etc ;-) ]
> 

Well, I've always considered this internal offset as something unfortunate
we didn't do well when we upstreamed RK3288.

The RK3288 TRM documents a so-called "VPU combo", and then documents
the encoder and the decoder cores as separate engines, with
separate register blocks (called VEPU and VDPU). In fact, for each
register block you'll see swreg0 documented at offset 0x0.

(In some integrations they can operate independently, but iirc not in RK3288.)

So to be clear, instead of:

        vpu: video-codec@ff9a0000 {
                compatible = "rockchip,rk3288-vpu";
                reg = <0x0 0xff9a0000 0x0 0x800>;
                interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
                             <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
                interrupt-names = "vepu", "vdpu";
                clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>;
                clock-names = "aclk", "hclk";
                ...

It could have looked like:

        vpu: video-codec@ff9a0000 {
                compatible = "rockchip,rk3288-vpu";
                reg = <0x0 0xff9a0000 0x0 0x400>
                      <0x0 0xff9a0400 0x0 0x400>;
                ...

I guess I missed this when RK3328 was pushed, but OTOH I don't
see any real impact in doing things this way. So at the end
of the day, I'm fine either way.

BTW, the series is not adding the vpu node for arch/arm/boot/dts/rk3036.dtsi right?

Thanks a lot!
Ezequiel
Alex Bee May 27, 2021, 3:44 p.m. UTC | #7
Hi list,

this series adds support for older Rockchip SoCs (RK3036, RK3066, RK3188
and RK322x) to the existing V4L2 video decoder/-encoder drivers - namely
hantro and rkvdec.
They can be used as-is or with very little modifications.

In preparation to that patches 1,8 and 9 add power-controller support for
RK3036 and RK322x, since both drivers rely on pm. The drivers for them 
exist already in the common Rockchip pm driver, they just haven't be added
to the device trees yet.

Changes in v2:
- reordered patches as suggested by Heiko
- fixed indentation and order issues in dt-bindings / DT patches
- added patch to reorder variants in hantro alphanumeric
- added patch that merges hantro platform drivers for Rockchip in one
- added missing DT patch for RK3036 (missed to submit with v1)
See individual patches for details about changes.

Thanks for your feedback,
Alex.

Alex Bee (12):
  dt-bindings: mfd: syscon: add Rockchip RK3036/RK3228 qos compatibles
  dt-bindings: media: rockchip-vpu: add new compatibles
  dt-bindings: media: rockchip-vdec: add RK3228 compatible
  media: hantro: reorder variants
  media: hantro: merge Rockchip platform drivers
  media: hantro: add support for Rockchip RK3066
  media: hantro: add support for Rockchip RK3036
  ARM: dts: rockchip: add power controller for RK3036
  ARM: dts: rockchip: add power controller for RK322x
  ARM: dts: rockchip: add vpu node for RK3036
  ARM: dts: rockchip: add vpu nodes for RK3066 and RK3188
  ARM: dts: rockchip: add vpu and vdec node for RK322x

 .../bindings/media/rockchip,vdec.yaml         |  10 +-
 .../bindings/media/rockchip-vpu.yaml          |  33 +-
 .../devicetree/bindings/mfd/syscon.yaml       |   2 +
 arch/arm/boot/dts/rk3036.dtsi                 |  72 +++
 arch/arm/boot/dts/rk3066a.dtsi                |   4 +
 arch/arm/boot/dts/rk3188.dtsi                 |   5 +
 arch/arm/boot/dts/rk322x.dtsi                 | 140 ++++-
 arch/arm/boot/dts/rk3xxx.dtsi                 |  12 +
 drivers/staging/media/hantro/Makefile         |   9 +-
 drivers/staging/media/hantro/hantro_drv.c     |   6 +-
 drivers/staging/media/hantro/hantro_hw.h      |  30 +-
 drivers/staging/media/hantro/rk3288_vpu_hw.c  | 208 -------
 drivers/staging/media/hantro/rk3399_vpu_hw.c  | 222 --------
 ...jpeg_enc.c => rockchip_vpu2_hw_jpeg_enc.c} |  30 +-
 ...eg2_dec.c => rockchip_vpu2_hw_mpeg2_dec.c} |  25 +-
 ...w_vp8_dec.c => rockchip_vpu2_hw_vp8_dec.c} |   2 +-
 ...rk3399_vpu_regs.h => rockchip_vpu2_regs.h} |   6 +-
 .../staging/media/hantro/rockchip_vpu_hw.c    | 526 ++++++++++++++++++
 18 files changed, 848 insertions(+), 494 deletions(-)
 delete mode 100644 drivers/staging/media/hantro/rk3288_vpu_hw.c
 delete mode 100644 drivers/staging/media/hantro/rk3399_vpu_hw.c
 rename drivers/staging/media/hantro/{rk3399_vpu_hw_jpeg_enc.c => rockchip_vpu2_hw_jpeg_enc.c} (87%)
 rename drivers/staging/media/hantro/{rk3399_vpu_hw_mpeg2_dec.c => rockchip_vpu2_hw_mpeg2_dec.c} (93%)
 rename drivers/staging/media/hantro/{rk3399_vpu_hw_vp8_dec.c => rockchip_vpu2_hw_vp8_dec.c} (99%)
 rename drivers/staging/media/hantro/{rk3399_vpu_regs.h => rockchip_vpu2_regs.h} (99%)
 create mode 100644 drivers/staging/media/hantro/rockchip_vpu_hw.c


base-commit: 5d765451c2409e63563fa6a3e8005bd03ab9e82f
Lee Jones June 1, 2021, 3:46 p.m. UTC | #8
On Thu, 27 May 2021, Alex Bee wrote:

> Document Rockchip RK3036/RK3228 qos compatibles
> 
> Signed-off-by: Alex Bee <knaerzche@gmail.com>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> ---
> 
>  Changes in v2:
>  - collect Reviewed tag
> 
>  Documentation/devicetree/bindings/mfd/syscon.yaml | 2 ++
>  1 file changed, 2 insertions(+)

Applied, thanks.
Rob Herring (Arm) June 4, 2021, 8:24 p.m. UTC | #9
On Thu, 27 May 2021 17:44:44 +0200, Alex Bee wrote:
> Document Rockchip RK3036/RK3228 qos compatibles
> 
> Signed-off-by: Alex Bee <knaerzche@gmail.com>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> ---
> 
>  Changes in v2:
>  - collect Reviewed tag
> 
>  Documentation/devicetree/bindings/mfd/syscon.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>
Rob Herring (Arm) June 4, 2021, 8:45 p.m. UTC | #10
On Thu, 27 May 2021 17:44:46 +0200, Alex Bee wrote:
> Document the RK3228 compatible for rockchip-vdec.
> Also add the optional assigned-clocks and assigned-clock-rates
> properties.
> 
> Signed-off-by: Alex Bee <knaerzche@gmail.com>
> ---
> 
>  Changes in v2:
>  - fix indentation
> 
>  .../devicetree/bindings/media/rockchip,vdec.yaml       | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>
Ezequiel Garcia June 11, 2021, 3:36 p.m. UTC | #11
Hi Alex,

This cleanup looks nice, thanks!

Just a comment about the clock rate and a minor typo.

On Thu, 2021-05-27 at 17:44 +0200, Alex Bee wrote:
> Merge the two Rockchip platform drivers into one as it was suggested at
> [1] and [2].
> This will hopefully make it easier to add new variants (which are surely
> to come for Rockchip).
> Also rename from "rk3288" to "v(d/e)pu1" and "rk3399" to "v(d/e)pu2"
> where applicable, as this is the dicition the vendor uses and will
> also refelect the variants that get added later in this series. Rename
> from "rk3288" to "rockchip" if applicable to both hardware versions.
> 
> [1] https://patchwork.kernel.org/project/linux-rockchip/patch/20210107134101.195426-6-paul.kocialkowski@bootlin.com/
> [2] https://patchwork.kernel.org/project/linux-rockchip/patch/20210525152225.154302-5-knaerzche@gmail.com/
> 
> Signed-off-by: Alex Bee <knaerzche@gmail.com>
> ---
> 
>  Changes in v2:
>  - added patch
> 
>  drivers/staging/media/hantro/Makefile         |   9 +-
>  drivers/staging/media/hantro/hantro_hw.h      |  22 +-
>  drivers/staging/media/hantro/rk3288_vpu_hw.c  | 208 ----------
>  drivers/staging/media/hantro/rk3399_vpu_hw.c  | 222 -----------
>  ...jpeg_enc.c => rockchip_vpu2_hw_jpeg_enc.c} |  30 +-
>  ...eg2_dec.c => rockchip_vpu2_hw_mpeg2_dec.c} |  25 +-
>  ...w_vp8_dec.c => rockchip_vpu2_hw_vp8_dec.c} |   2 +-
>  ...rk3399_vpu_regs.h => rockchip_vpu2_regs.h} |   6 +-
>  .../staging/media/hantro/rockchip_vpu_hw.c    | 356 ++++++++++++++++++
>  9 files changed, 402 insertions(+), 478 deletions(-)
>  delete mode 100644 drivers/staging/media/hantro/rk3288_vpu_hw.c
>  delete mode 100644 drivers/staging/media/hantro/rk3399_vpu_hw.c
>  rename drivers/staging/media/hantro/{rk3399_vpu_hw_jpeg_enc.c => rockchip_vpu2_hw_jpeg_enc.c} (87%)
>  rename drivers/staging/media/hantro/{rk3399_vpu_hw_mpeg2_dec.c => rockchip_vpu2_hw_mpeg2_dec.c} (93%)
>  rename drivers/staging/media/hantro/{rk3399_vpu_hw_vp8_dec.c => rockchip_vpu2_hw_vp8_dec.c} (99%)
>  rename drivers/staging/media/hantro/{rk3399_vpu_regs.h => rockchip_vpu2_regs.h} (99%)
>  create mode 100644 drivers/staging/media/hantro/rockchip_vpu_hw.c
> 
[..]
> diff --git a/drivers/staging/media/hantro/rockchip_vpu_hw.c b/drivers/staging/media/hantro/rockchip_vpu_hw.c
> new file mode 100644
> index 000000000000..175d0c5dfdbe
> --- /dev/null
> +++ b/drivers/staging/media/hantro/rockchip_vpu_hw.c
> @@ -0,0 +1,356 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Hantro VPU codec driver
> + *
> + * Copyright (C) 2018 Rockchip Electronics Co., Ltd.
> + *     Jeffy Chen <jeffy.chen@rock-chips.com>
> + */
> +
> +#include <linux/clk.h>
> +
> +#include "hantro.h"
> +#include "hantro_jpeg.h"
> +#include "hantro_h1_regs.h"
> +#include "rockchip_vpu2_regs.h"
> +
> +#define RK3288_ACLK_MAX_FREQ (400 * 1000 * 1000)
> +

Something for later, could we bump this clock?

Rockchip uses 300MHz and 600MHz even for RK3288,
see arch/arm/boot/dts/rk3288.dtsi:

        vdpu: vdpu@ff9a0400 {
                compatible = "rockchip,vpu-decoder-rk3288", "rockchip,vpu-decoder-v1";
                reg = <0x0 0xff9a0400 0x0 0x400>;
                interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
                interrupt-names = "irq_dec";
                clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>;
                clock-names = "aclk_vcodec", "hclk_vcodec";
                rockchip,normal-rates = <300000000>, <0>;
                rockchip,advanced-rates = <600000000>, <0>;

On vdpu1 for instance, it switches to the "advanced-rate"
when width > 2560, which I think it's too naive for us.

At least on RK3566 I tested bumping this rate, while still
passing conformance tests.

> +/*
> + * Supported formats.
> + */
> +
> +static const struct hantro_fmt rockchip_vpu_enc_fmts[] = {
> +       {
> +               .fourcc = V4L2_PIX_FMT_YUV420M,
> +               .codec_mode = HANTRO_MODE_NONE,
> +               .enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUV420P,
> +       },
> +       {
> +               .fourcc = V4L2_PIX_FMT_NV12M,
> +               .codec_mode = HANTRO_MODE_NONE,
> +               .enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUV420SP,
> +       },
> +       {
> +               .fourcc = V4L2_PIX_FMT_YUYV,
> +               .codec_mode = HANTRO_MODE_NONE,
> +               .enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUYV422,
> +       },
> +       {
> +               .fourcc = V4L2_PIX_FMT_UYVY,
> +               .codec_mode = HANTRO_MODE_NONE,
> +               .enc_fmt = ROCKCHIP_VPU_ENC_FMT_UYVY422,
> +       },
> +       {
> +               .fourcc = V4L2_PIX_FMT_JPEG,
> +               .codec_mode = HANTRO_MODE_JPEG_ENC,
> +               .max_depth = 2,
> +               .header_size = JPEG_HEADER_SIZE,
> +               .frmsize = {
> +                       .min_width = 96,
> +                       .max_width = 8192,
> +                       .step_width = MB_DIM,
> +                       .min_height = 32,
> +                       .max_height = 8192,
> +                       .step_height = MB_DIM,
> +               },
> +       },
> +};
> +
> +static const struct hantro_fmt rockchip_vpu1_postproc_fmts[] = {
> +       {
> +               .fourcc = V4L2_PIX_FMT_YUYV,
> +               .codec_mode = HANTRO_MODE_NONE,
> +       },
> +};
> +
> +static const struct hantro_fmt rk3288_vpu_dec_fmts[] = {
> +       {
> +               .fourcc = V4L2_PIX_FMT_NV12,
> +               .codec_mode = HANTRO_MODE_NONE,
> +       },
> +       {
> +               .fourcc = V4L2_PIX_FMT_H264_SLICE,
> +               .codec_mode = HANTRO_MODE_H264_DEC,
> +               .max_depth = 2,
> +               .frmsize = {
> +                       .min_width = 48,
> +                       .max_width = 4096,
> +                       .step_width = MB_DIM,
> +                       .min_height = 48,
> +                       .max_height = 2304,
> +                       .step_height = MB_DIM,
> +               },
> +       },
> +       {
> +               .fourcc = V4L2_PIX_FMT_MPEG2_SLICE,
> +               .codec_mode = HANTRO_MODE_MPEG2_DEC,
> +               .max_depth = 2,
> +               .frmsize = {
> +                       .min_width = 48,
> +                       .max_width = 1920,
> +                       .step_width = MB_DIM,
> +                       .min_height = 48,
> +                       .max_height = 1088,
> +                       .step_height = MB_DIM,
> +               },
> +       },
> +       {
> +               .fourcc = V4L2_PIX_FMT_VP8_FRAME,
> +               .codec_mode = HANTRO_MODE_VP8_DEC,
> +               .max_depth = 2,
> +               .frmsize = {
> +                       .min_width = 48,
> +                       .max_width = 3840,
> +                       .step_width = MB_DIM,
> +                       .min_height = 48,
> +                       .max_height = 2160,
> +                       .step_height = MB_DIM,
> +               },
> +       },
> +};
> +
> +static const struct hantro_fmt rk3399_vpu_dec_fmts[] = {
> +       {
> +               .fourcc = V4L2_PIX_FMT_NV12,
> +               .codec_mode = HANTRO_MODE_NONE,
> +       },
> +       {
> +               .fourcc = V4L2_PIX_FMT_MPEG2_SLICE,
> +               .codec_mode = HANTRO_MODE_MPEG2_DEC,
> +               .max_depth = 2,
> +               .frmsize = {
> +                       .min_width = 48,
> +                       .max_width = 1920,
> +                       .step_width = MB_DIM,
> +                       .min_height = 48,
> +                       .max_height = 1088,
> +                       .step_height = MB_DIM,
> +               },
> +       },
> +       {
> +               .fourcc = V4L2_PIX_FMT_VP8_FRAME,
> +               .codec_mode = HANTRO_MODE_VP8_DEC,
> +               .max_depth = 2,
> +               .frmsize = {
> +                       .min_width = 48,
> +                       .max_width = 3840,
> +                       .step_width = MB_DIM,
> +                       .min_height = 48,
> +                       .max_height = 2160,
> +                       .step_height = MB_DIM,
> +               },
> +       },
> +};
> +
> +static irqreturn_t rockchip_vpu1_vepu_irq(int irq, void *dev_id)
> +{
> +       struct hantro_dev *vpu = dev_id;
> +       enum vb2_buffer_state state;
> +       u32 status;
> +
> +       status = vepu_read(vpu, H1_REG_INTERRUPT);
> +       state = (status & H1_REG_INTERRUPT_FRAME_RDY) ?
> +               VB2_BUF_STATE_DONE : VB2_BUF_STATE_ERROR;
> +
> +       vepu_write(vpu, 0, H1_REG_INTERRUPT);
> +       vepu_write(vpu, 0, H1_REG_AXI_CTRL);
> +
> +       hantro_irq_done(vpu, state);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t rockchip_vpu2_vdpu_irq(int irq, void *dev_id)
> +{
> +       struct hantro_dev *vpu = dev_id;
> +       enum vb2_buffer_state state;
> +       u32 status;
> +
> +       status = vdpu_read(vpu, VDPU_REG_INTERRUPT);
> +       state = (status & VDPU_REG_INTERRUPT_DEC_IRQ) ?
> +               VB2_BUF_STATE_DONE : VB2_BUF_STATE_ERROR;
> +
> +       vdpu_write(vpu, 0, VDPU_REG_INTERRUPT);
> +       vdpu_write(vpu, 0, VDPU_REG_AXI_CTRL);
> +
> +       hantro_irq_done(vpu, state);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t rockchip_vpu2_vepu_irq(int irq, void *dev_id)
> +{
> +       struct hantro_dev *vpu = dev_id;
> +       enum vb2_buffer_state state;
> +       u32 status;
> +
> +       status = vepu_read(vpu, VEPU_REG_INTERRUPT);
> +       state = (status & VEPU_REG_INTERRUPT_FRAME_READY) ?
> +               VB2_BUF_STATE_DONE : VB2_BUF_STATE_ERROR;
> +
> +       vepu_write(vpu, 0, VEPU_REG_INTERRUPT);
> +       vepu_write(vpu, 0, VEPU_REG_AXI_CTRL);
> +
> +       hantro_irq_done(vpu, state);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static int rockchip_vpu_hw_init(struct hantro_dev *vpu)
> +{
> +       /* Bump ACLK to max. possible freq. to improve performance. */
> +       clk_set_rate(vpu->clocks[0].clk, RK3288_ACLK_MAX_FREQ);
> +       return 0;
> +}
> +
> +static void rockchip_vpu1_enc_reset(struct hantro_ctx *ctx)
> +{
> +       struct hantro_dev *vpu = ctx->dev;
> +
> +       vepu_write(vpu, H1_REG_INTERRUPT_DIS_BIT, H1_REG_INTERRUPT);
> +       vepu_write(vpu, 0, H1_REG_ENC_CTRL);
> +       vepu_write(vpu, 0, H1_REG_AXI_CTRL);
> +}
> +
> +static void rockchip_vpu2_dec_reset(struct hantro_ctx *ctx)
> +{
> +       struct hantro_dev *vpu = ctx->dev;
> +
> +       vdpu_write(vpu, VDPU_REG_INTERRUPT_DEC_IRQ_DIS, VDPU_REG_INTERRUPT);
> +       vdpu_write(vpu, 0, VDPU_REG_EN_FLAGS);
> +       vdpu_write(vpu, 1, VDPU_REG_SOFT_RESET);
> +}
> +
> +static void rockchip_vpu2_enc_reset(struct hantro_ctx *ctx)
> +{
> +       struct hantro_dev *vpu = ctx->dev;
> +
> +       vepu_write(vpu, VEPU_REG_INTERRUPT_DIS_BIT, VEPU_REG_INTERRUPT);
> +       vepu_write(vpu, 0, VEPU_REG_ENCODE_START);
> +       vepu_write(vpu, 0, VEPU_REG_AXI_CTRL);
> +}
> +
> +/*
> + * Supported codec ops.
> + */
> +
> +static const struct hantro_codec_ops rk3288_vpu_codec_ops[] = {
> +       [HANTRO_MODE_JPEG_ENC] = {
> +               .run = hantro_h1_jpeg_enc_run,
> +               .reset = rockchip_vpu1_enc_reset,
> +               .init = hantro_jpeg_enc_init,
> +               .done = hantro_jpeg_enc_done,
> +               .exit = hantro_jpeg_enc_exit,
> +       },
> +       [HANTRO_MODE_H264_DEC] = {
> +               .run = hantro_g1_h264_dec_run,
> +               .reset = hantro_g1_reset,
> +               .init = hantro_h264_dec_init,
> +               .exit = hantro_h264_dec_exit,
> +       },
> +       [HANTRO_MODE_MPEG2_DEC] = {
> +               .run = hantro_g1_mpeg2_dec_run,
> +               .reset = hantro_g1_reset,
> +               .init = hantro_mpeg2_dec_init,
> +               .exit = hantro_mpeg2_dec_exit,
> +       },
> +       [HANTRO_MODE_VP8_DEC] = {
> +               .run = hantro_g1_vp8_dec_run,
> +               .reset = hantro_g1_reset,
> +               .init = hantro_vp8_dec_init,
> +               .exit = hantro_vp8_dec_exit,
> +       },
> +};
> +
> +static const struct hantro_codec_ops rk3399_vpu_codec_ops[] = {
> +       [HANTRO_MODE_JPEG_ENC] = {
> +               .run = rockchip_vpu2_jpeg_enc_run,
> +               .reset = rockchip_vpu2_enc_reset,
> +               .init = hantro_jpeg_enc_init,
> +               .exit = hantro_jpeg_enc_exit,
> +       },
> +       [HANTRO_MODE_MPEG2_DEC] = {
> +               .run = rockchip_vpu2_mpeg2_dec_run,
> +               .reset = rockchip_vpu2_dec_reset,
> +               .init = hantro_mpeg2_dec_init,
> +               .exit = hantro_mpeg2_dec_exit,
> +       },
> +       [HANTRO_MODE_VP8_DEC] = {
> +               .run = rockchip_vpu2_vp8_dec_run,
> +               .reset = rockchip_vpu2_dec_reset,
> +               .init = hantro_vp8_dec_init,
> +               .exit = hantro_vp8_dec_exit,
> +       },
> +};
> +
> +/*
> + * VPU variant.
> + */
> +
> +static const struct hantro_irq rockchip_vpu1_irqs[] = {
> +       { "vepu", rockchip_vpu1_vepu_irq },
> +       { "vdpu", hantro_g1_irq },
> +};
> +
> +static const struct hantro_irq rockchip_vpdu2_irqs[] = {

Typo: vpdu -> vdpu?

> +       { "vdpu", rockchip_vpu2_vdpu_irq },
> +};
> +

Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>

Thanks,
Ezequiel
Ezequiel Garcia June 11, 2021, 3:50 p.m. UTC | #12
Hi Alex,

On Thu, 2021-05-27 at 17:44 +0200, Alex Bee wrote:
> RK3066's VPU IP block is the predecessor from what RK3288 has.
> The hardware differences are:
>   - supports decoding frame sizes up to 1920x1088 only
>   - doesn't have the 'G1_REG_SOFT_RESET' register
>     (requires another .reset callback for hantro_codec_ops,
>      since writing this register will result in non-working
>      IP block)
>   - has one ACLK/HCLK per vdpu/vepu
>   - ACLKs can be clocked up to 300 MHz only

Have you tested trying 400 MHz (or more) ?

>   - no MMU
>     (no changes required: CMA will be transparently used)
> 
> Add a new RK3066 variant which reflect this differences. This variant
> can be used for RK3188 as well.
> 

Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>

Thanks!
Ezequiel
Ezequiel Garcia June 11, 2021, 3:58 p.m. UTC | #13
Hi Heiko,

On Thu, 2021-05-27 at 17:44 +0200, Alex Bee wrote:
> Add the power controller node and the correspondending qos nodes for
> RK3036.
> Also add the power-domain property to the nodes that are already
> present.
> Note: Since the regiser offsets of the axi interconnect QoS are missing
> in the TRM (RK3036 TRM V1.0), they have been taken from vendor kernel.
> 

Can you take care of the device tree changes (patches 8 to 12)?

Thanks!
Ezequiel
Heiko Stuebner June 13, 2021, 4:27 p.m. UTC | #14
On Thu, 27 May 2021 17:44:43 +0200, Alex Bee wrote:
> this series adds support for older Rockchip SoCs (RK3036, RK3066, RK3188
> and RK322x) to the existing V4L2 video decoder/-encoder drivers - namely
> hantro and rkvdec.
> They can be used as-is or with very little modifications.
> 
> In preparation to that patches 1,8 and 9 add power-controller support for
> RK3036 and RK322x, since both drivers rely on pm. The drivers for them
> exist already in the common Rockchip pm driver, they just haven't be added
> to the device trees yet.
> 
> [...]

Removed some unnecessary added empty lines and

Applied, thanks!

[08/12] ARM: dts: rockchip: add power controller for RK3036
        commit: 1a4eb37f3174d3a54e40392abcfbb9b3949948bb
[09/12] ARM: dts: rockchip: add power controller for RK322x
        commit: 623ba75a5d6b8e196a21f3ed36d26a5f6db459ce

Best regards,
Hans Verkuil June 14, 2021, 11:09 a.m. UTC | #15
Hi Alex,

On 27/05/2021 17:44, Alex Bee wrote:
> Hi list,

> 

> this series adds support for older Rockchip SoCs (RK3036, RK3066, RK3188

> and RK322x) to the existing V4L2 video decoder/-encoder drivers - namely

> hantro and rkvdec.

> They can be used as-is or with very little modifications.

> 

> In preparation to that patches 1,8 and 9 add power-controller support for

> RK3036 and RK322x, since both drivers rely on pm. The drivers for them 

> exist already in the common Rockchip pm driver, they just haven't be added

> to the device trees yet.

> 

> Changes in v2:

> - reordered patches as suggested by Heiko

> - fixed indentation and order issues in dt-bindings / DT patches

> - added patch to reorder variants in hantro alphanumeric

> - added patch that merges hantro platform drivers for Rockchip in one

> - added missing DT patch for RK3036 (missed to submit with v1)

> See individual patches for details about changes.

> 

> Thanks for your feedback,

> Alex.

> 

> Alex Bee (12):

>   dt-bindings: mfd: syscon: add Rockchip RK3036/RK3228 qos compatibles

>   dt-bindings: media: rockchip-vpu: add new compatibles

>   dt-bindings: media: rockchip-vdec: add RK3228 compatible

>   media: hantro: reorder variants

>   media: hantro: merge Rockchip platform drivers

>   media: hantro: add support for Rockchip RK3066

>   media: hantro: add support for Rockchip RK3036

>   ARM: dts: rockchip: add power controller for RK3036

>   ARM: dts: rockchip: add power controller for RK322x

>   ARM: dts: rockchip: add vpu node for RK3036

>   ARM: dts: rockchip: add vpu nodes for RK3066 and RK3188

>   ARM: dts: rockchip: add vpu and vdec node for RK322x


Unfortunately this series no longer applies. Support for the G2/HEVC decoder
was merged, and that conflicts with this series.

It is best to rebase you series to the master branch of the media_stage
tree (https://git.linuxtv.org/media_stage.git/log/), which contains that
series. I expect/hope that this will be merged later this week to the
regular media_tree master branch, so you can wait for that as well.

Regards,

	Hans

> 

>  .../bindings/media/rockchip,vdec.yaml         |  10 +-

>  .../bindings/media/rockchip-vpu.yaml          |  33 +-

>  .../devicetree/bindings/mfd/syscon.yaml       |   2 +

>  arch/arm/boot/dts/rk3036.dtsi                 |  72 +++

>  arch/arm/boot/dts/rk3066a.dtsi                |   4 +

>  arch/arm/boot/dts/rk3188.dtsi                 |   5 +

>  arch/arm/boot/dts/rk322x.dtsi                 | 140 ++++-

>  arch/arm/boot/dts/rk3xxx.dtsi                 |  12 +

>  drivers/staging/media/hantro/Makefile         |   9 +-

>  drivers/staging/media/hantro/hantro_drv.c     |   6 +-

>  drivers/staging/media/hantro/hantro_hw.h      |  30 +-

>  drivers/staging/media/hantro/rk3288_vpu_hw.c  | 208 -------

>  drivers/staging/media/hantro/rk3399_vpu_hw.c  | 222 --------

>  ...jpeg_enc.c => rockchip_vpu2_hw_jpeg_enc.c} |  30 +-

>  ...eg2_dec.c => rockchip_vpu2_hw_mpeg2_dec.c} |  25 +-

>  ...w_vp8_dec.c => rockchip_vpu2_hw_vp8_dec.c} |   2 +-

>  ...rk3399_vpu_regs.h => rockchip_vpu2_regs.h} |   6 +-

>  .../staging/media/hantro/rockchip_vpu_hw.c    | 526 ++++++++++++++++++

>  18 files changed, 848 insertions(+), 494 deletions(-)

>  delete mode 100644 drivers/staging/media/hantro/rk3288_vpu_hw.c

>  delete mode 100644 drivers/staging/media/hantro/rk3399_vpu_hw.c

>  rename drivers/staging/media/hantro/{rk3399_vpu_hw_jpeg_enc.c => rockchip_vpu2_hw_jpeg_enc.c} (87%)

>  rename drivers/staging/media/hantro/{rk3399_vpu_hw_mpeg2_dec.c => rockchip_vpu2_hw_mpeg2_dec.c} (93%)

>  rename drivers/staging/media/hantro/{rk3399_vpu_hw_vp8_dec.c => rockchip_vpu2_hw_vp8_dec.c} (99%)

>  rename drivers/staging/media/hantro/{rk3399_vpu_regs.h => rockchip_vpu2_regs.h} (99%)

>  create mode 100644 drivers/staging/media/hantro/rockchip_vpu_hw.c

> 

> 

> base-commit: 5d765451c2409e63563fa6a3e8005bd03ab9e82f

>
Alex Bee June 14, 2021, 5:02 p.m. UTC | #16
Hi Ezequiel,

Am 11.06.21 um 17:36 schrieb Ezequiel Garcia:
> Hi Alex,
>
> This cleanup looks nice, thanks!
>
> Just a comment about the clock rate and a minor typo.
>
> On Thu, 2021-05-27 at 17:44 +0200, Alex Bee wrote:
>> Merge the two Rockchip platform drivers into one as it was suggested at
>> [1] and [2].
>> This will hopefully make it easier to add new variants (which are surely
>> to come for Rockchip).
>> Also rename from "rk3288" to "v(d/e)pu1" and "rk3399" to "v(d/e)pu2"
>> where applicable, as this is the dicition the vendor uses and will
>> also refelect the variants that get added later in this series. Rename
>> from "rk3288" to "rockchip" if applicable to both hardware versions.
>>
>> [1] https://patchwork.kernel.org/project/linux-rockchip/patch/20210107134101.195426-6-paul.kocialkowski@bootlin.com/
>> [2] https://patchwork.kernel.org/project/linux-rockchip/patch/20210525152225.154302-5-knaerzche@gmail.com/
>>
>> Signed-off-by: Alex Bee <knaerzche@gmail.com>
>> ---
>>
>>   Changes in v2:
>>   - added patch
>>
>>   drivers/staging/media/hantro/Makefile         |   9 +-
>>   drivers/staging/media/hantro/hantro_hw.h      |  22 +-
>>   drivers/staging/media/hantro/rk3288_vpu_hw.c  | 208 ----------
>>   drivers/staging/media/hantro/rk3399_vpu_hw.c  | 222 -----------
>>   ...jpeg_enc.c => rockchip_vpu2_hw_jpeg_enc.c} |  30 +-
>>   ...eg2_dec.c => rockchip_vpu2_hw_mpeg2_dec.c} |  25 +-
>>   ...w_vp8_dec.c => rockchip_vpu2_hw_vp8_dec.c} |   2 +-
>>   ...rk3399_vpu_regs.h => rockchip_vpu2_regs.h} |   6 +-
>>   .../staging/media/hantro/rockchip_vpu_hw.c    | 356 ++++++++++++++++++
>>   9 files changed, 402 insertions(+), 478 deletions(-)
>>   delete mode 100644 drivers/staging/media/hantro/rk3288_vpu_hw.c
>>   delete mode 100644 drivers/staging/media/hantro/rk3399_vpu_hw.c
>>   rename drivers/staging/media/hantro/{rk3399_vpu_hw_jpeg_enc.c => rockchip_vpu2_hw_jpeg_enc.c} (87%)
>>   rename drivers/staging/media/hantro/{rk3399_vpu_hw_mpeg2_dec.c => rockchip_vpu2_hw_mpeg2_dec.c} (93%)
>>   rename drivers/staging/media/hantro/{rk3399_vpu_hw_vp8_dec.c => rockchip_vpu2_hw_vp8_dec.c} (99%)
>>   rename drivers/staging/media/hantro/{rk3399_vpu_regs.h => rockchip_vpu2_regs.h} (99%)
>>   create mode 100644 drivers/staging/media/hantro/rockchip_vpu_hw.c
>>
> [..]
>> diff --git a/drivers/staging/media/hantro/rockchip_vpu_hw.c b/drivers/staging/media/hantro/rockchip_vpu_hw.c
>> new file mode 100644
>> index 000000000000..175d0c5dfdbe
>> --- /dev/null
>> +++ b/drivers/staging/media/hantro/rockchip_vpu_hw.c
>> @@ -0,0 +1,356 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Hantro VPU codec driver
>> + *
>> + * Copyright (C) 2018 Rockchip Electronics Co., Ltd.
>> + *     Jeffy Chen <jeffy.chen@rock-chips.com>
>> + */
>> +
>> +#include <linux/clk.h>
>> +
>> +#include "hantro.h"
>> +#include "hantro_jpeg.h"
>> +#include "hantro_h1_regs.h"
>> +#include "rockchip_vpu2_regs.h"
>> +
>> +#define RK3288_ACLK_MAX_FREQ (400 * 1000 * 1000)
>> +
> Something for later, could we bump this clock?
>
> Rockchip uses 300MHz and 600MHz even for RK3288,
> see arch/arm/boot/dts/rk3288.dtsi:
>
>          vdpu: vdpu@ff9a0400 {
>                  compatible = "rockchip,vpu-decoder-rk3288", "rockchip,vpu-decoder-v1";
>                  reg = <0x0 0xff9a0400 0x0 0x400>;
>                  interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
>                  interrupt-names = "irq_dec";
>                  clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>;
>                  clock-names = "aclk_vcodec", "hclk_vcodec";
>                  rockchip,normal-rates = <300000000>, <0>;
>                  rockchip,advanced-rates = <600000000>, <0>;
>
> On vdpu1 for instance, it switches to the "advanced-rate"
> when width > 2560, which I think it's too naive for us.
>
> At least on RK3566 I tested bumping this rate, while still
> passing conformance tests.

Yes, RK3288's can (and maybe also should) run at 600 MHz (since we are 
sugesting to "Bump ACLK to max. possible freq. to improve performance." 
currently) - but I never ran conformance tests after setting this rate, tbh.

I'm also not sure, VDPU2 (RK3399) variant supports VP8 up to 3840x2160p. 
It would be the only codec supported up that frame size (we already 
know, that H.264 is supported up to 1920x1088, even if it is not 
supported currently by upstream hantro driver) and TRM says the hantro 
IP block is a "1080P encoder/decoder Processing Core", without saying 
anything specific per codec.

Both should be checked and addressed in a upcoming series.

>> +/*
>> + * Supported formats.
>> + */
>> +
>> +static const struct hantro_fmt rockchip_vpu_enc_fmts[] = {
>> +       {
>> +               .fourcc = V4L2_PIX_FMT_YUV420M,
>> +               .codec_mode = HANTRO_MODE_NONE,
>> +               .enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUV420P,
>> +       },
>> +       {
>> +               .fourcc = V4L2_PIX_FMT_NV12M,
>> +               .codec_mode = HANTRO_MODE_NONE,
>> +               .enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUV420SP,
>> +       },
>> +       {
>> +               .fourcc = V4L2_PIX_FMT_YUYV,
>> +               .codec_mode = HANTRO_MODE_NONE,
>> +               .enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUYV422,
>> +       },
>> +       {
>> +               .fourcc = V4L2_PIX_FMT_UYVY,
>> +               .codec_mode = HANTRO_MODE_NONE,
>> +               .enc_fmt = ROCKCHIP_VPU_ENC_FMT_UYVY422,
>> +       },
>> +       {
>> +               .fourcc = V4L2_PIX_FMT_JPEG,
>> +               .codec_mode = HANTRO_MODE_JPEG_ENC,
>> +               .max_depth = 2,
>> +               .header_size = JPEG_HEADER_SIZE,
>> +               .frmsize = {
>> +                       .min_width = 96,
>> +                       .max_width = 8192,
>> +                       .step_width = MB_DIM,
>> +                       .min_height = 32,
>> +                       .max_height = 8192,
>> +                       .step_height = MB_DIM,
>> +               },
>> +       },
>> +};
>> +
>> +static const struct hantro_fmt rockchip_vpu1_postproc_fmts[] = {
>> +       {
>> +               .fourcc = V4L2_PIX_FMT_YUYV,
>> +               .codec_mode = HANTRO_MODE_NONE,
>> +       },
>> +};
>> +
>> +static const struct hantro_fmt rk3288_vpu_dec_fmts[] = {
>> +       {
>> +               .fourcc = V4L2_PIX_FMT_NV12,
>> +               .codec_mode = HANTRO_MODE_NONE,
>> +       },
>> +       {
>> +               .fourcc = V4L2_PIX_FMT_H264_SLICE,
>> +               .codec_mode = HANTRO_MODE_H264_DEC,
>> +               .max_depth = 2,
>> +               .frmsize = {
>> +                       .min_width = 48,
>> +                       .max_width = 4096,
>> +                       .step_width = MB_DIM,
>> +                       .min_height = 48,
>> +                       .max_height = 2304,
>> +                       .step_height = MB_DIM,
>> +               },
>> +       },
>> +       {
>> +               .fourcc = V4L2_PIX_FMT_MPEG2_SLICE,
>> +               .codec_mode = HANTRO_MODE_MPEG2_DEC,
>> +               .max_depth = 2,
>> +               .frmsize = {
>> +                       .min_width = 48,
>> +                       .max_width = 1920,
>> +                       .step_width = MB_DIM,
>> +                       .min_height = 48,
>> +                       .max_height = 1088,
>> +                       .step_height = MB_DIM,
>> +               },
>> +       },
>> +       {
>> +               .fourcc = V4L2_PIX_FMT_VP8_FRAME,
>> +               .codec_mode = HANTRO_MODE_VP8_DEC,
>> +               .max_depth = 2,
>> +               .frmsize = {
>> +                       .min_width = 48,
>> +                       .max_width = 3840,
>> +                       .step_width = MB_DIM,
>> +                       .min_height = 48,
>> +                       .max_height = 2160,
>> +                       .step_height = MB_DIM,
>> +               },
>> +       },
>> +};
>> +
>> +static const struct hantro_fmt rk3399_vpu_dec_fmts[] = {
>> +       {
>> +               .fourcc = V4L2_PIX_FMT_NV12,
>> +               .codec_mode = HANTRO_MODE_NONE,
>> +       },
>> +       {
>> +               .fourcc = V4L2_PIX_FMT_MPEG2_SLICE,
>> +               .codec_mode = HANTRO_MODE_MPEG2_DEC,
>> +               .max_depth = 2,
>> +               .frmsize = {
>> +                       .min_width = 48,
>> +                       .max_width = 1920,
>> +                       .step_width = MB_DIM,
>> +                       .min_height = 48,
>> +                       .max_height = 1088,
>> +                       .step_height = MB_DIM,
>> +               },
>> +       },
>> +       {
>> +               .fourcc = V4L2_PIX_FMT_VP8_FRAME,
>> +               .codec_mode = HANTRO_MODE_VP8_DEC,
>> +               .max_depth = 2,
>> +               .frmsize = {
>> +                       .min_width = 48,
>> +                       .max_width = 3840,
>> +                       .step_width = MB_DIM,
>> +                       .min_height = 48,
>> +                       .max_height = 2160,
>> +                       .step_height = MB_DIM,
>> +               },
>> +       },
>> +};
>> +
>> +static irqreturn_t rockchip_vpu1_vepu_irq(int irq, void *dev_id)
>> +{
>> +       struct hantro_dev *vpu = dev_id;
>> +       enum vb2_buffer_state state;
>> +       u32 status;
>> +
>> +       status = vepu_read(vpu, H1_REG_INTERRUPT);
>> +       state = (status & H1_REG_INTERRUPT_FRAME_RDY) ?
>> +               VB2_BUF_STATE_DONE : VB2_BUF_STATE_ERROR;
>> +
>> +       vepu_write(vpu, 0, H1_REG_INTERRUPT);
>> +       vepu_write(vpu, 0, H1_REG_AXI_CTRL);
>> +
>> +       hantro_irq_done(vpu, state);
>> +
>> +       return IRQ_HANDLED;
>> +}
>> +
>> +static irqreturn_t rockchip_vpu2_vdpu_irq(int irq, void *dev_id)
>> +{
>> +       struct hantro_dev *vpu = dev_id;
>> +       enum vb2_buffer_state state;
>> +       u32 status;
>> +
>> +       status = vdpu_read(vpu, VDPU_REG_INTERRUPT);
>> +       state = (status & VDPU_REG_INTERRUPT_DEC_IRQ) ?
>> +               VB2_BUF_STATE_DONE : VB2_BUF_STATE_ERROR;
>> +
>> +       vdpu_write(vpu, 0, VDPU_REG_INTERRUPT);
>> +       vdpu_write(vpu, 0, VDPU_REG_AXI_CTRL);
>> +
>> +       hantro_irq_done(vpu, state);
>> +
>> +       return IRQ_HANDLED;
>> +}
>> +
>> +static irqreturn_t rockchip_vpu2_vepu_irq(int irq, void *dev_id)
>> +{
>> +       struct hantro_dev *vpu = dev_id;
>> +       enum vb2_buffer_state state;
>> +       u32 status;
>> +
>> +       status = vepu_read(vpu, VEPU_REG_INTERRUPT);
>> +       state = (status & VEPU_REG_INTERRUPT_FRAME_READY) ?
>> +               VB2_BUF_STATE_DONE : VB2_BUF_STATE_ERROR;
>> +
>> +       vepu_write(vpu, 0, VEPU_REG_INTERRUPT);
>> +       vepu_write(vpu, 0, VEPU_REG_AXI_CTRL);
>> +
>> +       hantro_irq_done(vpu, state);
>> +
>> +       return IRQ_HANDLED;
>> +}
>> +
>> +static int rockchip_vpu_hw_init(struct hantro_dev *vpu)
>> +{
>> +       /* Bump ACLK to max. possible freq. to improve performance. */
>> +       clk_set_rate(vpu->clocks[0].clk, RK3288_ACLK_MAX_FREQ);
>> +       return 0;
>> +}
>> +
>> +static void rockchip_vpu1_enc_reset(struct hantro_ctx *ctx)
>> +{
>> +       struct hantro_dev *vpu = ctx->dev;
>> +
>> +       vepu_write(vpu, H1_REG_INTERRUPT_DIS_BIT, H1_REG_INTERRUPT);
>> +       vepu_write(vpu, 0, H1_REG_ENC_CTRL);
>> +       vepu_write(vpu, 0, H1_REG_AXI_CTRL);
>> +}
>> +
>> +static void rockchip_vpu2_dec_reset(struct hantro_ctx *ctx)
>> +{
>> +       struct hantro_dev *vpu = ctx->dev;
>> +
>> +       vdpu_write(vpu, VDPU_REG_INTERRUPT_DEC_IRQ_DIS, VDPU_REG_INTERRUPT);
>> +       vdpu_write(vpu, 0, VDPU_REG_EN_FLAGS);
>> +       vdpu_write(vpu, 1, VDPU_REG_SOFT_RESET);
>> +}
>> +
>> +static void rockchip_vpu2_enc_reset(struct hantro_ctx *ctx)
>> +{
>> +       struct hantro_dev *vpu = ctx->dev;
>> +
>> +       vepu_write(vpu, VEPU_REG_INTERRUPT_DIS_BIT, VEPU_REG_INTERRUPT);
>> +       vepu_write(vpu, 0, VEPU_REG_ENCODE_START);
>> +       vepu_write(vpu, 0, VEPU_REG_AXI_CTRL);
>> +}
>> +
>> +/*
>> + * Supported codec ops.
>> + */
>> +
>> +static const struct hantro_codec_ops rk3288_vpu_codec_ops[] = {
>> +       [HANTRO_MODE_JPEG_ENC] = {
>> +               .run = hantro_h1_jpeg_enc_run,
>> +               .reset = rockchip_vpu1_enc_reset,
>> +               .init = hantro_jpeg_enc_init,
>> +               .done = hantro_jpeg_enc_done,
>> +               .exit = hantro_jpeg_enc_exit,
>> +       },
>> +       [HANTRO_MODE_H264_DEC] = {
>> +               .run = hantro_g1_h264_dec_run,
>> +               .reset = hantro_g1_reset,
>> +               .init = hantro_h264_dec_init,
>> +               .exit = hantro_h264_dec_exit,
>> +       },
>> +       [HANTRO_MODE_MPEG2_DEC] = {
>> +               .run = hantro_g1_mpeg2_dec_run,
>> +               .reset = hantro_g1_reset,
>> +               .init = hantro_mpeg2_dec_init,
>> +               .exit = hantro_mpeg2_dec_exit,
>> +       },
>> +       [HANTRO_MODE_VP8_DEC] = {
>> +               .run = hantro_g1_vp8_dec_run,
>> +               .reset = hantro_g1_reset,
>> +               .init = hantro_vp8_dec_init,
>> +               .exit = hantro_vp8_dec_exit,
>> +       },
>> +};
>> +
>> +static const struct hantro_codec_ops rk3399_vpu_codec_ops[] = {
>> +       [HANTRO_MODE_JPEG_ENC] = {
>> +               .run = rockchip_vpu2_jpeg_enc_run,
>> +               .reset = rockchip_vpu2_enc_reset,
>> +               .init = hantro_jpeg_enc_init,
>> +               .exit = hantro_jpeg_enc_exit,
>> +       },
>> +       [HANTRO_MODE_MPEG2_DEC] = {
>> +               .run = rockchip_vpu2_mpeg2_dec_run,
>> +               .reset = rockchip_vpu2_dec_reset,
>> +               .init = hantro_mpeg2_dec_init,
>> +               .exit = hantro_mpeg2_dec_exit,
>> +       },
>> +       [HANTRO_MODE_VP8_DEC] = {
>> +               .run = rockchip_vpu2_vp8_dec_run,
>> +               .reset = rockchip_vpu2_dec_reset,
>> +               .init = hantro_vp8_dec_init,
>> +               .exit = hantro_vp8_dec_exit,
>> +       },
>> +};
>> +
>> +/*
>> + * VPU variant.
>> + */
>> +
>> +static const struct hantro_irq rockchip_vpu1_irqs[] = {
>> +       { "vepu", rockchip_vpu1_vepu_irq },
>> +       { "vdpu", hantro_g1_irq },
>> +};
>> +
>> +static const struct hantro_irq rockchip_vpdu2_irqs[] = {
> Typo: vpdu -> vdpu?
Will fix it in v3.
>
>> +       { "vdpu", rockchip_vpu2_vdpu_irq },
>> +};
>> +
> Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>
>
> Thanks,
> Ezequiel
>
Alex
Alex Bee June 14, 2021, 9:32 p.m. UTC | #17
Hi list,

this is v3 of series, that adds support for older Rockchip SoCs (RK3036,
RK3066, RK3188 and RK322x) to the existing V4L2 video decoder/-encoder
drivers - namely hantro and rkvdec.
They can be used as-is or with very little modifications.

In preparation to that previous versions added power-controller support for
RK3036 and RK322x, since both drivers rely on pm. The drivers for them
exist already in the common Rockchip pm driver, they just haven't been
added to the device trees yet.
Those patches have been applied already.

This is a basically a rebase of the patches that should get merged in
media_tree.
Please pick the DT patches, which are not applied yet from v2.

Changes in v3:
- rebased on current media_tree
- collected Reviewd-tags from v2 

Changes in v2:
- reordered patches as suggested by Heiko
- fixed indentation and order issues in dt-bindings / DT patches
- added patch to reorder variants in hantro alphanumeric
- added patch that merges hantro platform drivers for Rockchip in one
- added missing DT patch for RK3036 (missed to submit with v1)

See individual patches for details about changes.

Thanks for your feedback,
Alex.

Alex Bee (6):
  dt-bindings: media: rockchip-vpu: add new compatibles
  dt-bindings: media: rockchip-vdec: add RK3228 compatible
  media: hantro: reorder variants
  media: hantro: merge Rockchip platform drivers
  media: hantro: add support for Rockchip RK3066
  media: hantro: add support for Rockchip RK3036

 .../bindings/media/rockchip,vdec.yaml         |  10 +-
 .../bindings/media/rockchip-vpu.yaml          |  33 +-
 drivers/staging/media/hantro/Makefile         |   9 +-
 drivers/staging/media/hantro/hantro_drv.c     |   6 +-
 drivers/staging/media/hantro/hantro_hw.h      |  32 +-
 drivers/staging/media/hantro/rk3288_vpu_hw.c  | 208 -------
 drivers/staging/media/hantro/rk3399_vpu_hw.c  | 222 --------
 ...jpeg_enc.c => rockchip_vpu2_hw_jpeg_enc.c} |  30 +-
 ...eg2_dec.c => rockchip_vpu2_hw_mpeg2_dec.c} |  25 +-
 ...w_vp8_dec.c => rockchip_vpu2_hw_vp8_dec.c} |   2 +-
 ...rk3399_vpu_regs.h => rockchip_vpu2_regs.h} |   6 +-
 .../staging/media/hantro/rockchip_vpu_hw.c    | 526 ++++++++++++++++++
 12 files changed, 616 insertions(+), 493 deletions(-)
 delete mode 100644 drivers/staging/media/hantro/rk3288_vpu_hw.c
 delete mode 100644 drivers/staging/media/hantro/rk3399_vpu_hw.c
 rename drivers/staging/media/hantro/{rk3399_vpu_hw_jpeg_enc.c => rockchip_vpu2_hw_jpeg_enc.c} (87%)
 rename drivers/staging/media/hantro/{rk3399_vpu_hw_mpeg2_dec.c => rockchip_vpu2_hw_mpeg2_dec.c} (94%)
 rename drivers/staging/media/hantro/{rk3399_vpu_hw_vp8_dec.c => rockchip_vpu2_hw_vp8_dec.c} (99%)
 rename drivers/staging/media/hantro/{rk3399_vpu_regs.h => rockchip_vpu2_regs.h} (99%)
 create mode 100644 drivers/staging/media/hantro/rockchip_vpu_hw.c


base-commit: 45040f675041956ad763f9ef139ecee3647aa8bb
Alex Bee June 21, 2021, 11:58 a.m. UTC | #18
Hi Heiko,

Am 13.06.21 um 18:22 schrieb Heiko Stuebner:
> Am Freitag, 11. Juni 2021, 17:58:58 CEST schrieb Ezequiel Garcia:
>> Hi Heiko,
>>
>> On Thu, 2021-05-27 at 17:44 +0200, Alex Bee wrote:
>>> Add the power controller node and the correspondending qos nodes for
>>> RK3036.
>>> Also add the power-domain property to the nodes that are already
>>> present.
>>> Note: Since the regiser offsets of the axi interconnect QoS are missing
>>> in the TRM (RK3036 TRM V1.0), they have been taken from vendor kernel.
>>>
>> Can you take care of the device tree changes (patches 8 to 12)?
> sure, I'll pick the power-domains now but need to wait for
> the vpu nodes for the driver parts to land in the media tree.

Looks like I'm too late for 5.14 - just in case you didn't follow or 
didn't receive a notification:

hantro patches of this series have been merged in media_tree and vpu 
node patches (please pick from v2) could get reviewed/applied now.

Thanks,

Alex

>
> Heiko
>
>
Heiko Stuebner June 21, 2021, 11:04 p.m. UTC | #19
On Thu, 27 May 2021 17:44:43 +0200, Alex Bee wrote:
> this series adds support for older Rockchip SoCs (RK3036, RK3066, RK3188
> and RK322x) to the existing V4L2 video decoder/-encoder drivers - namely
> hantro and rkvdec.
> They can be used as-is or with very little modifications.
> 
> In preparation to that patches 1,8 and 9 add power-controller support for
> RK3036 and RK322x, since both drivers rely on pm. The drivers for them
> exist already in the common Rockchip pm driver, they just haven't be added
> to the device trees yet.
> 
> [...]

Applied, thanks!

[10/12] ARM: dts: rockchip: add vpu node for RK3036
        commit: 9d34d4aa896d00d398d799caa839a1494ba7c018
[11/12] ARM: dts: rockchip: add vpu nodes for RK3066 and RK3188
        commit: db3fc8fa0fcfa481cd8087c2ee068d1d1988c3a2
[12/12] ARM: dts: rockchip: add vpu and vdec node for RK322x
        commit: 36e9534dfcb5b09b919d2831d6a19aa3856b95a1

Best regards,
Alex Bee July 15, 2021, 10:09 a.m. UTC | #20
Hi Lee, Heiko,

Am 11.07.21 um 13:55 schrieb Heiko Stübner:
> Hi Lee,
>
> Am Dienstag, 1. Juni 2021, 17:46:51 CEST schrieb Lee Jones:
>> On Thu, 27 May 2021, Alex Bee wrote:
>>
>>> Document Rockchip RK3036/RK3228 qos compatibles
>>>
>>> Signed-off-by: Alex Bee <knaerzche@gmail.com>
>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>>> ---
>>>
>>>   Changes in v2:
>>>   - collect Reviewed tag
>>>
>>>   Documentation/devicetree/bindings/mfd/syscon.yaml | 2 ++
>>>   1 file changed, 2 insertions(+)
>> Applied, thanks.
> not sure if I'm missing something, but this patch wasn't part of your 5.14?
> And I also don't see it in your for-mfd-next branch. Did it get lost somewhere?

I can't find this patch in neihter the mentioned trees nor in 5.14-rc1.

Lee, could you queue it for rc2, please?

Thanks,

Alex

> Thanks
> Heiko
>
>
Lee Jones July 15, 2021, 11:07 a.m. UTC | #21
On Thu, 15 Jul 2021, Alex Bee wrote:

> Hi Lee, Heiko,

> 

> Am 11.07.21 um 13:55 schrieb Heiko Stübner:

> > Hi Lee,

> > 

> > Am Dienstag, 1. Juni 2021, 17:46:51 CEST schrieb Lee Jones:

> > > On Thu, 27 May 2021, Alex Bee wrote:

> > > 

> > > > Document Rockchip RK3036/RK3228 qos compatibles

> > > > 

> > > > Signed-off-by: Alex Bee <knaerzche@gmail.com>

> > > > Reviewed-by: Heiko Stuebner <heiko@sntech.de>

> > > > ---

> > > > 

> > > >   Changes in v2:

> > > >   - collect Reviewed tag

> > > > 

> > > >   Documentation/devicetree/bindings/mfd/syscon.yaml | 2 ++

> > > >   1 file changed, 2 insertions(+)

> > > Applied, thanks.

> > not sure if I'm missing something, but this patch wasn't part of your 5.14?

> > And I also don't see it in your for-mfd-next branch. Did it get lost somewhere?


Hmm... looks like it was dropped some how.  Apologies for that.

> I can't find this patch in neihter the mentioned trees nor in 5.14-rc1.

> 

> Lee, could you queue it for rc2, please?


Unfortunately, that's not how it works.

Only bug fixes are allowed for inclusion into the rcs.

I'll re-queue for v5.15 though.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog