| Message ID | 1431966291-1449-3-git-send-email-alexander.deucher@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19.05.2015 01:24, Alex Deucher wrote: > > @@ -96,10 +98,12 @@ static void radeon_dp_work_func(struct work_struct *work) > struct drm_connector *connector; > > /* this should take a mutex */ > + mutex_lock(&mode_config->mutex); This comment can be removed?
On 19 May 2015 at 12:27, Michel Dänzer <michel@daenzer.net> wrote: > On 19.05.2015 01:24, Alex Deucher wrote: >> >> @@ -96,10 +98,12 @@ static void radeon_dp_work_func(struct work_struct *work) >> struct drm_connector *connector; >> >> /* this should take a mutex */ >> + mutex_lock(&mode_config->mutex); > > This comment can be removed? I have vague memories of not doing this, because bad things happened. so keep an eye out for lockdep traces. Dave.
On Mon, May 18, 2015 at 11:24 PM, Dave Airlie <airlied@gmail.com> wrote: > On 19 May 2015 at 12:27, Michel Dänzer <michel@daenzer.net> wrote: >> On 19.05.2015 01:24, Alex Deucher wrote: >>> >>> @@ -96,10 +98,12 @@ static void radeon_dp_work_func(struct work_struct *work) >>> struct drm_connector *connector; >>> >>> /* this should take a mutex */ >>> + mutex_lock(&mode_config->mutex); >> >> This comment can be removed? > > I have vague memories of not doing this, because bad things happened. > > so keep an eye out for lockdep traces. I tested the non-MST handling pretty extensively, but I admit I didn't play with mst. Might be better to split this into two patches. Alex
On Mon, May 18, 2015 at 11:28:47PM -0400, Alex Deucher wrote: > On Mon, May 18, 2015 at 11:24 PM, Dave Airlie <airlied@gmail.com> wrote: > > On 19 May 2015 at 12:27, Michel Dänzer <michel@daenzer.net> wrote: > >> On 19.05.2015 01:24, Alex Deucher wrote: > >>> > >>> @@ -96,10 +98,12 @@ static void radeon_dp_work_func(struct work_struct *work) > >>> struct drm_connector *connector; > >>> > >>> /* this should take a mutex */ > >>> + mutex_lock(&mode_config->mutex); > >> > >> This comment can be removed? > > > > I have vague memories of not doing this, because bad things happened. > > > > so keep an eye out for lockdep traces. > > I tested the non-MST handling pretty extensively, but I admit I didn't > play with mst. Might be better to split this into two patches. Registering a new connector also needs the dev->mode_config.mutex. If you hold that while calling into mst hpd code you'll deadlock. -Daniel
diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c index 7162c93..5b12c3f 100644 --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c @@ -79,10 +79,12 @@ static void radeon_hotplug_work_func(struct work_struct *work) struct drm_mode_config *mode_config = &dev->mode_config; struct drm_connector *connector; + mutex_lock(&mode_config->mutex); if (mode_config->num_connector) { list_for_each_entry(connector, &mode_config->connector_list, head) radeon_connector_hotplug(connector); } + mutex_unlock(&mode_config->mutex); /* Just fire off a uevent and let userspace tell us what to do */ drm_helper_hpd_irq_event(dev); } @@ -96,10 +98,12 @@ static void radeon_dp_work_func(struct work_struct *work) struct drm_connector *connector; /* this should take a mutex */ + mutex_lock(&mode_config->mutex); if (mode_config->num_connector) { list_for_each_entry(connector, &mode_config->connector_list, head) radeon_connector_hotplug(connector); } + mutex_unlock(&mode_config->mutex); } /** * radeon_driver_irq_preinstall_kms - drm irq preinstall callback
Since we are messing with state in the worker. Signed-off-by: Alex Deucher <alexander.deucher@amd.com> Cc: stable@vger.kernel.org --- drivers/gpu/drm/radeon/radeon_irq_kms.c | 4 ++++ 1 file changed, 4 insertions(+)