* [PATCH 1/3] drm/vmwgfx: Drop svga_lock
@ 2020-12-11 16:29 Daniel Vetter
2020-12-11 16:29 ` [PATCH 2/3] drm/vmwgfx: Always evict vram _before_ disabling it Daniel Vetter
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Daniel Vetter @ 2020-12-11 16:29 UTC (permalink / raw)
To: DRI Development
Cc: Daniel Vetter, sroland, linux-graphics-maintainer, Daniel Vetter
This isn't actually protecting anything becuase:
- when running, ttm_resource_manager->use_type is protected through
vmw_private->reservation_semaphore against concurrent execbuf or
well anything else that might evict or reserve buffers
- during suspend/resume there's nothing else running, hence
vmw_pm_freeze and vmw_pm_restore do not need to take the same lock.
- this also holds for the SVGA_REG_ENABLE register write
Hence it is safe to just remove that spinlock.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Roland Scheidegger <sroland@vmware.com>
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 10 +---------
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 -
2 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 0008be02d31c..204f7a1830f0 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -672,7 +672,6 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
spin_lock_init(&dev_priv->hw_lock);
spin_lock_init(&dev_priv->waiter_lock);
spin_lock_init(&dev_priv->cap_lock);
- spin_lock_init(&dev_priv->svga_lock);
spin_lock_init(&dev_priv->cursor_lock);
for (i = vmw_res_context; i < vmw_res_max; ++i) {
@@ -1189,12 +1188,10 @@ static void __vmw_svga_enable(struct vmw_private *dev_priv)
{
struct ttm_resource_manager *man = ttm_manager_type(&dev_priv->bdev, TTM_PL_VRAM);
- spin_lock(&dev_priv->svga_lock);
if (!ttm_resource_manager_used(man)) {
vmw_write(dev_priv, SVGA_REG_ENABLE, SVGA_REG_ENABLE);
ttm_resource_manager_set_used(man, true);
}
- spin_unlock(&dev_priv->svga_lock);
}
/**
@@ -1220,14 +1217,12 @@ static void __vmw_svga_disable(struct vmw_private *dev_priv)
{
struct ttm_resource_manager *man = ttm_manager_type(&dev_priv->bdev, TTM_PL_VRAM);
- spin_lock(&dev_priv->svga_lock);
if (ttm_resource_manager_used(man)) {
ttm_resource_manager_set_used(man, false);
vmw_write(dev_priv, SVGA_REG_ENABLE,
SVGA_REG_ENABLE_HIDE |
SVGA_REG_ENABLE_ENABLE);
}
- spin_unlock(&dev_priv->svga_lock);
}
/**
@@ -1254,17 +1249,14 @@ void vmw_svga_disable(struct vmw_private *dev_priv)
*/
vmw_kms_lost_device(dev_priv->dev);
ttm_write_lock(&dev_priv->reservation_sem, false);
- spin_lock(&dev_priv->svga_lock);
if (ttm_resource_manager_used(man)) {
ttm_resource_manager_set_used(man, false);
- spin_unlock(&dev_priv->svga_lock);
if (ttm_resource_manager_evict_all(&dev_priv->bdev, man))
DRM_ERROR("Failed evicting VRAM buffers.\n");
vmw_write(dev_priv, SVGA_REG_ENABLE,
SVGA_REG_ENABLE_HIDE |
SVGA_REG_ENABLE_ENABLE);
- } else
- spin_unlock(&dev_priv->svga_lock);
+ }
ttm_write_unlock(&dev_priv->reservation_sem);
}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 5b9a28157dd3..715f2bfee08a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -596,7 +596,6 @@ struct vmw_private {
bool stealth;
bool enable_fb;
- spinlock_t svga_lock;
/**
* PM management.
--
2.29.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/3] drm/vmwgfx: Always evict vram _before_ disabling it 2020-12-11 16:29 [PATCH 1/3] drm/vmwgfx: Drop svga_lock Daniel Vetter @ 2020-12-11 16:29 ` Daniel Vetter 2020-12-11 16:29 ` [PATCH 3/3] drm/ttm: WARN_ON non-empty lru when disabling a resource manager Daniel Vetter 2021-01-12 8:49 ` [PATCH 1/3] drm/vmwgfx: Drop svga_lock Daniel Vetter 2 siblings, 0 replies; 7+ messages in thread From: Daniel Vetter @ 2020-12-11 16:29 UTC (permalink / raw) To: DRI Development Cc: Daniel Vetter, sroland, linux-graphics-maintainer, Daniel Vetter Other way round is a bit inconsistent (but not buggy in any kind). This is prep work so that ttm_resource_manager_set_used can assert that the resource manager is empty. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com> Cc: Roland Scheidegger <sroland@vmware.com> --- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index 204f7a1830f0..0d074c432152 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -1250,9 +1250,9 @@ void vmw_svga_disable(struct vmw_private *dev_priv) vmw_kms_lost_device(dev_priv->dev); ttm_write_lock(&dev_priv->reservation_sem, false); if (ttm_resource_manager_used(man)) { - ttm_resource_manager_set_used(man, false); if (ttm_resource_manager_evict_all(&dev_priv->bdev, man)) DRM_ERROR("Failed evicting VRAM buffers.\n"); + ttm_resource_manager_set_used(man, false); vmw_write(dev_priv, SVGA_REG_ENABLE, SVGA_REG_ENABLE_HIDE | SVGA_REG_ENABLE_ENABLE); -- 2.29.2 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] drm/ttm: WARN_ON non-empty lru when disabling a resource manager 2020-12-11 16:29 [PATCH 1/3] drm/vmwgfx: Drop svga_lock Daniel Vetter 2020-12-11 16:29 ` [PATCH 2/3] drm/vmwgfx: Always evict vram _before_ disabling it Daniel Vetter @ 2020-12-11 16:29 ` Daniel Vetter 2021-01-12 8:49 ` [PATCH 1/3] drm/vmwgfx: Drop svga_lock Daniel Vetter 2 siblings, 0 replies; 7+ messages in thread From: Daniel Vetter @ 2020-12-11 16:29 UTC (permalink / raw) To: DRI Development Cc: Daniel Vetter, sroland, Huang Rui, linux-graphics-maintainer, Daniel Vetter, Christian König ttm_resource_manager->use_type is only used for runtime changes by vmwgfx. I think ideally we'd push this functionality into drivers - ttm itself does not provide any locking to guarantee this is safe, so the only way this can work at runtime is if the driver does provide additional guarantees. vwmgfx does that through the vmw_private->reservation_sem. Therefore supporting this feature in shared code feels a bit misplaced. As a first step add a WARN_ON to make sure the resource manager is empty. This is just to make sure I actually understand correctly what vmwgfx is doing, and to make sure an eventual subsequent refactor doesn't break anything. This check should also be useful for other drivers, to make sure they haven't leaked anything. Reviewed-by: Christian König <christian.koenig@amd.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Christian Koenig <christian.koenig@amd.com> Cc: Huang Rui <ray.huang@amd.com> --- include/drm/ttm/ttm_resource.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h index f48a70d39ac5..789ec477b607 100644 --- a/include/drm/ttm/ttm_resource.h +++ b/include/drm/ttm/ttm_resource.h @@ -191,6 +191,10 @@ struct ttm_resource { static inline void ttm_resource_manager_set_used(struct ttm_resource_manager *man, bool used) { + int i; + + for (i = 0; i < TTM_MAX_BO_PRIORITY; i++) + WARN_ON(!list_empty(&man->lru[i])); man->use_type = used; } -- 2.29.2 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] drm/vmwgfx: Drop svga_lock 2020-12-11 16:29 [PATCH 1/3] drm/vmwgfx: Drop svga_lock Daniel Vetter 2020-12-11 16:29 ` [PATCH 2/3] drm/vmwgfx: Always evict vram _before_ disabling it Daniel Vetter 2020-12-11 16:29 ` [PATCH 3/3] drm/ttm: WARN_ON non-empty lru when disabling a resource manager Daniel Vetter @ 2021-01-12 8:49 ` Daniel Vetter 2021-01-14 15:27 ` Zack Rusin 2021-01-14 15:31 ` Roland Scheidegger 2 siblings, 2 replies; 7+ messages in thread From: Daniel Vetter @ 2021-01-12 8:49 UTC (permalink / raw) To: DRI Development; +Cc: Roland Scheidegger, VMware Graphics, Daniel Vetter Hi Roland, Hopefully you had a nice start into the new year! Ping for some review/testing on this series. Thanks, Daniel On Fri, Dec 11, 2020 at 5:29 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > This isn't actually protecting anything becuase: > - when running, ttm_resource_manager->use_type is protected through > vmw_private->reservation_semaphore against concurrent execbuf or > well anything else that might evict or reserve buffers > - during suspend/resume there's nothing else running, hence > vmw_pm_freeze and vmw_pm_restore do not need to take the same lock. > - this also holds for the SVGA_REG_ENABLE register write > > Hence it is safe to just remove that spinlock. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com> > Cc: Roland Scheidegger <sroland@vmware.com> > --- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 10 +--------- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 - > 2 files changed, 1 insertion(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > index 0008be02d31c..204f7a1830f0 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > @@ -672,7 +672,6 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset) > spin_lock_init(&dev_priv->hw_lock); > spin_lock_init(&dev_priv->waiter_lock); > spin_lock_init(&dev_priv->cap_lock); > - spin_lock_init(&dev_priv->svga_lock); > spin_lock_init(&dev_priv->cursor_lock); > > for (i = vmw_res_context; i < vmw_res_max; ++i) { > @@ -1189,12 +1188,10 @@ static void __vmw_svga_enable(struct vmw_private *dev_priv) > { > struct ttm_resource_manager *man = ttm_manager_type(&dev_priv->bdev, TTM_PL_VRAM); > > - spin_lock(&dev_priv->svga_lock); > if (!ttm_resource_manager_used(man)) { > vmw_write(dev_priv, SVGA_REG_ENABLE, SVGA_REG_ENABLE); > ttm_resource_manager_set_used(man, true); > } > - spin_unlock(&dev_priv->svga_lock); > } > > /** > @@ -1220,14 +1217,12 @@ static void __vmw_svga_disable(struct vmw_private *dev_priv) > { > struct ttm_resource_manager *man = ttm_manager_type(&dev_priv->bdev, TTM_PL_VRAM); > > - spin_lock(&dev_priv->svga_lock); > if (ttm_resource_manager_used(man)) { > ttm_resource_manager_set_used(man, false); > vmw_write(dev_priv, SVGA_REG_ENABLE, > SVGA_REG_ENABLE_HIDE | > SVGA_REG_ENABLE_ENABLE); > } > - spin_unlock(&dev_priv->svga_lock); > } > > /** > @@ -1254,17 +1249,14 @@ void vmw_svga_disable(struct vmw_private *dev_priv) > */ > vmw_kms_lost_device(dev_priv->dev); > ttm_write_lock(&dev_priv->reservation_sem, false); > - spin_lock(&dev_priv->svga_lock); > if (ttm_resource_manager_used(man)) { > ttm_resource_manager_set_used(man, false); > - spin_unlock(&dev_priv->svga_lock); > if (ttm_resource_manager_evict_all(&dev_priv->bdev, man)) > DRM_ERROR("Failed evicting VRAM buffers.\n"); > vmw_write(dev_priv, SVGA_REG_ENABLE, > SVGA_REG_ENABLE_HIDE | > SVGA_REG_ENABLE_ENABLE); > - } else > - spin_unlock(&dev_priv->svga_lock); > + } > ttm_write_unlock(&dev_priv->reservation_sem); > } > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > index 5b9a28157dd3..715f2bfee08a 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > @@ -596,7 +596,6 @@ struct vmw_private { > > bool stealth; > bool enable_fb; > - spinlock_t svga_lock; > > /** > * PM management. > -- > 2.29.2 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] drm/vmwgfx: Drop svga_lock 2021-01-12 8:49 ` [PATCH 1/3] drm/vmwgfx: Drop svga_lock Daniel Vetter @ 2021-01-14 15:27 ` Zack Rusin 2021-01-14 15:31 ` Roland Scheidegger 1 sibling, 0 replies; 7+ messages in thread From: Zack Rusin @ 2021-01-14 15:27 UTC (permalink / raw) To: Daniel Vetter Cc: Roland Scheidegger, Linux-graphics-maintainer, DRI Development, Daniel Vetter Looks good. Thanks! Reviewed-by: Zack Rusin <zackr@vmware.com> > On Jan 12, 2021, at 03:49, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > Hi Roland, > > Hopefully you had a nice start into the new year! Ping for some > review/testing on this series. > > Thanks, Daniel > > On Fri, Dec 11, 2020 at 5:29 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote: >> >> This isn't actually protecting anything becuase: >> - when running, ttm_resource_manager->use_type is protected through >> vmw_private->reservation_semaphore against concurrent execbuf or >> well anything else that might evict or reserve buffers >> - during suspend/resume there's nothing else running, hence >> vmw_pm_freeze and vmw_pm_restore do not need to take the same lock. >> - this also holds for the SVGA_REG_ENABLE register write >> >> Hence it is safe to just remove that spinlock. >> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com> >> Cc: Roland Scheidegger <sroland@vmware.com> >> --- >> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 10 +--------- >> drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 - >> 2 files changed, 1 insertion(+), 10 deletions(-) >> >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c >> index 0008be02d31c..204f7a1830f0 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c >> @@ -672,7 +672,6 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset) >> spin_lock_init(&dev_priv->hw_lock); >> spin_lock_init(&dev_priv->waiter_lock); >> spin_lock_init(&dev_priv->cap_lock); >> - spin_lock_init(&dev_priv->svga_lock); >> spin_lock_init(&dev_priv->cursor_lock); >> >> for (i = vmw_res_context; i < vmw_res_max; ++i) { >> @@ -1189,12 +1188,10 @@ static void __vmw_svga_enable(struct vmw_private *dev_priv) >> { >> struct ttm_resource_manager *man = ttm_manager_type(&dev_priv->bdev, TTM_PL_VRAM); >> >> - spin_lock(&dev_priv->svga_lock); >> if (!ttm_resource_manager_used(man)) { >> vmw_write(dev_priv, SVGA_REG_ENABLE, SVGA_REG_ENABLE); >> ttm_resource_manager_set_used(man, true); >> } >> - spin_unlock(&dev_priv->svga_lock); >> } >> >> /** >> @@ -1220,14 +1217,12 @@ static void __vmw_svga_disable(struct vmw_private *dev_priv) >> { >> struct ttm_resource_manager *man = ttm_manager_type(&dev_priv->bdev, TTM_PL_VRAM); >> >> - spin_lock(&dev_priv->svga_lock); >> if (ttm_resource_manager_used(man)) { >> ttm_resource_manager_set_used(man, false); >> vmw_write(dev_priv, SVGA_REG_ENABLE, >> SVGA_REG_ENABLE_HIDE | >> SVGA_REG_ENABLE_ENABLE); >> } >> - spin_unlock(&dev_priv->svga_lock); >> } >> >> /** >> @@ -1254,17 +1249,14 @@ void vmw_svga_disable(struct vmw_private *dev_priv) >> */ >> vmw_kms_lost_device(dev_priv->dev); >> ttm_write_lock(&dev_priv->reservation_sem, false); >> - spin_lock(&dev_priv->svga_lock); >> if (ttm_resource_manager_used(man)) { >> ttm_resource_manager_set_used(man, false); >> - spin_unlock(&dev_priv->svga_lock); >> if (ttm_resource_manager_evict_all(&dev_priv->bdev, man)) >> DRM_ERROR("Failed evicting VRAM buffers.\n"); >> vmw_write(dev_priv, SVGA_REG_ENABLE, >> SVGA_REG_ENABLE_HIDE | >> SVGA_REG_ENABLE_ENABLE); >> - } else >> - spin_unlock(&dev_priv->svga_lock); >> + } >> ttm_write_unlock(&dev_priv->reservation_sem); >> } >> >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h >> index 5b9a28157dd3..715f2bfee08a 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h >> @@ -596,7 +596,6 @@ struct vmw_private { >> >> bool stealth; >> bool enable_fb; >> - spinlock_t svga_lock; >> >> /** >> * PM management. >> -- >> 2.29.2 >> > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ffwll.ch%2F&data=04%7C01%7Czackr%40vmware.com%7C87dbfc543ea847f8e13808d8b6d6f273%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637460381607335152%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=TttOwJzt5g8Sk45jqfa9qFkVCQrslhOmdjDyKQJzkIk%3D&reserved=0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] drm/vmwgfx: Drop svga_lock 2021-01-12 8:49 ` [PATCH 1/3] drm/vmwgfx: Drop svga_lock Daniel Vetter 2021-01-14 15:27 ` Zack Rusin @ 2021-01-14 15:31 ` Roland Scheidegger 2021-01-18 13:15 ` Daniel Vetter 1 sibling, 1 reply; 7+ messages in thread From: Roland Scheidegger @ 2021-01-14 15:31 UTC (permalink / raw) To: Daniel Vetter, DRI Development; +Cc: Daniel Vetter, VMware Graphics Hi, looking at it, seems alright. Not sure why the lock was supposedly needed, maybe it was at some point (it seems like all usage of this lock was introduced way back in 2015, commit 153b3d5b037ee). For the series: Reviewed-by: Roland Scheidegger <sroland@vmware.com> Roland Am 12.01.21 um 09:49 schrieb Daniel Vetter: > Hi Roland, > > Hopefully you had a nice start into the new year! Ping for some > review/testing on this series. > > Thanks, Daniel > > On Fri, Dec 11, 2020 at 5:29 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote: >> >> This isn't actually protecting anything becuase: >> - when running, ttm_resource_manager->use_type is protected through >> vmw_private->reservation_semaphore against concurrent execbuf or >> well anything else that might evict or reserve buffers >> - during suspend/resume there's nothing else running, hence >> vmw_pm_freeze and vmw_pm_restore do not need to take the same lock. >> - this also holds for the SVGA_REG_ENABLE register write >> >> Hence it is safe to just remove that spinlock. >> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com> >> Cc: Roland Scheidegger <sroland@vmware.com> >> --- >> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 10 +--------- >> drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 - >> 2 files changed, 1 insertion(+), 10 deletions(-) >> >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c >> index 0008be02d31c..204f7a1830f0 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c >> @@ -672,7 +672,6 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset) >> spin_lock_init(&dev_priv->hw_lock); >> spin_lock_init(&dev_priv->waiter_lock); >> spin_lock_init(&dev_priv->cap_lock); >> - spin_lock_init(&dev_priv->svga_lock); >> spin_lock_init(&dev_priv->cursor_lock); >> >> for (i = vmw_res_context; i < vmw_res_max; ++i) { >> @@ -1189,12 +1188,10 @@ static void __vmw_svga_enable(struct vmw_private *dev_priv) >> { >> struct ttm_resource_manager *man = ttm_manager_type(&dev_priv->bdev, TTM_PL_VRAM); >> >> - spin_lock(&dev_priv->svga_lock); >> if (!ttm_resource_manager_used(man)) { >> vmw_write(dev_priv, SVGA_REG_ENABLE, SVGA_REG_ENABLE); >> ttm_resource_manager_set_used(man, true); >> } >> - spin_unlock(&dev_priv->svga_lock); >> } >> >> /** >> @@ -1220,14 +1217,12 @@ static void __vmw_svga_disable(struct vmw_private *dev_priv) >> { >> struct ttm_resource_manager *man = ttm_manager_type(&dev_priv->bdev, TTM_PL_VRAM); >> >> - spin_lock(&dev_priv->svga_lock); >> if (ttm_resource_manager_used(man)) { >> ttm_resource_manager_set_used(man, false); >> vmw_write(dev_priv, SVGA_REG_ENABLE, >> SVGA_REG_ENABLE_HIDE | >> SVGA_REG_ENABLE_ENABLE); >> } >> - spin_unlock(&dev_priv->svga_lock); >> } >> >> /** >> @@ -1254,17 +1249,14 @@ void vmw_svga_disable(struct vmw_private *dev_priv) >> */ >> vmw_kms_lost_device(dev_priv->dev); >> ttm_write_lock(&dev_priv->reservation_sem, false); >> - spin_lock(&dev_priv->svga_lock); >> if (ttm_resource_manager_used(man)) { >> ttm_resource_manager_set_used(man, false); >> - spin_unlock(&dev_priv->svga_lock); >> if (ttm_resource_manager_evict_all(&dev_priv->bdev, man)) >> DRM_ERROR("Failed evicting VRAM buffers.\n"); >> vmw_write(dev_priv, SVGA_REG_ENABLE, >> SVGA_REG_ENABLE_HIDE | >> SVGA_REG_ENABLE_ENABLE); >> - } else >> - spin_unlock(&dev_priv->svga_lock); >> + } >> ttm_write_unlock(&dev_priv->reservation_sem); >> } >> >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h >> index 5b9a28157dd3..715f2bfee08a 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h >> @@ -596,7 +596,6 @@ struct vmw_private { >> >> bool stealth; >> bool enable_fb; >> - spinlock_t svga_lock; >> >> /** >> * PM management. >> -- >> 2.29.2 >> > > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] drm/vmwgfx: Drop svga_lock 2021-01-14 15:31 ` Roland Scheidegger @ 2021-01-18 13:15 ` Daniel Vetter 0 siblings, 0 replies; 7+ messages in thread From: Daniel Vetter @ 2021-01-18 13:15 UTC (permalink / raw) To: Roland Scheidegger Cc: Daniel Vetter, VMware Graphics, DRI Development, Daniel Vetter On Thu, Jan 14, 2021 at 04:31:09PM +0100, Roland Scheidegger wrote: > Hi, > > looking at it, seems alright. Not sure why the lock was supposedly > needed, maybe it was at some point (it seems like all usage of this lock > was introduced way back in 2015, commit 153b3d5b037ee). > > For the series: Reviewed-by: Roland Scheidegger <sroland@vmware.com> Series merged, thanks for taking a look. -Daniel > > Roland > > Am 12.01.21 um 09:49 schrieb Daniel Vetter: > > Hi Roland, > > > > Hopefully you had a nice start into the new year! Ping for some > > review/testing on this series. > > > > Thanks, Daniel > > > > On Fri, Dec 11, 2020 at 5:29 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > >> > >> This isn't actually protecting anything becuase: > >> - when running, ttm_resource_manager->use_type is protected through > >> vmw_private->reservation_semaphore against concurrent execbuf or > >> well anything else that might evict or reserve buffers > >> - during suspend/resume there's nothing else running, hence > >> vmw_pm_freeze and vmw_pm_restore do not need to take the same lock. > >> - this also holds for the SVGA_REG_ENABLE register write > >> > >> Hence it is safe to just remove that spinlock. > >> > >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > >> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com> > >> Cc: Roland Scheidegger <sroland@vmware.com> > >> --- > >> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 10 +--------- > >> drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 - > >> 2 files changed, 1 insertion(+), 10 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > >> index 0008be02d31c..204f7a1830f0 100644 > >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > >> @@ -672,7 +672,6 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset) > >> spin_lock_init(&dev_priv->hw_lock); > >> spin_lock_init(&dev_priv->waiter_lock); > >> spin_lock_init(&dev_priv->cap_lock); > >> - spin_lock_init(&dev_priv->svga_lock); > >> spin_lock_init(&dev_priv->cursor_lock); > >> > >> for (i = vmw_res_context; i < vmw_res_max; ++i) { > >> @@ -1189,12 +1188,10 @@ static void __vmw_svga_enable(struct vmw_private *dev_priv) > >> { > >> struct ttm_resource_manager *man = ttm_manager_type(&dev_priv->bdev, TTM_PL_VRAM); > >> > >> - spin_lock(&dev_priv->svga_lock); > >> if (!ttm_resource_manager_used(man)) { > >> vmw_write(dev_priv, SVGA_REG_ENABLE, SVGA_REG_ENABLE); > >> ttm_resource_manager_set_used(man, true); > >> } > >> - spin_unlock(&dev_priv->svga_lock); > >> } > >> > >> /** > >> @@ -1220,14 +1217,12 @@ static void __vmw_svga_disable(struct vmw_private *dev_priv) > >> { > >> struct ttm_resource_manager *man = ttm_manager_type(&dev_priv->bdev, TTM_PL_VRAM); > >> > >> - spin_lock(&dev_priv->svga_lock); > >> if (ttm_resource_manager_used(man)) { > >> ttm_resource_manager_set_used(man, false); > >> vmw_write(dev_priv, SVGA_REG_ENABLE, > >> SVGA_REG_ENABLE_HIDE | > >> SVGA_REG_ENABLE_ENABLE); > >> } > >> - spin_unlock(&dev_priv->svga_lock); > >> } > >> > >> /** > >> @@ -1254,17 +1249,14 @@ void vmw_svga_disable(struct vmw_private *dev_priv) > >> */ > >> vmw_kms_lost_device(dev_priv->dev); > >> ttm_write_lock(&dev_priv->reservation_sem, false); > >> - spin_lock(&dev_priv->svga_lock); > >> if (ttm_resource_manager_used(man)) { > >> ttm_resource_manager_set_used(man, false); > >> - spin_unlock(&dev_priv->svga_lock); > >> if (ttm_resource_manager_evict_all(&dev_priv->bdev, man)) > >> DRM_ERROR("Failed evicting VRAM buffers.\n"); > >> vmw_write(dev_priv, SVGA_REG_ENABLE, > >> SVGA_REG_ENABLE_HIDE | > >> SVGA_REG_ENABLE_ENABLE); > >> - } else > >> - spin_unlock(&dev_priv->svga_lock); > >> + } > >> ttm_write_unlock(&dev_priv->reservation_sem); > >> } > >> > >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > >> index 5b9a28157dd3..715f2bfee08a 100644 > >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > >> @@ -596,7 +596,6 @@ struct vmw_private { > >> > >> bool stealth; > >> bool enable_fb; > >> - spinlock_t svga_lock; > >> > >> /** > >> * PM management. > >> -- > >> 2.29.2 > >> > > > > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-01-18 13:15 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-12-11 16:29 [PATCH 1/3] drm/vmwgfx: Drop svga_lock Daniel Vetter 2020-12-11 16:29 ` [PATCH 2/3] drm/vmwgfx: Always evict vram _before_ disabling it Daniel Vetter 2020-12-11 16:29 ` [PATCH 3/3] drm/ttm: WARN_ON non-empty lru when disabling a resource manager Daniel Vetter 2021-01-12 8:49 ` [PATCH 1/3] drm/vmwgfx: Drop svga_lock Daniel Vetter 2021-01-14 15:27 ` Zack Rusin 2021-01-14 15:31 ` Roland Scheidegger 2021-01-18 13:15 ` Daniel Vetter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.