On 09/20/2012 04:49 PM, Chris Wilson wrote:
On Thu, 20 Sep 2012 15:10:39 +0900, Joonyoung Shim <jy0922.shim@xxxxxxxxxxx> wrote:
On 09/20/2012 02:38 PM, Rob Clark wrote:
On Wed, Sep 19, 2012 at 9:52 PM, Joonyoung Shim <jy0922.shim@xxxxxxxxxxx> wrote:
On 09/17/2012 06:38 PM, Chris Wilson wrote:
As during the plane cleanup, we wish to disable the hardware and
so may modify state on the associated CRTC, that CRTC must continue to
exist until we are finished.
A similar issue can occur in the drm_framebuffer_cleanup(). If crtc and
plane use same framebuffer and the framebuffer is destroyed, crtc is
turned off prior to turning off plane.
I imagine my patch to add refcnt'ing to fb would help in this case..
BR,
-R
Even if the patch to add refcnt'ing to fb is applied, same issue will
occur in the drm_framebuffer_remove(). It can delay to destroy the fb,
but cannot change crtc and plane disable order.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54101
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
---
drivers/gpu/drm/drm_crtc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 6fbfc24..af81f77 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1034,15 +1034,15 @@ void drm_mode_config_cleanup(struct drm_device
*dev)
fb->funcs->destroy(fb);
}
- list_for_each_entry_safe(crtc, ct, &dev->mode_config.crtc_list,
head) {
- crtc->funcs->destroy(crtc);
- }
-
list_for_each_entry_safe(plane, plt, &dev->mode_config.plane_list,
head) {
plane->funcs->destroy(plane);
}
+ list_for_each_entry_safe(crtc, ct, &dev->mode_config.crtc_list,
head) {
+ crtc->funcs->destroy(crtc);
+ }
+
idr_remove_all(&dev->mode_config.crtc_idr);
idr_destroy(&dev->mode_config.crtc_idr);
}
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH] drm: Destroy the planes prior to destroying the associated CRTC
To: Rob Clark <rob.clark@xxxxxxxxxx>, Joonyoung Shim <jy0922.shim@xxxxxxxxxxx>
Cc: airlied@xxxxxxxxxx, stable@xxxxxxxxxxxxxxx, dri-devel@xxxxxxxxxxxxxxxxxxxxx
In-Reply-To: <CAF6AEGvOZa_ZhRJN212Rsn-gMMUyWoLT6UFY9iindi8AWx9GvA@xxxxxxxxxxxxxx>
References: <1347874683-21191-1-git-send-email-chris@xxxxxxxxxxxxxxxxxx> <505A850A.4010205@xxxxxxxxxxx> <CAF6AEGvOZa_ZhRJN212Rsn-gMMUyWoLT6UFY9iindi8AWx9GvA@xxxxxxxxxxxxxx>
On Thu, 20 Sep 2012 00:38:04 -0500, Rob Clark <rob.clark@xxxxxxxxxx> wrote:
On Wed, Sep 19, 2012 at 9:52 PM, Joonyoung Shim <jy0922.shim@xxxxxxxxxxx> wrote:
On 09/17/2012 06:38 PM, Chris Wilson wrote:
As during the plane cleanup, we wish to disable the hardware and
so may modify state on the associated CRTC, that CRTC must continue to
exist until we are finished.
A similar issue can occur in the drm_framebuffer_cleanup(). If crtc and
plane use same framebuffer and the framebuffer is destroyed, crtc is
turned off prior to turning off plane.
I imagine my patch to add refcnt'ing to fb would help in this case..
BR,
-R
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54101
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
---
drivers/gpu/drm/drm_crtc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 6fbfc24..af81f77 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1034,15 +1034,15 @@ void drm_mode_config_cleanup(struct drm_device
*dev)
fb->funcs->destroy(fb);
}
- list_for_each_entry_safe(crtc, ct, &dev->mode_config.crtc_list,
head) {
- crtc->funcs->destroy(crtc);
- }
-
list_for_each_entry_safe(plane, plt, &dev->mode_config.plane_list,
head) {
plane->funcs->destroy(plane);
}
+ list_for_each_entry_safe(crtc, ct, &dev->mode_config.crtc_list,
head) {
+ crtc->funcs->destroy(crtc);
+ }
+
idr_remove_all(&dev->mode_config.crtc_idr);
idr_destroy(&dev->mode_config.crtc_idr);
}
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH] drm: Destroy the planes prior to destroying the associated CRTC
To: Joonyoung Shim <jy0922.shim@xxxxxxxxxxx>
Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx, airlied@xxxxxxxxxx, stable@xxxxxxxxxxxxxxx
In-Reply-To: <505A850A.4010205@xxxxxxxxxxx>
References: <1347874683-21191-1-git-send-email-chris@xxxxxxxxxxxxxxxxxx> <505A850A.4010205@xxxxxxxxxxx>
On Thu, 20 Sep 2012 11:52:58 +0900, Joonyoung Shim <jy0922.shim@xxxxxxxxxxx> wrote:
On 09/17/2012 06:38 PM, Chris Wilson wrote:
As during the plane cleanup, we wish to disable the hardware and
so may modify state on the associated CRTC, that CRTC must continue to
exist until we are finished.
A similar issue can occur in the drm_framebuffer_cleanup(). If crtc and
plane use same framebuffer and the framebuffer is destroyed, crtc is
turned off prior to turning off plane.
This is not an issue in our code as disabling the CRTCs should
automatically disable any associated planes, and so the second pass over
the planes should be a no-op.
Right, but it is decided by each hw specific driver implementation. If
drm core can prevent any problem, it's better.
The issue during destroy drm_mode_config_cleanup() is that we actually
free the CRTC objects and then try to decouple the planes which causes
us to reference the just-freed objects in order to see if the hw needs
updating.
However, reordering the sequence to be CRTCs last would help for
consistency.
Agree.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
