mirror of https://gitee.com/openkylin/linux.git
drm/irq: Don't call ->get_vblank_counter directly from irq_uninstall/cleanup
The pipe might already have been shut down, and then it's not a good idea to call hw accessor functions. Instead use the same logic as drm_vblank_off which has all the necessary checks to avoid troubles or inconsistency. Noticed by Imre while reviewing my patches to remove some sanity checks from ->get_vblank_counter. v2: Try harder. disable_and_save can still access the vblank stuff when vblank->enabled isn't set. It has to, since vlbank irq could be disable but the pipe is still on when being called from drm_vblank_off. But we still want to use that code for more code sharing. So add a check for vblank->enabled on top - if that's not set we shouldn't have anyone waiting for the vblank. If we have that's a pretty serious bug. The other issue that Imre spotted is drm_vblank_cleanup. That code again calls disable_and_save and so suffers from the same issues. But really drm_irq_uninstall should have cleaned that all up, so replace the code with WARN_ON. Note that we can't delete the timer cleanup since drivers aren't required to use drm_irq_install/uninstall, but can do their own irq handling. v3: Make it clear that all that gunk in drm_irq_uninstall is really just bandaids for UMS races between the irq/vblank code. In UMS userspace is in control of enabling/disabling interrupts in general and vblanks specifically. v4: Imre observed that KMS drivers all call drm_vblank_cleanup before drm_irq_uninstall (as they should), so again the code in there is dead for KMS (due to dev->num_crtcs == 0 after drm_vblank_cleanup). Or should be, so only WARN for KMS - with UMS userspace could try to do evil things. v5: After more discussion on irc we've gone back to v3: the del_timer_sync is required in all cases in drm_vblank_cleanup, but let's restrict the WARN_ON to kms drivers only. Imre was also concerned that bad things could happen without the disable_and_save call. But we immediately free vblank structures afterwards which makes the save useless. And drm_handle_vblank has a check for dev->num_crtcs to avoid surprises with ums. Cc: Imre Deak <imre.deak@intel.com> Reviewed-by: Imre Deak <imre.deak@intel.com> Acked-by: Dave Airlie <airlied@redhat.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
This commit is contained in:
parent
1e3feefd5a
commit
3bff93d64c
|
@ -269,7 +269,6 @@ static void vblank_disable_fn(unsigned long arg)
|
||||||
void drm_vblank_cleanup(struct drm_device *dev)
|
void drm_vblank_cleanup(struct drm_device *dev)
|
||||||
{
|
{
|
||||||
int crtc;
|
int crtc;
|
||||||
unsigned long irqflags;
|
|
||||||
|
|
||||||
/* Bail if the driver didn't call drm_vblank_init() */
|
/* Bail if the driver didn't call drm_vblank_init() */
|
||||||
if (dev->num_crtcs == 0)
|
if (dev->num_crtcs == 0)
|
||||||
|
@ -278,11 +277,10 @@ void drm_vblank_cleanup(struct drm_device *dev)
|
||||||
for (crtc = 0; crtc < dev->num_crtcs; crtc++) {
|
for (crtc = 0; crtc < dev->num_crtcs; crtc++) {
|
||||||
struct drm_vblank_crtc *vblank = &dev->vblank[crtc];
|
struct drm_vblank_crtc *vblank = &dev->vblank[crtc];
|
||||||
|
|
||||||
del_timer_sync(&vblank->disable_timer);
|
WARN_ON(vblank->enabled &&
|
||||||
|
drm_core_check_feature(dev, DRIVER_MODESET));
|
||||||
|
|
||||||
spin_lock_irqsave(&dev->vbl_lock, irqflags);
|
del_timer_sync(&vblank->disable_timer);
|
||||||
vblank_disable_and_save(dev, crtc);
|
|
||||||
spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
kfree(dev->vblank);
|
kfree(dev->vblank);
|
||||||
|
@ -468,17 +466,23 @@ int drm_irq_uninstall(struct drm_device *dev)
|
||||||
dev->irq_enabled = false;
|
dev->irq_enabled = false;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Wake up any waiters so they don't hang.
|
* Wake up any waiters so they don't hang. This is just to paper over
|
||||||
|
* isssues for UMS drivers which aren't in full control of their
|
||||||
|
* vblank/irq handling. KMS drivers must ensure that vblanks are all
|
||||||
|
* disabled when uninstalling the irq handler.
|
||||||
*/
|
*/
|
||||||
if (dev->num_crtcs) {
|
if (dev->num_crtcs) {
|
||||||
spin_lock_irqsave(&dev->vbl_lock, irqflags);
|
spin_lock_irqsave(&dev->vbl_lock, irqflags);
|
||||||
for (i = 0; i < dev->num_crtcs; i++) {
|
for (i = 0; i < dev->num_crtcs; i++) {
|
||||||
struct drm_vblank_crtc *vblank = &dev->vblank[i];
|
struct drm_vblank_crtc *vblank = &dev->vblank[i];
|
||||||
|
|
||||||
|
if (!vblank->enabled)
|
||||||
|
continue;
|
||||||
|
|
||||||
|
WARN_ON(drm_core_check_feature(dev, DRIVER_MODESET));
|
||||||
|
|
||||||
|
vblank_disable_and_save(dev, i);
|
||||||
wake_up(&vblank->queue);
|
wake_up(&vblank->queue);
|
||||||
vblank->enabled = false;
|
|
||||||
vblank->last =
|
|
||||||
dev->driver->get_vblank_counter(dev, i);
|
|
||||||
}
|
}
|
||||||
spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
|
spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue