Skip to content

L
linux
Switch branch/tag
  1. 23 Feb, 2015 6 commits
    • Daniel Vetter's avatar
      drm: WARN if drm_handle_vblank is called errornously · ee3c7795
      Daniel Vetter authored 
      KMS drivers are in full control of their irq and vblank handling, if
they get a vblank interrupt before drm_vblank_init or after
drm_vblank_cleanup that's just a driver bug.

For ums driver there's only r128 and radeon which support vblank, and
they call drm_vblank_init in their driver load functions. Which again
means that userspace can do whatever it wants with interrupt, vblank
structures will always be there.

So this should never happen, let's catch driver issues with a WARN_ON.
Motivated by some discussions with Imre.

v2: Use WARN_ON_ONCE as suggested by Imre.

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>
      ee3c7795
    • 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
    • Daniel Vetter's avatar
      drm/i915: Switch to drm_crtc variants of vblank functions · 1e3feefd
      Daniel Vetter authored 
      Where possible right now. Just a small step towards nirvana ...

v2: git add. Uggh. Noticed by Imre.

Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: default avatarImre Deak <imre.deak@intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      1e3feefd
    • Daniel Vetter's avatar
      drm/i915: Flatten DRIVER_MODESET checks in i915_irq.c · f3a5c3f6
      Daniel Vetter authored 
      UMS is no more!

Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: default avatarImre Deak <imre.deak@intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      f3a5c3f6
    • Daniel Vetter's avatar
      drm/i915: Drop pipe_enable checks in vblank funcs · 1efa2e35
      Daniel Vetter authored 
      With Ville's rework to use drm_crtc_vblank_on/off the core will take
care of rejecting drm_vblank_get calls when the pipe is off. Also the
core won't call the get_vblank_counter hooks in that case either. And
since we've dropped ums support recently we can now remove these
hacks, yay!

Noticed while trying to answer questions Laurent had about how the new
atomic helpers work.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: default avatarImre Deak <imre.deak@intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      1efa2e35
    • Daniel Vetter's avatar
      drm/irq: Add drm_crtc_vblank_reset · 9625604c
      Daniel Vetter authored 
      At driver load we need to tell the vblank code about the state of the
pipes, so that the logic around reject vblank_get when the pipe is off
works correctly.

Thus far i915 used drm_vblank_off, but one of the side-effects of it
is that it also saves the vblank counter. And for that it calls down
into the ->get_vblank_counter hook. Which isn't really a good idea
when the pipe is off for a few reasons:
- With runtime pm the register might not respond.
- If the pipe is off some datastructures might not be around or
  unitialized.

The later is what blew up on gen3: We look at intel_crtc->config to
compute the vblank counter, and for a disabled pipe at boot-up that's
just not there. Thus far this was papered over by a check for
intel_crtc->active, but I want to get rid of that (since it's fairly
race, vblank hooks are called from all kinds of places).

So prep for that by adding a _reset functions which only does what we
really need to be done at driver load: Mark the vblank pipe as off,
but don't do any vblank counter saving or event flushing - neither of
that is required.

v2: Clarify the code flow slightly as suggested by Ville.

v3: Fix kerneldoc spelling, spotted by Laurent.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com> (v2)
Acked-by: default avatarDave Airlie <airlied@redhat.com>
Acked-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      9625604c
  3. 13 Feb, 2015 34 commits
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7