| Message ID | 1403204773-7112-4-git-send-email-benjamin.widawsky@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 19, 2014 at 12:06:13PM -0700, Ben Widawsky wrote: > This is one part in a few fixes needed to make FBC work with limited > stolen memory and large resolution displays. It is not the full > solution, but one (easy) step. > > The patch is straight-forward, it attempts to check there will be room > for FBC before trying to "reclaim" But it special cases one particular allocation. Why don't you just reserve stolen upfront for FBC? Compute the maximum buffer size the hardware could support and try to claim it during stolen init. -Chris
On Thu, Jun 19, 2014 at 08:28:11PM +0100, Chris Wilson wrote: > On Thu, Jun 19, 2014 at 12:06:13PM -0700, Ben Widawsky wrote: > > This is one part in a few fixes needed to make FBC work with limited > > stolen memory and large resolution displays. It is not the full > > solution, but one (easy) step. > > > > The patch is straight-forward, it attempts to check there will be room > > for FBC before trying to "reclaim" > > But it special cases one particular allocation. Why don't you just > reserve stolen upfront for FBC? Compute the maximum buffer size the > hardware could support and try to claim it during stolen init. > -Chris > This was my initial thought actually. I didn't want to have to rework all of our initialization sequence, and verify I didn't break anything. In particular I wasn't sure what happens when we try to recover the fb from stolen (if it fails). In particular, there is a logical conflict between fastboot and fbc - and one needs to assign the preference over who gets stolen memory first. Also, when I wrote this patch, I was unaware that the reclaimed fb by itself was enough to not allow the full CFB to fit (though it will with the 3 patches before this) Anyway, consider this the demonstration of the problem, and [my only] half-attempt to fix it. Hopefully you guys can fix it properly.
On Thu, Jun 19, 2014 at 08:28:11PM +0100, Chris Wilson wrote: > On Thu, Jun 19, 2014 at 12:06:13PM -0700, Ben Widawsky wrote: > > This is one part in a few fixes needed to make FBC work with limited > > stolen memory and large resolution displays. It is not the full > > solution, but one (easy) step. > > > > The patch is straight-forward, it attempts to check there will be room > > for FBC before trying to "reclaim" > > But it special cases one particular allocation. Why don't you just > reserve stolen upfront for FBC? Compute the maximum buffer size the > hardware could support and try to claim it during stolen init. > -Chris > I agree this would be the best approach (and what I had planned to do). For one, I didn't find the interfaces I wanted in the drm_mm to do what I needed (though I didn't look very hard). I ended up getting stuck with having to decide whether to reclaim the scanout (and fastboot), or FBC. I believe this should be a decision left to the user, where user is the distro packaging. I'd like to just point out some math at this point too. Common stolen size is 32M 3840 x 2160 x 4 = 31.64M So we have a real problem if we want to reuse any of stolen memory, which the first 3 patches address to some extent. Anyway, I think I was pretty clear that the patch is incomplete, and primarily meant to motivate the relevant parties to figure out how they want to handle the stolen reclaim.
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 27975c3..ca83af8 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -57,6 +57,36 @@ static struct fb_ops intelfb_ops = { .fb_debug_leave = drm_fb_helper_debug_leave, }; +static bool intelfb_use_stolen(struct drm_device *dev, struct drm_mm *mm, + int size) +{ + struct drm_mm_node *entry; + unsigned long adj_start; + unsigned long adj_end; + + if (!drm_mm_initialized(mm)) + return false; + + if (!HAS_FBC(dev)) + return true; + + /* It is more desirable to use FBC (if enabled) than to allocate the + * framebuffer from stolen. We can cheat this by rounding up the size by + * 2 (and hope to get lucky with alignment). The other options are more + * invasive, and arguably not any more effective. + */ + size *= 2; + __drm_mm_for_each_hole(entry, mm, adj_start, adj_end, + DRM_MM_SEARCH_DEFAULT) { + if (adj_end - adj_start < size) + continue; + + if (adj_end >= adj_start + size) + return true; + } + + return false; +} static int intelfb_alloc(struct drm_fb_helper *helper, struct drm_fb_helper_surface_size *sizes) { @@ -64,8 +94,9 @@ static int intelfb_alloc(struct drm_fb_helper *helper, container_of(helper, struct intel_fbdev, helper); struct drm_framebuffer *fb; struct drm_device *dev = helper->dev; + struct drm_i915_private *dev_priv = dev->dev_private; struct drm_mode_fb_cmd2 mode_cmd = {}; - struct drm_i915_gem_object *obj; + struct drm_i915_gem_object *obj = NULL; int size, ret; /* we don't do packed 24bpp */ @@ -82,7 +113,9 @@ static int intelfb_alloc(struct drm_fb_helper *helper, size = mode_cmd.pitches[0] * mode_cmd.height; size = ALIGN(size, PAGE_SIZE); - obj = i915_gem_object_create_stolen(dev, size); + + if (intelfb_use_stolen(dev, &dev_priv->mm.stolen, size)) + obj = i915_gem_object_create_stolen(dev, size); if (obj == NULL) obj = i915_gem_alloc_object(dev, size); if (!obj) {
This is one part in a few fixes needed to make FBC work with limited stolen memory and large resolution displays. It is not the full solution, but one (easy) step. The patch is straight-forward, it attempts to check there will be room for FBC before trying to "reclaim" This modifies behavior originally introduced: commit 0ffb0ff283cca16f72caf29c44496d83b0c291fb Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Thu Nov 15 11:32:27 2012 +0000 drm/i915: Allocate fbcon from stolen memory Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Jesse Barnes <jbarnes@virtuousgeek.org> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/intel_fbdev.c | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-)