* [PATCH] drm: Fixup locking WARNINGs in drm_mode_config_reset
@ 2015-07-29 6:27 Daniel Vetter
2015-07-29 6:32 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2015-07-29 6:27 UTC (permalink / raw)
To: Intel Graphics Development
Cc: Daniel Vetter, Laurent Pinchart, DRI Development, Daniel Vetter
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);
--
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] 6+ messages in thread* [PATCH] drm: Fixup locking WARNINGs in drm_mode_config_reset
2015-07-29 6:27 [PATCH] drm: Fixup locking WARNINGs in drm_mode_config_reset Daniel Vetter
@ 2015-07-29 6:32 ` Daniel Vetter
2015-07-29 8:19 ` Daniel Vetter
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Daniel Vetter @ 2015-07-29 6:32 UTC (permalink / raw)
To: Intel Graphics Development
Cc: Daniel Vetter, Laurent Pinchart, DRI Development, Daniel Vetter
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);
--
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] 6+ messages in thread* Re: [PATCH] drm: Fixup locking WARNINGs in drm_mode_config_reset
2015-07-29 6:32 ` Daniel Vetter
@ 2015-07-29 8:19 ` Daniel Vetter
2015-07-29 20:15 ` Laurent Pinchart
2015-07-30 7:44 ` shuang.he
2 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2015-07-29 8:19 UTC (permalink / raw)
To: Intel Graphics Development
Cc: Daniel Vetter, Laurent Pinchart, DRI Development, Daniel Vetter
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_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>
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
>
--
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] 6+ messages in thread
* Re: [PATCH] drm: Fixup locking WARNINGs in drm_mode_config_reset
2015-07-29 6:32 ` Daniel Vetter
2015-07-29 8:19 ` Daniel Vetter
@ 2015-07-29 20:15 ` Laurent Pinchart
2015-07-29 22:24 ` Daniel Vetter
2015-07-30 7:44 ` shuang.he
2 siblings, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2015-07-29 20:15 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, DRI Development
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_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>
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);
--
Regards,
Laurent Pinchart
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: Fixup locking WARNINGs in drm_mode_config_reset
2015-07-29 20:15 ` Laurent Pinchart
@ 2015-07-29 22:24 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2015-07-29 22:24 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
Daniel Vetter
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_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>
>
> 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
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: Fixup locking WARNINGs in drm_mode_config_reset
2015-07-29 6:32 ` Daniel Vetter
2015-07-29 8:19 ` Daniel Vetter
2015-07-29 20:15 ` Laurent Pinchart
@ 2015-07-30 7:44 ` shuang.he
2 siblings, 0 replies; 6+ messages in thread
From: shuang.he @ 2015-07-30 7:44 UTC (permalink / raw)
To: shuang.he, julianx.dumez, christophe.sureau, lei.a.liu,
intel-gfx, daniel.vetter
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6887
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
ILK -1 297/297 296/297
SNB 315/315 315/315
IVB 342/342 342/342
BYT 282/282 282/282
HSW 378/378 378/378
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*ILK igt@kms_flip@flip-vs-dpms-interruptible PASS(1) DMESG_WARN(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] 6+ messages in thread
end of thread, other threads:[~2015-07-30 7:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-29 6:27 [PATCH] drm: Fixup locking WARNINGs in drm_mode_config_reset Daniel Vetter
2015-07-29 6:32 ` Daniel Vetter
2015-07-29 8:19 ` Daniel Vetter
2015-07-29 20:15 ` Laurent Pinchart
2015-07-29 22:24 ` Daniel Vetter
2015-07-30 7:44 ` shuang.he
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.