Re: [PATCH 03/13] drm/i915: Split the framebuffer_info creation into a separate routine

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Wed, 20 Mar 2013 14:23:48 +0200
Imre Deak <imre.deak@intel.com> wrote:

> > +	if (!info->screen_base)
> 
> kfree(info->apertures) is missing. The same goes for
> intel_fbdev_destroy().

Fixed in both places.

> 
> > +		goto err_cmap;
> > +
> > +	/* If the object is shmemfs backed, it will have given us zeroed pages.
> > +	 * If the object is stolen however, it will be full of whatever
> > +	 * garbage was left in there.
> > +	 */
> > +	if (ifbdev->ifb.obj->stolen)
> > +		memset_io(info->screen_base, 0, info->screen_size);
> > +
> > +	/* Use default scratch pixmap (info->pixmap.flags = FB_PIXMAP_SYSTEM) */
> > +
> > +	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
> > +	drm_fb_helper_fill_var(info, &ifbdev->helper, fb->width, fb->height);
> > +
> > +	return info;
> > +
> > +err_cmap:
> > +	if (info->cmap.len)
> > +		fb_dealloc_cmap(&info->cmap);
> 
> Should be fine to call w/o checking cmap.len.

Fixed in both places.

> 
> > +err_info:
> > +	framebuffer_release(info);
> > +	return NULL;
> > +}
> > +
> >  static int intelfb_create(struct intel_fbdev *ifbdev,
> >  			  struct drm_fb_helper_surface_size *sizes)
> >  {
> >  	struct drm_device *dev = ifbdev->helper.dev;
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > -	struct fb_info *info;
> > -	struct drm_framebuffer *fb;
> > -	struct drm_mode_fb_cmd2 mode_cmd = {};
> > +	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> >  	struct drm_i915_gem_object *obj;
> > -	struct device *device = &dev->pdev->dev;
> > +	struct fb_info *info;
> >  	int size, ret;
> >  
> >  	/* we don't do packed 24bpp */
> >  	if (sizes->surface_bpp == 24)
> >  		sizes->surface_bpp = 32;
> >  
> > -	mode_cmd.width = sizes->surface_width;
> > +	mode_cmd.width  = sizes->surface_width;
> >  	mode_cmd.height = sizes->surface_height;
> >  
> > -	mode_cmd.pitches[0] = ALIGN(mode_cmd.width * ((sizes->surface_bpp + 7) /
> > -						      8), 64);
> > -	mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> > -							  sizes->surface_depth);
> > +	mode_cmd.pitches[0] =
> > +		intel_framebuffer_pitch_for_width(mode_cmd.width,
> > +						  sizes->surface_bpp);
> 
> This changes the way pitches[0] is calculated for surface_bpp % 8 != 0,
> but there's no mention of it in the commit message.

It just removes the open coding; we still do the rounding and alignment
to 64 bytes.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center