[PATCH] drm/amd/display: Add NULL checks for vblank workqueue

[Harry Wentland]

On 2021-09-07 9:41 p.m., Mike Lothian wrote:
> Hi
> 
> I've just tested this out against Linus's tree and it seems to fix things
> 

Thanks.

> Out of interest does Tonga have GPU reset when things go wrong?
> 

Not sure. Alex might know.

Harry

> Thanks
> 
> Mike
> 
> On Tue, 7 Sept 2021 at 15:20, Harry Wentland <harry.wentland at amd.com> wrote:
>>
>>
>>
>> On 2021-09-07 10:10 a.m., Nicholas Kazlauskas wrote:
>>> [Why]
>>> If we're running a headless config with 0 links then the vblank
>>> workqueue will be NULL - causing a NULL pointer exception during
>>> any commit.
>>>
>>> [How]
>>> Guard access to the workqueue if it's NULL and don't queue or flush
>>> work if it is.
>>>
>>> Cc: Roman Li <Roman.Li at amd.com>
>>> Cc: Wayne Lin <Wayne.Lin at amd.com>
>>> Cc: Harry Wentland <Harry.Wentland at amd.com>
>>> Reported-by: Mike Lothian <mike at fireburn.co.uk>
>>> BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1700>>>> Fixes: 91f86d4cce2 ("drm/amd/display: Use vblank control events for PSR enable/disable")
>>> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
>>
>> Reviewed-by: Harry Wentland <harry.wentland at amd.com>
>>
>> Harry
>>
>>> ---
>>>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 +++++++++++--------
>>>  1 file changed, 18 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> index 8837259215d..46e08736f94 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -6185,21 +6185,23 @@ static inline int dm_set_vblank(struct drm_crtc *crtc, bool enable)
>>>               return 0;
>>>
>>>  #if defined(CONFIG_DRM_AMD_DC_DCN)
>>> -     work = kzalloc(sizeof(*work), GFP_ATOMIC);
>>> -     if (!work)
>>> -             return -ENOMEM;
>>> +     if (dm->vblank_control_workqueue) {
>>> +             work = kzalloc(sizeof(*work), GFP_ATOMIC);
>>> +             if (!work)
>>> +                     return -ENOMEM;
>>>
>>> -     INIT_WORK(&work->work, vblank_control_worker);
>>> -     work->dm = dm;
>>> -     work->acrtc = acrtc;
>>> -     work->enable = enable;
>>> +             INIT_WORK(&work->work, vblank_control_worker);
>>> +             work->dm = dm;
>>> +             work->acrtc = acrtc;
>>> +             work->enable = enable;
>>>
>>> -     if (acrtc_state->stream) {
>>> -             dc_stream_retain(acrtc_state->stream);
>>> -             work->stream = acrtc_state->stream;
>>> -     }
>>> +             if (acrtc_state->stream) {
>>> +                     dc_stream_retain(acrtc_state->stream);
>>> +                     work->stream = acrtc_state->stream;
>>> +             }
>>>
>>> -     queue_work(dm->vblank_control_workqueue, &work->work);
>>> +             queue_work(dm->vblank_control_workqueue, &work->work);
>>> +     }
>>>  #endif
>>>
>>>       return 0;
>>> @@ -8809,7 +8811,8 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>>>                * If PSR or idle optimizations are enabled then flush out
>>>                * any pending work before hardware programming.
>>>                */
>>> -             flush_workqueue(dm->vblank_control_workqueue);
>>> +             if (dm->vblank_control_workqueue)
>>> +                     flush_workqueue(dm->vblank_control_workqueue);
>>>  #endif
>>>
>>>               bundle->stream_update.stream = acrtc_state->stream;
>>> @@ -9144,7 +9147,8 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
>>>               /* if there mode set or reset, disable eDP PSR */
>>>               if (mode_set_reset_required) {
>>>  #if defined(CONFIG_DRM_AMD_DC_DCN)
>>> -                     flush_workqueue(dm->vblank_control_workqueue);
>>> +                     if (dm->vblank_control_workqueue)
>>> +                             flush_workqueue(dm->vblank_control_workqueue);
>>>  #endif
>>>                       amdgpu_dm_psr_disable_all(dm);
>>>               }
>>>
>>