Hi,
On Mon, 2018-06-25 at 15:32 +0200, Maxime Ripard wrote:
> On Thu, Jun 21, 2018 at 05:38:23PM +0200, Paul Kocialkowski wrote:
> > Hi,
> >
> > On Wed, 2018-06-13 at 16:07 +0200, Maxime Ripard wrote:
> > > Some codec needs to perform some additional task when a decoding is started
> > > and stopped, and not only at every frame.
> > >
> > > For example, the H264 decoding support needs to allocate buffers that will
> > > be used in the decoding process, but do not need to change over time, or at
> > > each frame.
> > >
> > > In order to allow that for codecs, introduce a start and stop hook that
> > > will be called if present at start_streaming and stop_streaming time.
> > >
> > > Signed-off-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxx>
> > > ---
> > > .../platform/sunxi/cedrus/sunxi_cedrus_common.h | 2 ++
> > > .../platform/sunxi/cedrus/sunxi_cedrus_video.c | 14 +++++++++++++-
> > > 2 files changed, 15 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/platform/sunxi/cedrus/sunxi_cedrus_common.h b/drivers/media/platform/sunxi/cedrus/sunxi_cedrus_common.h
> > > index a2a507eb9fc9..20c78ec1f037 100644
> > > --- a/drivers/media/platform/sunxi/cedrus/sunxi_cedrus_common.h
> > > +++ b/drivers/media/platform/sunxi/cedrus/sunxi_cedrus_common.h
> > > @@ -120,6 +120,8 @@ struct sunxi_cedrus_dec_ops {
> > > enum sunxi_cedrus_irq_status (*irq_status)(struct sunxi_cedrus_ctx *ctx);
> > > void (*setup)(struct sunxi_cedrus_ctx *ctx,
> > > struct sunxi_cedrus_run *run);
> > > + int (*start)(struct sunxi_cedrus_ctx *ctx);
> > > + void (*stop)(struct sunxi_cedrus_ctx *ctx);
> > > void (*trigger)(struct sunxi_cedrus_ctx *ctx);
> > > };
> > >
> > > diff --git a/drivers/media/platform/sunxi/cedrus/sunxi_cedrus_video.c b/drivers/media/platform/sunxi/cedrus/sunxi_cedrus_video.c
> > > index fb7b081a5bb7..d93461178857 100644
> > > --- a/drivers/media/platform/sunxi/cedrus/sunxi_cedrus_video.c
> > > +++ b/drivers/media/platform/sunxi/cedrus/sunxi_cedrus_video.c
> > > @@ -416,6 +416,8 @@ static int sunxi_cedrus_buf_prepare(struct vb2_buffer *vb)
> > > static int sunxi_cedrus_start_streaming(struct vb2_queue *q, unsigned int count)
> > > {
> > > struct sunxi_cedrus_ctx *ctx = vb2_get_drv_priv(q);
> > > + struct sunxi_cedrus_dev *dev = ctx->dev;
> > > + int ret = 0;
> > >
> > > switch (ctx->vpu_src_fmt->fourcc) {
> > > case V4L2_PIX_FMT_MPEG2_FRAME:
> > > @@ -425,16 +427,26 @@ static int sunxi_cedrus_start_streaming(struct vb2_queue *q, unsigned int count)
> > > return -EINVAL;
> > > }
> > >
> > > - return 0;
> > > + if (V4L2_TYPE_IS_OUTPUT(q->type) &&
> >
> > I suppose this check was put in place to ensure that ->start is only
> > called once, but what if start_streaming is called multiple times on
> > output? Am I totally unsure about whether the API guarantees that we
> > only get one start_streaming call per buffer queue, regardless of how
> > many userspace issues.
> >
> > If we don't have such a guarantee, we probably need an internal
> > mechanism to avoid having ->start called more than once.
>
> As far as I understand it, start_streaming can only be called once:
> https://elixir.bootlin.com/linux/latest/source/include/media/videobuf2-core.h#L357
And there's definitely a check in vb2_core_streamon:
if (q->streaming) {
dprintk(3, "already streaming\n");
return 0;
}
so it looks like we're safe :)
Cheers,
Paul
--
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.comAttachment:
signature.asc
Description: This is a digitally signed message part
