| Message ID | 20210105081111.v6.3.I3af068abe30c9c85cabc4486385c52e56527a509@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/panfrost: Add support for mt8183 GPU | expand |
> GPUs with more than a single regulator (e.g. G-57 on MT8183) will
G72
On Tue, Jan 5, 2021 at 8:34 AM Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> 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.
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 <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)) { >
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)) { > > >
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)) {
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 <drinkcat@chromium.org> --- Changes in v6: - New change drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 +++++++++ 1 file changed, 9 insertions(+)