Hi again,
On Thu, Nov 19, 2015 at 05:02:04PM +0100, Daniel Vetter wrote:
> On Wed, Nov 18, 2015 at 01:43:20PM +0100, Lukas Wunner wrote:
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -408,7 +408,10 @@ static void intel_connector_add_to_fbdev(struct intel_connector *connector)
> > {
> > #ifdef CONFIG_DRM_FBDEV_EMULATION
> > struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > - drm_fb_helper_add_one_connector(&dev_priv->fbdev->helper, &connector->base);
> > +
> > + if (dev_priv->fbdev)
> > + drm_fb_helper_add_one_connector(&dev_priv->fbdev->helper,
> > + &connector->base);
> > #endif
> > }
> >
> > @@ -416,7 +419,10 @@ static void intel_connector_remove_from_fbdev(struct intel_connector *connector)
> > {
> > #ifdef CONFIG_DRM_FBDEV_EMULATION
> > struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > - drm_fb_helper_remove_one_connector(&dev_priv->fbdev->helper, &connector->base);
> > +
> > + if (dev_priv->fbdev)
> > + drm_fb_helper_remove_one_connector(&dev_priv->fbdev->helper,
> > + &connector->base);
> > #endif
> > }
> >
> Queued for -next, thanks for the patch. Aside, with this patch and the
> static inline dummies from Archit I think we can drop most of the #ifdef
> blocks (not the one in debugfs though). Care for a follow-up patch to
> remove them around add/remove_one_connector?
Looking at it once more I've realized that the fbdev member in struct
drm_i915_private is enclosed in an #ifdef CONFIG_DRM_FBDEV_EMULATION,
so we can't remove the #ifdefs here: The compiler would complain about
non-existence of the dev_priv->fbdev member.
On Thu, Nov 19, 2015 at 04:58:44PM +0100, Daniel Vetter wrote:
> On Wed, Nov 18, 2015 at 04:29:51PM +0100, Lukas Wunner wrote:
> > @@ -727,7 +730,8 @@ void intel_fbdev_fini(struct drm_device *dev)
> >
> > flush_work(&dev_priv->fbdev_suspend_work);
> >
> > - async_synchronize_full();
> > + if (!current_is_async())
> > + async_synchronize_full();
>
> I think this is a bit too fragile, and the core depency will make merging
> tricky. Can't we just push the async_synchronize_full into module unload
> for now?
Thinking about this a bit longer I believe that if anything the change
should make it more robust rather than fragile. After all we eliminate
the source of a deadlock that could occur here (async_synchronize_full()
waiting forever for itself to finish).
That said I'm not married to this solution. If you do find it concerning
I could change it according to Ville's suggestion, i.e. splitting
intel_fbdev_fini() in two parts.
Moving the async_synchronize_full() to i915_driver_unload() would be
contrary to the clarity Ville had sought by consolidating everything
in intel_fbdev.c.
Thanks & best regards,
Lukas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
