From 4adf0b49eea926a55fd956ef7d86750f771435ff Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Fri, 6 Dec 2019 09:19:01 +0100 Subject: [PATCH 1/6] drm/mgag200: Flag all G200 SE A machines as broken wrt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Several MGA G200 SE machines don't respect the value of the startadd register field. After more feedback on affected machines, neither PCI subvendor ID nor the internal ID seem to hint towards the bug. All affected machines have a PCI ID of 0x0522 (i.e., G200 SE A). It was decided to flag all G200 SE A machines as broken. Signed-off-by: Thomas Zimmermann Acked-by: Gerd Hoffmann Fixes: 1591fadf857c ("drm/mgag200: Add workaround for HW that does not support 'startadd'") Cc: Thomas Zimmermann Cc: John Donnelly Cc: Daniel Vetter Cc: Gerd Hoffmann Cc: Dave Airlie Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: David Airlie Cc: Sam Ravnborg Cc: "Y.C. Chen" Cc: Neil Armstrong Cc: Thomas Gleixner Cc: "José Roberto de Souza" Cc: Andrzej Pietrasiewicz Cc: dri-devel@lists.freedesktop.org Cc: # v5.3+ Cc: Greg Kroah-Hartman Cc: Allison Randal Cc: Alex Deucher Cc: "Noralf Trønnes" Link: https://patchwork.freedesktop.org/patch/msgid/20191206081901.9938-1-tzimmermann@suse.de --- drivers/gpu/drm/mgag200/mgag200_drv.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index d43951caeea0..b113876c2428 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -30,9 +30,8 @@ module_param_named(modeset, mgag200_modeset, int, 0400); static struct drm_driver driver; static const struct pci_device_id pciidlist[] = { - { PCI_VENDOR_ID_MATROX, 0x522, PCI_VENDOR_ID_SUN, 0x4852, 0, 0, + { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_A | MGAG200_FLAG_HW_BUG_NO_STARTADD}, - { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_A }, { PCI_VENDOR_ID_MATROX, 0x524, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_B }, { PCI_VENDOR_ID_MATROX, 0x530, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_EV }, { PCI_VENDOR_ID_MATROX, 0x532, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_WB }, From 22bd4df9dadf46f56f2eb16859439a7a600d106a Mon Sep 17 00:00:00 2001 From: Steven Price Date: Mon, 18 Nov 2019 17:30:02 +0000 Subject: [PATCH 2/6] drm/panfrost: devfreq: Round frequencies to OPPs Currently when setting a frequency in panfrost_devfreq_target the returned frequency is the actual frequency that the clock driver reports (the return of clk_get_rate()). However, where the provided OPPs don't precisely match the frequencies that the clock actually achieves devfreq will then complain (repeatedly): devfreq devfreq0: Couldn't update frequency transition information. To avoid this change panfrost_devfreq_target() to fetch the opp using devfreq_recommened_opp() and not actually query the clock for the frequency. A similar problem exists with panfrost_devfreq_get_cur_freq(), but in this case because the function is optional we can just remove it and devfreq will fall back to using the previously set frequency. Fixes: 221bc77914cb ("drm/panfrost: Use generic code for devfreq") Signed-off-by: Steven Price Reviewed-by: Alyssa Rosenzweig Signed-off-by: Rob Herring Link: https://patchwork.freedesktop.org/patch/msgid/20191118173002.32015-1-steven.price@arm.com --- drivers/gpu/drm/panfrost/panfrost_devfreq.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index 4c4e8a30a1ac..536ba93b0f46 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -18,15 +18,18 @@ static void panfrost_devfreq_update_utilization(struct panfrost_device *pfdev); static int panfrost_devfreq_target(struct device *dev, unsigned long *freq, u32 flags) { - struct panfrost_device *pfdev = dev_get_drvdata(dev); + struct dev_pm_opp *opp; int err; + opp = devfreq_recommended_opp(dev, freq, flags); + if (IS_ERR(opp)) + return PTR_ERR(opp); + dev_pm_opp_put(opp); + err = dev_pm_opp_set_rate(dev, *freq); if (err) return err; - *freq = clk_get_rate(pfdev->clock); - return 0; } @@ -60,20 +63,10 @@ static int panfrost_devfreq_get_dev_status(struct device *dev, return 0; } -static int panfrost_devfreq_get_cur_freq(struct device *dev, unsigned long *freq) -{ - struct panfrost_device *pfdev = platform_get_drvdata(to_platform_device(dev)); - - *freq = clk_get_rate(pfdev->clock); - - return 0; -} - static struct devfreq_dev_profile panfrost_devfreq_profile = { .polling_ms = 50, /* ~3 frames */ .target = panfrost_devfreq_target, .get_dev_status = panfrost_devfreq_get_dev_status, - .get_cur_freq = panfrost_devfreq_get_cur_freq, }; int panfrost_devfreq_init(struct panfrost_device *pfdev) From 70cc77952efebf6722d483cb83cfb563ac9768db Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Fri, 29 Nov 2019 14:59:02 +0100 Subject: [PATCH 3/6] drm/panfrost: Fix a race in panfrost_ioctl_madvise() If 2 threads change the MADVISE property of the same BO in parallel we might end up with an shmem->madv value that's inconsistent with the presence of the BO in the shrinker list. The easiest solution to fix that is to protect the drm_gem_shmem_madvise() call with the shrinker lock. Fixes: 013b65101315 ("drm/panfrost: Add madvise and shrinker support") Cc: Signed-off-by: Boris Brezillon Reviewed-by: Steven Price Acked-by: Alyssa Rosenzweig Signed-off-by: Rob Herring Link: https://patchwork.freedesktop.org/patch/msgid/20191129135908.2439529-3-boris.brezillon@collabora.com --- drivers/gpu/drm/panfrost/panfrost_drv.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 9d086133a84e..8b8d6546cfac 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -347,20 +347,19 @@ static int panfrost_ioctl_madvise(struct drm_device *dev, void *data, return -ENOENT; } + mutex_lock(&pfdev->shrinker_lock); args->retained = drm_gem_shmem_madvise(gem_obj, args->madv); if (args->retained) { struct panfrost_gem_object *bo = to_panfrost_bo(gem_obj); - mutex_lock(&pfdev->shrinker_lock); - if (args->madv == PANFROST_MADV_DONTNEED) - list_add_tail(&bo->base.madv_list, &pfdev->shrinker_list); + list_add_tail(&bo->base.madv_list, + &pfdev->shrinker_list); else if (args->madv == PANFROST_MADV_WILLNEED) list_del_init(&bo->base.madv_list); - - mutex_unlock(&pfdev->shrinker_lock); } + mutex_unlock(&pfdev->shrinker_lock); drm_gem_object_put_unlocked(gem_obj); return 0; From 3bb69dbcb9e8430e0cc9990cff427ca3ae25ffdc Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Fri, 29 Nov 2019 14:59:03 +0100 Subject: [PATCH 4/6] drm/panfrost: Fix a BO leak in panfrost_ioctl_mmap_bo() We should release the reference we grabbed when an error occurs. Fixes: 187d2929206e ("drm/panfrost: Add support for GPU heap allocations") Cc: Signed-off-by: Boris Brezillon Reviewed-by: Steven Price Acked-by: Alyssa Rosenzweig Signed-off-by: Rob Herring Link: https://patchwork.freedesktop.org/patch/msgid/20191129135908.2439529-4-boris.brezillon@collabora.com --- drivers/gpu/drm/panfrost/panfrost_drv.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 8b8d6546cfac..0a5d82078bc8 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -303,14 +303,17 @@ static int panfrost_ioctl_mmap_bo(struct drm_device *dev, void *data, } /* Don't allow mmapping of heap objects as pages are not pinned. */ - if (to_panfrost_bo(gem_obj)->is_heap) - return -EINVAL; + if (to_panfrost_bo(gem_obj)->is_heap) { + ret = -EINVAL; + goto out; + } ret = drm_gem_create_mmap_offset(gem_obj); if (ret == 0) args->offset = drm_vma_node_offset_addr(&gem_obj->vma_node); - drm_gem_object_put_unlocked(gem_obj); +out: + drm_gem_object_put_unlocked(gem_obj); return ret; } From aed44cbeae2b7674cd155ba5cc6506aafe46a94e Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Fri, 29 Nov 2019 14:59:04 +0100 Subject: [PATCH 5/6] drm/panfrost: Fix a race in panfrost_gem_free_object() panfrost_gem_shrinker_scan() might purge a BO (release the sgt and kill the GPU mapping) that's being freed by panfrost_gem_free_object() if we don't remove the BO from the shrinker list at the beginning of panfrost_gem_free_object(). Fixes: 013b65101315 ("drm/panfrost: Add madvise and shrinker support") Cc: Signed-off-by: Boris Brezillon Reviewed-by: Steven Price Acked-by: Alyssa Rosenzweig Signed-off-by: Rob Herring Link: https://patchwork.freedesktop.org/patch/msgid/20191129135908.2439529-5-boris.brezillon@collabora.com --- drivers/gpu/drm/panfrost/panfrost_gem.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c index deca0c30bbd4..fa1b2732a3a8 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gem.c +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c @@ -19,6 +19,16 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj) struct panfrost_gem_object *bo = to_panfrost_bo(obj); struct panfrost_device *pfdev = obj->dev->dev_private; + /* + * Make sure the BO is no longer inserted in the shrinker list before + * taking care of the destruction itself. If we don't do that we have a + * race condition between this function and what's done in + * panfrost_gem_shrinker_scan(). + */ + mutex_lock(&pfdev->shrinker_lock); + list_del_init(&bo->base.madv_list); + mutex_unlock(&pfdev->shrinker_lock); + if (bo->sgts) { int i; int n_sgt = bo->base.base.size / SZ_2M; @@ -33,11 +43,6 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj) kfree(bo->sgts); } - mutex_lock(&pfdev->shrinker_lock); - if (!list_empty(&bo->base.madv_list)) - list_del(&bo->base.madv_list); - mutex_unlock(&pfdev->shrinker_lock); - drm_gem_shmem_free_object(obj); } From 0a5239985a3bc084738851afdf3fceb7d5651b0c Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Fri, 29 Nov 2019 14:59:05 +0100 Subject: [PATCH 6/6] drm/panfrost: Open/close the perfcnt BO Commit a5efb4c9a562 ("drm/panfrost: Restructure the GEM object creation") moved the drm_mm_insert_node_generic() call to the gem->open() hook, but forgot to update perfcnt accordingly. Patch the perfcnt logic to call panfrost_gem_open/close() where appropriate. Fixes: a5efb4c9a562 ("drm/panfrost: Restructure the GEM object creation") Cc: Signed-off-by: Boris Brezillon Reviewed-by: Steven Price Acked-by: Alyssa Rosenzweig Signed-off-by: Rob Herring Link: https://patchwork.freedesktop.org/patch/msgid/20191129135908.2439529-6-boris.brezillon@collabora.com --- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- drivers/gpu/drm/panfrost/panfrost_gem.c | 4 ++-- drivers/gpu/drm/panfrost/panfrost_gem.h | 4 ++++ drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 23 +++++++++++++-------- drivers/gpu/drm/panfrost/panfrost_perfcnt.h | 2 +- 5 files changed, 22 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 0a5d82078bc8..ca9ba7534eb7 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -445,7 +445,7 @@ panfrost_postclose(struct drm_device *dev, struct drm_file *file) { struct panfrost_file_priv *panfrost_priv = file->driver_priv; - panfrost_perfcnt_close(panfrost_priv); + panfrost_perfcnt_close(file); panfrost_job_close(panfrost_priv); panfrost_mmu_pgtable_free(panfrost_priv); diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c index fa1b2732a3a8..fd766b1395fb 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gem.c +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c @@ -46,7 +46,7 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj) drm_gem_shmem_free_object(obj); } -static int panfrost_gem_open(struct drm_gem_object *obj, struct drm_file *file_priv) +int panfrost_gem_open(struct drm_gem_object *obj, struct drm_file *file_priv) { int ret; size_t size = obj->size; @@ -85,7 +85,7 @@ static int panfrost_gem_open(struct drm_gem_object *obj, struct drm_file *file_p return ret; } -static void panfrost_gem_close(struct drm_gem_object *obj, struct drm_file *file_priv) +void panfrost_gem_close(struct drm_gem_object *obj, struct drm_file *file_priv) { struct panfrost_gem_object *bo = to_panfrost_bo(obj); struct panfrost_file_priv *priv = file_priv->driver_priv; diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h index 50920819cc16..4b17e7308764 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gem.h +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h @@ -45,6 +45,10 @@ panfrost_gem_create_with_handle(struct drm_file *file_priv, u32 flags, uint32_t *handle); +int panfrost_gem_open(struct drm_gem_object *obj, struct drm_file *file_priv); +void panfrost_gem_close(struct drm_gem_object *obj, + struct drm_file *file_priv); + void panfrost_gem_shrinker_init(struct drm_device *dev); void panfrost_gem_shrinker_cleanup(struct drm_device *dev); diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c index 83c57d325ca8..33054b7524ea 100644 --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c @@ -66,9 +66,10 @@ static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev) } static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev, - struct panfrost_file_priv *user, + struct drm_file *file_priv, unsigned int counterset) { + struct panfrost_file_priv *user = file_priv->driver_priv; struct panfrost_perfcnt *perfcnt = pfdev->perfcnt; struct drm_gem_shmem_object *bo; u32 cfg; @@ -90,14 +91,14 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev, perfcnt->bo = to_panfrost_bo(&bo->base); /* Map the perfcnt buf in the address space attached to file_priv. */ - ret = panfrost_mmu_map(perfcnt->bo); + ret = panfrost_gem_open(&perfcnt->bo->base.base, file_priv); if (ret) goto err_put_bo; perfcnt->buf = drm_gem_shmem_vmap(&bo->base); if (IS_ERR(perfcnt->buf)) { ret = PTR_ERR(perfcnt->buf); - goto err_put_bo; + goto err_close_bo; } /* @@ -156,14 +157,17 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev, err_vunmap: drm_gem_shmem_vunmap(&perfcnt->bo->base.base, perfcnt->buf); +err_close_bo: + panfrost_gem_close(&perfcnt->bo->base.base, file_priv); err_put_bo: drm_gem_object_put_unlocked(&bo->base); return ret; } static int panfrost_perfcnt_disable_locked(struct panfrost_device *pfdev, - struct panfrost_file_priv *user) + struct drm_file *file_priv) { + struct panfrost_file_priv *user = file_priv->driver_priv; struct panfrost_perfcnt *perfcnt = pfdev->perfcnt; if (user != perfcnt->user) @@ -179,6 +183,7 @@ static int panfrost_perfcnt_disable_locked(struct panfrost_device *pfdev, perfcnt->user = NULL; drm_gem_shmem_vunmap(&perfcnt->bo->base.base, perfcnt->buf); perfcnt->buf = NULL; + panfrost_gem_close(&perfcnt->bo->base.base, file_priv); drm_gem_object_put_unlocked(&perfcnt->bo->base.base); perfcnt->bo = NULL; pm_runtime_mark_last_busy(pfdev->dev); @@ -190,7 +195,6 @@ static int panfrost_perfcnt_disable_locked(struct panfrost_device *pfdev, int panfrost_ioctl_perfcnt_enable(struct drm_device *dev, void *data, struct drm_file *file_priv) { - struct panfrost_file_priv *pfile = file_priv->driver_priv; struct panfrost_device *pfdev = dev->dev_private; struct panfrost_perfcnt *perfcnt = pfdev->perfcnt; struct drm_panfrost_perfcnt_enable *req = data; @@ -206,10 +210,10 @@ int panfrost_ioctl_perfcnt_enable(struct drm_device *dev, void *data, mutex_lock(&perfcnt->lock); if (req->enable) - ret = panfrost_perfcnt_enable_locked(pfdev, pfile, + ret = panfrost_perfcnt_enable_locked(pfdev, file_priv, req->counterset); else - ret = panfrost_perfcnt_disable_locked(pfdev, pfile); + ret = panfrost_perfcnt_disable_locked(pfdev, file_priv); mutex_unlock(&perfcnt->lock); return ret; @@ -247,15 +251,16 @@ out: return ret; } -void panfrost_perfcnt_close(struct panfrost_file_priv *pfile) +void panfrost_perfcnt_close(struct drm_file *file_priv) { + struct panfrost_file_priv *pfile = file_priv->driver_priv; struct panfrost_device *pfdev = pfile->pfdev; struct panfrost_perfcnt *perfcnt = pfdev->perfcnt; pm_runtime_get_sync(pfdev->dev); mutex_lock(&perfcnt->lock); if (perfcnt->user == pfile) - panfrost_perfcnt_disable_locked(pfdev, pfile); + panfrost_perfcnt_disable_locked(pfdev, file_priv); mutex_unlock(&perfcnt->lock); pm_runtime_mark_last_busy(pfdev->dev); pm_runtime_put_autosuspend(pfdev->dev); diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.h b/drivers/gpu/drm/panfrost/panfrost_perfcnt.h index 13b8fdaa1b43..8bbcf5f5fb33 100644 --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.h +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.h @@ -9,7 +9,7 @@ void panfrost_perfcnt_sample_done(struct panfrost_device *pfdev); void panfrost_perfcnt_clean_cache_done(struct panfrost_device *pfdev); int panfrost_perfcnt_init(struct panfrost_device *pfdev); void panfrost_perfcnt_fini(struct panfrost_device *pfdev); -void panfrost_perfcnt_close(struct panfrost_file_priv *pfile); +void panfrost_perfcnt_close(struct drm_file *file_priv); int panfrost_ioctl_perfcnt_enable(struct drm_device *dev, void *data, struct drm_file *file_priv); int panfrost_ioctl_perfcnt_dump(struct drm_device *dev, void *data,