[PATCH 2/7] drm/dp_mst: Register AUX devices for MST ports
Lyude Paul
lyude at redhat.com
Thu Jun 6 21:07:01 UTC 2019
On Thu, 2019-06-06 at 19:41 +0000, Li, Sun peng (Leo) wrote:
>
> On 2019-06-03 3:28 p.m., Lyude Paul wrote:
> > > I'm reproducing this just by reloading i915 on a machine with some MST
> > > displays connected. I uploaded a copy of the script that I use to do
> > > this
> > > here:
> > >
> > > https://people.freedesktop.org/~lyudess/archive/06-03-2019/unloadgpumod.sh
> > oops-almost forgot to mention. The argument you pass to make it reload
> > instead
> > of just unloading is --reload
> >
>
> Thanks for the script!
>
> So, the warning has to do with:
>
> 1. Having the aux device as a child of connector device, and
> 2. During driver unload, drm_connector_unregister() is called before
> drm_dp_mst_topology_put_port()
>
> Which means that connector_unregister() will recursively remove the
> child aux device, before put_port() can properly unregister it. Any
> further attempts to remove after the first will throw a "not found" warning.
>
> Call-stacks for reference:
>
> *drm_connector_unregister*+0x37/0x60 [drm]
> drm_connector_unregister_all+0x30/0x60 [drm]
> drm_modeset_unregister_all+0xe/0x30 [drm]
> drm_dev_unregister+0x9c/0xb0 [drm]
> i915_driver_unload+0x73/0x120 [i915]
>
> drm_dp_aux_unregister_devnode+0xf5/0x180 [drm_kms_helper]
> *drm_dp_mst_topology_put_port*+0x4e/0xf0 [drm_kms_helper]
> drm_dp_mst_topology_put_mstb+0x91/0x160 [drm_kms_helper]
> drm_dp_mst_topology_mgr_set_mst+0x12b/0x2b0 [drm_kms_helper]
> ? __finish_swait+0x10/0x40
> drm_dp_mst_topology_mgr_destroy+0x11/0xa0 [drm_kms_helper]
> intel_dp_encoder_flush_work+0x32/0xb0 [i915]
> intel_ddi_encoder_destroy+0x32/0x60 [i915]
> drm_mode_config_cleanup+0x51/0x2e0 [drm]
> intel_modeset_cleanup+0xc8/0x140 [i915]
> i915_driver_unload+0xa0/0x120 [i915]
>
> A solution is to unregister the aux device immediately before the
> connector device is unregistered - if we are to keep the aux device as a
> child. Following current scheme with SST, it looks like
> drm_connector_funcs->early_unregister() is the right place to do so.
> To keep the balance, aux registration will then happen in
> drm_connector_funcs->late_register(). This will leave the SDP
> transaction handling part in DRM still, but pass the responsibility of
> creating and removing remote (fake) aux devices to the driver.
>
> I have a WIP patch here for you to take a look. It should apply on top
> of the existing patchset:
> https://pastebin.com/1YJZhL4C
>
This approach seems fine to me! I was thinking we might end up doing something
like this. My only thought is that we can probably write up a
drm_dp_mst_connector_late_register() and
drm_dp_mst_connector_early_unregister() that drivers can call from their
→late_register()/→early_unregister() callbacks that handles printing the debug
message, setting the parent and registering/unregistering the aux device. I'd
imagine someday in the future we'll probably have more things to add to those
callbacks for mst as well.
> I'd like to hear your thoughts, before I go and modify other drivers :)
>
> Thanks,
> Leo
--
Cheers,
Lyude Paul
More information about the dri-devel
mailing list