| Message ID | 1357850897-27102-13-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 10 Jan 2013 21:47:53 +0100 Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > Only two places: > - suspend/resume > - Some really strange mode validation tool with too much funny-lucking > hand-rolled conversion code. > - The recently-added lastclose fbdev restore code. > > Better safe than sorry, so convert both places to keep the locking > semantics as much as possible. Seems fine. The last close behaviour really ought to be in the core DRM code. Telling people to discover magic incantations with sysrq is a total fail, especially on tablets and other devices that don't *have* a sysrq key. Alan
On Thu, Jan 10, 2013 at 11:36 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote: > On Thu, 10 Jan 2013 21:47:53 +0100 > Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > >> Only two places: >> - suspend/resume >> - Some really strange mode validation tool with too much funny-lucking >> hand-rolled conversion code. >> - The recently-added lastclose fbdev restore code. >> >> Better safe than sorry, so convert both places to keep the locking >> semantics as much as possible. > > Seems fine. The last close behaviour really ought to be in the core DRM > code. Telling people to discover magic incantations with sysrq is a total > fail, especially on tablets and other devices that don't *have* a sysrq > key. I've looked into doing that, but the fbdev emulation is an (optional) driver internal detail. So I've opted for the 2nd best option and unified the logic on all drivers that implement it - some (arm) drivers don't even bother with this :( I think the entire fbdev emulation vs native kms client stuff is ripe for overhaul, see my other patch to prevent fbdev from causing havoc with the i915 kernel testsuite. -Daniel
diff --git a/drivers/gpu/drm/gma500/psb_device.c b/drivers/gpu/drm/gma500/psb_device.c index b58c470..f6f534b 100644 --- a/drivers/gpu/drm/gma500/psb_device.c +++ b/drivers/gpu/drm/gma500/psb_device.c @@ -194,7 +194,7 @@ static int psb_save_display_registers(struct drm_device *dev) regs->saveCHICKENBIT = PSB_RVDC32(DSPCHICKENBIT); /* Save crtc and output state */ - mutex_lock(&dev->mode_config.mutex); + drm_modeset_lock_all(dev); list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { if (drm_helper_crtc_in_use(crtc)) crtc->funcs->save(crtc); @@ -204,7 +204,7 @@ static int psb_save_display_registers(struct drm_device *dev) if (connector->funcs->save) connector->funcs->save(connector); - mutex_unlock(&dev->mode_config.mutex); + drm_modeset_unlock_all(dev); return 0; } @@ -234,7 +234,7 @@ static int psb_restore_display_registers(struct drm_device *dev) /*make sure VGA plane is off. it initializes to on after reset!*/ PSB_WVDC32(0x80000000, VGACNTRL); - mutex_lock(&dev->mode_config.mutex); + drm_modeset_lock_all(dev); list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) if (drm_helper_crtc_in_use(crtc)) crtc->funcs->restore(crtc); @@ -243,7 +243,7 @@ static int psb_restore_display_registers(struct drm_device *dev) if (connector->funcs->restore) connector->funcs->restore(connector); - mutex_unlock(&dev->mode_config.mutex); + drm_modeset_unlock_all(dev); return 0; } diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index dbcefe9..111e3df 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -153,11 +153,11 @@ static void psb_lastclose(struct drm_device *dev) struct drm_psb_private *dev_priv = dev->dev_private; struct psb_fbdev *fbdev = dev_priv->fbdev; - mutex_lock(&dev->mode_config.mutex); + drm_modeset_lock_all(dev); ret = drm_fb_helper_restore_fbdev_mode(&fbdev->psb_fb_helper); if (ret) DRM_DEBUG("failed to restore crtc mode\n"); - mutex_unlock(&dev->mode_config.mutex); + drm_modeset_unlock_all(dev); return; } @@ -486,7 +486,7 @@ static int psb_mode_operation_ioctl(struct drm_device *dev, void *data, case PSB_MODE_OPERATION_MODE_VALID: umode = &arg->mode; - mutex_lock(&dev->mode_config.mutex); + drm_modeset_lock_all(dev); obj = drm_mode_object_find(dev, obj_id, DRM_MODE_OBJECT_CONNECTOR); @@ -535,7 +535,7 @@ static int psb_mode_operation_ioctl(struct drm_device *dev, void *data, if (mode) drm_mode_destroy(dev, mode); mode_op_out: - mutex_unlock(&dev->mode_config.mutex); + drm_modeset_unlock_all(dev); return ret; default:
Only two places: - suspend/resume - Some really strange mode validation tool with too much funny-lucking hand-rolled conversion code. - The recently-added lastclose fbdev restore code. Better safe than sorry, so convert both places to keep the locking semantics as much as possible. Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/gma500/psb_device.c | 8 ++++---- drivers/gpu/drm/gma500/psb_drv.c | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-)