diff mbox

[0426/1094] drm/fb-helper: improve drm_fb_helper_initial_config locking

Message ID 1413889294-31328-427-git-send-email-dheerajx.s.jamwal@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dheeraj Jamwal Oct. 21, 2014, 10:50 a.m. UTC 
From: Daniel Vetter <daniel.vetter@ffwll.ch>

The locking in drm_fb_helper_initial_config is a bit troublesome for a
few reasons:

- We can't just wrap the entire function up into modeset locks since
  the fbdev registration might call down into fbcon code, which then
  through our ->set_par implementation needs to be able to grab all
  modeset locks. So we'd have a neat deadlock.

- This implies though that all current callers don't hold any modeset
  locks by necessity, so we have free reign to grab any modeset locks
  we need to grab.

- The private state of the fbdev helper doesn't need any protection
  through locks, since once we have the fbdev registered it is mostly
  invariant or protected through the modeset locking in ->set_par and
  other callbacks. We can fully rely on driver having non-racy setup
  sequences here. For the initial config computation we actually may
  not grab locks since drivers which provide their own magic sauce
  (like i915) might need to grab locks themselves.

- We should grab locks though when we probe outputs. Currently there's
  not much risk, but already now userspace could start poking at sysfs
  files and so probe concurrently. I expect that in the future driver
  init will be much more async, and since probing is really
  time-consuming this is a prime candidate.

- We must not hold any crtc->mutex locks while calling probe functions
  since those might need to lock a crtc for e.g. load detection. i915
  is such a driver.

Also it's the probing calls which hit upon piles of new locking
asserts I've recently added in

commit 62ff94a5492175759546f8bc61383189d6b49122
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Thu Jan 23 22:18:47 2014 +0100

    drm/crtc-helper: remove LOCKING from kerneldoc

and

commit 63951385052f7974155fa38f962f0f4e9847f90a
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Thu Jan 23 15:14:15 2014 +0100

    drm/doc: Repleace LOCKING kerneldoc sections in drm_modes.c

Hence the right fix is to grab the mode_config mutex, but only that
and only right around the probe calls.

It seems to be sufficient to shut up all the locking WARNINGs I see on
i915 and nouveau in drm_fb_helper_initial_config.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Tested-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
(cherry picked from commit 53f1904bced78d7c00f5d874c662ec3ac85d0f9f)

Signed-off-by: Dheeraj Jamwal <dheerajx.s.jamwal@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c |    2 ++
 1 file changed, 2 insertions(+)
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 060d25a..0a19b02 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1541,9 +1541,11 @@  bool drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
 
 	drm_fb_helper_parse_command_line(fb_helper);
 
+	mutex_lock(&dev->mode_config.mutex);
 	count = drm_fb_helper_probe_connector_modes(fb_helper,
 						    dev->mode_config.max_width,
 						    dev->mode_config.max_height);
+	mutex_unlock(&dev->mode_config.mutex);
 	/*
 	 * we shouldn't end up with no modes here.
 	 */