Commit c2d85564 authored by Daniel Vetter's avatar Daniel Vetter
Browse files

drm: drop modeset_lock_all from drm_state_info 



If we push the locks down we don't have to take them all at the same
time.

Aside: Making dump_info fully safe should be fairly simple, if we
protect the ->state pointers with rcu. Simply putting a
synchronize_rcu() into the drm_atomic_state free function should be
all that's roughly needed. Well except we shouldn't block in there, so
better to put that into a work_struct. But I've not set out to fix
that little issue.

Cc: Rob Clark <robdclark@gmail.com>
Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170403083304.9083-6-daniel.vetter@ffwll.ch
parent a5b8444e

drivers/gpu/drm/drm_atomic.c

+39 −21
Original line number Diff line number Diff line
@@ -1676,22 +1676,8 @@ static void drm_atomic_print_state(const struct drm_atomic_state *state) 
		drm_atomic_connector_print_state(&p, connector_state);

}



/**

 * drm_state_dump - dump entire device atomic state

 * @dev: the drm device

 * @p: where to print the state to

 *

 * Just for debugging.  Drivers might want an option to dump state

 * to dmesg in case of error irq's.  (Hint, you probably want to

 * ratelimit this!)

 *

 * The caller must drm_modeset_lock_all(), or if this is called

 * from error irq handler, it should not be enabled by default.

 * (Ie. if you are debugging errors you might not care that this

 * is racey.  But calling this without all modeset locks held is

 * not inherently safe.)

 */

void drm_state_dump(struct drm_device *dev, struct drm_printer *p)

static void __drm_state_dump(struct drm_device *dev, struct drm_printer *p,

			     bool take_locks)

{

	struct drm_mode_config *config = &dev->mode_config;

	struct drm_plane *plane;
@@ -1702,17 +1688,51 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p) 
	if (!drm_core_check_feature(dev, DRIVER_ATOMIC))

		return;



	list_for_each_entry(plane, &config->plane_list, head)

	list_for_each_entry(plane, &config->plane_list, head) {

		if (take_locks)

			drm_modeset_lock(&plane->mutex, NULL);

		drm_atomic_plane_print_state(p, plane->state);

		if (take_locks)

			drm_modeset_unlock(&plane->mutex);

	}



	list_for_each_entry(crtc, &config->crtc_list, head)

	list_for_each_entry(crtc, &config->crtc_list, head) {

		if (take_locks)

			drm_modeset_lock(&crtc->mutex, NULL);

		drm_atomic_crtc_print_state(p, crtc->state);

		if (take_locks)

			drm_modeset_unlock(&crtc->mutex);

	}



	drm_connector_list_iter_begin(dev, &conn_iter);

	if (take_locks)

		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);

	drm_for_each_connector_iter(connector, &conn_iter)

		drm_atomic_connector_print_state(p, connector->state);

	if (take_locks)

		drm_modeset_unlock(&dev->mode_config.connection_mutex);

	drm_connector_list_iter_end(&conn_iter);

}



/**

 * drm_state_dump - dump entire device atomic state

 * @dev: the drm device

 * @p: where to print the state to

 *

 * Just for debugging.  Drivers might want an option to dump state

 * to dmesg in case of error irq's.  (Hint, you probably want to

 * ratelimit this!)

 *

 * The caller must drm_modeset_lock_all(), or if this is called

 * from error irq handler, it should not be enabled by default.

 * (Ie. if you are debugging errors you might not care that this

 * is racey.  But calling this without all modeset locks held is

 * not inherently safe.)

 */

void drm_state_dump(struct drm_device *dev, struct drm_printer *p)

{

	__drm_state_dump(dev, p, false);

}

EXPORT_SYMBOL(drm_state_dump);



#ifdef CONFIG_DEBUG_FS
@@ -1722,9 +1742,7 @@ static int drm_state_info(struct seq_file *m, void *data) 
	struct drm_device *dev = node->minor->dev;

	struct drm_printer p = drm_seq_file_printer(m);



	drm_modeset_lock_all(dev);

	drm_state_dump(dev, &p);

	drm_modeset_unlock_all(dev);

	__drm_state_dump(dev, &p, true);



	return 0;

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE