From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 79995C433FE for ; Fri, 4 Nov 2022 18:12:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=lpdFAfQBHoP5zgcV0lsH5qrskfcsHwa3BOZcxkfxkpQ=; b=ht/Z2A4Q/Re6yb tornXvvMU51SjGvTggTqCXCWduJgP+vI8emOT7aNR0XjsjR1ZzWrrhFKqXFDWyGc+qXM0Mr7A7rFP y/Zt/SZD3nKVs42JIvIhQaJGTgFqkLWLu/82ya2rxfqnRrMbsslWSelIz697M0wdlPhWzO757kAKX W+LKxgJZec4iEQIWeU6i9YmqcVq7pvgB1vbgbXbBMroO4QdXBEmm4rdnBzeNHR9bKwRFXuSxih3W5 c9rrUVyzzqHZ7A+hCmdmL9SEXUkAfZaAl9yJII7+encZi/DWi/+8GyKorxl9EPMbOhxs9Ra/sRTNe nr7KoFm9r0aI2sHTiWwg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1or19f-004kQm-9f; Fri, 04 Nov 2022 18:10:59 +0000 Received: from mail-ej1-x62f.google.com ([2a00:1450:4864:20::62f]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1or19c-004kQM-7w for linux-arm-kernel@lists.infradead.org; Fri, 04 Nov 2022 18:10:57 +0000 Received: by mail-ej1-x62f.google.com with SMTP id sc25so15270606ejc.12 for ; Fri, 04 Nov 2022 11:10:55 -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=zI5Rfzsxc8rNV7iJcfSyH9aK8iAlVJHWFV6jyxPEIok=; b=hso72QBPITUbbmDTleVSZzhKAzH1N/pTXymcIBO8s2I23+psuEJGPHJfZwz4jtvXhV iwYCaHDYdJe7kYiqgKRRga4szZW9IfpF9avjaBsJzHIyASoMf7l+zl3FpjWtpf9Flr+Y 0gMNBn/Jp4thIlhAt3qoSsgv0o0a5fUJSgUUwXMDaLih8ESneA6S784vbCEGsa4hcU1V FsXmcwZnXQVymAWTHDNppgdrNGcZQtfKjy3C0jn5wnLb7MIU2qDC8zIkh4bDkSP4E4IJ wIbR0PqzUGD+iEC6CvUKfuJ0Uvun6UAYkcUQ/bjE0BVbgIOZzsOlKRG/B1JT0Xplpfkn g+Rw== 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=zI5Rfzsxc8rNV7iJcfSyH9aK8iAlVJHWFV6jyxPEIok=; b=dnfFg6/2e6RvvzgtbIQt0HKBMfwI/Il5qf13AHmprtHUu5KfgPSVvHB7X9lpk5tmGH SSYBgAHJVf2Or9beniAySmuzCYs7OCtcvmeCtB06g+/QXgzENEt5F/ukR9HdJabp3n+7 QWV9bCoQ6JzXRDE3K7rKLCF59gNtfMpgG3AMlDbcyzor+dm9damnztoe8OZVYcmY0DZF ece8hfx7tdeMIJ4CToPMAlaXScCU7/yQyAjXCR47L5nhvn4yI39zgecNVp/krRd4EdtB kBQ6+ShfgkrcdDupx5dgOKKDOA/AhBDz3IPWM6i31+LOToFsT9OGnq5Wv6/kBFzofvmR U+CA== X-Gm-Message-State: ACrzQf0i9NJPmlydVeftuhnDEoK6Q2otpPQqd/Wtz17kxov9amiycuRM JtbUb8fBxlfyVZsfm1pFybY= X-Google-Smtp-Source: AMsMyM7thWyBYHE/ivRxLw9DZwILGgEJyeA4LgCqmhP078HXispIbQGy34uZRIqCeIXpq/OAQAQxrA== X-Received: by 2002:a17:906:fe44:b0:77c:e313:a8e8 with SMTP id wz4-20020a170906fe4400b0077ce313a8e8mr36352481ejb.700.1667585454095; Fri, 04 Nov 2022 11:10:54 -0700 (PDT) Received: from jernej-laptop.localnet (89-212-118-115.static.t-2.net. [89.212.118.115]) by smtp.gmail.com with ESMTPSA id gh30-20020a1709073c1e00b007919ba4295esm2086563ejc.216.2022.11.04.11.10.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 04 Nov 2022 11:10:53 -0700 (PDT) From: Jernej =?utf-8?B?xaBrcmFiZWM=?= To: mripard@kernel.org, paul.kocialkowski@bootlin.com, Hans Verkuil Cc: mchehab@kernel.org, gregkh@linuxfoundation.org, wens@csie.org, samuel@sholland.org, 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 Subject: Re: [PATCH v2 10/11] media: cedrus: Adjust buffer size based on control values Date: Fri, 04 Nov 2022 19:10:52 +0100 Message-ID: <1908973.PYKUYFuaPT@jernej-laptop> In-Reply-To: <76c3fc1b-e90f-748c-39ee-74be93fd6811@xs4all.nl> References: <20221102180810.267252-1-jernej.skrabec@gmail.com> <20221102180810.267252-11-jernej.skrabec@gmail.com> <76c3fc1b-e90f-748c-39ee-74be93fd6811@xs4all.nl> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221104_111056_334439_AD84E1F1 X-CRM114-Status: GOOD ( 34.21 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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 > > --- > > > > 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; > > > > } _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel