| Message ID | 20180419154124.17512-3-paul.kocialkowski@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Apr 20, 2018 at 12:43 AM Paul Kocialkowski < paul.kocialkowski@bootlin.com> wrote: > When using the request API in the context of a m2m driver, the > operations that come with a m2m run scheduling call in their > (m2m-specific) ioctl handler are delayed until the request is queued > (for instance, this includes queuing buffers and streamon). > Thus, the m2m run scheduling calls are not called in due time since the > request AP's internal plumbing will (rightfully) use the relevant core > functions directly instead of the ioctl handler. > This ends up in a situation where nothing happens if there is no > run-scheduling ioctl called after queuing the request. > In order to circumvent the issue, a new media operation is introduced, > called at the time of handling the media request queue ioctl. It gives > m2m drivers a chance to schedule a m2m device run at that time. > The existing req_queue operation cannot be used for this purpose, since > it is called with the request queue mutex held, that is eventually needed > in the device_run call to apply relevant controls. > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > --- > drivers/media/media-request.c | 3 +++ > include/media/media-device.h | 2 ++ > 2 files changed, 5 insertions(+) > diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c > index 415f7e31019d..28ac5ccfe6a2 100644 > --- a/drivers/media/media-request.c > +++ b/drivers/media/media-request.c > @@ -157,6 +157,9 @@ static long media_request_ioctl_queue(struct media_request *req) > media_request_get(req); > } > + if (mdev->ops->req_complete) > + mdev->ops->req_complete(req); > + > return ret; > } > diff --git a/include/media/media-device.h b/include/media/media-device.h > index 07e323c57202..c7dcf2079cc9 100644 > --- a/include/media/media-device.h > +++ b/include/media/media-device.h > @@ -55,6 +55,7 @@ struct media_entity_notify { > * @req_alloc: Allocate a request > * @req_free: Free a request > * @req_queue: Queue a request > + * @req_complete: Complete a request > */ > struct media_device_ops { > int (*link_notify)(struct media_link *link, u32 flags, > @@ -62,6 +63,7 @@ struct media_device_ops { > struct media_request *(*req_alloc)(struct media_device *mdev); > void (*req_free)(struct media_request *req); > int (*req_queue)(struct media_request *req); > + void (*req_complete)(struct media_request *req); This is called *before* the request is actually run, isn't it? In that case, wouldn't something like "req_schedule" be less confusing? req_complete implies that the request is already completed.
On 04/19/18 17:41, Paul Kocialkowski wrote: > When using the request API in the context of a m2m driver, the > operations that come with a m2m run scheduling call in their > (m2m-specific) ioctl handler are delayed until the request is queued > (for instance, this includes queuing buffers and streamon). > > Thus, the m2m run scheduling calls are not called in due time since the > request AP's internal plumbing will (rightfully) use the relevant core > functions directly instead of the ioctl handler. > > This ends up in a situation where nothing happens if there is no > run-scheduling ioctl called after queuing the request. > > In order to circumvent the issue, a new media operation is introduced, > called at the time of handling the media request queue ioctl. It gives > m2m drivers a chance to schedule a m2m device run at that time. > > The existing req_queue operation cannot be used for this purpose, since > it is called with the request queue mutex held, that is eventually needed > in the device_run call to apply relevant controls. I'll need to think about this a bit more. I understand the problem so something needs to be done. I would like to avoid adding yet another op, though. Regards, Hans > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > --- > drivers/media/media-request.c | 3 +++ > include/media/media-device.h | 2 ++ > 2 files changed, 5 insertions(+) > > diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c > index 415f7e31019d..28ac5ccfe6a2 100644 > --- a/drivers/media/media-request.c > +++ b/drivers/media/media-request.c > @@ -157,6 +157,9 @@ static long media_request_ioctl_queue(struct media_request *req) > media_request_get(req); > } > > + if (mdev->ops->req_complete) > + mdev->ops->req_complete(req); > + > return ret; > } > > diff --git a/include/media/media-device.h b/include/media/media-device.h > index 07e323c57202..c7dcf2079cc9 100644 > --- a/include/media/media-device.h > +++ b/include/media/media-device.h > @@ -55,6 +55,7 @@ struct media_entity_notify { > * @req_alloc: Allocate a request > * @req_free: Free a request > * @req_queue: Queue a request > + * @req_complete: Complete a request > */ > struct media_device_ops { > int (*link_notify)(struct media_link *link, u32 flags, > @@ -62,6 +63,7 @@ struct media_device_ops { > struct media_request *(*req_alloc)(struct media_device *mdev); > void (*req_free)(struct media_request *req); > int (*req_queue)(struct media_request *req); > + void (*req_complete)(struct media_request *req); > }; > > /** >
Hi Paul, On Thu, Apr 19, 2018 at 05:41:16PM +0200, Paul Kocialkowski wrote: > When using the request API in the context of a m2m driver, the > operations that come with a m2m run scheduling call in their > (m2m-specific) ioctl handler are delayed until the request is queued > (for instance, this includes queuing buffers and streamon). > > Thus, the m2m run scheduling calls are not called in due time since the > request AP's internal plumbing will (rightfully) use the relevant core > functions directly instead of the ioctl handler. > > This ends up in a situation where nothing happens if there is no > run-scheduling ioctl called after queuing the request. > > In order to circumvent the issue, a new media operation is introduced, > called at the time of handling the media request queue ioctl. It gives > m2m drivers a chance to schedule a m2m device run at that time. > > The existing req_queue operation cannot be used for this purpose, since > it is called with the request queue mutex held, that is eventually needed > in the device_run call to apply relevant controls. Based on the description I'm not entirely sure what's the intent here. What would be the purpose for acquiring the request queue mutex during the execution of the request? It is only intended for serialising access to the request objects while the request is being validated and queued. If a driver does a significant amount of processing that is not related to managing the request as such, then one option could be to add a work queue for it. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > --- > drivers/media/media-request.c | 3 +++ > include/media/media-device.h | 2 ++ > 2 files changed, 5 insertions(+) > > diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c > index 415f7e31019d..28ac5ccfe6a2 100644 > --- a/drivers/media/media-request.c > +++ b/drivers/media/media-request.c > @@ -157,6 +157,9 @@ static long media_request_ioctl_queue(struct media_request *req) > media_request_get(req); > } > > + if (mdev->ops->req_complete) > + mdev->ops->req_complete(req); > + > return ret; > } > > diff --git a/include/media/media-device.h b/include/media/media-device.h > index 07e323c57202..c7dcf2079cc9 100644 > --- a/include/media/media-device.h > +++ b/include/media/media-device.h > @@ -55,6 +55,7 @@ struct media_entity_notify { > * @req_alloc: Allocate a request > * @req_free: Free a request > * @req_queue: Queue a request > + * @req_complete: Complete a request > */ > struct media_device_ops { > int (*link_notify)(struct media_link *link, u32 flags, > @@ -62,6 +63,7 @@ struct media_device_ops { > struct media_request *(*req_alloc)(struct media_device *mdev); > void (*req_free)(struct media_request *req); > int (*req_queue)(struct media_request *req); > + void (*req_complete)(struct media_request *req); > }; > > /** > -- > 2.16.3 >
Hi, On Thu, 2018-04-19 at 17:41 +0200, Paul Kocialkowski wrote: > When using the request API in the context of a m2m driver, the > operations that come with a m2m run scheduling call in their > (m2m-specific) ioctl handler are delayed until the request is queued > (for instance, this includes queuing buffers and streamon). > > Thus, the m2m run scheduling calls are not called in due time since > the > request AP's internal plumbing will (rightfully) use the relevant core > functions directly instead of the ioctl handler. > > This ends up in a situation where nothing happens if there is no > run-scheduling ioctl called after queuing the request. > > In order to circumvent the issue, a new media operation is introduced, > called at the time of handling the media request queue ioctl. It gives > m2m drivers a chance to schedule a m2m device run at that time. > > The existing req_queue operation cannot be used for this purpose, > since > it is called with the request queue mutex held, that is eventually > needed > in the device_run call to apply relevant controls. This patch will be dropped since it's no longer useful with the latest version of the request API. > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > --- > drivers/media/media-request.c | 3 +++ > include/media/media-device.h | 2 ++ > 2 files changed, 5 insertions(+) > > diff --git a/drivers/media/media-request.c b/drivers/media/media- > request.c > index 415f7e31019d..28ac5ccfe6a2 100644 > --- a/drivers/media/media-request.c > +++ b/drivers/media/media-request.c > @@ -157,6 +157,9 @@ static long media_request_ioctl_queue(struct > media_request *req) > media_request_get(req); > } > > + if (mdev->ops->req_complete) > + mdev->ops->req_complete(req); > + > return ret; > } > > diff --git a/include/media/media-device.h b/include/media/media- > device.h > index 07e323c57202..c7dcf2079cc9 100644 > --- a/include/media/media-device.h > +++ b/include/media/media-device.h > @@ -55,6 +55,7 @@ struct media_entity_notify { > * @req_alloc: Allocate a request > * @req_free: Free a request > * @req_queue: Queue a request > + * @req_complete: Complete a request > */ > struct media_device_ops { > int (*link_notify)(struct media_link *link, u32 flags, > @@ -62,6 +63,7 @@ struct media_device_ops { > struct media_request *(*req_alloc)(struct media_device > *mdev); > void (*req_free)(struct media_request *req); > int (*req_queue)(struct media_request *req); > + void (*req_complete)(struct media_request *req); > }; > > /**
diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c index 415f7e31019d..28ac5ccfe6a2 100644 --- a/drivers/media/media-request.c +++ b/drivers/media/media-request.c @@ -157,6 +157,9 @@ static long media_request_ioctl_queue(struct media_request *req) media_request_get(req); } + if (mdev->ops->req_complete) + mdev->ops->req_complete(req); + return ret; } diff --git a/include/media/media-device.h b/include/media/media-device.h index 07e323c57202..c7dcf2079cc9 100644 --- a/include/media/media-device.h +++ b/include/media/media-device.h @@ -55,6 +55,7 @@ struct media_entity_notify { * @req_alloc: Allocate a request * @req_free: Free a request * @req_queue: Queue a request + * @req_complete: Complete a request */ struct media_device_ops { int (*link_notify)(struct media_link *link, u32 flags, @@ -62,6 +63,7 @@ struct media_device_ops { struct media_request *(*req_alloc)(struct media_device *mdev); void (*req_free)(struct media_request *req); int (*req_queue)(struct media_request *req); + void (*req_complete)(struct media_request *req); }; /**
When using the request API in the context of a m2m driver, the operations that come with a m2m run scheduling call in their (m2m-specific) ioctl handler are delayed until the request is queued (for instance, this includes queuing buffers and streamon). Thus, the m2m run scheduling calls are not called in due time since the request AP's internal plumbing will (rightfully) use the relevant core functions directly instead of the ioctl handler. This ends up in a situation where nothing happens if there is no run-scheduling ioctl called after queuing the request. In order to circumvent the issue, a new media operation is introduced, called at the time of handling the media request queue ioctl. It gives m2m drivers a chance to schedule a m2m device run at that time. The existing req_queue operation cannot be used for this purpose, since it is called with the request queue mutex held, that is eventually needed in the device_run call to apply relevant controls. Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> --- drivers/media/media-request.c | 3 +++ include/media/media-device.h | 2 ++ 2 files changed, 5 insertions(+)