* [Patch 06/35] media: ti-vpe: vpe: Do not perform job transaction atomically
@ 2016-09-28 21:20 Benoit Parrot
2016-10-17 14:17 ` Hans Verkuil
0 siblings, 1 reply; 3+ messages in thread
From: Benoit Parrot @ 2016-09-28 21:20 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, linux-kernel
From: Nikhil Devshatwar <nikhil.nd@ti.com>
Current VPE driver does not start the job until all the buffers for
a transaction are not queued. When running in multiple context, this might
increase the processing latency.
Alternate solution would be to try to continue the same context as long as
buffers for the transaction are ready; else switch the context. This may
increase number of context switches but it reduces latency significantly.
In this approach, the job_ready always succeeds as long as there are
buffers on the CAPTURE and OUTPUT stream. Processing may start immediately
as the first 2 iterations don't need extra source buffers. Shift all the
source buffers after each iteration and remove the oldest buffer.
Also, with this removes the constraint of pre buffering 3 buffers before
call to STREAMON in case of de-interlacing.
Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com>
Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
drivers/media/platform/ti-vpe/vpe.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
index a0b29685fb69..9c38eff5df46 100644
--- a/drivers/media/platform/ti-vpe/vpe.c
+++ b/drivers/media/platform/ti-vpe/vpe.c
@@ -898,15 +898,14 @@ static struct vpe_ctx *file2ctx(struct file *file)
static int job_ready(void *priv)
{
struct vpe_ctx *ctx = priv;
- int needed = ctx->bufs_per_job;
- if (ctx->deinterlacing && ctx->src_vbs[2] == NULL)
- needed += 2; /* need additional two most recent fields */
-
- if (v4l2_m2m_num_src_bufs_ready(ctx->fh.m2m_ctx) < needed)
- return 0;
-
- if (v4l2_m2m_num_dst_bufs_ready(ctx->fh.m2m_ctx) < needed)
+ /*
+ * This check is needed as this might be called directly from driver
+ * When called by m2m framework, this will always satisy, but when
+ * called from vpe_irq, this might fail. (src stream with zero buffers)
+ */
+ if (v4l2_m2m_num_src_bufs_ready(ctx->fh.m2m_ctx) <= 0 ||
+ v4l2_m2m_num_dst_bufs_ready(ctx->fh.m2m_ctx) <= 0)
return 0;
return 1;
@@ -1116,19 +1115,20 @@ static void device_run(void *priv)
struct sc_data *sc = ctx->dev->sc;
struct vpe_q_data *d_q_data = &ctx->q_data[Q_DATA_DST];
- if (ctx->deinterlacing && ctx->src_vbs[2] == NULL) {
- ctx->src_vbs[2] = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
- WARN_ON(ctx->src_vbs[2] == NULL);
- ctx->src_vbs[1] = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
- WARN_ON(ctx->src_vbs[1] == NULL);
- }
-
ctx->src_vbs[0] = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
WARN_ON(ctx->src_vbs[0] == NULL);
ctx->dst_vb = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
WARN_ON(ctx->dst_vb == NULL);
if (ctx->deinterlacing) {
+
+ if (ctx->src_vbs[2] == NULL) {
+ ctx->src_vbs[2] = ctx->src_vbs[0];
+ WARN_ON(ctx->src_vbs[2] == NULL);
+ ctx->src_vbs[1] = ctx->src_vbs[0];
+ WARN_ON(ctx->src_vbs[1] == NULL);
+ }
+
/*
* we have output the first 2 frames through line average, we
* now switch to EDI de-interlacer
@@ -1349,7 +1349,7 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data)
}
ctx->bufs_completed++;
- if (ctx->bufs_completed < ctx->bufs_per_job) {
+ if (ctx->bufs_completed < ctx->bufs_per_job && job_ready(ctx)) {
device_run(ctx);
goto handled;
}
--
2.9.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Patch 06/35] media: ti-vpe: vpe: Do not perform job transaction atomically
2016-09-28 21:20 [Patch 06/35] media: ti-vpe: vpe: Do not perform job transaction atomically Benoit Parrot
@ 2016-10-17 14:17 ` Hans Verkuil
2016-10-24 16:57 ` Benoit Parrot
0 siblings, 1 reply; 3+ messages in thread
From: Hans Verkuil @ 2016-10-17 14:17 UTC (permalink / raw)
To: Benoit Parrot; +Cc: linux-media, linux-kernel
On 09/28/2016 11:20 PM, Benoit Parrot wrote:
> From: Nikhil Devshatwar <nikhil.nd@ti.com>
>
> Current VPE driver does not start the job until all the buffers for
> a transaction are not queued. When running in multiple context, this might
I think this should be: s/not queued/queued/, right?
> increase the processing latency.
>
> Alternate solution would be to try to continue the same context as long as
> buffers for the transaction are ready; else switch the context. This may
> increase number of context switches but it reduces latency significantly.
>
> In this approach, the job_ready always succeeds as long as there are
> buffers on the CAPTURE and OUTPUT stream. Processing may start immediately
> as the first 2 iterations don't need extra source buffers. Shift all the
> source buffers after each iteration and remove the oldest buffer.
>
> Also, with this removes the constraint of pre buffering 3 buffers before
> call to STREAMON in case of de-interlacing.
>
> Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com>
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> ---
> drivers/media/platform/ti-vpe/vpe.c | 32 ++++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
> index a0b29685fb69..9c38eff5df46 100644
> --- a/drivers/media/platform/ti-vpe/vpe.c
> +++ b/drivers/media/platform/ti-vpe/vpe.c
> @@ -898,15 +898,14 @@ static struct vpe_ctx *file2ctx(struct file *file)
> static int job_ready(void *priv)
> {
> struct vpe_ctx *ctx = priv;
> - int needed = ctx->bufs_per_job;
>
> - if (ctx->deinterlacing && ctx->src_vbs[2] == NULL)
> - needed += 2; /* need additional two most recent fields */
> -
> - if (v4l2_m2m_num_src_bufs_ready(ctx->fh.m2m_ctx) < needed)
> - return 0;
> -
> - if (v4l2_m2m_num_dst_bufs_ready(ctx->fh.m2m_ctx) < needed)
> + /*
> + * This check is needed as this might be called directly from driver
> + * When called by m2m framework, this will always satisy, but when
typo: satisfy
> + * called from vpe_irq, this might fail. (src stream with zero buffers)
> + */
> + if (v4l2_m2m_num_src_bufs_ready(ctx->fh.m2m_ctx) <= 0 ||
> + v4l2_m2m_num_dst_bufs_ready(ctx->fh.m2m_ctx) <= 0)
> return 0;
>
> return 1;
> @@ -1116,19 +1115,20 @@ static void device_run(void *priv)
> struct sc_data *sc = ctx->dev->sc;
> struct vpe_q_data *d_q_data = &ctx->q_data[Q_DATA_DST];
>
> - if (ctx->deinterlacing && ctx->src_vbs[2] == NULL) {
> - ctx->src_vbs[2] = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> - WARN_ON(ctx->src_vbs[2] == NULL);
> - ctx->src_vbs[1] = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> - WARN_ON(ctx->src_vbs[1] == NULL);
> - }
> -
> ctx->src_vbs[0] = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> WARN_ON(ctx->src_vbs[0] == NULL);
> ctx->dst_vb = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> WARN_ON(ctx->dst_vb == NULL);
>
> if (ctx->deinterlacing) {
> +
> + if (ctx->src_vbs[2] == NULL) {
> + ctx->src_vbs[2] = ctx->src_vbs[0];
> + WARN_ON(ctx->src_vbs[2] == NULL);
> + ctx->src_vbs[1] = ctx->src_vbs[0];
> + WARN_ON(ctx->src_vbs[1] == NULL);
> + }
> +
> /*
> * we have output the first 2 frames through line average, we
> * now switch to EDI de-interlacer
> @@ -1349,7 +1349,7 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data)
> }
>
> ctx->bufs_completed++;
> - if (ctx->bufs_completed < ctx->bufs_per_job) {
> + if (ctx->bufs_completed < ctx->bufs_per_job && job_ready(ctx)) {
> device_run(ctx);
> goto handled;
> }
>
Regards,
Hans
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Patch 06/35] media: ti-vpe: vpe: Do not perform job transaction atomically
2016-10-17 14:17 ` Hans Verkuil
@ 2016-10-24 16:57 ` Benoit Parrot
0 siblings, 0 replies; 3+ messages in thread
From: Benoit Parrot @ 2016-10-24 16:57 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, linux-kernel
Hans Verkuil <hverkuil@xs4all.nl> wrote on Mon [2016-Oct-17 16:17:15 +0200]:
> On 09/28/2016 11:20 PM, Benoit Parrot wrote:
> > From: Nikhil Devshatwar <nikhil.nd@ti.com>
> >
> > Current VPE driver does not start the job until all the buffers for
> > a transaction are not queued. When running in multiple context, this might
>
> I think this should be: s/not queued/queued/, right?
Yep, will fix that.
>
> > increase the processing latency.
> >
> > Alternate solution would be to try to continue the same context as long as
> > buffers for the transaction are ready; else switch the context. This may
> > increase number of context switches but it reduces latency significantly.
> >
> > In this approach, the job_ready always succeeds as long as there are
> > buffers on the CAPTURE and OUTPUT stream. Processing may start immediately
> > as the first 2 iterations don't need extra source buffers. Shift all the
> > source buffers after each iteration and remove the oldest buffer.
> >
> > Also, with this removes the constraint of pre buffering 3 buffers before
> > call to STREAMON in case of de-interlacing.
> >
> > Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com>
> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
> > ---
> > drivers/media/platform/ti-vpe/vpe.c | 32 ++++++++++++++++----------------
> > 1 file changed, 16 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
> > index a0b29685fb69..9c38eff5df46 100644
> > --- a/drivers/media/platform/ti-vpe/vpe.c
> > +++ b/drivers/media/platform/ti-vpe/vpe.c
> > @@ -898,15 +898,14 @@ static struct vpe_ctx *file2ctx(struct file *file)
> > static int job_ready(void *priv)
> > {
> > struct vpe_ctx *ctx = priv;
> > - int needed = ctx->bufs_per_job;
> >
> > - if (ctx->deinterlacing && ctx->src_vbs[2] == NULL)
> > - needed += 2; /* need additional two most recent fields */
> > -
> > - if (v4l2_m2m_num_src_bufs_ready(ctx->fh.m2m_ctx) < needed)
> > - return 0;
> > -
> > - if (v4l2_m2m_num_dst_bufs_ready(ctx->fh.m2m_ctx) < needed)
> > + /*
> > + * This check is needed as this might be called directly from driver
> > + * When called by m2m framework, this will always satisy, but when
>
> typo: satisfy
Will fix.
>
> > + * called from vpe_irq, this might fail. (src stream with zero buffers)
> > + */
> > + if (v4l2_m2m_num_src_bufs_ready(ctx->fh.m2m_ctx) <= 0 ||
> > + v4l2_m2m_num_dst_bufs_ready(ctx->fh.m2m_ctx) <= 0)
> > return 0;
> >
> > return 1;
> > @@ -1116,19 +1115,20 @@ static void device_run(void *priv)
> > struct sc_data *sc = ctx->dev->sc;
> > struct vpe_q_data *d_q_data = &ctx->q_data[Q_DATA_DST];
> >
> > - if (ctx->deinterlacing && ctx->src_vbs[2] == NULL) {
> > - ctx->src_vbs[2] = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > - WARN_ON(ctx->src_vbs[2] == NULL);
> > - ctx->src_vbs[1] = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > - WARN_ON(ctx->src_vbs[1] == NULL);
> > - }
> > -
> > ctx->src_vbs[0] = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > WARN_ON(ctx->src_vbs[0] == NULL);
> > ctx->dst_vb = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > WARN_ON(ctx->dst_vb == NULL);
> >
> > if (ctx->deinterlacing) {
> > +
> > + if (ctx->src_vbs[2] == NULL) {
> > + ctx->src_vbs[2] = ctx->src_vbs[0];
> > + WARN_ON(ctx->src_vbs[2] == NULL);
> > + ctx->src_vbs[1] = ctx->src_vbs[0];
> > + WARN_ON(ctx->src_vbs[1] == NULL);
> > + }
> > +
> > /*
> > * we have output the first 2 frames through line average, we
> > * now switch to EDI de-interlacer
> > @@ -1349,7 +1349,7 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data)
> > }
> >
> > ctx->bufs_completed++;
> > - if (ctx->bufs_completed < ctx->bufs_per_job) {
> > + if (ctx->bufs_completed < ctx->bufs_per_job && job_ready(ctx)) {
> > device_run(ctx);
> > goto handled;
> > }
> >
>
> Regards,
>
> Hans
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-10-24 16:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-28 21:20 [Patch 06/35] media: ti-vpe: vpe: Do not perform job transaction atomically Benoit Parrot
2016-10-17 14:17 ` Hans Verkuil
2016-10-24 16:57 ` Benoit Parrot
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).