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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox