| Message ID | 20210624182612.177969-10-ezequiel@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hantro: Enable H.264 VDPU2 (Odroid Advance Go) | expand |
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),
(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
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 >
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.
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 --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),
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(-)