[PATCH] drm/radeon: take the mode_config mutex when dealing with hpds

2015-05-19 Thread Dave Airlie 
On 19 May 2015 at 12:27, Michel Dänzer  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(_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.

[PATCH] drm/radeon: take the mode_config mutex when dealing with hpds (v2)

2015-05-19 Thread Alex Deucher 
Since we are messing with state in the worker.

v2: drop the changes in the mst worker

Signed-off-by: Alex Deucher 
Cc: stable at vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_irq_kms.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c 
b/drivers/gpu/drm/radeon/radeon_irq_kms.c
index 7162c93..f682e53 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 = >mode_config;
struct drm_connector *connector;

+   mutex_lock(_config->mutex);
if (mode_config->num_connector) {
list_for_each_entry(connector, _config->connector_list, 
head)
radeon_connector_hotplug(connector);
}
+   mutex_unlock(_config->mutex);
/* Just fire off a uevent and let userspace tell us what to do */
drm_helper_hpd_irq_event(dev);
 }
-- 
1.8.3.1

[PATCH] drm/radeon: take the mode_config mutex when dealing with hpds

2015-05-19 Thread Michel Dänzer 
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(_config->mutex);

This comment can be removed?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

[PATCH] drm/radeon: take the mode_config mutex when dealing with hpds

2015-05-19 Thread Daniel Vetter 
On Mon, May 18, 2015 at 11:28:47PM -0400, Alex Deucher wrote:
> On Mon, May 18, 2015 at 11:24 PM, Dave Airlie  wrote:
> > On 19 May 2015 at 12:27, Michel Dänzer  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(_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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH] drm/radeon: take the mode_config mutex when dealing with hpds

2015-05-19 Thread Alex Deucher 
On Mon, May 18, 2015 at 11:24 PM, Dave Airlie  wrote:
> On 19 May 2015 at 12:27, Michel Dänzer  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(_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

[PATCH] drm/radeon: take the mode_config mutex when dealing with hpds

2015-05-18 Thread Alex Deucher 
Since we are messing with state in the worker.

Signed-off-by: Alex Deucher 
Cc: stable at vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_irq_kms.c | 4 
 1 file changed, 4 insertions(+)

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 = >mode_config;
struct drm_connector *connector;

+   mutex_lock(_config->mutex);
if (mode_config->num_connector) {
list_for_each_entry(connector, _config->connector_list, 
head)
radeon_connector_hotplug(connector);
}
+   mutex_unlock(_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(_config->mutex);
if (mode_config->num_connector) {
list_for_each_entry(connector, _config->connector_list, 
head)
radeon_connector_hotplug(connector);
}
+   mutex_unlock(_config->mutex);
 }
 /**
  * radeon_driver_irq_preinstall_kms - drm irq preinstall callback
-- 
1.8.3.1