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 0C06BC021B3 for ; Sat, 22 Feb 2025 02:50:13 +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:Content-Transfer-Encoding: Content-Type:Subject:References:In-Reply-To:Message-Id:Cc:To:From:Date: MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=+Ml4921wRMeo4Va2Ot0cFWeiFqoEb/SaXu+3hQO+9WI=; b=JJiYzEyVeWocZ1DHRzdrLVEar9 baqXu4xcxhG/N6i3aEIzGEQd8t8wvRMOJTTdIEZTBm0LYVIgy/uIyamz+qmIQfPfWCnsUv/qh1A44 3RNLGMUpBS0e9uxUAnhQHVYCOWTac7i/izGoYlOK5i7z8ebDOPTvoaSrR7yqLopRqppMOgXl8co+e BnnRdBXfQ/oAdwdC/nGnhVgdLem+9Cm9Ek68tMuRvllnNQNBLIT+V4vJIbD56kq1oI7tmQyYoZU5d UHaDTGftu1+i9xBJ+7Az/oSZMtH1RetRqZj3WDTtYRHGs0h8ftOyEdL1jHZCugRay/EcLT/+Pjioo P7aPpCMg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1tlfaa-00000007N3i-1e4g; Sat, 22 Feb 2025 02:50:00 +0000 Received: from fout-a8-smtp.messagingengine.com ([103.168.172.151]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1tlfZ6-00000007MzM-0aVO for linux-arm-kernel@lists.infradead.org; Sat, 22 Feb 2025 02:48:29 +0000 Received: from phl-compute-13.internal (phl-compute-13.phl.internal [10.202.2.53]) by mailfout.phl.internal (Postfix) with ESMTP id B21EA1380A37; Fri, 21 Feb 2025 21:48:24 -0500 (EST) Received: from phl-imap-07 ([10.202.2.97]) by phl-compute-13.internal (MEProxy); Fri, 21 Feb 2025 21:48:24 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=testtoast.com; h=cc:cc:content-transfer-encoding:content-type:content-type :date:date:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:subject:subject:to:to; s=fm1; t=1740192504; x=1740278904; bh=+Ml4921wRMeo4Va2Ot0cFWeiFqoEb/Sa Xu+3hQO+9WI=; b=W9eTPIktQ2s23LhhHptCJ14xAtHMXQN1J5/nwPzlwHn5mb8X sI024syw071x9sjmeAMIh2ftcBEykETf38Nl2CP7BTK7GqWMZfX4fUmBmRkCIlMW dQhWbSSu2hdIQXmrR1r69jnBmAj8YpxtfcPMQbmNzV0zUl0h3pu7Zk0nBdM6fELV mUc989mZIHLY8Gmsp5Kwl1IUwa4e6VG35ej8EaAbZjxey4rjBVIDgHrHeq8YTq+m 1fT5PIpqBLZdviWZZS6AOtgOS4tMgMONJuFtAay0fzNZVAHNRTAkMIM+T9de3zBG QVeI/YedqDEgPDzIHJripftg8fgCXMyWToPwsQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1740192504; x= 1740278904; bh=+Ml4921wRMeo4Va2Ot0cFWeiFqoEb/SaXu+3hQO+9WI=; b=G XuQ/77mgC6GJMn17RU4R49h0XDjO1x78fm9CQsYnhSGGz7FGqN7m8MPASSVsdKX2 XtRZlsjzBEfYdYiSaaOkLo4ipcW2Y4hjAO7Dsq52o/XbRhc9HHRJRuKsLGr70gid nIHrNBRBjZqfGl4KjkwXBQGlrzt3JaAZszJdI8sciJ47t51/JuxjoaQrFR2Qd7Ae Be/icjIs47N05h2qmtc4L4V3tlmDTwx7WbRgIvjm/YG2clO8rEg6VVOHohfflkql RecwZ3tCft1KmZ9d9l3ly40p8i3CiVbJMbc4vBTEIyX0hKsbASY/asSTDFO6YP4j dmRt/Px0h+KMkveO53KRw== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdejudejfecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpggftfghnshhusghstghrihgsvgdp uffrtefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivg hnthhsucdlqddutddtmdenucfjughrpefoggffhffvvefkjghfufgtgfesthhqredtredt jeenucfhrhhomhepfdfthigrnhcuhggrlhhklhhinhdfuceorhihrghnsehtvghsthhtoh grshhtrdgtohhmqeenucggtffrrghtthgvrhhnpeeghffgkedtueeiudeukedvveevhfdu uefhhedviefgffduheeuieeihefhfefgveenucevlhhushhtvghrufhiiigvpedtnecurf grrhgrmhepmhgrihhlfhhrohhmpehrhigrnhesthgvshhtthhorghsthdrtghomhdpnhgs pghrtghpthhtohepvdefpdhmohguvgepshhmthhpohhuthdprhgtphhtthhopegrnhgurh gvrdhprhiihiifrghrrgesrghrmhdrtghomhdprhgtphhtthhopehmthhurhhquhgvthht vgessggrhihlihgsrhgvrdgtohhmpdhrtghpthhtohepfigvnhhssegtshhivgdrohhrgh dprhgtphhtthhopegurghnihgvlhesfhhffihllhdrtghhpdhrtghpthhtoheprghirhhl ihgvugesghhmrghilhdrtghomhdprhgtphhtthhopehjvghrnhgvjhdrshhkrhgrsggvtg esghhmrghilhdrtghomhdprhgtphhtthhopehkihhkuhgthhgrnhelkeesghhmrghilhdr tghomhdprhgtphhtthhopehmrggtrhhorghlphhhrgekvdesghhmrghilhdrtghomhdprh gtphhtthhopehsihhmohhnshdrphhhihhlihhpphgvsehgmhgrihhlrdgtohhm X-ME-Proxy: Feedback-ID: idc0145fc:Fastmail Received: by mailuser.phl.internal (Postfix, from userid 501) id 1E245BA006F; Fri, 21 Feb 2025 21:48:23 -0500 (EST) X-Mailer: MessagingEngine.com Webmail Interface MIME-Version: 1.0 Date: Sat, 22 Feb 2025 15:48:01 +1300 From: "Ryan Walklin" To: "Jernej Skrabec" , "Maxime Ripard" , "Chen-Yu Tsai" , "Maarten Lankhorst" , "Thomas Zimmermann" , "David Airlie" , "Daniel Vetter" , "Samuel Holland" , "Rob Herring" , "Krzysztof Kozlowski" , "Conor Dooley" , "Michael Turquette" , "Stephen Boyd" Cc: "Andre Przywara" , "Chris Morgan" , "Hironori KIKUCHI" , "Philippe Simons" , "Dmitry Baryshkov" , dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, devicetree@vger.kernel.org, linux-clk@vger.kernel.org Message-Id: <2a864555-d81f-4048-aa0b-c286544faa50@app.fastmail.com> In-Reply-To: <2221204.Mh6RI2rZIc@jernej-laptop> References: <20250216183524.12095-1-ryan@testtoast.com> <2221204.Mh6RI2rZIc@jernej-laptop> Subject: Re: [PATCH v7 00/27] drm: sun4i: add Display Engine 3.3 (DE33) support Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250221_184828_683003_4CC5DC23 X-CRM114-Status: GOOD ( 34.44 ) 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 On Sat, 22 Feb 2025, at 7:57 AM, Jernej =C5=A0krabec wrote: > Hi Ryan, > > sorry for very late review, but here we go... No problem, thanks for the review! > This patchset actually introduces 3 disticnt features, which should IM= O=20 > be separated > and thus making reviewing patches easier. > > 1. native 10-bit YUV420 output (without YUV->RGB->YUV conversions) -=20 > this is used on > HDMI for proper 10-bit 4K@60 HDR output > 2. Display Engine 3.3 (DE33) support > 3. AFBC modifier/format support (used for more efficient GPU or VPU=20 > output rendering) > > If I'm not mistaken, your goal is only DE33 support.=20 This is my initial goal, but I would still like good HDMI and video supp= ort (including HW decode).=20 It took some refactoring and resequencing of (your) existing driver work= to get to this series, and I have left out the HDMI and separated the T= CON code for the same reason, that initially I am intending to just supp= ort RGB operation to an LCD. I do intend however to use the HDMI code ei= ther in or out of tree but think that will be a much bigger/more complex= change to mainline given the current HDMI code is more invasive to the = generic driver. The YUV and AFBC changes seemed more self-contained and seemed reasonabl= e approaches given that they were both features of the DE3 and up. I am = happy either to split these into either 4 or 5 separate patches ie: 1a. refactor CSC code to prepare for YUV 1b. add YUV for DE3 2. refactor mixer enumeration and regmaps to support DE3+ 3. add DE33 4. Add AFBC My only reluctance is that that adds more work to manage multiple patche= s which are logically grouped and in terms of ease of review, all these = 4 steps are in the current set in that order (apart from AFBC which is c= ompletely standalone), and I don't think submitting them separately redu= ces complexity. It however will reduce reviewer burden as you suggest, b= ut this has been a slow process already. I am happy to accept either process but this has been some time already = now with lots of stylistic feedback but not much on the correctness of t= he approach and code, and I am keen to get this landed. There is are two=20 > reasons why > I've never sent these patches myself: > > 1. scaling YUV images doesn't work > > Not a problem for any user who doesn't use video player with DRM plane=20 > rendering, > which I assume is most of them. We can get around of this issue to den= y=20 > scaling > YUV buffers until the issue is sorted out. Good to know, I hadn't appreciated that. Mostly relevant for video as yo= u say and it would be good to land YUV support without scaling, then ext= end subsequently, possibly when we get to the video engine? > 2. plane allocations are hackish > > DE33 for the first time introduced option to move planes between the=20 > mixers. DRM > has also support for this, but for time being static allocation is=20 > acceptable and tbh, > even dynamic support need appropriate initial setting. > As you might guessed this is done in DE33 clock driver using magic=20 > values. Proper > way would be to introduce some kind of connection between mixer/plane=20 > and clock > driver, so initial configuration can be moved to appropriate module=20 > instead of > being thrown into clock driver. I need to check where to put it and ho= w=20 > to properly > connect DT nodes. Maybe syscon phandle? I'll do some research. Thanks, I was not aware of this either. > There is one glaring issue (when you work on driver and test every=20 > aspect of it). > DE33 introduced RCQ, which is basically DMA, which copies shadowed=20 > registers > from memory buffer directly to hw registers. IIRC it does that at=20 > vblank time. This > tells you that current concept of writing register values at any time=20 > userspace feels > to do commit is wrong and we've been lucky that it works as well as it=20 > does. So, > in order to fix this, driver would need quite a big reorganization.=20 > Introducing > shadow buffers would solve most of the issues, most likely also with=20 > YUV scaling. > Implementing RCQ would be then relatively simple and even improve=20 > timings. > Note that even DE3 has occasional issue with YUV scaler and also=20 > read-modify-read > access to some of its register can produce bogus value and thus corrup= t=20 > image > until next commit. Thanks, again I wasn't aware. All my testing has shown remarkable stabil= ity and there are some downstream users out-of-tree with good feedback, = but would be happy to support an effort to improve this. Regards, Ryan