[PATCH 1/2] drm/gma500/psb: Unpin framebuffer on crtc disable

[Daniel Vetter]

On Sat, Jun 08, 2013 at 09:07:33PM +0200, Patrik Jakobsson wrote:
> On Sat, Jun 8, 2013 at 9:01 PM, Patrik Jakobsson
> <patrik.r.jakobsson at gmail.com> wrote:
> > The framebuffer needs to be unpinned in the crtc->disable callback
> > because of previous pinning in psb_intel_pipe_set_base(). This will fix
> > a memory leak where the framebuffer was released but not unpinned
> > properly. This patch only affects Poulsbo.
> >
> > 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/psb_intel_display.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c
> > index 6e8f42b..12d129e 100644
> > --- a/drivers/gpu/drm/gma500/psb_intel_display.c
> > +++ b/drivers/gpu/drm/gma500/psb_intel_display.c
> > @@ -1150,6 +1150,19 @@ static void psb_intel_crtc_destroy(struct drm_crtc *crtc)
> >         kfree(psb_intel_crtc);
> >  }
> >
> > +static void psb_intel_crtc_disable(struct drm_crtc *crtc)
> > +{
> > +       struct gtt_range *gt;
> > +       struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
> > +
> > +       crtc_funcs->dpms(crtc, DRM_MODE_DPMS_OFF);
> > +
> > +       if (crtc->fb) {
> > +               gt = to_psb_fb(crtc->fb)->gtt;
> > +               psb_gtt_unpin(gt);
> > +       }
> > +}
> > +
> >  const struct drm_crtc_helper_funcs psb_intel_helper_funcs = {
> >         .dpms = psb_intel_crtc_dpms,
> >         .mode_fixup = psb_intel_crtc_mode_fixup,
> > @@ -1157,6 +1170,7 @@ const struct drm_crtc_helper_funcs psb_intel_helper_funcs = {
> >         .mode_set_base = psb_intel_pipe_set_base,
> >         .prepare = psb_intel_crtc_prepare,
> >         .commit = psb_intel_crtc_commit,
> > +       .disable = psb_intel_crtc_disable,
> >  };
> >
> >  const struct drm_crtc_funcs psb_intel_crtc_funcs = {
> > --
> > 1.8.1.2
> >
> 
> Dave, this is a "less hackish" fix to the framebuffer leak bug I've been having.
> Daniel Vetter suggested this approach and it works as expected. ACK or NACK?

Yeah, this is how we've done it in i915 before switching away from crtc
helpers. For both patches:

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch