[AMD Official Use Only - General] > -----Original Message----- > From: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> > Sent: Monday, April 17, 2023 6:30 PM > To: Jeff Layton <jlayton@xxxxxxxxxx>; Lyude Paul <lyude@xxxxxxxxxx>; Lin, > Wayne <Wayne.Lin@xxxxxxx>; Alex Deucher <alexdeucher@xxxxxxxxx> > Cc: David Airlie <airlied@xxxxxxxxx>; Daniel Vetter <daniel@xxxxxxxx>; > Deucher, Alexander <Alexander.Deucher@xxxxxxx>; linux- > kernel@xxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH] drm: make drm_dp_add_payload_part2 gracefully > handle NULL state pointer > > On Mon, 17 Apr 2023, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > On Mon, 2023-04-17 at 11:44 +0300, Jani Nikula wrote: > >> On Fri, 14 Apr 2023, Lyude Paul <lyude@xxxxxxxxxx> wrote: > >> > On Fri, 2023-04-14 at 13:35 +0300, Jani Nikula wrote: > >> > > On Fri, 14 Apr 2023, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > >> > > > 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. > >> > > > >> > > Cc: Lyude. > >> > > > >> > > Looks like the state parameter was added in commit 4d07b0bc4034 > >> > > ("drm/display/dp_mst: Move all payload info into the atomic > >> > > state") and its only use is to get at state->dev for debug logging. > >> > > > >> > > What's the plan for the parameter? Surely something more than > >> > > that! :) > >> > > >> > I don't think there was any plan for that, or at least I certainly > >> > don't even remember adding that D:. It must totally have been by > >> > mistake and snuck by review, if that's the only thing that we're > >> > using it for I'd say it's definitely fine to just drop it entirely > >> > >> I guess we could use two patches then, first replace state->dev with > >> mgr->dev as something that can be backported as needed, and second > >> mgr->drop > >> the state parameter altogether. > >> > >> Jeff, up for it? At least the first one? > >> > >> > >> BR, > >> Jani. > >> > > > > Sure. I'm happy to test patches if you send them along. > > I was hoping to lure you into sending patches. ;) > > Anyway, I'm not working on this. > > Hi Jeff, I probably know the root cause. But it doesn't need to use the state in the end, I will just rely on the patch that Jani suggested to fix it. I can help to provide the patch later : ) > BR, > Jani. > > > > > Thanks, > > -- > Jani Nikula, Intel Open Source Graphics Center -- Regards, Wayne