Received: by 2002:a25:8b91:0:0:0:0:0 with SMTP id j17csp2425994ybl; Thu, 9 Jan 2020 12:32:25 -0800 (PST) X-Google-Smtp-Source: APXvYqyK3odPbpcdwc4Js2WS+2mbiK+EmXpap8eZ5ymp43KrF7XRq12SJOQ0BenOPEhlL4VgUN6G X-Received: by 2002:a05:6830:2147:: with SMTP id r7mr10242215otd.94.1578601945095; Thu, 09 Jan 2020 12:32:25 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1578601945; cv=none; d=google.com; s=arc-20160816; b=s5HTySQ4oYO7PDHBvPk4dAkvdqCBrhVsI6QQMEGuaHNfk5DtpRdv+NqHz7OD7+8WJx H5FdNl99BmKOtwwqI3C9H+T4VDHFJLuh/doPSn25y4u+mG4OH/EzLMYppoz3cxyEVraX PA8Lwo6CbXO0IjbRa/yBbzCfwixjkZFPBsmeLWaGBJyK/aePq8S0TA1x9PtW24DEzLzp 0GbBcJQdQNgjbEVyoVO34rtfWiM1ME4Rv1/wAmPNKtK1kt1nwnadX/N/eB93x+9q81G/ CgLRTKhkq04ihaVyyoIuWHe14vJxSz3aeGnRWASLZo37vnf3qFeLx30Uzl6K4IPIvYGA 4Gkw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject; bh=3v21NYYSTbwf4ZG7DFznz/xTbWIK5IKQGxKF5EawFOs=; b=rTknNUnlee+M67PSMVjBP6i/WT7GyMfp44l4xQnReQTHB4sDlnXuRMjUGGMb8PD4pm zm94jHTT4NqHeRQYhacbCQVhnSnHY5plUfmE459Cim7QuZ0CIkmg4ToMseNyfQSzoA4d FKbBKXOqo3H5Xv6/7ehR9YldrJYk3m40UjKKboix+nY84YNJVkkmLYNjOxC4tX35r9WD ukFrPj4zXklFEa+1SSbJsGDMyKXhA3YlnWNjDaKTFzxvK/d219CKpt0h1jtS5yPEGMvv hoPCbaLyHZrcov2qt6VQLtjVMV1EKYKmjr1Puq0/oou1udPrKZE8y4UDfI+JU6uw+kVf XqRw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id l11si4215669oie.231.2020.01.09.12.32.13; Thu, 09 Jan 2020 12:32:25 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387901AbgAIQxJ (ORCPT + 99 others); Thu, 9 Jan 2020 11:53:09 -0500 Received: from foss.arm.com ([217.140.110.172]:34632 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729572AbgAIQxI (ORCPT ); Thu, 9 Jan 2020 11:53:08 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E4EBE1FB; Thu, 9 Jan 2020 08:53:07 -0800 (PST) Received: from [10.1.38.29] (e122027.cambridge.arm.com [10.1.38.29]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9D45A3F703; Thu, 9 Jan 2020 08:53:04 -0800 (PST) Subject: Re: [PATCH v2 4/7] drm/panfrost: Add support for a second regulator for the GPU To: Mark Brown Cc: Mark Rutland , Devicetree List , Nicolas Boichat , Tomeu Vizoso , David Airlie , lkml , dri-devel@lists.freedesktop.org, Liam Girdwood , Rob Herring , "moderated list:ARM/Mediatek SoC support" , Alyssa Rosenzweig , Hsin-Yi Wang , Matthias Brugger , linux-arm Mailing List References: <20200108052337.65916-1-drinkcat@chromium.org> <20200108052337.65916-5-drinkcat@chromium.org> <20200108132302.GA3817@sirena.org.uk> <09ddfac3-da8d-c039-92a0-d0f51dc3fea5@arm.com> <20200109162814.GB3702@sirena.org.uk> From: Steven Price Message-ID: Date: Thu, 9 Jan 2020 16:53:02 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: <20200109162814.GB3702@sirena.org.uk> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/01/2020 16:28, Mark Brown wrote: > On Thu, Jan 09, 2020 at 02:14:42PM +0000, Steven Price wrote: >> On 08/01/2020 22:52, Nicolas Boichat wrote: > >>> That'd be a bit awkward to match, though... Currently all bifrost >>> share the same compatible "arm,mali-bifrost", and it'd seem >>> weird/wrong to match "mediatek,mt8183-mali" in this driver? I have no >>> idea if any other Mali implementation will require a second regulator, >>> but with the MT8183 we do need it, see below. > > This doesn't sound particularly hard, just new. Plenty of other devices > have quirks done based on the SoC they're in or the IP revision, this > would just be another of those quirks. > >>> Well if devfreq was working (see patch 7 >>> https://patchwork.kernel.org/patch/11322851/ for a partial >>> implementation), it would adjust both mali and sram regulators, see >>> the OPP table in patch 2 >>> (https://patchwork.kernel.org/patch/11322825/): SRAM voltage needs to >>> be increased for frequencies >=698Mhz. > >>> Now if you have some better idea how to implement this, I'm all ears! > > Set a flag based on the compatible, then base runtime decisions off > that. > >> I'm not sure if it's better, but could we just encode the list of >> regulators into device tree. I'm a bit worried about special casing an >> "sram" regulator given that other platforms might have a similar >> situation but call the second regulator a different name. > > Obviously the list of regulators bound on a given platform is encoded in > the device tree but you shouldn't really be relying on that to figure > out what to request in the driver - the driver should know what it's > expecting. From a driver perspective we don't expect to have to worry about power domains/multiple regulators - the hardware provides a bunch of power registers to turn on/off various parts of the hardware and this should be linked (in hardware) to a PDC which sorts it out. The GPU/PDC handles the required sequencing. So it *should* be a case of turn power/clocks on and go. However certain integrations may have quirks such that there are physically multiple supplies. These are expected to all be turned on before using the GPU. Quite how this is best represented is something I'm not sure about. > Bear in mind that getting regulator stuff wrong can result > in physical damage to the system so it pays to be careful and to > consider that platform integrators have a tendency to rely on things > that just happen to work but aren't a good idea or accurate > representations of the system. It's certainly *possible* to do > something like that, the information is there, but I would not in any > way recommend doing things that way as it's likely to not be robust. > > The possibility that the regulator setup may vary on other platforms > (which I'd expect TBH) does suggest that just requesting a bunch of > supply names optionally and hoping that we got all the ones that are > important on a given platform is going to lead to trouble down the line. Certainly if we miss a regulator the GPU isn't going to work properly (some cores won't be able to power up successfully). However at the moment the driver will happily do this if someone provides it with a DT which includes regulators that it doesn't know about. So I'm not sure how adding special case code for a SoC would help here. > Steve, please fix your mail client to word wrap within paragraphs at > something substantially less than 80 columns. Doing this makes your > messages much easier to read and reply to. Sorry about that - I switched to my laptop to escape the noisy work going on outside the office, and apparently that was misconfigured. Hopefully fixed now, thanks for letting me know! Steve