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 28AFEC2A099 for ; Mon, 5 Jan 2026 11:29:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Message-ID:Date:To:Cc:From: Subject:References:In-Reply-To:Content-Transfer-Encoding:MIME-Version: Content-Type:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Mu3cyRLYu86MCsdn6oOtKe+hE5fEosqU+bTYtq9HHFs=; b=jwo1XKzw45MDbRehDmnjW1mJwi F6Q5DAJ30cqG0W2AqXkCXBz4Qp2y6+fPFAcPokgdDgUZTvxpBOs6EpthogweywhoU067XpMl/MbM1 oQtcg2ZmBawYlqbZi0dWWN1O0ewNf8jB4x82a/jv0qL+Do0p102BVvK8JJS06uVUmKWdr4sOftU/U GtQUPGkYBh9pKMYUckEa9cvILVFJdVsQksx04jce7QD7o3g/RPhvdYwR8lqtaXScGnUc+9YvEyymS N/2CJJEfMULmQA+2/3MdnRjba7d2SnJbbjkG3IgTg1eINggrOLGRehGcm39lQtNUz3rHST191CK9P llotSrZA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1vcimG-0000000BC1d-38fj; Mon, 05 Jan 2026 11:29:36 +0000 Received: from perceval.ideasonboard.com ([213.167.242.64]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1vcimE-0000000BC1C-30WE; Mon, 05 Jan 2026 11:29:35 +0000 Received: from pendragon.ideasonboard.com (cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 15C42664; Mon, 5 Jan 2026 12:29:10 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1767612550; bh=iISwdZtvnnraV/VMYYVj79jvGkuZPTBqF1C6SfCyOhY=; h=In-Reply-To:References:Subject:From:Cc:To:Date:From; b=EzzMb8HOeneWwzjW6sYtITaQ8Sg50f2HmArAmxA9vGADkwgV6rGLYbY4Aps7IK0Ys qdJMpeVTgDso/wLf6WaCRV6v3Uylym63LK83uGB9IhtwGTE6E+6Vvc5YbsbEn0WI1L L90b+d9rvoArIvwQYmg3m+NbAijw4cYgT/Na4k7k= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable In-Reply-To: <176465775606.135635.13034018447792643478@localhost> References: <20251202015025.601549-1-rui.wang@ideasonboard.com> <20251202015025.601549-2-rui.wang@ideasonboard.com> <176465775606.135635.13034018447792643478@localhost> Subject: Re: [PATCH v1 1/1] media: rkisp1: Fix filter mode register configuration From: Kieran Bingham Cc: libcamera-devel@lists.libcamera.org, Rui Wang To: Rui Wang , Stefan Klug , dafna@fastmail.com, heiko@sntech.de, laurent.pinchart@ideasonboard.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org, mchehab@kernel.org Date: Mon, 05 Jan 2026 11:29:27 +0000 Message-ID: <176761256763.3192372.8001486757502337420@ping.linuxembedded.co.uk> User-Agent: alot/0.9.1 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20260105_032934_897653_2CA8F730 X-CRM114-Status: GOOD ( 19.86 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Quoting Stefan Klug (2025-12-02 06:42:36) > Hi Rui, >=20 > Thank you for the patch. >=20 > Quoting Rui Wang (2025-12-02 02:50:25) > > The rkisp1_flt_config() function performs an initial direct write to > > RKISP1_CIF_ISP_FILT_MODE without including the RKISP1_CIF_ISP_FLT_ENA > > bit, which clears the filter enable bit in the hardware. >=20 > That sentence is a bit hard for me to understand. Maybe: >=20 > "The rkisp1_flt_config() function overwrites RKISP1_CIF_ISP_FILT_MODE > without preserving the RKISP1_CIF_ISP_FLT_ENA bit thereby unconditionally > disabling the hardware block on reconfiguration. Stefan's proposal sounds good here. Reviewed-by: Kieran Bingham >=20 > But as I'm no native speaker you could maybe wait for feedback from a > native speaker. >=20 > Functionality wise the change is correct. >=20 > Reviewed-by: Stefan Klug >=20 > Best regards, > Stefan >=20 > >=20 > > The subsequent read/modify/write sequence then reads back the register > > with the enable bit already cleared and cannot restore it, resulting in > > the filter being inadvertently disabled. > >=20 > > Remove the redundant direct write. The read/modify/write sequence alone > > correctly preserves the existing enable bit state while updating the > > DNR mode and filter configuration bits. > >=20 > > Signed-off-by: Rui Wang > > --- > > drivers/media/platform/rockchip/rkisp1/rkisp1-params.c | 6 ------ > > 1 file changed, 6 deletions(-) > >=20 > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/d= rivers/media/platform/rockchip/rkisp1/rkisp1-params.c > > index c9f88635224c..6442436a5e42 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > > @@ -411,12 +411,6 @@ static void rkisp1_flt_config(struct rkisp1_params= *params, > > rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_FILT_LUM_WEIGHT, > > arg->lum_weight); > > =20 > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_FILT_MODE, > > - (arg->mode ? RKISP1_CIF_ISP_FLT_MODE_DNR : 0) | > > - RKISP1_CIF_ISP_FLT_CHROMA_V_MODE(arg->chr_v_mode) | > > - RKISP1_CIF_ISP_FLT_CHROMA_H_MODE(arg->chr_h_mode) | > > - RKISP1_CIF_ISP_FLT_GREEN_STAGE1(arg->grn_stage1)); > > - > > /* avoid to override the old enable value */ > > filt_mode =3D rkisp1_read(params->rkisp1, RKISP1_CIF_ISP_FILT_M= ODE); > > filt_mode &=3D RKISP1_CIF_ISP_FLT_ENA; > > --=20 > > 2.43.0 > >