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@linux.intel.com
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@keithp.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Harry Wentland harry.wentland@amd.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
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