Make sure .device_run is always called in non-atomic context

Now that the mem2mem framework guarantees that .device_run won't be called from interrupt context, it is safe to call v4l2_m2m_job_finish directly in the top-half.

So this means the bottom-half is no longer needed and we can get rid of it.

Signed-off-by Ezequiel Garcia <[email protected]>

---
 .../staging/media/sunxi/cedrus/cedrus_hw.c    | 26 ++++---------------
 1 file changed, 5 insertions(+), 21 deletions(-)

drivers/staging/media/sunxi/cedrus/cedrus_hw.c +5 -21

index 32adbcbe6175..493e65b17b30 100644
@@ -98,23 +98,6 @@ void cedrus_dst_format_set(struct cedrus_dev *dev,
 	}
 }
 
-static irqreturn_t cedrus_bh(int irq, void *data)
-{
-	struct cedrus_dev *dev = data;
-	struct cedrus_ctx *ctx;
-
-	ctx = v4l2_m2m_get_curr_priv(dev->m2m_dev);
-	if (!ctx) {
-		v4l2_err(&dev->v4l2_dev,
-			 "Instance released before the end of transaction\n");
-		return IRQ_HANDLED;
-	}
-
-	v4l2_m2m_job_finish(ctx->dev->m2m_dev, ctx->fh.m2m_ctx);
-
-	return IRQ_HANDLED;
-}
-
 static irqreturn_t cedrus_irq(int irq, void *data)
 {
 	struct cedrus_dev *dev = data;

@@ -165,7 +148,9 @@ static irqreturn_t cedrus_irq(int irq, void *data)
 
 	spin_unlock_irqrestore(&dev->irq_lock, flags);
 
-	return IRQ_WAKE_THREAD;
+	v4l2_m2m_job_finish(ctx->dev->m2m_dev, ctx->fh.m2m_ctx);
+
+	return IRQ_HANDLED;
 }
 
 int cedrus_hw_probe(struct cedrus_dev *dev)

@@ -187,9 +172,8 @@ int cedrus_hw_probe(struct cedrus_dev *dev)
 
 		return irq_dec;
 	}
-	ret = devm_request_threaded_irq(dev->dev, irq_dec, cedrus_irq,
-					cedrus_bh, 0, dev_name(dev->dev),
-					dev);
+	ret = devm_request_irq(dev->dev, irq_dec, cedrus_irq,
+			       0, dev_name(dev->dev), dev);
 	if (ret) {
 		v4l2_err(&dev->v4l2_dev, "Failed to request IRQ\n");
 

This series goal is to avoid drivers from having ad-hoc code to call .device_run in non-atomic context. Currently, .device_run can be called via v4l2_m2m_job_finish(), not only running in interrupt context, but also creating a nasty re-entrant path into mem2mem drivers.

The proposed solution is to add a per-device worker that is scheduled by v4l2_m2m_job_finish, which replaces drivers having a threaded interrupt or similar.

This change allows v4l2_m2m_job_finish() to be called in interrupt context, separating .device_run and v4l2_m2m_job_finish() contexts.

It's worth mentioning that v4l2_m2m_cancel_job() doesn't need to flush or cancel the new worker, because the job_spinlock synchronizes both and also because the core prevents simultaneous jobs. Either v4l2_m2m_cancel_job() will wait for the worker, or the worker will be unable to run a new job.

Patches apply on top of Request API and the Cedrus VPU driver.

Tested with cedrus driver using v4l2-request-test and vicodec driver using gstreamer.

Ezequiel Garcia (4):
mem2mem: Require capture and output mutexes to match v4l2-ioctl.c: Simplify locking for m2m devices
v4l2-mem2mem: Avoid calling .device_run in v4l2_m2m_job_finish
media: cedrus: Get rid of interrupt bottom-half

Sakari Ailus (1):
v4l2-mem2mem: Simplify exiting the function in __v4l2_m2m_try_schedule

 drivers/media/v4l2-core/v4l2-ioctl.c          | 47 +------------
 drivers/media/v4l2-core/v4l2-mem2mem.c        | 66 ++++++++++++-------
 .../staging/media/sunxi/cedrus/cedrus_hw.c    | 26 ++------
 3 files changed, 51 insertions(+), 88 deletions(-)

Quoted 31 lines · Ezequiel Garcia

This series goal is to avoid drivers from having ad-hoc code to call .device_run in non-atomic context. Currently, .device_run can be called via v4l2_m2m_job_finish(), not only running in interrupt context, but also creating a nasty re-entrant path into mem2mem drivers.

The proposed solution is to add a per-device worker that is scheduled by v4l2_m2m_job_finish, which replaces drivers having a threaded interrupt or similar.

This change allows v4l2_m2m_job_finish() to be called in interrupt context, separating .device_run and v4l2_m2m_job_finish() contexts.

It's worth mentioning that v4l2_m2m_cancel_job() doesn't need to flush or cancel the new worker, because the job_spinlock synchronizes both and also because the core prevents simultaneous jobs. Either v4l2_m2m_cancel_job() will wait for the worker, or the worker will be unable to run a new job.

Patches apply on top of Request API and the Cedrus VPU driver.

Tested with cedrus driver using v4l2-request-test and vicodec driver using gstreamer.

Ezequiel Garcia (4):
mem2mem: Require capture and output mutexes to match v4l2-ioctl.c: Simplify locking for m2m devices
v4l2-mem2mem: Avoid calling .device_run in v4l2_m2m_job_finish
media: cedrus: Get rid of interrupt bottom-half

Quoted 8 lines

Sakari Ailus (1):
v4l2-mem2mem: Simplify exiting the function in __v4l2_m2m_try_schedule

 drivers/media/v4l2-core/v4l2-ioctl.c          | 47 +------------
 drivers/media/v4l2-core/v4l2-mem2mem.c        | 66 ++++++++++++-------
 .../staging/media/sunxi/cedrus/cedrus_hw.c    | 26 ++------
 3 files changed, 51 insertions(+), 88 deletions(-)

Hans, Maxime:

Any feedback for this?

Hi,

Quoted 41 lines · Ezequiel Garcia

Quoted 39 lines · Ezequiel Garcia

This series goal is to avoid drivers from having ad-hoc code to call .device_run in non-atomic context. Currently, .device_run can be called via v4l2_m2m_job_finish(), not only running in interrupt context, but also creating a nasty re-entrant path into mem2mem drivers.

The proposed solution is to add a per-device worker that is scheduled by v4l2_m2m_job_finish, which replaces drivers having a threaded interrupt or similar.

This change allows v4l2_m2m_job_finish() to be called in interrupt context, separating .device_run and v4l2_m2m_job_finish() contexts.

It's worth mentioning that v4l2_m2m_cancel_job() doesn't need to flush or cancel the new worker, because the job_spinlock synchronizes both and also because the core prevents simultaneous jobs. Either v4l2_m2m_cancel_job() will wait for the worker, or the worker will be unable to run a new job.

Patches apply on top of Request API and the Cedrus VPU driver.

Tested with cedrus driver using v4l2-request-test and vicodec driver using gstreamer.

Ezequiel Garcia (4):
mem2mem: Require capture and output mutexes to match v4l2-ioctl.c: Simplify locking for m2m devices
v4l2-mem2mem: Avoid calling .device_run in v4l2_m2m_job_finish
media: cedrus: Get rid of interrupt bottom-half

Sakari Ailus (1):
v4l2-mem2mem: Simplify exiting the function in __v4l2_m2m_try_schedule

 drivers/media/v4l2-core/v4l2-ioctl.c          | 47 +------------
 drivers/media/v4l2-core/v4l2-mem2mem.c        | 66 ++++++++++++-------
 .../staging/media/sunxi/cedrus/cedrus_hw.c    | 26 ++------
 3 files changed, 51 insertions(+), 88 deletions(-)

Quoted 3 lines

Hans, Maxime:

Any feedback for this?

I just tested the whole series with the cedrus driver and everything looks good!

Removing the interrupt bottom-half in favor of a workqueue in the core seems like a good way to simplify m2m driver development by avoiding per-driver workqueues or threaded irqs.

Quoted 46 lines · Paul Kocialkowski

Hi,

Quoted 43 lines · Ezequiel Garcia

Quoted 31 lines · Ezequiel Garcia

This series goal is to avoid drivers from having ad-hoc code to call .device_run in non-atomic context. Currently, .device_run can be called via v4l2_m2m_job_finish(), not only running in interrupt context, but also creating a nasty re-entrant path into mem2mem drivers.

The proposed solution is to add a per-device worker that is scheduled by v4l2_m2m_job_finish, which replaces drivers having a threaded interrupt or similar.

This change allows v4l2_m2m_job_finish() to be called in interrupt context, separating .device_run and v4l2_m2m_job_finish() contexts.

It's worth mentioning that v4l2_m2m_cancel_job() doesn't need to flush or cancel the new worker, because the job_spinlock synchronizes both and also because the core prevents simultaneous jobs. Either v4l2_m2m_cancel_job() will wait for the worker, or the worker will be unable to run a new job.

Patches apply on top of Request API and the Cedrus VPU driver.

Tested with cedrus driver using v4l2-request-test and vicodec driver using gstreamer.

Ezequiel Garcia (4):
mem2mem: Require capture and output mutexes to match v4l2-ioctl.c: Simplify locking for m2m devices
v4l2-mem2mem: Avoid calling .device_run in v4l2_m2m_job_finish
media: cedrus: Get rid of interrupt bottom-half

Quoted 8 lines

Sakari Ailus (1):
v4l2-mem2mem: Simplify exiting the function in __v4l2_m2m_try_schedule

 drivers/media/v4l2-core/v4l2-ioctl.c          | 47 +------------
 drivers/media/v4l2-core/v4l2-mem2mem.c        | 66 ++++++++++++-------
 .../staging/media/sunxi/cedrus/cedrus_hw.c    | 26 ++------
 3 files changed, 51 insertions(+), 88 deletions(-)

Hans, Maxime:

Quoted 5 lines

Quoted 1 line

Any feedback for this?

I just tested the whole series with the cedrus driver and everything looks good!

Good! So this means we can add a Tested-by for the entire series?

Quoted 4 lines

Removing the interrupt bottom-half in favor of a workqueue in the core seems like a good way to simplify m2m driver development by avoiding per-driver workqueues or threaded irqs.

Thanks for the test!

This series goal is to avoid drivers from having ad-hoc code to call .device_run in non-atomic context. Currently, .device_run can be called via v4l2_m2m_job_finish(), not only running in interrupt context, but also creating a nasty re-entrant path into mem2mem drivers.

The proposed solution is to add a per-device worker that is scheduled by v4l2_m2m_job_finish, which replaces drivers having a threaded interrupt or similar.

This change allows v4l2_m2m_job_finish() to be called in interrupt context, separating .device_run and v4l2_m2m_job_finish() contexts.

It's worth mentioning that v4l2_m2m_cancel_job() doesn't need to flush or cancel the new worker, because the job_spinlock synchronizes both and also because the core prevents simultaneous jobs. Either v4l2_m2m_cancel_job() will wait for the worker, or the worker will be unable to run a new job.

Patches apply on top of Request API and the Cedrus VPU driver.

Tested with cedrus driver using v4l2-request-test and vicodec driver using gstreamer.

Ezequiel Garcia (4):
mem2mem: Require capture and output mutexes to match v4l2-ioctl.c: Simplify locking for m2m devices
v4l2-mem2mem: Avoid calling .device_run in v4l2_m2m_job_finish
media: cedrus: Get rid of interrupt bottom-half

Sakari Ailus (1):
v4l2-mem2mem: Simplify exiting the function in __v4l2_m2m_try_schedule

 drivers/media/v4l2-core/v4l2-ioctl.c          | 47 +------------
 drivers/media/v4l2-core/v4l2-mem2mem.c        | 66 ++++++++++++-------
 .../staging/media/sunxi/cedrus/cedrus_hw.c    | 26 ++------
 3 files changed, 51 insertions(+), 88 deletions(-)