* [PATCH 01/14] drm: Simplify drm_for_each_legacy_plane arguments
2015-07-09 21:44 [PATCH 00/14] drm_connector locking rules, v2 Daniel Vetter
@ 2015-07-09 21:44 ` Daniel Vetter
2015-07-09 21:44 ` [PATCH 02/14] drm: Add modeset object iterators Daniel Vetter
` (12 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2015-07-09 21:44 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
No need to pass the planelist when everyone just uses
dev->mode_config.plane_list anyway.
I want to add a pile more of iterators with unified (obj, dev)
arguments. This is just prep.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 2 +-
drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 2 +-
include/drm/drm_crtc.h | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6169893abb0f..6c0ce80d91a2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2349,7 +2349,7 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
p->pri.horiz_pixels = intel_crtc->config->pipe_src_w;
p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w;
- drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
+ drm_for_each_legacy_plane(plane, dev) {
struct intel_plane *intel_plane = to_intel_plane(plane);
if (intel_plane->pipe == pipe) {
diff --git a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
index 859ccb658601..e9272b0a8592 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
@@ -248,7 +248,7 @@ static void shmob_drm_crtc_start(struct shmob_drm_crtc *scrtc)
lcdc_write(sdev, LDDDSR, value);
/* Setup planes. */
- drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
+ drm_for_each_legacy_plane(plane, dev) {
if (plane->crtc == crtc)
shmob_drm_plane_setup(plane);
}
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 57ca8cc383a6..5cf0e6c3fc41 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1579,8 +1579,8 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
}
/* Plane list iterator for legacy (overlay only) planes. */
-#define drm_for_each_legacy_plane(plane, planelist) \
- list_for_each_entry(plane, planelist, head) \
+#define drm_for_each_legacy_plane(plane, dev) \
+ list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \
if (plane->type == DRM_PLANE_TYPE_OVERLAY)
#endif /* __DRM_CRTC_H__ */
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 02/14] drm: Add modeset object iterators
2015-07-09 21:44 [PATCH 00/14] drm_connector locking rules, v2 Daniel Vetter
2015-07-09 21:44 ` [PATCH 01/14] drm: Simplify drm_for_each_legacy_plane arguments Daniel Vetter
@ 2015-07-09 21:44 ` Daniel Vetter
2015-07-09 21:44 ` [PATCH 03/14] drm/probe-helper: Grab mode_config.mutex in poll_init/enable Daniel Vetter
` (11 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2015-07-09 21:44 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
And roll them out across drm_* files. The point here isn't code
prettification (it helps with that too) but that some of these lists
aren't static any more. And having macros will gives us a convenient
place to put locking checks into.
I didn't add an iterator for props since that's only used by a
list_for_each_entry_safe in the driver teardown code.
Search&replace was done with the below cocci spatch. Note that there's
a bunch more places that didn't match and which would need some manual
changes, but I've intentially left these out for this mostly automated
patch.
iterator name drm_for_each_crtc;
struct drm_crtc *crtc;
struct drm_device *dev;
expression head;
@@
- list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+ drm_for_each_crtc (crtc, dev) {
...
}
@@
iterator name drm_for_each_encoder;
struct drm_encoder *encoder;
struct drm_device *dev;
expression head;
@@
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+ drm_for_each_encoder (encoder, dev) {
...
}
@@
iterator name drm_for_each_fb;
struct drm_framebuffer *fb;
struct drm_device *dev;
expression head;
@@
- list_for_each_entry(fb, &dev->mode_config.fb_list, head) {
+ drm_for_each_fb (fb, dev) {
...
}
@@
iterator name drm_for_each_connector;
struct drm_connector *connector;
struct drm_device *dev;
expression head;
@@
- list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+ drm_for_each_connector (connector, dev) {
...
}
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/drm_crtc.c | 21 ++++++++-------------
drivers/gpu/drm/drm_crtc_helper.c | 34 +++++++++++++++++-----------------
drivers/gpu/drm/drm_fb_helper.c | 10 +++++-----
drivers/gpu/drm/drm_of.c | 2 +-
drivers/gpu/drm/drm_probe_helper.c | 6 +++---
include/drm/drm_crtc.h | 15 +++++++++++++++
6 files changed, 49 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index a717d18e7a97..ed58f1dbab59 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -615,7 +615,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
if (atomic_read(&fb->refcount.refcount) > 1) {
drm_modeset_lock_all(dev);
/* remove from any CRTC */
- list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+ drm_for_each_crtc(crtc, dev) {
if (crtc->primary->fb == fb) {
/* should turn off the crtc */
memset(&set, 0, sizeof(struct drm_mode_set));
@@ -627,7 +627,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
}
}
- list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
+ drm_for_each_plane(plane, dev) {
if (plane->fb == fb)
drm_plane_force_disable(plane);
}
@@ -1305,7 +1305,7 @@ drm_plane_from_index(struct drm_device *dev, int idx)
struct drm_plane *plane;
unsigned int i = 0;
- list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
+ drm_for_each_plane(plane, dev) {
if (i == idx)
return plane;
i++;
@@ -1838,8 +1838,7 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
copied = 0;
crtc_id = (uint32_t __user *)(unsigned long)card_res->crtc_id_ptr;
if (!mode_group) {
- list_for_each_entry(crtc, &dev->mode_config.crtc_list,
- head) {
+ drm_for_each_crtc(crtc, dev) {
DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id);
if (put_user(crtc->base.id, crtc_id + copied)) {
ret = -EFAULT;
@@ -1865,9 +1864,7 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
copied = 0;
encoder_id = (uint32_t __user *)(unsigned long)card_res->encoder_id_ptr;
if (!mode_group) {
- list_for_each_entry(encoder,
- &dev->mode_config.encoder_list,
- head) {
+ drm_for_each_encoder(encoder, dev) {
DRM_DEBUG_KMS("[ENCODER:%d:%s]\n", encoder->base.id,
encoder->name);
if (put_user(encoder->base.id, encoder_id +
@@ -1896,9 +1893,7 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
copied = 0;
connector_id = (uint32_t __user *)(unsigned long)card_res->connector_id_ptr;
if (!mode_group) {
- list_for_each_entry(connector,
- &dev->mode_config.connector_list,
- head) {
+ drm_for_each_connector(connector, dev) {
DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
connector->base.id,
connector->name);
@@ -2187,7 +2182,7 @@ static struct drm_crtc *drm_encoder_get_crtc(struct drm_encoder *encoder)
/* For atomic drivers only state objects are synchronously updated and
* protected by modeset locks, so check those first. */
- list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+ drm_for_each_connector(connector, dev) {
if (!connector->state)
continue;
@@ -5389,7 +5384,7 @@ void drm_mode_config_reset(struct drm_device *dev)
if (encoder->funcs->reset)
encoder->funcs->reset(encoder);
- list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+ drm_for_each_connector(connector, dev) {
connector->status = connector_status_unknown;
if (connector->funcs->reset)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index d3dfb0ebbeb2..e69a2a502084 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -180,7 +180,7 @@ static void __drm_helper_disable_unused_functions(struct drm_device *dev)
drm_warn_on_modeset_not_all_locked(dev);
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+ drm_for_each_encoder(encoder, dev) {
if (!drm_helper_encoder_in_use(encoder)) {
drm_encoder_disable(encoder);
/* disconnect encoder from any connector */
@@ -188,7 +188,7 @@ static void __drm_helper_disable_unused_functions(struct drm_device *dev)
}
}
- list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+ drm_for_each_crtc(crtc, dev) {
const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
crtc->enabled = drm_helper_crtc_in_use(crtc);
if (!crtc->enabled) {
@@ -230,7 +230,7 @@ drm_crtc_prepare_encoders(struct drm_device *dev)
const struct drm_encoder_helper_funcs *encoder_funcs;
struct drm_encoder *encoder;
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+ drm_for_each_encoder(encoder, dev) {
encoder_funcs = encoder->helper_private;
/* Disable unused encoders */
if (encoder->crtc == NULL)
@@ -305,7 +305,7 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
* adjust it according to limitations or connector properties, and also
* a chance to reject the mode entirely.
*/
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+ drm_for_each_encoder(encoder, dev) {
if (encoder->crtc != crtc)
continue;
@@ -334,7 +334,7 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
crtc->hwmode = *adjusted_mode;
/* Prepare the encoders and CRTCs before setting the mode. */
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+ drm_for_each_encoder(encoder, dev) {
if (encoder->crtc != crtc)
continue;
@@ -359,7 +359,7 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
if (!ret)
goto done;
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+ drm_for_each_encoder(encoder, dev) {
if (encoder->crtc != crtc)
continue;
@@ -376,7 +376,7 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
/* Now enable the clocks, plane, pipe, and connectors that we set up. */
crtc_funcs->commit(crtc);
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+ drm_for_each_encoder(encoder, dev) {
if (encoder->crtc != crtc)
continue;
@@ -418,11 +418,11 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
struct drm_encoder *encoder;
/* Decouple all encoders and their attached connectors from this crtc */
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+ drm_for_each_encoder(encoder, dev) {
if (encoder->crtc != crtc)
continue;
- list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+ drm_for_each_connector(connector, dev) {
if (connector->encoder != encoder)
continue;
@@ -519,12 +519,12 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
* restored, not the drivers personal bookkeeping.
*/
count = 0;
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+ drm_for_each_encoder(encoder, dev) {
save_encoders[count++] = *encoder;
}
count = 0;
- list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+ drm_for_each_connector(connector, dev) {
save_connectors[count++] = *connector;
}
@@ -562,7 +562,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
/* a) traverse passed in connector list and get encoders for them */
count = 0;
- list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+ drm_for_each_connector(connector, dev) {
const struct drm_connector_helper_funcs *connector_funcs =
connector->helper_private;
new_encoder = connector->encoder;
@@ -602,7 +602,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
}
count = 0;
- list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+ drm_for_each_connector(connector, dev) {
if (!connector->encoder)
continue;
@@ -685,12 +685,12 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
fail:
/* Restore all previous data. */
count = 0;
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+ drm_for_each_encoder(encoder, dev) {
*encoder = save_encoders[count++];
}
count = 0;
- list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+ drm_for_each_connector(connector, dev) {
*connector = save_connectors[count++];
}
@@ -862,7 +862,7 @@ void drm_helper_resume_force_mode(struct drm_device *dev)
bool ret;
drm_modeset_lock_all(dev);
- list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+ drm_for_each_crtc(crtc, dev) {
if (!crtc->enabled)
continue;
@@ -876,7 +876,7 @@ void drm_helper_resume_force_mode(struct drm_device *dev)
/* Turn off outputs that were already powered off */
if (drm_helper_choose_crtc_dpms(crtc)) {
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+ drm_for_each_encoder(encoder, dev) {
if(encoder->crtc != crtc)
continue;
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index cac422916c7a..3630d92c9738 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -98,7 +98,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
struct drm_connector *connector;
int i;
- list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+ drm_for_each_connector(connector, dev) {
struct drm_fb_helper_connector *fb_helper_connector;
fb_helper_connector = kzalloc(sizeof(struct drm_fb_helper_connector), GFP_KERNEL);
@@ -269,7 +269,7 @@ static struct drm_framebuffer *drm_mode_config_fb(struct drm_crtc *crtc)
struct drm_device *dev = crtc->dev;
struct drm_crtc *c;
- list_for_each_entry(c, &dev->mode_config.crtc_list, head) {
+ drm_for_each_crtc(c, dev) {
if (crtc->base.id == c->base.id)
return c->primary->fb;
}
@@ -321,7 +321,7 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper)
drm_warn_on_modeset_not_all_locked(dev);
- list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
+ drm_for_each_plane(plane, dev) {
if (plane->type != DRM_PLANE_TYPE_PRIMARY)
drm_plane_force_disable(plane);
@@ -458,7 +458,7 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper)
if (dev->primary->master)
return false;
- list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+ drm_for_each_crtc(crtc, dev) {
if (crtc->primary->fb)
crtcs_bound++;
if (crtc->primary->fb == fb_helper->fb)
@@ -655,7 +655,7 @@ int drm_fb_helper_init(struct drm_device *dev,
}
i = 0;
- list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+ drm_for_each_crtc(crtc, dev) {
fb_helper->crtc_info[i].mode_set.crtc = crtc;
i++;
}
diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index aaa130736bf8..be3884073ea4 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -19,7 +19,7 @@ static uint32_t drm_crtc_port_mask(struct drm_device *dev,
unsigned int index = 0;
struct drm_crtc *tmp;
- list_for_each_entry(tmp, &dev->mode_config.crtc_list, head) {
+ drm_for_each_crtc(tmp, dev) {
if (tmp->port == port)
return 1 << index;
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 7356f3e75e4f..68a33d15be65 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -312,7 +312,7 @@ static void output_poll_execute(struct work_struct *work)
goto out;
mutex_lock(&dev->mode_config.mutex);
- list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+ drm_for_each_connector(connector, dev) {
/* Ignore forced connectors. */
if (connector->force)
@@ -413,7 +413,7 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)
if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll)
return;
- list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+ drm_for_each_connector(connector, dev) {
if (connector->polled & (DRM_CONNECTOR_POLL_CONNECT |
DRM_CONNECTOR_POLL_DISCONNECT))
poll = true;
@@ -495,7 +495,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
return false;
mutex_lock(&dev->mode_config.mutex);
- list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+ drm_for_each_connector(connector, dev) {
/* Only handle HPD capable connectors. */
if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 5cf0e6c3fc41..7c95a7df6065 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1583,4 +1583,19 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \
if (plane->type == DRM_PLANE_TYPE_OVERLAY)
+#define drm_for_each_plane(plane, dev) \
+ list_for_each_entry(plane, &(dev)->mode_config.plane_list, head)
+
+#define drm_for_each_crtc(crtc, dev) \
+ list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head)
+
+#define drm_for_each_connector(connector, dev) \
+ list_for_each_entry(connector, &(dev)->mode_config.connector_list, head)
+
+#define drm_for_each_encoder(encoder, dev) \
+ list_for_each_entry(encoder, &(dev)->mode_config.encoder_list, head)
+
+#define drm_for_each_fb(fb, dev) \
+ list_for_each_entry(fb, &(dev)->mode_config.fb_list, head)
+
#endif /* __DRM_CRTC_H__ */
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 03/14] drm/probe-helper: Grab mode_config.mutex in poll_init/enable
2015-07-09 21:44 [PATCH 00/14] drm_connector locking rules, v2 Daniel Vetter
2015-07-09 21:44 ` [PATCH 01/14] drm: Simplify drm_for_each_legacy_plane arguments Daniel Vetter
2015-07-09 21:44 ` [PATCH 02/14] drm: Add modeset object iterators Daniel Vetter
@ 2015-07-09 21:44 ` Daniel Vetter
2015-07-09 21:44 ` [PATCH 04/14] drm/fbdev-helper: Grab mode_config.mutex in drm_fb_helper_single_add_all_connectors Daniel Vetter
` (10 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2015-07-09 21:44 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
So on first looks this seems superflous since drivers should ensure
correct ordering to not make this a problem. Otoh ordering constraints
between hdp, fbdev load and enabling polling are already tricky on
some hardware and it helps to be more robust.
But the real goal is to just shut up a locking WARN_ON I'd like to
add, which means init code gets some additional locks just for
uniformity.
v2: Also grab the lock for the public poll_enable, not just poll_init
which is used for resume, with the same justification.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/drm_probe_helper.c | 41 +++++++++++++++++++++++---------------
1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 68a33d15be65..ba94c1ddab9f 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -93,6 +93,27 @@ static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
return 1;
}
+#define DRM_OUTPUT_POLL_PERIOD (10*HZ)
+static void __drm_kms_helper_poll_enable(struct drm_device *dev)
+{
+ bool poll = false;
+ struct drm_connector *connector;
+
+ WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
+
+ if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll)
+ return;
+
+ drm_for_each_connector(connector, dev) {
+ if (connector->polled & (DRM_CONNECTOR_POLL_CONNECT |
+ DRM_CONNECTOR_POLL_DISCONNECT))
+ poll = true;
+ }
+
+ if (poll)
+ schedule_delayed_work(&dev->mode_config.output_poll_work, DRM_OUTPUT_POLL_PERIOD);
+}
+
static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connector *connector,
uint32_t maxX, uint32_t maxY, bool merge_type_bits)
{
@@ -153,7 +174,7 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
/* Re-enable polling in case the global poll config changed. */
if (drm_kms_helper_poll != dev->mode_config.poll_running)
- drm_kms_helper_poll_enable(dev);
+ __drm_kms_helper_poll_enable(dev);
dev->mode_config.poll_running = drm_kms_helper_poll;
@@ -295,7 +316,6 @@ void drm_kms_helper_hotplug_event(struct drm_device *dev)
}
EXPORT_SYMBOL(drm_kms_helper_hotplug_event);
-#define DRM_OUTPUT_POLL_PERIOD (10*HZ)
static void output_poll_execute(struct work_struct *work)
{
struct delayed_work *delayed_work = to_delayed_work(work);
@@ -407,20 +427,9 @@ EXPORT_SYMBOL(drm_kms_helper_poll_disable);
*/
void drm_kms_helper_poll_enable(struct drm_device *dev)
{
- bool poll = false;
- struct drm_connector *connector;
-
- if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll)
- return;
-
- drm_for_each_connector(connector, dev) {
- if (connector->polled & (DRM_CONNECTOR_POLL_CONNECT |
- DRM_CONNECTOR_POLL_DISCONNECT))
- poll = true;
- }
-
- if (poll)
- schedule_delayed_work(&dev->mode_config.output_poll_work, DRM_OUTPUT_POLL_PERIOD);
+ mutex_lock(&dev->mode_config.mutex);
+ __drm_kms_helper_poll_enable(dev);
+ mutex_unlock(&dev->mode_config.mutex);
}
EXPORT_SYMBOL(drm_kms_helper_poll_enable);
--
2.1.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 04/14] drm/fbdev-helper: Grab mode_config.mutex in drm_fb_helper_single_add_all_connectors
2015-07-09 21:44 [PATCH 00/14] drm_connector locking rules, v2 Daniel Vetter
` (2 preceding siblings ...)
2015-07-09 21:44 ` [PATCH 03/14] drm/probe-helper: Grab mode_config.mutex in poll_init/enable Daniel Vetter
@ 2015-07-09 21:44 ` Daniel Vetter
2015-07-09 21:44 ` [PATCH 05/14] drm: Check locking in drm_for_each_connector Daniel Vetter
` (9 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2015-07-09 21:44 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
This is now truly only duct-tape to keep locking checks happy since
calling this function when hpd or polling are already enabled is a
bug. The fbdev helper can't cope with hotplug changes yet at this
point, only after that.
Otoh a bit more robustness in this function can't hurt, and with this
fbdev can actually cope with hotplug changes. And it's also more
consistent with the connector hotadd/remove dp mst needs to do.
Therefore document this as new official behavior.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/drm_fb_helper.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 3630d92c9738..329d08167b77 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -89,8 +89,9 @@ static LIST_HEAD(kernel_fb_helper_list);
* connectors to the fbdev, e.g. if some are reserved for special purposes or
* not adequate to be used for the fbcon.
*
- * Since this is part of the initial setup before the fbdev is published, no
- * locking is required.
+ * This function is protected against concurrent connector hotadds/removals
+ * using drm_fb_helper_add_one_connector() and
+ * drm_fb_helper_remove_one_connector().
*/
int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
{
@@ -98,6 +99,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
struct drm_connector *connector;
int i;
+ mutex_lock(&dev->mode_config.mutex);
drm_for_each_connector(connector, dev) {
struct drm_fb_helper_connector *fb_helper_connector;
@@ -108,6 +110,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
fb_helper_connector->connector = connector;
fb_helper->connector_info[fb_helper->connector_count++] = fb_helper_connector;
}
+ mutex_unlock(&dev->mode_config.mutex);
return 0;
fail:
for (i = 0; i < fb_helper->connector_count; i++) {
@@ -115,6 +118,8 @@ fail:
fb_helper->connector_info[i] = NULL;
}
fb_helper->connector_count = 0;
+ mutex_unlock(&dev->mode_config.mutex);
+
return -ENOMEM;
}
EXPORT_SYMBOL(drm_fb_helper_single_add_all_connectors);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 05/14] drm: Check locking in drm_for_each_connector
2015-07-09 21:44 [PATCH 00/14] drm_connector locking rules, v2 Daniel Vetter
` (3 preceding siblings ...)
2015-07-09 21:44 ` [PATCH 04/14] drm/fbdev-helper: Grab mode_config.mutex in drm_fb_helper_single_add_all_connectors Daniel Vetter
@ 2015-07-09 21:44 ` Daniel Vetter
2015-07-28 22:40 ` Laurent Pinchart
2015-07-09 21:44 ` [PATCH 06/14] drm/i915: Use drm_for_each_fb in i915_debugfs.c Daniel Vetter
` (8 subsequent siblings)
13 siblings, 1 reply; 20+ messages in thread
From: Daniel Vetter @ 2015-07-09 21:44 UTC (permalink / raw)
To: DRI Development
Cc: Daniel Vetter, Daniel Vetter, Intel Graphics Development,
Dave Airlie
Because of DP MST connectors can now be hotplugged and we must hold
the right lock when walking the connector lists. Enforce this by
checking the locking in our shiny new list walking macros.
v2: Extract the locking check into a small static inline helper to
help readability. This will be more important when we make the
read list access rules more complicated in later patches. Inspired by
comments from Chris. Unfortunately, due to header loops around the
definition of struct drm_device the function interface is a bit funny.
v3: Encoders aren't hotadded/removed. For each dp mst encoder we
statically create one fake encoder per pipe so that we can support as
many mst sinks as the hw can (Dave).
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
include/drm/drm_crtc.h | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 7c95a7df6065..499562274353 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1589,8 +1589,18 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
#define drm_for_each_crtc(crtc, dev) \
list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head)
+static inline void
+assert_drm_connector_list_read_locked(struct drm_mode_config *mode_config)
+{
+ WARN_ON(!mutex_is_locked(&mode_config->mutex));
+}
+
#define drm_for_each_connector(connector, dev) \
- list_for_each_entry(connector, &(dev)->mode_config.connector_list, head)
+ for (assert_drm_connector_list_read_locked(&(dev)->mode_config), \
+ connector = list_first_entry(&(dev)->mode_config.connector_list, \
+ struct drm_connector, head); \
+ &connector->head != (&(dev)->mode_config.connector_list); \
+ connector = list_next_entry(connector, head))
#define drm_for_each_encoder(encoder, dev) \
list_for_each_entry(encoder, &(dev)->mode_config.encoder_list, head)
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 05/14] drm: Check locking in drm_for_each_connector
2015-07-09 21:44 ` [PATCH 05/14] drm: Check locking in drm_for_each_connector Daniel Vetter
@ 2015-07-28 22:40 ` Laurent Pinchart
0 siblings, 0 replies; 20+ messages in thread
From: Laurent Pinchart @ 2015-07-28 22:40 UTC (permalink / raw)
To: dri-devel
Cc: Daniel Vetter, Intel Graphics Development, Dave Airlie,
Daniel Vetter
Hi Daniel,
On Thursday 09 July 2015 23:44:28 Daniel Vetter wrote:
> Because of DP MST connectors can now be hotplugged and we must hold
> the right lock when walking the connector lists. Enforce this by
> checking the locking in our shiny new list walking macros.
>
> v2: Extract the locking check into a small static inline helper to
> help readability. This will be more important when we make the
> read list access rules more complicated in later patches. Inspired by
> comments from Chris. Unfortunately, due to header loops around the
> definition of struct drm_device the function interface is a bit funny.
>
> v3: Encoders aren't hotadded/removed. For each dp mst encoder we
> statically create one fake encoder per pipe so that we can support as
> many mst sinks as the hw can (Dave).
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> include/drm/drm_crtc.h | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 7c95a7df6065..499562274353 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1589,8 +1589,18 @@ static inline struct drm_property
> *drm_property_find(struct drm_device *dev, #define drm_for_each_crtc(crtc,
> dev) \
> list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head)
>
> +static inline void
> +assert_drm_connector_list_read_locked(struct drm_mode_config *mode_config)
> +{
> + WARN_ON(!mutex_is_locked(&mode_config->mutex));
I believe this introduced a regression for all drivers that call
drm_mode_config_reset() at .load() time (and there's lots of them), as the
mode config mutex isn't locked then.
> +}
> +
> #define drm_for_each_connector(connector, dev) \
> - list_for_each_entry(connector, &(dev)->mode_config.connector_list, head)
> + for (assert_drm_connector_list_read_locked(&(dev)->mode_config), \
> + connector = list_first_entry(&(dev)->mode_config.connector_list,
\
> + struct drm_connector, head); \
> + &connector->head != (&(dev)->mode_config.connector_list); \
> + connector = list_next_entry(connector, head))
>
> #define drm_for_each_encoder(encoder, dev) \
> list_for_each_entry(encoder, &(dev)->mode_config.encoder_list, head)
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 06/14] drm/i915: Use drm_for_each_fb in i915_debugfs.c
2015-07-09 21:44 [PATCH 00/14] drm_connector locking rules, v2 Daniel Vetter
` (4 preceding siblings ...)
2015-07-09 21:44 ` [PATCH 05/14] drm: Check locking in drm_for_each_connector Daniel Vetter
@ 2015-07-09 21:44 ` Daniel Vetter
2015-07-10 7:53 ` [Intel-gfx] " Ville Syrjälä
2015-07-10 17:02 ` [PATCH] " Daniel Vetter
2015-07-09 21:44 ` [PATCH 07/14] drm: Check locking in drm_for_each_fb Daniel Vetter
` (7 subsequent siblings)
13 siblings, 2 replies; 20+ messages in thread
From: Daniel Vetter @ 2015-07-09 21:44 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
Just so I have a user for this macro.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 053adbb97037..60eebf5f6da8 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1855,6 +1855,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
struct drm_device *dev = node->minor->dev;
struct intel_fbdev *ifbdev = NULL;
struct intel_framebuffer *fb;
+ struct drm_framebuffer *drm_fb;
#ifdef CONFIG_DRM_I915_FBDEV
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1874,7 +1875,8 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
#endif
mutex_lock(&dev->mode_config.fb_lock);
- list_for_each_entry(fb, &dev->mode_config.fb_list, base.head) {
+ drm_for_each_plane(drm_fb, dev) {
+ fb = to_intel_framebuffer(drm_fb);
if (ifbdev && &fb->base == ifbdev->helper.fb)
continue;
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Intel-gfx] [PATCH 06/14] drm/i915: Use drm_for_each_fb in i915_debugfs.c
2015-07-09 21:44 ` [PATCH 06/14] drm/i915: Use drm_for_each_fb in i915_debugfs.c Daniel Vetter
@ 2015-07-10 7:53 ` Ville Syrjälä
2015-07-10 17:02 ` [PATCH] " Daniel Vetter
1 sibling, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2015-07-10 7:53 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, DRI Development
On Thu, Jul 09, 2015 at 11:44:29PM +0200, Daniel Vetter wrote:
> Just so I have a user for this macro.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 053adbb97037..60eebf5f6da8 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1855,6 +1855,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
> struct drm_device *dev = node->minor->dev;
> struct intel_fbdev *ifbdev = NULL;
> struct intel_framebuffer *fb;
> + struct drm_framebuffer *drm_fb;
>
> #ifdef CONFIG_DRM_I915_FBDEV
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1874,7 +1875,8 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
> #endif
>
> mutex_lock(&dev->mode_config.fb_lock);
> - list_for_each_entry(fb, &dev->mode_config.fb_list, base.head) {
> + drm_for_each_plane(drm_fb, dev) {
^^^^^
That's no fb.
> + fb = to_intel_framebuffer(drm_fb);
> if (ifbdev && &fb->base == ifbdev->helper.fb)
> continue;
>
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH] drm/i915: Use drm_for_each_fb in i915_debugfs.c
2015-07-09 21:44 ` [PATCH 06/14] drm/i915: Use drm_for_each_fb in i915_debugfs.c Daniel Vetter
2015-07-10 7:53 ` [Intel-gfx] " Ville Syrjälä
@ 2015-07-10 17:02 ` Daniel Vetter
1 sibling, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2015-07-10 17:02 UTC (permalink / raw)
To: DRI Development, Intel Graphics Development; +Cc: Daniel Vetter, Daniel Vetter
Just so I have a user for this macro.
v2: Use the right macro - somehow I thought gcc should scream at me,
but list_for_each isn't really typesafe unfortunately. Spotted by
Ville.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 053adbb97037..ab8364984a89 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1855,6 +1855,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
struct drm_device *dev = node->minor->dev;
struct intel_fbdev *ifbdev = NULL;
struct intel_framebuffer *fb;
+ struct drm_framebuffer *drm_fb;
#ifdef CONFIG_DRM_I915_FBDEV
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1874,7 +1875,8 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
#endif
mutex_lock(&dev->mode_config.fb_lock);
- list_for_each_entry(fb, &dev->mode_config.fb_list, base.head) {
+ drm_for_each_fb(drm_fb, dev) {
+ fb = to_intel_framebuffer(drm_fb);
if (ifbdev && &fb->base == ifbdev->helper.fb)
continue;
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 07/14] drm: Check locking in drm_for_each_fb
2015-07-09 21:44 [PATCH 00/14] drm_connector locking rules, v2 Daniel Vetter
` (5 preceding siblings ...)
2015-07-09 21:44 ` [PATCH 06/14] drm/i915: Use drm_for_each_fb in i915_debugfs.c Daniel Vetter
@ 2015-07-09 21:44 ` Daniel Vetter
2015-07-09 21:44 ` [PATCH 08/14] drm/i915: Take all modeset locks for DP MST hotplug Daniel Vetter
` (6 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2015-07-09 21:44 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
Ever since framebuffers are reference counted we have a special lock
for the global fb list. Make sure users of that list do hold that
lock when using the new iterators.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
include/drm/drm_crtc.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 499562274353..10547be5684a 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1606,6 +1606,10 @@ assert_drm_connector_list_read_locked(struct drm_mode_config *mode_config)
list_for_each_entry(encoder, &(dev)->mode_config.encoder_list, head)
#define drm_for_each_fb(fb, dev) \
- list_for_each_entry(fb, &(dev)->mode_config.fb_list, head)
+ for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.fb_lock)), \
+ fb = list_first_entry(&(dev)->mode_config.fb_list, \
+ struct drm_framebuffer, head); \
+ &fb->head != (&(dev)->mode_config.fb_list); \
+ fb = list_next_entry(fb, head))
#endif /* __DRM_CRTC_H__ */
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 08/14] drm/i915: Take all modeset locks for DP MST hotplug
2015-07-09 21:44 [PATCH 00/14] drm_connector locking rules, v2 Daniel Vetter
` (6 preceding siblings ...)
2015-07-09 21:44 ` [PATCH 07/14] drm: Check locking in drm_for_each_fb Daniel Vetter
@ 2015-07-09 21:44 ` Daniel Vetter
2015-07-09 21:44 ` [PATCH 09/14] drm/radeon: " Daniel Vetter
` (5 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2015-07-09 21:44 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
While auditing various users of the connector/encoder lists I realized
that the atomic code is a very prolific user of them. And it only ever
grabs the mode_config->connection_mutex, but not the
mode_config->mutex like all the other code walking encoder/connector
lists.
The problem is that we can't grab the mode_config.mutex late in atomic
code since that would lead to locking inversions. And we don't want to
grab it unconditionally like the legacy set_config modeset path since
that would render all the fine-grained locking moot.
Instead just grab more locks in the dp mst hotplug code. Note that
drm_connector_init (which is the one adding the connector to these
lists) already uses drm_modeset_lock_all.
The other reason for grabbing all locks is that the dpms off in the
unplug function amounts to a modeset, so better to take all required
locks for that.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/i915/intel_dp_mst.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 6e4cc5334f47..d0b2569c6241 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -442,9 +442,9 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
drm_mode_connector_set_path_property(connector, pathprop);
drm_reinit_primary_mode_group(dev);
- mutex_lock(&dev->mode_config.mutex);
+ drm_modeset_lock_all(dev);
intel_connector_add_to_fbdev(intel_connector);
- mutex_unlock(&dev->mode_config.mutex);
+ drm_modeset_unlock_all(dev);
drm_connector_register(&intel_connector->base);
return connector;
}
@@ -455,16 +455,16 @@ static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
struct intel_connector *intel_connector = to_intel_connector(connector);
struct drm_device *dev = connector->dev;
/* need to nuke the connector */
- mutex_lock(&dev->mode_config.mutex);
+ drm_modeset_lock_all(dev);
intel_connector_dpms(connector, DRM_MODE_DPMS_OFF);
- mutex_unlock(&dev->mode_config.mutex);
+ drm_modeset_unlock_all(dev);
intel_connector->unregister(intel_connector);
- mutex_lock(&dev->mode_config.mutex);
+ drm_modeset_lock_all(dev);
intel_connector_remove_from_fbdev(intel_connector);
drm_connector_cleanup(connector);
- mutex_unlock(&dev->mode_config.mutex);
+ drm_modeset_unlock_all(dev);
drm_reinit_primary_mode_group(dev);
--
2.1.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 09/14] drm/radeon: Take all modeset locks for DP MST hotplug
2015-07-09 21:44 [PATCH 00/14] drm_connector locking rules, v2 Daniel Vetter
` (7 preceding siblings ...)
2015-07-09 21:44 ` [PATCH 08/14] drm/i915: Take all modeset locks for DP MST hotplug Daniel Vetter
@ 2015-07-09 21:44 ` Daniel Vetter
2015-07-09 21:44 ` [PATCH 10/14] drm: Amend connector list locking rules Daniel Vetter
` (4 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2015-07-09 21:44 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
Similar with the i915 take all modeset locks for mst hotplug. This is
needed to make sure radeon holds both mode_config.mutex and
mode_config.connection_mutex when updating the connector_list, which
is the new (interim) locking regime we want for that.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/radeon/radeon_dp_mst.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c
index 257b10be5cda..e649c8ff20a0 100644
--- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
@@ -286,9 +286,9 @@ static struct drm_connector *radeon_dp_add_mst_connector(struct drm_dp_mst_topol
drm_mode_connector_set_path_property(connector, pathprop);
drm_reinit_primary_mode_group(dev);
- mutex_lock(&dev->mode_config.mutex);
+ drm_modeset_lock_all(dev);
radeon_fb_add_connector(rdev, connector);
- mutex_unlock(&dev->mode_config.mutex);
+ drm_modeset_unlock_all(dev);
drm_connector_register(connector);
return connector;
@@ -303,12 +303,12 @@ static void radeon_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
drm_connector_unregister(connector);
/* need to nuke the connector */
- mutex_lock(&dev->mode_config.mutex);
+ drm_modeset_lock_all(dev);
/* dpms off */
radeon_fb_remove_connector(rdev, connector);
drm_connector_cleanup(connector);
- mutex_unlock(&dev->mode_config.mutex);
+ drm_modeset_unlock_all(dev);
drm_reinit_primary_mode_group(dev);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 10/14] drm: Amend connector list locking rules
2015-07-09 21:44 [PATCH 00/14] drm_connector locking rules, v2 Daniel Vetter
` (8 preceding siblings ...)
2015-07-09 21:44 ` [PATCH 09/14] drm/radeon: " Daniel Vetter
@ 2015-07-09 21:44 ` Daniel Vetter
2015-07-09 21:44 ` [PATCH 11/14] drm: Roll out drm_for_each_connector more Daniel Vetter
` (3 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2015-07-09 21:44 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Daniel Vetter, Intel Graphics Development
Now that dp mst hotplug takes all locks we can amend the locking rules
for the iterators. This is needed before we can roll these out in the
atomic code to avoid getting burried in WARNINGs.
v2: Rebase onto the extracted list locking assert and add a comment to
explain the rules.
v3: Fixup German->English translation fail in the comment.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
include/drm/drm_crtc.h | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 10547be5684a..fe3100115a41 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1592,7 +1592,15 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
static inline void
assert_drm_connector_list_read_locked(struct drm_mode_config *mode_config)
{
- WARN_ON(!mutex_is_locked(&mode_config->mutex));
+ /*
+ * The connector hotadd/remove code currently grabs both locks when
+ * updating lists. Hence readers need only hold either of them to be
+ * safe and the check amounts to
+ *
+ * WARN_ON(not_holding(A) && not_holding(B)).
+ */
+ WARN_ON(!mutex_is_locked(&mode_config->mutex) &&
+ !drm_modeset_is_locked(&mode_config->connection_mutex));
}
#define drm_for_each_connector(connector, dev) \
--
2.1.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 11/14] drm: Roll out drm_for_each_connector more
2015-07-09 21:44 [PATCH 00/14] drm_connector locking rules, v2 Daniel Vetter
` (9 preceding siblings ...)
2015-07-09 21:44 ` [PATCH 10/14] drm: Amend connector list locking rules Daniel Vetter
@ 2015-07-09 21:44 ` Daniel Vetter
2015-07-09 21:44 ` [PATCH 12/14] drm: Roll out drm_for_each_{plane,crtc,encoder} Daniel Vetter
` (2 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2015-07-09 21:44 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
Now that we also grab the connection_mutex and so fixed the race with
atomic modeset we can use the iterator there too.
The other special case is drm_connector_unplug_all which would have a
locking inversion with the sysfs store/show functions if we'd grab the
mode_config.mutex around the unplug. We could just grab
connection_mutex instead, but that's a bit too much a dirty trick for
my taste. Also it's only used by udl, which doesn't do any other kind
of connector hotplugging, so should be race-free. Hence just stick
with a comment for now.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/drm_atomic.c | 2 +-
drivers/gpu/drm/drm_atomic_helper.c | 4 ++--
drivers/gpu/drm/drm_crtc.c | 9 +++++----
drivers/gpu/drm/drm_crtc_helper.c | 6 +++---
drivers/gpu/drm/drm_edid.c | 2 +-
drivers/gpu/drm/drm_plane_helper.c | 3 ++-
6 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index acebd1617264..3efd91c0c6cb 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1063,7 +1063,7 @@ drm_atomic_add_affected_connectors(struct drm_atomic_state *state,
* Changed connectors are already in @state, so only need to look at the
* current configuration.
*/
- list_for_each_entry(connector, &config->connector_list, head) {
+ drm_for_each_connector(connector, state->dev) {
if (connector->state->crtc != crtc)
continue;
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 0898afbc9e23..91ad6bd13734 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -89,7 +89,7 @@ get_current_crtc_for_encoder(struct drm_device *dev,
WARN_ON(!drm_modeset_is_locked(&config->connection_mutex));
- list_for_each_entry(connector, &config->connector_list, head) {
+ drm_for_each_connector(connector, dev) {
if (connector->state->best_encoder != encoder)
continue;
@@ -1982,7 +1982,7 @@ retry:
WARN_ON(!drm_modeset_is_locked(&config->connection_mutex));
- list_for_each_entry(tmp_connector, &config->connector_list, head) {
+ drm_for_each_connector(tmp_connector, connector->dev) {
if (tmp_connector->state->crtc != crtc)
continue;
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index ed58f1dbab59..b3e992260eab 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -988,7 +988,7 @@ unsigned int drm_connector_index(struct drm_connector *connector)
WARN_ON(!drm_modeset_is_locked(&config->connection_mutex));
- list_for_each_entry(tmp, &connector->dev->mode_config.connector_list, head) {
+ drm_for_each_connector(tmp, connector->dev) {
if (tmp == connector)
return index;
@@ -1054,7 +1054,7 @@ void drm_connector_unplug_all(struct drm_device *dev)
{
struct drm_connector *connector;
- /* taking the mode config mutex ends up in a clash with sysfs */
+ /* FIXME: taking the mode config mutex ends up in a clash with sysfs */
list_for_each_entry(connector, &dev->mode_config.connector_list, head)
drm_connector_unregister(connector);
@@ -1726,7 +1726,7 @@ int drm_mode_group_init_legacy_group(struct drm_device *dev,
group->id_list[group->num_crtcs + group->num_encoders++] =
encoder->base.id;
- list_for_each_entry(connector, &dev->mode_config.connector_list, head)
+ drm_for_each_connector(connector, dev)
group->id_list[group->num_crtcs + group->num_encoders +
group->num_connectors++] = connector->base.id;
@@ -1810,12 +1810,13 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
* connector hot-adding. CRTC/Plane lists are invariant. */
mutex_lock(&dev->mode_config.mutex);
if (!drm_is_primary_client(file_priv)) {
+ struct drm_connector *connector;
mode_group = NULL;
list_for_each(lh, &dev->mode_config.crtc_list)
crtc_count++;
- list_for_each(lh, &dev->mode_config.connector_list)
+ drm_for_each_connector(connector, dev)
connector_count++;
list_for_each(lh, &dev->mode_config.encoder_list)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index e69a2a502084..228eda29b14f 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -121,7 +121,7 @@ bool drm_helper_encoder_in_use(struct drm_encoder *encoder)
WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
}
- list_for_each_entry(connector, &dev->mode_config.connector_list, head)
+ drm_for_each_connector(connector, dev)
if (connector->encoder == encoder)
return true;
return false;
@@ -712,7 +712,7 @@ static int drm_helper_choose_encoder_dpms(struct drm_encoder *encoder)
struct drm_connector *connector;
struct drm_device *dev = encoder->dev;
- list_for_each_entry(connector, &dev->mode_config.connector_list, head)
+ drm_for_each_connector(connector, dev)
if (connector->encoder == encoder)
if (connector->dpms < dpms)
dpms = connector->dpms;
@@ -746,7 +746,7 @@ static int drm_helper_choose_crtc_dpms(struct drm_crtc *crtc)
struct drm_connector *connector;
struct drm_device *dev = crtc->dev;
- list_for_each_entry(connector, &dev->mode_config.connector_list, head)
+ drm_for_each_connector(connector, dev)
if (connector->encoder && connector->encoder->crtc == crtc)
if (connector->dpms < dpms)
dpms = connector->dpms;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index baf1a1b58f19..4a403eb90ded 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3421,7 +3421,7 @@ struct drm_connector *drm_select_eld(struct drm_encoder *encoder,
WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
- list_for_each_entry(connector, &dev->mode_config.connector_list, head)
+ drm_for_each_connector(connector, dev)
if (connector->encoder == encoder && connector->eld[0])
return connector;
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index b07a213f5655..46c704573306 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -91,13 +91,14 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
*/
WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
- list_for_each_entry(connector, &dev->mode_config.connector_list, head)
+ drm_for_each_connector(connector, dev) {
if (connector->encoder && connector->encoder->crtc == crtc) {
if (connector_list != NULL && count < num_connectors)
*(connector_list++) = connector;
count++;
}
+ }
return count;
}
--
2.1.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 12/14] drm: Roll out drm_for_each_{plane,crtc,encoder}
2015-07-09 21:44 [PATCH 00/14] drm_connector locking rules, v2 Daniel Vetter
` (10 preceding siblings ...)
2015-07-09 21:44 ` [PATCH 11/14] drm: Roll out drm_for_each_connector more Daniel Vetter
@ 2015-07-09 21:44 ` Daniel Vetter
2015-07-09 21:44 ` [PATCH 13/14] drm: Stop filtering according to mode_group in getresources Daniel Vetter
2015-07-09 21:44 ` [PATCH 14/14] drm: gc now dead mode_group code Daniel Vetter
13 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2015-07-09 21:44 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
Remaining manual work in the drm core&helpers. Nothing special here,
no surprises.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/drm_crtc.c | 26 ++++++++++++++------------
drivers/gpu/drm/drm_crtc_helper.c | 2 +-
drivers/gpu/drm/drm_fb_cma_helper.c | 2 +-
drivers/gpu/drm/drm_modeset_lock.c | 7 +++----
4 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index b3e992260eab..9b91ef060c76 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -736,7 +736,7 @@ unsigned int drm_crtc_index(struct drm_crtc *crtc)
unsigned int index = 0;
struct drm_crtc *tmp;
- list_for_each_entry(tmp, &crtc->dev->mode_config.crtc_list, head) {
+ drm_for_each_crtc(tmp, crtc->dev) {
if (tmp == crtc)
return index;
@@ -1280,7 +1280,7 @@ unsigned int drm_plane_index(struct drm_plane *plane)
unsigned int index = 0;
struct drm_plane *tmp;
- list_for_each_entry(tmp, &plane->dev->mode_config.plane_list, head) {
+ drm_for_each_plane(tmp, plane->dev) {
if (tmp == plane)
return index;
@@ -1719,10 +1719,10 @@ int drm_mode_group_init_legacy_group(struct drm_device *dev,
if (ret)
return ret;
- list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
+ drm_for_each_crtc(crtc, dev)
group->id_list[group->num_crtcs++] = crtc->base.id;
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
+ drm_for_each_encoder(encoder, dev)
group->id_list[group->num_crtcs + group->num_encoders++] =
encoder->base.id;
@@ -1811,15 +1811,17 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
mutex_lock(&dev->mode_config.mutex);
if (!drm_is_primary_client(file_priv)) {
struct drm_connector *connector;
+ struct drm_encoder *encoder;
+ struct drm_crtc *crtc;
mode_group = NULL;
- list_for_each(lh, &dev->mode_config.crtc_list)
+ drm_for_each_crtc(crtc, dev)
crtc_count++;
drm_for_each_connector(connector, dev)
connector_count++;
- list_for_each(lh, &dev->mode_config.encoder_list)
+ drm_for_each_encoder(encoder, dev)
encoder_count++;
} else {
@@ -2287,7 +2289,7 @@ int drm_mode_getplane_res(struct drm_device *dev, void *data,
plane_ptr = (uint32_t __user *)(unsigned long)plane_resp->plane_id_ptr;
/* Plane lists are invariant, no locking needed. */
- list_for_each_entry(plane, &config->plane_list, head) {
+ drm_for_each_plane(plane, dev) {
/*
* Unless userspace set the 'universal planes'
* capability bit, only advertise overlays.
@@ -2592,7 +2594,7 @@ int drm_mode_set_config_internal(struct drm_mode_set *set)
* connectors from it), hence we need to refcount the fbs across all
* crtcs. Atomic modeset will have saner semantics ...
*/
- list_for_each_entry(tmp, &crtc->dev->mode_config.crtc_list, head)
+ drm_for_each_crtc(tmp, crtc->dev)
tmp->primary->old_fb = tmp->primary->fb;
fb = set->fb;
@@ -2603,7 +2605,7 @@ int drm_mode_set_config_internal(struct drm_mode_set *set)
crtc->primary->fb = fb;
}
- list_for_each_entry(tmp, &crtc->dev->mode_config.crtc_list, head) {
+ drm_for_each_crtc(tmp, crtc->dev) {
if (tmp->primary->fb)
drm_framebuffer_reference(tmp->primary->fb);
if (tmp->primary->old_fb)
@@ -5373,15 +5375,15 @@ void drm_mode_config_reset(struct drm_device *dev)
struct drm_encoder *encoder;
struct drm_connector *connector;
- list_for_each_entry(plane, &dev->mode_config.plane_list, head)
+ drm_for_each_plane(plane, dev)
if (plane->funcs->reset)
plane->funcs->reset(plane);
- list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
+ drm_for_each_crtc(crtc, dev)
if (crtc->funcs->reset)
crtc->funcs->reset(crtc);
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
+ drm_for_each_encoder(encoder, dev)
if (encoder->funcs->reset)
encoder->funcs->reset(encoder);
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 228eda29b14f..d3d038f05bf7 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -151,7 +151,7 @@ bool drm_helper_crtc_in_use(struct drm_crtc *crtc)
if (!oops_in_progress)
WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
+ drm_for_each_encoder(encoder, dev)
if (encoder->crtc == crtc && drm_helper_encoder_in_use(encoder))
return true;
return false;
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 46c4e6ee1cb1..f01dc25df2dc 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -211,7 +211,7 @@ int drm_fb_cma_debugfs_show(struct seq_file *m, void *arg)
struct drm_framebuffer *fb;
mutex_lock(&dev->mode_config.fb_lock);
- list_for_each_entry(fb, &dev->mode_config.fb_list, head)
+ drm_for_each_fb(fb, dev)
drm_fb_cma_describe(fb, m);
mutex_unlock(&dev->mode_config.fb_lock);
diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index c0a5cd8c5262..744dfbc6a329 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -276,7 +276,7 @@ void drm_warn_on_modeset_not_all_locked(struct drm_device *dev)
if (oops_in_progress)
return;
- list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
+ drm_for_each_crtc(crtc, dev)
WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
@@ -464,18 +464,17 @@ EXPORT_SYMBOL(drm_modeset_unlock);
int drm_modeset_lock_all_crtcs(struct drm_device *dev,
struct drm_modeset_acquire_ctx *ctx)
{
- struct drm_mode_config *config = &dev->mode_config;
struct drm_crtc *crtc;
struct drm_plane *plane;
int ret = 0;
- list_for_each_entry(crtc, &config->crtc_list, head) {
+ drm_for_each_crtc(crtc, dev) {
ret = drm_modeset_lock(&crtc->mutex, ctx);
if (ret)
return ret;
}
- list_for_each_entry(plane, &config->plane_list, head) {
+ drm_for_each_plane(plane, dev) {
ret = drm_modeset_lock(&plane->mutex, ctx);
if (ret)
return ret;
--
2.1.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 13/14] drm: Stop filtering according to mode_group in getresources
2015-07-09 21:44 [PATCH 00/14] drm_connector locking rules, v2 Daniel Vetter
` (11 preceding siblings ...)
2015-07-09 21:44 ` [PATCH 12/14] drm: Roll out drm_for_each_{plane,crtc,encoder} Daniel Vetter
@ 2015-07-09 21:44 ` Daniel Vetter
2015-07-09 21:44 ` [PATCH 14/14] drm: gc now dead mode_group code Daniel Vetter
13 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2015-07-09 21:44 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
It's been dead code since forever since mode groups haven't ever been
implemented. On top of that it's also been non-functional since we
only ever filtered the getresources ioctl and not any of the others
nor the mode object lookup code.
Given overwhelming evidence it looks like this isn't a feature we
need, hence remove it.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/drm_crtc.c | 110 +++++++++++++--------------------------------
1 file changed, 30 insertions(+), 80 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 9b91ef060c76..adf483cb2736 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1771,12 +1771,11 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
int crtc_count = 0;
int fb_count = 0;
int encoder_count = 0;
- int copied = 0, i;
+ int copied = 0;
uint32_t __user *fb_id;
uint32_t __user *crtc_id;
uint32_t __user *connector_id;
uint32_t __user *encoder_id;
- struct drm_mode_group *mode_group;
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EINVAL;
@@ -1809,27 +1808,14 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
/* mode_config.mutex protects the connector list against e.g. DP MST
* connector hot-adding. CRTC/Plane lists are invariant. */
mutex_lock(&dev->mode_config.mutex);
- if (!drm_is_primary_client(file_priv)) {
- struct drm_connector *connector;
- struct drm_encoder *encoder;
- struct drm_crtc *crtc;
-
- mode_group = NULL;
- drm_for_each_crtc(crtc, dev)
- crtc_count++;
-
- drm_for_each_connector(connector, dev)
- connector_count++;
+ drm_for_each_crtc(crtc, dev)
+ crtc_count++;
- drm_for_each_encoder(encoder, dev)
- encoder_count++;
- } else {
+ drm_for_each_connector(connector, dev)
+ connector_count++;
- mode_group = &file_priv->master->minor->mode_group;
- crtc_count = mode_group->num_crtcs;
- connector_count = mode_group->num_connectors;
- encoder_count = mode_group->num_encoders;
- }
+ drm_for_each_encoder(encoder, dev)
+ encoder_count++;
card_res->max_height = dev->mode_config.max_height;
card_res->min_height = dev->mode_config.min_height;
@@ -1840,24 +1826,13 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
if (card_res->count_crtcs >= crtc_count) {
copied = 0;
crtc_id = (uint32_t __user *)(unsigned long)card_res->crtc_id_ptr;
- if (!mode_group) {
- drm_for_each_crtc(crtc, dev) {
- DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id);
- if (put_user(crtc->base.id, crtc_id + copied)) {
- ret = -EFAULT;
- goto out;
- }
- copied++;
- }
- } else {
- for (i = 0; i < mode_group->num_crtcs; i++) {
- if (put_user(mode_group->id_list[i],
- crtc_id + copied)) {
- ret = -EFAULT;
- goto out;
- }
- copied++;
+ drm_for_each_crtc(crtc, dev) {
+ DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id);
+ if (put_user(crtc->base.id, crtc_id + copied)) {
+ ret = -EFAULT;
+ goto out;
}
+ copied++;
}
}
card_res->count_crtcs = crtc_count;
@@ -1866,27 +1841,15 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
if (card_res->count_encoders >= encoder_count) {
copied = 0;
encoder_id = (uint32_t __user *)(unsigned long)card_res->encoder_id_ptr;
- if (!mode_group) {
- drm_for_each_encoder(encoder, dev) {
- DRM_DEBUG_KMS("[ENCODER:%d:%s]\n", encoder->base.id,
- encoder->name);
- if (put_user(encoder->base.id, encoder_id +
- copied)) {
- ret = -EFAULT;
- goto out;
- }
- copied++;
- }
- } else {
- for (i = mode_group->num_crtcs; i < mode_group->num_crtcs + mode_group->num_encoders; i++) {
- if (put_user(mode_group->id_list[i],
- encoder_id + copied)) {
- ret = -EFAULT;
- goto out;
- }
- copied++;
+ drm_for_each_encoder(encoder, dev) {
+ DRM_DEBUG_KMS("[ENCODER:%d:%s]\n", encoder->base.id,
+ encoder->name);
+ if (put_user(encoder->base.id, encoder_id +
+ copied)) {
+ ret = -EFAULT;
+ goto out;
}
-
+ copied++;
}
}
card_res->count_encoders = encoder_count;
@@ -1895,29 +1858,16 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
if (card_res->count_connectors >= connector_count) {
copied = 0;
connector_id = (uint32_t __user *)(unsigned long)card_res->connector_id_ptr;
- if (!mode_group) {
- drm_for_each_connector(connector, dev) {
- DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
- connector->base.id,
- connector->name);
- if (put_user(connector->base.id,
- connector_id + copied)) {
- ret = -EFAULT;
- goto out;
- }
- copied++;
- }
- } else {
- int start = mode_group->num_crtcs +
- mode_group->num_encoders;
- for (i = start; i < start + mode_group->num_connectors; i++) {
- if (put_user(mode_group->id_list[i],
- connector_id + copied)) {
- ret = -EFAULT;
- goto out;
- }
- copied++;
+ drm_for_each_connector(connector, dev) {
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
+ connector->base.id,
+ connector->name);
+ if (put_user(connector->base.id,
+ connector_id + copied)) {
+ ret = -EFAULT;
+ goto out;
}
+ copied++;
}
}
card_res->count_connectors = connector_count;
--
2.1.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 14/14] drm: gc now dead mode_group code
2015-07-09 21:44 [PATCH 00/14] drm_connector locking rules, v2 Daniel Vetter
` (12 preceding siblings ...)
2015-07-09 21:44 ` [PATCH 13/14] drm: Stop filtering according to mode_group in getresources Daniel Vetter
@ 2015-07-09 21:44 ` Daniel Vetter
2015-07-22 13:05 ` Maarten Lankhorst
13 siblings, 1 reply; 20+ messages in thread
From: Daniel Vetter @ 2015-07-09 21:44 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
Two nice things here:
- drm_dev_register will truly register everything in the right order
if the driver doesn't have a ->load callback. Before this we had to
init the primary mode_group after the device nodes where already
registered.
- Less things to keep track of when reworking the connector locking,
yay!
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/drm_crtc.c | 64 ----------------------------------
drivers/gpu/drm/drm_drv.c | 12 -------
drivers/gpu/drm/i915/intel_dp_mst.c | 3 --
drivers/gpu/drm/radeon/radeon_dp_mst.c | 3 --
include/drm/drmP.h | 1 -
include/drm/drm_crtc.h | 26 --------------
6 files changed, 109 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index adf483cb2736..1f0da41ae2a1 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1679,70 +1679,6 @@ int drm_mode_create_suggested_offset_properties(struct drm_device *dev)
}
EXPORT_SYMBOL(drm_mode_create_suggested_offset_properties);
-static int drm_mode_group_init(struct drm_device *dev, struct drm_mode_group *group)
-{
- uint32_t total_objects = 0;
-
- total_objects += dev->mode_config.num_crtc;
- total_objects += dev->mode_config.num_connector;
- total_objects += dev->mode_config.num_encoder;
-
- group->id_list = kcalloc(total_objects, sizeof(uint32_t), GFP_KERNEL);
- if (!group->id_list)
- return -ENOMEM;
-
- group->num_crtcs = 0;
- group->num_connectors = 0;
- group->num_encoders = 0;
- return 0;
-}
-
-void drm_mode_group_destroy(struct drm_mode_group *group)
-{
- kfree(group->id_list);
- group->id_list = NULL;
-}
-
-/*
- * NOTE: Driver's shouldn't ever call drm_mode_group_init_legacy_group - it is
- * the drm core's responsibility to set up mode control groups.
- */
-int drm_mode_group_init_legacy_group(struct drm_device *dev,
- struct drm_mode_group *group)
-{
- struct drm_crtc *crtc;
- struct drm_encoder *encoder;
- struct drm_connector *connector;
- int ret;
-
- ret = drm_mode_group_init(dev, group);
- if (ret)
- return ret;
-
- drm_for_each_crtc(crtc, dev)
- group->id_list[group->num_crtcs++] = crtc->base.id;
-
- drm_for_each_encoder(encoder, dev)
- group->id_list[group->num_crtcs + group->num_encoders++] =
- encoder->base.id;
-
- drm_for_each_connector(connector, dev)
- group->id_list[group->num_crtcs + group->num_encoders +
- group->num_connectors++] = connector->base.id;
-
- return 0;
-}
-EXPORT_SYMBOL(drm_mode_group_init_legacy_group);
-
-void drm_reinit_primary_mode_group(struct drm_device *dev)
-{
- drm_modeset_lock_all(dev);
- drm_mode_group_destroy(&dev->primary->mode_group);
- drm_mode_group_init_legacy_group(dev, &dev->primary->mode_group);
- drm_modeset_unlock_all(dev);
-}
-EXPORT_SYMBOL(drm_reinit_primary_mode_group);
-
/**
* drm_mode_getresources - get graphics configuration
* @dev: drm device for the ioctl
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 9b51fe11ff19..53d09a19f7e1 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -285,7 +285,6 @@ static void drm_minor_free(struct drm_device *dev, unsigned int type)
if (!minor)
return;
- drm_mode_group_destroy(&minor->mode_group);
put_device(minor->kdev);
spin_lock_irqsave(&drm_minor_lock, flags);
@@ -700,20 +699,9 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
goto err_minors;
}
- /* setup grouping for legacy outputs */
- if (drm_core_check_feature(dev, DRIVER_MODESET)) {
- ret = drm_mode_group_init_legacy_group(dev,
- &dev->primary->mode_group);
- if (ret)
- goto err_unload;
- }
-
ret = 0;
goto out_unlock;
-err_unload:
- if (dev->driver->unload)
- dev->driver->unload(dev);
err_minors:
drm_minor_unregister(dev, DRM_MINOR_LEGACY);
drm_minor_unregister(dev, DRM_MINOR_RENDER);
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index d0b2569c6241..585f0a45b3f1 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -441,7 +441,6 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
drm_object_attach_property(&connector->base, dev->mode_config.tile_property, 0);
drm_mode_connector_set_path_property(connector, pathprop);
- drm_reinit_primary_mode_group(dev);
drm_modeset_lock_all(dev);
intel_connector_add_to_fbdev(intel_connector);
drm_modeset_unlock_all(dev);
@@ -466,8 +465,6 @@ static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
drm_connector_cleanup(connector);
drm_modeset_unlock_all(dev);
- drm_reinit_primary_mode_group(dev);
-
kfree(intel_connector);
DRM_DEBUG_KMS("\n");
}
diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c
index e649c8ff20a0..e4fc8f3bf58b 100644
--- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
@@ -284,7 +284,6 @@ static struct drm_connector *radeon_dp_add_mst_connector(struct drm_dp_mst_topol
drm_object_attach_property(&connector->base, dev->mode_config.path_property, 0);
drm_mode_connector_set_path_property(connector, pathprop);
- drm_reinit_primary_mode_group(dev);
drm_modeset_lock_all(dev);
radeon_fb_add_connector(rdev, connector);
@@ -309,8 +308,6 @@ static void radeon_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
drm_connector_cleanup(connector);
drm_modeset_unlock_all(dev);
- drm_reinit_primary_mode_group(dev);
-
kfree(connector);
DRM_DEBUG_KMS("\n");
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index ed650eb7ac4e..b191e951fe61 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -676,7 +676,6 @@ struct drm_minor {
/* currently active master for this node. Protected by master_mutex */
struct drm_master *master;
- struct drm_mode_group mode_group;
};
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index fe3100115a41..3071319ea194 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1018,29 +1018,6 @@ struct drm_mode_config_funcs {
};
/**
- * struct drm_mode_group - group of mode setting resources for potential sub-grouping
- * @num_crtcs: CRTC count
- * @num_encoders: encoder count
- * @num_connectors: connector count
- * @num_bridges: bridge count
- * @id_list: list of KMS object IDs in this group
- *
- * Currently this simply tracks the global mode setting state. But in the
- * future it could allow groups of objects to be set aside into independent
- * control groups for use by different user level processes (e.g. two X servers
- * running simultaneously on different heads, each with their own mode
- * configuration and freedom of mode setting).
- */
-struct drm_mode_group {
- uint32_t num_crtcs;
- uint32_t num_encoders;
- uint32_t num_connectors;
-
- /* list of object IDs for this group */
- uint32_t *id_list;
-};
-
-/**
* struct drm_mode_config - Mode configuration control structure
* @mutex: mutex protecting KMS related lists and structures
* @connection_mutex: ww mutex protecting connector state and routing
@@ -1324,9 +1301,6 @@ extern const char *drm_get_tv_select_name(int val);
extern void drm_fb_release(struct drm_file *file_priv);
extern void drm_property_destroy_user_blobs(struct drm_device *dev,
struct drm_file *file_priv);
-extern int drm_mode_group_init_legacy_group(struct drm_device *dev, struct drm_mode_group *group);
-extern void drm_mode_group_destroy(struct drm_mode_group *group);
-extern void drm_reinit_primary_mode_group(struct drm_device *dev);
extern bool drm_probe_ddc(struct i2c_adapter *adapter);
extern struct edid *drm_get_edid(struct drm_connector *connector,
struct i2c_adapter *adapter);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 14/14] drm: gc now dead mode_group code
2015-07-09 21:44 ` [PATCH 14/14] drm: gc now dead mode_group code Daniel Vetter
@ 2015-07-22 13:05 ` Maarten Lankhorst
2015-07-22 14:46 ` Daniel Vetter
0 siblings, 1 reply; 20+ messages in thread
From: Maarten Lankhorst @ 2015-07-22 13:05 UTC (permalink / raw)
To: Daniel Vetter, DRI Development; +Cc: Daniel Vetter, Intel Graphics Development
Op 09-07-15 om 23:44 schreef Daniel Vetter:
> Two nice things here:
> - drm_dev_register will truly register everything in the right order
> if the driver doesn't have a ->load callback. Before this we had to
> init the primary mode_group after the device nodes where already
> registered.
>
> - Less things to keep track of when reworking the connector locking,
> yay!
>
Been wondering what it was when investigating the dp hotplug.
Patch series looks sane. :-)
r-b
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 14/14] drm: gc now dead mode_group code
2015-07-22 13:05 ` Maarten Lankhorst
@ 2015-07-22 14:46 ` Daniel Vetter
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2015-07-22 14:46 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
Daniel Vetter
On Wed, Jul 22, 2015 at 03:05:06PM +0200, Maarten Lankhorst wrote:
> Op 09-07-15 om 23:44 schreef Daniel Vetter:
> > Two nice things here:
> > - drm_dev_register will truly register everything in the right order
> > if the driver doesn't have a ->load callback. Before this we had to
> > init the primary mode_group after the device nodes where already
> > registered.
> >
> > - Less things to keep track of when reworking the connector locking,
> > yay!
> >
> Been wondering what it was when investigating the dp hotplug.
> Patch series looks sane. :-)
>
> r-b
Thanks, all applied, plus one cma patch from the struct_mutex series which
is needed here. I've pinged Laurent for an r-b on that one.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread