From patchwork Wed Nov 7 22:35:51 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jordan Crouse X-Patchwork-Id: 10673181 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 7B80214E2 for ; Wed, 7 Nov 2018 22:36:18 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6C6862C653 for ; Wed, 7 Nov 2018 22:36:18 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 60A532C79E; Wed, 7 Nov 2018 22:36:18 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 785EE2C653 for ; Wed, 7 Nov 2018 22:36:17 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9A12589CE2; Wed, 7 Nov 2018 22:36:16 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from smtp.codeaurora.org (smtp.codeaurora.org [198.145.29.96]) by gabe.freedesktop.org (Postfix) with ESMTPS id 703CE89D57; Wed, 7 Nov 2018 22:36:13 +0000 (UTC) Received: by smtp.codeaurora.org (Postfix, from userid 1000) id F367A60B26; Wed, 7 Nov 2018 22:36:05 +0000 (UTC) Received: from jcrouse-lnx.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: jcrouse@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id 09BDF60B7A; Wed, 7 Nov 2018 22:36:02 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 09BDF60B7A From: Jordan Crouse To: freedreno@lists.freedesktop.org Subject: [PATCH 6/9] drm/msm: Count how many times iova memory is pinned Date: Wed, 7 Nov 2018 15:35:51 -0700 Message-Id: <20181107223554.27308-7-jcrouse@codeaurora.org> X-Mailer: git-send-email 2.18.0 In-Reply-To: <20181107223554.27308-1-jcrouse@codeaurora.org> References: <20181107223554.27308-1-jcrouse@codeaurora.org> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: bzwang@chromium.org, linux-arm-msm@vger.kernel.org, seanpaul@chromium.org, dri-devel@lists.freedesktop.org, sam@ravnborg.org MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP Add a reference count to track how many times a particular chunk of iova memory is pinned (mapped) in the iomu and add msm_gem_unpin_iova to give up references. It is important to note that msm_gem_unpin_iova replaces msm_gem_put_iova because the new implicit behavior that an assigned iova in a given vma is now valid for the life of the buffer and what we are really focusing on is the use of that iova. For now the unmappings are lazy; once the reference counts go to zero they *COULD* be unmapped dynamically but that will require an outside force such as a shrinker or mm_notifiers. For now, we're just focusing on getting the counting right and setting ourselves up to be ready for the future. Signed-off-by: Jordan Crouse --- drivers/gpu/drm/msm/adreno/a5xx_debugfs.c | 6 +-- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 9 ++-- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 3 +- drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 2 +- drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 2 +- drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 2 +- drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +- drivers/gpu/drm/msm/msm_drv.h | 8 +++- drivers/gpu/drm/msm/msm_fb.c | 2 +- drivers/gpu/drm/msm/msm_gem.c | 43 ++++++++++++------- drivers/gpu/drm/msm/msm_gem.h | 1 + drivers/gpu/drm/msm/msm_gem_submit.c | 2 +- drivers/gpu/drm/msm/msm_gem_vma.c | 51 +++++++++++++++++------ drivers/gpu/drm/msm/msm_gpu.c | 2 +- 14 files changed, 87 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c index 6983cd9102bd..d9af3aff690f 100644 --- a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c +++ b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c @@ -130,15 +130,13 @@ reset_set(void *data, u64 val) adreno_gpu->fw[ADRENO_FW_PFP] = NULL; if (a5xx_gpu->pm4_bo) { - if (a5xx_gpu->pm4_iova) - msm_gem_put_iova(a5xx_gpu->pm4_bo, gpu->aspace); + msm_gem_unpin_iova(a5xx_gpu->pm4_bo, gpu->aspace); drm_gem_object_put(a5xx_gpu->pm4_bo); a5xx_gpu->pm4_bo = NULL; } if (a5xx_gpu->pfp_bo) { - if (a5xx_gpu->pfp_iova) - msm_gem_put_iova(a5xx_gpu->pfp_bo, gpu->aspace); + msm_gem_unpin_iova(a5xx_gpu->pfp_bo, gpu->aspace); drm_gem_object_put(a5xx_gpu->pfp_bo); a5xx_gpu->pfp_bo = NULL; } diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c index d6f8dedc028a..135496393cf3 100644 --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c @@ -841,20 +841,17 @@ static void a5xx_destroy(struct msm_gpu *gpu) a5xx_preempt_fini(gpu); if (a5xx_gpu->pm4_bo) { - if (a5xx_gpu->pm4_iova) - msm_gem_put_iova(a5xx_gpu->pm4_bo, gpu->aspace); + msm_gem_unpin_iova(a5xx_gpu->pm4_bo, gpu->aspace); drm_gem_object_put_unlocked(a5xx_gpu->pm4_bo); } if (a5xx_gpu->pfp_bo) { - if (a5xx_gpu->pfp_iova) - msm_gem_put_iova(a5xx_gpu->pfp_bo, gpu->aspace); + msm_gem_unpin_iova(a5xx_gpu->pfp_bo, gpu->aspace); drm_gem_object_put_unlocked(a5xx_gpu->pfp_bo); } if (a5xx_gpu->gpmu_bo) { - if (a5xx_gpu->gpmu_iova) - msm_gem_put_iova(a5xx_gpu->gpmu_bo, gpu->aspace); + msm_gem_unpin_iova(a5xx_gpu->gpmu_bo, gpu->aspace); drm_gem_object_put_unlocked(a5xx_gpu->gpmu_bo); } diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index 38b7a5a92bfb..9acf12c21b23 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -746,8 +746,7 @@ static void a6xx_destroy(struct msm_gpu *gpu) struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu); if (a6xx_gpu->sqe_bo) { - if (a6xx_gpu->sqe_iova) - msm_gem_put_iova(a6xx_gpu->sqe_bo, gpu->aspace); + msm_gem_unpin_iova(a6xx_gpu->sqe_bo, gpu->aspace); drm_gem_object_put_unlocked(a6xx_gpu->sqe_bo); } diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c index ef6884f1fc34..8f2359dc87b4 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c @@ -128,7 +128,7 @@ static void unref_cursor_worker(struct drm_flip_work *work, void *val) struct mdp4_kms *mdp4_kms = get_kms(&mdp4_crtc->base); struct msm_kms *kms = &mdp4_kms->base.base; - msm_gem_put_iova(val, kms->aspace); + msm_gem_unpin_iova(val, kms->aspace); drm_gem_object_put_unlocked(val); } diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c index 9fd6b9b2dbcb..934a49818ea4 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c @@ -165,7 +165,7 @@ static void mdp4_destroy(struct msm_kms *kms) struct msm_gem_address_space *aspace = kms->aspace; if (mdp4_kms->blank_cursor_iova) - msm_gem_put_iova(mdp4_kms->blank_cursor_bo, kms->aspace); + msm_gem_unpin_iova(mdp4_kms->blank_cursor_bo, kms->aspace); drm_gem_object_put_unlocked(mdp4_kms->blank_cursor_bo); if (aspace) { diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c index c7cc276575fb..c5fde1a4191a 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c @@ -173,7 +173,7 @@ static void unref_cursor_worker(struct drm_flip_work *work, void *val) struct mdp5_kms *mdp5_kms = get_kms(&mdp5_crtc->base); struct msm_kms *kms = &mdp5_kms->base.base; - msm_gem_put_iova(val, kms->aspace); + msm_gem_unpin_iova(val, kms->aspace); drm_gem_object_put_unlocked(val); } diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 99122767abf4..3b7092e1dcc7 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -1118,7 +1118,7 @@ static void dsi_tx_buf_free(struct msm_dsi_host *msm_host) priv = dev->dev_private; if (msm_host->tx_gem_obj) { - msm_gem_put_iova(msm_host->tx_gem_obj, priv->kms->aspace); + msm_gem_unpin_iova(msm_host->tx_gem_obj, priv->kms->aspace); drm_gem_object_put_unlocked(msm_host->tx_gem_obj); msm_host->tx_gem_obj = NULL; } diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 4372505a0bda..955c2f522db6 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -243,10 +243,14 @@ void msm_atomic_state_free(struct drm_atomic_state *state); int msm_gem_init_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma, int npages); +void msm_gem_purge_vma(struct msm_gem_address_space *aspace, + struct msm_gem_vma *vma); void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma); int msm_gem_map_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma, struct sg_table *sgt, int npages); +void msm_gem_close_vma(struct msm_gem_address_space *aspace, + struct msm_gem_vma *vma); void msm_gem_address_space_put(struct msm_gem_address_space *aspace); @@ -275,10 +279,10 @@ int msm_gem_get_and_pin_iova(struct drm_gem_object *obj, struct msm_gem_address_space *aspace, uint64_t *iova); uint64_t msm_gem_iova(struct drm_gem_object *obj, struct msm_gem_address_space *aspace); +void msm_gem_unpin_iova(struct drm_gem_object *obj, + struct msm_gem_address_space *aspace); struct page **msm_gem_get_pages(struct drm_gem_object *obj); void msm_gem_put_pages(struct drm_gem_object *obj); -void msm_gem_put_iova(struct drm_gem_object *obj, - struct msm_gem_address_space *aspace); int msm_gem_dumb_create(struct drm_file *file, struct drm_device *dev, struct drm_mode_create_dumb *args); int msm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, diff --git a/drivers/gpu/drm/msm/msm_fb.c b/drivers/gpu/drm/msm/msm_fb.c index 4e1e435db5a6..1224739094f2 100644 --- a/drivers/gpu/drm/msm/msm_fb.c +++ b/drivers/gpu/drm/msm/msm_fb.c @@ -81,7 +81,7 @@ void msm_framebuffer_cleanup(struct drm_framebuffer *fb, int i, n = fb->format->num_planes; for (i = 0; i < n; i++) - msm_gem_put_iova(fb->obj[i], aspace); + msm_gem_unpin_iova(fb->obj[i], aspace); } uint32_t msm_framebuffer_iova(struct drm_framebuffer *fb, diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index d427d9d3f013..96ff698ddee4 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -352,7 +352,8 @@ put_iova(struct drm_gem_object *obj) WARN_ON(!mutex_is_locked(&msm_obj->lock)); list_for_each_entry_safe(vma, tmp, &msm_obj->vmas, list) { - msm_gem_unmap_vma(vma->aspace, vma); + msm_gem_purge_vma(vma->aspace, vma); + msm_gem_close_vma(vma->aspace, vma); del_vma(vma); } } @@ -430,7 +431,10 @@ int msm_gem_get_and_pin_iova(struct drm_gem_object *obj, return ret; } -/* Get an iova but don't pin the memory behind it */ +/* + * Get an iova but don't pin it. Doesn't need a put because iovas are currently + * valid for the life of the object + */ int msm_gem_get_iova(struct drm_gem_object *obj, struct msm_gem_address_space *aspace, uint64_t *iova) { @@ -444,7 +448,6 @@ int msm_gem_get_iova(struct drm_gem_object *obj, return ret; } - /* get iova without taking a reference, used in places where you have * already done a 'msm_gem_get_and_pin_iova' or 'msm_gem_get_iova' */ @@ -462,15 +465,24 @@ uint64_t msm_gem_iova(struct drm_gem_object *obj, return vma ? vma->iova : 0; } -void msm_gem_put_iova(struct drm_gem_object *obj, +/* + * Unpin a iova by updating the reference counts. The memory isn't actually + * purged until something else (shrinker, mm_notifier, destroy, etc) decides + * to get rid of it + */ +void msm_gem_unpin_iova(struct drm_gem_object *obj, struct msm_gem_address_space *aspace) { - // XXX TODO .. - // NOTE: probably don't need a _locked() version.. we wouldn't - // normally unmap here, but instead just mark that it could be - // unmapped (if the iova refcnt drops to zero), but then later - // if another _get_iova_locked() fails we can start unmapping - // things that are no longer needed.. + struct msm_gem_object *msm_obj = to_msm_bo(obj); + struct msm_gem_vma *vma; + + mutex_lock(&msm_obj->lock); + vma = lookup_vma(obj, aspace); + + if (!WARN_ON(!vma)) + msm_gem_unmap_vma(aspace, vma); + + mutex_unlock(&msm_obj->lock); } int msm_gem_dumb_create(struct drm_file *file, struct drm_device *dev, @@ -786,11 +798,12 @@ void msm_gem_describe(struct drm_gem_object *obj, struct seq_file *m) if (!list_empty(&msm_obj->vmas)) { - seq_puts(m, " vmas:"); + seq_puts(m, " vmas:"); list_for_each_entry(vma, &msm_obj->vmas, list) - seq_printf(m, " [%s: %08llx,%s]", vma->aspace->name, - vma->iova, vma->mapped ? "mapped" : "unmapped"); + seq_printf(m, " [%s: %08llx,%s,inuse=%d]", vma->aspace->name, + vma->iova, vma->mapped ? "mapped" : "unmapped", + vma->inuse); seq_puts(m, "\n"); } @@ -1095,7 +1108,7 @@ static void *_msm_gem_kernel_new(struct drm_device *dev, uint32_t size, vaddr = msm_gem_get_vaddr(obj); if (IS_ERR(vaddr)) { - msm_gem_put_iova(obj, aspace); + msm_gem_unpin_iova(obj, aspace); drm_gem_object_put(obj); return ERR_CAST(vaddr); } @@ -1127,7 +1140,7 @@ void msm_gem_kernel_put(struct drm_gem_object *bo, return; msm_gem_put_vaddr(bo); - msm_gem_put_iova(bo, aspace); + msm_gem_unpin_iova(bo, aspace); if (locked) drm_gem_object_put(bo); diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 32c13e6559b5..53d3bf49a9fa 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -42,6 +42,7 @@ struct msm_gem_vma { struct msm_gem_address_space *aspace; struct list_head list; /* node in msm_gem_object::vmas */ bool mapped; + int inuse; }; struct msm_gem_object { diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index da907260660c..9c78521ee08a 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -167,7 +167,7 @@ static void submit_unlock_unpin_bo(struct msm_gem_submit *submit, struct msm_gem_object *msm_obj = submit->bos[i].obj; if (submit->bos[i].flags & BO_PINNED) - msm_gem_put_iova(&msm_obj->base, submit->gpu->aspace); + msm_gem_unpin_iova(&msm_obj->base, submit->gpu->aspace); if (submit->bos[i].flags & BO_LOCKED) ww_mutex_unlock(&msm_obj->resv->lock); diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c index c4c42bf0db0e..ee46d8321b05 100644 --- a/drivers/gpu/drm/msm/msm_gem_vma.c +++ b/drivers/gpu/drm/msm/msm_gem_vma.c @@ -38,26 +38,32 @@ void msm_gem_address_space_put(struct msm_gem_address_space *aspace) kref_put(&aspace->kref, msm_gem_address_space_destroy); } -void -msm_gem_unmap_vma(struct msm_gem_address_space *aspace, +/* Actually unmap memory for the vma */ +void msm_gem_purge_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma) { - if (!aspace || !vma->iova) + unsigned size = vma->node.size << PAGE_SHIFT; + + /* Print a message if we try to purge a vma in use */ + if (WARN_ON(vma->inuse > 0)) return; - if (aspace->mmu) { - unsigned size = vma->node.size << PAGE_SHIFT; - aspace->mmu->funcs->unmap(aspace->mmu, vma->iova, size); - } + /* Don't do anything if the memory isn't mapped */ + if (!vma->mapped) + return; - spin_lock(&aspace->lock); - drm_mm_remove_node(&vma->node); - spin_unlock(&aspace->lock); + if (aspace->mmu) + aspace->mmu->funcs->unmap(aspace->mmu, vma->iova, size); - vma->iova = 0; vma->mapped = false; +} - msm_gem_address_space_put(aspace); +/* Remove reference counts for the mapping */ +void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, + struct msm_gem_vma *vma) +{ + if (!WARN_ON(!vma->iova)) + vma->inuse--; } int @@ -70,6 +76,9 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace, if (WARN_ON(!vma->iova)) return -EINVAL; + /* Increase the usage counter */ + vma->inuse++; + if (vma->mapped) return 0; @@ -85,6 +94,23 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace, return ret; } +/* Close an iova. Warn if it is still in use */ +void msm_gem_close_vma(struct msm_gem_address_space *aspace, + struct msm_gem_vma *vma) +{ + if (WARN_ON(vma->inuse > 0 || vma->mapped)) + return; + + spin_lock(&aspace->lock); + if (vma->iova) + drm_mm_remove_node(&vma->node); + spin_unlock(&aspace->lock); + + vma->iova = 0; + + msm_gem_address_space_put(aspace); +} + /* Initialize a new vma and allocate an iova for it */ int msm_gem_init_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma, int npages) @@ -109,6 +135,7 @@ int msm_gem_init_vma(struct msm_gem_address_space *aspace, return 0; } + struct msm_gem_address_space * msm_gem_address_space_create(struct device *dev, struct iommu_domain *domain, const char *name) diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index 13dab9527bda..e6ce24bfb9b1 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -670,7 +670,7 @@ static void retire_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit) struct msm_gem_object *msm_obj = submit->bos[i].obj; /* move to inactive: */ msm_gem_move_to_inactive(&msm_obj->base); - msm_gem_put_iova(&msm_obj->base, gpu->aspace); + msm_gem_unpin_iova(&msm_obj->base, gpu->aspace); drm_gem_object_put(&msm_obj->base); }