Received: by 2002:a25:1985:0:0:0:0:0 with SMTP id 127csp409093ybz; Fri, 17 Apr 2020 03:26:49 -0700 (PDT) X-Google-Smtp-Source: APiQypI03wMKRH1auCv/JqvUlJwD8nFdKQn8CswyYGLlZTL065H/qmLYGZxzuoECIetUwWmcoxFQ X-Received: by 2002:a17:906:2488:: with SMTP id e8mr2317673ejb.157.1587119209530; Fri, 17 Apr 2020 03:26:49 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1587119209; cv=none; d=google.com; s=arc-20160816; b=hUVby2cQANpnGEEJhinRsGx+l+AkMUE2kfVmqQlFjrse6mWZ4UP2cTExGD0HAAZGCq QetynOheQvm7hNu4fJXK56Er0WegxvKAKr9Mxbk8P9xOaElT/lgHANA9oZ9qhcpAaZlp siZpShqqcKwgY/nnR4oR4io6HfDsldqO6jTCif4o1gjYO8oGpt6gwCrevMZ4FC8PEqrm opq+kvWwBeovFybte6udjtd67F6aPi7o2hfq/NhX2voBUbs55AS6jcWmdQuZSmIWyVLC 8eXEQ8hRPs2Yrn2Y4Ydt0lNeINuOPZ9fJ2vUQ8vUtLpoNP1M6ZxnU7+ncVRCcFPjtAlH Ncmw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:in-reply-to:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :dkim-signature:dkim-signature; bh=6Fjyjvm8YMRLuu1BmCR24WjceIi+YAIylzDdqTQnb3g=; b=YwQadr6mlxvfh/VSONfsnngaOPnuECdcqctZ3p3MHSzJOLnfcGX4I+n0/hQ07f7Jpx bqWBFVzwkNJ+PeGiBAEnFY2k5AtYUPGOgOX/rjFxuSWA1kWvDkodqFUkA8em/iyA1wmo b9xbkbsc5V4HM47tqpw9+qNdunMk34ogCaWhrOKX+L9Bof3Cc98z3OVAvOzdOMLGM4oh HcKlWxCEaRs6pNepY+uAtHK46QoHcjWnlQi1DZQe8DxB/kIxrwbggWg1In2OKrlnRTY8 I/ayqua1NVIj7JNpSt85bHlZ9qrU7hWfYmJg4sN5Ficw53+8HIU0b+4h+bVg7SC7Ceho fDRw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@cerno.tech header.s=fm2 header.b=wFPCXDM9; dkim=pass header.i=@messagingengine.com header.s=fm2 header.b=AhWcqSBk; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=cerno.tech Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id w1si2462969ejq.325.2020.04.17.03.26.26; Fri, 17 Apr 2020 03:26:49 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@cerno.tech header.s=fm2 header.b=wFPCXDM9; dkim=pass header.i=@messagingengine.com header.s=fm2 header.b=AhWcqSBk; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=cerno.tech Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729316AbgDQKZP (ORCPT + 99 others); Fri, 17 Apr 2020 06:25:15 -0400 Received: from new2-smtp.messagingengine.com ([66.111.4.224]:33327 "EHLO new2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726207AbgDQKZM (ORCPT ); Fri, 17 Apr 2020 06:25:12 -0400 Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailnew.nyi.internal (Postfix) with ESMTP id 745C958044A; Fri, 17 Apr 2020 06:25:09 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute4.internal (MEProxy); Fri, 17 Apr 2020 06:25:09 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cerno.tech; h= date:from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm2; bh=6Fjyjvm8YMRLuu1BmCR24WjceIi +YAIylzDdqTQnb3g=; b=wFPCXDM9IltWfPw0iWL5vGXhLWQwfrvIxIJ5jtqH5LP iNYeF3dY1CbSLg8g63cxYe7g7OJrsLBzRTEV+CM7JGa0yjbulxBtPRDSQ7dvGn4K Y20sHZhdRUBJXsE5BnNBRASZtt8Lt2js0NSTOCFn/TCPG9VYO+c3kj4ykbR2HxV5 FhPGRUT4sRAXTH5D/OtzTvAgS/rl8rGzjv5TMcdTR2+SZ2brDNijvCrJ/BMY6TXN 5A4D+0eIeQKvvPQ42e9SSDm78Pytcy3PFCuJUYsrkFjTgeTaD863wMI+ZBy4ezeB +SG/SdXTccNJcWgTsLGSNRcmJIN0CRbG/fM5Xl7AsMg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; bh=6Fjyjv m8YMRLuu1BmCR24WjceIi+YAIylzDdqTQnb3g=; b=AhWcqSBkWGz1YpKq4MYJZo a8nVeBhj8xS8Buguy6hDjd6IBa78G/FiJgIAbZIgu5KDP7HiVEJi2TidZxs+5gLF Y6BnyqZ1NuYsU22cO5nD8JhddlpqDW97r7w7/Adw+THs+MkqCOXtezggmHoa2awd vDWtHFMlTEXbzz8uaU0xPOXmnd4a/lieArch2a+dUpKV4uWJHD8+aZkrKFscMGhX 45iIyT/IS3zXi12CGaaCxtCDmmmGnp7DUXhmW+AF7XEwBCk/0k6gixXN+eR+W0/C 9h69MyD3jfgO9mdKsK6eUWr+HmMMGPxPtTuqVYUkFJqh+PwAMd/VMpbPFAY/8/7w == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduhedrfeejgddvkecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvuffkfhggtggujgesghdtreertddtvdenucfhrhhomhepofgrgihimhgv ucftihhprghrugcuoehmrgigihhmvgestggvrhhnohdrthgvtghhqeenucfkphepledtrd ekledrieekrdejieenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhl fhhrohhmpehmrgigihhmvgestggvrhhnohdrthgvtghh X-ME-Proxy: Received: from localhost (lfbn-tou-1-1502-76.w90-89.abo.wanadoo.fr [90.89.68.76]) by mail.messagingengine.com (Postfix) with ESMTPA id 48A393280066; Fri, 17 Apr 2020 06:25:03 -0400 (EDT) Date: Fri, 17 Apr 2020 12:25:00 +0200 From: Maxime Ripard To: "H. Nikolaus Schaller" Cc: Neil Armstrong , Mark Rutland , David Airlie , James Hogan , dri-devel@lists.freedesktop.org, linux-mips@vger.kernel.org, Paul Cercueil , linux-samsung-soc@vger.kernel.org, letux-kernel@openphoenux.org, Paul Burton , Krzysztof Kozlowski , Tony Lindgren , Chen-Yu Tsai , Kukjin Kim , devicetree@vger.kernel.org, =?utf-8?Q?Beno=C3=AEt?= Cousson , Rob Herring , linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Thomas Bogendoerfer , Philipp Rossak , openpvrsgx-devgroup@letux.org, linux-kernel@vger.kernel.org, Ralf Baechle , Daniel Vetter , kernel@pyra-handheld.com Subject: Re: [PATCH v6 01/12] dt-bindings: add img, pvrsgx.yaml for Imagination GPUs Message-ID: <20200417102500.erayf6quenp3cvn3@gilmour.lan> References: <06fb6569259bb9183d0a0d0fe70ec4f3033b8aab.1586939718.git.hns@goldelico.com> <20200415101251.o3wi5t6xvf56xmhq@gilmour.lan> <72919514-0657-4B71-902F-3E775E528F64@goldelico.com> <535CAEBE-F43E-4BFC-B989-612C81F0D7EF@goldelico.com> <20200415142124.yzfh6mtqq7cdq22e@gilmour.lan> <20200415162151.rwym4ioqz27migfn@gilmour.lan> <45F411C0-150B-4FBA-A0E1-B863B3F36DF6@goldelico.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="2cdcxbvig57yd3pf" Content-Disposition: inline In-Reply-To: <45F411C0-150B-4FBA-A0E1-B863B3F36DF6@goldelico.com> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --2cdcxbvig57yd3pf Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, On Wed, Apr 15, 2020 at 06:42:18PM +0200, H. Nikolaus Schaller wrote: > > Am 15.04.2020 um 18:21 schrieb Maxime Ripard : > >=20 > > On Wed, Apr 15, 2020 at 05:09:45PM +0200, H. Nikolaus Schaller wrote: > >> Hi Maxime, > >>=20 > >> Hm. Yes. We know that there likely are clocks and maybe reset > >> but for some SoC this seems to be undocumented and the reset > >> line the VHDL of the sgx gpu provides may be permanently tied > >> to "inactive". > >>=20 > >> So if clocks are optional and not provided, a driver simply can assume > >> they are enabled somewhere else and does not have to care about. If > >> they are specified, the driver can enable/disable them. > >=20 > > Except that at the hardware level, the clock is always going to be > > there. You can't control it, but it's there. >=20 > Sure, we can deduce that from general hardware design knowledge. > But not every detail must be described in DT. Only the important > ones. >=20 > >>> If OMAP is too much of a pain, you can also make > >>> a separate binding for it, and a generic one for the rest of us. > >>=20 > >> No, omap isn't any pain at all. > >>=20 > >> The pain is that some other SoC are most easily defined by clocks in > >> the gpu node which the omap doesn't need to explicitly specify. > >>=20 > >> I would expect a much bigger nightmare if we split this into two > >> bindings variants. > >>=20 > >>> I'd say that it's pretty unlikely that the clocks, interrupts (and > >>> even regulators) are optional. It might be fixed on some SoCs, but > >>> that's up to the DT to express that using fixed clocks / regulators, > >>> not the GPU binding itself. > >>=20 > >> omap already has these defined them not to be part of the GPU binding. > >> The reason seems to be that this needs special clock gating control > >> especially for idle states which is beyond simple clock-enable. > >>=20 > >> This sysc target-module@56000000 node is already merged and therefore > >> we are only adding the gpu child node. Without defining clocks. > >>=20 > >> For example: > >>=20 > >> sgx_module: target-module@56000000 { > >> compatible =3D "ti,sysc-omap4", "ti,sysc"; > >> reg =3D <0x5600fe00 0x4>, > >> <0x5600fe10 0x4>; > >> reg-names =3D "rev", "sysc"; > >> ti,sysc-midle =3D , > >> , > >> ; > >> ti,sysc-sidle =3D , > >> , > >> ; > >> clocks =3D <&gpu_clkctrl OMAP5_GPU_CLKCTRL 0>; > >> clock-names =3D "fck"; > >> #address-cells =3D <1>; > >> #size-cells =3D <1>; > >> ranges =3D <0 0x56000000 0x2000000>; > >>=20 > >> gpu: gpu@0 { > >> compatible =3D "ti,omap5-sgx544-116", "img,sgx544-116", "img,sgx54= 4"; > >> reg =3D <0x0 0x10000>; > >> interrupts =3D ; > >> }; > >> }; > >>=20 > >> The jz4780 example will like this: > >>=20 > >> gpu: gpu@13040000 { > >> compatible =3D "ingenic,jz4780-sgx540-130", "img,sgx540-130", "img,s= gx540"; > >> reg =3D <0x13040000 0x4000>; > >>=20 > >> clocks =3D <&cgu JZ4780_CLK_GPU>; > >> clock-names =3D "gpu"; > >>=20 > >> interrupt-parent =3D <&intc>; > >> interrupts =3D <63>; > >> }; > >>=20 > >> So the question is which one is "generic for the rest of us"? > >=20 > > I'd say the latter. >=20 > Why? >=20 > TI SoC seem to be the broadest number of available users > of sgx5xx in the past and nowadays. Others are more the exception. And maybe TI has some complicated stuff around the GPU that others don't ha= ve? If I look quickly at the Allwinner stuff, I see nothing looking alike in the SoC, so making the binding like that for everyone just because TI did somet= hing doesn't really make much sense. > > If your clock is optional, then you define it but don't mandate > > it. Not documenting it will only result in a mess where everyone will > > put some clock into it, possibly with different semantics each and > > every time. >=20 > So you mean that we should require a dummy clock for the omap gpu node > or did I misunderstand that? > > Well, yes there is of course a clock connection between the > omap target-module and the sgx but it is IMHO pointless to > describe it because it can't and does not need to be controlled > separately. >=20 > As said the target-module is already accepted and upstream and my > proposal is to get the gpu node described there. There is simply > no need for a clocks node for the omap. There is no need for a clocks property *currently* *on the OMAP*. > What I also assume is that developers of DTS know what they do. > So the risk that there is different semantics is IMHO very low. Well, they know what they do if you document the binding. Let's say I have = two clocks now on my SoC, and you just document that you want a clocks property, with a generic name in clock-names like "gpu". > If you agree I can add the clocks/clock-names property as an > optional property. This should solve omap and all others. With the above example, what clock should I put in there? In which order? T= his isn't some random example pulled out of nowhere. The Allwinner A31 has (at least) 4 clocks for the GPU, 1 reset line and 1 regulator, so I can only as= sume that the GPU actually needs at least that amount to be properly integrated = into an SoC. This has nothing to do with being dumb or smart. > > This has nothing to do with the binding being complete. And if you use > > a binding like this one, you'll be severely limited when you'll want > > to implement things like DVFS. >=20 > Now you have unhooked me... Nobody seems to know if and how DVFS can be > applied to SGX. IMHO we should bake small bread first and get initial > support into mainline. On the software side, yes, of course. But the discussion here doesn't have = much to do with software support, this is about the hardware. No matter if you e= nable DVFS or not, you'll have those resources connected to the GPU. And if you want to enable the strict minimum in DT for now and expand it la= ter as the software gains support for more stuff, then you'll have to deal with= the minimal stuff in software later-on to keep the backward compatibility. But given that the current state on the Allwinner SoCs (at least) is that y= ou can't even read a register, it might be a good idea to delay the introducti= on of that binding until you have something that works to avoid drowning under the number of special cases to deal with backward compatibility. Maxime --2cdcxbvig57yd3pf Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCXpmD/AAKCRDj7w1vZxhR xS+DAQD1ud+4u/Gtzw+YK29b+79bwdplPbylCn4JhitPCi+ezgEAgOxiTpYRh+GB 1bgxLBKhqARErScbQPmvn22AzLAomAY= =Hsz5 -----END PGP SIGNATURE----- --2cdcxbvig57yd3pf--