From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 03/13] drm/i915: Split the framebuffer_info creation into a separate routine Date: Tue, 26 Mar 2013 16:07:27 -0700 Message-ID: <20130326160727.222a31ed@jbarnes-desktop> References: <1361309508-4901-1-git-send-email-jbarnes@virtuousgeek.org> <1361309508-4901-4-git-send-email-jbarnes@virtuousgeek.org> <1363782228.13528.17.camel@intelbox> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from oproxy1-pub.bluehost.com (oproxy1-pub.bluehost.com [66.147.249.253]) by gabe.freedesktop.org (Postfix) with SMTP id 7269EE5C6C for ; Tue, 26 Mar 2013 16:07:10 -0700 (PDT) In-Reply-To: <1363782228.13528.17.camel@intelbox> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: imre.deak@intel.com Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, 20 Mar 2013 14:23:48 +0200 Imre Deak 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