| Message ID | 1360149028-13531-3-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2013-02-06 at 11:10 +0000, Chris Wilson wrote: > This will be shared with wrapping the BIOS framebuffer into the fbdev > later. In the meantime, we can tidy the code slightly and improve the > error path handling. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/intel_display.c | 7 -- > drivers/gpu/drm/i915/intel_drv.h | 7 ++ > drivers/gpu/drm/i915/intel_fb.c | 154 ++++++++++++++++++---------------- > 3 files changed, 91 insertions(+), 77 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index f1dbd01..8f9cdd7 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6413,13 +6413,6 @@ intel_framebuffer_create(struct drm_device *dev, > } > > static u32 > -intel_framebuffer_pitch_for_width(int width, int bpp) > -{ > - u32 pitch = DIV_ROUND_UP(width * bpp, 8); > - return ALIGN(pitch, 64); > -} > - > -static u32 > intel_framebuffer_size_for_mode(struct drm_display_mode *mode, int bpp) > { > u32 pitch = intel_framebuffer_pitch_for_width(mode->hdisplay, bpp); > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 13afb37..07d95a1 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -134,6 +134,13 @@ struct intel_framebuffer { > struct drm_i915_gem_object *obj; > }; > > +inline static u32 > +intel_framebuffer_pitch_for_width(int width, int bpp) > +{ > + u32 pitch = DIV_ROUND_UP(width * bpp, 8); > + return ALIGN(pitch, 64); > +} > + > struct intel_fbdev { > struct drm_fb_helper helper; > struct intel_framebuffer ifb; > diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c > index 6591029..de0ac4c 100644 > --- a/drivers/gpu/drm/i915/intel_fb.c > +++ b/drivers/gpu/drm/i915/intel_fb.c > @@ -57,29 +57,96 @@ static struct fb_ops intelfb_ops = { > .fb_debug_leave = drm_fb_helper_debug_leave, > }; > > +static struct fb_info *intelfb_create_info(struct intel_fbdev *ifbdev) > +{ > + struct drm_framebuffer *fb = &ifbdev->ifb.base; > + struct drm_device *dev = fb->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct fb_info *info; > + u32 gtt_offset, size; > + int ret; > + > + info = framebuffer_alloc(0, &dev->pdev->dev); > + if (!info) > + return NULL; > + > + info->par = ifbdev; > + ifbdev->helper.fb = fb; > + ifbdev->helper.fbdev = info; > + > + strcpy(info->fix.id, "inteldrmfb"); > + > + info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT; > + info->fbops = &intelfb_ops; > + > + ret = fb_alloc_cmap(&info->cmap, 256, 0); > + if (ret) > + goto err_info; > + > + /* setup aperture base/size for vesafb takeover */ > + info->apertures = alloc_apertures(1); > + if (!info->apertures) > + goto err_cmap; > + > + info->apertures->ranges[0].base = dev->mode_config.fb_base; > + info->apertures->ranges[0].size = dev_priv->gtt.mappable_end; > + > + gtt_offset = ifbdev->ifb.obj->gtt_offset; > + size = ifbdev->ifb.obj->base.size; > + > + info->fix.smem_start = dev->mode_config.fb_base + gtt_offset; > + info->fix.smem_len = size; > + > + info->screen_size = size; > + info->screen_base = ioremap_wc(dev_priv->gtt.mappable_base + gtt_offset, > + size); > + if (!info->screen_base) > + goto err_cmap; kfree(info->apertures) is missing. Same in intel_fbdev_destroy(). > + > + /* 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); fb_dealloc_cmap() could be called unconditionally. > +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 pitches[0] for non 8-bit aligned bpps. If it's a fix a comment about it in the commit message would be nice. --Imre > + mode_cmd.pixel_format = > + drm_mode_legacy_fb_format(sizes->surface_bpp, > + sizes->surface_depth); > > size = mode_cmd.pitches[0] * mode_cmd.height; > size = ALIGN(size, PAGE_SIZE); > @@ -101,72 +168,19 @@ static int intelfb_create(struct intel_fbdev *ifbdev, > goto out_unref; > } > > - info = framebuffer_alloc(0, device); > - if (!info) { > - ret = -ENOMEM; > - goto out_unpin; > - } > - > - info->par = ifbdev; > - > ret = intel_framebuffer_init(dev, &ifbdev->ifb, &mode_cmd, obj); > if (ret) > goto out_unpin; > > - fb = &ifbdev->ifb.base; > - > - ifbdev->helper.fb = fb; > - ifbdev->helper.fbdev = info; > - > - strcpy(info->fix.id, "inteldrmfb"); > - > - info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT; > - info->fbops = &intelfb_ops; > + DRM_DEBUG_KMS("allocated %dx%d fb: 0x%08x, bo %p\n", > + mode_cmd.width, mode_cmd.height, > + obj->gtt_offset, obj); > > - ret = fb_alloc_cmap(&info->cmap, 256, 0); > - if (ret) { > - ret = -ENOMEM; > - goto out_unpin; > - } > - /* setup aperture base/size for vesafb takeover */ > - info->apertures = alloc_apertures(1); > - if (!info->apertures) { > + info = intelfb_create_info(ifbdev); > + if (info == NULL) { > ret = -ENOMEM; > goto out_unpin; > } > - info->apertures->ranges[0].base = dev->mode_config.fb_base; > - info->apertures->ranges[0].size = dev_priv->gtt.mappable_end; > - > - info->fix.smem_start = dev->mode_config.fb_base + obj->gtt_offset; > - info->fix.smem_len = size; > - > - info->screen_base = > - ioremap_wc(dev_priv->gtt.mappable_base + obj->gtt_offset, > - size); > - if (!info->screen_base) { > - ret = -ENOSPC; > - goto out_unpin; > - } > - info->screen_size = size; > - > -// memset(info->screen_base, 0, size); > - > - drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth); > - drm_fb_helper_fill_var(info, &ifbdev->helper, sizes->fb_width, sizes->fb_height); > - > - /* 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_DEBUG_KMS("allocated %dx%d fb: 0x%08x, bo %p\n", > - fb->width, fb->height, > - obj->gtt_offset, obj); > - > > mutex_unlock(&dev->struct_mutex); > vga_switcheroo_client_fb_set(dev->pdev, info); > @@ -206,11 +220,11 @@ static struct drm_fb_helper_funcs intel_fb_helper_funcs = { > static void intel_fbdev_destroy(struct drm_device *dev, > struct intel_fbdev *ifbdev) > { > - struct fb_info *info; > struct intel_framebuffer *ifb = &ifbdev->ifb; > > if (ifbdev->helper.fbdev) { > - info = ifbdev->helper.fbdev; > + struct fb_info *info = ifbdev->helper.fbdev; > + > unregister_framebuffer(info); > iounmap(info->screen_base); > if (info->cmap.len)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f1dbd01..8f9cdd7 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6413,13 +6413,6 @@ intel_framebuffer_create(struct drm_device *dev, } static u32 -intel_framebuffer_pitch_for_width(int width, int bpp) -{ - u32 pitch = DIV_ROUND_UP(width * bpp, 8); - return ALIGN(pitch, 64); -} - -static u32 intel_framebuffer_size_for_mode(struct drm_display_mode *mode, int bpp) { u32 pitch = intel_framebuffer_pitch_for_width(mode->hdisplay, bpp); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 13afb37..07d95a1 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -134,6 +134,13 @@ struct intel_framebuffer { struct drm_i915_gem_object *obj; }; +inline static u32 +intel_framebuffer_pitch_for_width(int width, int bpp) +{ + u32 pitch = DIV_ROUND_UP(width * bpp, 8); + return ALIGN(pitch, 64); +} + struct intel_fbdev { struct drm_fb_helper helper; struct intel_framebuffer ifb; diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c index 6591029..de0ac4c 100644 --- a/drivers/gpu/drm/i915/intel_fb.c +++ b/drivers/gpu/drm/i915/intel_fb.c @@ -57,29 +57,96 @@ static struct fb_ops intelfb_ops = { .fb_debug_leave = drm_fb_helper_debug_leave, }; +static struct fb_info *intelfb_create_info(struct intel_fbdev *ifbdev) +{ + struct drm_framebuffer *fb = &ifbdev->ifb.base; + struct drm_device *dev = fb->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct fb_info *info; + u32 gtt_offset, size; + int ret; + + info = framebuffer_alloc(0, &dev->pdev->dev); + if (!info) + return NULL; + + info->par = ifbdev; + ifbdev->helper.fb = fb; + ifbdev->helper.fbdev = info; + + strcpy(info->fix.id, "inteldrmfb"); + + info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT; + info->fbops = &intelfb_ops; + + ret = fb_alloc_cmap(&info->cmap, 256, 0); + if (ret) + goto err_info; + + /* setup aperture base/size for vesafb takeover */ + info->apertures = alloc_apertures(1); + if (!info->apertures) + goto err_cmap; + + info->apertures->ranges[0].base = dev->mode_config.fb_base; + info->apertures->ranges[0].size = dev_priv->gtt.mappable_end; + + gtt_offset = ifbdev->ifb.obj->gtt_offset; + size = ifbdev->ifb.obj->base.size; + + info->fix.smem_start = dev->mode_config.fb_base + gtt_offset; + info->fix.smem_len = size; + + info->screen_size = size; + info->screen_base = ioremap_wc(dev_priv->gtt.mappable_base + gtt_offset, + size); + if (!info->screen_base) + 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); +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); + mode_cmd.pixel_format = + drm_mode_legacy_fb_format(sizes->surface_bpp, + sizes->surface_depth); size = mode_cmd.pitches[0] * mode_cmd.height; size = ALIGN(size, PAGE_SIZE); @@ -101,72 +168,19 @@ static int intelfb_create(struct intel_fbdev *ifbdev, goto out_unref; } - info = framebuffer_alloc(0, device); - if (!info) { - ret = -ENOMEM; - goto out_unpin; - } - - info->par = ifbdev; - ret = intel_framebuffer_init(dev, &ifbdev->ifb, &mode_cmd, obj); if (ret) goto out_unpin; - fb = &ifbdev->ifb.base; - - ifbdev->helper.fb = fb; - ifbdev->helper.fbdev = info; - - strcpy(info->fix.id, "inteldrmfb"); - - info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT; - info->fbops = &intelfb_ops; + DRM_DEBUG_KMS("allocated %dx%d fb: 0x%08x, bo %p\n", + mode_cmd.width, mode_cmd.height, + obj->gtt_offset, obj); - ret = fb_alloc_cmap(&info->cmap, 256, 0); - if (ret) { - ret = -ENOMEM; - goto out_unpin; - } - /* setup aperture base/size for vesafb takeover */ - info->apertures = alloc_apertures(1); - if (!info->apertures) { + info = intelfb_create_info(ifbdev); + if (info == NULL) { ret = -ENOMEM; goto out_unpin; } - info->apertures->ranges[0].base = dev->mode_config.fb_base; - info->apertures->ranges[0].size = dev_priv->gtt.mappable_end; - - info->fix.smem_start = dev->mode_config.fb_base + obj->gtt_offset; - info->fix.smem_len = size; - - info->screen_base = - ioremap_wc(dev_priv->gtt.mappable_base + obj->gtt_offset, - size); - if (!info->screen_base) { - ret = -ENOSPC; - goto out_unpin; - } - info->screen_size = size; - -// memset(info->screen_base, 0, size); - - drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth); - drm_fb_helper_fill_var(info, &ifbdev->helper, sizes->fb_width, sizes->fb_height); - - /* 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_DEBUG_KMS("allocated %dx%d fb: 0x%08x, bo %p\n", - fb->width, fb->height, - obj->gtt_offset, obj); - mutex_unlock(&dev->struct_mutex); vga_switcheroo_client_fb_set(dev->pdev, info); @@ -206,11 +220,11 @@ static struct drm_fb_helper_funcs intel_fb_helper_funcs = { static void intel_fbdev_destroy(struct drm_device *dev, struct intel_fbdev *ifbdev) { - struct fb_info *info; struct intel_framebuffer *ifb = &ifbdev->ifb; if (ifbdev->helper.fbdev) { - info = ifbdev->helper.fbdev; + struct fb_info *info = ifbdev->helper.fbdev; + unregister_framebuffer(info); iounmap(info->screen_base); if (info->cmap.len)
This will be shared with wrapping the BIOS framebuffer into the fbdev later. In the meantime, we can tidy the code slightly and improve the error path handling. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_display.c | 7 -- drivers/gpu/drm/i915/intel_drv.h | 7 ++ drivers/gpu/drm/i915/intel_fb.c | 154 ++++++++++++++++++---------------- 3 files changed, 91 insertions(+), 77 deletions(-)