Re: [PATCH v6 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
On Thu, Jan 7, 2021 at 11:59 PM Steven Price 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
> > ---
> >
> > 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 = >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)) {
> >
>
Re: [PATCH v6 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
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.
Steve
Signed-off-by: Nicolas Boichat
---
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 = >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)) {
Re: [PATCH v6 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
On Tue, Jan 5, 2021 at 8:34 AM Alyssa Rosenzweig wrote: > > > GPUs with more than a single regulator (e.g. G-57 on MT8183) will > > G72 Duh, sorry, yes. I will fix that in v7.
Re: [PATCH v6 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
> GPUs with more than a single regulator (e.g. G-57 on MT8183) will G72 signature.asc Description: PGP signature
[PATCH v6 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
GPUs with more than a single regulator (e.g. G-57 on MT8183) will
require platform-specific handling, disable devfreq for now.
Signed-off-by: Nicolas Boichat
---
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 = >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)) {
--
2.29.2.729.g45daf8777d-goog
