Re: [Intel-gfx] [PATCH 3/8] drm/i915: Split the framebuffer_info creation into a separate routine

2013-03-07 Thread Imre Deak
On Wed, 2013-02-06 at 11:10 +, 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 ch...@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  = 

[Intel-gfx] [PATCH 3/8] drm/i915: Split the framebuffer_info creation into a separate routine

2013-02-06 Thread Chris Wilson
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 ch...@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;
+
+   /* 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) /
-