We had an unmatching pin/unpin count because psb_intel_pipe_set_base was pinning the framebuffer before use. This caused psb_gtt_free_range to leak memory and trigger a warning (see bug reports).
Pinning / Unpinning is now done at dumb buffer alloc / destroy and at vm fault time if needed by non-dumb non-stolen buffers (no use case yet)
Now whenever we call psb_gtt_free_range() it is assumed that the buffer is fully unpinned. It is also assumed that a framebuffer used when setting a pipe base is already pinned.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=889511 Bugzilla: https://bugzilla.novell.com/show_bug.cgi?id=812113 Signed-off-by: Patrik Jakobsson patrik.r.jakobsson@gmail.com --- drivers/gpu/drm/gma500/gem.c | 44 ++++++++++++++++++++++++++---- drivers/gpu/drm/gma500/gtt.c | 5 ---- drivers/gpu/drm/gma500/psb_intel_display.c | 17 ++++-------- 3 files changed, 45 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c index eefd6cc..d2d11bd 100644 --- a/drivers/gpu/drm/gma500/gem.c +++ b/drivers/gpu/drm/gma500/gem.c @@ -159,9 +159,32 @@ static int psb_gem_create(struct drm_file *file, int psb_gem_dumb_create(struct drm_file *file, struct drm_device *dev, struct drm_mode_create_dumb *args) { + struct drm_gem_object *obj = NULL; + struct gtt_range *gt; + int ret; + args->pitch = ALIGN(args->width * ((args->bpp + 7) / 8), 64); args->size = args->pitch * args->height; - return psb_gem_create(file, dev, args->size, &args->handle); + ret = psb_gem_create(file, dev, args->size, &args->handle); + + /* Pin all dumb allocated buffers since they're all supposed to be used + for scanout anyways */ + if (ret == 0) { + obj = drm_gem_object_lookup(dev, file, args->handle); + if (obj == NULL) + return -ENOENT; + + gt = container_of(obj, struct gtt_range, gem); + if (psb_gtt_pin(gt) < 0) { + ret = -ENOMEM; + goto unref; + } + } + +unref: + if (obj) + drm_gem_object_unreference(obj); + return ret; }
/** @@ -177,6 +200,17 @@ int psb_gem_dumb_create(struct drm_file *file, struct drm_device *dev, int psb_gem_dumb_destroy(struct drm_file *file, struct drm_device *dev, uint32_t handle) { + struct drm_gem_object *obj; + struct gtt_range *gt; + + obj = drm_gem_object_lookup(dev, file, handle); + if (obj == NULL) + return -ENOENT; + + gt = container_of(obj, struct gtt_range, gem); + psb_gtt_unpin(gt); + drm_gem_object_unreference(obj); + /* No special work needed, drop the reference and see what falls out */ return drm_gem_handle_delete(file, handle); } @@ -218,16 +252,16 @@ int psb_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf) something from beneath our feet */ mutex_lock(&dev->struct_mutex);
- /* For now the mmap pins the object and it stays pinned. As things - stand that will do us no harm */ - if (r->mmapping == 0) { + /* Pin the object if it's not already in gart. Dumb buffers should + already be pinned at this point */ + if (r->in_gart == 0) { ret = psb_gtt_pin(r); if (ret < 0) { dev_err(dev->dev, "gma500: pin failed: %d\n", ret); goto fail; } - r->mmapping = 1; } + r->mmapping = 1;
/* Page relative to the VMA start - we must calculate this ourselves because vmf->pgoff is the fake GEM offset */ diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c index 1f82183..0d62cd7 100644 --- a/drivers/gpu/drm/gma500/gtt.c +++ b/drivers/gpu/drm/gma500/gtt.c @@ -378,11 +378,6 @@ struct gtt_range *psb_gtt_alloc_range(struct drm_device *dev, int len, */ void psb_gtt_free_range(struct drm_device *dev, struct gtt_range *gt) { - /* Undo the mmap pin if we are destroying the object */ - if (gt->mmapping) { - psb_gtt_unpin(gt); - gt->mmapping = 0; - } WARN_ON(gt->in_gart && !gt->stolen); release_resource(>->resource); kfree(gt); diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c index 6e8f42b..a768c98 100644 --- a/drivers/gpu/drm/gma500/psb_intel_display.c +++ b/drivers/gpu/drm/gma500/psb_intel_display.c @@ -237,6 +237,7 @@ void psb_intel_wait_for_vblank(struct drm_device *dev) mdelay(20); }
+/* Framebuffer must be pinned before setting it as pipe base */ static int psb_intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, struct drm_framebuffer *old_fb) { @@ -256,14 +257,14 @@ static int psb_intel_pipe_set_base(struct drm_crtc *crtc, /* no fb bound */ if (!crtc->fb) { dev_dbg(dev->dev, "No FB bound\n"); - goto psb_intel_pipe_cleaner; + goto psb_intel_pipe_set_base_exit; }
- /* We are displaying this buffer, make sure it is actually loaded - into the GTT */ - ret = psb_gtt_pin(psbfb->gtt); - if (ret < 0) + if (psbfb->gtt->in_gart < 1) { + WARN_ON(1); goto psb_intel_pipe_set_base_exit; + } + start = psbfb->gtt->offset;
offset = y * crtc->fb->pitches[0] + x * (crtc->fb->bits_per_pixel / 8); @@ -290,7 +291,6 @@ static int psb_intel_pipe_set_base(struct drm_crtc *crtc, default: dev_err(dev->dev, "Unknown color depth\n"); ret = -EINVAL; - psb_gtt_unpin(psbfb->gtt); goto psb_intel_pipe_set_base_exit; } REG_WRITE(map->cntr, dspcntr); @@ -298,11 +298,6 @@ static int psb_intel_pipe_set_base(struct drm_crtc *crtc, REG_WRITE(map->base, start + offset); REG_READ(map->base);
-psb_intel_pipe_cleaner: - /* If there was a previous display we can now unpin it */ - if (old_fb) - psb_gtt_unpin(to_psb_fb(old_fb)->gtt); - psb_intel_pipe_set_base_exit: gma_power_end(dev); return ret;