* [PATCH] rkvdec: Do not require all controls to be present in every request
@ 2021-03-23 18:57 daniel.almeida
2021-03-26 16:13 ` Ezequiel Garcia
0 siblings, 1 reply; 2+ messages in thread
From: daniel.almeida @ 2021-03-23 18:57 UTC (permalink / raw)
To: ezequiel
Cc: kernel, linux-media, hverkuil-cisco, nicolas.dufresne, Daniel Almeida
From: Daniel Almeida <daniel.almeida@collabora.com>
According to the v4l2 api, it is allowed to skip
setting a control if its contents haven't changed for performance
reasons: userspace should only update the controls that changed from
last frame rather then updating them all. Still some ancient code
that checks for mandatory controls has been left in this driver.
Remove it.
Fixes: cd33c830448b ("media: rkvdec: Add the rkvdec driver")
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
drivers/staging/media/rkvdec/rkvdec.c | 48 +--------------------------
drivers/staging/media/rkvdec/rkvdec.h | 1 -
2 files changed, 1 insertion(+), 48 deletions(-)
diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
index d3eb81ee8dc2..5f0219d117fb 100644
--- a/drivers/staging/media/rkvdec/rkvdec.c
+++ b/drivers/staging/media/rkvdec/rkvdec.c
@@ -55,16 +55,13 @@ static const struct v4l2_ctrl_ops rkvdec_ctrl_ops = {
static const struct rkvdec_ctrl_desc rkvdec_h264_ctrl_descs[] = {
{
- .mandatory = true,
.cfg.id = V4L2_CID_STATELESS_H264_DECODE_PARAMS,
},
{
- .mandatory = true,
.cfg.id = V4L2_CID_STATELESS_H264_SPS,
.cfg.ops = &rkvdec_ctrl_ops,
},
{
- .mandatory = true,
.cfg.id = V4L2_CID_STATELESS_H264_PPS,
},
{
@@ -585,25 +582,7 @@ static const struct vb2_ops rkvdec_queue_ops = {
static int rkvdec_request_validate(struct media_request *req)
{
- struct media_request_object *obj;
- const struct rkvdec_ctrls *ctrls;
- struct v4l2_ctrl_handler *hdl;
- struct rkvdec_ctx *ctx = NULL;
- unsigned int count, i;
- int ret;
-
- list_for_each_entry(obj, &req->objects, list) {
- if (vb2_request_object_is_buffer(obj)) {
- struct vb2_buffer *vb;
-
- vb = container_of(obj, struct vb2_buffer, req_obj);
- ctx = vb2_get_drv_priv(vb->vb2_queue);
- break;
- }
- }
-
- if (!ctx)
- return -EINVAL;
+ unsigned int count;
count = vb2_request_buffer_cnt(req);
if (!count)
@@ -611,31 +590,6 @@ static int rkvdec_request_validate(struct media_request *req)
else if (count > 1)
return -EINVAL;
- hdl = v4l2_ctrl_request_hdl_find(req, &ctx->ctrl_hdl);
- if (!hdl)
- return -ENOENT;
-
- ret = 0;
- ctrls = ctx->coded_fmt_desc->ctrls;
- for (i = 0; ctrls && i < ctrls->num_ctrls; i++) {
- u32 id = ctrls->ctrls[i].cfg.id;
- struct v4l2_ctrl *ctrl;
-
- if (!ctrls->ctrls[i].mandatory)
- continue;
-
- ctrl = v4l2_ctrl_request_hdl_ctrl_find(hdl, id);
- if (!ctrl) {
- ret = -ENOENT;
- break;
- }
- }
-
- v4l2_ctrl_request_hdl_put(hdl);
-
- if (ret)
- return ret;
-
return vb2_request_validate(req);
}
diff --git a/drivers/staging/media/rkvdec/rkvdec.h b/drivers/staging/media/rkvdec/rkvdec.h
index 77a137cca88e..52ac3874c5e5 100644
--- a/drivers/staging/media/rkvdec/rkvdec.h
+++ b/drivers/staging/media/rkvdec/rkvdec.h
@@ -25,7 +25,6 @@
struct rkvdec_ctx;
struct rkvdec_ctrl_desc {
- u32 mandatory : 1;
struct v4l2_ctrl_config cfg;
};
--
2.31.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] rkvdec: Do not require all controls to be present in every request
2021-03-23 18:57 [PATCH] rkvdec: Do not require all controls to be present in every request daniel.almeida
@ 2021-03-26 16:13 ` Ezequiel Garcia
0 siblings, 0 replies; 2+ messages in thread
From: Ezequiel Garcia @ 2021-03-26 16:13 UTC (permalink / raw)
To: daniel.almeida; +Cc: kernel, linux-media, hverkuil-cisco, nicolas.dufresne
On Tue, 2021-03-23 at 15:57 -0300, daniel.almeida@collabora.com wrote:
> From: Daniel Almeida <daniel.almeida@collabora.com>
>
> According to the v4l2 api, it is allowed to skip
> setting a control if its contents haven't changed for performance
> reasons: userspace should only update the controls that changed from
> last frame rather then updating them all. Still some ancient code
> that checks for mandatory controls has been left in this driver.
>
> Remove it.
>
> Fixes: cd33c830448b ("media: rkvdec: Add the rkvdec driver")
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>
Thanks,
Ezequiel
> ---
> drivers/staging/media/rkvdec/rkvdec.c | 48 +--------------------------
> drivers/staging/media/rkvdec/rkvdec.h | 1 -
> 2 files changed, 1 insertion(+), 48 deletions(-)
>
> diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
> index d3eb81ee8dc2..5f0219d117fb 100644
> --- a/drivers/staging/media/rkvdec/rkvdec.c
> +++ b/drivers/staging/media/rkvdec/rkvdec.c
> @@ -55,16 +55,13 @@ static const struct v4l2_ctrl_ops rkvdec_ctrl_ops = {
>
> static const struct rkvdec_ctrl_desc rkvdec_h264_ctrl_descs[] = {
> {
> - .mandatory = true,
> .cfg.id = V4L2_CID_STATELESS_H264_DECODE_PARAMS,
> },
> {
> - .mandatory = true,
> .cfg.id = V4L2_CID_STATELESS_H264_SPS,
> .cfg.ops = &rkvdec_ctrl_ops,
> },
> {
> - .mandatory = true,
> .cfg.id = V4L2_CID_STATELESS_H264_PPS,
> },
> {
> @@ -585,25 +582,7 @@ static const struct vb2_ops rkvdec_queue_ops = {
>
> static int rkvdec_request_validate(struct media_request *req)
> {
> - struct media_request_object *obj;
> - const struct rkvdec_ctrls *ctrls;
> - struct v4l2_ctrl_handler *hdl;
> - struct rkvdec_ctx *ctx = NULL;
> - unsigned int count, i;
> - int ret;
> -
> - list_for_each_entry(obj, &req->objects, list) {
> - if (vb2_request_object_is_buffer(obj)) {
> - struct vb2_buffer *vb;
> -
> - vb = container_of(obj, struct vb2_buffer, req_obj);
> - ctx = vb2_get_drv_priv(vb->vb2_queue);
> - break;
> - }
> - }
> -
> - if (!ctx)
> - return -EINVAL;
> + unsigned int count;
>
> count = vb2_request_buffer_cnt(req);
> if (!count)
> @@ -611,31 +590,6 @@ static int rkvdec_request_validate(struct media_request *req)
> else if (count > 1)
> return -EINVAL;
>
> - hdl = v4l2_ctrl_request_hdl_find(req, &ctx->ctrl_hdl);
> - if (!hdl)
> - return -ENOENT;
> -
> - ret = 0;
> - ctrls = ctx->coded_fmt_desc->ctrls;
> - for (i = 0; ctrls && i < ctrls->num_ctrls; i++) {
> - u32 id = ctrls->ctrls[i].cfg.id;
> - struct v4l2_ctrl *ctrl;
> -
> - if (!ctrls->ctrls[i].mandatory)
> - continue;
> -
> - ctrl = v4l2_ctrl_request_hdl_ctrl_find(hdl, id);
> - if (!ctrl) {
> - ret = -ENOENT;
> - break;
> - }
> - }
> -
> - v4l2_ctrl_request_hdl_put(hdl);
> -
> - if (ret)
> - return ret;
> -
> return vb2_request_validate(req);
> }
>
> diff --git a/drivers/staging/media/rkvdec/rkvdec.h b/drivers/staging/media/rkvdec/rkvdec.h
> index 77a137cca88e..52ac3874c5e5 100644
> --- a/drivers/staging/media/rkvdec/rkvdec.h
> +++ b/drivers/staging/media/rkvdec/rkvdec.h
> @@ -25,7 +25,6 @@
> struct rkvdec_ctx;
>
> struct rkvdec_ctrl_desc {
> - u32 mandatory : 1;
> struct v4l2_ctrl_config cfg;
> };
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-03-26 16:14 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-23 18:57 [PATCH] rkvdec: Do not require all controls to be present in every request daniel.almeida
2021-03-26 16:13 ` Ezequiel Garcia
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).