* [PATCH] media: cedrus: Forbid setting new formats on busy queues
@ 2019-02-14 8:37 Paul Kocialkowski
2019-02-14 8:59 ` Hans Verkuil
0 siblings, 1 reply; 4+ messages in thread
From: Paul Kocialkowski @ 2019-02-14 8:37 UTC (permalink / raw)
To: linux-media, linux-arm-kernel, linux-kernel, linux-sunxi
Cc: Maxime Ripard, Paul Kocialkowski, Mauro Carvalho Chehab,
Hans Verkuil, Thomas Petazzoni
Check that our queues are not busy before setting the format or return
EBUSY if that's the case. This ensures that our format can't change
once buffers are allocated for the queue.
Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
drivers/staging/media/sunxi/cedrus/cedrus_video.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
index b5cc79389d67..3420a938a613 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
@@ -282,8 +282,15 @@ static int cedrus_s_fmt_vid_cap(struct file *file, void *priv,
{
struct cedrus_ctx *ctx = cedrus_file2ctx(file);
struct cedrus_dev *dev = ctx->dev;
+ struct vb2_queue *vq;
int ret;
+ vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
+ if (!vq)
+ return -EINVAL;
+ else if (vb2_is_busy(vq))
+ return -EBUSY;
+
ret = cedrus_try_fmt_vid_cap(file, priv, f);
if (ret)
return ret;
@@ -299,8 +306,15 @@ static int cedrus_s_fmt_vid_out(struct file *file, void *priv,
struct v4l2_format *f)
{
struct cedrus_ctx *ctx = cedrus_file2ctx(file);
+ struct vb2_queue *vq;
int ret;
+ vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
+ if (!vq)
+ return -EINVAL;
+ else if (vb2_is_busy(vq))
+ return -EBUSY;
+
ret = cedrus_try_fmt_vid_out(file, priv, f);
if (ret)
return ret;
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] media: cedrus: Forbid setting new formats on busy queues
2019-02-14 8:37 [PATCH] media: cedrus: Forbid setting new formats on busy queues Paul Kocialkowski
@ 2019-02-14 8:59 ` Hans Verkuil
2019-02-14 9:18 ` Paul Kocialkowski
0 siblings, 1 reply; 4+ messages in thread
From: Hans Verkuil @ 2019-02-14 8:59 UTC (permalink / raw)
To: Paul Kocialkowski, linux-media, linux-arm-kernel, linux-kernel,
linux-sunxi
Cc: Maxime Ripard, Mauro Carvalho Chehab, Thomas Petazzoni
On 2/14/19 9:37 AM, Paul Kocialkowski wrote:
> Check that our queues are not busy before setting the format or return
> EBUSY if that's the case. This ensures that our format can't change
> once buffers are allocated for the queue.
>
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
> drivers/staging/media/sunxi/cedrus/cedrus_video.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> index b5cc79389d67..3420a938a613 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> @@ -282,8 +282,15 @@ static int cedrus_s_fmt_vid_cap(struct file *file, void *priv,
> {
> struct cedrus_ctx *ctx = cedrus_file2ctx(file);
> struct cedrus_dev *dev = ctx->dev;
> + struct vb2_queue *vq;
> int ret;
>
> + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> + if (!vq)
> + return -EINVAL;
Can this ever happen?
Regards,
Hans
> + else if (vb2_is_busy(vq))
> + return -EBUSY;
> +
> ret = cedrus_try_fmt_vid_cap(file, priv, f);
> if (ret)
> return ret;
> @@ -299,8 +306,15 @@ static int cedrus_s_fmt_vid_out(struct file *file, void *priv,
> struct v4l2_format *f)
> {
> struct cedrus_ctx *ctx = cedrus_file2ctx(file);
> + struct vb2_queue *vq;
> int ret;
>
> + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> + if (!vq)
> + return -EINVAL;
> + else if (vb2_is_busy(vq))
> + return -EBUSY;
> +
> ret = cedrus_try_fmt_vid_out(file, priv, f);
> if (ret)
> return ret;
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] media: cedrus: Forbid setting new formats on busy queues
2019-02-14 8:59 ` Hans Verkuil
@ 2019-02-14 9:18 ` Paul Kocialkowski
2019-02-14 9:20 ` Hans Verkuil
0 siblings, 1 reply; 4+ messages in thread
From: Paul Kocialkowski @ 2019-02-14 9:18 UTC (permalink / raw)
To: Hans Verkuil, linux-media, linux-arm-kernel, linux-kernel, linux-sunxi
Cc: Maxime Ripard, Mauro Carvalho Chehab, Thomas Petazzoni
Hi,
On Thu, 2019-02-14 at 09:59 +0100, Hans Verkuil wrote:
> On 2/14/19 9:37 AM, Paul Kocialkowski wrote:
> > Check that our queues are not busy before setting the format or return
> > EBUSY if that's the case. This ensures that our format can't change
> > once buffers are allocated for the queue.
> >
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> > drivers/staging/media/sunxi/cedrus/cedrus_video.c | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> > index b5cc79389d67..3420a938a613 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> > @@ -282,8 +282,15 @@ static int cedrus_s_fmt_vid_cap(struct file *file, void *priv,
> > {
> > struct cedrus_ctx *ctx = cedrus_file2ctx(file);
> > struct cedrus_dev *dev = ctx->dev;
> > + struct vb2_queue *vq;
> > int ret;
> >
> > + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> > + if (!vq)
> > + return -EINVAL;
>
> Can this ever happen?
I guess not, or something would be very wrong.
I have seen this check around when looking at how other drivers
implement this, but it does seem overkill.
Should I get rid of it in v2?
Cheers,
Paul
> Regards,
>
> Hans
>
> > + else if (vb2_is_busy(vq))
> > + return -EBUSY;
> > +
> > ret = cedrus_try_fmt_vid_cap(file, priv, f);
> > if (ret)
> > return ret;
> > @@ -299,8 +306,15 @@ static int cedrus_s_fmt_vid_out(struct file *file, void *priv,
> > struct v4l2_format *f)
> > {
> > struct cedrus_ctx *ctx = cedrus_file2ctx(file);
> > + struct vb2_queue *vq;
> > int ret;
> >
> > + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> > + if (!vq)
> > + return -EINVAL;
> > + else if (vb2_is_busy(vq))
> > + return -EBUSY;
> > +
> > ret = cedrus_try_fmt_vid_out(file, priv, f);
> > if (ret)
> > return ret;
> >
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] media: cedrus: Forbid setting new formats on busy queues
2019-02-14 9:18 ` Paul Kocialkowski
@ 2019-02-14 9:20 ` Hans Verkuil
0 siblings, 0 replies; 4+ messages in thread
From: Hans Verkuil @ 2019-02-14 9:20 UTC (permalink / raw)
To: Paul Kocialkowski, linux-media, linux-arm-kernel, linux-kernel,
linux-sunxi
Cc: Maxime Ripard, Mauro Carvalho Chehab, Thomas Petazzoni
On 2/14/19 10:18 AM, Paul Kocialkowski wrote:
> Hi,
>
> On Thu, 2019-02-14 at 09:59 +0100, Hans Verkuil wrote:
>> On 2/14/19 9:37 AM, Paul Kocialkowski wrote:
>>> Check that our queues are not busy before setting the format or return
>>> EBUSY if that's the case. This ensures that our format can't change
>>> once buffers are allocated for the queue.
>>>
>>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
>>> ---
>>> drivers/staging/media/sunxi/cedrus/cedrus_video.c | 14 ++++++++++++++
>>> 1 file changed, 14 insertions(+)
>>>
>>> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
>>> index b5cc79389d67..3420a938a613 100644
>>> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
>>> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
>>> @@ -282,8 +282,15 @@ static int cedrus_s_fmt_vid_cap(struct file *file, void *priv,
>>> {
>>> struct cedrus_ctx *ctx = cedrus_file2ctx(file);
>>> struct cedrus_dev *dev = ctx->dev;
>>> + struct vb2_queue *vq;
>>> int ret;
>>>
>>> + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
>>> + if (!vq)
>>> + return -EINVAL;
>>
>> Can this ever happen?
>
> I guess not, or something would be very wrong.
> I have seen this check around when looking at how other drivers
> implement this, but it does seem overkill.
>
> Should I get rid of it in v2?
Yes please!
Regards,
Hans
>
> Cheers,
>
> Paul
>
>> Regards,
>>
>> Hans
>>
>>> + else if (vb2_is_busy(vq))
>>> + return -EBUSY;
>>> +
>>> ret = cedrus_try_fmt_vid_cap(file, priv, f);
>>> if (ret)
>>> return ret;
>>> @@ -299,8 +306,15 @@ static int cedrus_s_fmt_vid_out(struct file *file, void *priv,
>>> struct v4l2_format *f)
>>> {
>>> struct cedrus_ctx *ctx = cedrus_file2ctx(file);
>>> + struct vb2_queue *vq;
>>> int ret;
>>>
>>> + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
>>> + if (!vq)
>>> + return -EINVAL;
>>> + else if (vb2_is_busy(vq))
>>> + return -EBUSY;
>>> +
>>> ret = cedrus_try_fmt_vid_out(file, priv, f);
>>> if (ret)
>>> return ret;
>>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-02-14 9:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-14 8:37 [PATCH] media: cedrus: Forbid setting new formats on busy queues Paul Kocialkowski
2019-02-14 8:59 ` Hans Verkuil
2019-02-14 9:18 ` Paul Kocialkowski
2019-02-14 9:20 ` Hans Verkuil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).