* [RFC PATCH 0/2] Add v4l2_ctrl_request_create() @ 2020-07-28 9:48 Hans Verkuil 2020-07-28 9:48 ` [RFC PATCH 1/2] v4l2-ctrls.c: add v4l2_ctrl_request_create Hans Verkuil 2020-07-28 9:48 ` [RFC PATCH 2/2] vivid: call v4l2_ctrl_request_create() Hans Verkuil 0 siblings, 2 replies; 5+ messages in thread From: Hans Verkuil @ 2020-07-28 9:48 UTC (permalink / raw) To: linux-media; +Cc: Yunfei Dong The Request API is missing a feature: if userspace did not set any controls, then the request object will not contain a control object (that's created only if the user sets a control in the request). This is fine for e.g. stateless codecs since they require that each request contains controls, so this is always done. And this is also the reason that this hasn't been a problem before, since the Request API is almost exclusively used by stateless codecs. But for e.g. vivid this means that the completed request does not contain any controls in the request containing the control values at the time that the frame was captured (or output). In addition, if a driver needs to set a status control, then that control won't be part of the request either. This RFC series adds a v4l2_ctrl_request_create() function that can be called in the req_validate() callback of the request. If the request doesn't contain a control object, then it will add a new one. This is an RFC for now. Hopefully this will help Yunfei Dong with the development of Read-Only Requests, since that should be a lot simpler with this new function. Also, I am not entirely happy with the name of the function and I also think that we might need a helper in v4l2-common.c that combines with function with vb2_request_validate(), thus avoiding that drivers need to remember to call both v4l2_ctrl_request_create() and vb2_request_validate(). Regards, Hans Hans Verkuil (2): v4l2-ctrls.c: add v4l2_ctrl_request_create vivid: call v4l2_ctrl_request_create() drivers/media/mc/mc-request.c | 3 +- drivers/media/test-drivers/vivid/vivid-core.c | 4 +++ drivers/media/v4l2-core/v4l2-ctrls.c | 35 +++++++++++++++++++ include/media/v4l2-ctrls.h | 16 +++++++++ 4 files changed, 57 insertions(+), 1 deletion(-) -- 2.27.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [RFC PATCH 1/2] v4l2-ctrls.c: add v4l2_ctrl_request_create 2020-07-28 9:48 [RFC PATCH 0/2] Add v4l2_ctrl_request_create() Hans Verkuil @ 2020-07-28 9:48 ` Hans Verkuil 2020-08-18 11:30 ` Stanimir Varbanov 2020-07-28 9:48 ` [RFC PATCH 2/2] vivid: call v4l2_ctrl_request_create() Hans Verkuil 1 sibling, 1 reply; 5+ messages in thread From: Hans Verkuil @ 2020-07-28 9:48 UTC (permalink / raw) To: linux-media; +Cc: Yunfei Dong, Hans Verkuil Add a new v4l2_ctrl_request_create() function that can be called in req_validate() to create a control request object if needed. This is needed if the driver needs to set controls in the request, Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> --- drivers/media/mc/mc-request.c | 3 ++- drivers/media/v4l2-core/v4l2-ctrls.c | 35 ++++++++++++++++++++++++++++ include/media/v4l2-ctrls.h | 16 +++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/drivers/media/mc/mc-request.c b/drivers/media/mc/mc-request.c index c0782fd96c59..64df83c6f5e5 100644 --- a/drivers/media/mc/mc-request.c +++ b/drivers/media/mc/mc-request.c @@ -414,7 +414,8 @@ int media_request_object_bind(struct media_request *req, spin_lock_irqsave(&req->lock, flags); - if (WARN_ON(req->state != MEDIA_REQUEST_STATE_UPDATING)) + if (WARN_ON(req->state != MEDIA_REQUEST_STATE_UPDATING && + req->state != MEDIA_REQUEST_STATE_VALIDATING)) goto unlock; obj->req = req; diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index 3f3fbcd60cc6..0d4c8551ba2a 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -4345,6 +4345,41 @@ void v4l2_ctrl_request_complete(struct media_request *req, } EXPORT_SYMBOL(v4l2_ctrl_request_complete); +int v4l2_ctrl_request_create(struct media_request *req, + struct v4l2_ctrl_handler *main_hdl) +{ + struct media_request_object *obj; + struct v4l2_ctrl_handler *new_hdl; + int ret = 0; + + if (!req || !main_hdl) + return 0; + + if (WARN_ON(req->state != MEDIA_REQUEST_STATE_VALIDATING)) + return -EBUSY; + + /* If a request object is found, then do nothing. */ + obj = media_request_object_find(req, &req_ops, main_hdl); + if (obj) { + media_request_object_put(obj); + return 0; + } + + /* Create a new request so the driver can return controls */ + new_hdl = kzalloc(sizeof(*new_hdl), GFP_KERNEL); + if (!new_hdl) + return -ENOMEM; + + obj = &new_hdl->req_obj; + ret = v4l2_ctrl_handler_init(new_hdl, (main_hdl->nr_of_buckets - 1) * 8); + if (!ret) + ret = v4l2_ctrl_request_bind(req, new_hdl, main_hdl); + if (ret) + kfree(new_hdl); + return ret; +} +EXPORT_SYMBOL(v4l2_ctrl_request_create); + int v4l2_ctrl_request_setup(struct media_request *req, struct v4l2_ctrl_handler *main_hdl) { diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h index f40e2cbb21d3..2703baa170fa 100644 --- a/include/media/v4l2-ctrls.h +++ b/include/media/v4l2-ctrls.h @@ -1254,6 +1254,22 @@ int v4l2_ctrl_request_setup(struct media_request *req, void v4l2_ctrl_request_complete(struct media_request *req, struct v4l2_ctrl_handler *parent); +/** + * v4l2_ctrl_request_create - Create a control handler request object + * + * @req: The request + * @parent: The parent control handler ('priv' in media_request_object_find()) + * + * If the user created a request without controls, but the driver wants to + * set controls for the request, then this function can be called in the + * request's req_validate function. If there is no control object in the + * request, then this will create one. Now the driver can set controls + * and when v4l2_ctrl_request_complete() is called they will be automatically + * copied into the request. + */ +int v4l2_ctrl_request_create(struct media_request *req, + struct v4l2_ctrl_handler *parent); + /** * v4l2_ctrl_request_hdl_find - Find the control handler in the request * -- 2.27.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC PATCH 1/2] v4l2-ctrls.c: add v4l2_ctrl_request_create 2020-07-28 9:48 ` [RFC PATCH 1/2] v4l2-ctrls.c: add v4l2_ctrl_request_create Hans Verkuil @ 2020-08-18 11:30 ` Stanimir Varbanov 2020-08-18 11:39 ` Hans Verkuil 0 siblings, 1 reply; 5+ messages in thread From: Stanimir Varbanov @ 2020-08-18 11:30 UTC (permalink / raw) To: Hans Verkuil, linux-media; +Cc: Yunfei Dong Hi Hans, On 7/28/20 12:48 PM, Hans Verkuil wrote: > Add a new v4l2_ctrl_request_create() function that can be called in > req_validate() to create a control request object if needed. > > This is needed if the driver needs to set controls in the request, > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > --- > drivers/media/mc/mc-request.c | 3 ++- > drivers/media/v4l2-core/v4l2-ctrls.c | 35 ++++++++++++++++++++++++++++ > include/media/v4l2-ctrls.h | 16 +++++++++++++ > 3 files changed, 53 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/mc/mc-request.c b/drivers/media/mc/mc-request.c > index c0782fd96c59..64df83c6f5e5 100644 > --- a/drivers/media/mc/mc-request.c > +++ b/drivers/media/mc/mc-request.c > @@ -414,7 +414,8 @@ int media_request_object_bind(struct media_request *req, > > spin_lock_irqsave(&req->lock, flags); > > - if (WARN_ON(req->state != MEDIA_REQUEST_STATE_UPDATING)) > + if (WARN_ON(req->state != MEDIA_REQUEST_STATE_UPDATING && > + req->state != MEDIA_REQUEST_STATE_VALIDATING)) > goto unlock; > > obj->req = req; > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c > index 3f3fbcd60cc6..0d4c8551ba2a 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c > @@ -4345,6 +4345,41 @@ void v4l2_ctrl_request_complete(struct media_request *req, > } > EXPORT_SYMBOL(v4l2_ctrl_request_complete); > > +int v4l2_ctrl_request_create(struct media_request *req, > + struct v4l2_ctrl_handler *main_hdl) > +{ > + struct media_request_object *obj; > + struct v4l2_ctrl_handler *new_hdl; > + int ret = 0; > + > + if (!req || !main_hdl) > + return 0; > + > + if (WARN_ON(req->state != MEDIA_REQUEST_STATE_VALIDATING)) > + return -EBUSY; > + > + /* If a request object is found, then do nothing. */ > + obj = media_request_object_find(req, &req_ops, main_hdl); > + if (obj) { > + media_request_object_put(obj); > + return 0; > + } > + > + /* Create a new request so the driver can return controls */ > + new_hdl = kzalloc(sizeof(*new_hdl), GFP_KERNEL); > + if (!new_hdl) > + return -ENOMEM; > + > + obj = &new_hdl->req_obj; Why initialize 'obj' and then not use it? Did you forget something > + ret = v4l2_ctrl_handler_init(new_hdl, (main_hdl->nr_of_buckets - 1) * 8); > + if (!ret) > + ret = v4l2_ctrl_request_bind(req, new_hdl, main_hdl); > + if (ret) > + kfree(new_hdl); > + return ret; > +} > +EXPORT_SYMBOL(v4l2_ctrl_request_create); > + > int v4l2_ctrl_request_setup(struct media_request *req, > struct v4l2_ctrl_handler *main_hdl) > { > diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h > index f40e2cbb21d3..2703baa170fa 100644 > --- a/include/media/v4l2-ctrls.h > +++ b/include/media/v4l2-ctrls.h > @@ -1254,6 +1254,22 @@ int v4l2_ctrl_request_setup(struct media_request *req, > void v4l2_ctrl_request_complete(struct media_request *req, > struct v4l2_ctrl_handler *parent); > > +/** > + * v4l2_ctrl_request_create - Create a control handler request object > + * > + * @req: The request > + * @parent: The parent control handler ('priv' in media_request_object_find()) > + * > + * If the user created a request without controls, but the driver wants to > + * set controls for the request, then this function can be called in the > + * request's req_validate function. If there is no control object in the > + * request, then this will create one. Now the driver can set controls > + * and when v4l2_ctrl_request_complete() is called they will be automatically > + * copied into the request. > + */ > +int v4l2_ctrl_request_create(struct media_request *req, > + struct v4l2_ctrl_handler *parent); > + > /** > * v4l2_ctrl_request_hdl_find - Find the control handler in the request > * > -- regards, Stan ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH 1/2] v4l2-ctrls.c: add v4l2_ctrl_request_create 2020-08-18 11:30 ` Stanimir Varbanov @ 2020-08-18 11:39 ` Hans Verkuil 0 siblings, 0 replies; 5+ messages in thread From: Hans Verkuil @ 2020-08-18 11:39 UTC (permalink / raw) To: Stanimir Varbanov, linux-media; +Cc: Yunfei Dong On 18/08/2020 13:30, Stanimir Varbanov wrote: > Hi Hans, > > On 7/28/20 12:48 PM, Hans Verkuil wrote: >> Add a new v4l2_ctrl_request_create() function that can be called in >> req_validate() to create a control request object if needed. >> >> This is needed if the driver needs to set controls in the request, >> >> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> >> --- >> drivers/media/mc/mc-request.c | 3 ++- >> drivers/media/v4l2-core/v4l2-ctrls.c | 35 ++++++++++++++++++++++++++++ >> include/media/v4l2-ctrls.h | 16 +++++++++++++ >> 3 files changed, 53 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/media/mc/mc-request.c b/drivers/media/mc/mc-request.c >> index c0782fd96c59..64df83c6f5e5 100644 >> --- a/drivers/media/mc/mc-request.c >> +++ b/drivers/media/mc/mc-request.c >> @@ -414,7 +414,8 @@ int media_request_object_bind(struct media_request *req, >> >> spin_lock_irqsave(&req->lock, flags); >> >> - if (WARN_ON(req->state != MEDIA_REQUEST_STATE_UPDATING)) >> + if (WARN_ON(req->state != MEDIA_REQUEST_STATE_UPDATING && >> + req->state != MEDIA_REQUEST_STATE_VALIDATING)) >> goto unlock; >> >> obj->req = req; >> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c >> index 3f3fbcd60cc6..0d4c8551ba2a 100644 >> --- a/drivers/media/v4l2-core/v4l2-ctrls.c >> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c >> @@ -4345,6 +4345,41 @@ void v4l2_ctrl_request_complete(struct media_request *req, >> } >> EXPORT_SYMBOL(v4l2_ctrl_request_complete); >> >> +int v4l2_ctrl_request_create(struct media_request *req, >> + struct v4l2_ctrl_handler *main_hdl) >> +{ >> + struct media_request_object *obj; >> + struct v4l2_ctrl_handler *new_hdl; >> + int ret = 0; >> + >> + if (!req || !main_hdl) >> + return 0; >> + >> + if (WARN_ON(req->state != MEDIA_REQUEST_STATE_VALIDATING)) >> + return -EBUSY; >> + >> + /* If a request object is found, then do nothing. */ >> + obj = media_request_object_find(req, &req_ops, main_hdl); >> + if (obj) { >> + media_request_object_put(obj); >> + return 0; >> + } >> + >> + /* Create a new request so the driver can return controls */ >> + new_hdl = kzalloc(sizeof(*new_hdl), GFP_KERNEL); >> + if (!new_hdl) >> + return -ENOMEM; >> + >> + obj = &new_hdl->req_obj; > > Why initialize 'obj' and then not use it? Did you forget something Oops, spurious line, should be removed. Thanks for noticing! Hans > >> + ret = v4l2_ctrl_handler_init(new_hdl, (main_hdl->nr_of_buckets - 1) * 8); >> + if (!ret) >> + ret = v4l2_ctrl_request_bind(req, new_hdl, main_hdl); >> + if (ret) >> + kfree(new_hdl); >> + return ret; >> +} >> +EXPORT_SYMBOL(v4l2_ctrl_request_create); >> + >> int v4l2_ctrl_request_setup(struct media_request *req, >> struct v4l2_ctrl_handler *main_hdl) >> { >> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h >> index f40e2cbb21d3..2703baa170fa 100644 >> --- a/include/media/v4l2-ctrls.h >> +++ b/include/media/v4l2-ctrls.h >> @@ -1254,6 +1254,22 @@ int v4l2_ctrl_request_setup(struct media_request *req, >> void v4l2_ctrl_request_complete(struct media_request *req, >> struct v4l2_ctrl_handler *parent); >> >> +/** >> + * v4l2_ctrl_request_create - Create a control handler request object >> + * >> + * @req: The request >> + * @parent: The parent control handler ('priv' in media_request_object_find()) >> + * >> + * If the user created a request without controls, but the driver wants to >> + * set controls for the request, then this function can be called in the >> + * request's req_validate function. If there is no control object in the >> + * request, then this will create one. Now the driver can set controls >> + * and when v4l2_ctrl_request_complete() is called they will be automatically >> + * copied into the request. >> + */ >> +int v4l2_ctrl_request_create(struct media_request *req, >> + struct v4l2_ctrl_handler *parent); >> + >> /** >> * v4l2_ctrl_request_hdl_find - Find the control handler in the request >> * >> > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [RFC PATCH 2/2] vivid: call v4l2_ctrl_request_create() 2020-07-28 9:48 [RFC PATCH 0/2] Add v4l2_ctrl_request_create() Hans Verkuil 2020-07-28 9:48 ` [RFC PATCH 1/2] v4l2-ctrls.c: add v4l2_ctrl_request_create Hans Verkuil @ 2020-07-28 9:48 ` Hans Verkuil 1 sibling, 0 replies; 5+ messages in thread From: Hans Verkuil @ 2020-07-28 9:48 UTC (permalink / raw) To: linux-media; +Cc: Yunfei Dong, Hans Verkuil Call v4l2_ctrl_request_create() from req_validate() to create the control request object if needed. Without this the returned request object would not have a copy of the controls used for the captured frame. Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> --- drivers/media/test-drivers/vivid/vivid-core.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/media/test-drivers/vivid/vivid-core.c b/drivers/media/test-drivers/vivid/vivid-core.c index f7ee37e9508d..59e155a8765e 100644 --- a/drivers/media/test-drivers/vivid/vivid-core.c +++ b/drivers/media/test-drivers/vivid/vivid-core.c @@ -783,7 +783,11 @@ static void vivid_dev_release(struct v4l2_device *v4l2_dev) static int vivid_req_validate(struct media_request *req) { struct vivid_dev *dev = container_of(req->mdev, struct vivid_dev, mdev); + int ret; + ret = v4l2_ctrl_request_create(req, &dev->ctrl_hdl_vid_cap); + if (ret) + return ret; if (dev->req_validate_error) { dev->req_validate_error = false; return -EINVAL; -- 2.27.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-18 11:40 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-28 9:48 [RFC PATCH 0/2] Add v4l2_ctrl_request_create() Hans Verkuil 2020-07-28 9:48 ` [RFC PATCH 1/2] v4l2-ctrls.c: add v4l2_ctrl_request_create Hans Verkuil 2020-08-18 11:30 ` Stanimir Varbanov 2020-08-18 11:39 ` Hans Verkuil 2020-07-28 9:48 ` [RFC PATCH 2/2] vivid: call v4l2_ctrl_request_create() 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).