From patchwork Tue Dec 4 12:14:26 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Inki Dae X-Patchwork-Id: 1837321 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork1.kernel.org (Postfix) with ESMTP id 513C23FC64 for ; Tue, 4 Dec 2012 12:14:43 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2FB52E5FCB for ; Tue, 4 Dec 2012 04:14:43 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mailout1.samsung.com (mailout1.samsung.com [203.254.224.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 105D3E5D08 for ; Tue, 4 Dec 2012 04:14:29 -0800 (PST) Received: from epcpsbgm2.samsung.com (epcpsbgm2 [203.254.230.27]) by mailout1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0MEI00K429ZIHVN0@mailout1.samsung.com> for dri-devel@lists.freedesktop.org; Tue, 04 Dec 2012 21:14:27 +0900 (KST) Received: from epcpsbgm2.samsung.com ( [203.254.230.43]) by epcpsbgm2.samsung.com (EPCPMTA) with SMTP id 75.D9.12699.329EDB05; Tue, 04 Dec 2012 21:14:27 +0900 (KST) X-AuditID: cbfee61b-b7f616d00000319b-d1-50bde923c130 Received: from epmmp2 ( [203.254.227.17]) by epcpsbgm2.samsung.com (EPCPMTA) with SMTP id 25.D9.12699.329EDB05; Tue, 04 Dec 2012 21:14:27 +0900 (KST) Received: from daeinki-desktop.10.32.193.11 ([10.90.51.53]) by mmp2.samsung.com (Oracle Communications Messaging Server 7u4-24.01 (7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTPA id <0MEI009NQA03Q560@mmp2.samsung.com> for dri-devel@lists.freedesktop.org; Tue, 04 Dec 2012 21:14:27 +0900 (KST) From: Inki Dae To: airlied@linux.ie, dri-devel@lists.freedesktop.org Subject: [PATCH v2] drm/exynos: release fb pended by page flip Date: Tue, 04 Dec 2012 21:14:26 +0900 Message-id: <1354623266-17551-1-git-send-email-inki.dae@samsung.com> X-Mailer: git-send-email 1.7.4.1 In-reply-to: <1352883576-25552-2-git-send-email-inki.dae@samsung.com> References: <1352883576-25552-2-git-send-email-inki.dae@samsung.com> DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrNLMWRmVeSWpSXmKPExsVy+t8zbV3ll3sDDDqmKFpc+fqezYHR4373 caYAxigum5TUnMyy1CJ9uwSujIZpJxgLWuwr/jwsaWA8ZNzFyMkhIWAisWbiQzYIW0ziwr31 QDYXh5DAMkaJue92s8AUvVy2nhUiMZ1R4s2DWewQznomibYj6xlBqtgEVCUmrrgPNkpEwFSi Y9JSsG5mAT+JE+dXgtnCAnYSBz6dYOpi5OBgAao/c80dJMwr4CLxf8o7RohlChIL7r0FG8Mp 4CrROPs3E4gtBFSzY2IXWJxFQEDi2+RDLCBjJARkJTYdYAY5R0LgOpvEzI52qDmSEgdX3GCZ wCi8gJFhFaNoakFyQXFSeq6RXnFibnFpXrpecn7uJkZICErvYFzVYHGIUYCDUYmHd8GLPQFC rIllxZW5hxglOJiVRHh3PdwbIMSbklhZlVqUH19UmpNafIjRB+iSicxSosn5wPjIK4k3NDYw NjS0NDQztTQ1wCGsJM7b7JESICSQnliSmp2aWpBaBDOOiYNTqoFxraendWBUAP9i9ZzdfxKj Q46VrvmqvNtr0sKaJ1H8Vq3nGi7wHgpr7eCde6yvupyzc8nxey9ixPYWFEjec0pYrf++al+u mfOUaqXJoU429Xf3vUnKCGg8ubor/tbTS97t+7h3vbztUfW81XPb9EWTDvq+mf3525faXd8d lc5fuLpRTmGWCedsJZbijERDLeai4kQAyph6g24CAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupikeLIzCtJLcpLzFFi42I5/e+xoK7yy70BBj93Slhc+fqezYHR4373 caYAxqgGRpuM1MSU1CKF1Lzk/JTMvHRbJe/geOd4UzMDQ11DSwtzJYW8xNxUWyUXnwBdt8wc oLFKCmWJOaVAoYDE4mIlfTtME0JD3HQtYBojdH1DguB6jAzQQMI6xoyGaScYC1rsK/48LGlg PGTcxcjJISFgIvFy2XpWCFtM4sK99WxdjFwcQgLTGSXePJjFDuGsZ5JoO7KeEaSKTUBVYuKK +2wgtoiAqUTHpKUsIDazgJ/EifMrwWxhATuJA59OMHUxcnCwANWfueYOEuYVcJH4P+UdI8Qy BYkF996CjeEUcJVonP2bCcQWAqrZMbGLbQIj7wJGhlWMoqkFyQXFSem5RnrFibnFpXnpesn5 uZsYwQH+THoH46oGi0OMAhyMSjy8C17sCRBiTSwrrsw9xCjBwawkwrvr4d4AId6UxMqq1KL8 +KLSnNTiQ4w+QEdNZJYSTc4HRl9eSbyhsYmZkaWRmbGJubExDmElcd5mj5QAIYH0xJLU7NTU gtQimHFMHJxSDYz6Mr1r70yfMOHTrQupPg+XLeVcXSsyd+/E0m5u9qIvsT0ex1Xa74gEcWX/ XKXDeOqVrmXdNMlKu7cNS9da2shPPRqszhFjdVDES78xk6NUa8OfJK+MipiLLTOe/nlSnn/z iv6ivotGfQffzExjfrP/kdG0rIQZcfPruJUtt/ze87zVvXjF+0IlluKMREMt5qLiRAAN2qK8 nQIAAA== X-CFilter-Loop: Reflected Cc: Kyungmin Park X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Changelog v2: fix page fault issue. - defer to unreference old fb to avoid page fault issue. So with this fixup, new fb would be updated to hardware prior to old fb unreferencing. And it removes unnecessary patch, "drm/exynos: Unreference fb in exynos_disable_plane()" Changelog v1: This patch releases the fb pended by page flip after fbdev is restored propely. And fixes invalid memory access when drm is released while doing pageflip. This patch makes fb's refcount to be increased when setcrtc and pageflip are requested. In other words, it increases fb's refcount only if dma is going to access memory region to fb and decreases old fb's because the old fb isn't accessed by dma anymore. This could guarantee releasing gem buffer to the fb after dma access to the gem buffer has been completed. Signed-off-by: Inki Dae Signed-off-by: Kyungmin Park --- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 42 ++++++++++++++++++++++++++++- drivers/gpu/drm/exynos/exynos_drm_fb.c | 23 ++++++++++++++++ drivers/gpu/drm/exynos/exynos_drm_fb.h | 6 ++++ drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 9 ++++++ drivers/gpu/drm/exynos/exynos_drm_plane.c | 28 ++++++++++++++++++- 5 files changed, 106 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c index 2efa4b0..b9c37eb 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c @@ -32,6 +32,7 @@ #include "exynos_drm_drv.h" #include "exynos_drm_encoder.h" #include "exynos_drm_plane.h" +#include "exynos_drm_fb.h" #define to_exynos_crtc(x) container_of(x, struct exynos_drm_crtc,\ drm_crtc) @@ -139,8 +140,25 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, plane->crtc = crtc; plane->fb = crtc->fb; + /* + * Take a reference to new fb. + * + * Taking a reference means that this plane's dma is going to access + * memory region to the new fb. + */ + drm_framebuffer_reference(plane->fb); + exynos_drm_fn_encoder(crtc, &pipe, exynos_drm_encoder_crtc_pipe); + /* + * If old_fb exists, unreference the fb. + * + * This means that memory region to the fb isn't accessed by the dma + * of this plane anymore. + */ + if (old_fb) + drm_framebuffer_unreference(old_fb); + return 0; } @@ -169,8 +187,27 @@ static int exynos_drm_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, if (ret) return ret; + plane->fb = crtc->fb; + + /* + * Take a reference to new fb. + * + * Taking a reference means that this plane's dma is going to access + * memory region to the new fb. + */ + drm_framebuffer_reference(plane->fb); + exynos_drm_crtc_commit(crtc); + /* + * If old_fb exists, unreference the fb. + * + * This means that memory region to the fb isn't accessed by the dma + * of this plane anymore. + */ + if (old_fb) + drm_framebuffer_unreference(old_fb); + return 0; } @@ -243,7 +280,7 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc, crtc->fb = fb; ret = exynos_drm_crtc_mode_set_base(crtc, crtc->x, crtc->y, - NULL); + old_fb); if (ret) { crtc->fb = old_fb; @@ -254,6 +291,9 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc, goto out; } + + exynos_drm_fb_set_pending(old_fb, false); + exynos_drm_fb_set_pending(fb, true); } out: mutex_unlock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c index 7190b64..7ed5507 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c @@ -45,11 +45,15 @@ * @fb: drm framebuffer obejct. * @buf_cnt: a buffer count to drm framebuffer. * @exynos_gem_obj: array of exynos specific gem object containing a gem object. + * @pending: indicate whehter a fb was pended by page flip. + * if true, the fb should be released after fbdev is restored to avoid + * accessing invalid memory region. */ struct exynos_drm_fb { struct drm_framebuffer fb; unsigned int buf_cnt; struct exynos_drm_gem_obj *exynos_gem_obj[MAX_FB_BUFFER]; + bool pending; }; static int check_fb_gem_memory_type(struct drm_device *drm_dev, @@ -278,6 +282,25 @@ exynos_user_fb_create(struct drm_device *dev, struct drm_file *file_priv, return fb; } +void exynos_drm_fb_set_pending(struct drm_framebuffer *fb, bool pending) +{ + struct exynos_drm_fb *exynos_fb; + + exynos_fb = to_exynos_fb(fb); + + exynos_fb->pending = pending; +} + +void exynos_drm_release_pended_fb(struct drm_framebuffer *fb) +{ + struct exynos_drm_fb *exynos_fb; + + exynos_fb = to_exynos_fb(fb); + + if (exynos_fb->pending) + drm_framebuffer_unreference(fb); +} + struct exynos_drm_gem_buf *exynos_drm_fb_buffer(struct drm_framebuffer *fb, int index) { diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.h b/drivers/gpu/drm/exynos/exynos_drm_fb.h index 96262e5..6b63bb1 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fb.h +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.h @@ -33,6 +33,12 @@ exynos_drm_framebuffer_init(struct drm_device *dev, struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object *obj); +/* set fb->pending variable to desired value. */ +void exynos_drm_fb_set_pending(struct drm_framebuffer *fb, bool pending); + +/* release a fb pended by page flip. */ +void exynos_drm_release_pended_fb(struct drm_framebuffer *fb); + /* get memory information of a drm framebuffer */ struct exynos_drm_gem_buf *exynos_drm_fb_buffer(struct drm_framebuffer *fb, int index); diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index e7466c4..62f3b9e 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c @@ -314,9 +314,18 @@ void exynos_drm_fbdev_fini(struct drm_device *dev) void exynos_drm_fbdev_restore_mode(struct drm_device *dev) { struct exynos_drm_private *private = dev->dev_private; + struct drm_framebuffer *fb, *fbt; if (!private || !private->fb_helper) return; drm_fb_helper_restore_fbdev_mode(private->fb_helper); + + mutex_lock(&dev->mode_config.mutex); + + /* Release fb pended by page flip. */ + list_for_each_entry_safe(fb, fbt, &dev->mode_config.fb_list, head) + exynos_drm_release_pended_fb(fb); + + mutex_unlock(&dev->mode_config.mutex); } diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c index 862ca1e..81d7323 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c @@ -203,11 +203,29 @@ exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, if (ret < 0) return ret; - plane->crtc = crtc; + /* + * Take a reference to new fb. + * + * Taking a reference means that this plane's dma is going to access + * memory region to the new fb. + */ + drm_framebuffer_reference(fb); + plane->crtc = crtc; exynos_plane_commit(plane); exynos_plane_dpms(plane, DRM_MODE_DPMS_ON); + /* + * If plane->fb is existed, unreference the fb. + * + * This means that memory region to the fb isn't accessed by the dma + * of this plane anymore. + */ + if (plane->fb) + drm_framebuffer_unreference(plane->fb); + + plane->fb = fb; + return 0; } @@ -215,6 +233,14 @@ static int exynos_disable_plane(struct drm_plane *plane) { DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); + /* + * Unreference the (current)fb if plane->fb is existed. + * In exynos_update_plane(), the new fb reference count can be bigger + * than 1. So it can't be removed for that reference count. + */ + if (plane->fb) + drm_framebuffer_unreference(plane->fb); + exynos_plane_dpms(plane, DRM_MODE_DPMS_OFF); return 0;