| Message ID | 20210525152225.154302-1-knaerzche@gmail.com |
|---|---|
| Headers | show |
| Series | Add support for older Rockchip SoCs to V4L2 hantro and rkvdec drivers | expand |
Am Dienstag, 25. Mai 2021, 17:22:19 CEST schrieb Alex Bee: > 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 > - 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. > > 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 | 116 +++++++++++++++++++ > 3 files changed, 118 insertions(+) > > diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c > index 2f6b01c7a6a0..38ea7b24036e 100644 > --- a/drivers/staging/media/hantro/hantro_drv.c > +++ b/drivers/staging/media/hantro/hantro_drv.c > @@ -489,6 +489,7 @@ static const struct of_device_id of_hantro_match[] = { > { .compatible = "rockchip,rk3399-vpu", .data = &rk3399_vpu_variant, }, > { .compatible = "rockchip,rk3328-vpu", .data = &rk3328_vpu_variant, }, > { .compatible = "rockchip,rk3288-vpu", .data = &rk3288_vpu_variant, }, > + { .compatible = "rockchip,rk3066-vpu", .data = &rk3066_vpu_variant, }, NIT: "someone" should introduce a separate patch ordering that list ;-) > #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 3d8b53567f16..de2bc367a15a 100644 > --- a/drivers/staging/media/hantro/hantro_hw.h > +++ b/drivers/staging/media/hantro/hantro_hw.h > @@ -163,6 +163,7 @@ enum hantro_enc_fmt { > 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 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 fefd45269e52..29805c4bd92f 100644 > --- a/drivers/staging/media/hantro/rk3288_vpu_hw.c > +++ b/drivers/staging/media/hantro/rk3288_vpu_hw.c > @@ -10,8 +10,10 @@ > > #include "hantro.h" > #include "hantro_jpeg.h" > +#include "hantro_g1_regs.h" > #include "hantro_h1_regs.h" > > +#define RK3066_ACLK_MAX_FREQ (300 * 1000 * 1000) > #define RK3288_ACLK_MAX_FREQ (400 * 1000 * 1000) > > /* > @@ -62,6 +64,52 @@ static const struct hantro_fmt rk3288_vpu_postproc_fmts[] = { > }, > }; > > +static const struct hantro_fmt rk3066_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 = 1920, > + .step_width = MB_DIM, > + .min_height = 48, > + .max_height = 1088, > + .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 = 1920, > + .step_width = MB_DIM, > + .min_height = 48, > + .max_height = 1088, > + .step_height = MB_DIM, > + }, > + }, > +}; > + > static const struct hantro_fmt rk3288_vpu_dec_fmts[] = { > { > .fourcc = V4L2_PIX_FMT_NV12, > @@ -126,6 +174,14 @@ static irqreturn_t rk3288_vepu_irq(int irq, void *dev_id) > return IRQ_HANDLED; > } > > +static int rk3066_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); > + clk_set_rate(vpu->clocks[0].clk, RK3066_ACLK_MAX_FREQ); hmm, I don't think that line was supposed to be double? Heiko > + return 0; > +} > + > static int rk3288_vpu_hw_init(struct hantro_dev *vpu) > { > /* Bump ACLK to max. possible freq. to improve performance. */ > @@ -133,6 +189,14 @@ static int rk3288_vpu_hw_init(struct hantro_dev *vpu) > return 0; > } > > +static void rk3066_vpu_dec_reset(struct hantro_ctx *ctx) > +{ > + struct hantro_dev *vpu = ctx->dev; > + > + vdpu_write(vpu, G1_REG_INTERRUPT_DEC_IRQ_DIS, G1_REG_INTERRUPT); > + vdpu_write(vpu, G1_REG_CONFIG_DEC_CLK_GATE_E, G1_REG_CONFIG); > +} > + > static void rk3288_vpu_enc_reset(struct hantro_ctx *ctx) > { > struct hantro_dev *vpu = ctx->dev; > @@ -145,6 +209,33 @@ static void rk3288_vpu_enc_reset(struct hantro_ctx *ctx) > /* > * Supported codec ops. > */ > +static const struct hantro_codec_ops rk3066_vpu_codec_ops[] = { > + [HANTRO_MODE_JPEG_ENC] = { > + .run = hantro_h1_jpeg_enc_run, > + .reset = rk3288_vpu_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 = rk3066_vpu_dec_reset, > + .init = hantro_h264_dec_init, > + .exit = hantro_h264_dec_exit, > + }, > + [HANTRO_MODE_MPEG2_DEC] = { > + .run = hantro_g1_mpeg2_dec_run, > + .reset = rk3066_vpu_dec_reset, > + .init = hantro_mpeg2_dec_init, > + .exit = hantro_mpeg2_dec_exit, > + }, > + [HANTRO_MODE_VP8_DEC] = { > + .run = hantro_g1_vp8_dec_run, > + .reset = rk3066_vpu_dec_reset, > + .init = hantro_vp8_dec_init, > + .exit = hantro_vp8_dec_exit, > + }, > +}; > > static const struct hantro_codec_ops rk3288_vpu_codec_ops[] = { > [HANTRO_MODE_JPEG_ENC] = { > @@ -183,10 +274,35 @@ static const struct hantro_irq rk3288_irqs[] = { > { "vdpu", hantro_g1_irq }, > }; > > +static const char * const rk3066_clk_names[] = { > + "aclk_vdpu", "hclk_vdpu", > + "aclk_vepu", "hclk_vepu" > +}; > + > static const char * const rk3288_clk_names[] = { > "aclk", "hclk" > }; > > +const struct hantro_variant rk3066_vpu_variant = { > + .enc_offset = 0x0, > + .enc_fmts = rk3288_vpu_enc_fmts, > + .num_enc_fmts = ARRAY_SIZE(rk3288_vpu_enc_fmts), > + .dec_offset = 0x400, > + .dec_fmts = rk3066_vpu_dec_fmts, > + .num_dec_fmts = ARRAY_SIZE(rk3066_vpu_dec_fmts), > + .postproc_fmts = rk3288_vpu_postproc_fmts, > + .num_postproc_fmts = ARRAY_SIZE(rk3288_vpu_postproc_fmts), > + .postproc_regs = &hantro_g1_postproc_regs, > + .codec = HANTRO_JPEG_ENCODER | HANTRO_MPEG2_DECODER | > + HANTRO_VP8_DECODER | HANTRO_H264_DECODER, > + .codec_ops = rk3066_vpu_codec_ops, > + .irqs = rk3288_irqs, > + .num_irqs = ARRAY_SIZE(rk3288_irqs), > + .init = rk3066_vpu_hw_init, > + .clk_names = rk3066_clk_names, > + .num_clocks = ARRAY_SIZE(rk3066_clk_names) > +}; > + > const struct hantro_variant rk3288_vpu_variant = { > .enc_offset = 0x0, > .enc_fmts = rk3288_vpu_enc_fmts, >
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 >
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 { >
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
Hi Alex, Thanks for the patch. On Tue, 2021-05-25 at 17:22 +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 > - 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. > > 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 | 116 +++++++++++++++++++ > 3 files changed, 118 insertions(+) > > diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c > index 2f6b01c7a6a0..38ea7b24036e 100644 > --- a/drivers/staging/media/hantro/hantro_drv.c > +++ b/drivers/staging/media/hantro/hantro_drv.c > @@ -489,6 +489,7 @@ static const struct of_device_id of_hantro_match[] = { > { .compatible = "rockchip,rk3399-vpu", .data = &rk3399_vpu_variant, }, > { .compatible = "rockchip,rk3328-vpu", .data = &rk3328_vpu_variant, }, > { .compatible = "rockchip,rk3288-vpu", .data = &rk3288_vpu_variant, }, > + { .compatible = "rockchip,rk3066-vpu", .data = &rk3066_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 3d8b53567f16..de2bc367a15a 100644 > --- a/drivers/staging/media/hantro/hantro_hw.h > +++ b/drivers/staging/media/hantro/hantro_hw.h > @@ -163,6 +163,7 @@ enum hantro_enc_fmt { > 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 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 fefd45269e52..29805c4bd92f 100644 > --- a/drivers/staging/media/hantro/rk3288_vpu_hw.c > +++ b/drivers/staging/media/hantro/rk3288_vpu_hw.c > @@ -10,8 +10,10 @@ > > #include "hantro.h" > #include "hantro_jpeg.h" > +#include "hantro_g1_regs.h" > #include "hantro_h1_regs.h" > > +#define RK3066_ACLK_MAX_FREQ (300 * 1000 * 1000) > #define RK3288_ACLK_MAX_FREQ (400 * 1000 * 1000) > > /* > @@ -62,6 +64,52 @@ static const struct hantro_fmt rk3288_vpu_postproc_fmts[] = { > }, > }; > > +static const struct hantro_fmt rk3066_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 = 1920, > + .step_width = MB_DIM, > + .min_height = 48, > + .max_height = 1088, > + .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 = 1920, > + .step_width = MB_DIM, > + .min_height = 48, > + .max_height = 1088, > + .step_height = MB_DIM, > + }, > + }, > +}; > + > static const struct hantro_fmt rk3288_vpu_dec_fmts[] = { > { > .fourcc = V4L2_PIX_FMT_NV12, > @@ -126,6 +174,14 @@ static irqreturn_t rk3288_vepu_irq(int irq, void *dev_id) > return IRQ_HANDLED; > } > > +static int rk3066_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); > + clk_set_rate(vpu->clocks[0].clk, RK3066_ACLK_MAX_FREQ); > + return 0; > +} > + > static int rk3288_vpu_hw_init(struct hantro_dev *vpu) > { > /* Bump ACLK to max. possible freq. to improve performance. */ > @@ -133,6 +189,14 @@ static int rk3288_vpu_hw_init(struct hantro_dev *vpu) > return 0; > } > > +static void rk3066_vpu_dec_reset(struct hantro_ctx *ctx) > +{ > + struct hantro_dev *vpu = ctx->dev; > + > + vdpu_write(vpu, G1_REG_INTERRUPT_DEC_IRQ_DIS, G1_REG_INTERRUPT); > + vdpu_write(vpu, G1_REG_CONFIG_DEC_CLK_GATE_E, G1_REG_CONFIG); > +} > + > static void rk3288_vpu_enc_reset(struct hantro_ctx *ctx) > { > struct hantro_dev *vpu = ctx->dev; > @@ -145,6 +209,33 @@ static void rk3288_vpu_enc_reset(struct hantro_ctx *ctx) > /* > * Supported codec ops. > */ > +static const struct hantro_codec_ops rk3066_vpu_codec_ops[] = { > + [HANTRO_MODE_JPEG_ENC] = { > + .run = hantro_h1_jpeg_enc_run, > + .reset = rk3288_vpu_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 = rk3066_vpu_dec_reset, > + .init = hantro_h264_dec_init, > + .exit = hantro_h264_dec_exit, > + }, > + [HANTRO_MODE_MPEG2_DEC] = { > + .run = hantro_g1_mpeg2_dec_run, > + .reset = rk3066_vpu_dec_reset, > + .init = hantro_mpeg2_dec_init, > + .exit = hantro_mpeg2_dec_exit, > + }, > + [HANTRO_MODE_VP8_DEC] = { > + .run = hantro_g1_vp8_dec_run, > + .reset = rk3066_vpu_dec_reset, > + .init = hantro_vp8_dec_init, > + .exit = hantro_vp8_dec_exit, > + }, > +}; > > static const struct hantro_codec_ops rk3288_vpu_codec_ops[] = { > [HANTRO_MODE_JPEG_ENC] = { > @@ -183,10 +274,35 @@ static const struct hantro_irq rk3288_irqs[] = { > { "vdpu", hantro_g1_irq }, > }; > > +static const char * const rk3066_clk_names[] = { > + "aclk_vdpu", "hclk_vdpu", > + "aclk_vepu", "hclk_vepu" > +}; > + > static const char * const rk3288_clk_names[] = { > "aclk", "hclk" > }; > > +const struct hantro_variant rk3066_vpu_variant = { > + .enc_offset = 0x0, > + .enc_fmts = rk3288_vpu_enc_fmts, > + .num_enc_fmts = ARRAY_SIZE(rk3288_vpu_enc_fmts), > + .dec_offset = 0x400, Having decoder and encoder supported by a single devicetree node was done for RK3288 to cope with some bug in the hardware that was effectively linking the decoder and the encoder. AFAIK, Rockchip has fixed this, so unless there's a strong need, I prefer we keep them separated, with one DT node for the g1 decoder and one for the h1 encoder. Thanks! Ezequiel
Hi Ezequiel, thanks for your feedback. Am 26.05.21 um 12:32 schrieb Ezequiel Garcia: > Hi Alex, > > Thanks for the patch. > > On Tue, 2021-05-25 at 17:22 +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 >> - 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. >> >> 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 | 116 +++++++++++++++++++ >> 3 files changed, 118 insertions(+) >> >> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c >> index 2f6b01c7a6a0..38ea7b24036e 100644 >> --- a/drivers/staging/media/hantro/hantro_drv.c >> +++ b/drivers/staging/media/hantro/hantro_drv.c >> @@ -489,6 +489,7 @@ static const struct of_device_id of_hantro_match[] = { >> { .compatible = "rockchip,rk3399-vpu", .data = &rk3399_vpu_variant, }, >> { .compatible = "rockchip,rk3328-vpu", .data = &rk3328_vpu_variant, }, >> { .compatible = "rockchip,rk3288-vpu", .data = &rk3288_vpu_variant, }, >> + { .compatible = "rockchip,rk3066-vpu", .data = &rk3066_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 3d8b53567f16..de2bc367a15a 100644 >> --- a/drivers/staging/media/hantro/hantro_hw.h >> +++ b/drivers/staging/media/hantro/hantro_hw.h >> @@ -163,6 +163,7 @@ enum hantro_enc_fmt { >> 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 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 fefd45269e52..29805c4bd92f 100644 >> --- a/drivers/staging/media/hantro/rk3288_vpu_hw.c >> +++ b/drivers/staging/media/hantro/rk3288_vpu_hw.c >> @@ -10,8 +10,10 @@ >> >> #include "hantro.h" >> #include "hantro_jpeg.h" >> +#include "hantro_g1_regs.h" >> #include "hantro_h1_regs.h" >> >> +#define RK3066_ACLK_MAX_FREQ (300 * 1000 * 1000) >> #define RK3288_ACLK_MAX_FREQ (400 * 1000 * 1000) >> >> /* >> @@ -62,6 +64,52 @@ static const struct hantro_fmt rk3288_vpu_postproc_fmts[] = { >> }, >> }; >> >> +static const struct hantro_fmt rk3066_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 = 1920, >> + .step_width = MB_DIM, >> + .min_height = 48, >> + .max_height = 1088, >> + .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 = 1920, >> + .step_width = MB_DIM, >> + .min_height = 48, >> + .max_height = 1088, >> + .step_height = MB_DIM, >> + }, >> + }, >> +}; >> + >> static const struct hantro_fmt rk3288_vpu_dec_fmts[] = { >> { >> .fourcc = V4L2_PIX_FMT_NV12, >> @@ -126,6 +174,14 @@ static irqreturn_t rk3288_vepu_irq(int irq, void *dev_id) >> return IRQ_HANDLED; >> } >> >> +static int rk3066_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); >> + clk_set_rate(vpu->clocks[0].clk, RK3066_ACLK_MAX_FREQ); >> + return 0; >> +} >> + >> static int rk3288_vpu_hw_init(struct hantro_dev *vpu) >> { >> /* Bump ACLK to max. possible freq. to improve performance. */ >> @@ -133,6 +189,14 @@ static int rk3288_vpu_hw_init(struct hantro_dev *vpu) >> return 0; >> } >> >> +static void rk3066_vpu_dec_reset(struct hantro_ctx *ctx) >> +{ >> + struct hantro_dev *vpu = ctx->dev; >> + >> + vdpu_write(vpu, G1_REG_INTERRUPT_DEC_IRQ_DIS, G1_REG_INTERRUPT); >> + vdpu_write(vpu, G1_REG_CONFIG_DEC_CLK_GATE_E, G1_REG_CONFIG); >> +} >> + >> static void rk3288_vpu_enc_reset(struct hantro_ctx *ctx) >> { >> struct hantro_dev *vpu = ctx->dev; >> @@ -145,6 +209,33 @@ static void rk3288_vpu_enc_reset(struct hantro_ctx *ctx) >> /* >> * Supported codec ops. >> */ >> +static const struct hantro_codec_ops rk3066_vpu_codec_ops[] = { >> + [HANTRO_MODE_JPEG_ENC] = { >> + .run = hantro_h1_jpeg_enc_run, >> + .reset = rk3288_vpu_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 = rk3066_vpu_dec_reset, >> + .init = hantro_h264_dec_init, >> + .exit = hantro_h264_dec_exit, >> + }, >> + [HANTRO_MODE_MPEG2_DEC] = { >> + .run = hantro_g1_mpeg2_dec_run, >> + .reset = rk3066_vpu_dec_reset, >> + .init = hantro_mpeg2_dec_init, >> + .exit = hantro_mpeg2_dec_exit, >> + }, >> + [HANTRO_MODE_VP8_DEC] = { >> + .run = hantro_g1_vp8_dec_run, >> + .reset = rk3066_vpu_dec_reset, >> + .init = hantro_vp8_dec_init, >> + .exit = hantro_vp8_dec_exit, >> + }, >> +}; >> >> static const struct hantro_codec_ops rk3288_vpu_codec_ops[] = { >> [HANTRO_MODE_JPEG_ENC] = { >> @@ -183,10 +274,35 @@ static const struct hantro_irq rk3288_irqs[] = { >> { "vdpu", hantro_g1_irq }, >> }; >> >> +static const char * const rk3066_clk_names[] = { >> + "aclk_vdpu", "hclk_vdpu", >> + "aclk_vepu", "hclk_vepu" >> +}; >> + >> static const char * const rk3288_clk_names[] = { >> "aclk", "hclk" >> }; >> >> +const struct hantro_variant rk3066_vpu_variant = { >> + .enc_offset = 0x0, >> + .enc_fmts = rk3288_vpu_enc_fmts, >> + .num_enc_fmts = ARRAY_SIZE(rk3288_vpu_enc_fmts), >> + .dec_offset = 0x400, > Having decoder and encoder supported by a single devicetree > node was done for RK3288 to cope with some bug in the hardware > that was effectively linking the decoder and the encoder. > > AFAIK, Rockchip has fixed this, so unless there's a strong > need, I prefer we keep them separated, with one DT node > for the g1 decoder and one for the h1 encoder. > > Thanks! > Ezequiel > I just checked it: despite it looks like we could use the decoder and encoder separately (separate clocks for decoder / encoder) the VPU block won't work (SoC crashes), if not all 4 clocks are enabled for neither decoding nor encoding. I'd prefer the other way also, but it seems not possible. Alex.
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
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
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
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
On Thu, 2021-05-27 at 01:22 +0200, Alex Bee wrote: > Hi Ezequiel, > > thanks for your feedback. > > Am 26.05.21 um 12:32 schrieb Ezequiel Garcia: > > Hi Alex, > > > > Thanks for the patch. > > > > On Tue, 2021-05-25 at 17:22 +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 > > > - 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. > > > > > > 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 | 116 +++++++++++++++++++ > > > 3 files changed, 118 insertions(+) > > > > > > diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c > > > index 2f6b01c7a6a0..38ea7b24036e 100644 > > > --- a/drivers/staging/media/hantro/hantro_drv.c > > > +++ b/drivers/staging/media/hantro/hantro_drv.c > > > @@ -489,6 +489,7 @@ static const struct of_device_id of_hantro_match[] = { > > > { .compatible = "rockchip,rk3399-vpu", .data = &rk3399_vpu_variant, }, > > > { .compatible = "rockchip,rk3328-vpu", .data = &rk3328_vpu_variant, }, > > > { .compatible = "rockchip,rk3288-vpu", .data = &rk3288_vpu_variant, }, > > > + { .compatible = "rockchip,rk3066-vpu", .data = &rk3066_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 3d8b53567f16..de2bc367a15a 100644 > > > --- a/drivers/staging/media/hantro/hantro_hw.h > > > +++ b/drivers/staging/media/hantro/hantro_hw.h > > > @@ -163,6 +163,7 @@ enum hantro_enc_fmt { > > > 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 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 fefd45269e52..29805c4bd92f 100644 > > > --- a/drivers/staging/media/hantro/rk3288_vpu_hw.c > > > +++ b/drivers/staging/media/hantro/rk3288_vpu_hw.c > > > @@ -10,8 +10,10 @@ > > > > > > #include "hantro.h" > > > #include "hantro_jpeg.h" > > > +#include "hantro_g1_regs.h" > > > #include "hantro_h1_regs.h" > > > > > > +#define RK3066_ACLK_MAX_FREQ (300 * 1000 * 1000) > > > #define RK3288_ACLK_MAX_FREQ (400 * 1000 * 1000) > > > > > > /* > > > @@ -62,6 +64,52 @@ static const struct hantro_fmt rk3288_vpu_postproc_fmts[] = { > > > }, > > > }; > > > > > > +static const struct hantro_fmt rk3066_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 = 1920, > > > + .step_width = MB_DIM, > > > + .min_height = 48, > > > + .max_height = 1088, > > > + .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 = 1920, > > > + .step_width = MB_DIM, > > > + .min_height = 48, > > > + .max_height = 1088, > > > + .step_height = MB_DIM, > > > + }, > > > + }, > > > +}; > > > + > > > static const struct hantro_fmt rk3288_vpu_dec_fmts[] = { > > > { > > > .fourcc = V4L2_PIX_FMT_NV12, > > > @@ -126,6 +174,14 @@ static irqreturn_t rk3288_vepu_irq(int irq, void *dev_id) > > > return IRQ_HANDLED; > > > } > > > > > > +static int rk3066_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); > > > + clk_set_rate(vpu->clocks[0].clk, RK3066_ACLK_MAX_FREQ); > > > + return 0; > > > +} > > > + > > > static int rk3288_vpu_hw_init(struct hantro_dev *vpu) > > > { > > > /* Bump ACLK to max. possible freq. to improve performance. */ > > > @@ -133,6 +189,14 @@ static int rk3288_vpu_hw_init(struct hantro_dev *vpu) > > > return 0; > > > } > > > > > > +static void rk3066_vpu_dec_reset(struct hantro_ctx *ctx) > > > +{ > > > + struct hantro_dev *vpu = ctx->dev; > > > + > > > + vdpu_write(vpu, G1_REG_INTERRUPT_DEC_IRQ_DIS, G1_REG_INTERRUPT); > > > + vdpu_write(vpu, G1_REG_CONFIG_DEC_CLK_GATE_E, G1_REG_CONFIG); > > > +} > > > + > > > static void rk3288_vpu_enc_reset(struct hantro_ctx *ctx) > > > { > > > struct hantro_dev *vpu = ctx->dev; > > > @@ -145,6 +209,33 @@ static void rk3288_vpu_enc_reset(struct hantro_ctx *ctx) > > > /* > > > * Supported codec ops. > > > */ > > > +static const struct hantro_codec_ops rk3066_vpu_codec_ops[] = { > > > + [HANTRO_MODE_JPEG_ENC] = { > > > + .run = hantro_h1_jpeg_enc_run, > > > + .reset = rk3288_vpu_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 = rk3066_vpu_dec_reset, > > > + .init = hantro_h264_dec_init, > > > + .exit = hantro_h264_dec_exit, > > > + }, > > > + [HANTRO_MODE_MPEG2_DEC] = { > > > + .run = hantro_g1_mpeg2_dec_run, > > > + .reset = rk3066_vpu_dec_reset, > > > + .init = hantro_mpeg2_dec_init, > > > + .exit = hantro_mpeg2_dec_exit, > > > + }, > > > + [HANTRO_MODE_VP8_DEC] = { > > > + .run = hantro_g1_vp8_dec_run, > > > + .reset = rk3066_vpu_dec_reset, > > > + .init = hantro_vp8_dec_init, > > > + .exit = hantro_vp8_dec_exit, > > > + }, > > > +}; > > > > > > static const struct hantro_codec_ops rk3288_vpu_codec_ops[] = { > > > [HANTRO_MODE_JPEG_ENC] = { > > > @@ -183,10 +274,35 @@ static const struct hantro_irq rk3288_irqs[] = { > > > { "vdpu", hantro_g1_irq }, > > > }; > > > > > > +static const char * const rk3066_clk_names[] = { > > > + "aclk_vdpu", "hclk_vdpu", > > > + "aclk_vepu", "hclk_vepu" > > > +}; > > > + > > > static const char * const rk3288_clk_names[] = { > > > "aclk", "hclk" > > > }; > > > > > > +const struct hantro_variant rk3066_vpu_variant = { > > > + .enc_offset = 0x0, > > > + .enc_fmts = rk3288_vpu_enc_fmts, > > > + .num_enc_fmts = ARRAY_SIZE(rk3288_vpu_enc_fmts), > > > + .dec_offset = 0x400, > > Having decoder and encoder supported by a single devicetree > > node was done for RK3288 to cope with some bug in the hardware > > that was effectively linking the decoder and the encoder. > > > > AFAIK, Rockchip has fixed this, so unless there's a strong > > need, I prefer we keep them separated, with one DT node > > for the g1 decoder and one for the h1 encoder. > > > > Thanks! > > Ezequiel > > > I just checked it: despite it looks like we could use the decoder and > encoder separately > > (separate clocks for decoder / encoder) the VPU block won't work (SoC > crashes), > > if not all 4 clocks are enabled for neither decoding nor encoding. > > I'd prefer the other way also, but it seems not possible. > That's really useful, so it seems it's safer to represent this in a single node, given there's at least some degree of sharing going on, which the VPU Combo naming suggests anyways :) Could you add a comment in the code about this clock requirement? Also, does it make sense to merge rk3288_vpu_hw.c and rk3399_vpu_hw.c as just rockchip_vpu_hw.c? Thanks! Ezequiel
Hi Ezequiel, Am 27.05.21 um 03:27 schrieb Ezequiel Garcia: > 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. I've always looked at the "Address Mapping" section in the TRMs when I checked the register offsets. I can't find a seperation the vpu block there (for any SoC). I've found it more unfortunate, that they started with register offset 0x0 for vdpu and vepu, since none of the SoCs (so far) can use the blocks separately. > (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? Ups, yes - I missed to submit this patch with v1 - I added it in its original version to v2 (so we know, what we are talking about)- If you think it should be changed, please reply to v2. > Thanks a lot! > Ezequiel > Thanks, Alex