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 0F7A6C433F5 for ; Wed, 23 Mar 2022 15:59:52 +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-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-Transfer-Encoding:Content-ID:Content-Description:Resent-Date :Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=lqADDmyR3E19pWPTCEI/fzzXBBgrHl8EDpdiOkabssQ=; b=yU5Jhm5oVYYMFDz7F7ctgamumS alxmuscO/9/dqdYtk8zpO8S5x/XMZ3QuwzDjfkSpa0n/kitvlV6N+v79XUIrj3vaYPYpDF9m6rGeV OYEiTuK0/yji7ut4VQ/o6VxQrK59qwAA+oyKvsy/ETzF9+vlLsNGCKmHXShm2U8/yREY/jM1SWUME yf7LBiACmmjWg540kzSRRQ03kKpc4K3l34WiouVlycZ84eJr+gHGAV5NpRM6InqAPnTgx3dRpU/Zj gKC0oa8Y7fz6t0HKv2OnuzHwdo0nECPUTiIeultCiJNOjpWA74gtccWoG+9TLfn8+LLFfoaTYR+fh occrTHwA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nX3NW-00EBIK-EY; Wed, 23 Mar 2022 15:58:30 +0000 Received: from wout3-smtp.messagingengine.com ([64.147.123.19]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nX3NS-00EBGr-B2 for linux-arm-kernel@lists.infradead.org; Wed, 23 Mar 2022 15:58:28 +0000 Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.west.internal (Postfix) with ESMTP id 44AEE3201DB9; Wed, 23 Mar 2022 11:58:20 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Wed, 23 Mar 2022 11:58:21 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cerno.tech; h=cc :cc:content-type:date:date:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to; s=fm3; bh=dwi9X2Xm2WeckTStHnk4GJWLq9tynSwb7GGqOE 4Tk10=; b=iezCiuRZP4pEhXiDQd/oxlAUn+ec32LM4DopXZ2GB1SaZa9i8rOD7I iLTA9HJkT0T+B7+OkUegeeyiv8XborC7mUT+a2Kw5BIKUW34wOk/HdfrwQUGDYBY 5H7CGxXwoNmG889dg/wMPifS0g4pQzAbtAFch3Tto+gL933QfZLcdm4CGHySvr8e wFBPMkmvmaA7Hs9aTPtROWAVeI+r1IFHk8T8CaPDmAPYkPrjMme+LPIlGnCf2hSW J7erwTkBE3SEKdEHwpi5KG3Wv+2wFtl8OE6EfAw2q8nDEqBOEHnCWYMErCSYurEj yezVhwP9O1nGAOARMKzw2hHknQsWc1rw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm3; bh=dwi9X2Xm2WeckTStH nk4GJWLq9tynSwb7GGqOE4Tk10=; b=WjUAgr+rOoT5S1yHopnzGglVfz9l3EGAa MC3GeVSItAhCNQhdwucAYOlYdynExDWLFC2+3j5hV0GrysahgTWypdxleO7gfjmm +rcTy0GgP1IDNrgjOYqTHJXeUlow+0G6PYwiH30tg4grNxb1RtaNlMdhDXhiXtxO DRc063Vrldv2X4YLReWA4jB4cxJ9LXZb/xNlb67Gc3RRiSFlV0mf0XwDGV7PLztZ hdl0dbSnjvhyDF7nNwnyYW+BiKsCHKK/GxGR93KT02TnpKzXmfXjXqVt8CuTUubn 3BnVeS0K1jcc/56W2ZV3L5p8FV/s9NYFMbz7wlSKkXBFgvLluSyWw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddrudegjedgkedvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpeforgigihhm vgcutfhiphgrrhguuceomhgrgihimhgvsegtvghrnhhordhtvggthheqnecuggftrfgrth htvghrnhepveevfeffudeviedtgeethffhteeuffetfeffvdehvedvheetteehvdelfffg jedvnecuffhomhgrihhnpehkvghrnhgvlhdrohhrghenucevlhhushhtvghrufhiiigvpe dtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehmrgigihhmvgestggvrhhnohdrthgvtghh X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 23 Mar 2022 11:58:18 -0400 (EDT) Date: Wed, 23 Mar 2022 16:58:17 +0100 From: Maxime Ripard To: max.krummenacher@gmx.de Cc: Dave Stevenson , Marek Vasut , Christoph Niedermaier , Max Krummenacher , Pengutronix Kernel Team , David Airlie , Sam Ravnborg , Sascha Hauer , DRI Development , DenysDrozdov , Laurent Pinchart , Shawn Guo , Linux ARM , NXP Linux Team Subject: Re: [RFC PATCH] drm/panel: simple: panel-dpi: use bus-format to set bpc and bus_format Message-ID: <20220323155817.xcsqxothziot7ba3@houat> References: <20220302142142.zroy464l5etide2g@houat> <9c9a10ca-e6a1-c310-c0a5-37d4fed6efd6@denx.de> <20220318163549.5a5v3lex4btnnvgb@houat> <20220318171642.y72eqf5qbmuu2ln2@houat> <5ae44b7cd1f7577c98f316a7d288aa4cf423da2d.camel@active.ch> MIME-Version: 1.0 In-Reply-To: <5ae44b7cd1f7577c98f316a7d288aa4cf423da2d.camel@active.ch> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220323_085826_414465_C1A625AB X-CRM114-Status: GOOD ( 59.07 ) 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: , Content-Type: multipart/mixed; boundary="===============4999244177665670341==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============4999244177665670341== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="oto3uixman4x7lhu" Content-Disposition: inline --oto3uixman4x7lhu Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Mar 23, 2022 at 09:42:11AM +0100, Max Krummenacher wrote: > Am Freitag, den 18.03.2022, 17:53 +0000 schrieb Dave Stevenson: > > On Fri, 18 Mar 2022 at 17:16, Maxime Ripard wrote: > > > On Fri, Mar 18, 2022 at 05:05:11PM +0000, Dave Stevenson wrote: > > > > Hi Maxime > > > >=20 > > > > On Fri, 18 Mar 2022 at 16:35, Maxime Ripard wro= te: > > > > > On Mon, Mar 07, 2022 at 04:26:56PM +0100, Max Krummenacher wrote: > > > > > > On Wed, Mar 2, 2022 at 5:22 PM Marek Vasut wrot= e: > > > > > > > On 3/2/22 15:21, Maxime Ripard wrote: > > > > > > > > Hi, > > > > > > >=20 > > > > > > > Hi, > > > > > > >=20 > > > > > > > > Please try to avoid top posting > > > > > > Sorry. > > > > > >=20 > > > > > > > > On Wed, Feb 23, 2022 at 04:25:19PM +0100, Max Krummenacher = wrote: > > > > > > > > > The goal here is to set the element bus_format in the str= uct > > > > > > > > > panel_desc. This is an enum with the possible values defi= ned in > > > > > > > > > include/uapi/linux/media-bus-format.h. > > > > > > > > >=20 > > > > > > > > > The enum values are not constructed in a way that you cou= ld calculate > > > > > > > > > the value from color channel width/shift/mapping/whatever= =2E You rather > > > > > > > > > would have to check if the combination of color channel > > > > > > > > > width/shift/mapping/whatever maps to an existing value an= d otherwise > > > > > > > > > EINVAL out. > > > > > > > > >=20 > > > > > > > > > I don't see the value in having yet another way of how th= is > > > > > > > > > information can be specified and then having to write a m= ore > > > > > > > > > complicated parser which maps the dt data to bus_format. > > > > > > > >=20 > > > > > > > > Generally speaking, sending an RFC without explicitly stati= ng what you > > > > > > > > want a comment on isn't very efficient. > > > > > > >=20 > > > > > > > Isn't that what RFC stands for -- Request For Comment ? > > > > > >=20 > > > > > > I hoped that the link to the original discussion was enough. > > > > > >=20 > > > > > > panel-simple used to have a finite number of hardcoded panels s= elected > > > > > > by their compatible. > > > > > > The following patchsets added a compatible 'panel-dpi' which sh= ould > > > > > > allow to specify the panel in the device tree with timing etc. > > > > > > =20 > > > > > > https://patchwork.kernel.org/project/dri-devel/patch/2020021618= 1513.28109-6-sam@ravnborg.org/ > > > > > > In the same release cycle part of it got reverted: > > > > > > =20 > > > > > > https://patchwork.kernel.org/project/dri-devel/patch/2020031415= 3047.2486-3-sam@ravnborg.org/ > > > > > > With this it is no longer possible to set bus_format. > > > > > >=20 > > > > > > The explanation what makes the use of a property "data-mapping"= not a > > > > > > suitable way in that revert > > > > > > is a bit vague. > > > > >=20 > > > > > Indeed, but I can only guess. BGR666 in itself doesn't mean much = for > > > > > example. Chances are the DPI interface will use a 24 bit bus, so = where > > > > > is the padding? > > > > >=20 > > > > > I think that's what Sam and Laurent were talking about: there was= n't > > > > > enough information encoded in that property to properly describe = the > > > > > format, hence the revert. >=20 > I agree that the strings used to set "data-mapping" weren't self explaini= ng. > However, as there was a > clear 1:1 relation to the bus_format value the meaning > wasn't ambiguous at all. >=20 > > > >=20 > > > > MEDIA_BUS_FMT_RGB666_1X18 defines an 18bit bus, therefore there is = no > > > > padding. "bgr666" was selecting that media bus code (I won't ask ab= out > > > > the rgb/bgr swap). > > > >=20 > > > > If there is padding on a 24 bit bus, then you'd use (for example) > > > > MEDIA_BUS_FMT_RGB666_1X24_CPADHI to denote that the top 2 bits of e= ach > > > > colour are the padding. Define and use a PADLO variant if the paddi= ng > > > > is the low bits. > > >=20 > > > Yeah, that's kind of my point actually :) > >=20 > > Ah, OK :) > >=20 > > > Just having a rgb666 string won't allow to differentiate between > > > MEDIA_BUS_FMT_RGB666_1X18 and MEDIA_BUS_FMT_RGB666_1X24_CPADHI: both = are > > > RGB666 formats. Or we could say that it's MEDIA_BUS_FMT_RGB666_1X18 a= nd > > > then when we'll need MEDIA_BUS_FMT_RGB666_1X24_CPADHI we'll add a new > > > string but that usually leads to inconsistent or weird names, so this > > > isn't ideal. >=20 > We're on the same page that the strings that were used aren't self > explaining and do not follow a pattern which would make it easy to > extend. However that is something I addressed in my RFC proposal, not? >=20 > > >=20 > > > > The string matching would need to be extended to have some string to > > > > select those codes ("lvds666" is a weird choice from the original > > > > patch). > > > >=20 > > > > Taking those media bus codes and handling them appropriately is > > > > already done in vc4_dpi [1], and the vendor tree has gained > > > > BGR666_1X18 and BGR666_1X24_CPADHI [2] as they aren't defined in > > > > mainline. > > > >=20 > > > > Now this does potentially balloon out the number of MEDIA_BUS_FMT_x= xx > > > > defines needed, but that's the downside of having defines for all > > > > formats. > > > >=20 > > > > (I will admit to having a similar change in the Pi vendor tree that > > > > allows the media bus code to be selected explicitly by hex value). > > >=20 > > > I think having an integer value is indeed better: it doesn't change m= uch > > > in the device tree if we're using a header, it makes the driver simpl= er > > > since we don't have to parse a string, and we can easily extend it or > > > rename the define, it won't change the ABI. >=20 > Fine with me. >=20 > > >=20 > > > I'm not sure using the raw media bus format value is ideal though, si= nce > > > that value could then be used by any OS, and it would effectively for= ce > > > the mbus stuff down their throat. >=20 > I disagree here, this forces us to use code to map the device tree enum > to the kernel enum for Linux, i.e. adds complexity and maintenance work > if additional bus_formats are needed. > Assuming there is another OS which uses the device tree it would not > make a difference, that OS would still need to map the device tree enum > to the corresponding representation in their kernel. So, you don't want to do something in Linux, but would expect someone else to be completely ok with that? > I would copy the definitions of media-bus-format.h into a header in > include/dt-bindings similarly as it is done for > include/dt-bindings/display/sdtv-standards.h for TV standards. That might not be an option: that header is licensed under the GPL, device trees are usually licensed under GPL+MIT, and we don't have any requirements on the license for other projects using a DT (hence the dual license). Maxime --oto3uixman4x7lhu Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCYjtDmQAKCRDj7w1vZxhR xVvgAP46o9tHgfUnrlsA2sWXayXp+nWRUbhdROv4DrBryTGw5QEA14DuKNGCjWqz yKKal32xr9RzTgfR31Zln5sLMLbAEQ8= =up+M -----END PGP SIGNATURE----- --oto3uixman4x7lhu-- --===============4999244177665670341== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============4999244177665670341==--