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 0C44FCCA490 for ; Fri, 16 Oct 2026 16:06:50 +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:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=tXkKN1UtBj7ikOMvxuRtaUEvDsMhDoTeaBvbhqazg0M=; b=CwpcUMIwlyg7mu l+fj/qo5l2QbGmNxWul21ex8tYBTvKHq62ceOpjNqcofcffHM4ISiX5RlwHXpAtVU7syZ9nZqMyIQ gPQKRcI33cl26dVMIkg9lh2cX1ktNz7ngrfsZ+mSoax2jTQ4Tq4KlAm/ItS/nn3MpyXjhbxVMDAjx mYHBthuRHDsmWndGcVBjP32267zuP9cb/iwEgThmwrSSjvSEs8sCm2LDRPCRymHkeEdcH1XSLYjx3 aRXqRQ5oQoAo4vXRmdpxvMkWAESiRlsIk/Uit85V9a84JVVfjKZfrzwCaFIoIojw3SIb5Yz3gtj9D lbnWyr+rsVsaK1TDeY8w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1xHkSC-00000003OEo-22oL; Fri, 16 Oct 2026 16:06:44 +0000 Received: from desiato.infradead.org ([2001:8b0:10b:1:d65d:64ff:fe57:4e05]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1xHkSB-00000003ODY-0ntu; Fri, 16 Oct 2026 16:06:43 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=1GuIjwnutapXL5RSBydFeEzZIlwcykWtwbrgx8uSRFI=; b=pI5eHEriagnnfJmqgmrHf+pvLm x0z6xgRzyIHW/6yrtUFignoAe9Ru4CVAw/xQMp4iaHUycaGw0FfTHU5WM6a21sycCROf9dYjh52XD yUR+u0IT+IKAdWA4CQwHpRt3sAvPWtmA9g/aNskgEO9brx957kT6c5sI4bU4EyRTb973X8706lDUE xLlleizqXBbq3l7KrrFO5zoyMs0ryRQpbc+9uUYOuBrkVffB1mvxOJkyvARFArQHavGvjBXkNvdNx 7e/YY9CrWosNZuQu4uYb/re1BuUjwqEsNazoHUDJwiPJ7YBfVZxtPtBnQmdlgNFrDFLuK00jQAxrr 8ddmTpGw==; Received: from pidgin.makrotopia.org ([2a07:2ec0:3002::65]) by desiato.infradead.org with esmtps (Exim 4.99.5 #2 (Red Hat Linux)) id 1xHkS8-00000001i8G-2Ee3; Fri, 16 Oct 2026 16:06:42 +0000 Received: from local by pidgin.makrotopia.org with esmtpsa (TLS1.3:TLS_AES_256_GCM_SHA384:256:X25519MLKEM768) (Exim 4.100.1) (envelope-from ) id 1xHkS0-0000000086I-3HYE; Fri, 16 Oct 2026 16:06:32 +0000 Date: Fri, 16 Oct 2026 17:06:29 +0100 From: Daniel Golle To: Paul Elder Cc: Mauro Carvalho Chehab , Heiko Stuebner , Dafna Hirschfeld , Laurent Pinchart , Rob Herring , Krzysztof Kozlowski , Conor Dooley , michael.riesch@collabora.com, stefan.klug@ideasonboard.com, mehdi.djait@linux.intel.com, xuhf@rock-chips.com, zyc@rock-chips.com, dalong.deng@rock-chips.com, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, devicetree@vger.kernel.org Subject: Re: [PATCH v3 2/5] media: rkisp2: Add parameters output video node Message-ID: References: <20260827-rkisp2-v3-0-e91ff0a604d6@ideasonboard.com> <20260827-rkisp2-v3-2-e91ff0a604d6@ideasonboard.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20260827-rkisp2-v3-2-e91ff0a604d6@ideasonboard.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.9.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20261016_170640_664279_CA90988C X-CRM114-Status: GOOD ( 27.65 ) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+linux-rockchip=archiver.kernel.org@lists.infradead.org On Thu, Aug 27, 2026 at 07:48:31PM +0900, Paul Elder wrote: > Implement support for setting parameters on the ISP by queueing > parameter buffers to rkisp2. [...] Hi Paul, A few comments from running this on the RV1126; the RK3588 statements are by code inspection. [...] > +++ b/Documentation/userspace-api/media/v4l/metafmt-rkisp2.rst [...] > +The configuration parameters are passed to the :ref:`rkisp2_params > +` metadata output video node, using the There is no rkisp2_params label anywhere under Documentation/ (rkisp1 defines its labels in admin-guide/media/rkisp1.rst), so Sphinx warns about an undefined label. The same holds for rkisp2_stats in patch 3. [...] > +++ b/drivers/media/platform/rockchip/rkisp2/rkisp2-params.c [...] > +static void rkisp2_params_goc(struct rkisp2_params *params, [...] > + if (arg->mode == RKISP2_ISP_GOC_MODE_EQUIDISTANT) > + control = ISP3X_GAMMA_OUT_CTRL_MODE_EQUIDISTANT; > + if (arg->mode == RKISP2_ISP_GOC_SEGMENTS_48) > + control |= ISP3X_GAMMA_OUT_CTRL_SEGMENTS_48; The second test is meant for arg->segments. As RKISP2_ISP_GOC_SEGMENTS_48 and RKISP2_ISP_GOC_MODE_EQUIDISTANT are both 1, equidistant mode always gets the 48 segment layout and the segments member of the uapi is dead. [...] > +static void rkisp2_params_crop_init(struct rkisp2_params *params) > +{ > + struct rkisp2_device *rkisp2 = params->rkisp2; > + > + /* Create an initial crop configuration to configure */ > + struct rkisp2_params_crop arg = { > + .header.type = RKISP2_PARAMS_BLOCK_CROP, > + .header.flags = 0, > + .header.size = sizeof(struct rkisp2_params_crop), > + .crop_en = RKISP2_ISP_CROP_ENABLE_MAIN | RKISP2_ISP_CROP_ENABLE_SELF, > + > + .mp_crop.h_offs = 0, > + .mp_crop.v_offs = 0, > + .mp_crop.h_size = rkisp2->resizer_devs[0].source_size.width, > + .mp_crop.v_size = rkisp2->resizer_devs[0].source_size.height, Without a params buffer at stream start this crops a window of the output size at the top left corner, so a 1920x1080 capture from a 3864x2192 sensor shows that corner only. The sink size (what the CROP block uses when it is disabled), scaled by the resizer, is what rkisp1 does and what a plain v4l2-ctl capture expects. [...] > +static void rkisp2_params_configure(struct rkisp2_params *params, [...] > + /* update shadow register immediately */ > + rkisp2_param_set_bits(params, RKISP2_CIF_ISP_CTRL, > + RKISP2_CIF_ISP_CTRL_ISP_CFG_UPD); > + > + if (prio == RKISP2_PARAMS_CONFIG_PRIO_NONE) > + vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE); > +} [...] > +void rkisp2_params_pre_configure(struct rkisp2_params *params, [...] > + spin_lock_irq(¶ms->buf_lock); > + buf = list_first_entry_or_null(¶ms->params, > + struct rkisp2_params_buffer, queue); > + spin_unlock_irq(¶ms->buf_lock); > + > + if (!buf) { > + /* > + * Initialize crop in the event that are no pre-queued > + * parameter buffers > + */ > + rkisp2_params_crop_init(params); > + return; > + } > + > + rkisp2_params_configure(params, buf, RKISP2_PARAMS_CONFIG_PRIO_PRE); When a buffer is queued but carries no CROP block, neither path programs the dual crop and the resizer, so they keep the state of the previous stream (the IRCL pulse in rkisp2_isp_stop() resets the ISP and MIPI blocks only) or the reset state. With a capture size that differs from the sink size the MI is then fed frames of the wrong size. Running rkisp2_params_crop_init() first and letting a CROP block override it covers both cases. > +} > + > +void rkisp2_params_post_configure(struct rkisp2_params *params) > +{ > + struct rkisp2_params_buffer *buf; > + > + spin_lock_irq(¶ms->buf_lock); > + buf = list_first_entry_or_null(¶ms->params, > + struct rkisp2_params_buffer, queue); > + if (buf) > + list_del(&buf->queue); > + spin_unlock_irq(¶ms->buf_lock); > + > + if (!buf) > + return; > + > + rkisp2_params_configure(params, buf, RKISP2_PARAMS_CONFIG_PRIO_POST); > +} The buffer is taken off the list here and configured with PRIO_POST, which does not complete it (see above), so a params buffer queued before STREAMON never comes back from DQBUF; at STREAMOFF vb2 warns that stop_streaming left a buffer in active state and releases it itself. rkisp1_params_post_configure() completes the buffer after the LSC group; vb2_buffer_done() belongs here after the POST pass. [...] > + ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1); > + if (ret) { > + dev_err(rkisp2->dev, > + "failed to register %s, ret=%d\n", vdev->name, ret); > + return ret; > + } > + > + return 0; > + > +err_media: > + media_entity_cleanup(&vdev->entity); > + mutex_destroy(&node->vlock); > + return ret; goto err_media here as well. Neither path releases the vb2 queue that was initialised above. [...] > +++ b/drivers/media/platform/rockchip/rkisp2/rkisp2-resizer.c [...] > +void rkisp2_resizer_configure(struct rkisp2_resizer *rsz, [...] > + if (crop->width == rsz->source_size.width && crop->height == rsz->source_size.height) > + rkisp2_resizer_disable_rsz(rsz); > + else > + rkisp2_resizer_config_rsz(rsz, crop, > + &rsz->source_size, > + &rsz->source_format); The bypass test compares the luma size only. The ISP delivers 4:2:2, and rkisp2_resizer_config_rsz() handles the chroma subsampling when it runs, but an unscaled 4:2:0 stream (crop equal to the output size) disables the resizer and the MI gets one chroma row per luma row. On the RV1126 main path the chroma plane offset wraps halfway through the frame and the chroma of the lower half lands on the upper half (displaced colour ghosts and neutral halos). rkisp1_rsz_config() bypasses only when the size and the subsampling are both unchanged; the same test here fixed it for me. I do not know whether the 3.0 MI subsamples on its own in the ISP32_MI_OUTPUT_YUV420 mode; if it does not, this applies to the RK3588 too. Best regards, Daniel _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip