Re: [PATCH] devfreq: Register devfreq as a cooling device

[Lukasz Luba]

On 3/4/21 4:53 PM, Daniel Lezcano wrote:
> Hi Lukasz,
>
> thanks for commenting this patch,
>
> On 04/03/2021 14:47, Lukasz Luba wrote:
> > Hi Daniel,
> >
> > On 3/4/21 12:50 PM, Daniel Lezcano wrote:
> > > Currently the default behavior is to manually having the devfreq
> > > backend to register themselves as a devfreq cooling device.
> > >
> > > There are no so many and actually it makes more sense to register the
> > > devfreq device when adding it.
> > >
> > > Consequently, every devfreq becomes a cooling device like cpufreq is.
> > >
> > > Having a devfreq being registered as a cooling device can not mitigate
> > > a thermal zone if it is not bound to this one. Thus, the current
> > > configurations are not impacted by this change.
> >
> > There are also different type of devices, which register into devfreq
> > framework like NoC buses, UFS/eMMC, jpeg and video accelerators, ISP,
> > etc.
> > In some platforms there are plenty of those devices and they all would
> > occupy memory due to private freq_table in devfreq_cooling, function:
> > devfreq_cooling_gen_tables().
> >
> > IIRC in OdroidXU4 there are ~20 devfreq devs for NoC buses.
>
> I'm curious, do you have a pointer to such kernels having all those
> devfreq ?

Sure, it's mainline code, you can build it with exynos_defconfig.
You can check the DT files to find them arch/arm/boot/dts/exynos*.
(this particular OdroidXU4 is Exynos5422, but it grabs some generic dt
files).

Here is the mainline kernel content of /sys/class/devfreq/
----------------------------------------------------------
sys/class/devfreq /
10c20000.memory-controller  soc:bus-g2d          soc:bus-mfc
11800000.gpu                soc:bus-g2d-acp      soc:bus-mscl
soc:bus-disp1               soc:bus-gen          soc:bus-noc
soc:bus-disp1-fimd          soc:bus-gscl-scaler  soc:bus-peri
soc:bus-fsys-apb            soc:bus-jpeg         soc:bus-wcore
soc:bus-fsys2               soc:bus-jpeg-apb
----------------------------------------------------------

IIRC some Odroid kernel maintained by Hardkernel had more devices
in this dir.


Here is how these bus devices print themselves during boot:
----------------------------------------------------------
[    8.674840] exynos-bus: new bus device registered: soc:bus-wcore ( 
88700 KHz ~ 532000 KHz)
[    8.686412] exynos-bus: new bus device registered: soc:bus-noc ( 
66600 KHz ~ 111000 KHz)
[    8.696080] exynos-bus: new bus device registered: soc:bus-fsys-apb 
(111000 KHz ~ 222000 KHz)
[    8.706590] exynos-bus: new bus device registered: soc:bus-fsys2 ( 
75000 KHz ~ 200000 KHz)
[    8.717661] exynos-bus: new bus device registered: soc:bus-mfc ( 
83250 KHz ~ 333000 KHz)
[    8.728139] exynos-bus: new bus device registered: soc:bus-gen ( 
88700 KHz ~ 266000 KHz)
[    8.737551] exynos-bus: new bus device registered: soc:bus-peri ( 
66600 KHz ~  66600 KHz)
[    8.748625] exynos-bus: new bus device registered: soc:bus-g2d ( 
83250 KHz ~ 333000 KHz)
[    8.759427] exynos-bus: new bus device registered: soc:bus-g2d-acp ( 
66500 KHz ~ 266000 KHz)
[    8.770366] exynos-bus: new bus device registered: soc:bus-jpeg ( 
75000 KHz ~ 300000 KHz)
[    8.781135] exynos-bus: new bus device registered: soc:bus-jpeg-apb ( 
83250 KHz ~ 166500 KHz)
[    8.791366] exynos-bus: new bus device registered: soc:bus-disp1-fimd 
(120000 KHz ~ 200000 KHz)
[    8.802418] exynos-bus: new bus device registered: soc:bus-disp1 
(120000 KHz ~ 300000 KHz)
[    8.813050] exynos-bus: new bus device registered: 
soc:bus-gscl-scaler (150000 KHz ~ 300000 KHz)
[    8.825308] exynos-bus: new bus device registered: soc:bus-mscl ( 
84000 KHz ~ 666000 KHz)

----------------------------------------------------------


> > It's true that they will not affect thermal zones, but unnecessarily,
> > they all will show up in the /sys/class/thermal/ as
> > thermal-devfreq-X
> >
> >
> > IMO the devfreq shouldn't be tight with devfreq cooling thermal.
>
> The energy model is tied with a cooling device initialization.
>
> So if we want to do power limitation, the energy model must be
> initialized with the device, thus the cooling device also.
>
> That is the reason why I'm ending up with this change. Chanwoo
> suggestion makes sense and that will allow to move the initialization to
> devfreq which is more sane but it does not solve the initial problem
> with the energy model.

Make sense, the 'is_cooling_device' does the job.

Regards,
Lukasz