Re: [Intel-gfx] [PATCH 03/13] drm/i915: Split the framebuffer_info creation into a separate routine
On Tue, 2013-03-26 at 16:07 -0700, Jesse Barnes wrote:
On Wed, 20 Mar 2013 14:23:48 +0200
Imre Deak imre.d...@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.
Yea, but you get different results due to the different way of rounding
for certain bpps. For example:
sizes-surface_bpp = 4
mode_cmd.width = 1000
You get pitches[0]=1024 with the old way and 512 with the new way.
--Imre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/13] drm/i915: Split the framebuffer_info creation into a separate routine
On Wed, Mar 27, 2013 at 01:49:06PM +0200, Imre Deak wrote:
On Tue, 2013-03-26 at 16:07 -0700, Jesse Barnes wrote:
On Wed, 20 Mar 2013 14:23:48 +0200
Imre Deak imre.d...@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.
Yea, but you get different results due to the different way of rounding
for certain bpps. For example:
sizes-surface_bpp = 4
mode_cmd.width = 1000
You get pitches[0]=1024 with the old way and 512 with the new way.
Not a bug in Jesse's patch though, but an earlier one of mine trying to
use the kernel macros and failing.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/13] drm/i915: Split the framebuffer_info creation into a separate routine
On Wed, 20 Mar 2013 14:23:48 +0200
Imre Deak imre.d...@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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/13] drm/i915: Split the framebuffer_info creation into a separate routine
On Tue, 2013-02-19 at 13:31 -0800, Jesse Barnes wrote:
From: Chris Wilson ch...@chris-wilson.co.uk
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 f20555e..dc58b01 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6422,13 +6422,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 005a91f..f93653d 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 1c510da..5afc31b 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)
kfree(info-apertures) is missing. The same goes for
intel_fbdev_destroy().
+ 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.
+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 =
[Intel-gfx] [PATCH 03/13] drm/i915: Split the framebuffer_info creation into a separate routine
From: Chris Wilson ch...@chris-wilson.co.uk
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 f20555e..dc58b01 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6422,13 +6422,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 005a91f..f93653d 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 1c510da..5afc31b 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) /
-
