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
