From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C3945351C3E; Wed, 25 Mar 2026 09:00:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.11 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774429241; cv=none; b=VEx/YkMTcnFBQk/eh6ukiToPe2zEbpTqaj6lTN5kqGVbXzS0YP8qr/eMRRla3zV/VEO4l+6BzqlEjiG72BVathNuT+99b6v1yBa9V0hy3xk6SqIZ55Os3Zc5dhcE0dl/9zieoRrDYPSg65a6zBX3aUt7z1cgltKLf0GLTl7tba0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774429241; c=relaxed/simple; bh=DpD3cz3pQHgqJTBucX6RnJ1ygBMNYK/5RTUg9Cqf2cE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=tHr3yQZV+IJJ8KJMKT5c+TnYvcJrVzGMGm2c2Cd3JaB0cewqtUM+zXnHhRi1xZkcI2HB7AmzYjnAiqVdu/DOmm3RXy7QYeFEk3Jk+GLU8mr6HGf2nKR5NkaIX1hIBmSNFyQoC7J7AAzmPgah45jzW/jPml9I5yQXe2rJSGAT3SI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=pass smtp.mailfrom=linux.intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=l6MkTfVQ; arc=none smtp.client-ip=198.175.65.11 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="l6MkTfVQ" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1774429239; x=1805965239; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=DpD3cz3pQHgqJTBucX6RnJ1ygBMNYK/5RTUg9Cqf2cE=; b=l6MkTfVQmJwvWEtdrVy+i5529g+EA4XUY60E1u432RTM5kqyFygXz9PC q9T2NMs2Cy1liAhx+1om/JZSDTLLyTmSu0DwuqiUrDCRGEhz80F0lGjeR oWUyYHZG5/Vc1M9HnpZMLqdTTIdrFv8Ll2z57SnzTsLl5xOgjAvfTXNp5 ZFOXkldYZhx9oOK09PMZ1ZsVRE3Py5eYJUwJ7+4MubD9PeOPnh91MACsX bSGXOLNzilF3AP7o/CEwgTI0Uq1KTIt5s3aJAP4bPqWRVn9nBEOCUidxc JMvfySTiCGgciFemRewffv8D6WLGnOr1rZLxLwqUXwXMrASRDWZ1AHtIj w==; X-CSE-ConnectionGUID: GyY2p0t7QoWCoNBvakPBcg== X-CSE-MsgGUID: gVdthHgPRRalmZSYjvY+ZQ== X-IronPort-AV: E=McAfee;i="6800,10657,11739"; a="85771607" X-IronPort-AV: E=Sophos;i="6.23,139,1770624000"; d="scan'208";a="85771607" Received: from fmviesa004.fm.intel.com ([10.60.135.144]) by orvoesa103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Mar 2026 02:00:38 -0700 X-CSE-ConnectionGUID: FfBrG/siSUeCka5yu+K/8A== X-CSE-MsgGUID: YXrsMJFFTrywJEx/SOr1vg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.23,139,1770624000"; d="scan'208";a="226253949" Received: from klitkey1-mobl1.ger.corp.intel.com (HELO mdjait-mobl) ([10.245.245.94]) by fmviesa004-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Mar 2026 02:00:33 -0700 Date: Wed, 25 Mar 2026 10:00:25 +0100 From: Mehdi Djait To: Michael Riesch Cc: Laurent Pinchart , Mauro Carvalho Chehab , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Heiko Stuebner , Kever Yang , Jagan Teki , =?utf-8?B?0JrRg9C30L3QtdGG0L7QsiDQnNC40YXQsNC40Ls=?= , Sebastian Reichel , Nicolas Dufresne , Collabora Kernel Team , Sakari Ailus , linux-media@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 3/9] media: rockchip: rkcif: add support for rk3588 vicap mipi capture Message-ID: References: <20250430-rk3588-vicap-v2-0-77de5ee9048e@collabora.com> <20250430-rk3588-vicap-v2-3-77de5ee9048e@collabora.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Hi Michael, On Wed, Mar 25, 2026 at 09:04:57AM +0100, Michael Riesch wrote: > Hi Mehdi, > > On 3/17/26 14:28, Mehdi Djait wrote: > > Hi Michael, > > > > On Tue, Mar 17, 2026 at 02:21:20PM +0100, Michael Riesch wrote: > >> Hi Mehdi, > >> > >> On 3/17/26 14:08, Mehdi Djait wrote: > >>> Hi Michael, > >>> > >>> Thank you for this nice patch! > >>> > >>> On Tue, Mar 17, 2026 at 10:32:21AM +0100, Michael Riesch via B4 Relay wrote: > >>>> From: Michael Riesch > >>>> > >>>> The RK3588 Video Capture (VICAP) unit features a Digital Video Port > >>>> (DVP) and six MIPI CSI-2 capture interfaces. Add initial support > >>>> for this variant to the rkcif driver and enable the MIPI CSI-2 > >>>> capture interfaces. > >>>> > >>>> Signed-off-by: Michael Riesch > >>> > >>> [...] > >>> > >>>> static inline unsigned int rkcif_mipi_get_reg(struct rkcif_interface *interface, > >>>> unsigned int index) > >>>> { > >>>> @@ -631,6 +765,8 @@ static int rkcif_mipi_start_streaming(struct rkcif_stream *stream) > >>>> rkcif_mipi_stream_write(stream, RKCIF_MIPI_CTRL1, ctrl1); > >>>> rkcif_mipi_stream_write(stream, RKCIF_MIPI_CTRL0, ctrl0); > >>>> > >>>> + rkcif_mipi_write(interface, RKCIF_MIPI_CTRL, RKCIF_MIPI_CTRL_CAP_EN); > >>>> + > >>> > >>> while this is the correct solution for rk3588, for the rk3568 vicap this > >>> will write 0x1 to the VICAP_MIPI_CTRL : 0x00A0 which will enable the water line. > >> > >> nice catch ;-) However, the TRM (at least my version) claims that this > >> bit has a reset value of 0x1, so the bit in question should be already > >> set in the first place. Thus I decided to *not* make variant specific > >> code paths. > > > > Yes, the reset value is indeed 0x1 > > > >> > >> Do you see problems in your setup? > >> > > > > No problems, it works as expected, I was just confused to see the > > mipi capture enable added with this rk3588 patch and not before. > > > > I just find it a bit confusing but if a nicer solution is too much > > hassle we can leave it like this. > > I agree that this is (while correct) not the nicest way. I am still > bringing up the remaining features of the RK3588 VICAP (MUX + TOISP + > SCALER) and it looks like I need to do some refactoring anyway to > support them. When I do that, I shall rewrite this part. For the time > being it would be great to have this merged in order to provide initial > mainline support for this unit. That sounds good. How about adding a TODO comment ? -- Kind Regards Mehdi Djait