[PATCH 1/3] drm/gma500: Unpin framebuffer when destroying it

[Patrik Jakobsson]

On Sun, May 26, 2013 at 9:07 PM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Sun, May 26, 2013 at 08:03:53PM +0200, Patrik Jakobsson wrote:
>> A framebuffer is pinned when it's set as pipe base, so we also need to
>> unpin it when it's destroyed. Some framebuffers are never used as
>> scanout (no mode set on them) so if the pin count is less than one, no
>> unpinning is needed.
>>
>> 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 at gmail.com>
>> ---
>>  drivers/gpu/drm/gma500/framebuffer.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
>> index 8b1b6d9..1a11b69 100644
>> --- a/drivers/gpu/drm/gma500/framebuffer.c
>> +++ b/drivers/gpu/drm/gma500/framebuffer.c
>> @@ -668,6 +668,11 @@ static void psb_user_framebuffer_destroy(struct drm_framebuffer *fb)
>>
>>       /* Let DRM do its clean up */
>>       drm_framebuffer_cleanup(fb);
>> +
>> +     /* Unpin any psb_intel_pipe_set_base() pinning */
>> +     if (r->in_gart > 0)
>> +             psb_gtt_unpin(r);
>
> The drm core /should/ have removed the user framebuffer from all active
> users before it calls down into your ->destroy callback. Why can't gma500
> just pin/unpin on demand when the buffer is actually in use?

Thanks for the feedback

DRM core does correctly keep track of the users but the last ref is dropped in
our ->destroy callback and at that point it's still pinned from pipe_set_base().
So if it's considered too late to unpin the framebuffer in destroy, we never had
it right in the first place. Not a big surprise I guess :)

> If a pin survives the official use as seen by the drm core (e.g. to keep a
> buffer around until the pageflip completes) you can simply keep an
> additional reference around.

As I wrote above, that additional reference is not dropped until ->destroy
anyways and we don't have page flipping support and to be honest I haven't even
looked at implementing it yet. So the logical point to release the pin would be
in pipe_set_base() (or am I wrong?) by doing an unpin on the old_fb. Problem is
that when killing X and switching back to fbdev we get no old_fb.

I might be missing something, so any suggestions are welcome.

Thanks
Patrik