* [PATCH] drm/i915: Don't cleanup plane state in intel_plane_destroy()
@ 2015-01-16 15:25 Matt Roper
2015-01-16 16:16 ` Jani Nikula
2015-01-17 4:56 ` shuang.he
0 siblings, 2 replies; 4+ messages in thread
From: Matt Roper @ 2015-01-16 15:25 UTC (permalink / raw)
To: intel-gfx
When we transitioned to the atomic plane helpers in commit:
commit ea2c67bb4affa84080c616920f3899f123786e56
Author: Matt Roper <matthew.d.roper@intel.com>
Date: Tue Dec 23 10:41:52 2014 -0800
drm/i915: Move to atomic plane helpers (v9)
one of the changes was to call intel_plane_destroy_state() while tearing
down a plane to prevent leaks when unloading the driver. That made
sense when the patches were first written, but before they were merged,
commit 3009c0377f25c29852b218a6933a969d02cbdc5d
Author: Thierry Reding <treding@nvidia.com>
Date: Tue Nov 25 12:09:49 2014 +0100
drm: Free atomic state during cleanup
had already landed, which made this the responsibility of the DRM core.
The result was that we were kfree()'ing the state twice, and also
possibly double-unref'ing a framebuffer, leading to memory corruption
when the driver was unloaded.
The fix is to simply not try to cleanup the state in the i915 teardown
code now that the core handles this for us.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88433
Testcase: igt/drv_module_reload
Root-cause-analysis-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 91d8ada..cc3b9d8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11937,7 +11937,6 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc)
void intel_plane_destroy(struct drm_plane *plane)
{
struct intel_plane *intel_plane = to_intel_plane(plane);
- intel_plane_destroy_state(plane, plane->state);
drm_plane_cleanup(plane);
kfree(intel_plane);
}
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] drm/i915: Don't cleanup plane state in intel_plane_destroy()
2015-01-16 15:25 [PATCH] drm/i915: Don't cleanup plane state in intel_plane_destroy() Matt Roper
@ 2015-01-16 16:16 ` Jani Nikula
2015-01-19 9:05 ` Daniel Vetter
2015-01-17 4:56 ` shuang.he
1 sibling, 1 reply; 4+ messages in thread
From: Jani Nikula @ 2015-01-16 16:16 UTC (permalink / raw)
To: Matt Roper, intel-gfx
On Fri, 16 Jan 2015, Matt Roper <matthew.d.roper@intel.com> wrote:
> When we transitioned to the atomic plane helpers in commit:
>
> commit ea2c67bb4affa84080c616920f3899f123786e56
> Author: Matt Roper <matthew.d.roper@intel.com>
> Date: Tue Dec 23 10:41:52 2014 -0800
>
> drm/i915: Move to atomic plane helpers (v9)
>
> one of the changes was to call intel_plane_destroy_state() while tearing
> down a plane to prevent leaks when unloading the driver. That made
> sense when the patches were first written, but before they were merged,
>
> commit 3009c0377f25c29852b218a6933a969d02cbdc5d
> Author: Thierry Reding <treding@nvidia.com>
> Date: Tue Nov 25 12:09:49 2014 +0100
>
> drm: Free atomic state during cleanup
>
> had already landed, which made this the responsibility of the DRM core.
> The result was that we were kfree()'ing the state twice, and also
> possibly double-unref'ing a framebuffer, leading to memory corruption
> when the driver was unloaded.
>
> The fix is to simply not try to cleanup the state in the i915 teardown
> code now that the core handles this for us.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88433
> Testcase: igt/drv_module_reload
> Root-cause-analysis-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Thanked-by-and-good-weekend-wished-by-and-
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 91d8ada..cc3b9d8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11937,7 +11937,6 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc)
> void intel_plane_destroy(struct drm_plane *plane)
> {
> struct intel_plane *intel_plane = to_intel_plane(plane);
> - intel_plane_destroy_state(plane, plane->state);
> drm_plane_cleanup(plane);
> kfree(intel_plane);
> }
> --
> 1.8.5.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] drm/i915: Don't cleanup plane state in intel_plane_destroy()
2015-01-16 16:16 ` Jani Nikula
@ 2015-01-19 9:05 ` Daniel Vetter
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2015-01-19 9:05 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Fri, Jan 16, 2015 at 06:16:50PM +0200, Jani Nikula wrote:
> On Fri, 16 Jan 2015, Matt Roper <matthew.d.roper@intel.com> wrote:
> > When we transitioned to the atomic plane helpers in commit:
> >
> > commit ea2c67bb4affa84080c616920f3899f123786e56
> > Author: Matt Roper <matthew.d.roper@intel.com>
> > Date: Tue Dec 23 10:41:52 2014 -0800
> >
> > drm/i915: Move to atomic plane helpers (v9)
> >
> > one of the changes was to call intel_plane_destroy_state() while tearing
> > down a plane to prevent leaks when unloading the driver. That made
> > sense when the patches were first written, but before they were merged,
> >
> > commit 3009c0377f25c29852b218a6933a969d02cbdc5d
> > Author: Thierry Reding <treding@nvidia.com>
> > Date: Tue Nov 25 12:09:49 2014 +0100
> >
> > drm: Free atomic state during cleanup
> >
> > had already landed, which made this the responsibility of the DRM core.
> > The result was that we were kfree()'ing the state twice, and also
> > possibly double-unref'ing a framebuffer, leading to memory corruption
> > when the driver was unloaded.
> >
> > The fix is to simply not try to cleanup the state in the i915 teardown
> > code now that the core handles this for us.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88433
> > Testcase: igt/drv_module_reload
> > Root-cause-analysis-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
>
> Thanked-by-and-good-weekend-wished-by-and-
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Queued for -next, thanks for the patch.
-Daniel
>
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 91d8ada..cc3b9d8 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11937,7 +11937,6 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc)
> > void intel_plane_destroy(struct drm_plane *plane)
> > {
> > struct intel_plane *intel_plane = to_intel_plane(plane);
> > - intel_plane_destroy_state(plane, plane->state);
> > drm_plane_cleanup(plane);
> > kfree(intel_plane);
> > }
> > --
> > 1.8.5.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Don't cleanup plane state in intel_plane_destroy()
2015-01-16 15:25 [PATCH] drm/i915: Don't cleanup plane state in intel_plane_destroy() Matt Roper
2015-01-16 16:16 ` Jani Nikula
@ 2015-01-17 4:56 ` shuang.he
1 sibling, 0 replies; 4+ messages in thread
From: shuang.he @ 2015-01-17 4:56 UTC (permalink / raw)
To: shuang.he, ethan.gao, intel-gfx, matthew.d.roper
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5594
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV -1 353/353 352/353
ILK 200/200 200/200
SNB 400/422 400/422
IVB 487/487 487/487
BYT 296/296 296/296
HSW -1 487/508 486/508
BDW 401/402 401/402
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*PNV igt_gen3_render_linear_blits PASS(3, M25M23) CRASH(1, M23)
HSW igt_kms_flip_event_leak NSPT(5, M19)PASS(1, M19) NSPT(1, M19)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-01-19 9:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-16 15:25 [PATCH] drm/i915: Don't cleanup plane state in intel_plane_destroy() Matt Roper
2015-01-16 16:16 ` Jani Nikula
2015-01-19 9:05 ` Daniel Vetter
2015-01-17 4:56 ` shuang.he
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox