From: Nicolas Boichat <drinkcat@chromium.org> To: Steven Price <steven.price@arm.com> Cc: Rob Herring <robh@kernel.org>, Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>, Hsin-Yi Wang <hsinyi@chromium.org>, Kristian Kristensen <hoegsberg@chromium.org>, Fei Shao <fshao@chromium.org>, Boris Brezillon <boris.brezillon@collabora.com>, Daniel Vetter <daniel@ffwll.ch>, David Airlie <airlied@linux.ie>, Tomeu Vizoso <tomeu.vizoso@collabora.com>, dri-devel <dri-devel@lists.freedesktop.org>, lkml <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v6 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1 Date: Fri, 8 Jan 2021 07:51:29 +0800 [thread overview] Message-ID: <CANMq1KCbA9FawgtK4+rjBKO=dFomrAt7kpN8K_180mHRAeUkAg@mail.gmail.com> (raw) In-Reply-To: <a373044b-3e24-4e00-432f-46b0bd91b213@arm.com> On Thu, Jan 7, 2021 at 11:59 PM Steven Price <steven.price@arm.com> wrote: > > On 05/01/2021 00:11, Nicolas Boichat wrote: > > GPUs with more than a single regulator (e.g. G-57 on MT8183) will > > require platform-specific handling, disable devfreq for now. > > Can you explain what actually goes wrong here? AFAICT the existing code > does support controlling multiple regulators - but clearly this is the > first platform that exercises that code with num_supplies>1. Sure, I should have expanded in the commit message, will do in v9. We have support for >1 supplies, and we need to enable them to get the GPU running _at all_ (and the default voltages should be safe by design). For devfreq though: 1. There are constraints on the voltage difference between the core and SRAM, we have this caterpillar logic downstream [1], so somebody will need to port it (TBH I don't think it's overly critical at this point, as Bifrost support is still not very mature from what I can see, and devfreq is purely a performance thing). 2. The core [2] does not support multiple regulators, so we'll need custom code anyway. Even if we didn't have constraints. [1] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/chromeos-4.19/drivers/gpu/arm/bifrost/platform/mediatek/mali_kbase_runtime_pm.c#367 [2] https://elixir.bootlin.com/linux/latest/source/drivers/opp/core.c#L679 > > Steve > > > > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > > --- > > > > Changes in v6: > > - New change > > > > drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > > index f44d28fad085..1f49043aae73 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > > @@ -92,6 +92,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) > > struct thermal_cooling_device *cooling; > > struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq; > > > > + if (pfdev->comp->num_supplies > 1) { > > + /* > > + * GPUs with more than 1 supply require platform-specific handling: > > + * continue without devfreq > > + */ > > + DRM_DEV_ERROR(dev, "More than 1 supply is not supported yet\n"); > > + return 0; > > + } > > + > > opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names, > > pfdev->comp->num_supplies); > > if (IS_ERR(opp_table)) { > > >
WARNING: multiple messages have this Message-ID (diff)
From: Nicolas Boichat <drinkcat@chromium.org> To: Steven Price <steven.price@arm.com> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>, Fei Shao <fshao@chromium.org>, David Airlie <airlied@linux.ie>, lkml <linux-kernel@vger.kernel.org>, dri-devel <dri-devel@lists.freedesktop.org>, Boris Brezillon <boris.brezillon@collabora.com>, Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>, Hsin-Yi Wang <hsinyi@chromium.org>, Kristian Kristensen <hoegsberg@chromium.org> Subject: Re: [PATCH v6 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1 Date: Fri, 8 Jan 2021 07:51:29 +0800 [thread overview] Message-ID: <CANMq1KCbA9FawgtK4+rjBKO=dFomrAt7kpN8K_180mHRAeUkAg@mail.gmail.com> (raw) In-Reply-To: <a373044b-3e24-4e00-432f-46b0bd91b213@arm.com> On Thu, Jan 7, 2021 at 11:59 PM Steven Price <steven.price@arm.com> wrote: > > On 05/01/2021 00:11, Nicolas Boichat wrote: > > GPUs with more than a single regulator (e.g. G-57 on MT8183) will > > require platform-specific handling, disable devfreq for now. > > Can you explain what actually goes wrong here? AFAICT the existing code > does support controlling multiple regulators - but clearly this is the > first platform that exercises that code with num_supplies>1. Sure, I should have expanded in the commit message, will do in v9. We have support for >1 supplies, and we need to enable them to get the GPU running _at all_ (and the default voltages should be safe by design). For devfreq though: 1. There are constraints on the voltage difference between the core and SRAM, we have this caterpillar logic downstream [1], so somebody will need to port it (TBH I don't think it's overly critical at this point, as Bifrost support is still not very mature from what I can see, and devfreq is purely a performance thing). 2. The core [2] does not support multiple regulators, so we'll need custom code anyway. Even if we didn't have constraints. [1] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/chromeos-4.19/drivers/gpu/arm/bifrost/platform/mediatek/mali_kbase_runtime_pm.c#367 [2] https://elixir.bootlin.com/linux/latest/source/drivers/opp/core.c#L679 > > Steve > > > > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > > --- > > > > Changes in v6: > > - New change > > > > drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > > index f44d28fad085..1f49043aae73 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > > @@ -92,6 +92,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) > > struct thermal_cooling_device *cooling; > > struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq; > > > > + if (pfdev->comp->num_supplies > 1) { > > + /* > > + * GPUs with more than 1 supply require platform-specific handling: > > + * continue without devfreq > > + */ > > + DRM_DEV_ERROR(dev, "More than 1 supply is not supported yet\n"); > > + return 0; > > + } > > + > > opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names, > > pfdev->comp->num_supplies); > > if (IS_ERR(opp_table)) { > > > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2021-01-07 23:52 UTC|newest] Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-01-05 0:11 [PATCH v6 0/4] drm/panfrost: Add support for mt8183 GPU Nicolas Boichat 2021-01-05 0:11 ` Nicolas Boichat 2021-01-05 0:11 ` Nicolas Boichat 2021-01-05 0:11 ` Nicolas Boichat 2021-01-05 0:11 ` [PATCH v6 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 Nicolas Boichat 2021-01-05 0:11 ` Nicolas Boichat 2021-01-05 0:11 ` Nicolas Boichat 2021-01-05 0:11 ` Nicolas Boichat 2021-01-05 0:11 ` [PATCH v6 2/4] arm64: dts: mt8183: Add node for the Mali GPU Nicolas Boichat 2021-01-05 0:11 ` Nicolas Boichat 2021-01-05 0:11 ` Nicolas Boichat 2021-01-05 0:11 ` [PATCH v6 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1 Nicolas Boichat 2021-01-05 0:11 ` Nicolas Boichat 2021-01-05 0:34 ` Alyssa Rosenzweig 2021-01-05 0:34 ` Alyssa Rosenzweig 2021-01-05 0:59 ` Nicolas Boichat 2021-01-05 0:59 ` Nicolas Boichat 2021-01-07 15:59 ` Steven Price 2021-01-07 15:59 ` Steven Price 2021-01-07 23:51 ` Nicolas Boichat [this message] 2021-01-07 23:51 ` Nicolas Boichat 2021-01-05 0:11 ` [PATCH v6 4/4] drm/panfrost: Add mt8183-mali compatible string Nicolas Boichat 2021-01-05 0:11 ` Nicolas Boichat 2021-01-05 0:11 ` Nicolas Boichat 2021-01-05 0:11 ` Nicolas Boichat 2021-01-05 0:35 ` Alyssa Rosenzweig 2021-01-05 0:35 ` Alyssa Rosenzweig 2021-01-05 0:35 ` Alyssa Rosenzweig 2021-01-05 0:35 ` Alyssa Rosenzweig
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='CANMq1KCbA9FawgtK4+rjBKO=dFomrAt7kpN8K_180mHRAeUkAg@mail.gmail.com' \ --to=drinkcat@chromium.org \ --cc=airlied@linux.ie \ --cc=alyssa.rosenzweig@collabora.com \ --cc=boris.brezillon@collabora.com \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=fshao@chromium.org \ --cc=hoegsberg@chromium.org \ --cc=hsinyi@chromium.org \ --cc=linux-kernel@vger.kernel.org \ --cc=robh@kernel.org \ --cc=steven.price@arm.com \ --cc=tomeu.vizoso@collabora.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.