[Intel-gfx] [PATCH] drm/i915/display: Suspend MST topology manager before destroy fbdev

[Souza, Jose]

On Wed, 2019-11-27 at 10:43 -0800, Lucas De Marchi wrote:
> On Tue, Nov 26, 2019 at 06:16:09PM -0800, Jose Souza wrote:
> > MST do topology probe in threads, so this running threads needs to
> > be
> > flushed before fbdev is destroyed as when a new MST node is found
> > it
> > calls drm_kms_helper_hotplug_event() that calls fbdev functions
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109964
> > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 8f2770951459..372dd48691cf 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -17989,6 +17989,13 @@ void intel_modeset_driver_remove(struct
> > drm_i915_private *i915)
> > 	 */
> > 	intel_hpd_poll_fini(i915);
> > 
> > +	/*
> > +	 * MST do topology probe in threads, so this running threads
> > needs to
> > +	 * be flushed before fbdev is destroyed as when a new MST node
> > is found
> > +	 * it call drm_kms_helper_hotplug_event() that calls fbdev
> > functions
> > +	 */
> 
> this would normally be part of drm_mode_config_cleanup() in which the
> encoders will be destroyed, together with their mst_mgr via 
> drm_dp_mst_topology_mgr_destroy()
> 
> So I think the comment should actually mention why
> drm_mode_config_cleanup() can't be done before or just state where it
> will actually be destroyed. So... I guess something like:
> 
> /*
>   * MST topology needs to be suspended so we don't have any calls to
>   * fbdev after it's finalized. MST will be destroyed later as part
> of
>   * drm_mode_config_cleanup()
>   */
> 
> Of course, if we can figure out a better ordering to peel the onion
> would be better, but I think this is sufficient.

Sounds better, thanks.

> 
> With the comment update,
> 
> 
> Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>
> 
> Lucas De Marchi
> 
> > +	intel_dp_mst_suspend(i915);
> > +
> > 	/* poll work can call into fbdev, hence clean that up
> > afterwards */
> > 	intel_fbdev_fini(i915);
> > 
> > -- 
> > 2.24.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx