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 X-Spam-Level: X-Spam-Status: No, score=-2.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E0D6BC33CAE for ; Mon, 13 Jan 2020 17:10:28 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id BA45D2084D for ; Mon, 13 Jan 2020 17:10:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BA45D2084D Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 373FD89DA8; Mon, 13 Jan 2020 17:10:28 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by gabe.freedesktop.org (Postfix) with ESMTP id 5941989DA8 for ; Mon, 13 Jan 2020 17:10:26 +0000 (UTC) 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 98A4C11B3; Mon, 13 Jan 2020 09:10:25 -0800 (PST) Received: from [10.1.194.52] (e112269-lin.cambridge.arm.com [10.1.194.52]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 48CE13F534; Mon, 13 Jan 2020 09:10:24 -0800 (PST) Subject: Re: [PATCH RFT v1 3/3] drm/panfrost: Use the mali-supply regulator for control again To: Martin Blumenstingl References: <20200107230626.885451-1-martin.blumenstingl@googlemail.com> <20200107230626.885451-4-martin.blumenstingl@googlemail.com> <2ceffe46-57a8-79a8-2c41-d04b227d3792@arm.com> From: Steven Price Message-ID: <21d0730b-8299-8bfd-4321-746ccb3772d0@arm.com> Date: Mon, 13 Jan 2020 17:10:23 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.3.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: tomeu.vizoso@collabora.com, airlied@linux.ie, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-rockchip@lists.infradead.org, linux-amlogic@lists.infradead.org, robin.murphy@arm.com, alyssa@rosenzweig.io Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 09/01/2020 17:27, Martin Blumenstingl wrote: > On Thu, Jan 9, 2020 at 12:31 PM Steven Price wrote: >> >> On 07/01/2020 23:06, Martin Blumenstingl wrote: >>> dev_pm_opp_set_rate() needs a reference to the regulator which should be >>> updated when updating the GPU frequency. The name of the regulator has >>> to be passed at initialization-time using dev_pm_opp_set_regulators(). >>> Add the call to dev_pm_opp_set_regulators() so dev_pm_opp_set_rate() >>> will update the GPU regulator when updating the frequency (just like >>> we did this manually before when we open-coded dev_pm_opp_set_rate()). >> >> This patch causes a warning from debugfs on my firefly (RK3288) board: >> >> debugfs: Directory 'ffa30000.gpu-mali' with parent 'vdd_gpu' already >> present! >> >> So it looks like the regulator is being added twice - but I haven't >> investigated further. > I *think* it's because the regulator is already fetched by the > panfrost driver itself to enable it > (the devfreq code currently does not support enabling the regulator, > it can only control the voltage) > > I'm not sure what to do about this though Having a little play around with this, I think you can simply remove the panfrost_regulator_init() call. This at least works for me - the call to dev_pm_opp_set_regulators() seems to set everything up. However I suspect you need to do this unconditionally even if there are no operating points defined. > [...] >>> ret = dev_pm_opp_of_add_table(dev); >>> - if (ret) >>> + if (ret) { >>> + dev_pm_opp_put_regulators(pfdev->devfreq.regulators_opp_table); >> >> If we don't have a regulator then regulators_opp_table will be NULL and >> sadly dev_pm_opp_put_regulators() doesn't handle a NULL argument. The >> same applies to the two below calls obviously. > good catch, thank you! > are you happy with the general approach here or do you think that > dev_pm_opp_set_regulators is the wrong way to go (for whatever > reason)? To be honest this is an area I still don't fully understand. There's a lot of magic helper functions and very little in the way of helpful documentation to work out which are the right ones to call. It seems reasonable to me, hopefully someone more in the know will chime in it there's something fundamentally wrong! Thanks, Steve _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel