| Message ID | 1370718119-12903-1-git-send-email-patrik.r.jakobsson@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Sat, Jun 8, 2013 at 9:01 PM, Patrik Jakobsson <patrik.r.jakobsson@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@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? If this is ok I'll send you a pull request early next week so we can get it into 3.10. Thanks Patrik
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@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@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@ffwll.ch>
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 = {
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@gmail.com> --- drivers/gpu/drm/gma500/psb_intel_display.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)