LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Thierry Reding <thierry.reding@gmail.com>
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: "Dmitry Osipenko" <digetx@gmail.com>,
	"Jonathan Hunter" <jonathanh@nvidia.com>,
	"Viresh Kumar" <vireshk@kernel.org>,
	"Stephen Boyd" <sboyd@kernel.org>,
	"Peter De Schrijver" <pdeschrijver@nvidia.com>,
	"Mikko Perttunen" <mperttunen@nvidia.com>,
	"Peter Chen" <peter.chen@kernel.org>,
	"Mark Brown" <broonie@kernel.org>,
	"Lee Jones" <lee.jones@linaro.org>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Nishanth Menon" <nm@ti.com>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	"Richard Weinberger" <richard@nod.at>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Lucas Stach" <dev@lynxeye.de>, "Stefan Agner" <stefan@agner.ch>,
	"Adrian Hunter" <adrian.hunter@intel.com>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Michael Turquette" <mturquette@baylibre.com>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	linux-tegra <linux-tegra@vger.kernel.org>,
	"Linux PM" <linux-pm@vger.kernel.org>,
	"Linux USB List" <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 <linux-mmc@vger.kernel.org>,
	"Linux Media Mailing List" <linux-media@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	DTML <devicetree@vger.kernel.org>,
	linux-clk <linux-clk@vger.kernel.org>
Subject: Re: [PATCH v8 11/34] gpu: host1x: Add runtime PM and OPP support
Date: Tue, 17 Aug 2021 16:02:59 +0200	[thread overview]
Message-ID: <YRvBkyfFCqthBIBV@orome.fritz.box> (raw)
In-Reply-To: <CAPDyKFrax-EYtO03W5QWM2tcWLWeMM8hHZCRYFcsenuiP2zObQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 5172 bytes --]

On Tue, Aug 17, 2021 at 02:04:38PM +0200, Ulf Hansson wrote:
> On Tue, 17 Aug 2021 at 03:30, Dmitry Osipenko <digetx@gmail.com> wrote:
> >
> > Add runtime PM and OPP support to the Host1x driver. It's required for
> > enabling system-wide DVFS and supporting dynamic power management using
> > a generic power domain. For the starter we will keep host1x always-on
> > because dynamic power management require a major refactoring of the driver
> > code since lot's of code paths will need the RPM handling and we're going
> > to remove some of these paths in the future. Host1x doesn't consume much
> > power so it is good enough, we at least need to resume Host1x in order
> > to initialize the power state.
> >
> > Tested-by: Peter Geis <pgwipeout@gmail.com> # Ouya T30
> > Tested-by: Paul Fertser <fercerpav@gmail.com> # PAZ00 T20
> > Tested-by: Nicolas Chauvet <kwizart@gmail.com> # PAZ00 T20 and TK1 T124
> > Tested-by: Matt Merhar <mattmerhar@protonmail.com> # Ouya T30
> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> > ---
> 
> [...]
> 
> > +
> >  static int host1x_probe(struct platform_device *pdev)
> >  {
> >         struct host1x *host;
> > @@ -394,6 +423,10 @@ static int host1x_probe(struct platform_device *pdev)
> >         /* set common host1x device data */
> >         platform_set_drvdata(pdev, host);
> >
> > +       err = devm_tegra_core_dev_init_opp_table_simple(&pdev->dev);
> > +       if (err)
> > +               return err;
> > +
> >         host->regs = devm_ioremap_resource(&pdev->dev, regs);
> >         if (IS_ERR(host->regs))
> >                 return PTR_ERR(host->regs);
> > @@ -423,12 +456,9 @@ static int host1x_probe(struct platform_device *pdev)
> >                 return err;
> >         }
> >
> > -       host->rst = devm_reset_control_get(&pdev->dev, "host1x");
> > -       if (IS_ERR(host->rst)) {
> > -               err = PTR_ERR(host->rst);
> > -               dev_err(&pdev->dev, "failed to get reset: %d\n", err);
> > +       err = host1x_get_resets(host);
> > +       if (err)
> >                 return err;
> > -       }
> >
> >         err = host1x_iommu_init(host);
> >         if (err < 0) {
> > @@ -443,22 +473,10 @@ static int host1x_probe(struct platform_device *pdev)
> >                 goto iommu_exit;
> >         }
> >
> > -       err = clk_prepare_enable(host->clk);
> > -       if (err < 0) {
> > -               dev_err(&pdev->dev, "failed to enable clock\n");
> > -               goto free_channels;
> > -       }
> > -
> > -       err = reset_control_deassert(host->rst);
> > -       if (err < 0) {
> > -               dev_err(&pdev->dev, "failed to deassert reset: %d\n", err);
> > -               goto unprepare_disable;
> > -       }
> > -
> 
> Removing the clk_prepare_enable() and reset_control_deassert() from
> host1x_probe(), might not be a good idea. See more about why, below.
> 
> >         err = host1x_syncpt_init(host);
> >         if (err) {
> >                 dev_err(&pdev->dev, "failed to initialize syncpts\n");
> > -               goto reset_assert;
> > +               goto free_channels;
> >         }
> >
> >         err = host1x_intr_init(host, syncpt_irq);
> > @@ -467,10 +485,14 @@ static int host1x_probe(struct platform_device *pdev)
> >                 goto deinit_syncpt;
> >         }
> >
> > -       host1x_debug_init(host);
> > +       pm_runtime_enable(&pdev->dev);
> >
> > -       if (host->info->has_hypervisor)
> > -               host1x_setup_sid_table(host);
> > +       /* the driver's code isn't ready yet for the dynamic RPM */
> > +       err = pm_runtime_resume_and_get(&pdev->dev);
> 
> If the driver is being built with the CONFIG_PM Kconfig option being
> unset, pm_runtime_resume_and_get() will return 0 to indicate success -
> and without calling the ->runtime_resume() callback.
> In other words, the clock will remain gated and the reset will not be
> deasserted, likely causing the driver to be malfunctioning.
> 
> If the driver isn't ever being built with CONFIG_PM unset, feel free
> to ignore my above comments.
> 
> Otherwise, if it needs to work both with and without CONFIG_PM being
> set, you may use the following pattern in host1x_probe() to deploy
> runtime PM support:
> 
> "Enable the needed resources to probe the device"
> pm_runtime_get_noresume()
> pm_runtime_set_active()
> pm_runtime_enable()
> 
> "Before successfully completing probe"
> pm_runtime_put()

We made a conscious decision a few years ago to have ARCH_TEGRA select
PM on both 32-bit and 64-bit ARM, specifically to avoid the need to do
this dance (though there are still a few drivers left that do this, I
think).

So I think this should be unnecessary. Unless perhaps if the sysfs PM
controls have any influence on this. As far as I know, as long as the
PM kconfig option is enabled, the sysfs control only influence the
runtime behaviour (i.e. setting the sysfs PM control to "on" is going
to force runtime PM to be resumed) but there's no way to disable
runtime PM altogether via sysfs that would make the above necessary.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2021-08-17 14:03 UTC|newest]

Thread overview: 119+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-17  1:27 [PATCH v8 00/34] NVIDIA Tegra power management patches for 5.16 Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 01/34] opp: Add dev_pm_opp_sync() helper Dmitry Osipenko
2021-08-17  7:55   ` Viresh Kumar
2021-08-17 15:49     ` Dmitry Osipenko
2021-08-18  3:55       ` Viresh Kumar
2021-08-18  4:12         ` Dmitry Osipenko
2021-08-18  4:29           ` Dmitry Osipenko
2021-08-18  4:30             ` Dmitry Osipenko
2021-08-18  4:34               ` Viresh Kumar
2021-08-18  4:31           ` Viresh Kumar
2021-08-18  4:37             ` Dmitry Osipenko
2021-08-18  4:53               ` Viresh Kumar
2021-08-18  5:21                 ` Dmitry Osipenko
2021-08-18  5:58                   ` Viresh Kumar
2021-08-18  6:00                     ` Viresh Kumar
2021-08-18  6:22                     ` Dmitry Osipenko
2021-08-18  6:27                       ` Viresh Kumar
2021-08-18  8:29                         ` Ulf Hansson
2021-08-18  9:14                           ` Viresh Kumar
2021-08-18  9:41                             ` Ulf Hansson
2021-08-18  9:42                               ` Ulf Hansson
2021-08-18  9:50                               ` Viresh Kumar
2021-08-18 10:08                                 ` Ulf Hansson
2021-08-18 15:43                                   ` Dmitry Osipenko
2021-08-18 15:46                                     ` Dmitry Osipenko
2021-08-19 13:07                                     ` Ulf Hansson
2021-08-19 19:35                                       ` Dmitry Osipenko
2021-08-20  5:07                                         ` Viresh Kumar
2021-08-20 12:42                                         ` Ulf Hansson
2021-08-21 17:34                                           ` Dmitry Osipenko
2021-08-23 10:46                                             ` Ulf Hansson
2021-08-23 15:54                                               ` Dmitry Osipenko
2021-08-18 15:55                               ` Dmitry Osipenko
2021-08-19  6:16                                 ` Viresh Kumar
2021-08-19 14:55                                   ` Ulf Hansson
2021-08-20  5:18                                     ` Viresh Kumar
2021-08-20 12:57                                       ` Ulf Hansson
2021-08-23 20:24                                         ` Dmitry Osipenko
2021-08-24  3:04                                           ` Viresh Kumar
2021-08-22 18:35                                       ` Dmitry Osipenko
2021-08-25 15:41                                         ` Dmitry Osipenko
2021-08-26  2:54                                           ` Viresh Kumar
2021-08-26  2:55                                             ` Viresh Kumar
2021-08-17  1:27 ` [PATCH v8 02/34] soc/tegra: pmc: Disable PMC state syncing Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 03/34] soc/tegra: Don't print error message when OPPs not available Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 04/34] soc/tegra: Add devm_tegra_core_dev_init_opp_table_simple() Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 05/34] soc/tegra: Use dev_pm_opp_sync() Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 06/34] dt-bindings: clock: tegra-car: Document new tegra-clocks sub-node Dmitry Osipenko
2021-08-18  1:15   ` Rob Herring
2021-08-18  1:44     ` Dmitry Osipenko
2021-08-18 13:52       ` Thierry Reding
2021-08-18 15:04         ` Dmitry Osipenko
2021-08-18 13:59   ` Thierry Reding
2021-08-18 15:05     ` Dmitry Osipenko
2021-08-18 16:39       ` Thierry Reding
2021-08-18 16:57         ` Dmitry Osipenko
2021-08-18 17:16           ` Dmitry Osipenko
2021-08-19 16:31           ` Thierry Reding
2021-08-19 22:20             ` Dmitry Osipenko
2021-08-20  2:51             ` Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 07/34] clk: tegra: Support runtime PM and power domain Dmitry Osipenko
2021-08-18 14:07   ` Thierry Reding
2021-08-18 15:05     ` Dmitry Osipenko
2021-08-18 16:42       ` Thierry Reding
2021-08-18 17:11         ` Dmitry Osipenko
2021-08-19 16:54           ` Thierry Reding
2021-08-19 22:09             ` Dmitry Osipenko
2021-08-20 11:42               ` Thierry Reding
2021-08-20 13:08                 ` Ulf Hansson
2021-08-21 17:45                   ` Dmitry Osipenko
2021-08-23 14:33                     ` Thierry Reding
2021-08-23 18:54                       ` Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 08/34] dt-bindings: host1x: Document OPP and power domain properties Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 09/34] dt-bindings: host1x: Document Memory Client resets of Host1x, GR2D and GR3D Dmitry Osipenko
2021-08-18  1:16   ` Rob Herring
2021-08-18  1:37     ` Dmitry Osipenko
2021-08-18  2:04       ` Dmitry Osipenko
2021-08-18  2:07         ` Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 10/34] gpu: host1x: Add host1x_channel_stop() Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 11/34] gpu: host1x: Add runtime PM and OPP support Dmitry Osipenko
2021-08-17 12:04   ` Ulf Hansson
2021-08-17 14:02     ` Thierry Reding [this message]
2021-08-18  8:35       ` Ulf Hansson
2021-08-18 17:24         ` Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 12/34] drm/tegra: dc: Support OPP and SoC core voltage scaling Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 13/34] drm/tegra: hdmi: Add OPP support Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 14/34] drm/tegra: gr2d: Support power management Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 15/34] drm/tegra: gr3d: " Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 16/34] drm/tegra: vic: Support system suspend Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 17/34] usb: chipidea: tegra: Add runtime PM and OPP support Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 18/34] bus: tegra-gmi: " Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 19/34] pwm: tegra: " Dmitry Osipenko
2021-08-19 13:21   ` Thierry Reding
2021-08-19 14:04     ` Ulf Hansson
2021-08-19 16:17       ` Thierry Reding
2021-08-17  1:27 ` [PATCH v8 20/34] mmc: sdhci-tegra: " Dmitry Osipenko
2021-08-19 17:03   ` Thierry Reding
2021-08-19 22:37     ` Dmitry Osipenko
2021-08-20 11:35       ` Thierry Reding
2021-08-25  9:45         ` Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 21/34] mtd: rawnand: tegra: " Dmitry Osipenko
2021-08-17  8:41   ` Miquel Raynal
2021-08-17  1:27 ` [PATCH v8 22/34] spi: tegra20-slink: Add " Dmitry Osipenko
2021-08-17 12:22   ` Mark Brown
2021-08-17 15:53     ` Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 23/34] media: dt: bindings: tegra-vde: Convert to schema Dmitry Osipenko
2021-08-18  1:17   ` Rob Herring
2021-08-17  1:27 ` [PATCH v8 24/34] media: dt: bindings: tegra-vde: Document OPP and power domain Dmitry Osipenko
2021-08-18  1:17   ` Rob Herring
2021-08-17  1:27 ` [PATCH v8 25/34] media: staging: tegra-vde: Support generic power domain and OPP Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 26/34] soc/tegra: fuse: Add OPP support Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 27/34] soc/tegra: fuse: Reset hardware Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 28/34] soc/tegra: regulators: Prepare for suspend Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 29/34] soc/tegra: pmc: Enable core domain support for Tegra20 and Tegra30 Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 30/34] ARM: tegra: Add OPP tables and power domains to Tegra20 device-trees Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 31/34] ARM: tegra: Add OPP tables and power domains to Tegra30 device-trees Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 32/34] ARM: tegra: Add Memory Client resets to Tegra20 GR2D, GR3D and Host1x Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 33/34] ARM: tegra: Add Memory Client resets to Tegra30 " Dmitry Osipenko
2021-08-17  1:27 ` [PATCH v8 34/34] ARM: tegra20/30: Disable unused host1x hardware Dmitry Osipenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YRvBkyfFCqthBIBV@orome.fritz.box \
    --to=thierry.reding@gmail.com \
    --cc=adrian.hunter@intel.com \
    --cc=broonie@kernel.org \
    --cc=dev@lynxeye.de \
    --cc=devicetree@vger.kernel.org \
    --cc=digetx@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jonathanh@nvidia.com \
    --cc=lee.jones@linaro.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=linux-tegra@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=mperttunen@nvidia.com \
    --cc=mturquette@baylibre.com \
    --cc=nm@ti.com \
    --cc=pdeschrijver@nvidia.com \
    --cc=peter.chen@kernel.org \
    --cc=richard@nod.at \
    --cc=robh+dt@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=stefan@agner.ch \
    --cc=u.kleine-koenig@pengutronix.de \
    --cc=ulf.hansson@linaro.org \
    --cc=vigneshr@ti.com \
    --cc=vireshk@kernel.org \
    --subject='Re: [PATCH v8 11/34] gpu: host1x: Add runtime PM and OPP support' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).