Skip to content

Commit 51fd371

Browse files
robclarkairlied
authored andcommitted
drm: convert crtc and connection_mutex to ww_mutex (v5)
For atomic, it will be quite necessary to not need to care so much about locking order. And 'state' gives us a convenient place to stash a ww_ctx for any sort of update that needs to grab multiple crtc locks. Because we will want to eventually make locking even more fine grained (giving locks to planes, connectors, etc), split out drm_modeset_lock and drm_modeset_acquire_ctx to track acquired locks. Atomic will use this to keep track of which locks have been acquired in a transaction. v1: original v2: remove a few things not needed until atomic, for now v3: update for v3 of connection_mutex patch.. v4: squash in docbook v5: doc tweaks/fixes Signed-off-by: Rob Clark <robdclark@gmail.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Dave Airlie <airlied@redhat.com>
1 parent 4f71d0c commit 51fd371

21 files changed

+531
-87
lines changed

Documentation/DocBook/drm.tmpl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1791,6 +1791,12 @@ void intel_crt_init(struct drm_device *dev)
17911791
<title>KMS API Functions</title>
17921792
!Edrivers/gpu/drm/drm_crtc.c
17931793
</sect2>
1794+
<sect2>
1795+
<title>KMS Locking</title>
1796+
!Pdrivers/gpu/drm/drm_modeset_lock.c kms locking
1797+
!Iinclude/drm/drm_modeset_lock.h
1798+
!Edrivers/gpu/drm/drm_modeset_lock.c
1799+
</sect2>
17941800
</sect1>
17951801

17961802
<!-- Internals: kms helper functions -->

drivers/gpu/drm/Makefile

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ drm-y := drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \
1313
drm_crtc.o drm_modes.o drm_edid.o \
1414
drm_info.o drm_debugfs.o drm_encoder_slave.o \
1515
drm_trace_points.o drm_global.o drm_prime.o \
16-
drm_rect.o drm_vma_manager.o drm_flip_work.o
16+
drm_rect.o drm_vma_manager.o drm_flip_work.o \
17+
drm_modeset_lock.o
1718

1819
drm-$(CONFIG_COMPAT) += drm_ioc32.o
1920
drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o

drivers/gpu/drm/drm_crtc.c

Lines changed: 63 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include <drm/drm_crtc.h>
3838
#include <drm/drm_edid.h>
3939
#include <drm/drm_fourcc.h>
40+
#include <drm/drm_modeset_lock.h>
4041

4142
#include "drm_crtc_internal.h"
4243

@@ -50,14 +51,42 @@
5051
*/
5152
void drm_modeset_lock_all(struct drm_device *dev)
5253
{
53-
struct drm_crtc *crtc;
54+
struct drm_mode_config *config = &dev->mode_config;
55+
struct drm_modeset_acquire_ctx *ctx;
56+
int ret;
5457

55-
mutex_lock(&dev->mode_config.mutex);
58+
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
59+
if (WARN_ON(!ctx))
60+
return;
5661

57-
mutex_lock(&dev->mode_config.connection_mutex);
62+
mutex_lock(&config->mutex);
5863

59-
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
60-
mutex_lock_nest_lock(&crtc->mutex, &dev->mode_config.mutex);
64+
drm_modeset_acquire_init(ctx, 0);
65+
66+
retry:
67+
ret = drm_modeset_lock(&config->connection_mutex, ctx);
68+
if (ret)
69+
goto fail;
70+
ret = drm_modeset_lock_all_crtcs(dev, ctx);
71+
if (ret)
72+
goto fail;
73+
74+
WARN_ON(config->acquire_ctx);
75+
76+
/* now we hold the locks, so now that it is safe, stash the
77+
* ctx for drm_modeset_unlock_all():
78+
*/
79+
config->acquire_ctx = ctx;
80+
81+
drm_warn_on_modeset_not_all_locked(dev);
82+
83+
return;
84+
85+
fail:
86+
if (ret == -EDEADLK) {
87+
drm_modeset_backoff(ctx);
88+
goto retry;
89+
}
6190
}
6291
EXPORT_SYMBOL(drm_modeset_lock_all);
6392

@@ -69,12 +98,17 @@ EXPORT_SYMBOL(drm_modeset_lock_all);
6998
*/
7099
void drm_modeset_unlock_all(struct drm_device *dev)
71100
{
72-
struct drm_crtc *crtc;
101+
struct drm_mode_config *config = &dev->mode_config;
102+
struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx;
73103

74-
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
75-
mutex_unlock(&crtc->mutex);
104+
if (WARN_ON(!ctx))
105+
return;
106+
107+
config->acquire_ctx = NULL;
108+
drm_modeset_drop_locks(ctx);
109+
drm_modeset_acquire_fini(ctx);
76110

77-
mutex_unlock(&dev->mode_config.connection_mutex);
111+
kfree(ctx);
78112

79113
mutex_unlock(&dev->mode_config.mutex);
80114
}
@@ -95,9 +129,9 @@ void drm_warn_on_modeset_not_all_locked(struct drm_device *dev)
95129
return;
96130

97131
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
98-
WARN_ON(!mutex_is_locked(&crtc->mutex));
132+
WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
99133

100-
WARN_ON(!mutex_is_locked(&dev->mode_config.connection_mutex));
134+
WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
101135
WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
102136
}
103137
EXPORT_SYMBOL(drm_warn_on_modeset_not_all_locked);
@@ -671,6 +705,8 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
671705
}
672706
EXPORT_SYMBOL(drm_framebuffer_remove);
673707

708+
DEFINE_WW_CLASS(crtc_ww_class);
709+
674710
/**
675711
* drm_crtc_init_with_planes - Initialise a new CRTC object with
676712
* specified primary and cursor planes.
@@ -690,24 +726,26 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
690726
void *cursor,
691727
const struct drm_crtc_funcs *funcs)
692728
{
729+
struct drm_mode_config *config = &dev->mode_config;
693730
int ret;
694731

695732
crtc->dev = dev;
696733
crtc->funcs = funcs;
697734
crtc->invert_dimensions = false;
698735

699736
drm_modeset_lock_all(dev);
700-
mutex_init(&crtc->mutex);
701-
mutex_lock_nest_lock(&crtc->mutex, &dev->mode_config.mutex);
737+
drm_modeset_lock_init(&crtc->mutex);
738+
/* dropped by _unlock_all(): */
739+
drm_modeset_lock(&crtc->mutex, config->acquire_ctx);
702740

703741
ret = drm_mode_object_get(dev, &crtc->base, DRM_MODE_OBJECT_CRTC);
704742
if (ret)
705743
goto out;
706744

707745
crtc->base.properties = &crtc->properties;
708746

709-
list_add_tail(&crtc->head, &dev->mode_config.crtc_list);
710-
dev->mode_config.num_crtc++;
747+
list_add_tail(&crtc->head, &config->crtc_list);
748+
config->num_crtc++;
711749

712750
crtc->primary = primary;
713751
if (primary)
@@ -735,6 +773,8 @@ void drm_crtc_cleanup(struct drm_crtc *crtc)
735773
kfree(crtc->gamma_store);
736774
crtc->gamma_store = NULL;
737775

776+
drm_modeset_lock_fini(&crtc->mutex);
777+
738778
drm_mode_object_put(dev, &crtc->base);
739779
list_del(&crtc->head);
740780
dev->mode_config.num_crtc--;
@@ -1798,7 +1838,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
17981838
DRM_DEBUG_KMS("[CONNECTOR:%d:?]\n", out_resp->connector_id);
17991839

18001840
mutex_lock(&dev->mode_config.mutex);
1801-
mutex_lock(&dev->mode_config.connection_mutex);
1841+
drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
18021842

18031843
connector = drm_connector_find(dev, out_resp->connector_id);
18041844
if (!connector) {
@@ -1897,7 +1937,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
18971937
out_resp->count_encoders = encoders_count;
18981938

18991939
out:
1900-
mutex_unlock(&dev->mode_config.connection_mutex);
1940+
drm_modeset_unlock(&dev->mode_config.connection_mutex);
19011941
mutex_unlock(&dev->mode_config.mutex);
19021942

19031943
return ret;
@@ -2481,7 +2521,7 @@ static int drm_mode_cursor_common(struct drm_device *dev,
24812521
return -ENOENT;
24822522
}
24832523

2484-
mutex_lock(&crtc->mutex);
2524+
drm_modeset_lock(&crtc->mutex, NULL);
24852525
if (req->flags & DRM_MODE_CURSOR_BO) {
24862526
if (!crtc->funcs->cursor_set && !crtc->funcs->cursor_set2) {
24872527
ret = -ENXIO;
@@ -2505,7 +2545,7 @@ static int drm_mode_cursor_common(struct drm_device *dev,
25052545
}
25062546
}
25072547
out:
2508-
mutex_unlock(&crtc->mutex);
2548+
drm_modeset_unlock(&crtc->mutex);
25092549

25102550
return ret;
25112551

@@ -4198,7 +4238,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
41984238
if (!crtc)
41994239
return -ENOENT;
42004240

4201-
mutex_lock(&crtc->mutex);
4241+
drm_modeset_lock(&crtc->mutex, NULL);
42024242
if (crtc->primary->fb == NULL) {
42034243
/* The framebuffer is currently unbound, presumably
42044244
* due to a hotplug event, that userspace has not
@@ -4282,7 +4322,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
42824322
drm_framebuffer_unreference(fb);
42834323
if (old_fb)
42844324
drm_framebuffer_unreference(old_fb);
4285-
mutex_unlock(&crtc->mutex);
4325+
drm_modeset_unlock(&crtc->mutex);
42864326

42874327
return ret;
42884328
}
@@ -4647,7 +4687,7 @@ EXPORT_SYMBOL(drm_format_vert_chroma_subsampling);
46474687
void drm_mode_config_init(struct drm_device *dev)
46484688
{
46494689
mutex_init(&dev->mode_config.mutex);
4650-
mutex_init(&dev->mode_config.connection_mutex);
4690+
drm_modeset_lock_init(&dev->mode_config.connection_mutex);
46514691
mutex_init(&dev->mode_config.idr_mutex);
46524692
mutex_init(&dev->mode_config.fb_lock);
46534693
INIT_LIST_HEAD(&dev->mode_config.fb_list);
@@ -4747,5 +4787,6 @@ void drm_mode_config_cleanup(struct drm_device *dev)
47474787
}
47484788

47494789
idr_destroy(&dev->mode_config.crtc_idr);
4790+
drm_modeset_lock_fini(&dev->mode_config.connection_mutex);
47504791
}
47514792
EXPORT_SYMBOL(drm_mode_config_cleanup);

drivers/gpu/drm/drm_crtc_helper.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,7 @@ bool drm_helper_encoder_in_use(struct drm_encoder *encoder)
8989
struct drm_device *dev = encoder->dev;
9090

9191
WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
92-
WARN_ON(!mutex_is_locked(&dev->mode_config.connection_mutex));
93-
92+
WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
9493
list_for_each_entry(connector, &dev->mode_config.connector_list, head)
9594
if (connector->encoder == encoder)
9695
return true;

drivers/gpu/drm/drm_fb_helper.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,10 @@ static bool drm_fb_helper_force_kernel_mode(void)
331331
if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
332332
continue;
333333

334+
/* NOTE: we use lockless flag below to avoid grabbing other
335+
* modeset locks. So just trylock the underlying mutex
336+
* directly:
337+
*/
334338
if (!mutex_trylock(&dev->mode_config.mutex)) {
335339
error = true;
336340
continue;

0 commit comments

Comments
 (0)