| Message ID | 20191026074959.1073512-2-jernej.skrabec@siol.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: cedrus: Add support for 4k videos | expand |
Hi Jernej, On Sat 26 Oct 19, 09:49, Jernej Skrabec wrote: > Mode register also holds information if video width is bigger than 2048 > and if it is equal to 4096. > > Rework cedrus_engine_enable() to properly signal this properties. Thanks for the patch, looks good to me! Acked-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> One minor thing: maybe we should have a way to set the maximum dimensions depending on the generation of the engine in use and the actual maximum supported by the hardware. Maybe either as dedicated new fields in struct cedrus_variant or as capability flags. Anyway that can be done later since we were already hardcoding this. Cheers, Paul > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > --- > drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 2 +- > drivers/staging/media/sunxi/cedrus/cedrus_h265.c | 2 +- > drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 9 +++++++-- > drivers/staging/media/sunxi/cedrus/cedrus_hw.h | 2 +- > drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c | 2 +- > drivers/staging/media/sunxi/cedrus/cedrus_regs.h | 2 ++ > 6 files changed, 13 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > index 7487f6ab7576..d2c854ecdf15 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > @@ -485,7 +485,7 @@ static void cedrus_h264_setup(struct cedrus_ctx *ctx, > { > struct cedrus_dev *dev = ctx->dev; > > - cedrus_engine_enable(dev, CEDRUS_CODEC_H264); > + cedrus_engine_enable(ctx, CEDRUS_CODEC_H264); > > cedrus_write(dev, VE_H264_SDROT_CTRL, 0); > cedrus_write(dev, VE_H264_EXTRA_BUFFER1, > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c > index 9bc921866f70..6945dc74e1d7 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c > @@ -276,7 +276,7 @@ static void cedrus_h265_setup(struct cedrus_ctx *ctx, > } > > /* Activate H265 engine. */ > - cedrus_engine_enable(dev, CEDRUS_CODEC_H265); > + cedrus_engine_enable(ctx, CEDRUS_CODEC_H265); > > /* Source offset and length in bits. */ > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > index 570a9165dd5d..3acfa21bc124 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > @@ -30,7 +30,7 @@ > #include "cedrus_hw.h" > #include "cedrus_regs.h" > > -int cedrus_engine_enable(struct cedrus_dev *dev, enum cedrus_codec codec) > +int cedrus_engine_enable(struct cedrus_ctx *ctx, enum cedrus_codec codec) > { > u32 reg = 0; > > @@ -58,7 +58,12 @@ int cedrus_engine_enable(struct cedrus_dev *dev, enum cedrus_codec codec) > return -EINVAL; > } > > - cedrus_write(dev, VE_MODE, reg); > + if (ctx->src_fmt.width == 4096) > + reg |= VE_MODE_PIC_WIDTH_IS_4096; > + if (ctx->src_fmt.width > 2048) > + reg |= VE_MODE_PIC_WIDTH_MORE_2048; > + > + cedrus_write(ctx->dev, VE_MODE, reg); > > return 0; > } > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h > index 27d0882397aa..604ff932fbf5 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h > @@ -16,7 +16,7 @@ > #ifndef _CEDRUS_HW_H_ > #define _CEDRUS_HW_H_ > > -int cedrus_engine_enable(struct cedrus_dev *dev, enum cedrus_codec codec); > +int cedrus_engine_enable(struct cedrus_ctx *ctx, enum cedrus_codec codec); > void cedrus_engine_disable(struct cedrus_dev *dev); > > void cedrus_dst_format_set(struct cedrus_dev *dev, > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c > index 13c34927bad5..8bcd6b8f9e2d 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c > @@ -96,7 +96,7 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run) > quantization = run->mpeg2.quantization; > > /* Activate MPEG engine. */ > - cedrus_engine_enable(dev, CEDRUS_CODEC_MPEG2); > + cedrus_engine_enable(ctx, CEDRUS_CODEC_MPEG2); > > /* Set intra quantization matrix. */ > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h > index 4275a307d282..ace3d49fcd82 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h > @@ -35,6 +35,8 @@ > > #define VE_MODE 0x00 > > +#define VE_MODE_PIC_WIDTH_IS_4096 BIT(22) > +#define VE_MODE_PIC_WIDTH_MORE_2048 BIT(21) > #define VE_MODE_REC_WR_MODE_2MB (0x01 << 20) > #define VE_MODE_REC_WR_MODE_1MB (0x00 << 20) > #define VE_MODE_DDR_MODE_BW_128 (0x03 << 16) > -- > 2.23.0 >
Dne ponedeljek, 04. november 2019 ob 11:02:28 CET je Paul Kocialkowski napisal(a): > Hi Jernej, > > On Sat 26 Oct 19, 09:49, Jernej Skrabec wrote: > > Mode register also holds information if video width is bigger than 2048 > > and if it is equal to 4096. > > > > Rework cedrus_engine_enable() to properly signal this properties. > > Thanks for the patch, looks good to me! > > Acked-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > > One minor thing: maybe we should have a way to set the maximum dimensions > depending on the generation of the engine in use and the actual maximum > supported by the hardware. > > Maybe either as dedicated new fields in struct cedrus_variant or as > capability flags. I was thinking about first solution, but after going trough manuals, it was unclear what are real limitations. For example, H3 manual states that it is capable of decoding H264 1080p@60Hz. However, I know for a fact that it is also capable of decoding 4k videos, but probably not at 60 Hz. I don't own anything older that A83T, so I don't know what are capabilities of those SoCs. Anyway, being slow is still ok for some tasks, like transcoding, so we can't limit decoding to 1080p just because it's slow. It is probably still faster than doing it in SW. Not to mention that it's still ok for some videos, a lot of them uses 24 fps. Best regards, Jernej > > Anyway that can be done later since we were already hardcoding this. > > Cheers, > > Paul > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > --- > > > > drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 2 +- > > drivers/staging/media/sunxi/cedrus/cedrus_h265.c | 2 +- > > drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 9 +++++++-- > > drivers/staging/media/sunxi/cedrus/cedrus_hw.h | 2 +- > > drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c | 2 +- > > drivers/staging/media/sunxi/cedrus/cedrus_regs.h | 2 ++ > > 6 files changed, 13 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c index > > 7487f6ab7576..d2c854ecdf15 100644 > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > @@ -485,7 +485,7 @@ static void cedrus_h264_setup(struct cedrus_ctx *ctx, > > > > { > > > > struct cedrus_dev *dev = ctx->dev; > > > > - cedrus_engine_enable(dev, CEDRUS_CODEC_H264); > > + cedrus_engine_enable(ctx, CEDRUS_CODEC_H264); > > > > cedrus_write(dev, VE_H264_SDROT_CTRL, 0); > > cedrus_write(dev, VE_H264_EXTRA_BUFFER1, > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c > > b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c index > > 9bc921866f70..6945dc74e1d7 100644 > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c > > @@ -276,7 +276,7 @@ static void cedrus_h265_setup(struct cedrus_ctx *ctx, > > > > } > > > > /* Activate H265 engine. */ > > > > - cedrus_engine_enable(dev, CEDRUS_CODEC_H265); > > + cedrus_engine_enable(ctx, CEDRUS_CODEC_H265); > > > > /* Source offset and length in bits. */ > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > > b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c index > > 570a9165dd5d..3acfa21bc124 100644 > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > > @@ -30,7 +30,7 @@ > > > > #include "cedrus_hw.h" > > #include "cedrus_regs.h" > > > > -int cedrus_engine_enable(struct cedrus_dev *dev, enum cedrus_codec codec) > > +int cedrus_engine_enable(struct cedrus_ctx *ctx, enum cedrus_codec codec) > > > > { > > > > u32 reg = 0; > > > > @@ -58,7 +58,12 @@ int cedrus_engine_enable(struct cedrus_dev *dev, enum > > cedrus_codec codec)> > > return -EINVAL; > > > > } > > > > - cedrus_write(dev, VE_MODE, reg); > > + if (ctx->src_fmt.width == 4096) > > + reg |= VE_MODE_PIC_WIDTH_IS_4096; > > + if (ctx->src_fmt.width > 2048) > > + reg |= VE_MODE_PIC_WIDTH_MORE_2048; > > + > > + cedrus_write(ctx->dev, VE_MODE, reg); > > > > return 0; > > > > } > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h > > b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h index > > 27d0882397aa..604ff932fbf5 100644 > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h > > @@ -16,7 +16,7 @@ > > > > #ifndef _CEDRUS_HW_H_ > > #define _CEDRUS_HW_H_ > > > > -int cedrus_engine_enable(struct cedrus_dev *dev, enum cedrus_codec > > codec); > > +int cedrus_engine_enable(struct cedrus_ctx *ctx, enum cedrus_codec > > codec); > > > > void cedrus_engine_disable(struct cedrus_dev *dev); > > > > void cedrus_dst_format_set(struct cedrus_dev *dev, > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c > > b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c index > > 13c34927bad5..8bcd6b8f9e2d 100644 > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c > > @@ -96,7 +96,7 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, > > struct cedrus_run *run)> > > quantization = run->mpeg2.quantization; > > > > /* Activate MPEG engine. */ > > > > - cedrus_engine_enable(dev, CEDRUS_CODEC_MPEG2); > > + cedrus_engine_enable(ctx, CEDRUS_CODEC_MPEG2); > > > > /* Set intra quantization matrix. */ > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h > > b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h index > > 4275a307d282..ace3d49fcd82 100644 > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h > > @@ -35,6 +35,8 @@ > > > > #define VE_MODE 0x00 > > > > +#define VE_MODE_PIC_WIDTH_IS_4096 BIT(22) > > +#define VE_MODE_PIC_WIDTH_MORE_2048 BIT(21) > > > > #define VE_MODE_REC_WR_MODE_2MB (0x01 << 20) > > #define VE_MODE_REC_WR_MODE_1MB (0x00 << 20) > > #define VE_MODE_DDR_MODE_BW_128 (0x03 << 16)
Hi, On Mon 04 Nov 19, 17:33, Jernej Škrabec wrote: > Dne ponedeljek, 04. november 2019 ob 11:02:28 CET je Paul Kocialkowski > napisal(a): > > Hi Jernej, > > > > On Sat 26 Oct 19, 09:49, Jernej Skrabec wrote: > > > Mode register also holds information if video width is bigger than 2048 > > > and if it is equal to 4096. > > > > > > Rework cedrus_engine_enable() to properly signal this properties. > > > > Thanks for the patch, looks good to me! > > > > Acked-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > > > > One minor thing: maybe we should have a way to set the maximum dimensions > > depending on the generation of the engine in use and the actual maximum > > supported by the hardware. > > > > Maybe either as dedicated new fields in struct cedrus_variant or as > > capability flags. > > I was thinking about first solution, but after going trough manuals, it was > unclear what are real limitations. For example, H3 manual states that it is > capable of decoding H264 1080p@60Hz. However, I know for a fact that it is > also capable of decoding 4k videos, but probably not at 60 Hz. I don't own > anything older that A83T, so I don't know what are capabilities of those SoCs. So I guess in this case we should try and see. I could try to look into it at some point in the future too if you're not particulary interested. > Anyway, being slow is still ok for some tasks, like transcoding, so we can't > limit decoding to 1080p just because it's slow. It is probably still faster > than doing it in SW. Not to mention that it's still ok for some videos, a lot > of them uses 24 fps. I agree, it's best to expose the maximum supported resolution by the hardware, even if it means running at a lower fps. Do you know if we have a way to report some estimation of the maximum supported fps to userspace? It would be useful to let userspace decide whether it's a better fit than software decoding. Cheers, Paul > Best regards, > Jernej > > > > > Anyway that can be done later since we were already hardcoding this. > > > > Cheers, > > > > Paul > > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > > --- > > > > > > drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 2 +- > > > drivers/staging/media/sunxi/cedrus/cedrus_h265.c | 2 +- > > > drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 9 +++++++-- > > > drivers/staging/media/sunxi/cedrus/cedrus_hw.h | 2 +- > > > drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c | 2 +- > > > drivers/staging/media/sunxi/cedrus/cedrus_regs.h | 2 ++ > > > 6 files changed, 13 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c index > > > 7487f6ab7576..d2c854ecdf15 100644 > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > @@ -485,7 +485,7 @@ static void cedrus_h264_setup(struct cedrus_ctx *ctx, > > > > > > { > > > > > > struct cedrus_dev *dev = ctx->dev; > > > > > > - cedrus_engine_enable(dev, CEDRUS_CODEC_H264); > > > + cedrus_engine_enable(ctx, CEDRUS_CODEC_H264); > > > > > > cedrus_write(dev, VE_H264_SDROT_CTRL, 0); > > > cedrus_write(dev, VE_H264_EXTRA_BUFFER1, > > > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c > > > b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c index > > > 9bc921866f70..6945dc74e1d7 100644 > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c > > > @@ -276,7 +276,7 @@ static void cedrus_h265_setup(struct cedrus_ctx *ctx, > > > > > > } > > > > > > /* Activate H265 engine. */ > > > > > > - cedrus_engine_enable(dev, CEDRUS_CODEC_H265); > > > + cedrus_engine_enable(ctx, CEDRUS_CODEC_H265); > > > > > > /* Source offset and length in bits. */ > > > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > > > b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c index > > > 570a9165dd5d..3acfa21bc124 100644 > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > > > @@ -30,7 +30,7 @@ > > > > > > #include "cedrus_hw.h" > > > #include "cedrus_regs.h" > > > > > > -int cedrus_engine_enable(struct cedrus_dev *dev, enum cedrus_codec codec) > > > +int cedrus_engine_enable(struct cedrus_ctx *ctx, enum cedrus_codec codec) > > > > > > { > > > > > > u32 reg = 0; > > > > > > @@ -58,7 +58,12 @@ int cedrus_engine_enable(struct cedrus_dev *dev, enum > > > cedrus_codec codec)> > > > return -EINVAL; > > > > > > } > > > > > > - cedrus_write(dev, VE_MODE, reg); > > > + if (ctx->src_fmt.width == 4096) > > > + reg |= VE_MODE_PIC_WIDTH_IS_4096; > > > + if (ctx->src_fmt.width > 2048) > > > + reg |= VE_MODE_PIC_WIDTH_MORE_2048; > > > + > > > + cedrus_write(ctx->dev, VE_MODE, reg); > > > > > > return 0; > > > > > > } > > > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h > > > b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h index > > > 27d0882397aa..604ff932fbf5 100644 > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h > > > @@ -16,7 +16,7 @@ > > > > > > #ifndef _CEDRUS_HW_H_ > > > #define _CEDRUS_HW_H_ > > > > > > -int cedrus_engine_enable(struct cedrus_dev *dev, enum cedrus_codec > > > codec); > > > +int cedrus_engine_enable(struct cedrus_ctx *ctx, enum cedrus_codec > > > codec); > > > > > > void cedrus_engine_disable(struct cedrus_dev *dev); > > > > > > void cedrus_dst_format_set(struct cedrus_dev *dev, > > > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c > > > b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c index > > > 13c34927bad5..8bcd6b8f9e2d 100644 > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c > > > @@ -96,7 +96,7 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, > > > struct cedrus_run *run)> > > > quantization = run->mpeg2.quantization; > > > > > > /* Activate MPEG engine. */ > > > > > > - cedrus_engine_enable(dev, CEDRUS_CODEC_MPEG2); > > > + cedrus_engine_enable(ctx, CEDRUS_CODEC_MPEG2); > > > > > > /* Set intra quantization matrix. */ > > > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h > > > b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h index > > > 4275a307d282..ace3d49fcd82 100644 > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h > > > @@ -35,6 +35,8 @@ > > > > > > #define VE_MODE 0x00 > > > > > > +#define VE_MODE_PIC_WIDTH_IS_4096 BIT(22) > > > +#define VE_MODE_PIC_WIDTH_MORE_2048 BIT(21) > > > > > > #define VE_MODE_REC_WR_MODE_2MB (0x01 << 20) > > > #define VE_MODE_REC_WR_MODE_1MB (0x00 << 20) > > > #define VE_MODE_DDR_MODE_BW_128 (0x03 << 16) > > > >
Dne torek, 05. november 2019 ob 09:10:34 CET je Paul Kocialkowski napisal(a): > Hi, > > On Mon 04 Nov 19, 17:33, Jernej Škrabec wrote: > > Dne ponedeljek, 04. november 2019 ob 11:02:28 CET je Paul Kocialkowski > > > > napisal(a): > > > Hi Jernej, > > > > > > On Sat 26 Oct 19, 09:49, Jernej Skrabec wrote: > > > > Mode register also holds information if video width is bigger than > > > > 2048 > > > > and if it is equal to 4096. > > > > > > > > Rework cedrus_engine_enable() to properly signal this properties. > > > > > > Thanks for the patch, looks good to me! > > > > > > Acked-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > > > > > > One minor thing: maybe we should have a way to set the maximum > > > dimensions > > > depending on the generation of the engine in use and the actual maximum > > > supported by the hardware. > > > > > > Maybe either as dedicated new fields in struct cedrus_variant or as > > > capability flags. > > > > I was thinking about first solution, but after going trough manuals, it > > was > > unclear what are real limitations. For example, H3 manual states that it > > is > > capable of decoding H264 1080p@60Hz. However, I know for a fact that it is > > also capable of decoding 4k videos, but probably not at 60 Hz. I don't own > > anything older that A83T, so I don't know what are capabilities of those > > SoCs. > So I guess in this case we should try and see. I could try to look into it > at some point in the future too if you're not particulary interested. Well, I can take a look at my HW, but I have only few SoCs with more or less same capability. > > Anyway, being slow is still ok for some tasks, like transcoding, so we > > can't limit decoding to 1080p just because it's slow. It is probably > > still faster than doing it in SW. Not to mention that it's still ok for > > some videos, a lot of them uses 24 fps. > > I agree, it's best to expose the maximum supported resolution by the > hardware, even if it means running at a lower fps. > > Do you know if we have a way to report some estimation of the maximum > supported fps to userspace? It would be useful to let userspace decide > whether it's a better fit than software decoding. I took a quick look at existing controls, but I don't see anything appropriate. Best regards, Jernej > > Cheers, > > Paul > > > Best regards, > > Jernej > > > > > Anyway that can be done later since we were already hardcoding this. > > > > > > Cheers, > > > > > > Paul > > > > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > > > --- > > > > > > > > drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 2 +- > > > > drivers/staging/media/sunxi/cedrus/cedrus_h265.c | 2 +- > > > > drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 9 +++++++-- > > > > drivers/staging/media/sunxi/cedrus/cedrus_hw.h | 2 +- > > > > drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c | 2 +- > > > > drivers/staging/media/sunxi/cedrus/cedrus_regs.h | 2 ++ > > > > 6 files changed, 13 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > > b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c index > > > > 7487f6ab7576..d2c854ecdf15 100644 > > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > > @@ -485,7 +485,7 @@ static void cedrus_h264_setup(struct cedrus_ctx > > > > *ctx, > > > > > > > > { > > > > > > > > struct cedrus_dev *dev = ctx->dev; > > > > > > > > - cedrus_engine_enable(dev, CEDRUS_CODEC_H264); > > > > + cedrus_engine_enable(ctx, CEDRUS_CODEC_H264); > > > > > > > > cedrus_write(dev, VE_H264_SDROT_CTRL, 0); > > > > cedrus_write(dev, VE_H264_EXTRA_BUFFER1, > > > > > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c > > > > b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c index > > > > 9bc921866f70..6945dc74e1d7 100644 > > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c > > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c > > > > @@ -276,7 +276,7 @@ static void cedrus_h265_setup(struct cedrus_ctx > > > > *ctx, > > > > > > > > } > > > > > > > > /* Activate H265 engine. */ > > > > > > > > - cedrus_engine_enable(dev, CEDRUS_CODEC_H265); > > > > + cedrus_engine_enable(ctx, CEDRUS_CODEC_H265); > > > > > > > > /* Source offset and length in bits. */ > > > > > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > > > > b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c index > > > > 570a9165dd5d..3acfa21bc124 100644 > > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > > > > @@ -30,7 +30,7 @@ > > > > > > > > #include "cedrus_hw.h" > > > > #include "cedrus_regs.h" > > > > > > > > -int cedrus_engine_enable(struct cedrus_dev *dev, enum cedrus_codec > > > > codec) > > > > +int cedrus_engine_enable(struct cedrus_ctx *ctx, enum cedrus_codec > > > > codec) > > > > > > > > { > > > > > > > > u32 reg = 0; > > > > > > > > @@ -58,7 +58,12 @@ int cedrus_engine_enable(struct cedrus_dev *dev, > > > > enum > > > > cedrus_codec codec)> > > > > > > > > return -EINVAL; > > > > > > > > } > > > > > > > > - cedrus_write(dev, VE_MODE, reg); > > > > + if (ctx->src_fmt.width == 4096) > > > > + reg |= VE_MODE_PIC_WIDTH_IS_4096; > > > > + if (ctx->src_fmt.width > 2048) > > > > + reg |= VE_MODE_PIC_WIDTH_MORE_2048; > > > > + > > > > + cedrus_write(ctx->dev, VE_MODE, reg); > > > > > > > > return 0; > > > > > > > > } > > > > > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h > > > > b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h index > > > > 27d0882397aa..604ff932fbf5 100644 > > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h > > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h > > > > @@ -16,7 +16,7 @@ > > > > > > > > #ifndef _CEDRUS_HW_H_ > > > > #define _CEDRUS_HW_H_ > > > > > > > > -int cedrus_engine_enable(struct cedrus_dev *dev, enum cedrus_codec > > > > codec); > > > > +int cedrus_engine_enable(struct cedrus_ctx *ctx, enum cedrus_codec > > > > codec); > > > > > > > > void cedrus_engine_disable(struct cedrus_dev *dev); > > > > > > > > void cedrus_dst_format_set(struct cedrus_dev *dev, > > > > > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c > > > > b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c index > > > > 13c34927bad5..8bcd6b8f9e2d 100644 > > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c > > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c > > > > @@ -96,7 +96,7 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx > > > > *ctx, > > > > struct cedrus_run *run)> > > > > > > > > quantization = run->mpeg2.quantization; > > > > > > > > /* Activate MPEG engine. */ > > > > > > > > - cedrus_engine_enable(dev, CEDRUS_CODEC_MPEG2); > > > > + cedrus_engine_enable(ctx, CEDRUS_CODEC_MPEG2); > > > > > > > > /* Set intra quantization matrix. */ > > > > > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h > > > > b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h index > > > > 4275a307d282..ace3d49fcd82 100644 > > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h > > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h > > > > @@ -35,6 +35,8 @@ > > > > > > > > #define VE_MODE 0x00 > > > > > > > > +#define VE_MODE_PIC_WIDTH_IS_4096 BIT(22) > > > > +#define VE_MODE_PIC_WIDTH_MORE_2048 BIT(21) > > > > > > > > #define VE_MODE_REC_WR_MODE_2MB (0x01 << 20) > > > > #define VE_MODE_REC_WR_MODE_1MB (0x00 << 20) > > > > #define VE_MODE_DDR_MODE_BW_128 (0x03 << 16)
Hi Stefan, On Thu 07 Nov 19, 09:24, Stefan Monnier wrote: > > Do you know if we have a way to report some estimation of the maximum supported > > fps to userspace? It would be useful to let userspace decide whether it's a > > better fit than software decoding. > > Even if the fps ends up too low for the player's taste, I can't imagine > why software decoding would be preferable: it seems it could be only > even (substantially) slower. Or are there speed-up options in software > decoding not available in hardware decoding (such as playing only every > N'th frame, maybe?). This may be true for the Allwinner case as we know it today but not true in general. It could happen that the CPU is perfectly able to decode as fast as or faster than the hardware implementation, but userspace would still try to use hardware video decoding when it can provide good enough performance so that the CPU can do other things in the meantime. Having a good idea of the expected performance is important for userspace to make this kind of policy decision. This is kind of a common misconception that hardware offloading always implies a performance improvment. In our cases where the CPU is a bottleneck, it is more often true than not, but this is by far not true in general. Cheers, Paul
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c index 7487f6ab7576..d2c854ecdf15 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c @@ -485,7 +485,7 @@ static void cedrus_h264_setup(struct cedrus_ctx *ctx, { struct cedrus_dev *dev = ctx->dev; - cedrus_engine_enable(dev, CEDRUS_CODEC_H264); + cedrus_engine_enable(ctx, CEDRUS_CODEC_H264); cedrus_write(dev, VE_H264_SDROT_CTRL, 0); cedrus_write(dev, VE_H264_EXTRA_BUFFER1, diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c index 9bc921866f70..6945dc74e1d7 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c @@ -276,7 +276,7 @@ static void cedrus_h265_setup(struct cedrus_ctx *ctx, } /* Activate H265 engine. */ - cedrus_engine_enable(dev, CEDRUS_CODEC_H265); + cedrus_engine_enable(ctx, CEDRUS_CODEC_H265); /* Source offset and length in bits. */ diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c index 570a9165dd5d..3acfa21bc124 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c @@ -30,7 +30,7 @@ #include "cedrus_hw.h" #include "cedrus_regs.h" -int cedrus_engine_enable(struct cedrus_dev *dev, enum cedrus_codec codec) +int cedrus_engine_enable(struct cedrus_ctx *ctx, enum cedrus_codec codec) { u32 reg = 0; @@ -58,7 +58,12 @@ int cedrus_engine_enable(struct cedrus_dev *dev, enum cedrus_codec codec) return -EINVAL; } - cedrus_write(dev, VE_MODE, reg); + if (ctx->src_fmt.width == 4096) + reg |= VE_MODE_PIC_WIDTH_IS_4096; + if (ctx->src_fmt.width > 2048) + reg |= VE_MODE_PIC_WIDTH_MORE_2048; + + cedrus_write(ctx->dev, VE_MODE, reg); return 0; } diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h index 27d0882397aa..604ff932fbf5 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h @@ -16,7 +16,7 @@ #ifndef _CEDRUS_HW_H_ #define _CEDRUS_HW_H_ -int cedrus_engine_enable(struct cedrus_dev *dev, enum cedrus_codec codec); +int cedrus_engine_enable(struct cedrus_ctx *ctx, enum cedrus_codec codec); void cedrus_engine_disable(struct cedrus_dev *dev); void cedrus_dst_format_set(struct cedrus_dev *dev, diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c index 13c34927bad5..8bcd6b8f9e2d 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c @@ -96,7 +96,7 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run) quantization = run->mpeg2.quantization; /* Activate MPEG engine. */ - cedrus_engine_enable(dev, CEDRUS_CODEC_MPEG2); + cedrus_engine_enable(ctx, CEDRUS_CODEC_MPEG2); /* Set intra quantization matrix. */ diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h index 4275a307d282..ace3d49fcd82 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h +++ b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h @@ -35,6 +35,8 @@ #define VE_MODE 0x00 +#define VE_MODE_PIC_WIDTH_IS_4096 BIT(22) +#define VE_MODE_PIC_WIDTH_MORE_2048 BIT(21) #define VE_MODE_REC_WR_MODE_2MB (0x01 << 20) #define VE_MODE_REC_WR_MODE_1MB (0x00 << 20) #define VE_MODE_DDR_MODE_BW_128 (0x03 << 16)
Mode register also holds information if video width is bigger than 2048 and if it is equal to 4096. Rework cedrus_engine_enable() to properly signal this properties. Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> --- drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 2 +- drivers/staging/media/sunxi/cedrus/cedrus_h265.c | 2 +- drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 9 +++++++-- drivers/staging/media/sunxi/cedrus/cedrus_hw.h | 2 +- drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c | 2 +- drivers/staging/media/sunxi/cedrus/cedrus_regs.h | 2 ++ 6 files changed, 13 insertions(+), 6 deletions(-)