Message ID | 20221102180810.267252-11-jernej.skrabec@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp60306wru; Wed, 2 Nov 2022 11:11:10 -0700 (PDT) X-Google-Smtp-Source: AMsMyM6IA9MrS6nr2r3V7VV5sfJ0p3tAm+v5IFwsDoWwP7H6myi5P4+zU9wp/F5Iqxliho2etiFJ X-Received: by 2002:a65:6a0d:0:b0:470:3fb:64ab with SMTP id m13-20020a656a0d000000b0047003fb64abmr5640960pgu.560.1667412669707; Wed, 02 Nov 2022 11:11:09 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1667412669; cv=none; d=google.com; s=arc-20160816; b=LD7I/LJ9LxYGdGSObn0OvV7xnwD2g4zBjL5neeOVOSqsbFLqL86FTk0lPvL8sbSyQ0 Ea7XW/FmuXOxL1aY3LqRVRYFdQQDHz5fnJMXOctsUaryG2vbozO8pqtrNgwnm+9jzjtO v0mcE6C+kOCzbc5A0JnLgnnDlQmH+8CEEGcpBVGBnDIUYjyTQKtISgZxUua3+ZZ+oN+E p5DEOc6mzFM4xoKmw84YHpYk1Ge0NkpnmuaRN5kc0c3FkwvrJuq+PnzsaWoxbXQFUeJ3 HNSSUvTnu/WGaZheOC8Qf+/eG3lGH9fM7Df3XYFDJD2bwCQvsrKPKKYR8E655R6XVtSp 1V6w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=zTX/CJSb7MB7yBzrjB/XPnpPEfJPqcJy2sRni4DNHhc=; b=F9IrEkMBwQWxwKIiyGNM2jfLnd+pxRbotSH9a+2kll2KXqXE2Umh6AJl58OM6xOEpN sG9JnQscdvQqYIQrVdmIXVMvgtzwIsd7WhwLMc0U/eV1V7tGVHGnuR7RRYoTM0cC+fsM rkrjVJuQ+RfrcHsAgzmPXex+oKf3G/ulDHDs0Zv7ekSK2iN2INOWOym3adaJv5WEip2A lBvmU4mHe+d8qyNj9xnKmFa5JC7VQDkC9Tf+qEV8TDgC20PNyxlYoP4LGDRGH6EBO0sh R+yZ8ho7dLTg9ibAewq+o/5R3LyUiwH+lT78i4LW1GbLDPW9e4EIDt5hCzRxOKsvIs+U daow== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=BWwH0PLp; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id h7-20020a170902ac8700b0017f914799b4si16407432plr.175.2022.11.02.11.10.57; Wed, 02 Nov 2022 11:11:09 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=BWwH0PLp; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231626AbiKBSJb (ORCPT <rfc822;yves.mi.zy@gmail.com> + 99 others); Wed, 2 Nov 2022 14:09:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37812 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231351AbiKBSIc (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 2 Nov 2022 14:08:32 -0400 Received: from mail-ed1-x536.google.com (mail-ed1-x536.google.com [IPv6:2a00:1450:4864:20::536]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 48CB9E087; Wed, 2 Nov 2022 11:08:30 -0700 (PDT) Received: by mail-ed1-x536.google.com with SMTP id f7so22045628edc.6; Wed, 02 Nov 2022 11:08:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=zTX/CJSb7MB7yBzrjB/XPnpPEfJPqcJy2sRni4DNHhc=; b=BWwH0PLp6JQ9eYWdTML13I1KDHGFTpJL2HjwEIHw81SznXWlFHTHSGWOlc7rSeX503 9wqgaJhi0gATwz9JUOCOVIlEvlOcAAxSSw4jpkKHc1XQj8MYdzptxvQQa10K74lJnjKO jYsqyFNonzO42fSV0aDTHyZcLt48Xwneh3+krvC2SQV75tXrTQ6zEY8ieSrftyVNXarC FxZCPMocPoPCtGFhYjDdZjDakeFDyxbJUOA5ueFmLMmXM7e5+tsZ8JllrRhSr7gjS0Tj QXKdq4b+3GH/aCvfMgDMokftvmjIQP3LAxTJP355QlPNg/lpS8lip99Qg0Ha4m7/3OA1 daRw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=zTX/CJSb7MB7yBzrjB/XPnpPEfJPqcJy2sRni4DNHhc=; b=lqSTp2lxC4v8RpL9dswpddQguHcIva2fa9pGM+t/4V5j+TBpCYMd1W1zbahApcvGyp qr2I7WDVFVelnE0WI77lM4vC2KxZjkYG9/MAbUSTD8GvNtMBYX05Hy552kjQVmI5hE4H t3OaggQHwxDTsanL15QSzgtKSjo++ba4rf9VaGpAVaIT+CBct8RwPIF6w8BW9YYIgFSU UqsTM0KpW6BZ4pgq7AnWRWUCWMMxWx1Y52+mpKJPf5QpnEheCWTfPibmy98aXwRn8Ikr usJhO2YTC70JYfsVksW08NH94KnkIodo70ruBNG9rl9HH3tfQ2plQadLl7QEnzmh9KUr LH6g== X-Gm-Message-State: ACrzQf2iSMm2z284WjB0TIJxNfr5bqzwouRwfkryKbUSxXNc89clxFqo rHRf5ZgqX8y/CQyMU41xk2I= X-Received: by 2002:a05:6402:528a:b0:454:8613:6560 with SMTP id en10-20020a056402528a00b0045486136560mr26656883edb.252.1667412508810; Wed, 02 Nov 2022 11:08:28 -0700 (PDT) Received: from kista.localdomain (82-149-19-102.dynamic.telemach.net. [82.149.19.102]) by smtp.gmail.com with ESMTPSA id gy8-20020a170906f24800b007317f017e64sm5677460ejb.134.2022.11.02.11.08.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 02 Nov 2022 11:08:28 -0700 (PDT) From: Jernej Skrabec <jernej.skrabec@gmail.com> To: mripard@kernel.org, paul.kocialkowski@bootlin.com Cc: mchehab@kernel.org, gregkh@linuxfoundation.org, wens@csie.org, samuel@sholland.org, hverkuil-cisco@xs4all.nl, linux-media@vger.kernel.org, linux-staging@lists.linux.dev, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org, Jernej Skrabec <jernej.skrabec@gmail.com> Subject: [PATCH v2 10/11] media: cedrus: Adjust buffer size based on control values Date: Wed, 2 Nov 2022 19:08:09 +0100 Message-Id: <20221102180810.267252-11-jernej.skrabec@gmail.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221102180810.267252-1-jernej.skrabec@gmail.com> References: <20221102180810.267252-1-jernej.skrabec@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1748408907575779336?= X-GMAIL-MSGID: =?utf-8?q?1748408907575779336?= |
Series |
media: cedrus: Format handling improvements and 10-bit HEVC support
|
|
Commit Message
Jernej Škrabec
Nov. 2, 2022, 6:08 p.m. UTC
In some cases decoding engine needs extra space in capture buffers. This
is the case for decoding 10-bit HEVC frames into 8-bit capture format.
This commit only adds infrastructure for such cases.
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
drivers/staging/media/sunxi/cedrus/cedrus.c | 14 ++++++++++++++
drivers/staging/media/sunxi/cedrus/cedrus.h | 2 ++
drivers/staging/media/sunxi/cedrus/cedrus_video.c | 4 ++++
3 files changed, 20 insertions(+)
Comments
Hi Jernej, On 02/11/2022 19:08, Jernej Skrabec wrote: > In some cases decoding engine needs extra space in capture buffers. This > is the case for decoding 10-bit HEVC frames into 8-bit capture format. > This commit only adds infrastructure for such cases. > > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com> > --- > drivers/staging/media/sunxi/cedrus/cedrus.c | 14 ++++++++++++++ > drivers/staging/media/sunxi/cedrus/cedrus.h | 2 ++ > drivers/staging/media/sunxi/cedrus/cedrus_video.c | 4 ++++ > 3 files changed, 20 insertions(+) > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c > index 6a2c08928613..c36999e47591 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus.c > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c > @@ -68,8 +68,22 @@ static int cedrus_try_ctrl(struct v4l2_ctrl *ctrl) > return 0; > } > > +static int cedrus_s_ctrl(struct v4l2_ctrl *ctrl) > +{ > + struct cedrus_ctx *ctx = container_of(ctrl->handler, > + struct cedrus_ctx, hdl); > + struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, > + V4L2_BUF_TYPE_VIDEO_CAPTURE); > + > + if (!vb2_is_busy(vq) && !vb2_is_streaming(vq)) You can drop the !vb2_is_streaming part. If you are streaming, then it will also be 'busy'. > + cedrus_reset_cap_format(ctx); This is weird. I would expect that this is only called for specific controls, since presumably not all will change the sizeimage value. It looks like it applies only to V4L2_CID_STATELESS_HEVC_SPS, so I see no reason why you would call cedrus_reset_cap_format() for other controls as well. And what happens if someone makes such a change *while streaming*? Shouldn't cedrus_try_ctrl detect that and fail with -EBUSY in that case? Regardless, this is unexpected behavior, so some explanatory comments would be useful. I'm also not sure whether it isn't better to add cedrus_s_ctrl in the next patch, I think it would be more understandable when seen with an actual control. Regards, Hans > + > + return 0; > +} > + > static const struct v4l2_ctrl_ops cedrus_ctrl_ops = { > .try_ctrl = cedrus_try_ctrl, > + .s_ctrl = cedrus_s_ctrl, > }; > > static const struct cedrus_control cedrus_controls[] = { > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h > index 5904294f3108..774fe8048ce3 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus.h > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h > @@ -162,6 +162,8 @@ struct cedrus_dec_ops { > int (*start)(struct cedrus_ctx *ctx); > void (*stop)(struct cedrus_ctx *ctx); > void (*trigger)(struct cedrus_ctx *ctx); > + unsigned int (*extra_cap_size)(struct cedrus_ctx *ctx, > + struct v4l2_pix_format *pix_fmt); > }; > > struct cedrus_variant { > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c > index dec5d3ae4871..dc67940f1ade 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c > @@ -250,6 +250,10 @@ static int cedrus_try_fmt_vid_cap_p(struct cedrus_ctx *ctx, > pix_fmt->height = ctx->src_fmt.height; > cedrus_prepare_format(pix_fmt); > > + if (ctx->current_codec->extra_cap_size) > + pix_fmt->sizeimage += > + ctx->current_codec->extra_cap_size(ctx, pix_fmt); > + > return 0; > } >
Dne petek, 04. november 2022 ob 16:27:29 CET je Hans Verkuil napisal(a): > Hi Jernej, > > On 02/11/2022 19:08, Jernej Skrabec wrote: > > In some cases decoding engine needs extra space in capture buffers. This > > is the case for decoding 10-bit HEVC frames into 8-bit capture format. > > This commit only adds infrastructure for such cases. > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com> > > --- > > > > drivers/staging/media/sunxi/cedrus/cedrus.c | 14 ++++++++++++++ > > drivers/staging/media/sunxi/cedrus/cedrus.h | 2 ++ > > drivers/staging/media/sunxi/cedrus/cedrus_video.c | 4 ++++ > > 3 files changed, 20 insertions(+) > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c > > b/drivers/staging/media/sunxi/cedrus/cedrus.c index > > 6a2c08928613..c36999e47591 100644 > > --- a/drivers/staging/media/sunxi/cedrus/cedrus.c > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c > > @@ -68,8 +68,22 @@ static int cedrus_try_ctrl(struct v4l2_ctrl *ctrl) > > > > return 0; > > > > } > > > > +static int cedrus_s_ctrl(struct v4l2_ctrl *ctrl) > > +{ > > + struct cedrus_ctx *ctx = container_of(ctrl->handler, > > + struct cedrus_ctx, hdl); > > + struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, > > + V4L2_BUF_TYPE_VIDEO_CAPTURE); > > + > > + if (!vb2_is_busy(vq) && !vb2_is_streaming(vq)) > > You can drop the !vb2_is_streaming part. If you are streaming, then > it will also be 'busy'. > > > + cedrus_reset_cap_format(ctx); > > This is weird. I would expect that this is only called for specific > controls, since presumably not all will change the sizeimage value. It > looks like it applies only to V4L2_CID_STATELESS_HEVC_SPS, so I see no > reason why you would call cedrus_reset_cap_format() for other controls as > well. It's more general approach. It's harmless for other cases, still within specs. Anyway, I can make it more specific. > > And what happens if someone makes such a change *while streaming*? > Shouldn't cedrus_try_ctrl detect that and fail with -EBUSY in that case? SPS can also be changed during streaming, so we can't outright deny setting it during streaming. Also, if it changes from 10 to 8, that also works, since capture buffers are still big enough. Only change from 8 to 10 during streaming won't work and that check can be added. > > Regardless, this is unexpected behavior, so some explanatory comments > would be useful. > > I'm also not sure whether it isn't better to add cedrus_s_ctrl in the next > patch, I think it would be more understandable when seen with an actual > control. I guess I can move logic to cedrus_try_ctrl(), where some of this logic already exists and it would be just a little bit expanded and more selective. Best regards, Jernej > > Regards, > > Hans > > > + > > + return 0; > > +} > > + > > > > static const struct v4l2_ctrl_ops cedrus_ctrl_ops = { > > > > .try_ctrl = cedrus_try_ctrl, > > > > + .s_ctrl = cedrus_s_ctrl, > > > > }; > > > > static const struct cedrus_control cedrus_controls[] = { > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h > > b/drivers/staging/media/sunxi/cedrus/cedrus.h index > > 5904294f3108..774fe8048ce3 100644 > > --- a/drivers/staging/media/sunxi/cedrus/cedrus.h > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h > > @@ -162,6 +162,8 @@ struct cedrus_dec_ops { > > > > int (*start)(struct cedrus_ctx *ctx); > > void (*stop)(struct cedrus_ctx *ctx); > > void (*trigger)(struct cedrus_ctx *ctx); > > > > + unsigned int (*extra_cap_size)(struct cedrus_ctx *ctx, > > + struct v4l2_pix_format *pix_fmt); > > > > }; > > > > struct cedrus_variant { > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c > > b/drivers/staging/media/sunxi/cedrus/cedrus_video.c index > > dec5d3ae4871..dc67940f1ade 100644 > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c > > @@ -250,6 +250,10 @@ static int cedrus_try_fmt_vid_cap_p(struct cedrus_ctx > > *ctx,> > > pix_fmt->height = ctx->src_fmt.height; > > cedrus_prepare_format(pix_fmt); > > > > + if (ctx->current_codec->extra_cap_size) > > + pix_fmt->sizeimage += > > + ctx->current_codec->extra_cap_size(ctx, pix_fmt); > > + > > > > return 0; > > > > }
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c index 6a2c08928613..c36999e47591 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c @@ -68,8 +68,22 @@ static int cedrus_try_ctrl(struct v4l2_ctrl *ctrl) return 0; } +static int cedrus_s_ctrl(struct v4l2_ctrl *ctrl) +{ + struct cedrus_ctx *ctx = container_of(ctrl->handler, + struct cedrus_ctx, hdl); + struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, + V4L2_BUF_TYPE_VIDEO_CAPTURE); + + if (!vb2_is_busy(vq) && !vb2_is_streaming(vq)) + cedrus_reset_cap_format(ctx); + + return 0; +} + static const struct v4l2_ctrl_ops cedrus_ctrl_ops = { .try_ctrl = cedrus_try_ctrl, + .s_ctrl = cedrus_s_ctrl, }; static const struct cedrus_control cedrus_controls[] = { diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h index 5904294f3108..774fe8048ce3 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus.h +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h @@ -162,6 +162,8 @@ struct cedrus_dec_ops { int (*start)(struct cedrus_ctx *ctx); void (*stop)(struct cedrus_ctx *ctx); void (*trigger)(struct cedrus_ctx *ctx); + unsigned int (*extra_cap_size)(struct cedrus_ctx *ctx, + struct v4l2_pix_format *pix_fmt); }; struct cedrus_variant { diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c index dec5d3ae4871..dc67940f1ade 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c @@ -250,6 +250,10 @@ static int cedrus_try_fmt_vid_cap_p(struct cedrus_ctx *ctx, pix_fmt->height = ctx->src_fmt.height; cedrus_prepare_format(pix_fmt); + if (ctx->current_codec->extra_cap_size) + pix_fmt->sizeimage += + ctx->current_codec->extra_cap_size(ctx, pix_fmt); + return 0; }