Re: [PATCH] drm: make drm_dp_add_payload_part2 gracefully handle NULL state pointer

[Jeff Layton <jlayton@kernel.org>]

On Fri, 2023-04-14 at 04:40 +0000, Lin, Wayne wrote:
> [Public]
> 
> Hi Jeff,
> 
> Thanks. I might need more information to understand why we can't retrieve
> the drm atomic state. Also , "Failed to create MST payload for port" indicates
> error while configuring DPCD payload ID table. Could you help to provide log
> with KMS + ATOMIC + DP debug on please? Thanks in advance!
> 
> Regards,
> Wayne
> 

Possibly. I'm not that familiar with display driver debugging. Can you
send me some directions on how to crank up that sort of debug logging?

Note that this problem is _very_ intermittent too: I went about 2 weeks
between crashes, and then I got 3 in one day. I'd rather not run with a
lot of debug logging for a long time if that's what this is going to
require, as this is my main workstation.

The last time I got this log message, my proposed patch did prevent the
box from oopsing, so I'd really like to see it go in unless it's just
categorically wrong for the caller to pass down a NULL state pointer to
drm_dp_add_payload_part2.

> > -----Original Message-----
> > From: Alex Deucher <alexdeucher@gmail.com>
> > Sent: Thursday, April 13, 2023 8:59 PM
> > To: Jani Nikula <jani.nikula@linux.intel.com>; Lin, Wayne
> > <Wayne.Lin@amd.com>
> > Cc: Jeff Layton <jlayton@kernel.org>; David Airlie <airlied@gmail.com>;
> > Daniel Vetter <daniel@ffwll.ch>; Deucher, Alexander
> > <Alexander.Deucher@amd.com>; linux-kernel@vger.kernel.org; dri-
> > devel@lists.freedesktop.org
> > Subject: Re: [PATCH] drm: make drm_dp_add_payload_part2 gracefully
> > handle NULL state pointer
> > 
> > + Wayne
> > 
> > On Thu, Apr 13, 2023 at 8:31 AM Jani Nikula <jani.nikula@linux.intel.com>
> > wrote:
> > > 
> > > On Thu, 13 Apr 2023, Jeff Layton <jlayton@kernel.org> wrote:
> > > > I've been experiencing some intermittent crashes down in the display
> > > > driver code. The symptoms are ususally a line like this in dmesg:
> > > > 
> > > >     amdgpu 0000:30:00.0: [drm] Failed to create MST payload for port
> > > > 000000006d3a3885: -5
> > > > 
> > > > ...followed by an Oops due to a NULL pointer dereference.
> > > > 
> > > > The real bug is probably in the caller of this function, which is
> > > > passing it a NULL state pointer, but this patch at least keeps my
> > > > machine from oopsing when this occurs.
> > > 
> > > My fear is that papering over this makes the root cause harder to find.
> > > 
> > > Cc: Harry, Alex
> > > 
> > > 
> > > BR,
> > > Jani.
> > > 
> > > 
> > > > 
> > > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2184855
> > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > ---
> > > >  drivers/gpu/drm/display/drm_dp_mst_topology.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > > b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > > index 38dab76ae69e..87ad406c50f9 100644
> > > > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > > @@ -3404,7 +3404,8 @@ int drm_dp_add_payload_part2(struct
> > > > drm_dp_mst_topology_mgr *mgr,
> > > > 
> > > >       /* Skip failed payloads */
> > > >       if (payload->vc_start_slot == -1) {
> > > > -             drm_dbg_kms(state->dev, "Part 1 of payload creation for %s
> > failed, skipping part 2\n",
> > > > +             drm_dbg_kms(state ? state->dev : NULL,
> > > > +                         "Part 1 of payload creation for %s failed,
> > > > + skipping part 2\n",
> > > >                           payload->port->connector->name);
> > > >               return -EIO;
> > > >       }
> > > 
> > > --
> > > Jani Nikula, Intel Open Source Graphics Center

-- 
Jeff Layton <jlayton@kernel.org>