From b6047ebaf7b3fb63c3ad49ec9c381a0e0b9fb82a Mon Sep 17 00:00:00 2001 From: Christian Gmeiner Date: Thu, 19 Oct 2017 10:40:54 +0200 Subject: [PATCH 01/32] drm/etnaviv: add sensitive state for occlusion query address Add GL.OCCLUSION_QUERY_ADDR (0x03824): address where GPU stores the occlusion query result. Signed-off-by: Christian Gmeiner Signed-off-by: Lucas Stach --- drivers/gpu/drm/etnaviv/etnaviv_cmd_parser.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmd_parser.c b/drivers/gpu/drm/etnaviv/etnaviv_cmd_parser.c index 6e3bbcf24160e..68e6d3772ad84 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_cmd_parser.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_cmd_parser.c @@ -78,6 +78,7 @@ static const struct { ST(0x17c0, 8), ST(0x17e0, 8), ST(0x2400, 14 * 16), + ST(0x3824, 1), ST(0x10800, 32 * 16), ST(0x14600, 16), ST(0x14800, 8 * 8), From ff9815957768b6e76d6895a0597d1ede05b4378b Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Mon, 23 Oct 2017 21:27:30 +0200 Subject: [PATCH 02/32] drm/etnaviv: Improve unlocking of a mutex in etnaviv_iommu_map_gem() Add a jump target so that a call of the function "mutex_unlock" is stored only once at the end of this function implementation. Replace three calls by goto statements. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring Signed-off-by: Lucas Stach --- drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c index 35074b9447780..d113fe06e6b57 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c @@ -263,18 +263,16 @@ int etnaviv_iommu_map_gem(struct etnaviv_iommu *mmu, if (iova < 0x80000000 - sg_dma_len(sgt->sgl)) { mapping->iova = iova; list_add_tail(&mapping->mmu_node, &mmu->mappings); - mutex_unlock(&mmu->lock); - return 0; + ret = 0; + goto unlock; } } node = &mapping->vram_node; ret = etnaviv_iommu_find_iova(mmu, node, etnaviv_obj->base.size); - if (ret < 0) { - mutex_unlock(&mmu->lock); - return ret; - } + if (ret < 0) + goto unlock; mmu->last_iova = node->start + etnaviv_obj->base.size; mapping->iova = node->start; @@ -283,12 +281,12 @@ int etnaviv_iommu_map_gem(struct etnaviv_iommu *mmu, if (ret < 0) { drm_mm_remove_node(node); - mutex_unlock(&mmu->lock); - return ret; + goto unlock; } list_add_tail(&mapping->mmu_node, &mmu->mappings); mmu->need_flush = true; +unlock: mutex_unlock(&mmu->lock); return ret; From 49b82c389d2a40eaef1355aaa35868b367aec9d1 Mon Sep 17 00:00:00 2001 From: Philipp Zabel Date: Fri, 1 Dec 2017 16:00:41 +0100 Subject: [PATCH 03/32] drm/etnaviv: make THERMAL selectable The etnaviv driver causes a link failure if it is built-in but THERMAL is built as a module: drivers/gpu/drm/etnaviv/etnaviv_gpu.o: In function `etnaviv_gpu_bind': etnaviv_gpu.c:(.text+0x4c4): undefined reference to `thermal_of_cooling_device_register' etnaviv_gpu.c:(.text+0x600): undefined reference to `thermal_cooling_device_unregister' drivers/gpu/drm/etnaviv/etnaviv_gpu.o: In function `etnaviv_gpu_unbind': etnaviv_gpu.c:(.text+0x2aac): undefined reference to `thermal_cooling_device_unregister' Adding a Kconfig dependency on THERMAL || !THERMAL to avoid this causes a dependency loop on x86_64: drivers/gpu/drm/tve200/Kconfig:1:error: recursive dependency detected! For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/tve200/Kconfig:1: symbol DRM_TVE200 depends on CMA For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" mm/Kconfig:489: symbol CMA is selected by DRM_ETNAVIV For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/etnaviv/Kconfig:2: symbol DRM_ETNAVIV depends on THERMAL For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/thermal/Kconfig:5: symbol THERMAL is selected by ACPI_VIDEO For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/acpi/Kconfig:189: symbol ACPI_VIDEO is selected by BACKLIGHT_CLASS_DEVICE For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/backlight/Kconfig:158: symbol BACKLIGHT_CLASS_DEVICE is selected by DRM_PARADE_PS8622 For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/bridge/Kconfig:62: symbol DRM_PARADE_PS8622 depends on DRM_BRIDGE For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/bridge/Kconfig:1: symbol DRM_BRIDGE is selected by DRM_TVE200 To work around this, add a new option DRM_ETNAVIV_THERMAL to optionally enable thermal throttling support and make DRM_ETNAVIV select THERMAL at the same time. Reported-by: Stephen Rothwell Signed-off-by: Philipp Zabel Signed-off-by: Lucas Stach --- drivers/gpu/drm/etnaviv/Kconfig | 9 +++++++++ drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 8 +++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig index a29b8f59eb15d..3f58b4077767d 100644 --- a/drivers/gpu/drm/etnaviv/Kconfig +++ b/drivers/gpu/drm/etnaviv/Kconfig @@ -6,6 +6,7 @@ config DRM_ETNAVIV depends on MMU select SHMEM select SYNC_FILE + select THERMAL if DRM_ETNAVIV_THERMAL select TMPFS select WANT_DEV_COREDUMP select CMA if HAVE_DMA_CONTIGUOUS @@ -13,6 +14,14 @@ config DRM_ETNAVIV help DRM driver for Vivante GPUs. +config DRM_ETNAVIV_THERMAL + bool "enable ETNAVIV thermal throttling" + depends on DRM_ETNAVIV + default y + help + Compile in support for thermal throttling. + Say Y unless you want to risk burning your SoC. + config DRM_ETNAVIV_REGISTER_LOGGING bool "enable ETNAVIV register logging" depends on DRM_ETNAVIV diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index e19cbe05da2a3..968cbc2be9c45 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1738,7 +1738,7 @@ static int etnaviv_gpu_bind(struct device *dev, struct device *master, struct etnaviv_gpu *gpu = dev_get_drvdata(dev); int ret; - if (IS_ENABLED(CONFIG_THERMAL)) { + if (IS_ENABLED(CONFIG_DRM_ETNAVIV_THERMAL)) { gpu->cooling = thermal_of_cooling_device_register(dev->of_node, (char *)dev_name(dev), gpu, &cooling_ops); if (IS_ERR(gpu->cooling)) @@ -1751,7 +1751,8 @@ static int etnaviv_gpu_bind(struct device *dev, struct device *master, ret = etnaviv_gpu_clk_enable(gpu); #endif if (ret < 0) { - thermal_cooling_device_unregister(gpu->cooling); + if (IS_ENABLED(CONFIG_DRM_ETNAVIV_THERMAL)) + thermal_cooling_device_unregister(gpu->cooling); return ret; } @@ -1808,7 +1809,8 @@ static void etnaviv_gpu_unbind(struct device *dev, struct device *master, gpu->drm = NULL; - thermal_cooling_device_unregister(gpu->cooling); + if (IS_ENABLED(CONFIG_DRM_ETNAVIV_THERMAL)) + thermal_cooling_device_unregister(gpu->cooling); gpu->cooling = NULL; } From b9a48aa76c72fb4d0990425f82fdeb4fdff2b2b1 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Thu, 19 Oct 2017 13:48:40 +0200 Subject: [PATCH 04/32] drm/etnaviv: fix GPU vs sync point race If the FE is restarted before the sync point event is cleared, the GPU might trigger a completion IRQ for the next sync point, corrupting the state of the currently running worker. Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel Reviewed-by: Christian Gmeiner --- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 968cbc2be9c45..23b16d9746f9a 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1484,22 +1484,18 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, return ret; } -static void etnaviv_process_sync_point(struct etnaviv_gpu *gpu, - struct etnaviv_event *event) -{ - u32 addr = gpu_read(gpu, VIVS_FE_DMA_ADDRESS); - - event->sync_point(gpu, event); - etnaviv_gpu_start_fe(gpu, addr + 2, 2); -} - static void sync_point_worker(struct work_struct *work) { struct etnaviv_gpu *gpu = container_of(work, struct etnaviv_gpu, sync_point_work); + struct etnaviv_event *event = &gpu->event[gpu->sync_point_event]; + u32 addr = gpu_read(gpu, VIVS_FE_DMA_ADDRESS); - etnaviv_process_sync_point(gpu, &gpu->event[gpu->sync_point_event]); + event->sync_point(gpu, event); event_free(gpu, gpu->sync_point_event); + + /* restart FE last to avoid GPU and IRQ racing against this worker */ + etnaviv_gpu_start_fe(gpu, addr + 2, 2); } /* From d6a8743dd8cc1e2ca5e40e07fe3de7557ac117a0 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 17 Nov 2017 11:14:58 +0100 Subject: [PATCH 05/32] drm/etnaviv: split obj locks in different classes depending on the obj type Userptr, prime and shmem buffer objects have different lock ordering requirements. This is mostly due to the fact that we don't allow to mmap userptr buffers, so we won't ever end up in our fault handler for those, so some of the code paths are never called with the mmap_sem held. To avoid lockdep false positives, split them up into different lock classes. Signed-off-by: Lucas Stach Reviewed-by: Christian Gmeiner Reviewed-by: Philipp Zabel --- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 7 +++++++ drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 3 +++ 2 files changed, 10 insertions(+) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c index daee3f1196df8..e3582507963dd 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c @@ -24,6 +24,9 @@ #include "etnaviv_gpu.h" #include "etnaviv_mmu.h" +static struct lock_class_key etnaviv_shm_lock_class; +static struct lock_class_key etnaviv_userptr_lock_class; + static void etnaviv_gem_scatter_map(struct etnaviv_gem_object *etnaviv_obj) { struct drm_device *dev = etnaviv_obj->base.dev; @@ -653,6 +656,8 @@ static struct drm_gem_object *__etnaviv_gem_new(struct drm_device *dev, if (ret) goto fail; + lockdep_set_class(&to_etnaviv_bo(obj)->lock, &etnaviv_shm_lock_class); + ret = drm_gem_object_init(dev, obj, size); if (ret == 0) { struct address_space *mapping; @@ -897,6 +902,8 @@ int etnaviv_gem_new_userptr(struct drm_device *dev, struct drm_file *file, if (ret) return ret; + lockdep_set_class(&etnaviv_obj->lock, &etnaviv_userptr_lock_class); + etnaviv_obj->userptr.ptr = ptr; etnaviv_obj->userptr.task = current; etnaviv_obj->userptr.ro = !(flags & ETNA_USERPTR_WRITE); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c index ae884723e9b1b..ea87bf87b187f 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c @@ -19,6 +19,7 @@ #include "etnaviv_drv.h" #include "etnaviv_gem.h" +static struct lock_class_key etnaviv_prime_lock_class; struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj) { @@ -125,6 +126,8 @@ struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev, if (ret < 0) return ERR_PTR(ret); + lockdep_set_class(&etnaviv_obj->lock, &etnaviv_prime_lock_class); + npages = size / PAGE_SIZE; etnaviv_obj->sgt = sgt; From 783c06cb9c9072ff87903cce73317209b3af8568 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 17 Nov 2017 11:23:17 +0100 Subject: [PATCH 06/32] drm/etnaviv: add lockdep annotation for userptr object population The current userptr page population will defer work to a work item if needed to avoid ever taking the mmap_sem in the direct call path. With the more fine-grained locking in etnaviv this isn't needed anymore, so a future commit will simplify this code. Add a lockdep annotation to validate the assumption that the mmap_sem can be taken in the direct call path. Signed-off-by: Lucas Stach Reviewed-by: Christian Gmeiner --- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c index e3582507963dd..555a331c51a9a 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c @@ -804,6 +804,8 @@ static int etnaviv_gem_userptr_get_pages(struct etnaviv_gem_object *etnaviv_obj) struct mm_struct *mm; int ret, pinned, npages = etnaviv_obj->base.size >> PAGE_SHIFT; + might_lock_read(¤t->mm->mmap_sem); + if (etnaviv_obj->userptr.work) { if (IS_ERR(etnaviv_obj->userptr.work)) { ret = PTR_ERR(etnaviv_obj->userptr.work); From cdd325632d4b1723d5e3bbe7c20f030fbac304ae Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 17 Nov 2017 12:11:25 +0100 Subject: [PATCH 07/32] drm/etnaviv: fold __etnaviv_gem_new into caller This function has only one caller and it isn't expected that there will be any more in the future. Folding this function into the caller is helping the readability. Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel Reviewed-by: Christian Gmeiner --- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c index 555a331c51a9a..f75105b7e1a41 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c @@ -643,8 +643,9 @@ static int etnaviv_gem_new_impl(struct drm_device *dev, u32 size, u32 flags, return 0; } -static struct drm_gem_object *__etnaviv_gem_new(struct drm_device *dev, - u32 size, u32 flags) +/* convenience method to construct a GEM buffer object, and userspace handle */ +int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file, + u32 size, u32 flags, u32 *handle) { struct drm_gem_object *obj = NULL; int ret; @@ -665,7 +666,7 @@ static struct drm_gem_object *__etnaviv_gem_new(struct drm_device *dev, /* * Our buffers are kept pinned, so allocating them * from the MOVABLE zone is a really bad idea, and - * conflicts with CMA. See coments above new_inode() + * conflicts with CMA. See comments above new_inode() * why this is required _and_ expected if you're * going to pin these pages. */ @@ -677,24 +678,6 @@ static struct drm_gem_object *__etnaviv_gem_new(struct drm_device *dev, if (ret) goto fail; - return obj; - -fail: - drm_gem_object_put_unlocked(obj); - return ERR_PTR(ret); -} - -/* convenience method to construct a GEM buffer object, and userspace handle */ -int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file, - u32 size, u32 flags, u32 *handle) -{ - struct drm_gem_object *obj; - int ret; - - obj = __etnaviv_gem_new(dev, size, flags); - if (IS_ERR(obj)) - return PTR_ERR(obj); - ret = etnaviv_gem_obj_add(dev, obj); if (ret < 0) { drm_gem_object_put_unlocked(obj); @@ -704,6 +687,7 @@ int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file, ret = drm_gem_handle_create(file, obj, handle); /* drop reference from allocate - handle holds it now */ +fail: drm_gem_object_put_unlocked(obj); return ret; From 54f09288f912058bdf23c4946204729ee288f444 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 17 Nov 2017 12:17:14 +0100 Subject: [PATCH 08/32] drm/etnaviv: change return type of etnaviv_gem_obj_add to void This function never fails, as it does nothing more than adding the GEM object to the global device list. Making this explicit through the void return type allows to drop some unnecessary error handling. Signed-off-by: Lucas Stach Reviewed-by: Christian Gmeiner Reviewed-by: Philipp Zabel --- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 16 ++++------------ drivers/gpu/drm/etnaviv/etnaviv_gem.h | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 4 +--- 3 files changed, 6 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c index f75105b7e1a41..a52220eeee45e 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c @@ -586,7 +586,7 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj) kfree(etnaviv_obj); } -int etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj) +void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj) { struct etnaviv_drm_private *priv = dev->dev_private; struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj); @@ -594,8 +594,6 @@ int etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj) mutex_lock(&priv->gem_lock); list_add_tail(&etnaviv_obj->gem_node, &priv->gem_list); mutex_unlock(&priv->gem_lock); - - return 0; } static int etnaviv_gem_new_impl(struct drm_device *dev, u32 size, u32 flags, @@ -678,11 +676,7 @@ int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file, if (ret) goto fail; - ret = etnaviv_gem_obj_add(dev, obj); - if (ret < 0) { - drm_gem_object_put_unlocked(obj); - return ret; - } + etnaviv_gem_obj_add(dev, obj); ret = drm_gem_handle_create(file, obj, handle); @@ -895,12 +889,10 @@ int etnaviv_gem_new_userptr(struct drm_device *dev, struct drm_file *file, etnaviv_obj->userptr.ro = !(flags & ETNA_USERPTR_WRITE); get_task_struct(current); - ret = etnaviv_gem_obj_add(dev, &etnaviv_obj->base); - if (ret) - goto unreference; + etnaviv_gem_obj_add(dev, &etnaviv_obj->base); ret = drm_gem_handle_create(file, &etnaviv_obj->base, handle); -unreference: + /* drop reference from allocate - handle holds it now */ drm_gem_object_put_unlocked(&etnaviv_obj->base); return ret; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h index e437fba1209d9..00bd9c851a02c 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h @@ -117,7 +117,7 @@ int etnaviv_gem_wait_bo(struct etnaviv_gpu *gpu, struct drm_gem_object *obj, int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags, struct reservation_object *robj, const struct etnaviv_gem_ops *ops, struct etnaviv_gem_object **res); -int etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj); +void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj); struct page **etnaviv_gem_get_pages(struct etnaviv_gem_object *obj); void etnaviv_gem_put_pages(struct etnaviv_gem_object *obj); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c index ea87bf87b187f..5704305d41e69 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c @@ -142,9 +142,7 @@ struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev, if (ret) goto fail; - ret = etnaviv_gem_obj_add(dev, &etnaviv_obj->base); - if (ret) - goto fail; + etnaviv_gem_obj_add(dev, &etnaviv_obj->base); return &etnaviv_obj->base; From b2295c247c95b1a793eb700eef28a0f81ca1556e Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 17 Nov 2017 14:16:10 +0100 Subject: [PATCH 09/32] drm/etnaviv: get rid of userptr worker All code paths which populate userptr BOs are fine with the get_pages function taking the mmap_sem lock. This allows to get rid of the pretty involved architecture with a worker being scheduled if the mmap_sem needs to be taken, but instead call GUP directly and allow it to take the lock if necessary. This simplifies the code a lot and removes the possibility of this function returning -EAGAIN, which complicates object population handling at the callers. A notable change in behavior is that we don't allow a process to populate objects with user pages from a foreign MM anymore. This would have been an invalid use before, as it breaks the assumptions made in the etnaviv kernel driver to enfore cache coherence. We now disallow this by rejecting the request to populate those objects. Well behaving userspace is unaffected by this change. Signed-off-by: Lucas Stach --- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 146 ++++---------------------- drivers/gpu/drm/etnaviv/etnaviv_gem.h | 3 +- 2 files changed, 23 insertions(+), 126 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c index a52220eeee45e..fcc969fa0e692 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c @@ -705,141 +705,41 @@ int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags, return 0; } -struct get_pages_work { - struct work_struct work; - struct mm_struct *mm; - struct task_struct *task; - struct etnaviv_gem_object *etnaviv_obj; -}; - -static struct page **etnaviv_gem_userptr_do_get_pages( - struct etnaviv_gem_object *etnaviv_obj, struct mm_struct *mm, struct task_struct *task) -{ - int ret = 0, pinned, npages = etnaviv_obj->base.size >> PAGE_SHIFT; - struct page **pvec; - uintptr_t ptr; - unsigned int flags = 0; - - pvec = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL); - if (!pvec) - return ERR_PTR(-ENOMEM); - - if (!etnaviv_obj->userptr.ro) - flags |= FOLL_WRITE; - - pinned = 0; - ptr = etnaviv_obj->userptr.ptr; - - down_read(&mm->mmap_sem); - while (pinned < npages) { - ret = get_user_pages_remote(task, mm, ptr, npages - pinned, - flags, pvec + pinned, NULL, NULL); - if (ret < 0) - break; - - ptr += ret * PAGE_SIZE; - pinned += ret; - } - up_read(&mm->mmap_sem); - - if (ret < 0) { - release_pages(pvec, pinned); - kvfree(pvec); - return ERR_PTR(ret); - } - - return pvec; -} - -static void __etnaviv_gem_userptr_get_pages(struct work_struct *_work) -{ - struct get_pages_work *work = container_of(_work, typeof(*work), work); - struct etnaviv_gem_object *etnaviv_obj = work->etnaviv_obj; - struct page **pvec; - - pvec = etnaviv_gem_userptr_do_get_pages(etnaviv_obj, work->mm, work->task); - - mutex_lock(&etnaviv_obj->lock); - if (IS_ERR(pvec)) { - etnaviv_obj->userptr.work = ERR_CAST(pvec); - } else { - etnaviv_obj->userptr.work = NULL; - etnaviv_obj->pages = pvec; - } - - mutex_unlock(&etnaviv_obj->lock); - drm_gem_object_put_unlocked(&etnaviv_obj->base); - - mmput(work->mm); - put_task_struct(work->task); - kfree(work); -} - static int etnaviv_gem_userptr_get_pages(struct etnaviv_gem_object *etnaviv_obj) { struct page **pvec = NULL; - struct get_pages_work *work; - struct mm_struct *mm; - int ret, pinned, npages = etnaviv_obj->base.size >> PAGE_SHIFT; + struct etnaviv_gem_userptr *userptr = &etnaviv_obj->userptr; + int ret, pinned = 0, npages = etnaviv_obj->base.size >> PAGE_SHIFT; might_lock_read(¤t->mm->mmap_sem); - if (etnaviv_obj->userptr.work) { - if (IS_ERR(etnaviv_obj->userptr.work)) { - ret = PTR_ERR(etnaviv_obj->userptr.work); - etnaviv_obj->userptr.work = NULL; - } else { - ret = -EAGAIN; - } - return ret; - } + if (userptr->mm != current->mm) + return -EPERM; - mm = get_task_mm(etnaviv_obj->userptr.task); - pinned = 0; - if (mm == current->mm) { - pvec = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL); - if (!pvec) { - mmput(mm); - return -ENOMEM; - } + pvec = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL); + if (!pvec) + return -ENOMEM; + + do { + unsigned num_pages = npages - pinned; + uint64_t ptr = userptr->ptr + pinned * PAGE_SIZE; + struct page **pages = pvec + pinned; - pinned = __get_user_pages_fast(etnaviv_obj->userptr.ptr, npages, - !etnaviv_obj->userptr.ro, pvec); - if (pinned < 0) { + ret = get_user_pages_fast(ptr, num_pages, + !userptr->ro ? FOLL_WRITE : 0, pages); + if (ret < 0) { + release_pages(pvec, pinned); kvfree(pvec); - mmput(mm); - return pinned; + return ret; } - if (pinned == npages) { - etnaviv_obj->pages = pvec; - mmput(mm); - return 0; - } - } - - release_pages(pvec, pinned); - kvfree(pvec); - - work = kmalloc(sizeof(*work), GFP_KERNEL); - if (!work) { - mmput(mm); - return -ENOMEM; - } - - get_task_struct(current); - drm_gem_object_get(&etnaviv_obj->base); - - work->mm = mm; - work->task = current; - work->etnaviv_obj = etnaviv_obj; + pinned += ret; - etnaviv_obj->userptr.work = &work->work; - INIT_WORK(&work->work, __etnaviv_gem_userptr_get_pages); + } while (pinned < npages); - etnaviv_queue_work(etnaviv_obj->base.dev, &work->work); + etnaviv_obj->pages = pvec; - return -EAGAIN; + return 0; } static void etnaviv_gem_userptr_release(struct etnaviv_gem_object *etnaviv_obj) @@ -855,7 +755,6 @@ static void etnaviv_gem_userptr_release(struct etnaviv_gem_object *etnaviv_obj) release_pages(etnaviv_obj->pages, npages); kvfree(etnaviv_obj->pages); } - put_task_struct(etnaviv_obj->userptr.task); } static int etnaviv_gem_userptr_mmap_obj(struct etnaviv_gem_object *etnaviv_obj, @@ -885,9 +784,8 @@ int etnaviv_gem_new_userptr(struct drm_device *dev, struct drm_file *file, lockdep_set_class(&etnaviv_obj->lock, &etnaviv_userptr_lock_class); etnaviv_obj->userptr.ptr = ptr; - etnaviv_obj->userptr.task = current; + etnaviv_obj->userptr.mm = current->mm; etnaviv_obj->userptr.ro = !(flags & ETNA_USERPTR_WRITE); - get_task_struct(current); etnaviv_gem_obj_add(dev, &etnaviv_obj->base); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h index 00bd9c851a02c..d1a7d040ac97f 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h @@ -26,8 +26,7 @@ struct etnaviv_gem_object; struct etnaviv_gem_userptr { uintptr_t ptr; - struct task_struct *task; - struct work_struct *work; + struct mm_struct *mm; bool ro; }; From b7b17e5cec541f58131f4a7963bbe3448f0a9c00 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 17 Nov 2017 14:24:34 +0100 Subject: [PATCH 10/32] drm/etnaviv: remove -EAGAIN handling from submit path Now that the userptr BO handling doesn't rely on the userspace restarting the submit after object population, there is no need to special case the -EAGAIN return value anymore. Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel --- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index ff911541a190a..8fa31ab1fb0a4 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -534,14 +534,6 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, out: submit_unpin_objects(submit); - /* - * If we're returning -EAGAIN, it may be due to the userptr code - * wanting to run its workqueue outside of any locks. Flush our - * workqueue to ensure that it is run in a timely manner. - */ - if (ret == -EAGAIN) - flush_workqueue(priv->wq); - err_submit_objects: if (in_fence) dma_fence_put(in_fence); From 3057e3f74516bd61d0aa3a44e169714b76616cfd Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 17 Nov 2017 15:09:05 +0100 Subject: [PATCH 11/32] drm/etnaviv: remove stale TODO in etnaviv_gpu_submit Flush and prefetch are properly handled in the buffer code, data endianess would need much wider changes than adding something to this single function. Signed-off-by: Lucas Stach Reviewed-by: Christian Gmeiner --- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 23b16d9746f9a..95187ebe16606 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1390,15 +1390,6 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, if (ret < 0) return ret; - /* - * TODO - * - * - flush - * - data endian - * - prefetch - * - */ - /* * if there are performance monitor requests we need to have * - a sync point to re-configure gpu and process ETNA_PM_PROCESS_PRE From fa67ac84a33eeb5a3702970fe75083be79f460f9 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 17 Nov 2017 16:35:32 +0100 Subject: [PATCH 12/32] drm/etnaviv: don't flush workqueue in etnaviv_gpu_wait_obj_inactive There is no need to synchronize with oustanding retire jobs if the object has gone idle. Retire jobs only ever change the object state from active to idle, not the other way around. The IOVA put race is uncritical, as the GEM_WAIT ioctl itself is holding a reference to the GEM object, so the retire worker will not pull the object into the CPU domain, which is the thing we are trying to guard against with etnaviv_gpu_wait_obj_inactive. The ordering of the various counts and waits may change a bit, but the userspace visible behavior at the bounds of the syscall are unchanged. Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel --- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 95187ebe16606..3738383474fbb 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1295,17 +1295,12 @@ int etnaviv_gpu_wait_obj_inactive(struct etnaviv_gpu *gpu, ret = wait_event_interruptible_timeout(gpu->fence_event, !is_active(etnaviv_obj), remaining); - if (ret > 0) { - struct etnaviv_drm_private *priv = gpu->drm->dev_private; - - /* Synchronise with the retire worker */ - flush_workqueue(priv->wq); + if (ret > 0) return 0; - } else if (ret == -ERESTARTSYS) { + else if (ret == -ERESTARTSYS) return -ERESTARTSYS; - } else { + else return -ETIMEDOUT; - } } int etnaviv_gpu_pm_get_sync(struct etnaviv_gpu *gpu) From 4375ffffbf099f14815380a3d9e5784ffc55bf31 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 17 Nov 2017 17:19:50 +0100 Subject: [PATCH 13/32] drm/etnaviv: remove switch_context member from etnaviv_gpu There is no need to store this in the gpu struct. MMU flushes are triggered correctly in reaction to MMU maps and unmaps, independent of the current ctx. Any required pipe switches can be infered from the current and the desired GPU exec state. Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel Reviewed-by: Christian Gmeiner --- drivers/gpu/drm/etnaviv/etnaviv_buffer.c | 10 ++++++---- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 8 +------- drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 1 - 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c index 9e7098e3207f5..6ad8972a59cc3 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c @@ -294,6 +294,7 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, unsigned int event, unsigned int waitlink_offset = buffer->user_size - 16; u32 return_target, return_dwords; u32 link_target, link_dwords; + bool switch_context = gpu->exec_state != cmdbuf->exec_state; if (drm_debug & DRM_UT_DRIVER) etnaviv_buffer_dump(gpu, buffer, 0, 0x50); @@ -306,7 +307,7 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, unsigned int event, * need to append a mmu flush load state, followed by a new * link to this buffer - a total of four additional words. */ - if (gpu->mmu->need_flush || gpu->switch_context) { + if (gpu->mmu->need_flush || switch_context) { u32 target, extra_dwords; /* link command */ @@ -321,7 +322,7 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, unsigned int event, } /* pipe switch commands */ - if (gpu->switch_context) + if (switch_context) extra_dwords += 4; target = etnaviv_buffer_reserve(gpu, buffer, extra_dwords); @@ -349,10 +350,9 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, unsigned int event, gpu->mmu->need_flush = false; } - if (gpu->switch_context) { + if (switch_context) { etnaviv_cmd_select_pipe(gpu, buffer, cmdbuf->exec_state); gpu->exec_state = cmdbuf->exec_state; - gpu->switch_context = false; } /* And the link to the submitted buffer */ @@ -421,4 +421,6 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, unsigned int event, if (drm_debug & DRM_UT_DRIVER) etnaviv_buffer_dump(gpu, buffer, 0, 0x50); + + gpu->lastctx = cmdbuf->ctx; } diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 3738383474fbb..6ce4b9d236b4d 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1416,12 +1416,6 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, submit->fence = dma_fence_get(fence); gpu->active_fence = submit->fence->seqno; - if (gpu->lastctx != cmdbuf->ctx) { - gpu->mmu->need_flush = true; - gpu->switch_context = true; - gpu->lastctx = cmdbuf->ctx; - } - if (cmdbuf->nr_pmrs) { gpu->event[event[1]].sync_point = &sync_point_perfmon_sample_pre; gpu->event[event[1]].cmdbuf = cmdbuf; @@ -1662,7 +1656,7 @@ static int etnaviv_gpu_hw_resume(struct etnaviv_gpu *gpu) etnaviv_gpu_update_clock(gpu); etnaviv_gpu_hw_init(gpu); - gpu->switch_context = true; + gpu->lastctx = NULL; gpu->exec_state = -1; mutex_unlock(&gpu->lock); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h index 4f10f147297ab..15090bb68f5a1 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h @@ -106,7 +106,6 @@ struct etnaviv_gpu { struct mutex lock; struct etnaviv_chip_identity identity; struct etnaviv_file_private *lastctx; - bool switch_context; /* 'ring'-buffer: */ struct etnaviv_cmdbuf *buffer; From a7790d78092e5904beb4de71e1ea43b260d2092a Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 17 Nov 2017 17:43:37 +0100 Subject: [PATCH 14/32] drm/etnaviv: move workqueue to be per GPU While the etnaviv workqueue needs to be ordered, as we rely on work items being executed in queuing order, this is only true for a single GPU. Having a shared workqueue for all GPUs in the system limits concurrency artificially. Getting each GPU its own ordered workqueue still meets our ordering expectations and enables retire workers to run concurrently. Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel --- drivers/gpu/drm/etnaviv/etnaviv_drv.c | 12 ------------ drivers/gpu/drm/etnaviv/etnaviv_drv.h | 10 ---------- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 19 +++++++++++++++---- drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 1 + 4 files changed, 16 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c index 491eddf9b1502..ca03b5e4789ba 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c @@ -580,12 +580,6 @@ static int etnaviv_bind(struct device *dev) } drm->dev_private = priv; - priv->wq = alloc_ordered_workqueue("etnaviv", 0); - if (!priv->wq) { - ret = -ENOMEM; - goto out_wq; - } - mutex_init(&priv->gem_lock); INIT_LIST_HEAD(&priv->gem_list); priv->num_gpus = 0; @@ -607,9 +601,6 @@ static int etnaviv_bind(struct device *dev) out_register: component_unbind_all(dev, drm); out_bind: - flush_workqueue(priv->wq); - destroy_workqueue(priv->wq); -out_wq: kfree(priv); out_unref: drm_dev_unref(drm); @@ -624,9 +615,6 @@ static void etnaviv_unbind(struct device *dev) drm_dev_unregister(drm); - flush_workqueue(priv->wq); - destroy_workqueue(priv->wq); - component_unbind_all(dev, drm); drm->dev_private = NULL; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h index d249acb6da082..8668bfd4abd51 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h @@ -56,18 +56,8 @@ struct etnaviv_drm_private { /* list of GEM objects: */ struct mutex gem_lock; struct list_head gem_list; - - struct workqueue_struct *wq; }; -static inline void etnaviv_queue_work(struct drm_device *dev, - struct work_struct *w) -{ - struct etnaviv_drm_private *priv = dev->dev_private; - - queue_work(priv->wq, w); -} - int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, struct drm_file *file); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 6ce4b9d236b4d..6176704bdae32 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -958,7 +958,7 @@ static void recover_worker(struct work_struct *work) pm_runtime_put_autosuspend(gpu->dev); /* Retire the buffer objects in a work */ - etnaviv_queue_work(gpu->drm, &gpu->retire_work); + queue_work(gpu->wq, &gpu->retire_work); } static void hangcheck_timer_reset(struct etnaviv_gpu *gpu) @@ -994,7 +994,7 @@ static void hangcheck_handler(struct timer_list *t) dev_err(gpu->dev, " completed fence: %u\n", fence); dev_err(gpu->dev, " active fence: %u\n", gpu->active_fence); - etnaviv_queue_work(gpu->drm, &gpu->recover_work); + queue_work(gpu->wq, &gpu->recover_work); } /* if still more pending work, reset the hangcheck timer: */ @@ -1526,7 +1526,7 @@ static irqreturn_t irq_handler(int irq, void *data) if (gpu->event[event].sync_point) { gpu->sync_point_event = event; - etnaviv_queue_work(gpu->drm, &gpu->sync_point_work); + queue_work(gpu->wq, &gpu->sync_point_work); } fence = gpu->event[event].fence; @@ -1552,7 +1552,7 @@ static irqreturn_t irq_handler(int irq, void *data) } /* Retire the buffer objects in a work */ - etnaviv_queue_work(gpu->drm, &gpu->retire_work); + queue_work(gpu->wq, &gpu->retire_work); ret = IRQ_HANDLED; } @@ -1721,12 +1721,20 @@ static int etnaviv_gpu_bind(struct device *dev, struct device *master, return PTR_ERR(gpu->cooling); } + gpu->wq = alloc_ordered_workqueue(dev_name(dev), 0); + if (!gpu->wq) { + if (IS_ENABLED(CONFIG_DRM_ETNAVIV_THERMAL)) + thermal_cooling_device_unregister(gpu->cooling); + return -ENOMEM; + } + #ifdef CONFIG_PM ret = pm_runtime_get_sync(gpu->dev); #else ret = etnaviv_gpu_clk_enable(gpu); #endif if (ret < 0) { + destroy_workqueue(gpu->wq); if (IS_ENABLED(CONFIG_DRM_ETNAVIV_THERMAL)) thermal_cooling_device_unregister(gpu->cooling); return ret; @@ -1761,6 +1769,9 @@ static void etnaviv_gpu_unbind(struct device *dev, struct device *master, hangcheck_disable(gpu); + flush_workqueue(gpu->wq); + destroy_workqueue(gpu->wq); + #ifdef CONFIG_PM pm_runtime_get_sync(gpu->dev); pm_runtime_put_sync_suspend(gpu->dev); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h index 15090bb68f5a1..ccef6139cf707 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h @@ -106,6 +106,7 @@ struct etnaviv_gpu { struct mutex lock; struct etnaviv_chip_identity identity; struct etnaviv_file_private *lastctx; + struct workqueue_struct *wq; /* 'ring'-buffer: */ struct etnaviv_cmdbuf *buffer; From 40c27bdeb00ba234d5062b7c51467a9a51c4606c Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 17 Nov 2017 17:59:26 +0100 Subject: [PATCH 15/32] drm/etnaviv: hold GPU lock while inserting END command Inserting the END command when suspending the GPU is changing the command buffer state, which requires the GPU to be held. Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel Reviewed-by: Christian Gmeiner --- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 6176704bdae32..c4f518d56ead7 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1631,7 +1631,9 @@ static int etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu) { if (gpu->buffer) { /* Replace the last WAIT with END */ + mutex_lock(&gpu->lock); etnaviv_buffer_end(gpu); + mutex_unlock(&gpu->lock); /* * We know that only the FE is busy here, this should From b6d6223f5029d3c4fe466ce1711fe4cb23d06013 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 17 Nov 2017 17:51:19 +0100 Subject: [PATCH 16/32] drm/etnaviv: add lockdep annotations to buffer manipulation functions When manipulating the kernel command buffer the GPU mutex must be held, as otherwise different callers might try to replace the same part of the buffer, wreacking havok in the GPU execution. Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel --- drivers/gpu/drm/etnaviv/etnaviv_buffer.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c index 6ad8972a59cc3..b0e046d8ad2dd 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c @@ -100,6 +100,8 @@ static void etnaviv_cmd_select_pipe(struct etnaviv_gpu *gpu, { u32 flush = 0; + lockdep_assert_held(&gpu->lock); + /* * This assumes that if we're switching to 2D, we're switching * away from 3D, and vice versa. Hence, if we're switching to @@ -166,6 +168,8 @@ u16 etnaviv_buffer_init(struct etnaviv_gpu *gpu) { struct etnaviv_cmdbuf *buffer = gpu->buffer; + lockdep_assert_held(&gpu->lock); + /* initialize buffer */ buffer->user_size = 0; @@ -180,6 +184,8 @@ u16 etnaviv_buffer_config_mmuv2(struct etnaviv_gpu *gpu, u32 mtlb_addr, u32 safe { struct etnaviv_cmdbuf *buffer = gpu->buffer; + lockdep_assert_held(&gpu->lock); + buffer->user_size = 0; if (gpu->identity.features & chipFeatures_PIPE_3D) { @@ -215,6 +221,8 @@ void etnaviv_buffer_end(struct etnaviv_gpu *gpu) unsigned int waitlink_offset = buffer->user_size - 16; u32 link_target, flush = 0; + lockdep_assert_held(&gpu->lock); + if (gpu->exec_state == ETNA_PIPE_2D) flush = VIVS_GL_FLUSH_CACHE_PE2D; else if (gpu->exec_state == ETNA_PIPE_3D) @@ -257,6 +265,8 @@ void etnaviv_sync_point_queue(struct etnaviv_gpu *gpu, unsigned int event) unsigned int waitlink_offset = buffer->user_size - 16; u32 dwords, target; + lockdep_assert_held(&gpu->lock); + /* * We need at most 3 dwords in the return target: * 1 event + 1 end + 1 wait + 1 link. @@ -296,6 +306,8 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, unsigned int event, u32 link_target, link_dwords; bool switch_context = gpu->exec_state != cmdbuf->exec_state; + lockdep_assert_held(&gpu->lock); + if (drm_debug & DRM_UT_DRIVER) etnaviv_buffer_dump(gpu, buffer, 0, 0x50); From c52837238038f61b6511eadee94fcab622d635f0 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 24 Nov 2017 10:43:07 +0100 Subject: [PATCH 17/32] drm/etnaviv: simplify submit_create Use kzalloc so other code doesn't need to worry about uninitialized members. Drop the non-standard GFP flags, as we really don't want to fail the submit when under slight memory pressure. Remove one level of indentation by using an early return if the allocation failed. Also remove the unused drm device member. Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel --- drivers/gpu/drm/etnaviv/etnaviv_gem.h | 1 - drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 14 +++++--------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h index d1a7d040ac97f..dc478f014d29f 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h @@ -101,7 +101,6 @@ struct etnaviv_gem_submit_bo { * lasts for the duration of the submit-ioctl. */ struct etnaviv_gem_submit { - struct drm_device *dev; struct etnaviv_gpu *gpu; struct ww_acquire_ctx ticket; struct dma_fence *fence; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 8fa31ab1fb0a4..51ed34586c108 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -38,17 +38,13 @@ static struct etnaviv_gem_submit *submit_create(struct drm_device *dev, struct etnaviv_gem_submit *submit; size_t sz = size_vstruct(nr, sizeof(submit->bos[0]), sizeof(*submit)); - submit = kmalloc(sz, GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY); - if (submit) { - submit->dev = dev; - submit->gpu = gpu; + submit = kzalloc(sz, GFP_KERNEL); + if (!submit) + return NULL; - /* initially, until copy_from_user() and bo lookup succeeds: */ - submit->nr_bos = 0; - submit->fence = NULL; + submit->gpu = gpu; - ww_acquire_init(&submit->ticket, &reservation_ww_class); - } + ww_acquire_init(&submit->ticket, &reservation_ww_class); return submit; } From 0236efe97e165a1eefef9c38c8b3d10133a3e627 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Thu, 23 Nov 2017 17:49:59 +0100 Subject: [PATCH 18/32] drm/etnaviv: move object fence attachment to gem_submit path The object fencing has nothing to do with the actual GPU buffer submit, so move it to the gem submit path to have a cleaner split. Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel --- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 21 ++++++++++++++++++++ drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 7 ------- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 51ed34586c108..72468f11dd160 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -180,6 +180,24 @@ static int submit_fence_sync(const struct etnaviv_gem_submit *submit) return ret; } +static void submit_attach_object_fences(struct etnaviv_gem_submit *submit) +{ + int i; + + for (i = 0; i < submit->nr_bos; i++) { + struct etnaviv_gem_object *etnaviv_obj = submit->bos[i].obj; + + if (submit->bos[i].flags & ETNA_SUBMIT_BO_WRITE) + reservation_object_add_excl_fence(etnaviv_obj->resv, + submit->fence); + else + reservation_object_add_shared_fence(etnaviv_obj->resv, + submit->fence); + + submit_unlock_object(submit, i); + } +} + static void submit_unpin_objects(struct etnaviv_gem_submit *submit) { int i; @@ -335,6 +353,7 @@ static void submit_cleanup(struct etnaviv_gem_submit *submit) for (i = 0; i < submit->nr_bos; i++) { struct etnaviv_gem_object *etnaviv_obj = submit->bos[i].obj; + /* if the GPU submit failed, objects might still be locked */ submit_unlock_object(submit, i); drm_gem_object_put_unlocked(&etnaviv_obj->base); } @@ -507,6 +526,8 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, if (ret) goto out; + submit_attach_object_fences(submit); + cmdbuf = NULL; if (args->flags & ETNA_SUBMIT_FENCE_FD_OUT) { diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index c4f518d56ead7..93d71ad2d6815 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1443,13 +1443,6 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, etnaviv_gem_mapping_reference(submit->bos[i].mapping); cmdbuf->bo_map[i] = submit->bos[i].mapping; atomic_inc(&etnaviv_obj->gpu_active); - - if (submit->bos[i].flags & ETNA_SUBMIT_BO_WRITE) - reservation_object_add_excl_fence(etnaviv_obj->resv, - fence); - else - reservation_object_add_shared_fence(etnaviv_obj->resv, - fence); } cmdbuf->nr_bos = submit->nr_bos; hangcheck_timer_reset(gpu); From 10009ea2e4cf86c678605cf154e09acb483f2be1 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Thu, 23 Nov 2017 17:57:12 +0100 Subject: [PATCH 19/32] drm/etnaviv: rename submit fence to out_fence This is the fence passed out on a sucessful GPU submit. Make the name more clear. Signed-off-by: Lucas Stach --- drivers/gpu/drm/etnaviv/etnaviv_gem.h | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 12 ++++++------ drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h index dc478f014d29f..848daf719cda6 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h @@ -103,7 +103,7 @@ struct etnaviv_gem_submit_bo { struct etnaviv_gem_submit { struct etnaviv_gpu *gpu; struct ww_acquire_ctx ticket; - struct dma_fence *fence; + struct dma_fence *out_fence; u32 flags; unsigned int nr_bos; struct etnaviv_gem_submit_bo bos[0]; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 72468f11dd160..930577c8794e1 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -189,10 +189,10 @@ static void submit_attach_object_fences(struct etnaviv_gem_submit *submit) if (submit->bos[i].flags & ETNA_SUBMIT_BO_WRITE) reservation_object_add_excl_fence(etnaviv_obj->resv, - submit->fence); + submit->out_fence); else reservation_object_add_shared_fence(etnaviv_obj->resv, - submit->fence); + submit->out_fence); submit_unlock_object(submit, i); } @@ -359,8 +359,8 @@ static void submit_cleanup(struct etnaviv_gem_submit *submit) } ww_acquire_fini(&submit->ticket); - if (submit->fence) - dma_fence_put(submit->fence); + if (submit->out_fence) + dma_fence_put(submit->out_fence); kfree(submit); } @@ -537,7 +537,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, * fence to the sync file here, eliminating the ENOMEM * possibility at this stage. */ - sync_file = sync_file_create(submit->fence); + sync_file = sync_file_create(submit->out_fence); if (!sync_file) { ret = -ENOMEM; goto out; @@ -546,7 +546,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, } args->fence_fd = out_fence_fd; - args->fence = submit->fence->seqno; + args->fence = submit->out_fence->seqno; out: submit_unpin_objects(submit); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 93d71ad2d6815..0d0c9e3964cb2 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1413,8 +1413,8 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, } gpu->event[event[0]].fence = fence; - submit->fence = dma_fence_get(fence); - gpu->active_fence = submit->fence->seqno; + submit->out_fence = dma_fence_get(fence); + gpu->active_fence = submit->out_fence->seqno; if (cmdbuf->nr_pmrs) { gpu->event[event[1]].sync_point = &sync_point_perfmon_sample_pre; From 9efabd7392f0b42bd872a0e650759e9f1ef43db2 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Thu, 23 Nov 2017 18:02:43 +0100 Subject: [PATCH 20/32] drm/etnaviv: attach in fence to submit and move fence wait to fence_sync Simplifies the cleanup path and moves fence waiting to a central location. Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel --- drivers/gpu/drm/etnaviv/etnaviv_gem.h | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 28 +++++++++----------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h index 848daf719cda6..21cb3460046fa 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h @@ -103,7 +103,7 @@ struct etnaviv_gem_submit_bo { struct etnaviv_gem_submit { struct etnaviv_gpu *gpu; struct ww_acquire_ctx ticket; - struct dma_fence *out_fence; + struct dma_fence *out_fence, *in_fence; u32 flags; unsigned int nr_bos; struct etnaviv_gem_submit_bo bos[0]; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 930577c8794e1..20906c22998c7 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -177,6 +177,15 @@ static int submit_fence_sync(const struct etnaviv_gem_submit *submit) break; } + if (submit->flags & ETNA_SUBMIT_FENCE_FD_IN) { + /* + * Wait if the fence is from a foreign context, or if the fence + * array contains any fence from a foreign context. + */ + if (!dma_fence_match_context(submit->in_fence, context)) + ret = dma_fence_wait(submit->in_fence, true); + } + return ret; } @@ -359,6 +368,8 @@ static void submit_cleanup(struct etnaviv_gem_submit *submit) } ww_acquire_fini(&submit->ticket); + if (submit->in_fence) + dma_fence_put(submit->in_fence); if (submit->out_fence) dma_fence_put(submit->out_fence); kfree(submit); @@ -375,7 +386,6 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, struct etnaviv_gem_submit *submit; struct etnaviv_cmdbuf *cmdbuf; struct etnaviv_gpu *gpu; - struct dma_fence *in_fence = NULL; struct sync_file *sync_file = NULL; int out_fence_fd = -1; void *stream; @@ -485,21 +495,11 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, } if (args->flags & ETNA_SUBMIT_FENCE_FD_IN) { - in_fence = sync_file_get_fence(args->fence_fd); - if (!in_fence) { + submit->in_fence = sync_file_get_fence(args->fence_fd); + if (!submit->in_fence) { ret = -EINVAL; goto err_submit_objects; } - - /* - * Wait if the fence is from a foreign context, or if the fence - * array contains any fence from a foreign context. - */ - if (!dma_fence_match_context(in_fence, gpu->fence_context)) { - ret = dma_fence_wait(in_fence, true); - if (ret) - goto err_submit_objects; - } } ret = submit_fence_sync(submit); @@ -552,8 +552,6 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, submit_unpin_objects(submit); err_submit_objects: - if (in_fence) - dma_fence_put(in_fence); submit_cleanup(submit); err_submit_cmds: From 33a63e68f62bef6981fae781090d480080a73570 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Thu, 23 Nov 2017 18:13:40 +0100 Subject: [PATCH 21/32] drm/etnaviv: move object unpinning to submit cleanup This is safe to call in all paths, as the BO_PINNED flag tells us if the BO needs unpinning. Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel --- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 33 +++++++------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 20906c22998c7..9b5541207d339 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -207,19 +207,6 @@ static void submit_attach_object_fences(struct etnaviv_gem_submit *submit) } } -static void submit_unpin_objects(struct etnaviv_gem_submit *submit) -{ - int i; - - for (i = 0; i < submit->nr_bos; i++) { - if (submit->bos[i].flags & BO_PINNED) - etnaviv_gem_mapping_unreference(submit->bos[i].mapping); - - submit->bos[i].mapping = NULL; - submit->bos[i].flags &= ~BO_PINNED; - } -} - static int submit_pin_objects(struct etnaviv_gem_submit *submit) { int i, ret = 0; @@ -362,6 +349,13 @@ static void submit_cleanup(struct etnaviv_gem_submit *submit) for (i = 0; i < submit->nr_bos; i++) { struct etnaviv_gem_object *etnaviv_obj = submit->bos[i].obj; + /* unpin all objects */ + if (submit->bos[i].flags & BO_PINNED) { + etnaviv_gem_mapping_unreference(submit->bos[i].mapping); + submit->bos[i].mapping = NULL; + submit->bos[i].flags &= ~BO_PINNED; + } + /* if the GPU submit failed, objects might still be locked */ submit_unlock_object(submit, i); drm_gem_object_put_unlocked(&etnaviv_obj->base); @@ -508,23 +502,23 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, ret = submit_pin_objects(submit); if (ret) - goto out; + goto err_submit_objects; ret = submit_reloc(submit, stream, args->stream_size / 4, relocs, args->nr_relocs); if (ret) - goto out; + goto err_submit_objects; ret = submit_perfmon_validate(submit, cmdbuf, pmrs, args->nr_pmrs); if (ret) - goto out; + goto err_submit_objects; memcpy(cmdbuf->vaddr, stream, args->stream_size); cmdbuf->user_size = ALIGN(args->stream_size, 8); ret = etnaviv_gpu_submit(gpu, submit, cmdbuf); if (ret) - goto out; + goto err_submit_objects; submit_attach_object_fences(submit); @@ -540,7 +534,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, sync_file = sync_file_create(submit->out_fence); if (!sync_file) { ret = -ENOMEM; - goto out; + goto err_submit_objects; } fd_install(out_fence_fd, sync_file->file); } @@ -548,9 +542,6 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, args->fence_fd = out_fence_fd; args->fence = submit->out_fence->seqno; -out: - submit_unpin_objects(submit); - err_submit_objects: submit_cleanup(submit); From 08301d73f27d59e2ac45411ed7bb2235d68190db Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 24 Nov 2017 14:34:46 +0100 Subject: [PATCH 22/32] drm/etnaviv: move ww_acquire_ctx out of submit object The acquire_ctx is special in that it needs to be released from the same thread as has been used to initialize it. This collides with the intention to extend the submit lifetime beyond the gem_submit function with potentially other threads doing the final cleanup. Move the ww_acquire_ctx to the function local stack as suggested in the documentation. Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel --- drivers/gpu/drm/etnaviv/etnaviv_gem.h | 1 - drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 22 ++++++++++++-------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h index 21cb3460046fa..6b78d059ed2d4 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h @@ -102,7 +102,6 @@ struct etnaviv_gem_submit_bo { */ struct etnaviv_gem_submit { struct etnaviv_gpu *gpu; - struct ww_acquire_ctx ticket; struct dma_fence *out_fence, *in_fence; u32 flags; unsigned int nr_bos; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 9b5541207d339..3090a46979aff 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -44,8 +44,6 @@ static struct etnaviv_gem_submit *submit_create(struct drm_device *dev, submit->gpu = gpu; - ww_acquire_init(&submit->ticket, &reservation_ww_class); - return submit; } @@ -107,7 +105,8 @@ static void submit_unlock_object(struct etnaviv_gem_submit *submit, int i) } } -static int submit_lock_objects(struct etnaviv_gem_submit *submit) +static int submit_lock_objects(struct etnaviv_gem_submit *submit, + struct ww_acquire_ctx *ticket) { int contended, slow_locked = -1, i, ret = 0; @@ -122,7 +121,7 @@ static int submit_lock_objects(struct etnaviv_gem_submit *submit) if (!(submit->bos[i].flags & BO_LOCKED)) { ret = ww_mutex_lock_interruptible(&etnaviv_obj->resv->lock, - &submit->ticket); + ticket); if (ret == -EALREADY) DRM_ERROR("BO at index %u already on submit list\n", i); @@ -132,7 +131,7 @@ static int submit_lock_objects(struct etnaviv_gem_submit *submit) } } - ww_acquire_done(&submit->ticket); + ww_acquire_done(ticket); return 0; @@ -150,7 +149,7 @@ static int submit_lock_objects(struct etnaviv_gem_submit *submit) /* we lost out in a seqno race, lock and retry.. */ ret = ww_mutex_lock_slow_interruptible(&etnaviv_obj->resv->lock, - &submit->ticket); + ticket); if (!ret) { submit->bos[contended].flags |= BO_LOCKED; slow_locked = contended; @@ -361,7 +360,6 @@ static void submit_cleanup(struct etnaviv_gem_submit *submit) drm_gem_object_put_unlocked(&etnaviv_obj->base); } - ww_acquire_fini(&submit->ticket); if (submit->in_fence) dma_fence_put(submit->in_fence); if (submit->out_fence) @@ -381,6 +379,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, struct etnaviv_cmdbuf *cmdbuf; struct etnaviv_gpu *gpu; struct sync_file *sync_file = NULL; + struct ww_acquire_ctx ticket; int out_fence_fd = -1; void *stream; int ret; @@ -466,10 +465,12 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, } } + ww_acquire_init(&ticket, &reservation_ww_class); + submit = submit_create(dev, gpu, args->nr_bos); if (!submit) { ret = -ENOMEM; - goto err_submit_cmds; + goto err_submit_ww_acquire; } submit->flags = args->flags; @@ -478,7 +479,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, if (ret) goto err_submit_objects; - ret = submit_lock_objects(submit); + ret = submit_lock_objects(submit, &ticket); if (ret) goto err_submit_objects; @@ -545,6 +546,9 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, err_submit_objects: submit_cleanup(submit); +err_submit_ww_acquire: + ww_acquire_fini(&ticket); + err_submit_cmds: if (ret && (out_fence_fd >= 0)) put_unused_fd(out_fence_fd); From e0329e6cfa6274ceb54d175b5e6ac19c00024c33 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 24 Nov 2017 11:36:03 +0100 Subject: [PATCH 23/32] drm/etnaviv: refcount the submit object The submit object lifetime will get extended to the actual GPU execution. As multiple users will depend on this, add a kref to properly control destruction of the object. Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel --- drivers/gpu/drm/etnaviv/etnaviv_gem.h | 3 +++ drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 12 ++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h index 6b78d059ed2d4..4238f8d8541d9 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h @@ -101,6 +101,7 @@ struct etnaviv_gem_submit_bo { * lasts for the duration of the submit-ioctl. */ struct etnaviv_gem_submit { + struct kref refcount; struct etnaviv_gpu *gpu; struct dma_fence *out_fence, *in_fence; u32 flags; @@ -109,6 +110,8 @@ struct etnaviv_gem_submit { /* No new members here, the previous one is variable-length! */ }; +void etnaviv_submit_put(struct etnaviv_gem_submit * submit); + int etnaviv_gem_wait_bo(struct etnaviv_gpu *gpu, struct drm_gem_object *obj, struct timespec *timeout); int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags, diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 3090a46979aff..9b4436c380ea4 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -43,6 +43,7 @@ static struct etnaviv_gem_submit *submit_create(struct drm_device *dev, return NULL; submit->gpu = gpu; + kref_init(&submit->refcount); return submit; } @@ -341,8 +342,10 @@ static int submit_perfmon_validate(struct etnaviv_gem_submit *submit, return 0; } -static void submit_cleanup(struct etnaviv_gem_submit *submit) +static void submit_cleanup(struct kref *kref) { + struct etnaviv_gem_submit *submit = + container_of(kref, struct etnaviv_gem_submit, refcount); unsigned i; for (i = 0; i < submit->nr_bos; i++) { @@ -367,6 +370,11 @@ static void submit_cleanup(struct etnaviv_gem_submit *submit) kfree(submit); } +void etnaviv_submit_put(struct etnaviv_gem_submit *submit) +{ + kref_put(&submit->refcount, submit_cleanup); +} + int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, struct drm_file *file) { @@ -544,7 +552,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, args->fence = submit->out_fence->seqno; err_submit_objects: - submit_cleanup(submit); + etnaviv_submit_put(submit); err_submit_ww_acquire: ww_acquire_fini(&ticket); From ef146c00e2c29c5f926c6a7e9dc354c0cbeb2818 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 24 Nov 2017 12:02:38 +0100 Subject: [PATCH 24/32] drm/etnaviv: move PMRs to submit object To make them available to the event worker even after the actual command stream execution has finished. Signed-off-by: Lucas Stach --- drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c | 14 +------ drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h | 5 +-- drivers/gpu/drm/etnaviv/etnaviv_gem.h | 5 ++- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 40 +++++++++++--------- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 27 +++++++------ drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 2 +- 6 files changed, 44 insertions(+), 49 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c index 66ac79558bbd4..4ce4639d028d0 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c @@ -88,10 +88,9 @@ void etnaviv_cmdbuf_suballoc_destroy(struct etnaviv_cmdbuf_suballoc *suballoc) struct etnaviv_cmdbuf * etnaviv_cmdbuf_new(struct etnaviv_cmdbuf_suballoc *suballoc, u32 size, - size_t nr_bos, size_t nr_pmrs) + size_t nr_bos) { struct etnaviv_cmdbuf *cmdbuf; - struct etnaviv_perfmon_request *pmrs; size_t sz = size_vstruct(nr_bos, sizeof(cmdbuf->bo_map[0]), sizeof(*cmdbuf)); int granule_offs, order, ret; @@ -100,12 +99,6 @@ etnaviv_cmdbuf_new(struct etnaviv_cmdbuf_suballoc *suballoc, u32 size, if (!cmdbuf) return NULL; - sz = sizeof(*pmrs) * nr_pmrs; - pmrs = kzalloc(sz, GFP_KERNEL); - if (!pmrs) - goto out_free_cmdbuf; - - cmdbuf->pmrs = pmrs; cmdbuf->suballoc = suballoc; cmdbuf->size = size; @@ -132,10 +125,6 @@ etnaviv_cmdbuf_new(struct etnaviv_cmdbuf_suballoc *suballoc, u32 size, cmdbuf->vaddr = suballoc->vaddr + cmdbuf->suballoc_offset; return cmdbuf; - -out_free_cmdbuf: - kfree(cmdbuf); - return NULL; } void etnaviv_cmdbuf_free(struct etnaviv_cmdbuf *cmdbuf) @@ -151,7 +140,6 @@ void etnaviv_cmdbuf_free(struct etnaviv_cmdbuf *cmdbuf) suballoc->free_space = 1; mutex_unlock(&suballoc->lock); wake_up_all(&suballoc->free_event); - kfree(cmdbuf->pmrs); kfree(cmdbuf); } diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h index b6348b9f2a9dd..930b9c7f22496 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h @@ -39,9 +39,6 @@ struct etnaviv_cmdbuf { u32 exec_state; /* per GPU in-flight list */ struct list_head node; - /* perfmon requests */ - unsigned int nr_pmrs; - struct etnaviv_perfmon_request *pmrs; /* BOs attached to this command buffer */ unsigned int nr_bos; struct etnaviv_vram_mapping *bo_map[0]; @@ -53,7 +50,7 @@ void etnaviv_cmdbuf_suballoc_destroy(struct etnaviv_cmdbuf_suballoc *suballoc); struct etnaviv_cmdbuf * etnaviv_cmdbuf_new(struct etnaviv_cmdbuf_suballoc *suballoc, u32 size, - size_t nr_bos, size_t nr_pmrs); + size_t nr_bos); void etnaviv_cmdbuf_free(struct etnaviv_cmdbuf *cmdbuf); u32 etnaviv_cmdbuf_get_va(struct etnaviv_cmdbuf *buf); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h index 4238f8d8541d9..f525fd1ef63c3 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h @@ -97,14 +97,15 @@ struct etnaviv_gem_submit_bo { /* Created per submit-ioctl, to track bo's and cmdstream bufs, etc, * associated with the cmdstream submission for synchronization (and - * make it easier to unwind when things go wrong, etc). This only - * lasts for the duration of the submit-ioctl. + * make it easier to unwind when things go wrong, etc). */ struct etnaviv_gem_submit { struct kref refcount; struct etnaviv_gpu *gpu; struct dma_fence *out_fence, *in_fence; u32 flags; + unsigned int nr_pmrs; + struct etnaviv_perfmon_request *pmrs; unsigned int nr_bos; struct etnaviv_gem_submit_bo bos[0]; /* No new members here, the previous one is variable-length! */ diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 9b4436c380ea4..c8c576993c627 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -33,15 +33,23 @@ #define BO_PINNED 0x2000 static struct etnaviv_gem_submit *submit_create(struct drm_device *dev, - struct etnaviv_gpu *gpu, size_t nr) + struct etnaviv_gpu *gpu, size_t nr_bos, size_t nr_pmrs) { struct etnaviv_gem_submit *submit; - size_t sz = size_vstruct(nr, sizeof(submit->bos[0]), sizeof(*submit)); + size_t sz = size_vstruct(nr_bos, sizeof(submit->bos[0]), sizeof(*submit)); submit = kzalloc(sz, GFP_KERNEL); if (!submit) return NULL; + submit->pmrs = kcalloc(nr_pmrs, sizeof(struct etnaviv_perfmon_request), + GFP_KERNEL); + if (!submit->pmrs) { + kfree(submit); + return NULL; + } + submit->nr_pmrs = nr_pmrs; + submit->gpu = gpu; kref_init(&submit->refcount); @@ -295,13 +303,11 @@ static int submit_reloc(struct etnaviv_gem_submit *submit, void *stream, } static int submit_perfmon_validate(struct etnaviv_gem_submit *submit, - struct etnaviv_cmdbuf *cmdbuf, - const struct drm_etnaviv_gem_submit_pmr *pmrs, - u32 nr_pms) + u32 exec_state, const struct drm_etnaviv_gem_submit_pmr *pmrs) { u32 i; - for (i = 0; i < nr_pms; i++) { + for (i = 0; i < submit->nr_pmrs; i++) { const struct drm_etnaviv_gem_submit_pmr *r = pmrs + i; struct etnaviv_gem_submit_bo *bo; int ret; @@ -326,17 +332,17 @@ static int submit_perfmon_validate(struct etnaviv_gem_submit *submit, return -EINVAL; } - if (etnaviv_pm_req_validate(r, cmdbuf->exec_state)) { + if (etnaviv_pm_req_validate(r, exec_state)) { DRM_ERROR("perfmon request: domain or signal not valid"); return -EINVAL; } - cmdbuf->pmrs[i].flags = r->flags; - cmdbuf->pmrs[i].domain = r->domain; - cmdbuf->pmrs[i].signal = r->signal; - cmdbuf->pmrs[i].sequence = r->sequence; - cmdbuf->pmrs[i].offset = r->read_offset; - cmdbuf->pmrs[i].bo_vma = etnaviv_gem_vmap(&bo->obj->base); + submit->pmrs[i].flags = r->flags; + submit->pmrs[i].domain = r->domain; + submit->pmrs[i].signal = r->signal; + submit->pmrs[i].sequence = r->sequence; + submit->pmrs[i].offset = r->read_offset; + submit->pmrs[i].bo_vma = etnaviv_gem_vmap(&bo->obj->base); } return 0; @@ -367,6 +373,7 @@ static void submit_cleanup(struct kref *kref) dma_fence_put(submit->in_fence); if (submit->out_fence) dma_fence_put(submit->out_fence); + kfree(submit->pmrs); kfree(submit); } @@ -427,7 +434,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, stream = kvmalloc_array(1, args->stream_size, GFP_KERNEL); cmdbuf = etnaviv_cmdbuf_new(gpu->cmdbuf_suballoc, ALIGN(args->stream_size, 8) + 8, - args->nr_bos, args->nr_pmrs); + args->nr_bos); if (!bos || !relocs || !pmrs || !stream || !cmdbuf) { ret = -ENOMEM; goto err_submit_cmds; @@ -456,7 +463,6 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, ret = -EFAULT; goto err_submit_cmds; } - cmdbuf->nr_pmrs = args->nr_pmrs; ret = copy_from_user(stream, u64_to_user_ptr(args->stream), args->stream_size); @@ -475,7 +481,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, ww_acquire_init(&ticket, &reservation_ww_class); - submit = submit_create(dev, gpu, args->nr_bos); + submit = submit_create(dev, gpu, args->nr_bos, args->nr_pmrs); if (!submit) { ret = -ENOMEM; goto err_submit_ww_acquire; @@ -518,7 +524,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, if (ret) goto err_submit_objects; - ret = submit_perfmon_validate(submit, cmdbuf, pmrs, args->nr_pmrs); + ret = submit_perfmon_validate(submit, args->exec_state, pmrs); if (ret) goto err_submit_objects; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 0d0c9e3964cb2..ed3f8c6b70352 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -717,7 +717,7 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu) } /* Create buffer: */ - gpu->buffer = etnaviv_cmdbuf_new(gpu->cmdbuf_suballoc, PAGE_SIZE, 0, 0); + gpu->buffer = etnaviv_cmdbuf_new(gpu->cmdbuf_suballoc, PAGE_SIZE, 0); if (!gpu->buffer) { ret = -ENOMEM; dev_err(gpu->dev, "could not create command buffer\n"); @@ -1317,11 +1317,11 @@ void etnaviv_gpu_pm_put(struct etnaviv_gpu *gpu) static void sync_point_perfmon_sample(struct etnaviv_gpu *gpu, struct etnaviv_event *event, unsigned int flags) { - const struct etnaviv_cmdbuf *cmdbuf = event->cmdbuf; + const struct etnaviv_gem_submit *submit = event->submit; unsigned int i; - for (i = 0; i < cmdbuf->nr_pmrs; i++) { - const struct etnaviv_perfmon_request *pmr = cmdbuf->pmrs + i; + for (i = 0; i < submit->nr_pmrs; i++) { + const struct etnaviv_perfmon_request *pmr = submit->pmrs + i; if (pmr->flags == flags) etnaviv_perfmon_process(gpu, pmr); @@ -1349,14 +1349,14 @@ static void sync_point_perfmon_sample_pre(struct etnaviv_gpu *gpu, static void sync_point_perfmon_sample_post(struct etnaviv_gpu *gpu, struct etnaviv_event *event) { - const struct etnaviv_cmdbuf *cmdbuf = event->cmdbuf; + const struct etnaviv_gem_submit *submit = event->submit; unsigned int i; u32 val; sync_point_perfmon_sample(gpu, event, ETNA_PM_PROCESS_POST); - for (i = 0; i < cmdbuf->nr_pmrs; i++) { - const struct etnaviv_perfmon_request *pmr = cmdbuf->pmrs + i; + for (i = 0; i < submit->nr_pmrs; i++) { + const struct etnaviv_perfmon_request *pmr = submit->pmrs + i; *pmr->bo_vma = pmr->sequence; } @@ -1392,7 +1392,7 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, * - a sync point to re-configure gpu, process ETNA_PM_PROCESS_POST requests * and update the sequence number for userspace. */ - if (cmdbuf->nr_pmrs) + if (submit->nr_pmrs) nr_events = 3; ret = event_alloc(gpu, nr_events, event); @@ -1416,17 +1416,19 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, submit->out_fence = dma_fence_get(fence); gpu->active_fence = submit->out_fence->seqno; - if (cmdbuf->nr_pmrs) { + if (submit->nr_pmrs) { gpu->event[event[1]].sync_point = &sync_point_perfmon_sample_pre; - gpu->event[event[1]].cmdbuf = cmdbuf; + kref_get(&submit->refcount); + gpu->event[event[1]].submit = submit; etnaviv_sync_point_queue(gpu, event[1]); } etnaviv_buffer_queue(gpu, event[0], cmdbuf); - if (cmdbuf->nr_pmrs) { + if (submit->nr_pmrs) { gpu->event[event[2]].sync_point = &sync_point_perfmon_sample_post; - gpu->event[event[2]].cmdbuf = cmdbuf; + kref_get(&submit->refcount); + gpu->event[event[2]].submit = submit; etnaviv_sync_point_queue(gpu, event[2]); } @@ -1465,6 +1467,7 @@ static void sync_point_worker(struct work_struct *work) u32 addr = gpu_read(gpu, VIVS_FE_DMA_ADDRESS); event->sync_point(gpu, event); + etnaviv_submit_put(event->submit); event_free(gpu, gpu->sync_point_event); /* restart FE last to avoid GPU and IRQ racing against this worker */ diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h index ccef6139cf707..eea823838b5fb 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h @@ -89,7 +89,7 @@ struct etnaviv_chip_identity { struct etnaviv_event { struct dma_fence *fence; - struct etnaviv_cmdbuf *cmdbuf; + struct etnaviv_gem_submit *submit; void (*sync_point)(struct etnaviv_gpu *gpu, struct etnaviv_event *event); }; From 797b0159e6b9124c24968c9b2b4631eba7c112b9 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 24 Nov 2017 15:16:58 +0100 Subject: [PATCH 25/32] drm/etnaviv: move exec_state to submit object We'll need this in some places where only the submit is available. Also this is a first step at slimming down the cmdbuf object. Signed-off-by: Lucas Stach --- drivers/gpu/drm/etnaviv/etnaviv_buffer.c | 10 +++++----- drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h | 2 -- drivers/gpu/drm/etnaviv/etnaviv_drv.h | 4 ++-- drivers/gpu/drm/etnaviv/etnaviv_gem.h | 1 + drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 2 +- 6 files changed, 10 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c index b0e046d8ad2dd..ccb9562a3d82a 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c @@ -297,14 +297,14 @@ void etnaviv_sync_point_queue(struct etnaviv_gpu *gpu, unsigned int event) } /* Append a command buffer to the ring buffer. */ -void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, unsigned int event, - struct etnaviv_cmdbuf *cmdbuf) +void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, u32 exec_state, + unsigned int event, struct etnaviv_cmdbuf *cmdbuf) { struct etnaviv_cmdbuf *buffer = gpu->buffer; unsigned int waitlink_offset = buffer->user_size - 16; u32 return_target, return_dwords; u32 link_target, link_dwords; - bool switch_context = gpu->exec_state != cmdbuf->exec_state; + bool switch_context = gpu->exec_state != exec_state; lockdep_assert_held(&gpu->lock); @@ -363,8 +363,8 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, unsigned int event, } if (switch_context) { - etnaviv_cmd_select_pipe(gpu, buffer, cmdbuf->exec_state); - gpu->exec_state = cmdbuf->exec_state; + etnaviv_cmd_select_pipe(gpu, buffer, exec_state); + gpu->exec_state = exec_state; } /* And the link to the submitted buffer */ diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h index 930b9c7f22496..26460e5d252eb 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h @@ -35,8 +35,6 @@ struct etnaviv_cmdbuf { u32 user_size; /* fence after which this buffer is to be disposed */ struct dma_fence *fence; - /* target exec state */ - u32 exec_state; /* per GPU in-flight list */ struct list_head node; /* BOs attached to this command buffer */ diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h index 8668bfd4abd51..a54f0b758a5c9 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h @@ -87,8 +87,8 @@ u16 etnaviv_buffer_init(struct etnaviv_gpu *gpu); u16 etnaviv_buffer_config_mmuv2(struct etnaviv_gpu *gpu, u32 mtlb_addr, u32 safe_addr); void etnaviv_buffer_end(struct etnaviv_gpu *gpu); void etnaviv_sync_point_queue(struct etnaviv_gpu *gpu, unsigned int event); -void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, unsigned int event, - struct etnaviv_cmdbuf *cmdbuf); +void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, u32 exec_state, + unsigned int event, struct etnaviv_cmdbuf *cmdbuf); void etnaviv_validate_init(void); bool etnaviv_cmd_validate_one(struct etnaviv_gpu *gpu, u32 *stream, unsigned int size, diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h index f525fd1ef63c3..63285101850c2 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h @@ -103,6 +103,7 @@ struct etnaviv_gem_submit { struct kref refcount; struct etnaviv_gpu *gpu; struct dma_fence *out_fence, *in_fence; + u32 exec_state; u32 flags; unsigned int nr_pmrs; struct etnaviv_perfmon_request *pmrs; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index c8c576993c627..d87a7f00bca33 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -440,7 +440,6 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, goto err_submit_cmds; } - cmdbuf->exec_state = args->exec_state; cmdbuf->ctx = file->driver_priv; ret = copy_from_user(bos, u64_to_user_ptr(args->bos), @@ -487,6 +486,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, goto err_submit_ww_acquire; } + submit->exec_state = args->exec_state; submit->flags = args->flags; ret = submit_lookup_objects(submit, file, bos, args->nr_bos); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index ed3f8c6b70352..675c31be86a75 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1423,7 +1423,7 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, etnaviv_sync_point_queue(gpu, event[1]); } - etnaviv_buffer_queue(gpu, event[0], cmdbuf); + etnaviv_buffer_queue(gpu, submit->exec_state, event[0], cmdbuf); if (submit->nr_pmrs) { gpu->event[event[2]].sync_point = &sync_point_perfmon_sample_post; From 7a9c0fe20e04f16db075a71065685628ec79e6c6 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 24 Nov 2017 15:19:16 +0100 Subject: [PATCH 26/32] drm/etnaviv: use submit exec_state for perfmon sampling The GPU exec state may have changed at the time when the perfmon sampling is done, as it reflects the state of the last submission, not the current GPU execution state. So for proper sampling we must use the submit exec_state. Signed-off-by: Lucas Stach --- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 4 ++-- drivers/gpu/drm/etnaviv/etnaviv_perfmon.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 675c31be86a75..afc4b6c5fbf56 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1324,7 +1324,7 @@ static void sync_point_perfmon_sample(struct etnaviv_gpu *gpu, const struct etnaviv_perfmon_request *pmr = submit->pmrs + i; if (pmr->flags == flags) - etnaviv_perfmon_process(gpu, pmr); + etnaviv_perfmon_process(gpu, pmr, submit->exec_state); } } diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c index 768f5aafdd186..26dddfc41aacb 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c @@ -479,9 +479,9 @@ int etnaviv_pm_req_validate(const struct drm_etnaviv_gem_submit_pmr *r, } void etnaviv_perfmon_process(struct etnaviv_gpu *gpu, - const struct etnaviv_perfmon_request *pmr) + const struct etnaviv_perfmon_request *pmr, u32 exec_state) { - const struct etnaviv_pm_domain_meta *meta = &doms_meta[gpu->exec_state]; + const struct etnaviv_pm_domain_meta *meta = &doms_meta[exec_state]; const struct etnaviv_pm_domain *dom; const struct etnaviv_pm_signal *sig; u32 *bo = pmr->bo_vma; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.h b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.h index 35dce194cb004..c1653c64ab6b1 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.h @@ -44,6 +44,6 @@ int etnaviv_pm_req_validate(const struct drm_etnaviv_gem_submit_pmr *r, u32 exec_state); void etnaviv_perfmon_process(struct etnaviv_gpu *gpu, - const struct etnaviv_perfmon_request *pmr); + const struct etnaviv_perfmon_request *pmr, u32 exec_state); #endif /* __ETNAVIV_PERFMON_H__ */ From 2f9225dbc09abe7cacb9820ebdeef5b6c0eb9c72 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 24 Nov 2017 16:56:37 +0100 Subject: [PATCH 27/32] drm/etnaviv: move cmdbuf into submit object Less dynamic allocations and slims down the cmdbuf object to only the required information, as everything else is already available in the submit object. This also simplifies buffer and mappings lifetime management, as they are now exlusively attached to the submit object and not additionally to the cmdbuf. Signed-off-by: Lucas Stach --- drivers/gpu/drm/etnaviv/etnaviv_buffer.c | 10 +-- drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c | 17 ++--- drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h | 13 +--- drivers/gpu/drm/etnaviv/etnaviv_drv.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_dump.c | 23 +++---- drivers/gpu/drm/etnaviv/etnaviv_gem.h | 3 + drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 28 ++++---- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 68 ++++++++------------ drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 7 +- drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c | 2 +- 10 files changed, 72 insertions(+), 101 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c index ccb9562a3d82a..99ad2f073c6e4 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c @@ -166,7 +166,7 @@ static u32 etnaviv_buffer_reserve(struct etnaviv_gpu *gpu, u16 etnaviv_buffer_init(struct etnaviv_gpu *gpu) { - struct etnaviv_cmdbuf *buffer = gpu->buffer; + struct etnaviv_cmdbuf *buffer = &gpu->buffer; lockdep_assert_held(&gpu->lock); @@ -182,7 +182,7 @@ u16 etnaviv_buffer_init(struct etnaviv_gpu *gpu) u16 etnaviv_buffer_config_mmuv2(struct etnaviv_gpu *gpu, u32 mtlb_addr, u32 safe_addr) { - struct etnaviv_cmdbuf *buffer = gpu->buffer; + struct etnaviv_cmdbuf *buffer = &gpu->buffer; lockdep_assert_held(&gpu->lock); @@ -217,7 +217,7 @@ u16 etnaviv_buffer_config_mmuv2(struct etnaviv_gpu *gpu, u32 mtlb_addr, u32 safe void etnaviv_buffer_end(struct etnaviv_gpu *gpu) { - struct etnaviv_cmdbuf *buffer = gpu->buffer; + struct etnaviv_cmdbuf *buffer = &gpu->buffer; unsigned int waitlink_offset = buffer->user_size - 16; u32 link_target, flush = 0; @@ -261,7 +261,7 @@ void etnaviv_buffer_end(struct etnaviv_gpu *gpu) /* Append a 'sync point' to the ring buffer. */ void etnaviv_sync_point_queue(struct etnaviv_gpu *gpu, unsigned int event) { - struct etnaviv_cmdbuf *buffer = gpu->buffer; + struct etnaviv_cmdbuf *buffer = &gpu->buffer; unsigned int waitlink_offset = buffer->user_size - 16; u32 dwords, target; @@ -300,7 +300,7 @@ void etnaviv_sync_point_queue(struct etnaviv_gpu *gpu, unsigned int event) void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, u32 exec_state, unsigned int event, struct etnaviv_cmdbuf *cmdbuf) { - struct etnaviv_cmdbuf *buffer = gpu->buffer; + struct etnaviv_cmdbuf *buffer = &gpu->buffer; unsigned int waitlink_offset = buffer->user_size - 16; u32 return_target, return_dwords; u32 link_target, link_dwords; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c index 4ce4639d028d0..3746827f45ebd 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c @@ -86,19 +86,11 @@ void etnaviv_cmdbuf_suballoc_destroy(struct etnaviv_cmdbuf_suballoc *suballoc) kfree(suballoc); } -struct etnaviv_cmdbuf * -etnaviv_cmdbuf_new(struct etnaviv_cmdbuf_suballoc *suballoc, u32 size, - size_t nr_bos) +int etnaviv_cmdbuf_init(struct etnaviv_cmdbuf_suballoc *suballoc, + struct etnaviv_cmdbuf *cmdbuf, u32 size) { - struct etnaviv_cmdbuf *cmdbuf; - size_t sz = size_vstruct(nr_bos, sizeof(cmdbuf->bo_map[0]), - sizeof(*cmdbuf)); int granule_offs, order, ret; - cmdbuf = kzalloc(sz, GFP_KERNEL); - if (!cmdbuf) - return NULL; - cmdbuf->suballoc = suballoc; cmdbuf->size = size; @@ -116,7 +108,7 @@ etnaviv_cmdbuf_new(struct etnaviv_cmdbuf_suballoc *suballoc, u32 size, if (!ret) { dev_err(suballoc->gpu->dev, "Timeout waiting for cmdbuf space\n"); - return NULL; + return -ETIMEDOUT; } goto retry; } @@ -124,7 +116,7 @@ etnaviv_cmdbuf_new(struct etnaviv_cmdbuf_suballoc *suballoc, u32 size, cmdbuf->suballoc_offset = granule_offs * SUBALLOC_GRANULE; cmdbuf->vaddr = suballoc->vaddr + cmdbuf->suballoc_offset; - return cmdbuf; + return 0; } void etnaviv_cmdbuf_free(struct etnaviv_cmdbuf *cmdbuf) @@ -140,7 +132,6 @@ void etnaviv_cmdbuf_free(struct etnaviv_cmdbuf *cmdbuf) suballoc->free_space = 1; mutex_unlock(&suballoc->lock); wake_up_all(&suballoc->free_event); - kfree(cmdbuf); } u32 etnaviv_cmdbuf_get_va(struct etnaviv_cmdbuf *buf) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h index 26460e5d252eb..ddc3f7ea169cd 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h @@ -33,22 +33,15 @@ struct etnaviv_cmdbuf { void *vaddr; u32 size; u32 user_size; - /* fence after which this buffer is to be disposed */ - struct dma_fence *fence; - /* per GPU in-flight list */ - struct list_head node; - /* BOs attached to this command buffer */ - unsigned int nr_bos; - struct etnaviv_vram_mapping *bo_map[0]; }; struct etnaviv_cmdbuf_suballoc * etnaviv_cmdbuf_suballoc_new(struct etnaviv_gpu * gpu); void etnaviv_cmdbuf_suballoc_destroy(struct etnaviv_cmdbuf_suballoc *suballoc); -struct etnaviv_cmdbuf * -etnaviv_cmdbuf_new(struct etnaviv_cmdbuf_suballoc *suballoc, u32 size, - size_t nr_bos); + +int etnaviv_cmdbuf_init(struct etnaviv_cmdbuf_suballoc *suballoc, + struct etnaviv_cmdbuf *cmdbuf, u32 size); void etnaviv_cmdbuf_free(struct etnaviv_cmdbuf *cmdbuf); u32 etnaviv_cmdbuf_get_va(struct etnaviv_cmdbuf *buf); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c index ca03b5e4789ba..f2948885859d7 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c @@ -172,7 +172,7 @@ static int etnaviv_mmu_show(struct etnaviv_gpu *gpu, struct seq_file *m) static void etnaviv_buffer_dump(struct etnaviv_gpu *gpu, struct seq_file *m) { - struct etnaviv_cmdbuf *buf = gpu->buffer; + struct etnaviv_cmdbuf *buf = &gpu->buffer; u32 size = buf->size; u32 *ptr = buf->vaddr; u32 i; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_dump.c b/drivers/gpu/drm/etnaviv/etnaviv_dump.c index 2d955d7d7b6d8..6d0909c589d10 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_dump.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_dump.c @@ -120,7 +120,7 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu) struct core_dump_iterator iter; struct etnaviv_vram_mapping *vram; struct etnaviv_gem_object *obj; - struct etnaviv_cmdbuf *cmd; + struct etnaviv_gem_submit *submit; unsigned int n_obj, n_bomap_pages; size_t file_size, mmu_size; __le64 *bomap, *bomap_start; @@ -132,11 +132,11 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu) n_bomap_pages = 0; file_size = ARRAY_SIZE(etnaviv_dump_registers) * sizeof(struct etnaviv_dump_registers) + - mmu_size + gpu->buffer->size; + mmu_size + gpu->buffer.size; /* Add in the active command buffers */ - list_for_each_entry(cmd, &gpu->active_cmd_list, node) { - file_size += cmd->size; + list_for_each_entry(submit, &gpu->active_submit_list, node) { + file_size += submit->cmdbuf.size; n_obj++; } @@ -176,13 +176,14 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu) etnaviv_core_dump_registers(&iter, gpu); etnaviv_core_dump_mmu(&iter, gpu, mmu_size); - etnaviv_core_dump_mem(&iter, ETDUMP_BUF_RING, gpu->buffer->vaddr, - gpu->buffer->size, - etnaviv_cmdbuf_get_va(gpu->buffer)); - - list_for_each_entry(cmd, &gpu->active_cmd_list, node) - etnaviv_core_dump_mem(&iter, ETDUMP_BUF_CMD, cmd->vaddr, - cmd->size, etnaviv_cmdbuf_get_va(cmd)); + etnaviv_core_dump_mem(&iter, ETDUMP_BUF_RING, gpu->buffer.vaddr, + gpu->buffer.size, + etnaviv_cmdbuf_get_va(&gpu->buffer)); + + list_for_each_entry(submit, &gpu->active_submit_list, node) + etnaviv_core_dump_mem(&iter, ETDUMP_BUF_CMD, + submit->cmdbuf.vaddr, submit->cmdbuf.size, + etnaviv_cmdbuf_get_va(&submit->cmdbuf)); /* Reserve space for the bomap */ if (n_bomap_pages) { diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h index 63285101850c2..fe99f0c0d068f 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h @@ -18,6 +18,7 @@ #define __ETNAVIV_GEM_H__ #include +#include "etnaviv_cmdbuf.h" #include "etnaviv_drv.h" struct dma_fence; @@ -103,6 +104,8 @@ struct etnaviv_gem_submit { struct kref refcount; struct etnaviv_gpu *gpu; struct dma_fence *out_fence, *in_fence; + struct list_head node; /* GPU active submit list */ + struct etnaviv_cmdbuf cmdbuf; u32 exec_state; u32 flags; unsigned int nr_pmrs; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index d87a7f00bca33..db40cd1c14fde 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -354,6 +354,9 @@ static void submit_cleanup(struct kref *kref) container_of(kref, struct etnaviv_gem_submit, refcount); unsigned i; + if (submit->cmdbuf.suballoc) + etnaviv_cmdbuf_free(&submit->cmdbuf); + for (i = 0; i < submit->nr_bos; i++) { struct etnaviv_gem_object *etnaviv_obj = submit->bos[i].obj; @@ -391,7 +394,6 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, struct drm_etnaviv_gem_submit_pmr *pmrs; struct drm_etnaviv_gem_submit_bo *bos; struct etnaviv_gem_submit *submit; - struct etnaviv_cmdbuf *cmdbuf; struct etnaviv_gpu *gpu; struct sync_file *sync_file = NULL; struct ww_acquire_ctx ticket; @@ -432,16 +434,11 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, relocs = kvmalloc_array(args->nr_relocs, sizeof(*relocs), GFP_KERNEL); pmrs = kvmalloc_array(args->nr_pmrs, sizeof(*pmrs), GFP_KERNEL); stream = kvmalloc_array(1, args->stream_size, GFP_KERNEL); - cmdbuf = etnaviv_cmdbuf_new(gpu->cmdbuf_suballoc, - ALIGN(args->stream_size, 8) + 8, - args->nr_bos); - if (!bos || !relocs || !pmrs || !stream || !cmdbuf) { + if (!bos || !relocs || !pmrs || !stream) { ret = -ENOMEM; goto err_submit_cmds; } - cmdbuf->ctx = file->driver_priv; - ret = copy_from_user(bos, u64_to_user_ptr(args->bos), args->nr_bos * sizeof(*bos)); if (ret) { @@ -486,6 +483,12 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, goto err_submit_ww_acquire; } + ret = etnaviv_cmdbuf_init(gpu->cmdbuf_suballoc, &submit->cmdbuf, + ALIGN(args->stream_size, 8) + 8); + if (ret) + goto err_submit_objects; + + submit->cmdbuf.ctx = file->driver_priv; submit->exec_state = args->exec_state; submit->flags = args->flags; @@ -528,17 +531,15 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, if (ret) goto err_submit_objects; - memcpy(cmdbuf->vaddr, stream, args->stream_size); - cmdbuf->user_size = ALIGN(args->stream_size, 8); + memcpy(submit->cmdbuf.vaddr, stream, args->stream_size); + submit->cmdbuf.user_size = ALIGN(args->stream_size, 8); - ret = etnaviv_gpu_submit(gpu, submit, cmdbuf); + ret = etnaviv_gpu_submit(gpu, submit); if (ret) goto err_submit_objects; submit_attach_object_fences(submit); - cmdbuf = NULL; - if (args->flags & ETNA_SUBMIT_FENCE_FD_OUT) { /* * This can be improved: ideally we want to allocate the sync @@ -566,9 +567,6 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, err_submit_cmds: if (ret && (out_fence_fd >= 0)) put_unused_fd(out_fence_fd); - /* if we still own the cmdbuf */ - if (cmdbuf) - etnaviv_cmdbuf_free(cmdbuf); if (stream) kvfree(stream); if (bos) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index afc4b6c5fbf56..efa79af676d63 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -644,7 +644,7 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu) prefetch = etnaviv_buffer_init(gpu); gpu_write(gpu, VIVS_HI_INTR_ENBL, ~0U); - etnaviv_gpu_start_fe(gpu, etnaviv_cmdbuf_get_va(gpu->buffer), + etnaviv_gpu_start_fe(gpu, etnaviv_cmdbuf_get_va(&gpu->buffer), prefetch); } @@ -717,15 +717,15 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu) } /* Create buffer: */ - gpu->buffer = etnaviv_cmdbuf_new(gpu->cmdbuf_suballoc, PAGE_SIZE, 0); - if (!gpu->buffer) { - ret = -ENOMEM; + ret = etnaviv_cmdbuf_init(gpu->cmdbuf_suballoc, &gpu->buffer, + PAGE_SIZE); + if (ret) { dev_err(gpu->dev, "could not create command buffer\n"); goto destroy_iommu; } if (gpu->mmu->version == ETNAVIV_IOMMU_V1 && - etnaviv_cmdbuf_get_va(gpu->buffer) > 0x80000000) { + etnaviv_cmdbuf_get_va(&gpu->buffer) > 0x80000000) { ret = -EINVAL; dev_err(gpu->dev, "command buffer outside valid memory window\n"); @@ -751,8 +751,7 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu) return 0; free_buffer: - etnaviv_cmdbuf_free(gpu->buffer); - gpu->buffer = NULL; + etnaviv_cmdbuf_free(&gpu->buffer); destroy_iommu: etnaviv_iommu_destroy(gpu->mmu); gpu->mmu = NULL; @@ -1201,27 +1200,20 @@ static void retire_worker(struct work_struct *work) struct etnaviv_gpu *gpu = container_of(work, struct etnaviv_gpu, retire_work); u32 fence = gpu->completed_fence; - struct etnaviv_cmdbuf *cmdbuf, *tmp; + struct etnaviv_gem_submit *submit, *tmp; unsigned int i; mutex_lock(&gpu->lock); - list_for_each_entry_safe(cmdbuf, tmp, &gpu->active_cmd_list, node) { - if (!dma_fence_is_signaled(cmdbuf->fence)) + list_for_each_entry_safe(submit, tmp, &gpu->active_submit_list, node) { + if (!dma_fence_is_signaled(submit->out_fence)) break; - list_del(&cmdbuf->node); - dma_fence_put(cmdbuf->fence); + list_del(&submit->node); - for (i = 0; i < cmdbuf->nr_bos; i++) { - struct etnaviv_vram_mapping *mapping = cmdbuf->bo_map[i]; - struct etnaviv_gem_object *etnaviv_obj = mapping->object; - - atomic_dec(&etnaviv_obj->gpu_active); - /* drop the refcount taken in etnaviv_gpu_submit */ - etnaviv_gem_mapping_unreference(mapping); - } + for (i = 0; i < submit->nr_bos; i++) + atomic_dec(&submit->bos[i].obj->gpu_active); - etnaviv_cmdbuf_free(cmdbuf); + etnaviv_submit_put(submit); /* * We need to balance the runtime PM count caused by * each submission. Upon submission, we increment @@ -1375,9 +1367,8 @@ static void sync_point_perfmon_sample_post(struct etnaviv_gpu *gpu, /* add bo's to gpu's ring, and kick gpu: */ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, - struct etnaviv_gem_submit *submit, struct etnaviv_cmdbuf *cmdbuf) + struct etnaviv_gem_submit *submit) { - struct dma_fence *fence; unsigned int i, nr_events = 1, event[3]; int ret; @@ -1403,8 +1394,8 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, mutex_lock(&gpu->lock); - fence = etnaviv_gpu_fence_alloc(gpu); - if (!fence) { + submit->out_fence = etnaviv_gpu_fence_alloc(gpu); + if (!submit->out_fence) { for (i = 0; i < nr_events; i++) event_free(gpu, event[i]); @@ -1412,8 +1403,6 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, goto out_unlock; } - gpu->event[event[0]].fence = fence; - submit->out_fence = dma_fence_get(fence); gpu->active_fence = submit->out_fence->seqno; if (submit->nr_pmrs) { @@ -1423,7 +1412,10 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, etnaviv_sync_point_queue(gpu, event[1]); } - etnaviv_buffer_queue(gpu, submit->exec_state, event[0], cmdbuf); + kref_get(&submit->refcount); + gpu->event[event[0]].fence = submit->out_fence; + etnaviv_buffer_queue(gpu, submit->exec_state, event[0], + &submit->cmdbuf); if (submit->nr_pmrs) { gpu->event[event[2]].sync_point = &sync_point_perfmon_sample_post; @@ -1432,21 +1424,15 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, etnaviv_sync_point_queue(gpu, event[2]); } - cmdbuf->fence = fence; - list_add_tail(&cmdbuf->node, &gpu->active_cmd_list); + list_add_tail(&submit->node, &gpu->active_submit_list); /* We're committed to adding this command buffer, hold a PM reference */ pm_runtime_get_noresume(gpu->dev); for (i = 0; i < submit->nr_bos; i++) { struct etnaviv_gem_object *etnaviv_obj = submit->bos[i].obj; - - /* Each cmdbuf takes a refcount on the mapping */ - etnaviv_gem_mapping_reference(submit->bos[i].mapping); - cmdbuf->bo_map[i] = submit->bos[i].mapping; atomic_inc(&etnaviv_obj->gpu_active); } - cmdbuf->nr_bos = submit->nr_bos; hangcheck_timer_reset(gpu); ret = 0; @@ -1625,7 +1611,7 @@ int etnaviv_gpu_wait_idle(struct etnaviv_gpu *gpu, unsigned int timeout_ms) static int etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu) { - if (gpu->buffer) { + if (gpu->buffer.suballoc) { /* Replace the last WAIT with END */ mutex_lock(&gpu->lock); etnaviv_buffer_end(gpu); @@ -1742,7 +1728,7 @@ static int etnaviv_gpu_bind(struct device *dev, struct device *master, gpu->fence_context = dma_fence_context_alloc(1); spin_lock_init(&gpu->fence_spinlock); - INIT_LIST_HEAD(&gpu->active_cmd_list); + INIT_LIST_HEAD(&gpu->active_submit_list); INIT_WORK(&gpu->retire_work, retire_worker); INIT_WORK(&gpu->sync_point_work, sync_point_worker); INIT_WORK(&gpu->recover_work, recover_worker); @@ -1777,10 +1763,8 @@ static void etnaviv_gpu_unbind(struct device *dev, struct device *master, etnaviv_gpu_hw_suspend(gpu); #endif - if (gpu->buffer) { - etnaviv_cmdbuf_free(gpu->buffer); - gpu->buffer = NULL; - } + if (gpu->buffer.suballoc) + etnaviv_cmdbuf_free(&gpu->buffer); if (gpu->cmdbuf_suballoc) { etnaviv_cmdbuf_suballoc_destroy(gpu->cmdbuf_suballoc); @@ -1918,7 +1902,7 @@ static int etnaviv_gpu_rpm_resume(struct device *dev) return ret; /* Re-initialise the basic hardware state */ - if (gpu->drm && gpu->buffer) { + if (gpu->drm && gpu->buffer.suballoc) { ret = etnaviv_gpu_hw_resume(gpu); if (ret) { etnaviv_gpu_clk_disable(gpu); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h index eea823838b5fb..7623905210dc3 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h @@ -20,6 +20,7 @@ #include #include +#include "etnaviv_cmdbuf.h" #include "etnaviv_drv.h" struct etnaviv_gem_submit; @@ -109,7 +110,7 @@ struct etnaviv_gpu { struct workqueue_struct *wq; /* 'ring'-buffer: */ - struct etnaviv_cmdbuf *buffer; + struct etnaviv_cmdbuf buffer; int exec_state; /* bus base address of memory */ @@ -122,7 +123,7 @@ struct etnaviv_gpu { spinlock_t event_spinlock; /* list of currently in-flight command buffers */ - struct list_head active_cmd_list; + struct list_head active_submit_list; u32 idle_mask; @@ -202,7 +203,7 @@ int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu, int etnaviv_gpu_wait_obj_inactive(struct etnaviv_gpu *gpu, struct etnaviv_gem_object *etnaviv_obj, struct timespec *timeout); int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, - struct etnaviv_gem_submit *submit, struct etnaviv_cmdbuf *cmdbuf); + struct etnaviv_gem_submit *submit); int etnaviv_gpu_pm_get_sync(struct etnaviv_gpu *gpu); void etnaviv_gpu_pm_put(struct etnaviv_gpu *gpu); int etnaviv_gpu_wait_idle(struct etnaviv_gpu *gpu, unsigned int timeout_ms); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c b/drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c index fc60fc8ddbf01..1e956e266aa38 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c @@ -229,7 +229,7 @@ void etnaviv_iommuv2_restore(struct etnaviv_gpu *gpu) prefetch = etnaviv_buffer_config_mmuv2(gpu, (u32)etnaviv_domain->mtlb_dma, (u32)etnaviv_domain->base.bad_page_dma); - etnaviv_gpu_start_fe(gpu, (u32)etnaviv_cmdbuf_get_pa(gpu->buffer), + etnaviv_gpu_start_fe(gpu, (u32)etnaviv_cmdbuf_get_pa(&gpu->buffer), prefetch); etnaviv_gpu_wait_idle(gpu, 100); From 5b223e94a8842e5e9deaa1b8198fc52558fee782 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Mon, 27 Nov 2017 17:46:15 +0100 Subject: [PATCH 28/32] drm/etnaviv: move GPU active handling to bo pin/unpin The active count is used to check if the BO is idle, where idle is defined as not active on the GPU and all VM mappings and reference counts dropped to the initial state. As the idling of the mappings and references now only happens in the submit cleanup, the active state handling must be moved to the same location in order to keep the userspace semantics. Signed-off-by: Lucas Stach --- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 4 ++++ drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 10 ---------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index db40cd1c14fde..5a351b0f1087e 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -229,6 +229,7 @@ static int submit_pin_objects(struct etnaviv_gem_submit *submit) ret = PTR_ERR(mapping); break; } + atomic_inc(&etnaviv_obj->gpu_active); submit->bos[i].flags |= BO_PINNED; submit->bos[i].mapping = mapping; @@ -363,6 +364,7 @@ static void submit_cleanup(struct kref *kref) /* unpin all objects */ if (submit->bos[i].flags & BO_PINNED) { etnaviv_gem_mapping_unreference(submit->bos[i].mapping); + atomic_dec(&etnaviv_obj->gpu_active); submit->bos[i].mapping = NULL; submit->bos[i].flags &= ~BO_PINNED; } @@ -372,6 +374,8 @@ static void submit_cleanup(struct kref *kref) drm_gem_object_put_unlocked(&etnaviv_obj->base); } + wake_up_all(&submit->gpu->fence_event); + if (submit->in_fence) dma_fence_put(submit->in_fence); if (submit->out_fence) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index efa79af676d63..5742e023c5d83 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1201,7 +1201,6 @@ static void retire_worker(struct work_struct *work) retire_work); u32 fence = gpu->completed_fence; struct etnaviv_gem_submit *submit, *tmp; - unsigned int i; mutex_lock(&gpu->lock); list_for_each_entry_safe(submit, tmp, &gpu->active_submit_list, node) { @@ -1210,9 +1209,6 @@ static void retire_worker(struct work_struct *work) list_del(&submit->node); - for (i = 0; i < submit->nr_bos; i++) - atomic_dec(&submit->bos[i].obj->gpu_active); - etnaviv_submit_put(submit); /* * We need to balance the runtime PM count caused by @@ -1227,8 +1223,6 @@ static void retire_worker(struct work_struct *work) gpu->retired_fence = fence; mutex_unlock(&gpu->lock); - - wake_up_all(&gpu->fence_event); } int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu, @@ -1429,10 +1423,6 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, /* We're committed to adding this command buffer, hold a PM reference */ pm_runtime_get_noresume(gpu->dev); - for (i = 0; i < submit->nr_bos; i++) { - struct etnaviv_gem_object *etnaviv_obj = submit->bos[i].obj; - atomic_inc(&etnaviv_obj->gpu_active); - } hangcheck_timer_reset(gpu); ret = 0; From 8bda1516fb4acf4bd6eaeb746258a9f536aeeb5d Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 24 Nov 2017 17:56:29 +0100 Subject: [PATCH 29/32] drm/etnaviv: couple runtime PM management to submit object lifetime As long as there is an active submit, we want the GPU to stay awake. This is slightly complicated by the fact that we really want to wake the GPU at the last possible moment to achieve maximum power savings. Signed-off-by: Lucas Stach --- drivers/gpu/drm/etnaviv/etnaviv_gem.h | 1 + drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 3 ++ drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 30 ++------------------ 3 files changed, 7 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h index fe99f0c0d068f..be72a9833f2b4 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h @@ -106,6 +106,7 @@ struct etnaviv_gem_submit { struct dma_fence *out_fence, *in_fence; struct list_head node; /* GPU active submit list */ struct etnaviv_cmdbuf cmdbuf; + bool runtime_resumed; u32 exec_state; u32 flags; unsigned int nr_pmrs; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 5a351b0f1087e..1f8202bca0615 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -355,6 +355,9 @@ static void submit_cleanup(struct kref *kref) container_of(kref, struct etnaviv_gem_submit, refcount); unsigned i; + if (submit->runtime_resumed) + pm_runtime_put_autosuspend(submit->gpu->dev); + if (submit->cmdbuf.suballoc) etnaviv_cmdbuf_free(&submit->cmdbuf); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 5742e023c5d83..072384f3637ef 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1210,14 +1210,6 @@ static void retire_worker(struct work_struct *work) list_del(&submit->node); etnaviv_submit_put(submit); - /* - * We need to balance the runtime PM count caused by - * each submission. Upon submission, we increment - * the runtime PM counter, and allocate one event. - * So here, we put the runtime PM count for each - * completed event. - */ - pm_runtime_put_autosuspend(gpu->dev); } gpu->retired_fence = fence; @@ -1289,17 +1281,6 @@ int etnaviv_gpu_wait_obj_inactive(struct etnaviv_gpu *gpu, return -ETIMEDOUT; } -int etnaviv_gpu_pm_get_sync(struct etnaviv_gpu *gpu) -{ - return pm_runtime_get_sync(gpu->dev); -} - -void etnaviv_gpu_pm_put(struct etnaviv_gpu *gpu) -{ - pm_runtime_mark_last_busy(gpu->dev); - pm_runtime_put_autosuspend(gpu->dev); -} - static void sync_point_perfmon_sample(struct etnaviv_gpu *gpu, struct etnaviv_event *event, unsigned int flags) { @@ -1366,9 +1347,10 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, unsigned int i, nr_events = 1, event[3]; int ret; - ret = etnaviv_gpu_pm_get_sync(gpu); + ret = pm_runtime_get_sync(gpu->dev); if (ret < 0) return ret; + submit->runtime_resumed = true; /* * if there are performance monitor requests we need to have @@ -1383,7 +1365,7 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, ret = event_alloc(gpu, nr_events, event); if (ret) { DRM_ERROR("no free events\n"); - goto out_pm_put; + return ret; } mutex_lock(&gpu->lock); @@ -1420,18 +1402,12 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu, list_add_tail(&submit->node, &gpu->active_submit_list); - /* We're committed to adding this command buffer, hold a PM reference */ - pm_runtime_get_noresume(gpu->dev); - hangcheck_timer_reset(gpu); ret = 0; out_unlock: mutex_unlock(&gpu->lock); -out_pm_put: - etnaviv_gpu_pm_put(gpu); - return ret; } From a7cfa565d48ff08f7df95d59c963ce1381526c30 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Fri, 24 Nov 2017 12:13:25 +0100 Subject: [PATCH 30/32] drm/etnaviv: re-enable perfmon support Now that the PMR lifetime issues are solved we can safely re-enable performance counter profiling support. Signed-off-by: Lucas Stach --- drivers/gpu/drm/etnaviv/etnaviv_drv.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c index f2948885859d7..6faf4042db234 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c @@ -459,9 +459,6 @@ static int etnaviv_ioctl_pm_query_dom(struct drm_device *dev, void *data, struct drm_etnaviv_pm_domain *args = data; struct etnaviv_gpu *gpu; - /* reject as long as the feature isn't stable */ - return -EINVAL; - if (args->pipe >= ETNA_MAX_PIPES) return -EINVAL; @@ -479,9 +476,6 @@ static int etnaviv_ioctl_pm_query_sig(struct drm_device *dev, void *data, struct drm_etnaviv_pm_signal *args = data; struct etnaviv_gpu *gpu; - /* reject as long as the feature isn't stable */ - return -EINVAL; - if (args->pipe >= ETNA_MAX_PIPES) return -EINVAL; @@ -556,7 +550,7 @@ static struct drm_driver etnaviv_drm_driver = { .desc = "etnaviv DRM", .date = "20151214", .major = 1, - .minor = 1, + .minor = 2, }; /* From 2e3a2dda2515cc85b8600e3d8a20d1d4a9100ebd Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Wed, 29 Nov 2017 14:33:57 +0100 Subject: [PATCH 31/32] drm/etnaviv: move submit free out of critical section There is no need to hold the GPU lock while freeing the submit object. Only move the retired submits from the GPU active list to a temporary retire list under the GPU lock. Signed-off-by: Lucas Stach --- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 072384f3637ef..21d0d22f11688 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1201,20 +1201,22 @@ static void retire_worker(struct work_struct *work) retire_work); u32 fence = gpu->completed_fence; struct etnaviv_gem_submit *submit, *tmp; + LIST_HEAD(retire_list); mutex_lock(&gpu->lock); list_for_each_entry_safe(submit, tmp, &gpu->active_submit_list, node) { if (!dma_fence_is_signaled(submit->out_fence)) break; - list_del(&submit->node); - - etnaviv_submit_put(submit); + list_move(&submit->node, &retire_list); } gpu->retired_fence = fence; mutex_unlock(&gpu->lock); + + list_for_each_entry_safe(submit, tmp, &retire_list, node) + etnaviv_submit_put(submit); } int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu, From 2f20fc4fc9db236dc3806a93e78c7575c9561590 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Mon, 27 Nov 2017 15:33:28 +0100 Subject: [PATCH 32/32] drm/etnaviv: use memset32 to init pagetable Now that memset32 is available, the open-coded pagetable initialization loop can be replaced. Signed-off-by: Lucas Stach --- drivers/gpu/drm/etnaviv/etnaviv_iommu.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_iommu.c b/drivers/gpu/drm/etnaviv/etnaviv_iommu.c index 14e24ac6573fe..7a8c947317486 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_iommu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_iommu.c @@ -70,9 +70,8 @@ static int __etnaviv_iommu_init(struct etnaviv_iommuv1_domain *etnaviv_domain) return -ENOMEM; } - for (i = 0; i < PT_ENTRIES; i++) - etnaviv_domain->pgtable_cpu[i] = - etnaviv_domain->base.bad_page_dma; + memset32(etnaviv_domain->pgtable_cpu, etnaviv_domain->base.bad_page_dma, + PT_ENTRIES); return 0; }