From: Imre Deak <imre.deak@intel.com>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 03/13] drm/i915: Split the framebuffer_info creation into a separate routine
Date: Wed, 20 Mar 2013 14:23:48 +0200 [thread overview]
Message-ID: <1363782228.13528.17.camel@intelbox> (raw)
In-Reply-To: <1361309508-4901-4-git-send-email-jbarnes@virtuousgeek.org>
On Tue, 2013-02-19 at 13:31 -0800, Jesse Barnes wrote:
> From: Chris Wilson <chris@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 <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 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 = 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.
> + 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)
next prev parent reply other threads:[~2013-03-20 12:23 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-19 21:31 More fastboot bits Jesse Barnes
2013-02-19 21:31 ` [PATCH 01/13] drm/i915: Skip modifying PCH DREF if not changing clock sources Jesse Barnes
2013-03-20 12:17 ` Imre Deak
2013-03-26 22:57 ` Jesse Barnes
2013-02-19 21:31 ` [PATCH 02/13] drm/i915: Introduce i915_gem_object_create_stolen_for_preallocated Jesse Barnes
2013-03-26 19:46 ` Daniel Vetter
2013-03-26 20:58 ` Jesse Barnes
2013-02-19 21:31 ` [PATCH 03/13] drm/i915: Split the framebuffer_info creation into a separate routine Jesse Barnes
2013-03-20 12:23 ` Imre Deak [this message]
2013-03-26 23:07 ` Jesse Barnes
2013-03-27 11:49 ` Imre Deak
2013-03-27 13:48 ` Chris Wilson
2013-02-19 21:31 ` [PATCH 04/13] drm: add initial_config function to fb helper Jesse Barnes
2013-03-26 20:34 ` Daniel Vetter
2013-03-26 20:52 ` Chris Wilson
2013-03-26 20:57 ` Jesse Barnes
2013-02-19 21:31 ` [PATCH 05/13] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon Jesse Barnes
2013-03-20 12:31 ` Imre Deak
2013-03-26 23:20 ` Jesse Barnes
2013-02-19 21:31 ` [PATCH 06/13] drm/i915: Retrieve the current mode upon KMS takeover Jesse Barnes
2013-03-20 12:36 ` Imre Deak
2013-03-26 23:24 ` Jesse Barnes
2013-03-26 23:52 ` Daniel Vetter
2013-03-26 23:59 ` Jesse Barnes
2013-02-19 21:31 ` [PATCH 07/13] drm/i915: Only preserve the BIOS modes if they are the preferred ones Jesse Barnes
2013-02-19 21:31 ` [PATCH 08/13] drm/i915: Validate that the framebuffer accommodates the current mode Jesse Barnes
2013-02-19 21:31 ` [PATCH 09/13] drm/i915: fix build in intel_display.c Jesse Barnes
2013-03-20 12:41 ` Imre Deak
2013-03-26 23:26 ` Jesse Barnes
2013-02-19 21:31 ` [PATCH 10/13] drm/i915: check panel fit status at update_plane time Jesse Barnes
2013-03-20 12:46 ` Imre Deak
2013-02-19 21:31 ` [PATCH 11/13] drm/i915: add clock_get for ironlake+ Jesse Barnes
2013-03-20 12:48 ` Imre Deak
2013-03-26 23:29 ` Jesse Barnes
2013-02-19 21:31 ` [PATCH 12/13] drm/i915: treat no fb -> fb as simple flip instead of full mode set Jesse Barnes
2013-02-19 21:31 ` [PATCH 13/13] drm/i915: check for non-native modes when inheriting a BIOS fb Jesse Barnes
2013-03-20 12:51 ` Imre Deak
2013-03-26 23:29 ` Jesse Barnes
2013-03-20 12:15 ` More fastboot bits Imre Deak
2013-03-20 15:20 ` Jesse Barnes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1363782228.13528.17.camel@intelbox \
--to=imre.deak@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jbarnes@virtuousgeek.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.