diff mbox

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

Message ID 1360149028-13531-3-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson Feb. 6, 2013, 11:10 a.m. UTC 
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(-)

Comments

Imre Deak March 7, 2013, 2:49 p.m. UTC | #1 
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 mbox

Patch

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)