[PATCH V4 2/2] drm/vc4: v3d: add PM suspend/resume support

[Stefan Wahren]

Hi Maíra,

Am 09.10.24 um 14:16 schrieb Maíra Canal:
> Hi Stefan,
>
> On 10/8/24 13:05, Stefan Wahren wrote:
>> Hi Maíra,
>>
>> Am 08.10.24 um 15:49 schrieb Maíra Canal:
>>> Hi Stefan,
>>>
>>> On 10/3/24 09:41, Stefan Wahren wrote:
>>>> Add suspend/resume support for the VC4 V3D component in order
>>>> to handle suspend to idle properly.
>>>>
>>>> Signed-off-by: Stefan Wahren <wahrenst at gmx.net>
>>>> ---
>>>>   drivers/gpu/drm/vc4/vc4_v3d.c | 7 +++----
>>>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c
>>>> b/drivers/gpu/drm/vc4/vc4_v3d.c
>>>> index 2423826c89eb..8057b06c1f16 100644
>>>> --- a/drivers/gpu/drm/vc4/vc4_v3d.c
>>>> +++ b/drivers/gpu/drm/vc4/vc4_v3d.c
>>>> @@ -368,7 +368,6 @@ void vc4_v3d_bin_bo_put(struct vc4_dev *vc4)
>>>>       mutex_unlock(&vc4->bin_bo_lock);
>>>>   }
>>>>
>>>> -#ifdef CONFIG_PM
>>>>   static int vc4_v3d_runtime_suspend(struct device *dev)
>>>>   {
>>>>       struct vc4_v3d *v3d = dev_get_drvdata(dev);
>>>> @@ -397,7 +396,6 @@ static int vc4_v3d_runtime_resume(struct device
>>>> *dev)
>>>>
>>>>       return 0;
>>>>   }
>>>> -#endif
>>>>
>>>>   int vc4_v3d_debugfs_init(struct drm_minor *minor)
>>>>   {
>>>> @@ -507,7 +505,8 @@ static void vc4_v3d_unbind(struct device *dev,
>>>> struct device *master,
>>>>   }
>>>>
>>>>   static const struct dev_pm_ops vc4_v3d_pm_ops = {
>>>> -    SET_RUNTIME_PM_OPS(vc4_v3d_runtime_suspend,
>>>> vc4_v3d_runtime_resume, NULL)
>>>> +    RUNTIME_PM_OPS(vc4_v3d_runtime_suspend, vc4_v3d_runtime_resume,
>>>> NULL)
>>>> +    SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>>>> pm_runtime_force_resume)
>>>
>>> I have a question: How can we guarantee that no jobs are running when
>>> the system is forced to suspend?
>> Not sure what do you mean with job. userspace task or v3d job within the
>> driver?
>
> I mean a V3D job. See, when we submit a CL to the GPU, we only know when
> it's done through a IRQ. I'm thinking in the case where:
>
> 1. We submitted a CL to the GPU
> 2. We suspend but the job wasn't ended yet
okay, now i understand. I was under the naive impression that everything
is handled in the ARM cores. Sorry, i don't really have a deeper
understanding of the V3D stuff.
>
> What happens to this job? Is the GPU going to be in a unstable state
> when we resume?
I don't know what happens to the GPU, but during suspend most of the
IRQs would be disabled so the ARM core(s) won't serve the interrupt handler.

At least i need some guidance here, how to prevent sending new jobs to
the GPU. I assume such a logic must be already part of the driver removal.

Best regards
>
>>
>> Do you have something specific in mind.
>>
>> Why is there a difference between runtime pm and system pm?
>>
> As far as I can see, for system PM, we need at least to suspend V3D in
> a stable state, without any jobs running and with IRQs disabled.
>
> Best Regards,
> - Maíra
>
>> I must confess that i didn't test a system sleep while running a v3d
>> application.
>>
>> Best regards
>> Stefan
>>>
>>> Best Regards,
>>> - Maíra
>>>
>>>>   };
>>>>
>>>>   static const struct component_ops vc4_v3d_ops = {
>>>> @@ -538,6 +537,6 @@ struct platform_driver vc4_v3d_driver = {
>>>>       .driver = {
>>>>           .name = "vc4_v3d",
>>>>           .of_match_table = vc4_v3d_dt_match,
>>>> -        .pm = &vc4_v3d_pm_ops,
>>>> +        .pm = pm_ptr(&vc4_v3d_pm_ops),
>>>>       },
>>>>   };
>>>> --
>>>> 2.34.1
>>>>
>>