• Daniel Vetter's avatar
    drm/irq: Don't call ->get_vblank_counter directly from irq_uninstall/cleanup · 3bff93d6
    Daniel Vetter authored
    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: default avatarImre Deak <imre.deak@intel.com>
    Acked-by: default avatarDave Airlie <airlied@redhat.com>
    Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
    3bff93d6
drm_irq.c 51.7 KB