From 4ea50e99bd3501aea394aa7a9e9bd3115faabf37 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 9 Jul 2015 23:44:24 +0200 Subject: [PATCH 01/15] drm: Simplify drm_for_each_legacy_plane arguments 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. Reviewed-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- 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 0d3e01434860f..5004c4a46a9e3 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2356,7 +2356,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 859ccb658601e..e9272b0a85924 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 57ca8cc383a61..5cf0e6c3fc41d 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__ */ From 6295d607ad34ee4e43aab3f20714c2ef7a6adea1 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 9 Jul 2015 23:44:25 +0200 Subject: [PATCH 02/15] drm: Add modeset object iterators 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) { ... } Reviewed-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- 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 9c978d9de3b8e..9b05dc76379e3 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; @@ -5393,7 +5388,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 393114df88a3d..30254fb249fed 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 cac422916c7af..3630d92c9738a 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 aaa130736bf8d..be3884073ea4d 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 04203c0d2ecbe..64d85c1bd18bf 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 5cf0e6c3fc41d..7c95a7df60651 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__ */ From 8c4ccc4ab6f64e859d4ff8d7c02c2ed2e956e07f Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 9 Jul 2015 23:44:26 +0200 Subject: [PATCH 03/15] drm/probe-helper: Grab mode_config.mutex in poll_init/enable 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. Reviewed-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- 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 64d85c1bd18bf..d734780b31c0f 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); From 169faecadd2444fc295c724c0a2f622b99e6bf19 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 9 Jul 2015 23:44:27 +0200 Subject: [PATCH 04/15] drm/fbdev-helper: Grab mode_config.mutex in drm_fb_helper_single_add_all_connectors 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. Reviewed-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- 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 3630d92c9738a..329d08167b77d 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 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper) 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); From 7a3f3d6667f5f9ffd1517f6b21d64bbf5312042c Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 9 Jul 2015 23:44:28 +0200 Subject: [PATCH 05/15] drm: Check locking in drm_for_each_connector 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 Cc: Dave Airlie Reviewed-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- 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 7c95a7df60651..499562274353a 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) From 3a58ee106529ebea5710087e9b77bc11365665ae Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 10 Jul 2015 19:02:51 +0200 Subject: [PATCH 06/15] drm/i915: Use drm_for_each_fb in i915_debugfs.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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ä Reviewed-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- 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 bc817da9fef7a..51580bdd587fd 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1866,6 +1866,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; @@ -1885,7 +1886,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; From 4676ba0be756b2b02b9737147713d458042962f7 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 9 Jul 2015 23:44:30 +0200 Subject: [PATCH 07/15] drm: Check locking in drm_for_each_fb 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. Reviewed-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- 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 499562274353a..10547be5684a4 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__ */ From 8bb4da1df54a20d68c34427356e34315ba122c0f Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 9 Jul 2015 23:44:31 +0200 Subject: [PATCH 08/15] drm/i915: Take all modeset locks for DP MST hotplug 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. Reviewed-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- 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 6e4cc5334f47d..d0b2569c62412 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); From 2ee6bcdcfa4d8b56b20bc6308cd5f9bced5b5324 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 9 Jul 2015 23:44:32 +0200 Subject: [PATCH 09/15] drm/radeon: Take all modeset locks for DP MST hotplug 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. Reviewed-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- 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 257b10be5cda9..e649c8ff20a0d 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); From cff20ba2758d6b82978be5b1f40536bfc121af88 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 9 Jul 2015 23:44:33 +0200 Subject: [PATCH 10/15] drm: Amend connector list locking rules 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 Reviewed-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- 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 10547be5684a4..fe3100115a41d 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) \ From 9a9f5ce8db176499a7f3f93172bf34176aa460f5 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 9 Jul 2015 23:44:34 +0200 Subject: [PATCH 11/15] drm: Roll out drm_for_each_connector more 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. Reviewed-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- 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 f6f2fb58eb37f..dfc8aaa1457e4 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 5b59d5ad7d1c2..4ff334ba0382f 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; @@ -1986,7 +1986,7 @@ void drm_atomic_helper_connector_dpms(struct drm_connector *connector, 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 9b05dc76379e3..d928bf7ef2219 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 30254fb249fed..4a83c22f5e617 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 7087da37dae0b..e6e05bb75a771 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 2f0ed11024eb8..7e1cde803bf30 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; } From 9e75c0ef1428ae8a6bfb340ba2402471752c4af5 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 9 Jul 2015 23:32:34 +0200 Subject: [PATCH 12/15] drm/cma-helper: Fix locking in drm_fb_cma_debugfs_show This function takes two locks, both of them the wrong ones. This wasn't an oversight from my fb locking rework since both patches landed in parallel. We really only need fb_lock when walking that list, since everything we can reach from that is refcounted properly already. v2: Drop unused dev spotted by 0day. Cc: Rob Clark Cc: Laurent Pinchart Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_fb_cma_helper.c | 16 ++-------------- drivers/gpu/drm/drm_gem_cma_helper.c | 3 --- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index 5c1aca443e54f..46c4e6ee1cb1a 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -209,23 +209,11 @@ int drm_fb_cma_debugfs_show(struct seq_file *m, void *arg) struct drm_info_node *node = (struct drm_info_node *) m->private; struct drm_device *dev = node->minor->dev; struct drm_framebuffer *fb; - int ret; - - ret = mutex_lock_interruptible(&dev->mode_config.mutex); - if (ret) - return ret; - - ret = mutex_lock_interruptible(&dev->struct_mutex); - if (ret) { - mutex_unlock(&dev->mode_config.mutex); - return ret; - } + mutex_lock(&dev->mode_config.fb_lock); list_for_each_entry(fb, &dev->mode_config.fb_list, head) drm_fb_cma_describe(fb, m); - - mutex_unlock(&dev->struct_mutex); - mutex_unlock(&dev->mode_config.mutex); + mutex_unlock(&dev->mode_config.fb_lock); return 0; } diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index bd75f303da63d..9edad11dca985 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -381,11 +381,8 @@ void drm_gem_cma_describe(struct drm_gem_cma_object *cma_obj, struct seq_file *m) { struct drm_gem_object *obj = &cma_obj->base; - struct drm_device *dev = obj->dev; uint64_t off; - WARN_ON(!mutex_is_locked(&dev->struct_mutex)); - off = drm_vma_node_start(&obj->vma_node); seq_printf(m, "%2d (%2d) %08llx %pad %p %zu", From e4f62546325724168e0b7ee7180762cb07859ca7 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 9 Jul 2015 23:44:35 +0200 Subject: [PATCH 13/15] drm: Roll out drm_for_each_{plane,crtc,encoder} Remaining manual work in the drm core&helpers. Nothing special here, no surprises. Reviewed-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- 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 d928bf7ef2219..138ef22078d9f 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) @@ -5377,15 +5379,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 4a83c22f5e617..2e89d8b7de18c 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 46c4e6ee1cb1a..f01dc25df2dcb 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 c0a5cd8c52621..744dfbc6a329a 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; From 9c7060f7e3b09837621f93bd8666cf4cfac45001 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 9 Jul 2015 23:44:36 +0200 Subject: [PATCH 14/15] drm: Stop filtering according to mode_group in getresources 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. Reviewed-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- 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 138ef22078d9f..805ef2f408a74 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; From 3fdefa399e4644399ce3e74e65a75122d52dba6a Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 9 Jul 2015 23:44:37 +0200 Subject: [PATCH 15/15] drm: gc now dead mode_group code 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! Reviewed-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- 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 805ef2f408a74..e385014ed6160 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 b7bf4ce8c012b..f7798c3b543e1 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); @@ -705,20 +704,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 d0b2569c62412..585f0a45b3f1c 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 e649c8ff20a0d..e4fc8f3bf58b0 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 48db6a56975f5..c89351ede92c4 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -675,7 +675,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 fe3100115a41d..3071319ea1940 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1017,29 +1017,6 @@ struct drm_mode_config_funcs { void (*atomic_state_free)(struct drm_atomic_state *state); }; -/** - * 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 @@ -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);