Re: [PATCH 2/2] drm/panfrost: add devfreq regulator support

From: ClÃment PÃron
Date: Tue Apr 14 2020 - 15:26:01 EST


Hi Mark,

On Tue, 14 Apr 2020 at 20:55, Mark Brown <broonie@xxxxxxxxxx> wrote:
>
> On Tue, Apr 14, 2020 at 08:20:23PM +0200, ClÃment PÃron wrote:
> > Hi Liam and Mark,
>
> You might want to flag stuff like this in the subject line, I very
> nearly deleted this without opening it since most of the email I get
> about panfrost appears to be coming from me having sent patches rather
> than being relevant.

Ok will do next time,

>
> > We are having an issue with Panfrost driver registering two times the
> > same regulator and giving an error when trying to create the debugfs
> > folder.
>
> > Could you clarify if it is allowed for a device to register two times
> > the same regulator?
>
> > I check Documentation/power/regulator/regulator.rst but this point is
> > not specified.
>
> We don't actively prevent it and I can't think what other than debugfs
> might run into problems (and that's just a warning) but it does seem
> like a weird thing to want to do and like it's pointing to some
> confusion in your code with two different parts of the device
> controlling the same supply independently. What's the use case here?

Panfrost first probe clock, reset and regulator in device_init:
https://elixir.bootlin.com/linux/v5.7-rc1/source/drivers/gpu/drm/panfrost/panfrost_drv.c#L602
Then it probe for optional devfreq, devfreq will get opp tables and
also get regulator again.
https://elixir.bootlin.com/linux/v5.7-rc1/source/drivers/gpu/drm/panfrost/panfrost_drv.c#L609

That's can be reworked and Panfrost can only probe regulator if there
is no opp-table.

But if multiple regulator is not an issue and as each request is logic.
The first in device_init assure to enable the regulator and the second
in OPP assure the voltage level.

Maybe we can just fix this warning?

Thanks,
Clement