Re: [PATCH v3 4/4] drm/panfrost: Register to the Energy Model with devfreq device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 26/02/2020 10:06 am, Lukasz Luba wrote:
[...]
@@ -118,6 +120,7 @@ void panfrost_devfreq_fini(struct panfrost_device *pfdev)
  {
         if (pfdev->devfreq.cooling)
                 devfreq_cooling_unregister(pfdev->devfreq.cooling);
+       dev_pm_opp_of_unregister_em(&pfdev->pdev->dev);
         dev_pm_opp_of_remove_table(&pfdev->pdev->dev);

Does it make sense to keep this (and the registration side) as
separate calls? Perhaps there's some ordering requirement with
everything between dev_pm_opp_of_add_table() and
dev_pm_opp_of_register_em()?

Yes, dev_pm_opp_of_register_em() uses em_data_callback which operates
on OPPs to calculate power values and costs, so the the OPP table should
be already there.


While you're just adding 2 lines, it seems there's a lot of complexity
exposed to the driver just to initialize devfreq/opp.

It depends, for example devfreq devices like buses would likely never
use the energy model. Potential clients would be GPUs, DSPs, ISPs.

Still, it seems less than ideal for every client to have to remember to make all these individual calls, all in the right order (especially when it comes to undoing them in failure paths).

I haven't quite grasped whether the energy model is conceptually "owned" by the OPP table or by the cooling device, but either way it would seem to be a much nicer API if there were simply an additional "with energy model" variant of the registration call, and the standard removal call just automatically cleaned up an energy model as well if one was present.

Robin.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux