diff mbox series

[09/12] media: hantro: Enable H.264 on Rockchip VDPU2

Message ID 20210624182612.177969-10-ezequiel@collabora.com
State Superseded
Headers show
Series [01/12] drm/panel: kd35t133: Add panel orientation support | expand

Commit Message

Ezequiel Garcia June 24, 2021, 6:26 p.m. UTC
Given H.264 support for VDPU2 was just added, let's enable it.
For now, this is only enabled on platform that don't have
an RKVDEC core, such as RK3328.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 .../staging/media/hantro/rockchip_vpu_hw.c    | 26 ++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

Comments

Alex Bee June 24, 2021, 11:13 p.m. UTC | #1
Hi Ezequiel,

Am 24.06.21 um 20:26 schrieb Ezequiel Garcia:
> Given H.264 support for VDPU2 was just added, let's enable it.
> For now, this is only enabled on platform that don't have
> an RKVDEC core, such as RK3328.

Is there any reason, you do not want to enabe H.264 on RK3399? I know 
H.264 can be done by by rkvdec already, but from what I understand that 
shouldn't be an issue: The first decoder found that meets the 
requirements will be taken.

RK3328 has a variant (mpp calls it vdpu341) of rkvdec also which also 
supports H.264 (and HEVC/VP9). rkvdec driver needs a (much simpler) 
variant implementation in order to support it there also, since its has 
some additional registers.

Thanks,

Alex

>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>   .../staging/media/hantro/rockchip_vpu_hw.c    | 26 ++++++++++++++++++-
>   1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/hantro/rockchip_vpu_hw.c b/drivers/staging/media/hantro/rockchip_vpu_hw.c
> index 3ccc16413f42..e4e3b5e7689b 100644
> --- a/drivers/staging/media/hantro/rockchip_vpu_hw.c
> +++ b/drivers/staging/media/hantro/rockchip_vpu_hw.c
> @@ -162,6 +162,19 @@ static const struct hantro_fmt rk3399_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,
> @@ -388,6 +401,12 @@ static const struct hantro_codec_ops rk3399_vpu_codec_ops[] = {
>   		.init = hantro_jpeg_enc_init,
>   		.exit = hantro_jpeg_enc_exit,
>   	},
> +	[HANTRO_MODE_H264_DEC] = {
> +		.run = rockchip_vpu2_h264_dec_run,
> +		.reset = rockchip_vpu2_dec_reset,
> +		.init = hantro_h264_dec_init,
> +		.exit = hantro_h264_dec_exit,
> +	},
>   	[HANTRO_MODE_MPEG2_DEC] = {
>   		.run = rockchip_vpu2_mpeg2_dec_run,
>   		.reset = rockchip_vpu2_dec_reset,
> @@ -433,6 +452,8 @@ static const char * const rockchip_vpu_clk_names[] = {
>   	"aclk", "hclk"
>   };
>   
> +/* VDPU1/VEPU1 */
> +
>   const struct hantro_variant rk3036_vpu_variant = {
>   	.dec_offset = 0x400,
>   	.dec_fmts = rk3066_vpu_dec_fmts,
> @@ -495,11 +516,14 @@ const struct hantro_variant rk3288_vpu_variant = {
>   	.num_clocks = ARRAY_SIZE(rockchip_vpu_clk_names)
>   };
>   
> +/* VDPU2/VEPU2 */
> +
>   const struct hantro_variant rk3328_vpu_variant = {
>   	.dec_offset = 0x400,
>   	.dec_fmts = rk3399_vpu_dec_fmts,
>   	.num_dec_fmts = ARRAY_SIZE(rk3399_vpu_dec_fmts),
> -	.codec = HANTRO_MPEG2_DECODER | HANTRO_VP8_DECODER,
> +	.codec = HANTRO_MPEG2_DECODER | HANTRO_VP8_DECODER |
> +		 HANTRO_H264_DECODER,
>   	.codec_ops = rk3399_vpu_codec_ops,
>   	.irqs = rockchip_vdpu2_irqs,
>   	.num_irqs = ARRAY_SIZE(rockchip_vdpu2_irqs),
Ezequiel Garcia June 26, 2021, 12:46 a.m. UTC | #2
(Adding Nicolas)

Hi Alex,

On Fri, 2021-06-25 at 01:13 +0200, Alex Bee wrote:
> Hi Ezequiel,
> 
> Am 24.06.21 um 20:26 schrieb Ezequiel Garcia:
> > Given H.264 support for VDPU2 was just added, let's enable it.
> > For now, this is only enabled on platform that don't have
> > an RKVDEC core, such as RK3328.
> 
> Is there any reason, you do not want to enabe H.264 on RK3399? I know 
> H.264 can be done by by rkvdec already, but from what I understand that 
> shouldn't be an issue: The first decoder found that meets the 
> requirements will be taken.
> 

Thanks a lot the review.

I really doubt userspace stacks are readily supporting that strategy.

The first decoder device supporting the codec format will be selected,
I doubt features such as profile and levels are checked to decide
which decoder to use.

I'd rather play safe on the kernel side and avoid offering
two competing devices for the same codec.

Kindly,
Ezequiel
Alex Bee June 26, 2021, 8:33 a.m. UTC | #3
Hi Ezequiel,

Am 26.06.21 um 02:46 schrieb Ezequiel Garcia:
> (Adding Nicolas)

>

> Hi Alex,

>

> On Fri, 2021-06-25 at 01:13 +0200, Alex Bee wrote:

>> Hi Ezequiel,

>>

>> Am 24.06.21 um 20:26 schrieb Ezequiel Garcia:

>>> Given H.264 support for VDPU2 was just added, let's enable it.

>>> For now, this is only enabled on platform that don't have

>>> an RKVDEC core, such as RK3328.

>> Is there any reason, you do not want to enabe H.264 on RK3399? I know

>> H.264 can be done by by rkvdec already, but from what I understand that

>> shouldn't be an issue: The first decoder found that meets the

>> requirements will be taken.

>>

> Thanks a lot the review.

>

> I really doubt userspace stacks are readily supporting that strategy.

>

> The first decoder device supporting the codec format will be selected,

> I doubt features such as profile and levels are checked to decide

> which decoder to use.

>

> I'd rather play safe on the kernel side and avoid offering

> two competing devices for the same codec.


I wasn't aware of that. Current ffmpeg v4l2_request implementation seems 
to not do VIDIOC_ENUM_FRAMESIZES - so we might end up being able to 
decode up to 1920x1088 only if hantro decoder is picked/checked first.

Thanks for pointing that out.

Alex

> Kindly,

> Ezequiel

>
Ezequiel Garcia June 29, 2021, 12:28 p.m. UTC | #4
Hi Alex,

On Sat, 2021-06-26 at 10:33 +0200, Alex Bee wrote:
> Hi Ezequiel,

> 

> Am 26.06.21 um 02:46 schrieb Ezequiel Garcia:

> > (Adding Nicolas)

> > 

> > Hi Alex,

> > 

> > On Fri, 2021-06-25 at 01:13 +0200, Alex Bee wrote:

> > > Hi Ezequiel,

> > > 

> > > Am 24.06.21 um 20:26 schrieb Ezequiel Garcia:

> > > > Given H.264 support for VDPU2 was just added, let's enable it.

> > > > For now, this is only enabled on platform that don't have

> > > > an RKVDEC core, such as RK3328.

> > > Is there any reason, you do not want to enabe H.264 on RK3399? I know

> > > H.264 can be done by by rkvdec already, but from what I understand that

> > > shouldn't be an issue: The first decoder found that meets the

> > > requirements will be taken.

> > > 

> > Thanks a lot the review.

> > 

> > I really doubt userspace stacks are readily supporting that strategy.

> > 

> > The first decoder device supporting the codec format will be selected,

> > I doubt features such as profile and levels are checked to decide

> > which decoder to use.

> > 

> > I'd rather play safe on the kernel side and avoid offering

> > two competing devices for the same codec.

> 

> I wasn't aware of that. Current ffmpeg v4l2_request implementation seems 

> to not do VIDIOC_ENUM_FRAMESIZES - so we might end up being able to 

> decode up to 1920x1088 only if hantro decoder is picked/checked first.

> 


Speaking of ffmpeg, now that MPEG-2, VP8 and H.264 control interfaces
are stable, I think one of the next priorities would be to push Jonas'
ffmpeg patches.

It would be really cool if someone could take the lead on that front,
as it would reduce kodi's out of tree stack, enable mpv, and so on.
-- 
Kindly,
Ezequiel
Alex Bee June 30, 2021, 11:36 a.m. UTC | #5
Hi Ezequiel,

Am 29.06.21 um 14:28 schrieb Ezequiel Garcia:
> Hi Alex,

>

> On Sat, 2021-06-26 at 10:33 +0200, Alex Bee wrote:

>> Hi Ezequiel,

>>

>> Am 26.06.21 um 02:46 schrieb Ezequiel Garcia:

>>> (Adding Nicolas)

>>>

>>> Hi Alex,

>>>

>>> On Fri, 2021-06-25 at 01:13 +0200, Alex Bee wrote:

>>>> Hi Ezequiel,

>>>>

>>>> Am 24.06.21 um 20:26 schrieb Ezequiel Garcia:

>>>>> Given H.264 support for VDPU2 was just added, let's enable it.

>>>>> For now, this is only enabled on platform that don't have

>>>>> an RKVDEC core, such as RK3328.

>>>> Is there any reason, you do not want to enabe H.264 on RK3399? I know

>>>> H.264 can be done by by rkvdec already, but from what I understand that

>>>> shouldn't be an issue: The first decoder found that meets the

>>>> requirements will be taken.

>>>>

>>> Thanks a lot the review.

>>>

>>> I really doubt userspace stacks are readily supporting that strategy.

>>>

>>> The first decoder device supporting the codec format will be selected,

>>> I doubt features such as profile and levels are checked to decide

>>> which decoder to use.

>>>

>>> I'd rather play safe on the kernel side and avoid offering

>>> two competing devices for the same codec.

>> I wasn't aware of that. Current ffmpeg v4l2_request implementation seems

>> to not do VIDIOC_ENUM_FRAMESIZES - so we might end up being able to

>> decode up to 1920x1088 only if hantro decoder is picked/checked first.

>>

> Speaking of ffmpeg, now that MPEG-2, VP8 and H.264 control interfaces

> are stable, I think one of the next priorities would be to push Jonas'

> ffmpeg patches.

>

> It would be really cool if someone could take the lead on that front,

> as it would reduce kodi's out of tree stack, enable mpv, and so on.


That's absolutely true.

Note that Jonas himself started upstreaming those patches right after 
H264 uapi got stable [1].

Unfortunately I'm the absolut wrong person for doing/continuing this, 
since the very first time I ever looked at the implementation details 
was just for the response I wrote here. So I asked Jernej whos know all 
the details and contributed to those patches as well - he told me he'll 
continue whenever he finds time next.

Best,

Alex

[1] https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=2898
diff mbox series

Patch

diff --git a/drivers/staging/media/hantro/rockchip_vpu_hw.c b/drivers/staging/media/hantro/rockchip_vpu_hw.c
index 3ccc16413f42..e4e3b5e7689b 100644
--- a/drivers/staging/media/hantro/rockchip_vpu_hw.c
+++ b/drivers/staging/media/hantro/rockchip_vpu_hw.c
@@ -162,6 +162,19 @@  static const struct hantro_fmt rk3399_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,
@@ -388,6 +401,12 @@  static const struct hantro_codec_ops rk3399_vpu_codec_ops[] = {
 		.init = hantro_jpeg_enc_init,
 		.exit = hantro_jpeg_enc_exit,
 	},
+	[HANTRO_MODE_H264_DEC] = {
+		.run = rockchip_vpu2_h264_dec_run,
+		.reset = rockchip_vpu2_dec_reset,
+		.init = hantro_h264_dec_init,
+		.exit = hantro_h264_dec_exit,
+	},
 	[HANTRO_MODE_MPEG2_DEC] = {
 		.run = rockchip_vpu2_mpeg2_dec_run,
 		.reset = rockchip_vpu2_dec_reset,
@@ -433,6 +452,8 @@  static const char * const rockchip_vpu_clk_names[] = {
 	"aclk", "hclk"
 };
 
+/* VDPU1/VEPU1 */
+
 const struct hantro_variant rk3036_vpu_variant = {
 	.dec_offset = 0x400,
 	.dec_fmts = rk3066_vpu_dec_fmts,
@@ -495,11 +516,14 @@  const struct hantro_variant rk3288_vpu_variant = {
 	.num_clocks = ARRAY_SIZE(rockchip_vpu_clk_names)
 };
 
+/* VDPU2/VEPU2 */
+
 const struct hantro_variant rk3328_vpu_variant = {
 	.dec_offset = 0x400,
 	.dec_fmts = rk3399_vpu_dec_fmts,
 	.num_dec_fmts = ARRAY_SIZE(rk3399_vpu_dec_fmts),
-	.codec = HANTRO_MPEG2_DECODER | HANTRO_VP8_DECODER,
+	.codec = HANTRO_MPEG2_DECODER | HANTRO_VP8_DECODER |
+		 HANTRO_H264_DECODER,
 	.codec_ops = rk3399_vpu_codec_ops,
 	.irqs = rockchip_vdpu2_irqs,
 	.num_irqs = ARRAY_SIZE(rockchip_vdpu2_irqs),