dri-devel May 2022

dri-devel@lists.freedesktop.org

344 participants
736 discussions

Re: [PATCH] WIP: drm/dp_mst: Add support for dumping topology ref histories from debugfs
by Lyude Paul 06 May '22

06 May '22

(Adding this back to the dri-devel mailing list since I didn't notice it got dropped from there) Hm, some comments on this issue down below. Sorry for the delayed response, I was going to try this right after I finished the MST legacy removal but that's ending up taking longer than I hoped. On Tue, 2022-03-08 at 01:46 +0000, Lin, Wayne wrote: > [AMD Official Use Only] > > Oops... > I didn't notice that I replied to wrong mail previously : P > Sorry for the confusion and reply to the correct thread. > > Good day! > I probably know why you can't reproduce the issue there. Please refer to the > attached > patch file which makes me easier to explain this. > > In dm_dp_mst_get_modes(), we will create a new dc_sink by calling > dc_link_add_remote_sink() and add that dc_sink into the array dc_link- > >remote_sinks[]. > However, if we find that aconnector->dc_sink is not null, then we won't > create a new > dc_sink for that aconnector and won't add the sink to the array dc_link- > >remote_sinks[]. > > When the issue that I mentioned before occurs, we won't be able to release > the dc_sink > for the aconnector when we unplug the sst monitor. Hence, we can't create > new dc_sink > for the latter on connected new stream sink of that port. Which leads to two > issues > 1. Unplug monitor and plug in another monitor "to the same port" > => Since we use the same dc_sink to reflect the capability of the new > connected stream > sink, we might send out inappropriate stream to the new connected sink. > 2. Because we can't release dc_sink in the array dc_link->remote_sinks[], > when we > plug/unplug sst monitor to more than 4 different mst port, we will break the > criteria > "dc_link->sink_count >= MAX_SINKS_PER_LINK" in > link_add_remote_sink_helper(). Ok, a lot of this seems to be happening in amdgpu which certainly explains why I had so much trouble following along with this originally (also, having learned a bit more about how DC works definitely has helped a bit). I already see a bunch of issues though with how this code is structured that would definitely break things, though note I haven't sat down and tried this on a real machine yet - will try tomorrow. So - it seems like dc_sink == a bunch of hotplugging state for a dm connector, which actually tells me for one that this is definitely the wrong spot for this code. get_modes() really should just handle filling out DRM modes and pretty much nothing else, because callers from DRM aren't going to expect side-effects like this when get_modes() is called - which could potentially lead to all sorts of weirdness down the line if the DRM call paths that use this ever change. i915 and nouveau have good examples of what these functions should typically look like, and amdgpu actually seems to mostly follow this advice for it's other get_modes() callback. Note there's also another problem here: the assumption "no EDID == disconnected" isn't correct. It's totally possible to run into EDID-less displays if the display is some ancient pre-historic relic, or if the ROM (or EEPROM? can't remember what type of chip computer displays tend to use…) chip in the monitor containing the EDID has died. Note that the second situation is suprisingly more common then one might think! I ran into a 140Hz 4k ASUS display where this happened, and I know another friend of mine who had the ROM in their display die randomly as well. So with this being said: I, think the solution here is actually just to add/remove dc_sink from dm_dp_mst_detect() based on the CSN, and drop the calls from get_modes()? detect() should always be called before the connector has been removed, and we could use the return status from drm_dp_mst_detect_port() in order to tell whether or not we should add/remove the dc_sink. If you don't get to it first tomorrow, I'll see if I can reproduce this locally and try writing up a patch. Also a final note: I don't think it's a good idea in general for there to be a fixed size array of dc_sinks like this. Those probably should just be stored somehow on a per-connector basis. I say this because in addition to the fact MST connectors aren't destroyed immediately (which could lead to a situation where we have more then 6 connected MST ports briefly before their destructors run, e.g. during suspend/resume reprobing). Also, one question below: > > Anyway, these are my steps to reproduce the issue. > I use a startech mst 1to4 mst hub, a sst monitor and a mst monitor. > > 1. boot up with sst monitor connected to the hub and plug the hub to the > system. > 2. unplug/plug sst monitor to all left three different DP output ports. > Which is used to > Pollute all four aconnectors to have them keep the stale dc_sinks. > 3. plug in the mst monitor to arbitrary ports. Since the new connected mst > monitor will > have us add new mst port to the topology and attach a new aconnector for > that mst port, > we'll try to create a new dc_sink to that new aconnector and break the > sink_count > limitation. As you can see in the log the error message : "*ERROR* Unable to > add a > remote sink" > > (Note that different hub might influence the result. Some hubs will pull > down the HPD > when there is no monitor connected to its downstream ports which will run > into different > code flow) by "pull down" I assume you mean doing a long HPD instead of short (or vice versa?). > > I also attach the dmesg log under different debug level (0x104 & 0x116) for > your reference. > > Please let me know if you can reproduce then : ) > > Sorry about that Lyude and hope this time I send to the right thread.. > Really appreciate for your help!!! > > Regards, > Wayne > > -----Original Message----- > > From: Lyude Paul <lyude(a)redhat.com> > > Sent: Wednesday, March 2, 2022 9:10 AM > > To: Lin, Wayne <Wayne.Lin(a)amd.com> > > Subject: Re: [PATCH] WIP: drm/dp_mst: Add support for dumping topology ref > > histories from debugfs > > > > Ran into some issues with my system yesterday (hooray being the first to > > find > > a rather annoying zsh bug that broke my scripts!), but I've finally got > > things > > setup. I'm not able to reproduce this issue though (I've tried hotplugging > > the > > way you mentioned here, but nothing seems to leak), which makes me think > > there's something special in your setup we might not have noticed. > > > > Do you think you could get some drm logs with drm.debug=0x116 > > log_buf_len=50M > > from when this happens so I can get a better idea of the drm events > > leading up > > to this? Also, if you've been testing this in a GUI could you confirm > > whether > > this happens when you're just in a plain old VT? > > > > On Mon, 2022-02-28 at 16:43 -0500, Lyude Paul wrote: > > > Alright - starting to look into this now and setting up a system to try > > > reproducing this, will let you know as soon as I've got results. Sorry > > > again > > > about all of the delays > > > > > > On Fri, 2022-02-25 at 15:47 -0500, Lyude Paul wrote: > > > > I didn't get a chance yet, but I've gotten through most of my backlog > > > > now. > > > > Sorry about the delay - I've been catching up since I had to take two > > > > weeks > > > > off work very unexpectedly. I'll put aside time to look at this first > > > > thing > > > > monday. > > > > > > > > On Fri, 2022-02-25 at 01:33 +0000, Lin, Wayne wrote: > > > > > [AMD Official Use Only] > > > > > > > > > > Hi Lyude, > > > > > > > > > > Good day! > > > > > Sorry to bother you, but would like to know if you can reproduce the > > > > > issue? > > > > > > > > > > Thanks! > > > > > > > > > > Regards, > > > > > Wayne > > > > > > > > > > > -----Original Message----- > > > > > > From: Lyude Paul <lyude(a)redhat.com> > > > > > > Sent: Thursday, February 3, 2022 4:12 AM > > > > > > To: Lin, Wayne <Wayne.Lin(a)amd.com> > > > > > > Cc: dri-devel(a)lists.freedesktop.org > > > > > > Subject: Re: [PATCH] WIP: drm/dp_mst: Add support for dumping > > > > > > topology > > > > > > ref > > > > > > histories from debugfs > > > > > > > > > > > > JFYI I should be able to respond to this soon, I think I'm > > > > > > following > > > > > > your > > > > > > issue a bit more (haven't gotten to go through the patches too so > > > > > > I'm > > > > > > probably > > > > > > going to see if I can reproduce it locally as well. Sorry for the > > > > > > delay > > > > > > - > > > > > > dealing with some X.org stuff and work issues at the moment > > > > > > > > > > > > On Tue, 2022-01-25 at 08:32 +0000, Lin, Wayne wrote: > > > > > > > [Public] > > > > > > > > > > > > > > Hey Lyude, > > > > > > > > > > > > > > Sorry, just got chance to get back to this and BIG THANKS to > > > > > > > elaborating > > > > > > > in > > > > > > > such details! > > > > > > > Following are some of my comments : ) > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > From: Lyude Paul <lyude(a)redhat.com> > > > > > > > > Sent: Wednesday, January 19, 2022 9:59 AM > > > > > > > > To: Lin, Wayne <Wayne.Lin(a)amd.com> > > > > > > > > Cc: dri-devel(a)lists.freedesktop.org > > > > > > > > Subject: Re: [PATCH] WIP: drm/dp_mst: Add support for dumping > > > > > > > > topology > > > > > > > > ref > > > > > > > > histories from debugfs > > > > > > > > > > > > > > > > Hey - so, the original issue here was that payloads were not > > > > > > > > always > > > > > > > > deleted when we expected them to be - correct? > > > > > > > > > > > > > > Actually, what I was trying to do is not relevant to payload > > > > > > > maintenance. > > > > > > > What I wanted to do is having a way to notify > > > > > > > amdgpu to release its enumerated resource "dc_sink" when unplug > > > > > > > event > > > > > > > occurs. Currently, under specific scenarios, > > > > > > > we can't release remote "dc_sink" and ends up can't allocate > > > > > > > resource > > > > > > > for > > > > > > > new connected monitors. I'll explain the scenario > > > > > > > underneath. > > > > > > > > > > > > > > So, get back to the main issue that I was trying to solve when I > > > > > > > pushed > > > > > > > the > > > > > > > patchset: > > > > > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork… > > ayne.lin%40amd.com%7Cbbc27043edc04bf8067c08d9fbe9620c%7C3dd8961fe4884e608e > > 11a82d994e183d%7C0%7C0%7C6378178020798 > > 98960%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6 > > Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdat > > a=RVUc8KgTO3mhFL9qemshHJqiyXJs%2Fvytx21AMyUaW%2Bs%3D&reserved=0 > > > > > > > - > > > > > > devel%2Fcover%2F20210720160342.11415-1- > > > > > > Wayne.Lin%40amd.com%2F&data=04%7C01%7Cwayne.lin%40amd.com%7C2e > > > > > > 19d9 > > > > > > 32 > > > > > > 1b > > > > > > 5f4e948c1208d9e6884f24%7C3dd8961fe488 > > > > > > 4e608e11a82d994e183d%7C0%7C0%7C637794295422403301%7CUnknown%7CTWFp > > > > > > bGZs > > > > > > b3 > > > > > > d8 > > > > > > eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiL > > > > > > CJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=rvtwQioO3pT1BoHb7S5 > > > > > > 32ms > > > > > > Ra > > > > > > 61 > > > > > > gP1vIyE8AB7M9I4E%3D&reserved=0 > > > > > > > > > > > > > > I was trying to fix some problems that I observed after these 2 > > > > > > > patches > > > > > > > * 09b974e8983 drm/amd/amdgpu_dm/mst: Remove - > > > > > > > >destroy_connector() > > > > > > > callback > > > > > > > * 72dc0f51591 drm/dp_mst: Remove > > > > > > > drm_dp_mst_topology_cbs.destroy_connector > > > > > > > > > > > > > > With above patches, it changes our flow to remove dc_sink (by > > > > > > > calling > > > > > > > dc_link_remote_sink()) when connector is about to be > > > > > > > destroyed. And let's see below use cases: > > > > > > > a). Disconnection happens between mst branch devices > > > > > > > e.g. > > > > > > > src - 1st_mstb - 2nd_mstb - sst_monitor => src - 1st_mstb > > > > > > > (disconnect) > > > > > > > 2nd_mstb - sst_monitor > > > > > > > > > > > > > > In above case, after receiving CSN, we will put topology > > > > > > > reference > > > > > > > count > > > > > > > of > > > > > > > 2nd mstb and its port which connected to the sst monitor. > > > > > > > As you suggested before, we will destroy 2nd mst branch device > > > > > > > and > > > > > > > its > > > > > > > all > > > > > > > ports due to there is no more mst path can route to them. > > > > > > > As the result of that, we at end call > > > > > > > drm_dp_delayed_destroy_port() > > > > > > > and > > > > > > > unregister/put the drm connector and eventually call > > > > > > > dc_link_remote_sink() to release the dc_sinck we enumerate for > > > > > > > the > > > > > > > specific > > > > > > > stream sink of sst_monitor. > > > > > > > > > > > > > > However, in below case: > > > > > > > b). Disconnection happens between mst branch devices and SST > > > > > > > monitor > > > > > > > e.g. > > > > > > > src - 1st_mstb - sst_monitor => src - 1st_mstb (disconnect) > > > > > > > sst_monitor > > > > > > > > > > > > > > In this case, which is the case having problem, it definitely > > > > > > > won't > > > > > > > decrease > > > > > > > the topology references count of the port of 1st_mstb which > > > > > > > was connected to the sst monitor to zero since the port is still > > > > > > > existing in > > > > > > > the topology. Same as the malloc reference since the port can't > > > > > > > get > > > > > > > destroyed. Hence, the port still exists and we won't call > > > > > > > drm_dp_delayed_destroy_port() to unregister and put the drm > > > > > > > connector. > > > > > > > As > > > > > > > the > > > > > > > result of that, we can't release corresponding remote dc_sink > > > > > > > under > > > > > > > this > > > > > > > case. And after few times hotplugs, we can't create any new > > > > > > > dc_sink since the number of stream sink is exceeding our > > > > > > > limitation. > > > > > > > > > > > > > > Hence, what I'm thinking here is to register a callback function > > > > > > > for > > > > > > > notifying us that the remote sink is detached. This just aligns > > > > > > > what > > > > > > > we > > > > > > > do > > > > > > > for handling long HPD event (unplug monitor from system) of > > > > > > > legacy > > > > > > > DP > > > > > > > (none > > > > > > > mst). For which case, once we detect long HPD event > > > > > > > indicating the monitor is detached, we will immediately try to > > > > > > > release > > > > > > > the > > > > > > > dc_link->local_sink and fire hotplug event to upper layer. > > > > > > > Same as here, once receives a CSN message notifying a drm > > > > > > > connector > > > > > > > is > > > > > > > changed from connected to disconnected, trigger the callback > > > > > > > function and we can try to release the dc_sink resource. > > > > > > > > > > > > > > > > > > > > > > > If I'm remembering that correctly, then (and I'm not 100% sure > > > > > > > > on > > > > > > > > this > > > > > > > > yet) I might already have noticed something very possibly > > > > > > > > wonky in > > > > > > > > how we handle payload allocations currently, that I found > > > > > > > > while > > > > > > > > working on > > > > > > > > the non-atomic removal branch that I linked to you in my > > > > > > > > previous response. Basically, in the > > > > > > > > step1() function it looks like that we follow this general > > > > > > > > flow > > > > > > > > with > > > > > > > > updating > > > > > > > > payloads: > > > > > > > > > > > > > > > > * Loop through proposed payloads and compare to previously > > > > > > > > programmed > > > > > > > > payloads > > > > > > > > - If a payload has a different allocation then it > > > > > > > > previously > > > > > > > > did, > > > > > > > > update the > > > > > > > > payload > > > > > > > > - If the payload is new, create it > > > > > > > > - If a payload no longer has an allocation, remove the > > > > > > > > payload > > > > > > > > > > > > > > > > At first glance this seems totally correct - but I'm not > > > > > > > > actually > > > > > > > > entirely > > > > > > > > convinced it is? Particularly because it seems like the order > > > > > > > > in > > > > > > > > which > > > > > > > > we do creation/deletion of payloads is totally arbitrary. To > > > > > > > > explain > > > > > > > > what > > > > > > > > I mean by that, consider a state transition like this: > > > > > > > > > > > > > > > > vcpi_slots=15 vcpi_slots=35 vcpi_slots=14 > > > > > > > > > 1 | 2 |xxxxxxxx| > > > > > > > > > > > > > > > > Let's say we want to increase payload #1 from 15 to 50, and > > > > > > > > disable > > > > > > > > payload #2 in the same atomic commit on DRM's side. If the > > > > > > > > order > > > > > > > > we > > > > > > > > update payloads is entirely arbitrary, we could end up doing > > > > > > > > this: > > > > > > > > > > > > > > > > * Increase VCPI slots payload #1 from 15->50 (total VCPI > > > > > > > > slots=99, > > > > > > > > overflow!) > > > > > > > > * Decrease VCPI slots payload #2 from 35->0 (total VCPI > > > > > > > > slots=50) > > > > > > > > > > > > > > > > Notice on the first step, we've technically overflowed the > > > > > > > > available > > > > > > > > number of VCPI slots in the payload table. This is still > > > > > > > > before > > > > > > > > step > > > > > > > > 2 > > > > > > > > though, and I could be totally wrong here - perhaps this is > > > > > > > > entirely > > > > > > > > OK in > > > > > > > > the real world, and we're allowed to overflow VC slots as long > > > > > > > > as > > > > > > > > > > > > > > I think it's forbidden to allocate time slots beyond 64 when > > > > > > > we're > > > > > > > updating > > > > > > > payload ID table of the branch in the real world. Branch > > > > > > > probably > > > > > > > will NACK the DPCD write. > > > > > > > > > > > > > > > we repair the issue before step 2. But so far I can't seem to > > > > > > > > find > > > > > > > > anything in the DP 2.0 spec that explicitly states this would > > > > > > > > be > > > > > > > > OK > > > > > > > > - > > > > > > > > which > > > > > > > > makes me think we might fail some payload allocations if we > > > > > > > > don't > > > > > > > > always > > > > > > > > ensure we avoid overflows in between VC payload table changes > > > > > > > > > > > > > > Agree. > > > > > > > For amdgpu, I think we always do deallocation (call hws- > > > > > > > > funcs.reset_hw_ctx_wrap()) before allocation > > > > > > > (apply_single_controller_ctx_to_hw()). > > > > > > > > > > > > > > > Note that avoiding overflows would be as simple as just making > > > > > > > > sure > > > > > > > > we > > > > > > > > send all VC payload table changes that free VC slots before > > > > > > > > sending > > > > > > > > any that take new slots. > > > > > > > > > > > > > > > > Again - I haven't actually confirmed this yet and am hoping to > > > > > > > > test > > > > > > > > stuff > > > > > > > > like this very soon as I'm pretty close finishing up the > > > > > > > > initial > > > > > > > > attempt > > > > > > > > at removing the non-atomic MST code in the DRM helpers now. If > > > > > > > > my > > > > > > > > theory > > > > > > > > ends up being correct here, I can fix this in my rewrite of > > > > > > > > the > > > > > > > > MST payload management code. But I figured it might be worth > > > > > > > > mentioning in > > > > > > > > the mean time in case you think it might be relevant to the > > > > > > > > problem here :). > > > > > > > > > > > > > > > > On Wed, 2022-01-12 at 16:47 -0500, Lyude Paul wrote: > > > > > > > > > (CC'ing this to dri-devel, since this is basically patch > > > > > > > > > review) > > > > > > > > > > > > > > > > > > Alright - so first off sorry about the broken debugging > > > > > > > > > patch! I > > > > > > > > > thought I had tested that but I guess I hadn't tested it > > > > > > > > > well > > > > > > > > > enough, > > > > > > > > > I'll get it fixed asap, but feel free to try getting to it > > > > > > > > > before > > > > > > > > > I > > > > > > > > > do. > > > > > > > > > > > > > > > > > > As for this patch... (comments below) > > > > > > > > > > > > > > > > > > On Mon, 2021-12-20 at 02:17 +0000, Lin, Wayne wrote: > > > > > > > > > > [AMD Official Use Only] > > > > > > > > > > > > > > > > > > > > Hi Lyude, > > > > > > > > > > > > > > > > > > > > No rush and thanks for your time! : ) Just want to help > > > > > > > > > > clarify > > > > > > > > > > what > > > > > > > > > > I mean that "registering a callback function" > > > > > > > > > > for CSN of disconnecting > > > > > > > > > > stream sink device (not mst branch case). Attach below the > > > > > > > > > > tentative > > > > > > > > > > patch that I planned to do. Thanks so much! > > > > > > > > > > > > > > > > > > > > Regards, > > > > > > > > > > Wayne > > > > > > > > > > --- > > > > > > > > > > .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 53 > > > > > > > > > > +++++++++++++++++++ > > > > > > > > > > drivers/gpu/drm/drm_dp_mst_topology.c | 16 +++++- > > > > > > > > > > include/drm/drm_dp_mst_helper.h | 1 + > > > > > > > > > > 3 files changed, 68 insertions(+), 2 deletions(-) > > > > > > > > > > > > > > > > > > > > diff --git > > > > > > > > > > a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_type > > > > > > > > > > s.c > > > > > > > > > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_type > > > > > > > > > > s.c > > > > > > > > > > index cc34a35d0bcb..d7343c64908c 100644 > > > > > > > > > > --- > > > > > > > > > > a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_type > > > > > > > > > > s.c > > > > > > > > > > +++ > > > > > > > > > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_type > > > > > > > > > > s.c > > > > > > > > > > @@ -472,8 +472,61 @@ dm_dp_add_mst_connector(struct > > > > > > > > > > drm_dp_mst_topology_mgr *mgr, > > > > > > > > > > return connector; > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > +void dm_dp_notify_csn_disconnection(struct drm_connector > > > > > > > > > > +*connector) { > > > > > > > > > > + struct amdgpu_dm_connector *aconnector = > > > > > > > > > > + to_amdgpu_dm_connector(connector); > > > > > > > > > > + struct dc_sink *dc_sink = aconnector->dc_sink; > > > > > > > > > > + struct dc_link *dc_link = aconnector->dc_link; > > > > > > > > > > + struct amdgpu_device *adev = drm_to_adev(ddev); > > > > > > > > > > + > > > > > > > > > > + ASSERT(dc_link); > > > > > > > > > > + > > > > > > > > > > + if (dc_sink) { > > > > > > > > > > + mutex_lock(&adev->dm.dc_lock); > > > > > > > > > > + > > > > > > > > > > + /*clear the remote sink of the link*/ > > > > > > > > > > + dc_link_remove_remote_sink(dc_link, > > > > > > > > > > dc_sink); > > > > > > > > > > + dc_sink_release(dc_sink); > > > > > > > > > > + aconnector->dc_sink = NULL; > > > > > > > > > > + > > > > > > > > > > + mutex_unlock(&adev->dm.dc_lock); > > > > > > > > > > + } > > > > > > > > > > +} > > > > > > > > > > + > > > > > > > > > > static const struct drm_dp_mst_topology_cbs dm_mst_cbs = > > > > > > > > > > { > > > > > > > > > > .add_connector = dm_dp_add_mst_connector, > > > > > > > > > > + .notify_csn_disconnection = > > > > > > > > > > dm_dp_notify_csn_disconnection, > > > > > > > > > > }; > > > > > > > > > > > > > > > > > > I still don't really think this is a good idea. This seems > > > > > > > > > like > > > > > > > > > it's > > > > > > > > > just adding another hotplugging path to the code in order to > > > > > > > > > avoid > > > > > > > > > sending hotplugs for non-endpoint devices. In addition to > > > > > > > > > the > > > > > > > > > > > > > > So, what I wanted to do is to add a callback function for > > > > > > > endpoint > > > > > > > devices > > > > > > > case as described above. For disconnection of non-endpoint > > > > > > > devices > > > > > > > is > > > > > > > already taken care by current code. > > > > > > > > > > > > > > > > drm_connector issues I mentioned before, we also really need > > > > > > > > > to > > > > > > > > > stop > > > > > > > > > doing any kind of payload maintence in hotplugging paths. > > > > > > > > > The > > > > > > > > > reality > > > > > > > > > is any kind of payload maintanence we do outside of normal > > > > > > > > > modesetting > > > > > > > > > paths is a relic from legacy modesetting that I'm dropping > > > > > > > > > ASAP > > > > > > > > > (more > > > > > > > > > on this below), and we can't keep adding to it because it > > > > > > > > > dramatically > > > > > > > > > complicates maintanence as well. > > > > > > > > > > > > > > > > > > > > > > > > > Sorry for repeating this point so often but - the biggest > > > > > > > > > issue > > > > > > > > > too > > > > > > > > > is > > > > > > > > > I'm still not sure what it is we're even avoiding here. We > > > > > > > > > know > > > > > > > > > resources aren't released consistently, and that we're able > > > > > > > > > to > > > > > > > > > reproduce the behavior with repeated hotplugs. We also know > > > > > > > > > that > > > > > > > > > if > > > > > > > > > we > > > > > > > > > skip sending certain hotplug events, that fixes the issue. > > > > > > > > > And > > > > > > > > > we > > > > > > > > > know > > > > > > > > > we can workaround it by adding a special case for forcing a > > > > > > > > > payload > > > > > > > > > release in DC. But none of those actually tell us exactly > > > > > > > > > what > > > > > > > > > piece > > > > > > > > > of code is leaking and why, which means that any workarounds > > > > > > > > > we're > > > > > > > > > putting in to avoid this mysterious guilty section of code > > > > > > > > > we > > > > > > > > > don't > > > > > > > > > entirely understand either - which means we're just adding > > > > > > > > > more > > > > > > > > > code > > > > > > > > > in that no one actually fully understands. This just ends up > > > > > > > > > making > > > > > > > > > maintence difficult because every change in code nearby > > > > > > > > > workarounds > > > > > > > > > like this has to strugle to try to figure out said > > > > > > > > > workarounds > > > > > > > > > in > > > > > > > > > order to avoid breaking things. > > > > > > > > > > > > > > Thank you Lyude for the reminder and I totally agree with you by > > > > > > > avoiding > > > > > > > adding workaround since it will get the code maintenance harder. > > > > > > > And > > > > > > > I > > > > > > > think the root cause of the issue that I observed before is > > > > > > > quite > > > > > > > concrete. > > > > > > > There is no code path for us to release dc_sink under the unplug > > > > > > > scenario > > > > > > > that I described above. > > > > > > > > > > > > > > > > > > > > > > > > > I'm actually currently running into these "later" issues > > > > > > > > > right > > > > > > > > > now, > > > > > > > > > as > > > > > > > > > recently I've (-actively-, finally!!!) been working on > > > > > > > > > trying to > > > > > > > > > remove as much non-atomic MST as I can because. As it turns > > > > > > > > > out > > > > > > > > > - > > > > > > > > > a > > > > > > > > > huge amount of the payload maintanence code just isn't > > > > > > > > > needed at > > > > > > > > > all > > > > > > > > > when we stop caring about non-atomic drivers and stick > > > > > > > > > everything > > > > > > > > > in > > > > > > > > > atomic state structs. Step 1 for updating updating the > > > > > > > > > payload > > > > > > > > > table, > > > > > > > > > e.g. drm_dp_update_payload_part1(), is a great example of > > > > > > > > > how > > > > > > > > > messy > > > > > > > > > things have become. Here's a small sample of some of the > > > > > > > > > stuff > > > > > > > > > I've > > > > > > > > > seen from just that one function so far that either just > > > > > > > > > don't > > > > > > > > > make > > > > > > > > > sense here or is totally redundant. I should note that a lot > > > > > > > > > of > > > > > > > > > these > > > > > > > > > things also come from patches I reviwed, but didn't really > > > > > > > > > look > > > > > > > > > at > > > > > > > > > as > > > > > > > > > closely as I should have because I was swamped at work, some > > > > > > > > > are > > > > > > > > > historical artifacts, and others are less-than-ideal patches > > > > > > > > > I > > > > > > > > > got > > > > > > > > > wrote myself when I was first started working on MST and > > > > > > > > > didn't > > > > > > > > > know > > > > > > > > > the code as well as I do > > > > > > > > > now: > > > > > > > > > > > > > > > > > > * We try to avoid some sort of userspace issue by using > > > > > > > > > drm_dp_mst_port_downstream_of_branch() to avoid releasing > > > > > > > > > payloads > > > > > > > > > for a > > > > > > > > > branch if we can't prove it's downstream of the top of > > > > > > > > > the > > > > > > > > > topology. This > > > > > > > > > seems to workaround either a userspace bug. This is a > > > > > > > > > redundant, > > > > > > > > > since > > > > > > > > > that's what topology refs are already supposed to be > > > > > > > > > doing to > > > > > > > > > the > > > > > > > > > extent is > > > > > > > > > reasonably possibly. It's also unfortunately racy > > > > > > > > > solution > > > > > > > > > because > > > > > > > > > we have > > > > > > > > > to be able to handle the situation where a connector is > > > > > > > > > removed > > > > > > > > > from under > > > > > > > > > us. That can happen at any time, including _immediately_ > > > > > > > > > after > > > > > > > > > we > > > > > > > > > call > > > > > > > > > drm_dp_mst_port_downstream_of_branch() - rendering the > > > > > > > > > call > > > > > > > > > not > > > > > > > > > really > > > > > > > > > useful. > > > > > > > > > * If we fail to validate the sink in > > > > > > > > > drm_dp_update_payload_part, > > > > > > > > > we > > > > > > > > > don't > > > > > > > > > update the payload table. I think at best this solution > > > > > > > > > is > > > > > > > > > racy > > > > > > > > > and > > > > > > > > > not > > > > > > > > > useful, at worst it leaves us with a payload table that > > > > > > > > > doesn't > > > > > > > > > match what > > > > > > > > > we attempted to set in the atomic state - which at worst > > > > > > > > > brings > > > > > > > > > us > > > > > > > > > into > > > > > > > > > undefined territory where we're just plain out of sync > > > > > > > > > with > > > > > > > > > the > > > > > > > > > reality in > > > > > > > > > hw. > > > > > > > > > * Actually fun fact - mgr->payloads and mgr->proposed_vcpis > > > > > > > > > both > > > > > > > > > can > > > > > > > > > and > > > > > > > > > definitely should be removed entirely. All of the info > > > > > > > > > for > > > > > > > > > mgr->payloads > > > > > > > > > could just be in the atomic state, because that + the > > > > > > > > > magic > > > > > > > > > of > > > > > > > > > atomic state > > > > > > > > > duplication means we'll also have an accurate view of the > > > > > > > > > previous > > > > > > > > > state's > > > > > > > > > payload allocations: which renders mgr->proposed_vcpis > > > > > > > > > redundant. > > > > > > > > > > > > > > > > > > Apologies for the long explanation again, but I hope that > > > > > > > > > explains > > > > > > > > > my > > > > > > > > > > > > > > Really really thanks for your kindly help on this : ) > > > > > > > I'll try my best to get to your WIP branch soon. Thank you > > > > > > > Lyude! > > > > > > > > > > > > > > > > point here a bit. I'm going to be trying to get to moving > > > > > > > > > amdgpu's > > > > > > > > > DSC > > > > > > > > > code out of amdgpu and into DRM helpers as well soon, so I'm > > > > > > > > > really > > > > > > > > > determined to clean stuff up beforehand as every time I've > > > > > > > > > done > > > > > > > > > so > > > > > > > > > it's become substantially easier to make changes to this > > > > > > > > > code. > > > > > > > > > Things > > > > > > > > > used to be even worse before I started cleaning things up 2 > > > > > > > > > or 3 > > > > > > > > > years > > > > > > > > > ago, where simple changes would end up getting me stuck > > > > > > > > > spending > > > > > > > > > hours > > > > > > > > > trying to dig through lockdep or memory manegement issues. > > > > > > > > > As > > > > > > > > > well, > > > > > > > > > I > > > > > > > > > would be entirely unsurprised if bugs like this very payload > > > > > > > > > leak > > > > > > > > > we're working on just disappear once we've gotten rid of all > > > > > > > > > the > > > > > > > > > extraneous workarounds and state tracking here - especially > > > > > > > > > with > > > > > > > > > how > > > > > > > > > many special cases we have for maintaining the payload table > > > > > > > > > right > > > > > > > > > now. That's certainly ended up being the case in the past > > > > > > > > > with a > > > > > > > > > number of other difficult to track down issues I've dealt > > > > > > > > > with > > > > > > > > > in > > > > > > > > > MST. > > > > > > > > > > > > > > > > > > Anyhow. I've been way more productive this year then last > > > > > > > > > because > > > > > > > > > I > > > > > > > > > had over a month off work and am finally not super burnt out > > > > > > > > > from > > > > > > > > > my > > > > > > > > > job, and so far I've been making progress on the payload > > > > > > > > > state > > > > > > > > > cleanup > > > > > > > > > far faster then I was last year :). I think if you can't > > > > > > > > > figure > > > > > > > > > out > > > > > > > > > where the leak is coming from even once I get the debugging > > > > > > > > > patches > > > > > > > > > I > > > > > > > > > mentioned fixed up, it might be a good idea for us to try > > > > > > > > > again > > > > > > > > > after > > > > > > > > > I've got some of this code cleaned up. I've got a currently > > > > > > > > > WIP > > > > > > > > > branch > > > > > > > > > here: > > > > > > > > > > > > > > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl > > > > > > > > > ab.freedesktop.org%2Flyudess%2Flinux%2F- > > > > > > > > > %2Fcommit%2F624dd68fa804e64b5b > > > > > > > > > 2060e4735d7317090692b5&data=04%7C01%7Cwayne.lin%40amd.co > > > > > > > > > m%7C > > > > > > > > > d4 > > > > > > > > > bd > > > > > > > > > c7 > > > > > > > > > 59eb274bfccc8208d9daef41fe%7C3dd8961fe4884e608e11a82d994e183 > > > > > > > > > d%7C > > > > > > > > > 0% > > > > > > > > > 7C > > > > > > > > > 0% > > > > > > > > > 7C637781543416430632%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA > > > > > > > > > wMDA > > > > > > > > > iL > > > > > > > > > CJ > > > > > > > > > QI > > > > > > > > > joiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=n > > > > > > > > > FpVl > > > > > > > > > %2 > > > > > > > > > BV > > > > > > > > > oF > > > > > > > > > 40JPabfRMcR6Cz7ZHDwt%2BBpLDBNdXftJaA%3D&reserved=0 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > void amdgpu_dm_initialize_dp_connector(struct > > > > > > > > > > amdgpu_display_manager *dm, diff --git > > > > > > > > > > a/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > > > > > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > > > > > > index 857c5d15e81d..a70e26c5d084 100644 > > > > > > > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > > > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > > > > > > @@ -2508,6 +2508,7 @@ drm_dp_mst_handle_conn_stat(struct > > > > > > > > > > drm_dp_mst_branch *mstb, > > > > > > > > > > u8 new_pdt; > > > > > > > > > > bool new_mcs; > > > > > > > > > > bool dowork = false, create_connector = false; > > > > > > > > > > + bool notify_unplug_event = false; > > > > > > > > > > > > > > > > > > > > port = drm_dp_get_port(mstb, conn_stat- > > > > > > > > > > >port_number); > > > > > > > > > > if (!port) > > > > > > > > > > @@ -2520,6 +2521,9 @@ drm_dp_mst_handle_conn_stat(struct > > > > > > > > > > drm_dp_mst_branch *mstb, > > > > > > > > > > * port, so just throw the port > > > > > > > > > > out > > > > > > > > > > and > > > > > > > > > > make > > > > > > > > > > sure we > > > > > > > > > > * reprobe the link address of > > > > > > > > > > it's > > > > > > > > > > parent > > > > > > > > > > MSTB > > > > > > > > > > */ > > > > > > > > > > + /* should also consider > > > > > > > > > > notify_unplug_event > > > > > > > > > > here. > > > > > > > > > > + * but it's not a normal case for > > > > > > > > > > products > > > > > > > > > > +in the > > > > > > > > > > market > > > > > > > > > > + */ > > > > > > > > > > > > > > > > > > > > drm_dp_mst_topology_unlink_port(mgr, > > > > > > > > > > port); > > > > > > > > > > mstb->link_address_sent = false; > > > > > > > > > > dowork = true; @@ -2541,10 > > > > > > > > > > +2545,14 @@ > > > > > > > > > > drm_dp_mst_handle_conn_stat(struct > > > > > > > > > > drm_dp_mst_branch > > > > > > > > > > *mstb, > > > > > > > > > > port->ddps = conn_stat- > > > > > > > > > > > displayport_device_plug_status; > > > > > > > > > > > > > > > > > > > > if (old_ddps != port->ddps) { > > > > > > > > > > - if (port->ddps && !port->input) > > > > > > > > > > + if (port->ddps && !port->input) { > > > > > > > > > > > > > > > > > > > > drm_dp_send_enum_path_resources(mgr, > > > > > > > > > > mstb, > > > > > > > > > > port); > > > > > > > > > > - else > > > > > > > > > > + } else { > > > > > > > > > > port->full_pbn = 0; > > > > > > > > > > + if (port->connector && > > > > > > > > > > + > > > > > > > > > > drm_dp_mst_is_end_device(port- > > > > > > > > > > > pdt, > > > > > > > > > > +port- > > > > > > > > > > > mcs)) > > > > > > > > > > + notify_unplug_event = > > > > > > > > > > true; > > > > > > > > > > + } > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > new_pdt = port->input ? DP_PEER_DEVICE_NONE : > > > > > > > > > > conn_stat- > > > > > > > > > > > peer_device_type; > > > > > > > > > > @@ -2557,11 +2565,15 @@ drm_dp_mst_handle_conn_stat(struct > > > > > > > > > > drm_dp_mst_branch > > > > > > > > > > *mstb, > > > > > > > > > > dowork = false; > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > + if (notify_unplug_event && > > > > > > > > > > +mgr->cbs->notify_csn_disconnection) > > > > > > > > > > + mgr->cbs->notify_csn_disconnection(port- > > > > > > > > > > > connector); > > > > > > > > > > + > > > > > > > > > > if (port->connector) > > > > > > > > > > drm_modeset_unlock(&mgr->base.lock); > > > > > > > > > > else if (create_connector) > > > > > > > > > > drm_dp_mst_port_add_connector(mstb, port); > > > > > > > > > > > > > > > > > > > > + > > > > > > > > > > out: > > > > > > > > > > drm_dp_mst_topology_put_port(port); > > > > > > > > > > if (dowork) > > > > > > > > > > diff --git a/include/drm/drm_dp_mst_helper.h > > > > > > > > > > b/include/drm/drm_dp_mst_helper.h index > > > > > > > > > > 78044ac5b59b..ff9e47729841 > > > > > > > > > > 100644 > > > > > > > > > > --- a/include/drm/drm_dp_mst_helper.h > > > > > > > > > > +++ b/include/drm/drm_dp_mst_helper.h > > > > > > > > > > @@ -525,6 +525,7 @@ struct drm_dp_mst_topology_cbs { > > > > > > > > > > * IRQ pulse handler. > > > > > > > > > > */ > > > > > > > > > > void (*poll_hpd_irq)(struct > > > > > > > > > > drm_dp_mst_topology_mgr > > > > > > > > > > *mgr); > > > > > > > > > > + void (*notify_csn_disconnection)(struct > > > > > > > > > > drm_connector > > > > > > > > > > +*connector); > > > > > > > > > > }; > > > > > > > > > > > > > > > > > > > > #define DP_MAX_PAYLOAD (sizeof(unsigned long) * 8) > > > > > > > > > > -- > > > > > > > > > > 2.31.0 > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > > > From: Lin, Wayne > > > > > > > > > > > Sent: Wednesday, December 8, 2021 11:39 AM > > > > > > > > > > > To: 'Lyude Paul' <lyude(a)redhat.com> > > > > > > > > > > > Subject: RE: [PATCH] WIP: drm/dp_mst: Add support for > > > > > > > > > > > dumping > > > > > > > > > > > topology ref histories from debugfs > > > > > > > > > > > > > > > > > > > > > > No worries Lyude! > > > > > > > > > > > Thanks for keeping helping on this. Take your time : ) > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > > > > From: Lyude Paul <lyude(a)redhat.com> > > > > > > > > > > > > Sent: Wednesday, December 8, 2021 7:05 AM > > > > > > > > > > > > To: Lin, Wayne <Wayne.Lin(a)amd.com> > > > > > > > > > > > > Subject: Re: [PATCH] WIP: drm/dp_mst: Add support for > > > > > > > > > > > > dumping > > > > > > > > > > > > topology ref histories from debugfs > > > > > > > > > > > > > > > > > > > > > > > > Sorry! I will try to get to this tomorrow, if not then > > > > > > > > > > > > sometime > > > > > > > > > > > > this week. > > > > > > > > > > > > > > > > > > > > > > > > On Tue, 2021-11-30 at 08:41 +0000, Lin, Wayne wrote: > > > > > > > > > > > > > [Public] > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Lyude, > > > > > > > > > > > > > > > > > > > > > > > > > > Finally have some bandwidth to get back to this > > > > > > > > > > > > > problem! > > > > > > > > > > > > > I roughly went through this patch and I'm just aware > > > > > > > > > > > > > that > > > > > > > > > > > > > we > > > > > > > > > > > > > already have such kind of convenient tool for a > > > > > > > > > > > > > while. > > > > > > > > > > > > > I think it's definitely useful for us to track > > > > > > > > > > > > > port/mstb > > > > > > > > > > > > > reference count issues and I'll start to embrace > > > > > > > > > > > > > this > > > > > > > > > > > > > feature > > > > > > > > > > > > > for cleaning up those issues. Thank you Lyude! > > > > > > > > > > > > > > > > > > > > > > > > > > However, I think the issue that I was trying to fix > > > > > > > > > > > > > is > > > > > > > > > > > > > not > > > > > > > > > > > > > related to what you suggested: > > > > > > > > > > > > > " The idea here is that if stream resources aren't > > > > > > > > > > > > > being > > > > > > > > > > > > > released, my guess would be that we're not dropping > > > > > > > > > > > > > topology > > > > > > > > > > > > > references for the port which means the connector > > > > > > > > > > > > > never > > > > > > > > > > > > > goes > > > > > > > > > > > > > away." > > > > > > > > > > > > > The issue I was trying to fix is about releasing > > > > > > > > > > > > > dc_link->remote_sinks while receiving a CSN message > > > > > > > > > > > > > notifying > > > > > > > > > > > > > the connection status of a sst connector of a port > > > > > > > > > > > > > changed > > > > > > > > > > > > > from connected to disconnected. Not the connection > > > > > > > > > > > > > status > > > > > > > > > > > > > changed of a mst branch device. > > > > > > > > > > > > > e.g. > > > > > > > > > > > > > src - 1st_mstb - 2nd_mstb - sst_monitor => src - > > > > > > > > > > > > > 1st_mstb > > > > > > > > > > > > > (disconnect) 2nd_mstb - sst_monitor > > > > > > > > > > > > > > > > > > > > > > > > > > In above case, after receiving CSN, we will put > > > > > > > > > > > > > topology > > > > > > > > > > > > > references of 2nd mstb and its port which is > > > > > > > > > > > > > connected > > > > > > > > > > > > > with > > > > > > > > > > > > > the sst monitor. As the result of that, we can call > > > > > > > > > > > > > drm_dp_delayed_destroy_port() to unregister and put > > > > > > > > > > > > > the > > > > > > > > > > > > > drm > > > > > > > > > > > > > connector. > > > > > > > > > > > > > > > > > > > > > > > > > > However, in below case: > > > > > > > > > > > > > e.g. > > > > > > > > > > > > > src - 1st_mstb - sst_monitor => src - 1st_mstb > > > > > > > > > > > > > (disconnect) > > > > > > > > > > > > > sst_monitor > > > > > > > > > > > > > > > > > > > > > > > > > > In this case, which is the case having problem, it > > > > > > > > > > > > > definitely > > > > > > > > > > > > > won't decrease the topology references count of the > > > > > > > > > > > > > port > > > > > > > > > > > > > which > > > > > > > > > > > > > was connected to the sst monitor to zero since the > > > > > > > > > > > > > port > > > > > > > > > > > > > is > > > > > > > > > > > > > still existing in the topology. Same as the malloc > > > > > > > > > > > > > reference > > > > > > > > > > > > > since the port can't get destroyed. Hence, the port > > > > > > > > > > > > > still > > > > > > > > > > > > > exists and we won't call > > > > > > > > > > > > > drm_dp_delayed_destroy_port() to unregister and put > > > > > > > > > > > > > the > > > > > > > > > > > > > drm > > > > > > > > > > > > > connector. > > > > > > > > > > > > > I looked up the code and > > > > > > > > > > > > > drm_dp_delayed_destroy_port() > > > > > > > > > > > > > seems > > > > > > > > > > > > > like the only place to call drm_connector_put() > > > > > > > > > > > > > which > > > > > > > > > > > > > means > > > > > > > > > > > > > we > > > > > > > > > > > > > can't put reference count of drm connector under > > > > > > > > > > > > > this > > > > > > > > > > > > > case > > > > > > > > > > > > > and > > > > > > > > > > > > > can't release dc_sink resource by destroying drm > > > > > > > > > > > > > connector. > > > > > > > > > > > > > > > > > > > > > > > > > > I would also like to point out that this resource > > > > > > > > > > > > > (remote_sinks) is specific to different stream > > > > > > > > > > > > > sinks. So > > > > > > > > > > > > > if > > > > > > > > > > > > > we're trying to release this dc_sink resource by > > > > > > > > > > > > > destroying > > > > > > > > > > > > > the drm connector, it conflicts the idea that you > > > > > > > > > > > > > suggested > > > > > > > > > > > > > before that we should always keep the drm connector > > > > > > > > > > > > > until > > > > > > > > > > > > > it's > > > > > > > > > > > > > no longer reachable in the topology. > > > > > > > > > > > > > Releasing dc_sink should be binding with the > > > > > > > > > > > > > disconnection > > > > > > > > > > > > > event. > > > > > > > > > > > > > > > > > > > > > > > > > > I understand your concern that we should not just > > > > > > > > > > > > > easily > > > > > > > > > > > > > change the logic here since it's the result after > > > > > > > > > > > > > solving > > > > > > > > > > > > > tons > > > > > > > > > > > > > of bugs before and might cause other side effect. > > > > > > > > > > > > > So, > > > > > > > > > > > > > just > > > > > > > > > > > > > my > > > > > > > > > > > > > 2 cents, what I'm thinking is to register a callback > > > > > > > > > > > > > function > > > > > > > > > > > > > for our driver to notify us that the remote sink is > > > > > > > > > > > > > detached. > > > > > > > > > > > > > This just aligns our flow handling long HPD event of > > > > > > > > > > > > > legacy > > > > > > > > > > > > > (sst) DP. > > > > > > > > > > > > > For sst case, once we detect long HPD event > > > > > > > > > > > > > indicating > > > > > > > > > > > > > the > > > > > > > > > > > > > monitor is detached, we will immediately try to > > > > > > > > > > > > > release > > > > > > > > > > > > > the > > > > > > > > > > > > > dc_link->local_sink and fire hotplug event to upper > > > > > > > > > > > > > layer. > > > > > > > > > > > > > Same as here, once receives a CSN message notifying > > > > > > > > > > > > > a > > > > > > > > > > > > > drm > > > > > > > > > > > > > connector is changed from connected to disconnected, > > > > > > > > > > > > > trigger > > > > > > > > > > > > > the callback function and we can try to release the > > > > > > > > > > > > > dc_sink > > > > > > > > > > > > > resource. > > > > > > > > > > > > > > > > > > > > > > > > > > Would like to know your thought and insight please : > > > > > > > > > > > > > ) > > > > > > > > > > > > > > > > > > > > > > > > > > Btw, I got some errors and warnings while building > > > > > > > > > > > > > and > > > > > > > > > > > > > have > > > > > > > > > > > > > some code adjustments as below : ) Thank you Lyude > > > > > > > > > > > > > for > > > > > > > > > > > > > your > > > > > > > > > > > > > always kindly help!! > > > > > > > > > > > > > > > > > > > > > > > > > > Regards, > > > > > > > > > > > > > Wayne > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > > > > > > From: Lyude Paul <lyude(a)redhat.com> > > > > > > > > > > > > > > Sent: Wednesday, November 3, 2021 7:15 AM > > > > > > > > > > > > > > To: Lin, Wayne <Wayne.Lin(a)amd.com> > > > > > > > > > > > > > > Subject: [PATCH] WIP: drm/dp_mst: Add support for > > > > > > > > > > > > > > dumping > > > > > > > > > > > > > > topology ref histories from debugfs > > > > > > > > > > > > > > > > > > > > > > > > > > > > TODO: > > > > > > > > > > > > > > * Implement support for i915 > > > > > > > > > > > > > > * Finish writing this commit message > > > > > > > > > > > > > > * ??? > > > > > > > > > > > > > > * profit > > > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Lyude Paul <lyude(a)redhat.com> > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hey wayne! SO-hopefully if I did this correctly > > > > > > > > > > > > > > then > > > > > > > > > > > > > > this > > > > > > > > > > > > > > should just work on amdgpu. What this patch should > > > > > > > > > > > > > > do > > > > > > > > > > > > > > is > > > > > > > > > > > > > > add > > > > > > > > > > > > > > a debugfs file to amdgpu called > > > > > > > > > > > > > > "amdgpu_dp_mst_topology_refs", and when you read > > > > > > > > > > > > > > the > > > > > > > > > > > > > > file > > > > > > > > > > > > > > it > > > > > > > > > > > > > > should print out the topology reference history of > > > > > > > > > > > > > > every > > > > > > > > > > > > > > MSTB and Port in memory, along with how many times > > > > > > > > > > > > > > we've > > > > > > > > > > > > > > hit > > > > > > > > > > > > > > the codepath in each backtrace. An example: > > > > > > > > > > > > > > > > > > > > > > > > > > > > Port DP-5 (0000000005c37748) topology ref history: > > > > > > > > > > > > > > 1 gets (last at 58.468973): > > > > > > > > > > > > > > drm_dp_send_link_address+0x6a5/0xa00 > > > > > > > > > > > > > > [drm_kms_helper] > > > > > > > > > > > > > > drm_dp_check_and_send_link_address+0xad/0xd0 > > > > > > > > > > > > > > [drm_kms_helper] > > > > > > > > > > > > > > drm_dp_mst_link_probe_work+0x14e/0x1a0 > > > > > > > > > > > > > > [drm_kms_helper] > > > > > > > > > > > > > > process_one_work+0x1e3/0x390 > > > > > > > > > > > > > > worker_thread+0x50/0x3a0 > > > > > > > > > > > > > > kthread+0x124/0x150 > > > > > > > > > > > > > > ret_from_fork+0x1f/0x30 > > > > > > > > > > > > > > 1 puts (last at 58.469357): > > > > > > > > > > > > > > drm_dp_mst_topology_put_port+0x6a/0x210 > > > > > > > > > > > > > > [drm_kms_helper] > > > > > > > > > > > > > > drm_dp_send_link_address+0x39e/0xa00 > > > > > > > > > > > > > > [drm_kms_helper] > > > > > > > > > > > > > > drm_dp_check_and_send_link_address+0xad/0xd0 > > > > > > > > > > > > > > [drm_kms_helper] > > > > > > > > > > > > > > drm_dp_mst_link_probe_work+0x14e/0x1a0 > > > > > > > > > > > > > > [drm_kms_helper] > > > > > > > > > > > > > > process_one_work+0x1e3/0x390 > > > > > > > > > > > > > > worker_thread+0x50/0x3a0 > > > > > > > > > > > > > > kthread+0x124/0x150 > > > > > > > > > > > > > > ret_from_fork+0x1f/0x30 > > > > > > > > > > > > > > > > > > > > > > > > > > > > The idea here is that if stream resources aren't > > > > > > > > > > > > > > being > > > > > > > > > > > > > > released, my guess would be that we're not > > > > > > > > > > > > > > dropping > > > > > > > > > > > > > > topology > > > > > > > > > > > > > > references for the port which means the connector > > > > > > > > > > > > > > never > > > > > > > > > > > > > > goes > > > > > > > > > > > > > > away. So, if that's really the case then once we > > > > > > > > > > > > > > unplug > > > > > > > > > > > > > > the > > > > > > > > > > > > > > offending connector we should be able to find a > > > > > > > > > > > > > > pair > > > > > > > > > > > > > > of > > > > > > > > > > > > > > gets/puts for the offending leaked connector where > > > > > > > > > > > > > > the > > > > > > > > > > > > > > get/put count doesn't match up. Also, if the frame > > > > > > > > > > > > > > count > > > > > > > > > > > > > > on > > > > > > > > > > > > > > the backtrace isn't long enough you can increase > > > > > > > > > > > > > > the > > > > > > > > > > > > > > value > > > > > > > > > > > > > > of STACK_DEPTH in > > > > > > > > > > > > > > drivers/gpu/drm/drm_dp_mst_topology.c > > > > > > > > > > > > > > and > > > > > > > > > > > > > > recompile to get more frames. > > > > > > > > > > > > > > > > > > > > > > > > > > > > To enable this, first enable CONFIG_EXPERT for > > > > > > > > > > > > > > your > > > > > > > > > > > > > > kernel > > > > > > > > > > > > > > which will unhide > > > > > > > > > > > > > > CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS. > > > > > > > > > > > > > > Then just enable > > > > > > > > > > > > > > CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS, > > > > > > > > > > > > > > recompile, and it should be good to go. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Let me know if this works for you, and hopefully > > > > > > > > > > > > > > this > > > > > > > > > > > > > > should > > > > > > > > > > > > > > tell us exactly what the problem actually is here. > > > > > > > > > > > > > > > > > > > > > > > > > > > > .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | > > > > > > > > > > > > > > 35 > > > > > > > > > > > > > > ++++ > > > > > > > > > > > > > > drivers/gpu/drm/drm_dp_mst_topology.c | > > > > > > > > > > > > > > 173 > > > > > > > > > > > > > > ++++++++++++++---- > > > > > > > > > > > > > > drivers/gpu/drm/nouveau/nouveau_debugfs.c | > > > > > > > > > > > > > > 35 > > > > > > > > > > > > > > ++++ > > > > > > > > > > > > > > include/drm/drm_dp_mst_helper.h | > > > > > > > > > > > > > > 18 > > > > > > > > > > > > > > ++ > > > > > > > > > > > > > > 4 files changed, 228 insertions(+), 33 > > > > > > > > > > > > > > deletions(-) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git > > > > > > > > > > > > > > a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_ > > > > > > > > > > > > > > debu > > > > > > > > > > > > > > gf > > > > > > > > > > > > > > s. > > > > > > > > > > > > > > c > > > > > > > > > > > > > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_ > > > > > > > > > > > > > > debu > > > > > > > > > > > > > > gf > > > > > > > > > > > > > > s. > > > > > > > > > > > > > > c > > > > > > > > > > > > > > index 1a68a674913c..1a14732c52b4 100644 > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_ > > > > > > > > > > > > > > debu > > > > > > > > > > > > > > gf > > > > > > > > > > > > > > s. > > > > > > > > > > > > > > c > > > > > > > > > > > > > > +++ > > > > > > > > > > > > > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_ > > > > > > > > > > > > > > debu > > > > > > > > > > > > > > gf > > > > > > > > > > > > > > +++ s.c > > > > > > > > > > > > > > @@ -3063,6 +3063,37 @@ static int > > > > > > > > > > > > > > mst_topo_show(struct > > > > > > > > > > > > > > seq_file *m, void > > > > > > > > > > > > > > *unused) > > > > > > > > > > > > > > return 0; > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > +#ifdef CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS > > > > > > > > > > > > > > +static int mst_topology_ref_dump_show(struct > > > > > > > > > > > > > > seq_file > > > > > > > > > > > > > > *m, > > > > > > > > > > > > > > +void > > > > > > > > > > > > > > +*unused) { > > > > > > > > > > > > > > + struct amdgpu_device *adev = (struct > > > > > > > > > > > > > > amdgpu_device > > > > > > > > > > > > > > +*)m->private; > > > > > > > > > > > > > > + struct drm_device *dev = adev_to_drm(adev); > > > > > > > > > > > > > > + struct drm_connector *connector; > > > > > > > > > > > > > > + struct drm_connector_list_iter conn_iter; > > > > > > > > > > > > > > + struct amdgpu_dm_connector *aconnector; > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + drm_connector_list_iter_begin(dev, > > > > > > > > > > > > > > &conn_iter); > > > > > > > > > > > > > > + drm_for_each_connector_iter(connector, > > > > > > > > > > > > > > &conn_iter) > > > > > > > > > > > > > > { > > > > > > > > > > > > > > + if (connector->connector_type != > > > > > > > > > > > > > > DRM_MODE_CONNECTOR_DisplayPort) > > > > > > > > > > > > > > + continue; > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + aconnector = > > > > > > > > > > > > > > +to_amdgpu_dm_connector(connector); > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + /* Ensure we're only dumping the > > > > > > > > > > > > > > topology > > > > > > > > > > > > > > of > > > > > > > > > > > > > > a > > > > > > > > > > > > > > +root mst node > > > > > > > > > > > > > > */ > > > > > > > > > > > > > > + if (!aconnector- > > > > > > > > > > > > > > >mst_mgr.max_payloads) > > > > > > > > > > > > > > + continue; > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + seq_printf(m, "\nMST topology for > > > > > > > > > > > > > > connector > > > > > > > > > > > > > > +%d\n", > > > > > > > > > > > > > > aconnector->connector_id); > > > > > > > > > > > > > > + drm_dp_mst_dump_topology_refs(m, > > > > > > > > > > > > > > +&aconnector->mst_mgr); > > > > > > > > > > > > > > + } > > > > > > > > > > > > > > + drm_connector_list_iter_end(&conn_iter); > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + return 0; > > > > > > > > > > > > > > +} > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +DEFINE_SHOW_ATTRIBUTE(mst_topology_ref_dump); > > > > > > > > > > > > > > +#endif > > > > > > > > > > > > > > + > > > > > > > > > > > > > > /* > > > > > > > > > > > > > > * Sets trigger hpd for MST topologies. > > > > > > > > > > > > > > * All connected connectors will be rediscovered > > > > > > > > > > > > > > and > > > > > > > > > > > > > > re > > > > > > > > > > > > > > started as needed if val of 1 is sent. > > > > > > > > > > > > > > @@ -3299,6 +3330,10 @@ void > > > > > > > > > > > > > > dtn_debugfs_init(struct > > > > > > > > > > > > > > amdgpu_device > > > > > > > > > > > > > > *adev) > > > > > > > > > > > > > > > > > > > > > > > > > > > > debugfs_create_file("amdgpu_mst_topology", > > > > > > > > > > > > > > 0444, > > > > > > > > > > > > > > root, > > > > > > > > > > > > > > adev, &mst_topo_fops); > > > > > > > > > > > > > > +#ifdef CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS > > > > > > > > > > > > > > + > > > > > > > > > > > > > > debugfs_create_file("amdgpu_dp_mst_topology_refs", > > > > > > > > > > > > > > +0444, root, adev, > > > > > > > > > > > > > > + > > > > > > > > > > > > > > &mst_topology_ref_dump_fops); > > > > > > > > > > > > > > +#endif > > > > > > > > > > > > > > debugfs_create_file("amdgpu_dm_dtn_log", > > > > > > > > > > > > > > 0644, > > > > > > > > > > > > > > root, > > > > > > > > > > > > > > adev, > > > > > > > > > > > > > > &dtn_log_fops); > > > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > > > > > > > > > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > > > > > > > > > > index 1aa8702383d4..0159828c494d 100644 > > > > > > > > > > > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > > > > > > > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > > > > > > > > > > > > > @@ -1366,23 +1366,6 @@ static int > > > > > > > > > > > > > > drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch > > > > > > > > > > > > > > *mstb, > > > > > > > > > > > > > > return ret; > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > -static struct drm_dp_mst_branch > > > > > > > > > > > > > > *drm_dp_add_mst_branch_device(u8 lct, u8 > > > > > > > > > > > > > > *rad) -{ > > > > > > > > > > > > > > - struct drm_dp_mst_branch *mstb; > > > > > > > > > > > > > > - > > > > > > > > > > > > > > - mstb = kzalloc(sizeof(*mstb), GFP_KERNEL); > > > > > > > > > > > > > > - if (!mstb) > > > > > > > > > > > > > > - return NULL; > > > > > > > > > > > > > > - > > > > > > > > > > > > > > - mstb->lct = lct; > > > > > > > > > > > > > > - if (lct > 1) > > > > > > > > > > > > > > - memcpy(mstb->rad, rad, lct / 2); > > > > > > > > > > > > > > - INIT_LIST_HEAD(&mstb->ports); > > > > > > > > > > > > > > - kref_init(&mstb->topology_kref); > > > > > > > > > > > > > > - kref_init(&mstb->malloc_kref); > > > > > > > > > > > > > > - return mstb; > > > > > > > > > > > > > > -} > > > > > > > > > > > > > > - > > > > > > > > > > > > > > static void drm_dp_free_mst_branch_device(struct > > > > > > > > > > > > > > kref > > > > > > > > > > > > > > *kref) { > > > > > > > > > > > > > > struct drm_dp_mst_branch *mstb = @@ -1642,12 > > > > > > > > > > > > > > +1625,20 > > > > > > > > > > > > > > @@ topology_ref_type_to_str(enum > > > > > > > > > > > > > > drm_dp_mst_topology_ref_type type) > > > > > > > > > > > > > > return "put"; > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > +static const char > > > > > > > > > > > > > > *topology_ref_history_type_to_str(enum > > > > > > > > > > > > > > +drm_dp_mst_topology_history_type type) { > > > > > > > > > > > > > > + if (type == > > > > > > > > > > > > > > DRM_DP_MST_TOPOLOGY_HISTORY_PORT) > > > > > > > > > > > > > > + return "Port"; > > > > > > > > > > > > > > + else > > > > > > > > > > > > > > + return "MSTB"; } > > > > > > > > > > > > > > + > > > > > > > > > > > > > > static void > > > > > > > > > > > > > > -__dump_topology_ref_history(struct > > > > > > > > > > > > > > drm_dp_mst_topology_ref_history *history, > > > > > > > > > > > > > > - void *ptr, const char > > > > > > > > > > > > > > *type_str) > > > > > > > > > > > > > > +dump_topology_ref_history(struct > > > > > > > > > > > > > > +drm_dp_mst_topology_ref_history *history, struct > > > > > > > > > > > > > > +drm_printer p) > > > > > > > > > > > > > > { > > > > > > > > > > > > > > - struct drm_printer p = > > > > > > > > > > > > > > drm_debug_printer(DBG_PREFIX); > > > > > > > > > > > > > > + char *connector_name = NULL; > > > > > > > > > > > > > > char *buf = kzalloc(PAGE_SIZE, GFP_KERNEL); > > > > > > > > > > > > > > + void *ptr; > > > > > > > > > > > > > > int i; > > > > > > > > > > > > > > > > > > > > > > > > > > > > if (!buf) > > > > > > > > > > > > > > @@ -1656,14 +1647,29 @@ > > > > > > > > > > > > > > __dump_topology_ref_history(struct > > > > > > > > > > > > > > drm_dp_mst_topology_ref_history *history, > > > > > > > > > > > > > > if (!history->len) > > > > > > > > > > > > > > goto out; > > > > > > > > > > > > > > > > > > > > > > > > > > > > + /* Get a pointer to the actual MSTB/port so > > > > > > > > > > > > > > we > > > > > > > > > > > > > > can > > > > > > > > > > > > > > the > > > > > > > > > > > > > > +memory > > > > > > > > > > > > > > address to the kernel log */ > > > > > > > > > > > > > > + if (history->type == > > > > > > > > > > > > > > DRM_DP_MST_TOPOLOGY_HISTORY_PORT) > > > > > > > > > > > > > > + ptr = container_of(history, struct > > > > > > > > > > > > > > +drm_dp_mst_port, > > > > > > > > > > > > > > topology_ref_history); > > > > > > > > > > > > > > + else > > > > > > > > > > > > > > + ptr = container_of(history, struct > > > > > > > > > > > > > > +drm_dp_mst_branch, topology_ref_history); > > > > > > > > > > > > > > + > > > > > > > > > > > > > > /* First, sort the list so that it goes from > > > > > > > > > > > > > > oldest > > > > > > > > > > > > > > to > > > > > > > > > > > > > > newest > > > > > > > > > > > > > > * reference entry > > > > > > > > > > > > > > */ > > > > > > > > > > > > > > sort(history->entries, history->len, > > > > > > > > > > > > > > sizeof(*history->entries), > > > > > > > > > > > > > > topology_ref_history_cmp, NULL); > > > > > > > > > > > > > > > > > > > > > > > > > > > > - drm_printf(&p, "%s (%p) topology count > > > > > > > > > > > > > > reached > > > > > > > > > > > > > > 0, > > > > > > > > > > > > > > dumping history:\n", > > > > > > > > > > > > > > - type_str, ptr); > > > > > > > > > > > > > > + if (history->type == > > > > > > > > > > > > > > DRM_DP_MST_TOPOLOGY_HISTORY_PORT) > > > > > > > > > > > > > > +{ > > > > > > > > > > > > > > + struct drm_dp_mst_port *port = ptr; > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + if (port->connector) > > > > > > > > > > > > > > + connector_name = > > > > > > > > > > > > > > +port->connector->name; > > > > > > > > > > > > > > + } > > > > > > > > > > > > > > + if (connector_name) > > > > > > > > > > > > > > + drm_printf(&p, "Port %s (%p) > > > > > > > > > > > > > > topology > > > > > > > > > > > > > > ref > > > > > > > > > > > > > > +history:\n", > > > > > > > > > > > > > > connector_name, ptr); > > > > > > > > > > > > > > + else > > > > > > > > > > > > > > + drm_printf(&p, "%s (%p) topology ref > > > > > > > > > > > > > > +history:\n", > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +topology_ref_history_type_to_str(history->type), > > > > > > > > > > > > > > ptr); > > > > > > > > > > > > > > > > > > > > > > > > > > > > for (i = 0; i < history->len; i++) { > > > > > > > > > > > > > > const struct > > > > > > > > > > > > > > drm_dp_mst_topology_ref_entry > > > > > > > > > > > > > > *entry = @@ > > > > > > > > > > > > > > - > > > > > > > > > > > > > > 1682,24 +1688,92 @@ > > > > > > > > > > > > > > __dump_topology_ref_history(struct > > > > > > > > > > > > > > drm_dp_mst_topology_ref_history *history, > > > > > > > > > > > > > > ts_nsec, rem_nsec / 1000, > > > > > > > > > > > > > > buf); > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > - /* Now free the history, since this is the > > > > > > > > > > > > > > only > > > > > > > > > > > > > > time > > > > > > > > > > > > > > we expose it */ > > > > > > > > > > > > > > - kfree(history->entries); > > > > > > > > > > > > > > out: > > > > > > > > > > > > > > kfree(buf); > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > +/** > > > > > > > > > > > > > > + * drm_dp_mst_dump_topology_refs - helper > > > > > > > > > > > > > > function > > > > > > > > > > > > > > for > > > > > > > > > > > > > > +dumping the topology ref history > > > > > > > > > > > > > > + * @m: File to print to > > > > > > > > > > > > > > + * @mgr: &struct drm_dp_mst_topology_mgr to use > > > > > > > > > > > > > > + * > > > > > > > > > > > > > > + * Prints the topology ref history of all ports > > > > > > > > > > > > > > and > > > > > > > > > > > > > > MSTBs > > > > > > > > > > > > > > +on @mgr that are still in memory, > > > > > > > > > > > > > > + * regardless of whether they're actually still > > > > > > > > > > > > > > reachable > > > > > > > > > > > > > > +through the topology or not. Only enabled > > > > > > > > > > > > > > + * when %CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS is > > > > > > > > > > > > > > enabled. > > > > > > > > > > > > > > +Can be implemented by drivers to assist > > > > > > > > > > > > > > + * with debugging leaks in the DP MST helpers. > > > > > > > > > > > > > > + */ > > > > > > > > > > > > > > +void drm_dp_mst_dump_topology_refs(struct > > > > > > > > > > > > > > seq_file > > > > > > > > > > > > > > *m, > > > > > > > > > > > > > > +struct drm_dp_mst_topology_mgr *mgr) { > > > > > > > > > > > > > > + struct drm_dp_mst_topology_ref_history > > > > > > > > > > > > > > *history; > > > > > > > > > > > > > > + struct drm_printer p = > > > > > > > > > > > > > > drm_seq_file_printer(m); > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + mutex_lock(&mgr->topology_ref_history_lock); > > > > > > > > > > > > > > + list_for_each_entry(history, > > > > > > > > > > > > > > +&mgr->topology_ref_history_list, > > > > > > > > > > > > > > +node) > > > > > > > > > > > > > > + dump_topology_ref_history(history, > > > > > > > > > > > > > > p); > > > > > > > > > > > > > > + mutex_unlock(&mgr- > > > > > > > > > > > > > > >topology_ref_history_lock); > > > > > > > > > > > > > > +} > > > > > > > > > > > > > > +EXPORT_SYMBOL(drm_dp_mst_dump_topology_refs); > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +static void > > > > > > > > > > > > > > +__init_topology_ref_history(struct > > > > > > > > > > > > > > +drm_dp_mst_topology_ref_history > > > > > > > > > > > > > > *history, > > > > > > > > > > > > > > + struct > > > > > > > > > > > > > > drm_dp_mst_topology_mgr > > > > > > > > > > > > > > +*mgr, > > > > > > > > > > > > > > + enum > > > > > > > > > > > > > > +drm_dp_mst_topology_history_type > > > > > > > > > > > > > > +type) { > > > > > > > > > > > > > > + history->type = type; > > > > > > > > > > > > > > + INIT_LIST_HEAD(&history->node); > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + mutex_lock(&mgr->topology_ref_history_lock); > > > > > > > > > > > > > > + list_add(&history->node, > > > > > > > > > > > > > > +&mgr->topology_ref_history_list); > > > > > > > > > > > > > > + mutex_unlock(&mgr- > > > > > > > > > > > > > > >topology_ref_history_lock); > > > > > > > > > > > > > > +} > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +static void > > > > > > > > > > > > > > +__destroy_topology_ref_history(struct > > > > > > > > > > > > > > +drm_dp_mst_topology_ref_history > > > > > > > > > > > > > > *history, > > > > > > > > > > > > > > + struct > > > > > > > > > > > > > > drm_dp_mst_topology_mgr > > > > > > > > > > > > > > +*mgr) { > > > > > > > > > > > > > > + mutex_lock(&mgr->topology_ref_history_lock); > > > > > > > > > > > > > > + list_del(&mgr->topology_ref_history_list); > > > > > > > > > > > > > > + mutex_unlock(&mgr- > > > > > > > > > > > > > > >topology_ref_history_lock); > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + kfree(history->entries); } > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +static __always_inline void > > > > > > > > > > > > > > +init_port_topology_history(struct > > > > > > > > > > > > > > drm_dp_mst_topology_mgr > > > > > > > > > > > > > > +*mgr, struct drm_dp_mst_port *port) { > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +__init_topology_ref_history(&port- > > > > > > > > > > > > > > > topology_ref_history, > > > > > > > > > > > > > > +mgr, > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +DRM_DP_MST_TOPOLOGY_HISTORY_PORT); > > > > > > > > > > > > > > +} > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +static __always_inline void > > > > > > > > > > > > > > +init_mstb_topology_history(struct > > > > > > > > > > > > > > drm_dp_mst_topology_mgr > > > > > > > > > > > > > > +*mgr, struct drm_dp_mst_branch *mstb) { > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +__init_topology_ref_history(&mstb- > > > > > > > > > > > > > > > topology_ref_history, > > > > > > > > > > > > > > +mgr, > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +DRM_DP_MST_TOPOLOGY_HISTORY_MSTB); > > > > > > > > > > > > > > +} > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +static __always_inline void > > > > > > > > > > > > > > +destroy_port_topology_history(struct > > > > > > > > > > > > > > drm_dp_mst_port > > > > > > > > > > > > > > *port) > > > > > > > > > > > > > > +{ > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +__destroy_topology_ref_history(&port- > > > > > > > > > > > > > > > topology_ref_history, > > > > > > > > > > > > > > +port->mgr); } > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +static __always_inline void > > > > > > > > > > > > > > +destroy_mstb_topology_history(struct > > > > > > > > > > > > > > drm_dp_mst_branch > > > > > > > > > > > > > > +*mstb) { > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +__destroy_topology_ref_history(&mstb- > > > > > > > > > > > > > > > topology_ref_history, > > > > > > > > > > > > > > +mstb->mgr); } > > > > > > > > > > > > > > + > > > > > > > > > > > > > > static __always_inline void > > > > > > > > > > > > > > dump_mstb_topology_history(struct > > > > > > > > > > > > > > drm_dp_mst_branch > > > > > > > > > > > > > > *mstb) > > > > > > > > > > > > > > { > > > > > > > > > > > > > > - > > > > > > > > > > > > > > __dump_topology_ref_history(&mstb- > > > > > > > > > > > > > > > topology_ref_history, > > > > > > > > > > > > > > mstb, > > > > > > > > > > > > > > - "MSTB"); > > > > > > > > > > > > > > + dump_topology_ref_history(&mstb- > > > > > > > > > > > > > > > topology_ref_history, > > > > > > > > > > > > > > +drm_debug_printer(DBG_PREFIX)); > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > static __always_inline void > > > > > > > > > > > > > > dump_port_topology_history(struct drm_dp_mst_port > > > > > > > > > > > > > > *port) > > > > > > > > > > > > > > { > > > > > > > > > > > > > > - > > > > > > > > > > > > > > __dump_topology_ref_history(&port- > > > > > > > > > > > > > > > topology_ref_history, > > > > > > > > > > > > > > port, > > > > > > > > > > > > > > - "Port"); > > > > > > > > > > > > > > + dump_topology_ref_history(&port- > > > > > > > > > > > > > > > topology_ref_history, > > > > > > > > > > > > > > +drm_debug_printer(DBG_PREFIX)); > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > static __always_inline void @@ -1729,6 +1803,14 > > > > > > > > > > > > > > @@ > > > > > > > > > > > > > > topology_ref_history_unlock(struct > > > > > > > > > > > > > > drm_dp_mst_topology_mgr *mgr) } #else static > > > > > > > > > > > > > > inline > > > > > > > > > > > > > > void > > > > > > > > > > > > > > +init_port_topology_history(struct > > > > > > > > > > > > > > drm_dp_mst_topology_mgr > > > > > > > > > > > > > > +*mgr, struct drm_dp_mst_port *port); static > > > > > > > > > > > > > > inline > > > > > > > > > > > > > > void > > > > > > > > > > > > > Should also add the bracket, otherwise will get > > > > > > > > > > > > > warnings. > > > > > > > > > > > > > => static inline void > > > > > > > > > > > > > init_port_topology_history(struct > > > > > > > > > > > > > drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port > > > > > > > > > > > > > *port) > > > > > > > > > > > > > {}; > > > > > > > > > > > > > > > > > > > > > > > > > > > +init_mstb_topology_history(struct > > > > > > > > > > > > > > drm_dp_mst_topology_mgr > > > > > > > > > > > > > > +*mgr, struct drm_dp_mst_branch *mstb); static > > > > > > > > > > > > > > inline > > > > > > > > > > > > > > void > > > > > > > > > > > > > Same as above > > > > > > > > > > > > > => static inline void > > > > > > > > > > > > > init_mstb_topology_history(struct > > > > > > > > > > > > > drm_dp_mst_topology_mgr *mgr, struct > > > > > > > > > > > > > drm_dp_mst_branch > > > > > > > > > > > > > *mstb) > > > > > > > > > > > > > {}; > > > > > > > > > > > > > > > > > > > > > > > > > > > +destroy_port_topology_history(struct > > > > > > > > > > > > > > +drm_dp_mst_topology_mgr *mgr, struct > > > > > > > > > > > > > > drm_dp_mst_port > > > > > > > > > > > > > > +*port); static inline void > > > > > > > > > > > > > destroy_port_topology_history() takes one parameter > > > > > > > > > > > > > only > > > > > > > > > > > > > which > > > > > > > > > > > > > is port. > > > > > > > > > > > > > => destroy_port_topology_history(struct > > > > > > > > > > > > > drm_dp_mst_port > > > > > > > > > > > > > *port) > > > > > > > > > > > > > {}; > > > > > > > > > > > > > > > > > > > > > > > > > > > +destroy_mstb_topology_history(struct > > > > > > > > > > > > > > +drm_dp_mst_topology_mgr *mgr, struct > > > > > > > > > > > > > > drm_dp_mst_branch > > > > > > > > > > > > > > +*mstb); static inline void > > > > > > > > > > > > > destroy_mstb_topology_history() takes one parameter > > > > > > > > > > > > > only > > > > > > > > > > > > > which > > > > > > > > > > > > > is mstb => destroy_mstb_topology_history(struct > > > > > > > > > > > > > drm_dp_mst_branch > > > > > > > > > > > > > *mstb) {}; > > > > > > > > > > > > > > > > > > > > > > > > > > > topology_ref_history_lock(struct > > > > > > > > > > > > > > drm_dp_mst_topology_mgr > > > > > > > > > > > > > > *mgr) {} static inline void > > > > > > > > > > > > > > topology_ref_history_unlock(struct > > > > > > > > > > > > > > drm_dp_mst_topology_mgr *mgr) {} @@ -1740,6 > > > > > > > > > > > > > > +1822,25 > > > > > > > > > > > > > > @@ > > > > > > > > > > > > > > dump_port_topology_history(struct drm_dp_mst_port > > > > > > > > > > > > > > *port) > > > > > > > > > > > > > > {} > > > > > > > > > > > > > > #define save_port_topology_ref(port, type) #endif > > > > > > > > > > > > > > > > > > > > > > > > > > > > +static struct drm_dp_mst_branch * > > > > > > > > > > > > > > +drm_dp_add_mst_branch_device(struct > > > > > > > > > > > > > > drm_dp_mst_topology_mgr > > > > > > > > > > > > > > +*mgr, > > > > > > > > > > > > > > +u8 lct, u8 *rad) { > > > > > > > > > > > > > > + struct drm_dp_mst_branch *mstb; > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + mstb = kzalloc(sizeof(*mstb), GFP_KERNEL); > > > > > > > > > > > > > > + if (!mstb) > > > > > > > > > > > > > > + return NULL; > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + mstb->lct = lct; > > > > > > > > > > > > > > + if (lct > 1) > > > > > > > > > > > > > > + memcpy(mstb->rad, rad, lct / 2); > > > > > > > > > > > > > > + INIT_LIST_HEAD(&mstb->ports); > > > > > > > > > > > > > > + kref_init(&mstb->topology_kref); > > > > > > > > > > > > > > + kref_init(&mstb->malloc_kref); > > > > > > > > > > > > > > + init_mstb_topology_history(mgr, mstb); > > > > > > > > > > > > > > + return mstb; > > > > > > > > > > > > > > +} > > > > > > > > > > > > > > + > > > > > > > > > > > > > > static void > > > > > > > > > > > > > > drm_dp_destroy_mst_branch_device(struct > > > > > > > > > > > > > > kref > > > > > > > > > > > > > > *kref) { > > > > > > > > > > > > > > struct drm_dp_mst_branch *mstb = @@ -1747,6 > > > > > > > > > > > > > > +1848,7 > > > > > > > > > > > > > > @@ > > > > > > > > > > > > > > static void > > > > > > > > > > > > > > drm_dp_destroy_mst_branch_device(struct > > > > > > > > > > > > > > kref *kref) > > > > > > > > > > > > > > struct drm_dp_mst_topology_mgr *mgr = mstb- > > > > > > > > > > > > > > >mgr; > > > > > > > > > > > > > > > > > > > > > > > > > > > > dump_mstb_topology_history(mstb); > > > > > > > > > > > > > > + destroy_mstb_topology_history(mstb); > > > > > > > > > > > > > > > > > > > > > > > > > > > > INIT_LIST_HEAD(&mstb->destroy_next); > > > > > > > > > > > > > > > > > > > > > > > > > > > > @@ -1856,6 +1958,7 @@ static void > > > > > > > > > > > > > > drm_dp_destroy_port(struct > > > > > > > > > > > > > > kref > > > > > > > > > > > > > > *kref) > > > > > > > > > > > > > > struct drm_dp_mst_topology_mgr *mgr = port- > > > > > > > > > > > > > > >mgr; > > > > > > > > > > > > > > > > > > > > > > > > > > > > dump_port_topology_history(port); > > > > > > > > > > > > > > + destroy_port_topology_history(port); > > > > > > > > > > > > > > > > > > > > > > > > > > > > /* There's nothing that needs locking to > > > > > > > > > > > > > > destroy > > > > > > > > > > > > > > an > > > > > > > > > > > > > > input port yet */ > > > > > > > > > > > > > > if (port->input) { > > > > > > > > > > > > > > @@ -2135,7 +2238,7 @@ drm_dp_port_set_pdt(struct > > > > > > > > > > > > > > drm_dp_mst_port *port, u8 new_pdt, > > > > > > > > > > > > > > ret = > > > > > > > > > > > > > > drm_dp_mst_register_i2c_bus(port); > > > > > > > > > > > > > > } else { > > > > > > > > > > > > > > lct = > > > > > > > > > > > > > > drm_dp_calculate_rad(port, > > > > > > > > > > > > > > rad); > > > > > > > > > > > > > > - mstb = > > > > > > > > > > > > > > drm_dp_add_mst_branch_device(lct, rad); > > > > > > > > > > > > > > + mstb = > > > > > > > > > > > > > > +drm_dp_add_mst_branch_device(mgr, lct, rad); > > > > > > > > > > > > > > if (!mstb) { > > > > > > > > > > > > > > ret = -ENOMEM; > > > > > > > > > > > > > > drm_err(mgr->dev, > > > > > > > > > > > > > > "Failed > > > > > > > > > > > > > > to > > > > > > > > > > > > > > create MSTB for port %p", port); @@ -2366,6 > > > > > > > > > > > > > > +2469,8 @@ > > > > > > > > > > > > > > drm_dp_mst_add_port(struct drm_device *dev, > > > > > > > > > > > > > > */ > > > > > > > > > > > > > > drm_dp_mst_get_mstb_malloc(mstb); > > > > > > > > > > > > > > > > > > > > > > > > > > > > + init_port_topology_history(mgr, port); > > > > > > > > > > > > > > + > > > > > > > > > > > > > > return port; > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > @@ -3745,7 +3850,7 @@ int > > > > > > > > > > > > > > drm_dp_mst_topology_mgr_set_mst(struct > > > > > > > > > > > > > > drm_dp_mst_topology_mgr *mgr, bool ms > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > /* add initial branch device at LCT > > > > > > > > > > > > > > 1 */ > > > > > > > > > > > > > > - mstb = > > > > > > > > > > > > > > drm_dp_add_mst_branch_device(1, > > > > > > > > > > > > > > NULL); > > > > > > > > > > > > > > + mstb = > > > > > > > > > > > > > > drm_dp_add_mst_branch_device(mgr, > > > > > > > > > > > > > > 1, > > > > > > > > > > > > > > +NULL); > > > > > > > > > > > > > > if (mstb == NULL) { > > > > > > > > > > > > > > ret = -ENOMEM; > > > > > > > > > > > > > > goto out_unlock; @@ -5512,14 > > > > > > > > > > > > > > +5617,16 > > > > > > > > > > > > > > @@ int drm_dp_mst_topology_mgr_init(struct > > > > > > > > > > > > > > drm_dp_mst_topology_mgr *mgr, > > > > > > > > > > > > > > mutex_init(&mgr->delayed_destroy_lock); > > > > > > > > > > > > > > mutex_init(&mgr->up_req_lock); > > > > > > > > > > > > > > mutex_init(&mgr->probe_lock); -#if > > > > > > > > > > > > > > IS_ENABLED(CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS) > > > > > > > > > > > > > > - mutex_init(&mgr->topology_ref_history_lock); > > > > > > > > > > > > > > -#endif > > > > > > > > > > > > > > INIT_LIST_HEAD(&mgr->tx_msg_downq); > > > > > > > > > > > > > > INIT_LIST_HEAD(&mgr->destroy_port_list); > > > > > > > > > > > > > > INIT_LIST_HEAD(&mgr- > > > > > > > > > > > > > > > destroy_branch_device_list); > > > > > > > > > > > > > > INIT_LIST_HEAD(&mgr->up_req_list); > > > > > > > > > > > > > > > > > > > > > > > > > > > > +#if > > > > > > > > > > > > > > IS_ENABLED(CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS) > > > > > > > > > > > > > > + mutex_init(&mgr->topology_ref_history_lock); > > > > > > > > > > > > > > + INIT_LIST_HEAD(&mgr- > > > > > > > > > > > > > > >topology_ref_history_list); > > > > > > > > > > > > > > +#endif > > > > > > > > > > > > > > + > > > > > > > > > > > > > > /* > > > > > > > > > > > > > > * delayed_destroy_work will be queued on a > > > > > > > > > > > > > > dedicated > > > > > > > > > > > > > > WQ, so that any > > > > > > > > > > > > > > * requeuing will be also flushed when > > > > > > > > > > > > > > deiniting > > > > > > > > > > > > > > the > > > > > > > > > > > > > > topology manager. > > > > > > > > > > > > > > diff --git > > > > > > > > > > > > > > a/drivers/gpu/drm/nouveau/nouveau_debugfs.c > > > > > > > > > > > > > > b/drivers/gpu/drm/nouveau/nouveau_debugfs.c > > > > > > > > > > > > > > index 1cbe01048b93..53ec7c1daada 100644 > > > > > > > > > > > > > > --- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c > > > > > > > > > > > > > > +++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c > > > > > > > > > > > > > > @@ -29,9 +29,13 @@ > > > > > > > > > > > > > > */ > > > > > > > > > > > > > > > > > > > > > > > > > > > > #include <linux/debugfs.h> > > > > > > > > > > > > > > +#include <drm/drm_dp_mst_helper.h> #include > > > > > > > > > > > > > > +<drm/drm_encoder.h> > > > > > > > > > > > > > > #include <nvif/class.h> > > > > > > > > > > > > > > #include <nvif/if0001.h> > > > > > > > > > > > > > > +#include <nouveau_encoder.h> > > > > > > > > > > > > > > #include "nouveau_debugfs.h" > > > > > > > > > > > > > > +#include "nouveau_display.h" > > > > > > > > > > > > > > #include "nouveau_drv.h" > > > > > > > > > > > > > > > > > > > > > > > > > > > > static int > > > > > > > > > > > > > > @@ -68,6 +72,34 @@ > > > > > > > > > > > > > > nouveau_debugfs_strap_peek(struct > > > > > > > > > > > > > > seq_file *m, void > > > > > > > > > > > > > > *data) > > > > > > > > > > > > > > return 0; > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > +#ifdef CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS > > > > > > > > > > > > > > +static int > > > > > > > > > > > > > > nouveau_debugfs_mst_topology_history(struct > > > > > > > > > > > > > > +seq_file *m, void *data) { > > > > > > > > > > > > > > + struct drm_info_node *node = m->private; > > > > > > > > > > > > > > + struct drm_device *dev = node->minor->dev; > > > > > > > > > > > > > > + struct drm_encoder *encoder; > > > > > > > > > > > > > > + struct nouveau_display *disp = > > > > > > > > > > > > > > nouveau_display(dev); > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + if (disp->disp.object.oclass < GF110_DISP) > > > > > > > > > > > > > > + return -EINVAL; > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + drm_for_each_encoder(encoder, dev) { > > > > > > > > > > > > > > + struct nv50_mstm *mstm; > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + /* We need the real encoder for each > > > > > > > > > > > > > > MST > > > > > > > > > > > > > > mgr > > > > > > > > > > > > > > +*/ > > > > > > > > > > > > > > + if (encoder->encoder_type != > > > > > > > > > > > > > > +DRM_MODE_ENCODER_TMDS) > > > > > > > > > > > > > > + continue; > > > > > > > > > > > > > > + mstm = nouveau_encoder(encoder)- > > > > > > > > > > > > > > > dp.mstm; > > > > > > > > > > > > > > + if (!mstm) > > > > > > > > > > > > > > + continue; > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + seq_printf(m, "MSTM on %s:\n", > > > > > > > > > > > > > > encoder- > > > > > > > > > > > > > > > name); > > > > > > > > > > > > > > + drm_dp_mst_dump_topology_refs(m, > > > > > > > > > > > > > > &mstm- > > > > > > > > > > > > > > > mgr); > > > > > > > > > > > > > > + } > > > > > > > > > > > > > > + return 0; > > > > > > > > > > > > > > +} > > > > > > > > > > > > > > +#endif > > > > > > > > > > > > > > + > > > > > > > > > > > > > > static int > > > > > > > > > > > > > > nouveau_debugfs_pstate_get(struct seq_file *m, > > > > > > > > > > > > > > void > > > > > > > > > > > > > > *data) > > > > > > > > > > > > > > { @@ > > > > > > > > > > > > > > -213,6 > > > > > > > > > > > > > > +245,9 @@ static const struct file_operations > > > > > > > > > > > > > > nouveau_pstate_fops = { static struct > > > > > > > > > > > > > > drm_info_list > > > > > > > > > > > > > > nouveau_debugfs_list[] = { > > > > > > > > > > > > > > { "vbios.rom", nouveau_debugfs_vbios_image, > > > > > > > > > > > > > > 0, > > > > > > > > > > > > > > NULL > > > > > > > > > > > > > > }, > > > > > > > > > > > > > > { "strap_peek", nouveau_debugfs_strap_peek, > > > > > > > > > > > > > > 0, > > > > > > > > > > > > > > NULL > > > > > > > > > > > > > > }, > > > > > > > > > > > > > > +#ifdef CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS > > > > > > > > > > > > > > + { "dp_mst_topology_refs", > > > > > > > > > > > > > > +nouveau_debugfs_mst_topology_history, 0, NULL }, > > > > > > > > > > > > > > #endif > > > > > > > > > > > > > > }; > > > > > > > > > > > > > > #define NOUVEAU_DEBUGFS_ENTRIES > > > > > > > > > > > > > > ARRAY_SIZE(nouveau_debugfs_list) > > > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/include/drm/drm_dp_mst_helper.h > > > > > > > > > > > > > > b/include/drm/drm_dp_mst_helper.h index > > > > > > > > > > > > > > 78044ac5b59b..e3a73d8d97c0 > > > > > > > > > > > > > > 100644 > > > > > > > > > > > > > > --- a/include/drm/drm_dp_mst_helper.h > > > > > > > > > > > > > > +++ b/include/drm/drm_dp_mst_helper.h > > > > > > > > > > > > > > @@ -35,6 +35,11 @@ enum > > > > > > > > > > > > > > drm_dp_mst_topology_ref_type { > > > > > > > > > > > > > > DRM_DP_MST_TOPOLOGY_REF_PUT, > > > > > > > > > > > > > > }; > > > > > > > > > > > > > > > > > > > > > > > > > > > > +enum drm_dp_mst_topology_history_type { > > > > > > > > > > > > > > + DRM_DP_MST_TOPOLOGY_HISTORY_PORT, > > > > > > > > > > > > > > + DRM_DP_MST_TOPOLOGY_HISTORY_MSTB, }; > > > > > > > > > > > > > > + > > > > > > > > > > > > > > struct drm_dp_mst_topology_ref_history { > > > > > > > > > > > > > > struct drm_dp_mst_topology_ref_entry { > > > > > > > > > > > > > > enum drm_dp_mst_topology_ref_type > > > > > > > > > > > > > > type; > > > > > > > > > > > > > > @@ > > > > > > > > > > > > > > -43,6 > > > > > > > > > > > > > > +48,9 @@ struct drm_dp_mst_topology_ref_history { > > > > > > > > > > > > > > depot_stack_handle_t backtrace; > > > > > > > > > > > > > > } *entries; > > > > > > > > > > > > > > int len; > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + enum drm_dp_mst_topology_history_type type; > > > > > > > > > > > > > > + struct list_head node; > > > > > > > > > > > > > > }; > > > > > > > > > > > > > > #endif /* > > > > > > > > > > > > > > IS_ENABLED(CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS) > > > > > > > > > > > > > > */ > > > > > > > > > > > > > > > > > > > > > > > > > > > > @@ -769,6 +777,12 @@ struct > > > > > > > > > > > > > > drm_dp_mst_topology_mgr { > > > > > > > > > > > > > > * &drm_dp_mst_branch.topology_ref_history. > > > > > > > > > > > > > > */ > > > > > > > > > > > > > > struct mutex topology_ref_history_lock; > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + /** > > > > > > > > > > > > > > + * @topology_ref_history_list: contains a > > > > > > > > > > > > > > list > > > > > > > > > > > > > > of > > > > > > > > > > > > > > +topology ref > > > > > > > > > > > > > > histories for any MSTBs or > > > > > > > > > > > > > > + * ports that are still in memory > > > > > > > > > > > > > > + */ > > > > > > > > > > > > > > + struct list_head topology_ref_history_list; > > > > > > > > > > > > > > #endif > > > > > > > > > > > > > > }; > > > > > > > > > > > > > > > > > > > > > > > > > > > > @@ -873,6 +887,10 @@ void > > > > > > > > > > > > > > drm_dp_mst_put_port_malloc(struct > > > > > > > > > > > > > > drm_dp_mst_port *port); > > > > > > > > > > > > > > > > > > > > > > > > > > > > struct drm_dp_aux > > > > > > > > > > > > > > *drm_dp_mst_dsc_aux_for_port(struct > > > > > > > > > > > > > > drm_dp_mst_port *port); > > > > > > > > > > > > > > > > > > > > > > > > > > > > +#ifdef CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS > > > > > > > > > > > > > > +void drm_dp_mst_dump_topology_refs(struct > > > > > > > > > > > > > > seq_file > > > > > > > > > > > > > > *m, > > > > > > > > > > > > > > +struct drm_dp_mst_topology_mgr *mgr); #endif > > > > > > > > > > > > > > + > > > > > > > > > > > > > > extern const struct drm_private_state_funcs > > > > > > > > > > > > > > drm_dp_mst_topology_state_funcs; > > > > > > > > > > > > > > > > > > > > > > > > > > > > /** > > > > > > > > > > > > > > -- > > > > > > > > > > > > > > 2.31.1 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > > Cheers, > > > > > > > > > > > > Lyude Paul (she/her) > > > > > > > > > > > > Software Engineer at Red Hat > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > Cheers, Lyude Paul (she/her) Software Engineer at Red Hat > > > > > > > -- > > > > > > > Regards, > > > > > > > Wayne Lin > > > > > > > > > > > > > > > > > > > -- > > > > > > Cheers, > > > > > > Lyude Paul (she/her) > > > > > > Software Engineer at Red Hat > > > > > > > > > > > > > > > > -- > > Cheers, > > Lyude Paul (she/her) > > Software Engineer at Red Hat > -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat

2 4

[PATCH v5 1/4] dt-bindings: display: simple: Add DataImage FG1001L0DSSWMG01 compatible string
by Philip Oberfichtner 06 May '22

06 May '22

Add DataImage FG1001L0DSSWMG01 10.1" 1280x800 TFT LCD panel compatible string. Signed-off-by: Philip Oberfichtner <pro(a)denx.de> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> --- Cc: Thierry Reding <thierry.reding(a)gmail.com> Cc: Sam Ravnborg <sam(a)ravnborg.org> Cc: David Airlie <airlied(a)linux.ie> Cc: Daniel Vetter <daniel(a)ffwll.ch> Cc: Rob Herring <robh+dt(a)kernel.org> Cc: Krzysztof Kozlowski <krzk+dt(a)kernel.org> Cc: dri-devel(a)lists.freedesktop.org Cc: devicetree(a)vger.kernel.org Cc: linux-kernel(a)vger.kernel.org Cc: matthias.winker(a)de.bosch.com --- Changes in v5: none Changes in v4: Acked-by Krzysztof Changes in v3: new --- .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml index 1eb9dd4f8f58..a6db3eff3b1a 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml @@ -105,6 +105,8 @@ properties: - chunghwa,claa101wb01 # Chunghwa Picture Tubes Ltd. 10.1" WXGA TFT LCD panel - chunghwa,claa101wb03 + # DataImage, Inc. 10.1" WXGA (1280×800) TFT LCD panel + - dataimage,fg1001l0dsswmg01 # DataImage, Inc. 7" WVGA (800x480) TFT LCD panel with 24-bit parallel interface. - dataimage,scf0700c48ggu18 # DLC Display Co. DLC1010GIG 10.1" WXGA TFT LCD Panel -- 2.34.1

1 1

[PATCH 0/2] *dma-buf: Add an API for exporting sync files (v13)
by Jason Ekstrand 06 May '22

06 May '22

Modern userspace APIs like Vulkan are built on an explicit synchronization model. This doesn't always play nicely with the implicit synchronization used in the kernel and assumed by X11 and Wayland. The client -> compositor half of the synchronization isn't too bad, at least on intel, because we can control whether or not i915 synchronizes on the buffer and whether or not it's considered written. The harder part is the compositor -> client synchronization when we get the buffer back from the compositor. We're required to be able to provide the client with a VkSemaphore and VkFence representing the point in time where the window system (compositor and/or display) finished using the buffer. With current APIs, it's very hard to do this in such a way that we don't get confused by the Vulkan driver's access of the buffer. In particular, once we tell the kernel that we're rendering to the buffer again, any CPU waits on the buffer or GPU dependencies will wait on some of the client rendering and not just the compositor. This new IOCTL solves this problem by allowing us to get a snapshot of the implicit synchronization state of a given dma-buf in the form of a sync file. It's effectively the same as a poll() or I915_GEM_WAIT only, instead of CPU waiting directly, it encapsulates the wait operation, at the current moment in time, in a sync_file so we can check/wait on it later. As long as the Vulkan driver does the sync_file export from the dma-buf before we re-introduce it for rendering, it will only contain fences from the compositor or display. This allows to accurately turn it into a VkFence or VkSemaphore without any over-synchronization. This patch series actually contains two new ioctls. There is the export one mentioned above as well as an RFC for an import ioctl which provides the other half. The intention is to land the export ioctl since it seems like there's no real disagreement on that one. The import ioctl, however, has a lot of debate around it so it's intended to be RFC-only for now. Mesa MR: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4037 IGT tests: https://patchwork.freedesktop.org/series/90490/ v10 (Jason Ekstrand, Daniel Vetter): - Add reviews/acks - Add a patch to rename _rcu to _unlocked - Split things better so import is clearly RFC status v11 (Daniel Vetter): - Add more CCs to try and get maintainers - Add a patch to document DMA_BUF_IOCTL_SYNC - Generally better docs - Use separate structs for import/export (easier to document) - Fix an issue in the import patch v12 (Daniel Vetter): - Better docs for DMA_BUF_IOCTL_SYNC v12 (Christian König): - Drop the rename patch in favor of Christian's series - Add a comment to the commit message for the dma-buf sync_file export ioctl saying why we made it an ioctl on dma-buf v13 (Jason Ekstrand): - Rebase on Christian König's fence rework Cc: Christian König <christian.koenig(a)amd.com> Cc: Michel Dänzer <michel(a)daenzer.net> Cc: Dave Airlie <airlied(a)redhat.com> Cc: Bas Nieuwenhuizen <bas(a)basnieuwenhuizen.nl> Cc: Daniel Stone <daniels(a)collabora.com> Cc: mesa-dev(a)lists.freedesktop.org Cc: wayland-devel(a)lists.freedesktop.org Jason Ekstrand (2): dma-buf: Add an API for exporting sync files (v13) dma-buf: Add an API for importing sync files (v8) drivers/dma-buf/dma-buf.c | 100 +++++++++++++++++++++++++++++++++++ include/uapi/linux/dma-buf.h | 57 ++++++++++++++++++++ 2 files changed, 157 insertions(+) -- 2.36.0

3 13

[PATCH] drm: Document that power requirements for DP AUX transfers
by Douglas Anderson 05 May '22

05 May '22

When doing DP AUX transfers there are two actors that need to be powered in order for the DP AUX transfer to work: the DP source and the DP sync. Commit bacbab58f09d ("drm: Mention the power state requirement on side-channel operations") added some documentation saying that the DP source is required to power itself up (if needed) to do AUX transfers. However, that commit doesn't talk anything about the DP sink. For full fledged DP the sink isn't really a problem. It's expected that if an external DP monitor isn't plugged in that attempting to do AUX transfers won't work. It's also expected that if a DP monitor is plugged in (and thus asserting HPD) that it AUX transfers will work. When we're looking at eDP, however, things are less obvious. Let's add some documentation about expectations. Here's what we'll say: 1. We don't expect the DP AUX transfer function to power on an eDP panel. If an eDP panel is physically connected but powered off then it makes sense for the transfer to fail. 2. We'll document that the official way to power on a panel is via the bridge chain, specifically by making sure that the panel's prepare function has been called (which is called by panel_bridge_pre_enable()). It's already specified in the kernel doc of drm_panel_prepare() that this is the way to power the panel on and also that after this call "it is possible to communicate with any integrated circuitry via a command bus." 3. We'll also document that for code running in the panel driver itself that it is legal for the panel driver to power itself up however it wants (it doesn't need to officially call drm_panel_pre_enable()) and then it can do AUX bus transfers. This is currently the way that edp-panel works when it's running atop the DP AUX bus. Signed-off-by: Douglas Anderson <dianders(a)chromium.org> --- include/drm/display/drm_dp_helper.h | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h index dca40a045dd6..e5165b708a40 100644 --- a/include/drm/display/drm_dp_helper.h +++ b/include/drm/display/drm_dp_helper.h @@ -370,9 +370,17 @@ struct drm_dp_aux { * helpers assume this is the case. * * Also note that this callback can be called no matter the - * state @dev is in. Drivers that need that device to be powered - * to perform this operation will first need to make sure it's - * been properly enabled. + * state @dev is in and also no matter what state the panel is + * in. It's expected: + * - If the @dev providing the AUX bus is currently unpowered then + * it will power itself up for the transfer. + * - If we're on eDP and the panel is not in a state where it can + * respond (it's not powered or it's in a low power state) then this + * function will return an error (but not crash). Note that if a + * panel driver is initiating a DP AUX transfer it may power itself + * up however it wants. All other code should ensure that the + * pre_enable() bridge chain (which eventually calls the panel + * prepare function) has powered the panel. */ ssize_t (*transfer)(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg); -- 2.36.0.464.gb9c8b46e94-goog

5 18

AMD display drivers handling DRM CRTC color mgmt props
by Melissa Wen 05 May '22

05 May '22

Hi all, I'm examining how DRM color management properties (degamma, ctm, gamma) are applied to AMD display drivers. As far I could understand thanks Nicholas documentation on amdgpu_dm/amdgpu_dm_color, DC drivers have per-plane color correction features: * - Input gamma LUT (de-normalized) * - Input CSC (normalized) * - Surface degamma LUT (normalized) * - Surface CSC (normalized) * - Surface regamma LUT (normalized) * - Output CSC (normalized) so DM is "adapting" those DRM per-CRTC properties to fit into three of these color correction stages, which I guess are the surface stages: * - Surface degamma LUT (normalized) * - Surface CSC (normalized) * - Surface regamma LUT (normalized) I'm trying to understand what this mapping is doing. A comment mentions that is not possible to do these color corrections after blending, so, the same color correction pipe is performed on every plane before blending? (is the surface the plane?) Does this adaptation affect the expected output? Moreover, is there something that I misunderstood? :) That said, if the DRM color mgmt supports per-CRTC 3D LUT as the last step of color correction, I don't see how to accommodate it in the mapping above, but I see DC already supports programming 3D LUT on DPP. Once DRM has the 3D LUT interface and DM mapped it as a DPP property, the 3D LUT will be at the end of the color correction pipeline? Is there anything I need to worry about mapping DRM 3D LUT support? Or any advice? Thanks in advance, Melissa

2 6

[PATCH] drm/tve200: Fix smatch warning
by Linus Walleij 05 May '22

05 May '22

The "ret" variable is ambiguously returning something that could be zero in the tve200_modeset_init() function, assign it an explicit error return code to make this unambiguous. Reported-by: Dan Carpenter <dan.carpenter(a)oracle.com> Signed-off-by: Linus Walleij <linus.walleij(a)linaro.org> --- drivers/gpu/drm/tve200/tve200_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tve200/tve200_drv.c b/drivers/gpu/drm/tve200/tve200_drv.c index 6d9d2921abf4..3f4fef574706 100644 --- a/drivers/gpu/drm/tve200/tve200_drv.c +++ b/drivers/gpu/drm/tve200/tve200_drv.c @@ -65,7 +65,7 @@ static int tve200_modeset_init(struct drm_device *dev) struct tve200_drm_dev_private *priv = dev->dev_private; struct drm_panel *panel; struct drm_bridge *bridge; - int ret = 0; + int ret; drm_mode_config_init(dev); mode_config = &dev->mode_config; @@ -93,6 +93,7 @@ static int tve200_modeset_init(struct drm_device *dev) * method to get the connector out of the bridge. */ dev_err(dev->dev, "the bridge is not a panel\n"); + ret = -EINVAL; goto out_bridge; } -- 2.35.1

1 0

[PATCH 1/2] drm/probe-helper: Add helper for drm_helper_probe_single_connector_modes()
by Douglas Anderson 05 May '22

05 May '22

The drm_helper_probe_single_connector_modes() is a bit long. Let's break a chunk off to update and validate modes. This helps avoid one goto and also will allow us to more easily call the helper a second time in a future patch without adding looping or another goto. This change is intended to be a no-op change--just code movement. Signed-off-by: Douglas Anderson <dianders(a)chromium.org> --- drivers/gpu/drm/drm_probe_helper.c | 105 ++++++++++++++++------------- 1 file changed, 59 insertions(+), 46 deletions(-) diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 682359512996..819225629010 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -354,6 +354,61 @@ drm_helper_probe_detect(struct drm_connector *connector, } EXPORT_SYMBOL(drm_helper_probe_detect); +static bool _drm_helper_update_and_validate(struct drm_connector *connector, + uint32_t maxX, uint32_t maxY, + struct drm_modeset_acquire_ctx *ctx) +{ + struct drm_device *dev = connector->dev; + struct drm_display_mode *mode; + int mode_flags = 0; + int ret; + + drm_connector_list_update(connector); + + if (connector->interlace_allowed) + mode_flags |= DRM_MODE_FLAG_INTERLACE; + if (connector->doublescan_allowed) + mode_flags |= DRM_MODE_FLAG_DBLSCAN; + if (connector->stereo_allowed) + mode_flags |= DRM_MODE_FLAG_3D_MASK; + + list_for_each_entry(mode, &connector->modes, head) { + if (mode->status != MODE_OK) + continue; + + mode->status = drm_mode_validate_driver(dev, mode); + if (mode->status != MODE_OK) + continue; + + mode->status = drm_mode_validate_size(mode, maxX, maxY); + if (mode->status != MODE_OK) + continue; + + mode->status = drm_mode_validate_flag(mode, mode_flags); + if (mode->status != MODE_OK) + continue; + + ret = drm_mode_validate_pipeline(mode, connector, ctx, + &mode->status); + if (ret) { + drm_dbg_kms(dev, + "drm_mode_validate_pipeline failed: %d\n", + ret); + + if (drm_WARN_ON_ONCE(dev, ret != -EDEADLK)) + mode->status = MODE_ERROR; + else + return true; + } + + if (mode->status != MODE_OK) + continue; + mode->status = drm_mode_validate_ycbcr420(mode, connector); + } + + return false; +} + /** * drm_helper_probe_single_connector_modes - get complete set of display modes * @connector: connector to probe @@ -421,7 +476,6 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, const struct drm_connector_helper_funcs *connector_funcs = connector->helper_private; int count = 0, ret; - int mode_flags = 0; bool verbose_prune = true; enum drm_connector_status old_status; struct drm_modeset_acquire_ctx ctx; @@ -519,52 +573,11 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, connector->status == connector_status_unknown)) count = drm_add_modes_noedid(connector, 1024, 768); count += drm_helper_probe_add_cmdline_mode(connector); - if (count == 0) - goto prune; - - drm_connector_list_update(connector); - - if (connector->interlace_allowed) - mode_flags |= DRM_MODE_FLAG_INTERLACE; - if (connector->doublescan_allowed) - mode_flags |= DRM_MODE_FLAG_DBLSCAN; - if (connector->stereo_allowed) - mode_flags |= DRM_MODE_FLAG_3D_MASK; - - list_for_each_entry(mode, &connector->modes, head) { - if (mode->status != MODE_OK) - continue; - - mode->status = drm_mode_validate_driver(dev, mode); - if (mode->status != MODE_OK) - continue; - - mode->status = drm_mode_validate_size(mode, maxX, maxY); - if (mode->status != MODE_OK) - continue; - - mode->status = drm_mode_validate_flag(mode, mode_flags); - if (mode->status != MODE_OK) - continue; - - ret = drm_mode_validate_pipeline(mode, connector, &ctx, - &mode->status); - if (ret) { - drm_dbg_kms(dev, - "drm_mode_validate_pipeline failed: %d\n", - ret); - - if (drm_WARN_ON_ONCE(dev, ret != -EDEADLK)) { - mode->status = MODE_ERROR; - } else { - drm_modeset_backoff(&ctx); - goto retry; - } + if (count != 0) { + if (_drm_helper_update_and_validate(connector, maxX, maxY, &ctx)) { + drm_modeset_backoff(&ctx); + goto retry; } - - if (mode->status != MODE_OK) - continue; - mode->status = drm_mode_validate_ycbcr420(mode, connector); } prune: -- 2.36.0.rc2.479.g8af0fa9b8e-goog

6 16

[PATCH v2 0/4] drm/i915: Prepare for GSC-loaded HuC
by Daniele Ceraolo Spurio 05 May '22

05 May '22

On newer platforms (starting DG2 G10 B-step and G11 A-step), ownership of HuC loading and authentication has been moved from the GuC to the GSC, with both actions being performed via a single PXP command. Given that the mei code has not fully landed yet (see [1]), we can't implement the new load mechanism, but we can start getting ready for it by taking care of the changes required for the existing code: - The HuC header is now different from the GuC one. This also means that if the FW is for GSC-loading and the HW fuse is set to legacy load (or vice-versa) we can't load the HuC. - To send a PXP message to the GSC we need both MEI_GSC and MEI_PXP. - All legacy HuC loading paths can be skipped. Note that the HuC fw version for DG2 is still not defined, so the HuC code will be skipped until the define is added. v2: drop changes in auth checking for legacy paths. [1] https://patchwork.freedesktop.org/series/102339/ Cc: Alan Previn <alan.previn.teres.alexis(a)intel.com> Cc: John Harrison <john.c.harrison(a)intel.com> Daniele Ceraolo Spurio (4): drm/i915/huc: drop intel_huc_is_authenticated drm/i915/huc: Add fetch support for gsc-loaded HuC binary drm/i915/huc: Prepare for GSC-loaded HuC drm/i915/huc: Don't fail the probe if HuC init fails drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 1 + drivers/gpu/drm/i915/gt/uc/intel_huc.c | 97 +++++++++++++++---- drivers/gpu/drm/i915/gt/uc/intel_huc.h | 5 +- drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 5 +- drivers/gpu/drm/i915/gt/uc/intel_uc.c | 22 +++-- drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 99 ++++++++++++-------- drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 2 + drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h | 9 ++ 8 files changed, 172 insertions(+), 68 deletions(-) -- 2.25.1

2 6

[PATCH] drm/nouveau/gr/gf100-: Clean up some inconsistent indenting
by Jiapeng Chong 05 May '22

05 May '22

Eliminate the follow smatch warning: drivers/gpu/drm/nouveau/nvkm/engine/gr/gf100.c:1925 gf100_gr_oneinit_tiles() warn: inconsistent indenting. Reported-by: Abaci Robot <abaci(a)linux.alibaba.com> Signed-off-by: Jiapeng Chong <jiapeng.chong(a)linux.alibaba.com> --- drivers/gpu/drm/nouveau/nvkm/engine/gr/gf100.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/gr/gf100.c b/drivers/gpu/drm/nouveau/nvkm/engine/gr/gf100.c index 397ff4fe9df8..f16eabf4f642 100644 --- a/drivers/gpu/drm/nouveau/nvkm/engine/gr/gf100.c +++ b/drivers/gpu/drm/nouveau/nvkm/engine/gr/gf100.c @@ -1922,8 +1922,8 @@ gf100_gr_oneinit_tiles(struct gf100_gr *gr) for (i = 0; i < gr->gpc_nr; i++) { init_frac[i] = gr->tpc_nr[gpc_map[i]] * gr->gpc_nr * mul_factor; - init_err[i] = i * gr->tpc_max * mul_factor - comm_denom/2; - run_err[i] = init_frac[i] + init_err[i]; + init_err[i] = i * gr->tpc_max * mul_factor - comm_denom/2; + run_err[i] = init_frac[i] + init_err[i]; } for (i = 0; i < gr->tpc_total;) { -- 2.20.1.7.g153144c

2 1

[PATCH 1/5] dma-buf: cleanup dma_fence_unwrap selftest
by Christian König 05 May '22

05 May '22

The selftests, fix the error handling, remove unused functions and stop leaking memory in failed tests. Signed-off-by: Christian König <christian.koenig(a)amd.com> --- drivers/dma-buf/st-dma-fence-unwrap.c | 40 +++++++++++---------------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c b/drivers/dma-buf/st-dma-fence-unwrap.c index 039f016b57be..59628add93f5 100644 --- a/drivers/dma-buf/st-dma-fence-unwrap.c +++ b/drivers/dma-buf/st-dma-fence-unwrap.c @@ -4,27 +4,19 @@ * Copyright (C) 2022 Advanced Micro Devices, Inc. */ +#include <linux/dma-fence.h> +#include <linux/dma-fence-array.h> +#include <linux/dma-fence-chain.h> #include <linux/dma-fence-unwrap.h> -#if 0 -#include <linux/kernel.h> -#include <linux/kthread.h> -#include <linux/mm.h> -#include <linux/sched/signal.h> -#include <linux/slab.h> -#include <linux/spinlock.h> -#include <linux/random.h> -#endif #include "selftest.h" #define CHAIN_SZ (4 << 10) -static inline struct mock_fence { +struct mock_fence { struct dma_fence base; spinlock_t lock; -} *to_mock_fence(struct dma_fence *f) { - return container_of(f, struct mock_fence, base); -} +}; static const char *mock_name(struct dma_fence *f) { @@ -45,7 +37,8 @@ static struct dma_fence *mock_fence(void) return NULL; spin_lock_init(&f->lock); - dma_fence_init(&f->base, &mock_ops, &f->lock, 0, 0); + dma_fence_init(&f->base, &mock_ops, &f->lock, + dma_fence_context_alloc(1), 1); return &f->base; } @@ -113,7 +106,6 @@ static int sanitycheck(void *arg) if (!chain) return -ENOMEM; - dma_fence_signal(f); dma_fence_put(chain); return err; } @@ -154,10 +146,10 @@ static int unwrap_array(void *arg) err = -EINVAL; } - dma_fence_signal(f1); - dma_fence_signal(f2); + dma_fence_put(f1); + dma_fence_put(f2); dma_fence_put(array); - return 0; + return err; } static int unwrap_chain(void *arg) @@ -196,10 +188,10 @@ static int unwrap_chain(void *arg) err = -EINVAL; } - dma_fence_signal(f1); - dma_fence_signal(f2); + dma_fence_put(f1); + dma_fence_put(f2); dma_fence_put(chain); - return 0; + return err; } static int unwrap_chain_array(void *arg) @@ -242,10 +234,10 @@ static int unwrap_chain_array(void *arg) err = -EINVAL; } - dma_fence_signal(f1); - dma_fence_signal(f2); + dma_fence_put(f1); + dma_fence_put(f2); dma_fence_put(chain); - return 0; + return err; } int dma_fence_unwrap(void) -- 2.25.1

3 10

← Newer
1
...
59
60
61
62
63
64
65
...
74
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

dri-devel May 2022