Reboot crash at msm_atomic_commit_tail

[Rob Clark]

On Mon, Jan 18, 2021 at 11:00 PM Daniel Vetter <daniel at ffwll.ch> wrote:
>
> On Mon, Jan 18, 2021 at 11:00 PM Fabio Estevam <festevam at gmail.com> wrote:
> >
> > On Mon, Jan 18, 2021 at 6:44 PM Fabio Estevam <festevam at gmail.com> wrote:
> > >
> > > Adding some more folks in case anyone has any suggestions to fix this
> > > reboot hang.
> >
> > Not sure if this is a valid fix, but the change below makes reboot
> > works correctly.
> >
> > kmscube still works.
> >
> > --- a/drivers/gpu/drm/msm/msm_atomic.c
> > +++ b/drivers/gpu/drm/msm/msm_atomic.c
> > @@ -207,8 +207,12 @@ void msm_atomic_commit_tail(struct drm_atomic_state *state)
> >         struct msm_kms *kms = priv->kms;
> >         struct drm_crtc *async_crtc = NULL;
> >         unsigned crtc_mask = get_crtc_mask(state);
> > -       bool async = kms->funcs->vsync_time &&
> > -                       can_do_async(state, &async_crtc);
> > +       bool async;
> > +
> > +       if (!kms)
> > +               return;
>
> That looks a bit like a hack papering over the real issue.
>
> From your report it sounds like earlier kernels worked, did you
> attempt bisecting? Also for regressions put regressions into the
> subject, it's the magic work that gets much more attention.

the root issue is how are we doing KMS stuff on imx (where drm/msm is
only used for gpu).. which I think is this commit:

----------
commit 9d5cbf5fe46e350715389d89d0c350d83289a102
Author:     Krishna Manikandan <mkrishn at codeaurora.org>
AuthorDate: Mon Jun 1 16:33:22 2020 +0530
Commit:     Rob Clark <robdclark at chromium.org>
CommitDate: Tue Aug 18 08:09:01 2020 -0700

    drm/msm: add shutdown support for display platform_driver

    Define shutdown callback for display drm driver,
    so as to disable all the CRTCS when shutdown
    notification is received by the driver.

    This change will turn off the timing engine so
    that no display transactions are requested
    while mmu translations are getting disabled
    during reboot sequence.

    Signed-off-by: Krishna Manikandan <mkrishn at codeaurora.org>

    Changes in v2:
            - Remove NULL check from msm_pdev_shutdown (Stephen Boyd)
            - Change commit text to reflect when this issue
              was uncovered (Sai Prakash Ranjan)

    Signed-off-by: Rob Clark <robdclark at chromium.org>
----------

I suppose we should do the drm_atomic_helper_shutdown() conditionally?
 Or the helper should bail if there is no kms?

BR,
-R

> -Daniel
>
> > +
> > +       async = kms->funcs->vsync_time && can_do_async(state, &async_crtc);
> >
> >         trace_msm_atomic_commit_tail_start(async, crtc_mask);
> >
> > Any comments?
> >
> > Thanks
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch