On 2014년 09월 17일 15:35, Andrzej Hajda wrote: > Hi, > > On 09/16/2014 08:35 AM, Daniel Vetter wrote: >> On Mon, Sep 15, 2014 at 12:52:17PM -0600, Daniel Drake wrote: >>> Pageflipping currently causes some inconsistencies that lead to >>> crashes. Just run an app that causes a CRTC pageflip in a raw X session >>> and check that it exits cleanly and can be restarted - you'll see >>> crashes like: >>> Unable to handle kernel NULL pointer dereference at virtual address 00000334 >>> PC is at exynos_drm_crtc_plane_commit+0x20/0x40 >>> LR is at exynos_drm_crtc_plane_commit+0x20/0x40 >>> [<c03749b4>] (exynos_drm_crtc_plane_commit) from [<c03741bc>] (exynos_drm_crtc_commit+0x44/0x70) >>> [<c03741bc>] (exynos_drm_crtc_commit) from [<c03743a0>] (exynos_drm_crtc_mode_set_commit.isra.2+0xb4/0xc4) >>> [<c03743a0>] (exynos_drm_crtc_mode_set_commit.isra.2) from [<c03744f4>] (exynos_drm_crtc_page_flip+0x140/0x1a8) >>> [<c03744f4>] (exynos_drm_crtc_page_flip) from [<c036b20c>] (drm_mode_page_flip_ioctl+0x224/0x2dc) >>> [<c036b20c>] (drm_mode_page_flip_ioctl) from [<c035c324>] (drm_ioctl+0x338/0x4fc) >>> >>> These crashes happen because drm_plane_force_disable has previously set >>> plane->crtc to NULL. >>> >>> When drm_mode_page_flip_ioctl() is used to flip another framebuffer >>> onto the primary plane, crtc->primary->fb is correctly updated (this is >>> a virtual plane created by plane_helper), but plane->fb is not (this >>> plane is the real one, created by exynos_drm_crtc_create). >>> >>> We then come to handle rmfb of the backbuffer, which the "real" primary >>> plane is incorrectly pointing at. So drm_framebuffer_remove() decides that >>> the buffer is actually active on a plane and force-disables the plane. >>> >>> Ensuring that plane->fb is kept up-to-date solves that issue, but >>> exposes a reference counting problem. Now we see crashes when rmfb is >>> called on the front-buffer, because the rmfb code expects to drop 3 >>> references here, and there are only 2. >>> >>> That can be fixed by adopting the reference management found in omapdrm: >>> Framebuffer references are not taken directly in crtc mode_set context, >>> but rather in the context of updating the plane, which also covers >>> flips. Like omapdrm we also unreference the old framebuffer here. >>> >>> Signed-off-by: Daniel Drake <drake@xxxxxxxxxxxx> >> This sounds very much like exynos should switch to universal planes so >> that the fake primary plane created by the helpers doesn't get in the way. >> And for chips which already use planes for everything internally this >> shouldn't be a lot more than a few lines. >> -Daniel > > The patch proposed here of course supersedes my patch fixing fb refcounting. > But the best solution is to get rid of virtual plane as Daniel Vetter > stated. > Daniel (Drake of course :) ) do you want to prepare patch switching to > universal planes? > Maybe other volunteers? If not I can try to do it, as it seems quite > straightforward. I think you can do it and you would be a right person to do it. Thanks, Inki Dae > > Regards > Andrzej > >>> --- >>> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 12 ++---------- >>> drivers/gpu/drm/exynos/exynos_drm_plane.c | 8 ++++++++ >>> 2 files changed, 10 insertions(+), 10 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c >>> index b68e58f..7aa9dee 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c >>> @@ -140,16 +140,8 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, >>> if (manager->ops->mode_set) >>> manager->ops->mode_set(manager, &crtc->mode); >>> >>> - ret = exynos_plane_mode_set(plane, crtc, crtc->primary->fb, 0, 0, crtc_w, crtc_h, >>> - x, y, crtc_w, crtc_h); >>> - if (ret) >>> - return ret; >>> - >>> - plane->crtc = crtc; >>> - plane->fb = crtc->primary->fb; >>> - drm_framebuffer_reference(plane->fb); >>> - >>> - return 0; >>> + return exynos_plane_mode_set(plane, crtc, crtc->primary->fb, 0, 0, >>> + crtc_w, crtc_h, x, y, crtc_w, crtc_h); >>> } >>> >>> static int exynos_drm_crtc_mode_set_commit(struct drm_crtc *crtc, int x, int y, >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c >>> index 8371cbd..df27e35 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c >>> @@ -139,6 +139,14 @@ int exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc, >>> overlay->crtc_x, overlay->crtc_y, >>> overlay->crtc_width, overlay->crtc_height); >>> >>> + if (plane->fb) >>> + drm_framebuffer_unreference(plane->fb); >>> + >>> + drm_framebuffer_reference(fb); >>> + >>> + plane->fb = fb; >>> + plane->crtc = crtc; >>> + >>> exynos_drm_crtc_plane_mode_set(crtc, overlay); >>> >>> return 0; >>> -- >>> 1.9.1 >>> >>> _______________________________________________ >>> dri-devel mailing list >>> dri-devel@xxxxxxxxxxxxxxxxxxxxx >>> http://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
