On to, 2016-03-31 at 14:57 +0100, Chris Wilson wrote: > If the initialisation fails, we may be left with a dangling pointer with > an incomplete fbdev structure. Here we want to disable internal calls > into fbdev. Similarly, the initialisation may be slow and we haven't yet > enabled the fbdev (e.g. quick suspend or last-close before the async init > completes). > > v3: To create a typo introduced when retyping > v4: Prevent info==NULL dereference in early boot > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93580 > Reported-by: "Li, Weinan Z" <weinan.z.li@xxxxxxxxx> > Tested-by: Gabriel Feceoru <gabriel.feceoru@xxxxxxxxx> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_fbdev.c | 72 +++++++++++++++++++++++++------------- > 1 file changed, 48 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c > index 153ea7a3fcf6..5d4be71bdf22 100644 > --- a/drivers/gpu/drm/i915/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > @@ -756,17 +756,47 @@ void intel_fbdev_fini(struct drm_device *dev) > dev_priv->fbdev = NULL; > } > > +static struct intel_fbdev *intel_fbdev_get(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = to_i915(dev); > + struct fb_info *info; > + > + if (dev_priv->fbdev == NULL) > + return NULL; > + > + info = dev_priv->fbdev->helper.fbdev; > + if (info == NULL) > + return NULL; > + > + if (info->screen_base == NULL) > + return NULL; > + This is rather verbose to my liking, I'd rather be dropping those '== NULL' and convert to !. But either way to me. > + return dev_priv->fbdev; > +} > + > +static struct intel_fbdev *intel_fbdev_get_if_active(struct drm_device *dev) > +{ > + struct intel_fbdev *ifbdev; > + > + ifbdev = intel_fbdev_get(dev); > + if (ifbdev == NULL) > + return NULL; > + > + if (ifbdev->helper.fbdev->state != FBINFO_STATE_RUNNING) > + return NULL; > + > + return ifbdev; > +} > + > void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous) > { > struct drm_i915_private *dev_priv = dev->dev_private; > - struct intel_fbdev *ifbdev = dev_priv->fbdev; > - struct fb_info *info; > + struct intel_fbdev *ifbdev; > > - if (!ifbdev) > + ifbdev = intel_fbdev_get(dev); > + if (ifbdev == NULL) > return; > > - info = ifbdev->helper.fbdev; > - > if (synchronous) { > /* Flush any pending work to turn the console on, and then > * wait to turn it off. It must be synchronous as we are > @@ -798,8 +828,10 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous > * been restored from swap. If the object is stolen however, it will be > * full of whatever garbage was left in there. > */ > - if (state == FBINFO_STATE_RUNNING && ifbdev->fb->obj->stolen) > + if (state == FBINFO_STATE_RUNNING && ifbdev->fb->obj->stolen) { > + struct fb_info *info = ifbdev->helper.fbdev; > memset_io(info->screen_base, 0, info->screen_size); > + } > > drm_fb_helper_set_suspend(&ifbdev->helper, state); > console_unlock(); > @@ -807,32 +839,24 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous > > void intel_fbdev_output_poll_changed(struct drm_device *dev) > { > - struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_fbdev *ifbdev = intel_fbdev_get_if_active(dev); > + > + if (ifbdev == NULL) > + return; > > - async_synchronize_full(); > - if (dev_priv->fbdev) > - drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper); > + drm_fb_helper_hotplug_event(&ifbdev->helper); > } > > void intel_fbdev_restore_mode(struct drm_device *dev) > { > - int ret; > - struct drm_i915_private *dev_priv = dev->dev_private; > - struct intel_fbdev *ifbdev = dev_priv->fbdev; > - struct drm_fb_helper *fb_helper; > + struct intel_fbdev *ifbdev = intel_fbdev_get_if_active(dev); > > - async_synchronize_full(); What's with the async_synchronize_full() begin removed completely? > - if (!ifbdev) > + if (ifbdev == NULL) Argh. > return; > > - fb_helper = &ifbdev->helper; > - > - ret = drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper); > - if (ret) { > - DRM_DEBUG("failed to restore crtc mode\n"); > - } else { > - mutex_lock(&fb_helper->dev->struct_mutex); > + if (drm_fb_helper_restore_fbdev_mode_unlocked(&ifbdev->helper) == 0) { > + mutex_lock(&dev->struct_mutex); > intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT); > - mutex_unlock(&fb_helper->dev->struct_mutex); > + mutex_unlock(&dev->struct_mutex); Above addressed, Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > } > } -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
