1. 11 Jul, 2013 11 commits
    • Damien Lespiau's avatar
      drm/i915: Use for_each_pipe() when possible · 08e2a7de
      Damien Lespiau authored
      Came accross two open coding of for_each_pipe(), might as well use the
      macro.
      Signed-off-by: default avatarDamien Lespiau <damien.lespiau@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      08e2a7de
    • Daniel Vetter's avatar
      drm/i915: don't enable PM_VEBOX_CS_ERROR_INTERRUPT · c0d6a3dd
      Daniel Vetter authored
      The code to handle it is broken - there's simply no code to clear CS
      parser errors on gen5+. And behold, for all the other rings we also
      don't enable it!
      
      Leave the handling code itself in place just to be consistent with the
      existing mess though. And in case someone feels like fixing it all up.
      
      This has been errornously enabled in
      
      commit 12638c57
      Author: Ben Widawsky <ben@bwidawsk.net>
      Date:   Tue May 28 19:22:31 2013 -0700
      
          drm/i915: Enable vebox interrupts
      
      Cc: Damien Lespiau <damien.lespiau@intel.com>
      Cc: Ben Widawsky <ben@bwidawsk.net>
      Reviewed-by: default avatarBen Widawsky <ben@bwidawsk.net>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      c0d6a3dd
    • Daniel Vetter's avatar
      drm/i915: unify ring irq refcounts (again) · c7113cc3
      Daniel Vetter authored
      With the simplified locking there's no reason any more to keep the
      refcounts seperate.
      
      v2: Readd the lost comment that ring->irq_refcount is protected by
      dev_priv->irq_lock.
      Reviewed-by: default avatarBen Widawsky <ben@bwidawsk.net>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      c7113cc3
    • Daniel Vetter's avatar
      drm/i915: kill dev_priv->rps.lock · 59cdb63d
      Daniel Vetter authored
      Now that the rps interrupt locking isn't clearly separated (at elast
      conceptually) from all the other interrupt locking having a different
      lock stopped making sense: It protects much more than just the rps
      workqueue it started out with. But with the addition of VECS the
      separation started to blurr and resulted in some more complex locking
      for the ring interrupt refcount.
      
      With this we can (again) unifiy the ringbuffer irq refcounts without
      causing a massive confusion, but that's for the next patch.
      
      v2: Explain better why the rps.lock once made sense and why no longer,
      requested by Ben.
      Reviewed-by: default avatarBen Widawsky <ben@bwidawsk.net>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      59cdb63d
    • Daniel Vetter's avatar
      drm/i915: queue work outside spinlock in hsw_pm_irq_handler · 2adbee62
      Daniel Vetter authored
      And kill the comment about it. Queueing work is a barrier type event,
      no amount of locking will help in ordering things (as long as we queue
      the work after having updated all relevant data structures). Also, the
      queue_work works itself as a sufficient memory barrier.
      
      Again on the surface this is just a tiny micro-optimization to reduce
      the hold-time of dev_priv->irq_lock. But the better reason is that it
      reduces superficial locking and so makes it clearer what we actually
      need for correctness.
      Reviewed-by: default avatarBen Widawsky <ben@bwidawsk.net>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      2adbee62
    • Daniel Vetter's avatar
      drm/i915: streamline hsw_pm_irq_handler · 41a05a3a
      Daniel Vetter authored
      The if (pm_iir & ~GEN6_PM_RPS_EVENTS) check was redunandant. Otoh
      adding a check for rps events allows us to avoid the spinlock grabbing
      for VECS interrupts.
      
      v2: Drop misplaced hunk which now moved to the right patch.
      Reviewed-by: default avatarBen Widawsky <ben@bwidawsk.net>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      41a05a3a
    • Daniel Vetter's avatar
      drm/i915: irq handlers don't need interrupt-safe spinlocks · d0ecd7e2
      Daniel Vetter authored
      Since we only have one interrupt handler and interrupt handlers are
      non-reentrant.
      
      To drive the point really home give them all an _irq_handler suffix.
      
      This is a tiny micro-optimization but even more important it makes it
      clearer what locking we actually need. And in case someone screws this
      up: lockdep will catch hardirq vs. other context deadlocks.
      
      v2: Fix up compile fail.
      Reviewed-by: default avatarPaulo Zanoni <paulo.r.zanoni@intel.com>
      Reviewed-by: default avatarBen Widawsky <ben@bwidawsk.net>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      d0ecd7e2
    • Daniel Vetter's avatar
      drm/i915: kill lpt pch transcoder->crtc mapping code for fifo underruns · de28075d
      Daniel Vetter authored
      It's racy: There's no guarantee that we won't walk this code (due to a
      pch fifo underrun interrupt) while someone is changing the pointers
      around.
      
      The only reason we do this is to use the righ crtc for the pch fifo
      underrun accounting. But we never expose this to userspace, so
      essentially no one really cares if we use the "wrong" crtc.
      
      So let's just rip it out.
      
      With this patch fifo underrun code will always use crtc A for tracking
      underruns on the (only) pch transcoder on LPT.
      
      v2: Add a big comment explaining what's going on. Requested by Paulo.
      
      v3: Fixup spelling in comment as spotted by Paulo.
      
      Cc: Paulo Zanoni <przanoni@gmail.com>
      Reviewed-by: default avatarPaulo Zanoni <paulo.r.zanoni@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      de28075d
    • Daniel Vetter's avatar
      drm/i915: improve GEN7_ERR_INT clearing for fifo underrun reporting · 7336df65
      Daniel Vetter authored
      Same treatment as for SERR_INT: If we clear only the bit for the pipe
      we're enabling (but unconditionally) then we can always check for
      possible underruns after having disabled the interrupt. That way pipe
      underruns won't be lost, but at worst only get reported in a delayed
      fashion.
      
      v2: The same logic bug as in the SERR handling change also existed
      here. The same bugfix of only reporting missed underruns when the
      error interrupt was masked applies, too.
      
      v3: Do the same fixes as for the SERR handling that Paulo suggested in
      his review:
      - s/%i/%c/ fix in the debug output
      - move the DE_ERR_INT_IVB read into the respective if block
      
      Cc: Paulo Zanoni <przanoni@gmail.com>
      Reviewed-by: default avatarPaulo Zanoni <paulo.r.zanoni@intel.com>
      [danvet: Fix up the checkpatch bikeshed Paulo noticed.]
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      7336df65
    • Daniel Vetter's avatar
      drm/i915: improve SERR_INT clearing for fifo underrun reporting · 1dd246fb
      Daniel Vetter authored
      The current code won't report any fifo underruns on cpt if just one
      pipe has fifo underrun reporting disabled. We can't enable the
      interrupts, but we can still check the per-transcoder bits and so
      report the underrun delayed if:
      - We always clear the transcoder's bit (and none of the other bits)
        when enabling.
      - We check the transcoder's bit after disabling (to avoid racing with
        the interrupt handler).
      
      v2: I've forgotten to actually remove the old SERR_INT clearing.
      
      v3: Use transcoder_name as suggested by Paulo Zanoni. Paulo also
      noticed a logic bug: When an underrun interrupt fires we report it
      both in the interrupt handler and when checking for underruns when
      disabling it in cpt_set_fifo_underrun_reporting. But that second check
      is only required if the interrupt is disabled and we're switching of
      underrun reporting (e.g. because we're disabling the crtc). Hence
      check for that condition.
      
      At first I wanted to rework the code to pass that bit of information
      from the uppper functions down to cpt_set_fifo_underrun_reporting. But
      that turned out too messy. Hence the quick&dirty check whether the
      south error interrupt source is masked off or not.
      
      v4: Streamline the control flow a bit.
      
      v5: s/pipe/pch transcoder/ in the dmesg output, suggested by Paulo.
      
      v6: Review from Paulo:
      - Reorder the was_enabled assignment to only read the register when we
        need it. Also add a comment that we need to do that before updating
        the register.
      - s/%i/%c/ fix for the debug output.
      - Fix the checkpath complaint in the SERR_INT_TRANS_FIFO_UNDERRUN
        #define.
      
      v7: Hopefully put that elusive SERR hunk back into this patch, spotted
      by Paulo.
      
      Cc: Paulo Zanoni <przanoni@gmail.com>
      Reviewed-by: default avatarPaulo Zanoni <paulo.r.zanoni@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      1dd246fb
    • Daniel Vetter's avatar
      drm/i915: extract ibx_display_interrupt_update · fee884ed
      Daniel Vetter authored
      This way all changes to SDEIMR all go through the same function, with
      the exception of the (single-threaded) setup/teardown code.
      
      For paranoia again add an assert_spin_locked.
      
      v2: For even more paranoia also sprinkle a spinlock assert over
      cpt_can_enable_serr_int since we need to have that one there, too.
      
      v3: Fix the logic of interrupt enabling, add enable/disable macros for
      the simple cases in the fifo code and add a comment. All requested by
      Paulo.
      Reviewed-by: default avatarPaulo Zanoni <paulo.r.zanoni@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      fee884ed
  2. 10 Jul, 2013 2 commits
    • Maarten Lankhorst's avatar
    • Daniel Vetter's avatar
      drm/i915: don't frob mm.suspended when not using ums · db1b76ca
      Daniel Vetter authored
      In kernel modeset driver mode we're in full control of the chip,
      always. So there's no need at all to set mm.suspended in
      i915_gem_idle. Hence move that out into the leavevt ioctl. Since
      i915_gem_idle doesn't suspend gem any more we can also drop the
      re-enabling for KMS in the thaw function.
      
      Also clean up the handling of mm.suspend at driver load by coalescing
      all the assignments.
      
      Stumbled over while reading through our resume code for unrelated
      reasons.
      
      v2: Shovel mm.suspended into the (newly created) ums dungeon as
      suggested by Chris Wilson. The plan is that once we've completely
      stopped relying on the register save/restore code we could shovel even
      that in there.
      
      v3: Improve the locking for the entervt/leavevt ioctls a bit by moving
      the dev->struct_mutex locking outside of i915_gem_idle. Also don't
      clear dev_priv->ums.mm_suspended for the kms case, we allocate it with
      kzalloc. Both suggested by Chris Wilson.
      
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v2)
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      db1b76ca
  3. 09 Jul, 2013 4 commits
  4. 08 Jul, 2013 8 commits
    • Daniel Vetter's avatar
      drm/i915: convert debugfs creation/destruction to table · 34b9674c
      Daniel Vetter authored
      At least for the common cases where we only need special file
      operations. The forcewake file is still rather more special.
      
      v2: Fix up the debugfs unregister code.
      
      v3: Actually squash in the right fixup.
      Acked-by: default avatarBen Widawsky <ben@bwidawsk.net>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      34b9674c
    • Daniel Vetter's avatar
      drm/i915: dvo needs a P2 divisor of 4 · 5d536e28
      Daniel Vetter authored
      Section 1.5.4, "DPLL A Control Register" from Bspec about bit 23
      "FPA0/A1 P2 Clock Divide":
      
      0 = Divide by 2
      1 = Divide by 4. This bit must be set in DVO non-gang mode
      
      So copy the current limits (which should be good for i8xx) and create
      a new set for dvo encoders.
      Reviewed-by: default avatarChris Wilson <chris@chris-wilson.oc.uk>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      5d536e28
    • Daniel Vetter's avatar
      drm/i915: fix dvo DPLL regression · 4a33e48d
      Daniel Vetter authored
      I've missed that intel_dvo_mode_set changes the dpll configuration.
      Hence when I've reworked the sequence to only enable the dpll in the
      crtc_enable callback in
      
      commit 66e3d5c0
      Author: Daniel Vetter <daniel.vetter@ffwll.ch>
      Date:   Sun Jun 16 21:24:16 2013 +0200
      
          drm/i915: move i9xx dpll enabling into crtc enable function
      
      that special DVO bit was lost. Some BSpec reading confirms that it's
      only needed for DVO encoders. Section 1.5.4, "DPLL A Control Register"
      for bit 30:
      
      "2X Clock Enable. When driving In non-gang DVO modes such as a
      connected flat panel or TV, a 2X" version of the clock is needed. When
      not using the 2X output it should be disabled. This bit cannot be set
      when driving the integrated LVDS port on devices such as Montara-GM."
      
      Fix this regression up.
      
      Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66516
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Reported-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Partially-tested-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      4a33e48d
    • Ben Widawsky's avatar
      drm/i915: Embed drm_mm_node in i915 gem obj · c6cfb325
      Ben Widawsky authored
      Embedding the node in the obj is more natural in the transition to VMAs
      which will also have embedded nodes. This change also helps transition
      away from put_block to remove node.
      
      Though it's quite an uncommon occurrence, it's somewhat convenient to not
      fail at bind time because we cannot allocate the node. Though in
      practice there are other allocations (like the request structure) which
      would probably make this point not terribly useful.
      
      Quoting Daniel:
      Note that the only difference between put_block and remove_node is
      that the former fills up the preallocation cache. Which we don't need
      anyway and hence is just wasted space.
      
      v2: Clean up the stolen preallocation code.
      Rebased on the reserve_node patches
      renames ggtt_ stuff to gtt_ stuff
      WARN_ON if the object is already bound (which doesn't mean it's in the
      bound list, tricky)
      Signed-off-by: default avatarBen Widawsky <ben@bwidawsk.net>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      c6cfb325
    • Ben Widawsky's avatar
      drm/i915: Kill obj->gtt_offset · edd41a87
      Ben Widawsky authored
      With the getters in place from the previous patch this members serves no
      purpose other than saving one spare pointer chase, which will be killed
      in the next patch anyway.
      
      Moving to VMAs, this members adds unnecessary confusion since an object
      may exist at different offsets in different VMs.
      
      v2: Properly preserve the stolen offset. This code is a bit hacky but it
      all goes away when we embed the drm_mm_node and removes the need for the
      incorrect patch I submitted previously: "Use gtt_space->start for stolen
      reservation"
      Signed-off-by: default avatarBen Widawsky <ben@bwidawsk.net>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      edd41a87
    • Ben Widawsky's avatar
      drm/i915: Getter/setter for object attributes · f343c5f6
      Ben Widawsky authored
      Soon we want to gut a lot of our existing assumptions how many address
      spaces an object can live in, and in doing so, embed the drm_mm_node in
      the object (and later the VMA).
      
      It's possible in the future we'll want to add more getter/setter
      methods, but for now this is enough to enable the VMAs.
      
      v2: Reworked commit message (Ben)
      Added comments to the main functions (Ben)
      sed -i "s/i915_gem_obj_set_color/i915_gem_obj_ggtt_set_color/" drivers/gpu/drm/i915/*.[ch]
      sed -i "s/i915_gem_obj_bound/i915_gem_obj_ggtt_bound/" drivers/gpu/drm/i915/*.[ch]
      sed -i "s/i915_gem_obj_size/i915_gem_obj_ggtt_size/" drivers/gpu/drm/i915/*.[ch]
      sed -i "s/i915_gem_obj_offset/i915_gem_obj_ggtt_offset/" drivers/gpu/drm/i915/*.[ch]
      (Daniel)
      
      v3: Rebased on new reserve_node patch
      Changed DRM_DEBUG_KMS to actually work (will need fixing later)
      Signed-off-by: default avatarBen Widawsky <ben@bwidawsk.net>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      f343c5f6
    • Ben Widawsky's avatar
      drm: Change create block to reserve node · 338710e7
      Ben Widawsky authored
      With the previous patch we no longer actually create a node, we simply
      find the correct hole and occupy it. This very well could have been
      squashed with the last patch, but since I already had David's review, I
      figured it's easiest to keep it distinct.
      
      Also update the users in i915. Conveniently this is the only user of the
      interface.
      
      CC: David Airlie <airlied@linux.ie>
      CC: <dri-devel@lists.freedesktop.org>
      Signed-off-by: default avatarBen Widawsky <ben@bwidawsk.net>
      Acked-by: default avatarDavid Airlie <airlied@linux.ie>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      338710e7
    • Ben Widawsky's avatar
      drm: pre allocate node for create_block · b3a070cc
      Ben Widawsky authored
      For an upcoming patch where we introduce the i915 VMA, it's ideal to
      have the drm_mm_node as part of the VMA struct (ie. it's pre-allocated).
      Part of the conversion to VMAs is to kill off obj->gtt_space. Doing this
      will break a bunch of code, but amongst them are 2 callers of
      drm_mm_create_block(), both related to stolen memory.
      
      It also allows us to embed the drm_mm_node into the object currently
      which provides a nice transition over to the new code.
      
      v2: Reordered to do before ripping out obj->gtt_offset.
      Some minor cleanups made available because of reordering.
      
      v3: s/continue/break on failed stolen node allocation (David)
      Set obj->gtt_space on failed node allocation (David)
      Only unref stolen (fix double free) on failed create_stolen (David)
      Free node, and NULL it in failed create_stolen (David)
      Add back accidentally removed newline (David)
      
      CC: <dri-devel@lists.freedesktop.org>
      Reviewed-by: default avatarDavid Herrmann <dh.herrmann@gmail.com>
      Signed-off-by: default avatarBen Widawsky <ben@bwidawsk.net>
      Acked-by: default avatarDavid Airlie <airlied@linux.ie>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      b3a070cc
  5. 05 Jul, 2013 4 commits
  6. 02 Jul, 2013 2 commits
  7. 01 Jul, 2013 9 commits