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

[Mike Lothian <mike@fireburn.co.uk>]

Hi

I've just tested this out against Linus's tree and it seems to fix things

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

Thanks

Mike

On Tue, 7 Sept 2021 at 15:20, Harry Wentland <harry.wentland@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@amd.com>
> > Cc: Wayne Lin <Wayne.Lin@amd.com>
> > Cc: Harry Wentland <Harry.Wentland@amd.com>
> > Reported-by: Mike Lothian <mike@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@amd.com>
>
> Reviewed-by: Harry Wentland <harry.wentland@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);
> >               }
> >
>