On 2018-03-06 07:18 AM, Ville Syrjälä wrote:
> On Tue, Mar 06, 2018 at 10:31:27AM +0100, Daniel Vetter wrote:
>> On Tue, Feb 27, 2018 at 02:57:00PM +0200, Ville Syrjala wrote:
>>> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>>>
>>> edid and display_info are protected by mode_config.mutex. Add lockdep
>>> asserts to make sure we're not accessing things w/o the lock.
>>>
>>> FIXME: pretty sure this will blow up with amdgpu as they seem
>>> to be doing edid updates even from the modeset path. Need to figure
>>> out what to do about that. Maybe protect the edid/display info with
>>> with connection_mutex instead of mode_config.mutex?
>>
>> Imo not doing EDID udpates from the modeset path is the right fix. I can't
>> think of any reasonable reason to do that at least. Can you point me at
>> the relevant amdgpu code pls (I'm lazy, sry)?
>
> It was some MST thing I believe... (should have written it down)
>
> amdgpu_dm_atomic_check() -> dm_update_crtcs_state() ->
> create_stream_for_sink() -> dm_dp_mst_dc_sink_create() ->
> get_edid/update_edid_property
>
Yeah, it's because the dc_sink carries the EDID and is only created at this point for us. It's bugged me ever since we did this. Might be time to think of a solution to it now.
Harry
>>
>> Otherwise I think this is a real good patch.
>>
>> Thanks, Daniel
>>
>>>
>>> Cc: Keith Packard <keithp@xxxxxxxxxx>
>>> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
>>> Cc: Harry Wentland <harry.wentland@xxxxxxx>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>>> ---
>>> drivers/gpu/drm/drm_connector.c | 4 ++++
>>> drivers/gpu/drm/drm_edid.c | 2 ++
>>> drivers/gpu/drm/drm_probe_helper.c | 2 +-
>>> 3 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>>> index 122060792b6f..a9f3536f4e94 100644
>>> --- a/drivers/gpu/drm/drm_connector.c
>>> +++ b/drivers/gpu/drm/drm_connector.c
>>> @@ -1374,6 +1374,8 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector,
>>> size_t size = 0;
>>> int ret;
>>>
>>> + lockdep_assert_held(&dev->mode_config.mutex);
>>> +
>>> /* ignore requests to set edid when overridden */
>>> if (connector->override_edid)
>>> return 0;
>>> @@ -1770,6 +1772,8 @@ void drm_connector_reset_display_info(struct drm_connector *connector)
>>> {
>>> struct drm_display_info *info = &connector->display_info;
>>>
>>> + lockdep_assert_held(&connector->dev->mode_config.mutex);
>>> +
>>> memset(info, 0, sizeof(*info));
>>> }
>>> EXPORT_SYMBOL_GPL(drm_connector_reset_display_info);
>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>>> index 618093c4a039..7f9e9236114b 100644
>>> --- a/drivers/gpu/drm/drm_edid.c
>>> +++ b/drivers/gpu/drm/drm_edid.c
>>> @@ -4440,6 +4440,8 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi
>>> struct drm_display_info *info = &connector->display_info;
>>> u32 quirks = edid_get_quirks(edid);
>>>
>>> + lockdep_assert_held(&connector->dev->mode_config.mutex);
>>> +
>>> info->width_mm = edid->width_cm * 10;
>>> info->height_mm = edid->height_cm * 10;
>>>
>>> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
>>> index 7dc7e635d7e4..2a2afcf72788 100644
>>> --- a/drivers/gpu/drm/drm_probe_helper.c
>>> +++ b/drivers/gpu/drm/drm_probe_helper.c
>>> @@ -400,7 +400,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>>> enum drm_connector_status old_status;
>>> struct drm_modeset_acquire_ctx ctx;
>>>
>>> - WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
>>> + lockdep_assert_held(&dev->mode_config.mutex);
>>>
>>> drm_modeset_acquire_init(&ctx, 0);
>>>
>>> --
>>> 2.13.6
>>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
