On Tue, 17 Dec 2013 19:29:00 +0000 Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > On Mon, Dec 16, 2013 at 04:34:27PM -0800, Jesse Barnes wrote: > > int intel_fbdev_init(struct drm_device *dev) > > @@ -268,17 +461,25 @@ int intel_fbdev_init(struct drm_device *dev) > > struct drm_i915_private *dev_priv = dev->dev_private; > > int ret; > > > > - ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL); > > - if (!ifbdev) > > - return -ENOMEM; > > + /* This may fail, if so, dev_priv->fbdev won't be set below */ > > + intel_fbdev_init_bios(dev); > > > > - dev_priv->fbdev = ifbdev; > > - ifbdev->helper.funcs = &intel_fb_helper_funcs; > > + if ((ifbdev = dev_priv->fbdev) == NULL) { > > + ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL); > > + if (ifbdev == NULL) > > + return -ENOMEM; > > + > > + ifbdev->helper.funcs = &intel_fb_helper_funcs; > > + ifbdev->preferred_bpp = 32; > > + > > + dev_priv->fbdev = ifbdev; > > + } > > Since Daniel also mentioned the same bikeshed, I think this would look > neater with the allocation here and ifbdev being passed to > intel_fbdev_init_bios to fill in: > > ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL); > if (ifbdev == NULL) > return -ENODEV; > > if (!intel_fbdev_init_bios(dev, ifbdev)) { > ifbdev->helper.funcs = &intel_fb_helper_funcs; > ifbdev->preferred_bpp = 32; > } > > dev_priv->fbdev = ifbdev; > return 0; Yeah, looks better that way. Fixed. -- Jesse Barnes, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx