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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2DA0BC54EBE for ; Sun, 8 Jan 2023 21:13:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233580AbjAHVNS (ORCPT ); Sun, 8 Jan 2023 16:13:18 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60476 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231410AbjAHVMv (ORCPT ); Sun, 8 Jan 2023 16:12:51 -0500 Received: from mail-lj1-x22b.google.com (mail-lj1-x22b.google.com [IPv6:2a00:1450:4864:20::22b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1A1D96561 for ; Sun, 8 Jan 2023 13:12:48 -0800 (PST) Received: by mail-lj1-x22b.google.com with SMTP id s22so7021234ljp.5 for ; Sun, 08 Jan 2023 13:12:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vanguardiasur-com-ar.20210112.gappssmtp.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=x53uwOLKbwK81PFlMs31vANCaDhh1Zsvs/a9zuGpPQk=; b=0PDk1gKy7Bf3a6EwyRvCAxmVMnlOtuNpFYpwSfF8y6YltxiIQb7a28VlfLbdiYUi6Z 8MUnG9BR3yVVQpLQU8XqLEmftHA2d1Gw5X/S61ZSMcY9nerIvO6sUXlrqj5+Ry8uK6s4 9+cxshzjPhDQxmL8A46alekPNVEs72Y5+5WPOa/B1jrup3Id5EloKkFXO0A1+jXx9HcN 4DK1RRwIkfvPYHrFAU2H0zcozt4Orl+qAk871SXv+SFwN217D4TvTQj04jycZig2r0f2 EQxBzqR9uic9UGoshpdACfObFi9jPenr8z/7THbGN2Te4ymD3Z1yCRA1igE8Ygf8s6Xr +M/g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=x53uwOLKbwK81PFlMs31vANCaDhh1Zsvs/a9zuGpPQk=; b=PmtwG41m/7BITg8swsiBUod4ckMF7o3Y9CLC0Gzt7A0EfSyFW0qfL3vbtlZ5xw4YeY uARUA1iFu16Y7FZkXs4rlkk4XHoC/WB5ku8mKm4v7QgdsSAJy0uydCXT9AL0SRpP6Z+v Vul/RyEvpYHgjETZ9y2qtjGs29dJLZl2DoqV8fjxgmkIo0BkkJMokx8amYmxDuoRWmPe AemHLQdNGcHyPbgHJwWK3BoF0GvADq9bF3DjDVT2GhUf0wpwrpKMKWHkT2JNb6spkJDk qngBtj2mmVHHaAe/9LET1CkljyDvlxMcyvnaUDkjYikrTaZ9TyVl1nSZtF4PcIkB0RO5 gdzQ== X-Gm-Message-State: AFqh2kqoroGhArp5FjjGMtIvW8BFLUj9JLztipRlcvSFTfFshmDh5O9s rzPEqiTSsm/hlsLopjlboeMnApDYdfco9A8hvqqW6A== X-Google-Smtp-Source: AMrXdXssLooCUB3JcSjFGQGlVuCaFAVGO6fwtMCDpc/mUlglB++tww+r2xlSFr4vPtbjYEwsLwWpTCjfAlStv55FVrU= X-Received: by 2002:a2e:90d2:0:b0:27f:f22d:afe9 with SMTP id o18-20020a2e90d2000000b0027ff22dafe9mr1990761ljg.404.1673212366368; Sun, 08 Jan 2023 13:12:46 -0800 (PST) MIME-Version: 1.0 References: <20230103170058.810597-1-benjamin.gaignard@collabora.com> <20230103170058.810597-14-benjamin.gaignard@collabora.com> In-Reply-To: <20230103170058.810597-14-benjamin.gaignard@collabora.com> From: Ezequiel Garcia Date: Sun, 8 Jan 2023 18:12:34 -0300 Message-ID: Subject: Re: [PATCH v2 13/13] media: verisilicon: Conditionnaly ignore native formats To: Benjamin Gaignard Cc: p.zabel@pengutronix.de, mchehab@kernel.org, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, heiko@sntech.de, daniel.almeida@collabora.com, nicolas.dufresne@collabora.co.uk, linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, kernel@collabora.com Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On Tue, Jan 3, 2023 at 2:01 PM Benjamin Gaignard wrote: > > AV1 film grain feature requires to use the postprocessor to produce > valid frames. In such case the driver shouldn't propose native pixels > format but only post-processed pixels format. > If a codec set need_postproc field in hantro_ctx structure to true > native pixel formats will be ignored. > > Signed-off-by: Benjamin Gaignard > --- > drivers/media/platform/verisilicon/hantro.h | 3 ++ > .../media/platform/verisilicon/hantro_drv.c | 5 ++ > .../platform/verisilicon/hantro_postproc.c | 4 ++ > .../media/platform/verisilicon/hantro_v4l2.c | 46 +++++++++++++------ > 4 files changed, 45 insertions(+), 13 deletions(-) > > diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h > index a98cb40a8d3b..7a5357e810fb 100644 > --- a/drivers/media/platform/verisilicon/hantro.h > +++ b/drivers/media/platform/verisilicon/hantro.h > @@ -231,6 +231,8 @@ struct hantro_dev { > * @ctrl_handler: Control handler used to register controls. > * @jpeg_quality: User-specified JPEG compression quality. > * @bit_depth: Bit depth of current frame > + * @need_postproc: Set to true if the bitstream features require to > + * use the post-processor. > * > * @codec_ops: Set of operations related to codec mode. > * @postproc: Post-processing context. > @@ -258,6 +260,7 @@ struct hantro_ctx { > struct v4l2_ctrl_handler ctrl_handler; > int jpeg_quality; > int bit_depth; > + bool need_postproc; > > const struct hantro_codec_ops *codec_ops; > struct hantro_postproc_ctx postproc; > diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c > index 4fc6dea16ae6..8d7055c0bf3b 100644 > --- a/drivers/media/platform/verisilicon/hantro_drv.c > +++ b/drivers/media/platform/verisilicon/hantro_drv.c > @@ -346,6 +346,11 @@ static int hantro_av1_s_ctrl(struct v4l2_ctrl *ctrl) > return -EINVAL; > > ctx->bit_depth = bit_depth; > + > + if (ctrl->p_new.p_av1_sequence->flags > + & V4L2_AV1_SEQUENCE_FLAG_FILM_GRAIN_PARAMS_PRESENT) > + ctx->need_postproc = true; > + > break; > default: > return -EINVAL; > diff --git a/drivers/media/platform/verisilicon/hantro_postproc.c b/drivers/media/platform/verisilicon/hantro_postproc.c > index 7dc39519a2ee..293e5612e2ce 100644 > --- a/drivers/media/platform/verisilicon/hantro_postproc.c > +++ b/drivers/media/platform/verisilicon/hantro_postproc.c > @@ -57,6 +57,10 @@ bool hantro_needs_postproc(const struct hantro_ctx *ctx, > { > if (ctx->is_encoder) > return false; > + > + if (ctx->need_postproc) > + return true; > + > return fmt->postprocessed; > } > > diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c > index bbe79dbd2cd9..5c381766cca3 100644 > --- a/drivers/media/platform/verisilicon/hantro_v4l2.c > +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c > @@ -38,6 +38,11 @@ hantro_get_formats(const struct hantro_ctx *ctx, unsigned int *num_fmts) > { > const struct hantro_fmt *formats; > > + if (ctx->need_postproc) { > + *num_fmts = 0; > + return NULL; > + } > + > if (ctx->is_encoder) { > formats = ctx->dev->variant->enc_fmts; > *num_fmts = ctx->dev->variant->num_enc_fmts; > @@ -132,6 +137,15 @@ hantro_get_default_fmt(const struct hantro_ctx *ctx, bool bitstream) > hantro_check_depth_match(ctx, &formats[i])) > return &formats[i]; > } > + > + formats = hantro_get_postproc_formats(ctx, &num_fmts); > + for (i = 0; i < num_fmts; i++) { > + if (bitstream == (formats[i].codec_mode != > + HANTRO_MODE_NONE) && > + hantro_check_depth_match(ctx, &formats[i])) > + return &formats[i]; > + } > + > return NULL; > } > > @@ -261,19 +275,6 @@ static int vidioc_g_fmt_out_mplane(struct file *file, void *priv, > return 0; > } > > -static int vidioc_g_fmt_cap_mplane(struct file *file, void *priv, > - struct v4l2_format *f) > -{ > - struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp; > - struct hantro_ctx *ctx = fh_to_ctx(priv); > - > - vpu_debug(4, "f->type = %d\n", f->type); > - > - *pix_mp = ctx->dst_fmt; > - > - return 0; > -} > - > static int hantro_try_fmt(const struct hantro_ctx *ctx, > struct v4l2_pix_format_mplane *pix_mp, > enum v4l2_buf_type type) > @@ -353,6 +354,25 @@ static int hantro_try_fmt(const struct hantro_ctx *ctx, > return 0; > } > > +static int vidioc_g_fmt_cap_mplane(struct file *file, void *priv, > + struct v4l2_format *f) > +{ > + struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp; > + struct hantro_ctx *ctx = fh_to_ctx(priv); > + int ret; > + > + vpu_debug(4, "f->type = %d\n", f->type); > + > + ret = hantro_try_fmt(ctx, pix_mp, f->type); > + if (ret) > + return ret; > + > + ctx->vpu_dst_fmt = hantro_find_format(ctx, pix_mp->pixelformat); > + ctx->dst_fmt = *pix_mp; > + This looks like the g_fmt is setting some state in the context, this looks incorrect. Thanks, Ezequiel