[PATCH] drm/panfrost: Make devfreq truly optional

[Tomeu Vizoso]

On 5/14/19 5:36 PM, Ezequiel Garcia wrote:
> On Tue, 2019-05-14 at 08:38 -0500, Rob Herring wrote:
>> On Mon, May 13, 2019 at 12:56 PM Ezequiel Garcia <ezequiel at collabora.com> wrote:
>>> Currently, there is some logic to make devfreq optional,
>>> but it fails to cover some cases such as !CONFIG_PM_DEVFREQ.
>>
>> Fails how? compiling? runtime? Or just builds extra code?
>>
> 
> Well, given we currently don't enforce PM_DEVFREQ, the driver fails
> to probe. Specifically,
> 
> panfrost_devfreq_init
>    devfreq_recommended_opp
>      -EINVAL
>   
>>> Moreover, depending on return codes is not resilient to change,
>>> so let's take a different approach, introducing proper
>>> stubs and only conditionally compiling the devfreq support.
>>
>> The downside here is another build time configuration to test. Isn't
>> devfreq essentially going to be required to run the GPU at higher
>> frequencies and to not melt it?
>>
> 
> Right. So the alternative is to select or depend on PM_DEVFREQ.
> 
> Otherwise, it's quite confusing for developers to choose the driver
> and have it fail to probe because some other option is not selected.

Yeah, I don't see a good reason to not use devfreq, so I think we should 
require it.

Thanks,

Tomeu

> Let me know what you think and I'll cook a v2.
> 
> Thanks,
> Eze
> 
>>> Signed-off-by: Ezequiel Garcia <ezequiel at collabora.com>
>>> ---
>>>   drivers/gpu/drm/panfrost/Makefile           |  3 ++-
>>>   drivers/gpu/drm/panfrost/panfrost_devfreq.c | 13 ++-----------
>>>   drivers/gpu/drm/panfrost/panfrost_devfreq.h | 19 +++++++++++++++++--
>>>   3 files changed, 21 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/panfrost/Makefile b/drivers/gpu/drm/panfrost/Makefile
>>> index 6de72d13c58f..8b690672f9d9 100644
>>> --- a/drivers/gpu/drm/panfrost/Makefile
>>> +++ b/drivers/gpu/drm/panfrost/Makefile
>>> @@ -3,10 +3,11 @@
>>>   panfrost-y := \
>>>          panfrost_drv.o \
>>>          panfrost_device.o \
>>> -       panfrost_devfreq.o \
>>>          panfrost_gem.o \
>>>          panfrost_gpu.o \
>>>          panfrost_job.o \
>>>          panfrost_mmu.o
>>>
>>> +panfrost-$(CONFIG_PM_DEVFREQ) += panfrost_devfreq.o
>>> +
>>>   obj-$(CONFIG_DRM_PANFROST) += panfrost.o
>>> diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
>>> index 238bd1d89d43..29fcffdf2d57 100644
>>> --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
>>> +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
>>> @@ -140,8 +140,8 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
>>>                  return 0;
>>>
>>>          ret = dev_pm_opp_of_add_table(&pfdev->pdev->dev);
>>> -       if (ret == -ENODEV) /* Optional, continue without devfreq */
>>> -               return 0;
>>> +       if (ret)
>>> +               return ret;
>>>
>>>          panfrost_devfreq_reset(pfdev);
>>>
>>> @@ -170,9 +170,6 @@ void panfrost_devfreq_resume(struct panfrost_device *pfdev)
>>>   {
>>>          int i;
>>>
>>> -       if (!pfdev->devfreq.devfreq)
>>> -               return;
>>> -
>>>          panfrost_devfreq_reset(pfdev);
>>>          for (i = 0; i < NUM_JOB_SLOTS; i++)
>>>                  pfdev->devfreq.slot[i].busy = false;
>>> @@ -182,9 +179,6 @@ void panfrost_devfreq_resume(struct panfrost_device *pfdev)
>>>
>>>   void panfrost_devfreq_suspend(struct panfrost_device *pfdev)
>>>   {
>>> -       if (!pfdev->devfreq.devfreq)
>>> -               return;
>>> -
>>>          devfreq_suspend_device(pfdev->devfreq.devfreq);
>>>   }
>>>
>>> @@ -194,9 +188,6 @@ static void panfrost_devfreq_update_utilization(struct panfrost_device *pfdev, i
>>>          ktime_t now;
>>>          ktime_t last;
>>>
>>> -       if (!pfdev->devfreq.devfreq)
>>> -               return;
>>> -
>>>          now = ktime_get();
>>>          last = pfdev->devfreq.slot[slot].time_last_update;
>>>
>>> diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.h b/drivers/gpu/drm/panfrost/panfrost_devfreq.h
>>> index eb999531ed90..76b56a8de6e3 100644
>>> --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.h
>>> +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.h
>>> @@ -4,11 +4,26 @@
>>>   #ifndef __PANFROST_DEVFREQ_H__
>>>   #define __PANFROST_DEVFREQ_H__
>>>
>>> +#if defined(CONFIG_PM_DEVFREQ)
>>>   int panfrost_devfreq_init(struct panfrost_device *pfdev);
>>> -
>>>   void panfrost_devfreq_resume(struct panfrost_device *pfdev);
>>>   void panfrost_devfreq_suspend(struct panfrost_device *pfdev);
>>> -
>>>   void panfrost_devfreq_record_transition(struct panfrost_device *pfdev, int slot);
>>> +#else
>>> +static inline int panfrost_devfreq_init(struct panfrost_device *pfdev)
>>> +{
>>> +       return 0;
>>> +}
>>> +
>>> +static inline void panfrost_devfreq_resume(struct panfrost_device *pfdev)
>>> +{}
>>> +
>>> +static inline void panfrost_devfreq_suspend(struct panfrost_device *pfdev)
>>> +{}
>>> +
>>> +static inline void
>>> +panfrost_devfreq_record_transition(struct panfrost_device *pfdev, int slot)
>>> +{}
>>> +#endif
>>>
>>>   #endif /* __PANFROST_DEVFREQ_H__ */
>>> --
>>> 2.20.1
>>>
> 
> 
>