On Tue, 17 Dec 2013 22:17:22 +0100
Daniel Vetter <dan...@ffwll.ch> wrote:
> On Tue, Dec 17, 2013 at 10:05 PM, Jesse Barnes <jbar...@virtuousgeek.org>
> wrote:
> >> On Mon, Dec 16, 2013 at 04:34:27PM -0800, Jesse Barnes wrote:
> >> > @@ -333,7 +535,8 @@ MODULE_LICENSE("GPL and additional rights");
> >> > void intel_fbdev_output_poll_changed(struct drm_device *dev)
> >> > {
> >> > struct drm_i915_private *dev_priv = dev->dev_private;
> >> > - drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
> >> > + if (dev_priv->fbdev)
> >> > + drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
> >> > }
> >>
> >> Also intel_fbdev_restore_mode() needs the NULL fbdev safeguard.
> >
> > Fixed.
>
> I still don't get why we need this check - for CONFIG_FB=n we have a
> special dummy function and we are really careful in the setup code to
> only enable the interrupt handling code once fbdev is fully set up. Or
> do I miss some change here which makes this required? If so the right
> fix imo would be to shuffle the init sequence again (and update all
> the tons of comments about it, ofc).
In the init code I'm more careful now to avoid leaving a bogus
pointer around:
ret = drm_fb_helper_init(dev, &ifbdev->helper,
INTEL_INFO(dev)->num_pipes,
4);
if (ret) {
+ dev_priv->fbdev = NULL;
kfree(ifbdev);
return ret;
}
So in the unlikely event that the fb helper code fails I don't want to
fall over.
But that shouldn't happen in practice. I only have the checks in place
to catch when I failed to set the fbdev field in one path (which is now
fixed).
--
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
