With
commit 7a3f3d6667f5f9ffd1517f6b21d64bbf5312042c Author: Daniel Vetter daniel.vetter@ffwll.ch Date: Thu Jul 9 23:44:28 2015 +0200
drm: Check locking in drm_for_each_connector
we started checking the locking in drm_for_each_connector but somehow I totally missed drm_mode_config_reset. There's no problem there since this function should only be called in single-threaded contexts (driver load or resume), so just wrap the loop with the right lock.
Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Reported-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/drm_crtc.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index c91c18b2b1d4..a8e5ca79aad1 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5273,9 +5273,11 @@ void drm_mode_config_reset(struct drm_device *dev) if (encoder->funcs->reset) encoder->funcs->reset(encoder);
+ mutex_lock(dev->mode_config.mutex); drm_for_each_connector(connector, dev) if (connector->funcs->reset) connector->funcs->reset(connector); + mutex_unlock(dev->mode_config.mutex); } EXPORT_SYMBOL(drm_mode_config_reset);
With
commit 7a3f3d6667f5f9ffd1517f6b21d64bbf5312042c Author: Daniel Vetter daniel.vetter@ffwll.ch Date: Thu Jul 9 23:44:28 2015 +0200
drm: Check locking in drm_for_each_connector
we started checking the locking in drm_for_each_connector but somehow I totally missed drm_mode_config_reset. There's no problem there since this function should only be called in single-threaded contexts (driver load or resume), so just wrap the loop with the right lock.
v2: Drink coffee and all that ...
Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Reported-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/drm_crtc.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index c91c18b2b1d4..10c1a0f6680c 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5273,9 +5273,11 @@ void drm_mode_config_reset(struct drm_device *dev) if (encoder->funcs->reset) encoder->funcs->reset(encoder);
+ mutex_lock(&dev->mode_config.mutex); drm_for_each_connector(connector, dev) if (connector->funcs->reset) connector->funcs->reset(connector); + mutex_unlock(&dev->mode_config.mutex); } EXPORT_SYMBOL(drm_mode_config_reset);
On Wed, Jul 29, 2015 at 08:32:43AM +0200, Daniel Vetter wrote:
With
commit 7a3f3d6667f5f9ffd1517f6b21d64bbf5312042c Author: Daniel Vetter daniel.vetter@ffwll.ch Date: Thu Jul 9 23:44:28 2015 +0200
drm: Check locking in drm_for_each_connectorwe started checking the locking in drm_for_each_connector but somehow I totally missed drm_mode_config_reset. There's no problem there since this function should only be called in single-threaded contexts (driver load or resume), so just wrap the loop with the right lock.
v2: Drink coffee and all that ...
Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Reported-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
Tested-by: Jianwei Wang jianwei.wang.chn@gmail.com
drivers/gpu/drm/drm_crtc.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index c91c18b2b1d4..10c1a0f6680c 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5273,9 +5273,11 @@ void drm_mode_config_reset(struct drm_device *dev) if (encoder->funcs->reset) encoder->funcs->reset(encoder);
- mutex_lock(&dev->mode_config.mutex); drm_for_each_connector(connector, dev) if (connector->funcs->reset) connector->funcs->reset(connector);
- mutex_unlock(&dev->mode_config.mutex);
} EXPORT_SYMBOL(drm_mode_config_reset);
-- 2.1.4
Hi Daniel,
Thank you for the patch.
On Wednesday 29 July 2015 08:32:43 Daniel Vetter wrote:
With
commit 7a3f3d6667f5f9ffd1517f6b21d64bbf5312042c Author: Daniel Vetter daniel.vetter@ffwll.ch Date: Thu Jul 9 23:44:28 2015 +0200
drm: Check locking in drm_for_each_connectorwe started checking the locking in drm_for_each_connector but somehow I totally missed drm_mode_config_reset. There's no problem there since this function should only be called in single-threaded contexts (driver load or resume), so just wrap the loop with the right lock.
v2: Drink coffee and all that ...
Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Reported-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
This gets rid of the warning at driver load time with the rcar-du-drm driver.
Tested-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
drivers/gpu/drm/drm_crtc.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index c91c18b2b1d4..10c1a0f6680c 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5273,9 +5273,11 @@ void drm_mode_config_reset(struct drm_device *dev) if (encoder->funcs->reset) encoder->funcs->reset(encoder);
- mutex_lock(&dev->mode_config.mutex); drm_for_each_connector(connector, dev) if (connector->funcs->reset) connector->funcs->reset(connector);
- mutex_unlock(&dev->mode_config.mutex);
Wouldn't it make sense to protect the whole function with the mode_config mutex, given that it's documented as "protecting the KMS related lists and structures" ?
} EXPORT_SYMBOL(drm_mode_config_reset);
On Wed, Jul 29, 2015 at 11:15:23PM +0300, Laurent Pinchart wrote:
Hi Daniel,
Thank you for the patch.
On Wednesday 29 July 2015 08:32:43 Daniel Vetter wrote:
With
commit 7a3f3d6667f5f9ffd1517f6b21d64bbf5312042c Author: Daniel Vetter daniel.vetter@ffwll.ch Date: Thu Jul 9 23:44:28 2015 +0200
drm: Check locking in drm_for_each_connectorwe started checking the locking in drm_for_each_connector but somehow I totally missed drm_mode_config_reset. There's no problem there since this function should only be called in single-threaded contexts (driver load or resume), so just wrap the loop with the right lock.
v2: Drink coffee and all that ...
Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Reported-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
This gets rid of the warning at driver load time with the rcar-du-drm driver.
Tested-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
Thanks for testing, applied patch to drm-misc.
drivers/gpu/drm/drm_crtc.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index c91c18b2b1d4..10c1a0f6680c 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5273,9 +5273,11 @@ void drm_mode_config_reset(struct drm_device *dev) if (encoder->funcs->reset) encoder->funcs->reset(encoder);
- mutex_lock(&dev->mode_config.mutex); drm_for_each_connector(connector, dev) if (connector->funcs->reset) connector->funcs->reset(connector);
- mutex_unlock(&dev->mode_config.mutex);
Wouldn't it make sense to protect the whole function with the mode_config mutex, given that it's documented as "protecting the KMS related lists and structures" ?
Well that was correct ages ago I guess but since then we've split out tons of other locks. It doesn't protect anything else really since those lists are all static after driver load, and even here it's just to shut up the WARNING (which is right for runtime in general and not harmful to grab the lock really either).
I do have a plan to fix up kerneldoc, but I want to get the various improvements collabora is working on in first. -Daniel
} EXPORT_SYMBOL(drm_mode_config_reset);
-- Regards,
Laurent Pinchart
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Daniel Vetter -
Laurent Pinchart