* [PATCH 01/11] drm: Simplify drm_for_each_legacy_plane arguments
2015-06-23 20:45 [PATCH 00/11] [RFC] drm_for_each_* macros and list locking Daniel Vetter
@ 2015-06-23 20:45 ` Daniel Vetter
2015-06-23 20:45 ` [PATCH 02/11] drm: Add modeset object iterators Daniel Vetter
` (10 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2015-06-23 20:45 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 32ff034a0875..d4657868a78c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2084,7 +2084,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 3b4d8a4a23fb..bc58c030ac89 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1577,8 +1577,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] 17+ messages in thread* [PATCH 02/11] drm: Add modeset object iterators
2015-06-23 20:45 [PATCH 00/11] [RFC] drm_for_each_* macros and list locking Daniel Vetter
2015-06-23 20:45 ` [PATCH 01/11] drm: Simplify drm_for_each_legacy_plane arguments Daniel Vetter
@ 2015-06-23 20:45 ` Daniel Vetter
2015-06-23 20:45 ` [PATCH 03/11] drm/probe-helper: Grab mode_config.mutex in poll_init/enable Daniel Vetter
` (9 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2015-06-23 20:45 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 2d57fc56dcbf..6f572733cbdd 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 393114df88a3..30254fb249fe 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 04203c0d2ecb..64d85c1bd18b 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 bc58c030ac89..5994750f523a 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1581,4 +1581,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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 03/11] drm/probe-helper: Grab mode_config.mutex in poll_init/enable
2015-06-23 20:45 [PATCH 00/11] [RFC] drm_for_each_* macros and list locking Daniel Vetter
2015-06-23 20:45 ` [PATCH 01/11] drm: Simplify drm_for_each_legacy_plane arguments Daniel Vetter
2015-06-23 20:45 ` [PATCH 02/11] drm: Add modeset object iterators Daniel Vetter
@ 2015-06-23 20:45 ` Daniel Vetter
2015-06-23 20:45 ` [PATCH 04/11] drm/fbdev-helper: Grab mode_config.mutex in drm_fb_helper_single_add_all_connectors Daniel Vetter
` (8 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2015-06-23 20:45 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 64d85c1bd18b..d734780b31c0 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] 17+ messages in thread* [PATCH 04/11] drm/fbdev-helper: Grab mode_config.mutex in drm_fb_helper_single_add_all_connectors
2015-06-23 20:45 [PATCH 00/11] [RFC] drm_for_each_* macros and list locking Daniel Vetter
` (2 preceding siblings ...)
2015-06-23 20:45 ` [PATCH 03/11] drm/probe-helper: Grab mode_config.mutex in poll_init/enable Daniel Vetter
@ 2015-06-23 20:45 ` Daniel Vetter
2015-06-23 20:45 ` [PATCH 05/11] drm: Check locking in drm_for_each_encoder/connector Daniel Vetter
` (7 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2015-06-23 20:45 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 05/11] drm: Check locking in drm_for_each_encoder/connector
2015-06-23 20:45 [PATCH 00/11] [RFC] drm_for_each_* macros and list locking Daniel Vetter
` (3 preceding siblings ...)
2015-06-23 20:45 ` [PATCH 04/11] drm/fbdev-helper: Grab mode_config.mutex in drm_fb_helper_single_add_all_connectors Daniel Vetter
@ 2015-06-23 20:45 ` Daniel Vetter
2015-06-23 20:45 ` [PATCH 06/11] drm/i915: Use drm_for_each_fb in i915_debugfs.c Daniel Vetter
` (6 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2015-06-23 20:45 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
Because of DP MST encoders/connectors can now be hotplugged and we
must hold the right lock when walking the encoder/connector lists.
Enforce this by checking the locking in our shiny new list walking
macros.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
include/drm/drm_crtc.h | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 5994750f523a..a912ba06cf45 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1588,10 +1588,18 @@ static inline struct drm_property *drm_property_find(struct drm_device *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)
+ for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex)), \
+ 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)
+ for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex)), \
+ encoder = list_first_entry(&(dev)->mode_config.encoder_list, \
+ struct drm_encoder, head); \
+ &encoder->head != (&(dev)->mode_config.encoder_list); \
+ encoder = list_next_entry(encoder, head))
#define drm_for_each_fb(fb, dev) \
list_for_each_entry(fb, &(dev)->mode_config.fb_list, head)
--
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] 17+ messages in thread* [PATCH 06/11] drm/i915: Use drm_for_each_fb in i915_debugfs.c
2015-06-23 20:45 [PATCH 00/11] [RFC] drm_for_each_* macros and list locking Daniel Vetter
` (4 preceding siblings ...)
2015-06-23 20:45 ` [PATCH 05/11] drm: Check locking in drm_for_each_encoder/connector Daniel Vetter
@ 2015-06-23 20:45 ` Daniel Vetter
2015-06-23 20:45 ` [PATCH 07/11] drm: Check locking in drm_for_each_fb Daniel Vetter
` (5 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2015-06-23 20:45 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 c49fe2afa223..6db920c913ee 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1815,6 +1815,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;
@@ -1834,7 +1835,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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 07/11] drm: Check locking in drm_for_each_fb
2015-06-23 20:45 [PATCH 00/11] [RFC] drm_for_each_* macros and list locking Daniel Vetter
` (5 preceding siblings ...)
2015-06-23 20:45 ` [PATCH 06/11] drm/i915: Use drm_for_each_fb in i915_debugfs.c Daniel Vetter
@ 2015-06-23 20:45 ` Daniel Vetter
2015-06-23 20:45 ` [PATCH 08/11] drm/i915: Take all modeset locks for DP MST hotplug Daniel Vetter
` (4 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2015-06-23 20:45 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 a912ba06cf45..3646c47b43de 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1602,6 +1602,10 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
encoder = list_next_entry(encoder, 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 08/11] drm/i915: Take all modeset locks for DP MST hotplug
2015-06-23 20:45 [PATCH 00/11] [RFC] drm_for_each_* macros and list locking Daniel Vetter
` (6 preceding siblings ...)
2015-06-23 20:45 ` [PATCH 07/11] drm: Check locking in drm_for_each_fb Daniel Vetter
@ 2015-06-23 20:45 ` Daniel Vetter
2015-06-23 20:46 ` [PATCH 09/11] drm: Amend connector/encoder list locking rules Daniel Vetter
` (3 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2015-06-23 20:45 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 09/11] drm: Amend connector/encoder list locking rules
2015-06-23 20:45 [PATCH 00/11] [RFC] drm_for_each_* macros and list locking Daniel Vetter
` (7 preceding siblings ...)
2015-06-23 20:45 ` [PATCH 08/11] drm/i915: Take all modeset locks for DP MST hotplug Daniel Vetter
@ 2015-06-23 20:46 ` Daniel Vetter
2015-06-24 7:57 ` Chris Wilson
2015-06-23 20:46 ` [PATCH 10/11] drm: Roll out drm_for_each_connector more Daniel Vetter
` (2 subsequent siblings)
11 siblings, 1 reply; 17+ messages in thread
From: Daniel Vetter @ 2015-06-23 20:46 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
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.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
include/drm/drm_crtc.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 3646c47b43de..c1c4f16f01ca 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1588,14 +1588,16 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head)
#define drm_for_each_connector(connector, dev) \
- for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex)), \
+ for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex) && \
+ !drm_modeset_is_locked(&(dev)->mode_config.connection_mutex)), \
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) \
- for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex)), \
+ for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex) && \
+ !drm_modeset_is_locked(&(dev)->mode_config.connection_mutex)), \
encoder = list_first_entry(&(dev)->mode_config.encoder_list, \
struct drm_encoder, head); \
&encoder->head != (&(dev)->mode_config.encoder_list); \
--
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] 17+ messages in thread* Re: [PATCH 09/11] drm: Amend connector/encoder list locking rules
2015-06-23 20:46 ` [PATCH 09/11] drm: Amend connector/encoder list locking rules Daniel Vetter
@ 2015-06-24 7:57 ` Chris Wilson
2015-06-24 8:36 ` Daniel Vetter
0 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2015-06-24 7:57 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, DRI Development
On Tue, Jun 23, 2015 at 10:46:00PM +0200, Daniel Vetter wrote:
> 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.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> include/drm/drm_crtc.h | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 3646c47b43de..c1c4f16f01ca 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1588,14 +1588,16 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
> list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head)
>
> #define drm_for_each_connector(connector, dev) \
> - for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex)), \
> + for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex) && \
> + !drm_modeset_is_locked(&(dev)->mode_config.connection_mutex)), \
> 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) \
> - for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex)), \
> + for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex) && \
> + !drm_modeset_is_locked(&(dev)->mode_config.connection_mutex)), \
> encoder = list_first_entry(&(dev)->mode_config.encoder_list, \
> struct drm_encoder, head); \
> &encoder->head != (&(dev)->mode_config.encoder_list); \
Maybe something like:
// not sure how specifc each lock will end up being
static inline void assert_drm_mode_list_locked(dev)
{
/* lockdep for accuracy and verbose debug reportts,
* WARN_ON for warnings everywhere.
*/
lockdep_assert_held(&dev->mode_config.mutex);
WARN_ON(!(mutex_is_locked(&dev->mode_config.mutex) &&
drm_modeset_is_locked(&dev->mode_config.connection_mutex)));
// Daniel, it is not clear from context whether you mean
// !a && !b, or !(a && b)
}
static inline struct drm_connector *drm_first_connector(dev)
{
assert_drm_mode_list_locked(dev);
return list_first_entry(&dev->mode_config.connector_list,
struct drm_connector, head);
}
#define drm_for_each_connector(connector, dev) \
for (connector = drm_first_connector(dev); \
&connector->head != (&(dev)->mode_config.connector_list); \
connector = list_next_entry(connector, head))
static inline struct drm_encoder *drm_first_encoder(dev)
{
assert_drm_mode_list_locked(dev);
return list_first_entry(&dev->mode_config.encoder_list,
struct drm_encoder, head);
}
#define drm_for_each_encoder(encoder, dev) \
for (encoder = drm_first_encoder(dev); \
&encoder->head != (&(dev)->mode_config.encoder_list); \
encoder = list_next_entry(encoder, head))
Usual arguments of reusing code for clarity.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 09/11] drm: Amend connector/encoder list locking rules
2015-06-24 7:57 ` Chris Wilson
@ 2015-06-24 8:36 ` Daniel Vetter
0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2015-06-24 8:36 UTC (permalink / raw)
To: Chris Wilson, Daniel Vetter, DRI Development,
Intel Graphics Development, Daniel Vetter
On Wed, Jun 24, 2015 at 08:57:23AM +0100, Chris Wilson wrote:
> On Tue, Jun 23, 2015 at 10:46:00PM +0200, Daniel Vetter wrote:
> > 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.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > include/drm/drm_crtc.h | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index 3646c47b43de..c1c4f16f01ca 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -1588,14 +1588,16 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
> > list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head)
> >
> > #define drm_for_each_connector(connector, dev) \
> > - for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex)), \
> > + for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex) && \
> > + !drm_modeset_is_locked(&(dev)->mode_config.connection_mutex)), \
> > 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) \
> > - for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex)), \
> > + for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex) && \
> > + !drm_modeset_is_locked(&(dev)->mode_config.connection_mutex)), \
> > encoder = list_first_entry(&(dev)->mode_config.encoder_list, \
> > struct drm_encoder, head); \
> > &encoder->head != (&(dev)->mode_config.encoder_list); \
>
> Maybe something like:
>
> // not sure how specifc each lock will end up being
> static inline void assert_drm_mode_list_locked(dev)
> {
> /* lockdep for accuracy and verbose debug reportts,
> * WARN_ON for warnings everywhere.
> */
> lockdep_assert_held(&dev->mode_config.mutex);
> WARN_ON(!(mutex_is_locked(&dev->mode_config.mutex) &&
> drm_modeset_is_locked(&dev->mode_config.connection_mutex)));
>
> // Daniel, it is not clear from context whether you mean
> // !a && !b, or !(a && b)
Write access holds both locks, which means for read access it's ok to just
hold one. But since that's caused confusions I fully agree that extracting
this into a static inline and commenting properly is a good idea.
-Daniel
> }
>
> static inline struct drm_connector *drm_first_connector(dev)
> {
> assert_drm_mode_list_locked(dev);
> return list_first_entry(&dev->mode_config.connector_list,
> struct drm_connector, head);
> }
>
> #define drm_for_each_connector(connector, dev) \
> for (connector = drm_first_connector(dev); \
> &connector->head != (&(dev)->mode_config.connector_list); \
> connector = list_next_entry(connector, head))
>
> static inline struct drm_encoder *drm_first_encoder(dev)
> {
> assert_drm_mode_list_locked(dev);
> return list_first_entry(&dev->mode_config.encoder_list,
> struct drm_encoder, head);
> }
>
> #define drm_for_each_encoder(encoder, dev) \
> for (encoder = drm_first_encoder(dev); \
> &encoder->head != (&(dev)->mode_config.encoder_list); \
> encoder = list_next_entry(encoder, head))
>
> Usual arguments of reusing code for clarity.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 10/11] drm: Roll out drm_for_each_connector more
2015-06-23 20:45 [PATCH 00/11] [RFC] drm_for_each_* macros and list locking Daniel Vetter
` (8 preceding siblings ...)
2015-06-23 20:46 ` [PATCH 09/11] drm: Amend connector/encoder list locking rules Daniel Vetter
@ 2015-06-23 20:46 ` Daniel Vetter
2015-06-23 20:46 ` [PATCH 11/11] drm: Roll out drm_for_each_{plane, crtc, encoder} Daniel Vetter
2015-06-24 7:44 ` [PATCH 00/11] [RFC] drm_for_each_* macros and list locking Chris Wilson
11 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2015-06-23 20:46 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 f6f2fb58eb37..dfc8aaa1457e 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 6f572733cbdd..9c1440b2d84c 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 30254fb249fe..4a83c22f5e61 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 7087da37dae0..e6e05bb75a77 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3413,7 +3413,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 2f0ed11024eb..7e1cde803bf3 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 11/11] drm: Roll out drm_for_each_{plane, crtc, encoder}
2015-06-23 20:45 [PATCH 00/11] [RFC] drm_for_each_* macros and list locking Daniel Vetter
` (9 preceding siblings ...)
2015-06-23 20:46 ` [PATCH 10/11] drm: Roll out drm_for_each_connector more Daniel Vetter
@ 2015-06-23 20:46 ` Daniel Vetter
2015-06-29 3:41 ` shuang.he
2015-06-24 7:44 ` [PATCH 00/11] [RFC] drm_for_each_* macros and list locking Chris Wilson
11 siblings, 1 reply; 17+ messages in thread
From: Daniel Vetter @ 2015-06-23 20:46 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 9c1440b2d84c..b4ecf92cd668 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 4a83c22f5e61..2e89d8b7de18 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 5c1aca443e54..87ce7c36b0db 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -221,7 +221,7 @@ int drm_fb_cma_debugfs_show(struct seq_file *m, void *arg)
return ret;
}
- 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->struct_mutex);
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 11/11] drm: Roll out drm_for_each_{plane, crtc, encoder}
2015-06-23 20:46 ` [PATCH 11/11] drm: Roll out drm_for_each_{plane, crtc, encoder} Daniel Vetter
@ 2015-06-29 3:41 ` shuang.he
0 siblings, 0 replies; 17+ messages in thread
From: shuang.he @ 2015-06-29 3:41 UTC (permalink / raw)
To: shuang.he, lei.a.liu, intel-gfx, daniel.vetter
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6587
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
ILK -6 303/303 297/303
SNB -2 312/312 310/312
IVB -2 343/343 341/343
BYT -3 284/284 281/284
HSW -3 380/380 377/380
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*ILK igt@drv_debugfs_reader PASS(1) DMESG_WARN(1)
(dmesg patch applied)BUG:unable_to_handle_kernel@BUG: unable to handle kernel
Oops@Oops: 0000
RIP:describe_obj[i915]@RIP: .* describe_obj+0x
*ILK igt@drv_suspend@debugfs-reader PASS(1) DMESG_WARN(1)
*ILK igt@drv_suspend@fence-restore-tiled2untiled PASS(1) DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/drm_crtc.c:#drm_mode_config_reset[drm]()@WARNING:.* at .* drm_mode_config_reset+0x
*ILK igt@drv_suspend@fence-restore-untiled PASS(1) DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/drm_crtc.c:#drm_mode_config_reset[drm]()@WARNING:.* at .* drm_mode_config_reset+0x
*ILK igt@drv_suspend@forcewake PASS(1) DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/drm_crtc.c:#drm_mode_config_reset[drm]()@WARNING:.* at .* drm_mode_config_reset+0x
*ILK igt@gem_workarounds@suspend-resume PASS(1) DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/drm_crtc.c:#drm_mode_config_reset[drm]()@WARNING:.* at .* drm_mode_config_reset+0x
*SNB igt@drv_debugfs_reader PASS(1) DMESG_WARN(1)
(dmesg patch applied)BUG:unable_to_handle_kernel@BUG: unable to handle kernel
Oops@Oops: 0000
RIP:describe_obj[i915]@RIP: .* describe_obj+0x
*SNB igt@kms_addfb@framebuffer-vs-set-tiling PASS(1) TIMEOUT(1)
*IVB igt@drv_debugfs_reader PASS(1) DMESG_WARN(1)
(dmesg patch applied)BUG:unable_to_handle_kernel@BUG: unable to handle kernel
Oops@Oops: 0000
RIP:describe_obj[i915]@RIP: .* describe_obj+0x
*IVB igt@kms_addfb@framebuffer-vs-set-tiling PASS(1) TIMEOUT(1)
*BYT igt@drv_debugfs_reader PASS(1) DMESG_WARN(1)
(dmesg patch applied)BUG:unable_to_handle_kernel@BUG: unable to handle kernel
Oops@Oops: 0000
RIP:describe_obj[i915]@RIP: .* describe_obj+0x
*BYT igt@gem_partial_pwrite_pread@reads-display PASS(1) FAIL(1)
*BYT igt@kms_addfb@framebuffer-vs-set-tiling PASS(1) TIMEOUT(1)
*HSW igt@drv_debugfs_reader PASS(1) DMESG_WARN(1)
(dmesg patch applied)BUG:unable_to_handle_kernel@BUG: unable to handle kernel
Oops@Oops: 0000
RIP:describe_obj[i915]@RIP: .* describe_obj+0x
*HSW igt@kms_addfb@framebuffer-vs-set-tiling PASS(1) TIMEOUT(1)
*HSW igt@pm_rpm@debugfs-read PASS(1) TIMEOUT(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 00/11] [RFC] drm_for_each_* macros and list locking
2015-06-23 20:45 [PATCH 00/11] [RFC] drm_for_each_* macros and list locking Daniel Vetter
` (10 preceding siblings ...)
2015-06-23 20:46 ` [PATCH 11/11] drm: Roll out drm_for_each_{plane, crtc, encoder} Daniel Vetter
@ 2015-06-24 7:44 ` Chris Wilson
2015-06-24 8:35 ` Daniel Vetter
11 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2015-06-24 7:44 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Intel Graphics Development, DRI Development
On Tue, Jun 23, 2015 at 10:45:51PM +0200, Daniel Vetter wrote:
> Hi all,
>
> Dave&I have been discussing connector hotplug and unplugging around DP MST and
> if there's one thing that's clear it's that we don't even really know where all
> the problems are. Hence first step is to figure that out. One of the bigger
> items is walking the encoder/connector lists without appropriate locking to
> protect against concurrent hotadd/removal of connectors.
>
> This patch series tries to untangle things a bit here. RFC since only lightly
> tested and missing conversion of the radoen mst code to the new locking scheme.
> I think rolling out the new macros for i915 should be done as a second step, to
> avoid hitting too many WARN_ON ;-)
Bah, I was hoping to see scru! :)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 00/11] [RFC] drm_for_each_* macros and list locking
2015-06-24 7:44 ` [PATCH 00/11] [RFC] drm_for_each_* macros and list locking Chris Wilson
@ 2015-06-24 8:35 ` Daniel Vetter
0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2015-06-24 8:35 UTC (permalink / raw)
To: Chris Wilson, Daniel Vetter, DRI Development,
Intel Graphics Development
On Wed, Jun 24, 2015 at 08:44:47AM +0100, Chris Wilson wrote:
> On Tue, Jun 23, 2015 at 10:45:51PM +0200, Daniel Vetter wrote:
> > Hi all,
> >
> > Dave&I have been discussing connector hotplug and unplugging around DP MST and
> > if there's one thing that's clear it's that we don't even really know where all
> > the problems are. Hence first step is to figure that out. One of the bigger
> > items is walking the encoder/connector lists without appropriate locking to
> > protect against concurrent hotadd/removal of connectors.
> >
> > This patch series tries to untangle things a bit here. RFC since only lightly
> > tested and missing conversion of the radoen mst code to the new locking scheme.
> > I think rolling out the new macros for i915 should be done as a second step, to
> > avoid hitting too many WARN_ON ;-)
>
> Bah, I was hoping to see scru! :)
This is just to start figuring out how bad the situation is and who's all
affected. Later on we need to figure out how to properly protect the
connector/encoder list without just grabbing all kinds of locks in the
hotadd/remove code - stalling screen updates on the main screen just
because you plug something in tends to upset people.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread