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=-7.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,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 04D88C41537 for ; Wed, 18 Aug 2021 05:58:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D52EB610A3 for ; Wed, 18 Aug 2021 05:58:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237976AbhHRF72 (ORCPT ); Wed, 18 Aug 2021 01:59:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42668 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237987AbhHRF70 (ORCPT ); Wed, 18 Aug 2021 01:59:26 -0400 Received: from mail-pj1-x1030.google.com (mail-pj1-x1030.google.com [IPv6:2607:f8b0:4864:20::1030]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2FC16C0617AF for ; Tue, 17 Aug 2021 22:58:52 -0700 (PDT) Received: by mail-pj1-x1030.google.com with SMTP id w13-20020a17090aea0db029017897a5f7bcso1583512pjy.5 for ; Tue, 17 Aug 2021 22:58:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=/oqvyX1+fZTz+wtMw2AZVqGvrykbTQj8oIvIXJk4fnQ=; b=BiNyX/6IYojCpnpyPGFzrgnh8ilV1S48JvAdE+mM4BuP7CRu2FKtWLmHb+6pMAnjSh /wfP9QCG/Q2Ukj1ZByvNzYSCb76F2+QCOquffMB7/VgJzMfI6biDam1D93oJFOcKLxY5 r0ui6XTLNwvZZ2QgeJ+nHG0xrCIvTYYP76CrkUOXqGHCkZrCx8j9IwfpGrJOMhMeq8y0 k2mkEWBfMzJhbTlNZAz6IL2S8V/0dB0U3qYFm8ylkevVbHpEgfdJ2weIyvMA0dAHIkMr BJ+3m2Wz3q/+kbkLCXBH7Ci4zKCVmfsRmjz+f2zh3gygpbMTckDnXDsC8v0GIrUYRu/5 PVGA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=/oqvyX1+fZTz+wtMw2AZVqGvrykbTQj8oIvIXJk4fnQ=; b=QrK3dJ47aMWoFV5YIG/64I14WHW8rTaAzilpCKd65XPQSjtJugMiDuA//lF4kxh+dm tQ6Hn0vPv95Lp2K338EDKkg7OmjcZ1CMe59BmU21NnmLLfA9Z1Gj+mEs5rgExhcMEo6D wuS55FLtkyfI7Vkoo4crBBn6ksEoe8d0UAGcFe46JjIFiSvJtSL4nDhwIHkdtYPyguf/ VN4BV0mahEhTAUMdzFMhE9CgL2fDxJUDSG/JmSqzOHk3saqcJIPh43aO0WnA/r3XuB7h hX6e7OXdBHEaw/h/rT2sOp//XKgfA2BmBTHi2aaSPowko9TVjIi4AKay1c+JJDzD5LyK bP5A== X-Gm-Message-State: AOAM5304Q4V6vn6PiXvMemAUdoQcj+qRwXLaqToZr4EiMrVU3Ay8Yq2D MdoXhK8rpaJj+d9Myy/9P0oNGQ== X-Google-Smtp-Source: ABdhPJxmfawJmjScyveR+qRjue44WOBbTuswMGRMZAcW5NbsgTVLKIK8N3zHMPdQxwr8K+gezRoAzQ== X-Received: by 2002:a17:90a:db89:: with SMTP id h9mr7820049pjv.214.1629266331439; Tue, 17 Aug 2021 22:58:51 -0700 (PDT) Received: from localhost ([122.172.201.85]) by smtp.gmail.com with ESMTPSA id g14sm4668532pfr.31.2021.08.17.22.58.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 17 Aug 2021 22:58:51 -0700 (PDT) Date: Wed, 18 Aug 2021 11:28:49 +0530 From: Viresh Kumar To: Dmitry Osipenko Cc: Thierry Reding , Jonathan Hunter , Ulf Hansson , Viresh Kumar , Stephen Boyd , Peter De Schrijver , Mikko Perttunen , Peter Chen , Mark Brown , Lee Jones , Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= , Nishanth Menon , Vignesh Raghavendra , Richard Weinberger , Miquel Raynal , Lucas Stach , Stefan Agner , Adrian Hunter , Mauro Carvalho Chehab , Rob Herring , Michael Turquette , linux-kernel@vger.kernel.org, linux-tegra@vger.kernel.org, linux-pm@vger.kernel.org, linux-usb@vger.kernel.org, linux-staging@lists.linux.dev, linux-spi@vger.kernel.org, linux-pwm@vger.kernel.org, linux-mtd@lists.infradead.org, linux-mmc@vger.kernel.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org, linux-clk@vger.kernel.org Subject: Re: [PATCH v8 01/34] opp: Add dev_pm_opp_sync() helper Message-ID: <20210818055849.ybfajzu75ecpdrbn@vireshk-i7> References: <20210817012754.8710-1-digetx@gmail.com> <20210817012754.8710-2-digetx@gmail.com> <20210817075515.vyyv7z37e6jcrhsl@vireshk-i7> <710261d9-7ae3-5155-c0a2-f8aed2408d0b@gmail.com> <20210818035533.ieqkexltfvvf2p4n@vireshk-i7> <5b2a80c1-9743-e633-6257-ede94c8a274c@gmail.com> <20210818043131.7klajx6drvvkftoc@vireshk-i7> <20210818045307.4brb6cafkh3adjth@vireshk-i7> <080469b3-612b-3a34-86e5-7037a64de2fe@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <080469b3-612b-3a34-86e5-7037a64de2fe@gmail.com> User-Agent: NeoMutt/20180716-391-311a52 Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On 18-08-21, 08:21, Dmitry Osipenko wrote: > Yes, GENPD will cache the perf state across suspend/resume and initially > cached value is out of sync with h/w. > > Nothing else. But let me clarify it all again. Thanks for your explanation. > Initially the performance state of all GENPDs is 0 for all devices. > > The clock rate is preinitialized for all devices to a some default rate > by clk driver, or by bootloader or by assigned-clocks in DT. > > When device is rpm-resumed, the resume callback of a device driver > enables the clock. > > Before clock is enabled, the voltage needs to be configured in > accordance to the clk rate. > > So now we have a GENPD with pstate=0 on a first rpm-resume, which > doesn't match the h/w configuration. Calling dev_pm_opp_sync() sets the > pstate in accordance to the h/w config. What about calling dev_pm_opp_set_rate(dev, clk_get_rate(dev)) here instead ? That will work, right ? The advantage is it works without any special routine to do so. I also wonder looking at your gr3d.c changes, you set a set-opp helper, but the driver doesn't call set_opp_rate at all. Who calls it ? And if it is all about just syncing the genpd core, then can the genpd core do something like what clk framework does? i.e. allow a new optional genpd callback, get_performance_state() (just like set_performance_state()), which can be called initially by the core to get the performance to something other than zero. opp-set-rate is there to set the performance state and enable the stuff as well. That's why it looks incorrect in your case, where the function was only required to be called once, and you are ending up calling it on each resume. Limiting that with another local variable is bad as well. > In a previous v7 I proposed to preset the rpm_pstate of GENPD (perf > level that is restored before device is rpm-resumed) from PD's > attach_dev callback, but Ulf didn't like that because it requires to use > and modify GENPD 'private' variables from a PD driver. We decided that > will be better to make device drivers to explicitly sync the perf state, > which I implemented in this v8. -- viresh