drm/radeon: take the mode_config mutex when dealing with hpds
diff mbox

Message ID 1431966291-1449-3-git-send-email-alexander.deucher@amd.com
State New
Headers show

Commit Message

Alex Deucher May 18, 2015, 4:24 p.m. UTC
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(+)

Comments

Michel Dänzer May 19, 2015, 2:27 a.m. UTC | #1
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?
Dave Airlie May 19, 2015, 3:24 a.m. UTC | #2
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.
Alex Deucher May 19, 2015, 3:28 a.m. UTC | #3
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
Daniel Vetter May 19, 2015, 7:49 a.m. UTC | #4
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

Patch
diff mbox

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