On 2017-09-01 05:40 PM, Daniel Vetter wrote:
> On Fri, Sep 1, 2017 at 8:50 PM, Harry Wentland <harry.wentland at amd.com> wrote:
>> This is no longer needed in 4.13
>>
>> Signed-off-by: Harry Wentland <harry.wentland at amd.com>
>> ---
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 12 +-----------
>> 1 file changed, 1 insertion(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> index 3ce087f4e0ef..e41bb0bb0d66 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> @@ -241,7 +241,6 @@ static struct drm_connector *dm_dp_add_mst_connector(struct drm_dp_mst_topology_
>> struct amdgpu_connector *aconnector;
>> struct drm_connector *connector;
>>
>> - drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>> list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>
> You need the fancy new connector_list iterators here, otherwise this
> isn't safe. You probably want to add this as another audit-item to
> your TODO.
> -Daniel
Makes sense. Gonna post a v2.
>
>> aconnector = to_amdgpu_connector(connector);
>> if (aconnector->mst_port == master
>> @@ -252,11 +251,9 @@ static struct drm_connector *dm_dp_add_mst_connector(struct drm_dp_mst_topology_
>> aconnector->port = port;
>> drm_mode_connector_set_path_property(connector, pathprop);
>>
>> - drm_modeset_unlock(&dev->mode_config.connection_mutex);
>> return &aconnector->base;
>> }
>> }
>> - drm_modeset_unlock(&dev->mode_config.connection_mutex);
>>
>> aconnector = kzalloc(sizeof(*aconnector), GFP_KERNEL);
>> if (!aconnector)
>> @@ -349,7 +346,6 @@ static void dm_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
>> struct edid *edid;
>> struct dc_sink *dc_sink;
>>
>> - drm_modeset_lock_all(dev);
>> list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>> aconnector = to_amdgpu_connector(connector);
>> if (aconnector->port &&
>> @@ -400,7 +396,6 @@ static void dm_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
>> aconnector->edid);
>> }
>> }
>> - drm_modeset_unlock_all(dev);
>>
>> schedule_work(&adev->dm.mst_hotplug_work);
>> }
>> @@ -411,15 +406,12 @@ static void dm_dp_mst_register_connector(struct drm_connector *connector)
>> struct amdgpu_device *adev = dev->dev_private;
>> int i;
>>
>> - drm_modeset_lock_all(dev);
>> if (adev->mode_info.rfbdev) {
>> /*Do not add if already registered in past*/
>> for (i = 0; i < adev->mode_info.rfbdev->helper.connector_count; i++) {
>
> Don't digg around in helper internals. Especially not since you're not
> holding the right locks.
>
> Just from what I've spotted in your context here this all smells
> mildly fishy, but I'm too lazy to digg out the latest dc code from
> Alex' tree.
>
> If you do it like radeon then it /should/ be ok, no idea why this
> stuff here crept in.
> -Daniel
>
This was added after porting the radeon code and is supposed to solve
some issue when going headless. I'm gonna take it out and see if I can
find someone to test it thoroughly.
Andrey, do you remember what caused things to go haywire here? Headless
in fbcon mode?
Harry
>> if (adev->mode_info.rfbdev->helper.connector_info[i]->connector
>> - == connector) {
>> - drm_modeset_unlock_all(dev);
>> + == connector)
>> return;
>> - }
>> }
>>
>> drm_fb_helper_add_one_connector(&adev->mode_info.rfbdev->helper, connector);
>> @@ -427,8 +419,6 @@ static void dm_dp_mst_register_connector(struct drm_connector *connector)
>> else
>> DRM_ERROR("adev->mode_info.rfbdev is NULL\n");
>>
>> - drm_modeset_unlock_all(dev);
>> -
>> drm_connector_register(connector);
>>
>> }
>> --
>> 2.11.0
>>
>
>
>
