1. 21 8月, 2013 1 次提交
  2. 19 8月, 2013 1 次提交
  3. 28 6月, 2013 1 次提交
    • D
      drm: add hotspot support for cursors. · 4c813d4d
      Dave Airlie 提交于
      So it looks like for virtual hw cursors on QXL we need to inform
      the "hw" device what the cursor hotspot parameters are. This
      makes sense if you think the host has to draw the cursor and interpret
      clicks from it. However the current modesetting interface doesn't support
      passing the hotspot information from userspace.
      
      This implements a new cursor ioctl, that takes the hotspot info as well,
      userspace can try calling the new interface and if it gets -ENOSYS it means
      its on an older kernel and can just fallback.
      Reviewed-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      4c813d4d
  4. 25 6月, 2013 2 次提交
    • D
      drm: fix fb leak in setcrtc · 5cef29aa
      Daniel Vetter 提交于
      Drivers are allowed (actually have to) disable unrelated crtcs in
      their ->set_config callback (when we steal all the connectors from
      that crtc). If they do that they'll clear crtc->fb to NULL.
      
      Which results in a refcount leak, since the drm core is keeping track
      of that reference.
      
      To fix this track the old fb of all crtcs and adjust references for
      all of them. Of course, since we only hold an additional reference for
      the fb for the current crtc we need to increase refcounts before we
      drop the old one.
      
      This approach has the benefit that it inches us a bit closer to an
      atomic modeset world, where we want to update the config of all crtcs
      in one step.
      
      This regression has been introduce in the framebuffer refcount
      conversion, specifically in
      
      commit b0d12325
      Author: Daniel Vetter <daniel.vetter@ffwll.ch>
      Date:   Tue Dec 11 01:07:12 2012 +0100
      
          drm: refcounting for crtc framebuffers
      Reported-by: NRussell King <linux@arm.linux.org.uk>
      Cc: Russell King <linux@arm.linux.org.uk>
      Cc: stable@vger.kernel.org
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      5cef29aa
    • D
      drm: check that ->set_config properly updates the fb · cc85e121
      Daniel Vetter 提交于
      Historically drm lacked fb refcounting, so the updating of crtc->fb
      was done by the lower levels at a point convenient to get their own
      refcounting (e.g. refcounts for the underlying gem bo, pinning
      refcounts) right. With the introduction of refcounted fbs the drm core
      handled the fb refcounts, but still relied on drivers to update the
      crtc->fb pointer (this approach required the least invasive changes in
      drivers).
      
      Enforce this contract with a WARN_ON.
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      cc85e121
  5. 17 6月, 2013 3 次提交
  6. 11 6月, 2013 4 次提交
  7. 13 5月, 2013 1 次提交
  8. 03 5月, 2013 1 次提交
  9. 30 4月, 2013 2 次提交
    • V
      drm: Kill user_modes list and the associated ioctls · c55b6b3d
      Ville Syrjälä 提交于
      There is no way to use modes added to the user_modes list. We never
      look at the contents of said list in the kernel, and the only operations
      userspace can do are attach and detach. So the only "benefit" of this
      interface is wasting kernel memory.
      
      Fortunately it seems no real user space application ever used these
      ioctls. So just kill them.
      
      Also remove the prototypes for the non-existing drm_mode_addmode_ioctl()
      and drm_mode_rmmode_ioctl() functions.
      
      v2: Use drm_noop instead of completely removing the ioctls
      Signed-off-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      c55b6b3d
    • V
      drm: Silence some sparse warnings · ea9cbb06
      Ville Syrjälä 提交于
      drivers/gpu/drm/drm_pci.c:155:5: warning: symbol 'drm_pci_set_busid' was not declared. Should it be static?
      drivers/gpu/drm/drm_pci.c:197:5: warning: symbol 'drm_pci_set_unique' was not declared. Should it be static?
      drivers/gpu/drm/drm_pci.c:269:5: warning: symbol 'drm_pci_agp_init' was not declared. Should it be static?
      
      drivers/gpu/drm/drm_crtc.c:181:1: warning: symbol 'drm_get_dirty_info_name' was not declared. Should it be static?
      drivers/gpu/drm/drm_crtc.c:1123:5: warning: symbol 'drm_mode_group_init' was not declared. Should it be static?
      
      drivers/gpu/drm/drm_modes.c:918:6: warning: symbol 'drm_mode_validate_clocks' was not declared. Should it be static?
      Signed-off-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      ea9cbb06
  10. 22 4月, 2013 1 次提交
  11. 16 4月, 2013 1 次提交
  12. 12 4月, 2013 1 次提交
  13. 28 3月, 2013 1 次提交
  14. 28 2月, 2013 2 次提交
  15. 22 2月, 2013 1 次提交
  16. 20 2月, 2013 3 次提交
  17. 18 2月, 2013 1 次提交
  18. 14 2月, 2013 1 次提交
    • D
      drm: review locking for drm_fb_helper_restore_fbdev_mode · 6aed8ec3
      Daniel Vetter 提交于
      ... it's required. Fix up exynos and the cma helper, and add a
      corresponding WARN_ON to drm_fb_helper_restore_fbdev_mode.
      
      Note that tegra calls the fbdev cma helper restore function also from
      it's driver-load callback. Which is a bit against current practice,
      since usually the call is only from ->lastclose, and initial setup is
      done by drm_fb_helper_initial_config.
      
      Also add the relevant drm DocBook entry.
      
      v2: Add promised WARN to restore_fbdev_mode.
      Reviewed-by: NRob Clark <robdclark@gmail.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      6aed8ec3
  19. 21 1月, 2013 12 次提交
    • D
      drm: don't hold crtc mutexes for connector ->detect callbacks · 7b24056b
      Daniel Vetter 提交于
      The coup de grace of the entire journey. No more dropped frames every
      10s on my testbox!
      
      I've tried to audit all ->detect and ->get_modes callbacks, but things
      became a bit fuzzy after trying to piece together the umpteenth
      implemenation. Afaict most drivers just have bog-standard output
      register frobbing with a notch of i2c edid reading, nothing which
      could potentially race with the newly concurrent pageflip/set_cursor
      code. The big exception is load-detection code which requires a
      running pipe, but radeon/nouveau seem to to this without touching any
      state which can be observed from page_flip (e.g. disabled crtcs
      temporarily getting enabled and so a pageflip succeeding).
      
      The only special case I could find is the i915 load detect code. That
      uses the normal modeset interface to enable the load-detect crtc, and
      so userspace could try to squeeze in a pageflip on the load-detect
      pipe. So we need to grab the relevant crtc mutex in there, to avoid
      the temporary crtc enabling to sneak out and be visible to userspace.
      
      Note that the sysfs files already stopped grabbing the per-crtc locks,
      since I didn't want to bother with doing a interruptible
      modeset_lock_all. But since there's very little in-between breakage
      (essentially just the ability for userspace to pageflip on load-detect
      crtcs when it shouldn't on the i915 driver) I figured I don't need to
      bother.
      Reviewed-by: NRob Clark <rob@ti.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      7b24056b
    • D
      drm: only grab the crtc lock for pageflips · b4d5e7d1
      Daniel Vetter 提交于
      The pagelip ioctl itself is rather simply, so the hard work for this
      patch is auditing all the drivers:
      
      - exynos: Pageflip is protect with dev->struct_mutex and ...
        synchronous. But nothing fancy going on, besides a check whether the
        crtc is enabled, which should probably be somewhere in the drm core
        so that we have unified behaviour across all drivers.
      
      - i915: hw-state is protected with dev->struct_mutex, the delayed
        unpin work together with the other stuff the pageflip complete irq
        handler needs is protected by the event_lock spinlock.
      
      - nouveau: With the pin/unpin functions fixed, everything looks safe:
        A bit of ttm wrestling and refcounting, and a few channel accesses.
        The later are either already proteced sufficiently, or are now safe
        with the channel locking introduced to make cursor updates safe.
      
      - radeon: The irq_get/put functions look a bit race, since the
        atomic_inc/dec isn't protect with locks. Otoh they're all per-crtc,
        so we should be safe with per-crtc locking from the drm core. Then
        there's tons of per-crtc register access, which could potentially go
        through the indirect reg acces. But that's fixed to make cursor
        updates concurrent. Bookeeping for the drm even is also protected
        with the even_lock, which also protects against the pageflip irq
        handler since radeon hw seems to have no way to queue these up
        asynchronously. Otherwise just a bit of ttm-based buffer handling
        and fencing, which is now safe with the previous patch to hold
        bdev->fence_lock while grabbing the ttm fence.
      
      - shmob: Only one crtc. That's an easy one ...
      
      - vmwgfx: As usual a bit special with tons different things:
        - Flippable check using is_implicit and num_implicit. Changes to
          those seem to be nicely covered with the global modeset lock, so
          we should be fine.
        - Some dirty cliprect handling stuff, or at least that is my guess.
          Looks like it's fine since either it's per-crtc, invariant or
          (like the execbuf stuff launched) protected otherwise.
        - Adding the actual flip to the fence_event list. On a quick look
          this seems to have solid locking in place, too.
        ... but generally this is all way over my head.
      
      - imx: Impressive display of races between the page_flip
        implementation and the irq handler. Also, ipu_drm_set_base which
        gets eventually called from the irq handler to update the display
        base isn't really protected against concurrent set_config calls from
        process context.  In any case, going for per-crtc locking won't make
        this worse, so nothing to do.
      
      - omap: The new async callback code merged into 3.8 seems to have
        solid locking in place, and there doesn't seem to be any shared
        state at risk. Especially since the callbacks still use
        modeset_lock_all and are so not converted.
      
      v2: Update omapdrm analysis to 3.8 code per the discussion with Rob
      Clark.
      Reviewed-by: NRob Clark <rob@ti.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      b4d5e7d1
    • D
      drm: optimize drm_framebuffer_remove · b62584e3
      Daniel Vetter 提交于
      Now that all framebuffer usage is properly refcounted, we are no
      longer required to hold the modeset locks while dropping the last
      reference. Hence implemented a fastpath which avoids the potential
      stalls associated with grabbing mode_config.lock for the case where
      there's no other reference around.
      
      Explain in a big comment why it is safe. Also update kerneldocs with
      the new locking rules around drm_framebuffer_remove.
      Reviewed-by: NRob Clark <rob@ti.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      b62584e3
    • D
      drm: refcounting for crtc framebuffers · b0d12325
      Daniel Vetter 提交于
      With the prep patch to encapsulate ->set_crtc calls, this is now
      rather easy. Hooray for inconsistent semantics between ->set_crtc and
      ->page_flip, where the driver callback is supposed to update the fb
      pointer, and ->update_plane, where the drm core does the same.
      
      Also, since the drm core functions check crtc->fb before calling into
      driver callbacks, we can't really reduce the critical sections
      protected by the mode_config locks.
      Reviewed-by: NRob Clark <rob@ti.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      b0d12325
    • D
      drm: refcounting for sprite framebuffers · 6c2a7532
      Daniel Vetter 提交于
      Now plane->fb holds a reference onto it's framebuffer. Nothing too
      fancy going on here:
      - Extract __drm_framebuffer_unreference to be called when we know
        we're not dropping the last reference, e.g. useful in the fb cleanup
        code.
      - Reduce the locked sections in the set_plane ioctl to only protect
        plane->fb/plane->crtc and the driver callback (i.e. hw state).
        Everything either doesn't disappear (crtc, plane) or is refcounted
        (fb), and all the data we check is invariant over the respective
        object's lifetimes.
      Reviewed-by: NRob Clark <rob@ti.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      6c2a7532
    • D
      drm: fb refcounting for dirtyfb_ioctl · 4ccf097f
      Daniel Vetter 提交于
      We only need to ensure that the fb stays around for long enough. While
      at it, only grab the modeset locks when we need them (since most
      drivers don't implement the dirty callback, this should help jitter
      and stalls when using the generic modeset driver).
      Reviewed-by: NRob Clark <rob@ti.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      4ccf097f
    • D
      drm: don't take modeset locks in getfb ioctl · 58c0dca1
      Daniel Vetter 提交于
      We only need to push the fb unreference a bit down. While at it,
      properly pass the return value from ->create_handle back to userspace.
      
      Most drivers either return -ENODEV if they don't have a concept of
      buffer objects (ast, cirrus, ...) or just install a handle for the
      underlying gem object (which is ok since we hold a reference on that
      through the framebuffer).
      
      v2: Split out the ->create_handle rework in the individual drivers.
      Reviewed-by: NRob Clark <rob@ti.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      58c0dca1
    • D
      drm: push modeset_lock_all into ->fb_create driver callbacks · 468174f7
      Daniel Vetter 提交于
      And drop it where it's not needed. Most driver just lookup the gem
      object, allocate an fb struct, fill in all the useful fields and then
      register it with drm_framebuffer_init.
      
      All of these operations are already separately locked, and since we
      only put the fb into the fpriv->fbs list _after_ having called
      ->fb_create, we can't also race with rmfb. We can otoh race with other
      ioctls that put the framebuffer to use, but all drivers have been
      reorganized already to call drm_framebuffer_init last in the fb
      creation sequence.
      
      So essentially, we can completely remove any modeset locks from the
      addfb ioctl paths. Yeah!
      
      Also, reference-counting is solid - we get a reference from fb_create
      which we transfer to the fpriv->fbs list. And after unlocking the
      fpriv->fbs_lock we don't touch the framebuffer any longer. Furthermore
      drm_framebuffer_init has added a 2nd reference for the idr lookup, and
      any access through that table will do it's own refcounting.
      Reviewed-by: NRob Clark <rob@ti.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      468174f7
    • D
      drm: nest modeset locks within fpriv->fbs_lock · 7d331595
      Daniel Vetter 提交于
      Atm we still need to unconditionally take the modeset locks in the
      rmfb paths. But eventually we only want to take them if there are
      other users around as a slow-path. This way sane userspace avoids
      blocking on edid reads and other stuff in rmfb if it ensures that the
      fb isn't used anywhere by a crtc/plane.
      
      We can do a quick check for such other users once framebuffers are
      properly refcounting by locking at the refcount - if it's more than 1,
      there are other users left. Again, rmfb racing against other ioctls
      isn't a real problem, userspace is allowed to shoot its foot.
      
      This patch just prepares this by moving the modeset locks to nest
      within fpriv->fbs_lock. Now the distinction between the fbs_lock and
      the device-global fb_lock is clear, since we need to hold the fbs_lock
      outside of any modeset_locks in fb_release.
      Reviewed-by: NRob Clark <rob@ti.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      7d331595
    • D
      drm: reference framebuffers which are on the idr · 2b677e8c
      Daniel Vetter 提交于
      Since otherwise looking and reference-counting around
      drm_framebuffer_lookup will be an unmanageable mess. With this change,
      an object can either be found in the idr and will stay around once we
      incremented the reference counter. Or it will be gone for good and
      can't be looked up using its id any more.
      
      Atomicity is guaranteed by the dev->mode_config.fb_lock. The
      newly-introduce fpriv->fbs_lock looks a bit redundant, but the next
      patch will shuffle the locking order between these two locks and all
      the modeset locks taken in modeset_lock_all, so we'll need it.
      
      Also, since userspace could do really funky stuff and race e.g. a
      getresources with an rmfb, we need to make sure that the kernel
      doesn't fall over trying to look-up an inexistent fb, or causing
      confusion by having two fbs around with the same id. Simply reset the
      framebuffer id to 0, which marks it as reaped. Any lookups of that id
      will fail, so the object is really gone for good from userspace's pov.
      
      Note that we still need to protect the "remove framebuffer from all
      use-cases" and the final unreference with the modeset-lock, since most
      framebuffer use-sites don't implement proper reference counting yet.
      We can only lift this once _all_ users are converted.
      
      With this change, two references are held on alife, but unused
      framebuffers:
      - The reference for the idr lookup, created in this patch.
      - For user-created framebuffers the fpriv->fbs reference, for
        driver-private fbs the driver is supposed to hold it's own last
        reference.
      
      Note that the dev->mode_config.fb_list itself does _not_ hold a
      reference onto the framebuffers (this list is essentially only used
      for debugfs files). Hence if there's anything left there when the
      driver has cleaned up all it's modeset resources, this is a ref-leak.
      WARN about it.
      
      Now we only need to fix up all other places to properly reference
      count framebuffers.
      
      v2: Fix spelling fail in a comment spotted by Rob Clark.
      Reviewed-by: NRob Clark <rob@ti.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      2b677e8c
    • D
      drm: revamp framebuffer cleanup interfaces · 36206361
      Daniel Vetter 提交于
      We have two classes of framebuffer
      - Created by the driver (atm only for fbdev), and the driver holds
        onto the last reference count until destruction.
      - Created by userspace and associated with a given fd. These
        framebuffers will be reaped when their assoiciated fb is closed.
      
      Now these two cases are set up differently, the framebuffers are on
      different lists and hence destruction needs to clean up different
      things. Also, for userspace framebuffers we remove them from any
      current usage, whereas for internal framebuffers it is assumed that
      the driver has done this already.
      
      Long story short, we need two different ways to cleanup such drivers.
      Three functions are involved in total:
      - drm_framebuffer_remove: Convenience function which removes the fb
        from all active usage and then drops the passed-in reference.
      - drm_framebuffer_unregister_private: Will remove driver-private
        framebuffers from relevant lists and drop the corresponding
        references. Should be called for driver-private framebuffers before
        dropping the last reference (or like for a lot of the drivers where
        the fbdev is embedded someplace else, before doing the cleanup
        manually).
      - drm_framebuffer_cleanup: Final cleanup for both classes of fbs,
        should be called by the driver's ->destroy callback once the last
        reference is gone.
      
      This patch just rolls out the new interfaces and updates all drivers
      (by adding calls to drm_framebuffer_unregister_private at all the
      right places)- no functional changes yet. Follow-on patches will move
      drm core code around and update the lifetime management for
      framebuffers, so that we are no longer required to keep framebuffers
      alive by locking mode_config.mutex.
      
      I've also updated the kerneldoc already.
      
      vmwgfx seems to again be a bit special, at least I haven't figured out
      how the fbdev support in that driver works. It smells like it's
      external though.
      
      v2: The i915 driver creates another private framebuffer in the
      load-detect code. Adjust its cleanup code, too.
      Reviewed-by: NRob Clark <rob@ti.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      36206361
    • D
      drm: create drm_framebuffer_lookup · 786b99ed
      Daniel Vetter 提交于
      And replace all fb lookups with it. Also add a WARN to
      drm_mode_object_find since that is now no longer the blessed interface
      to look up an fb. And add kerneldoc to both functions.
      
      This only updates all callsites, but immediately drops the acquired
      refence again. Hence all callers still rely on the fact that a mode fb
      can't disappear while they're holding the struct mutex. Subsequent
      patches will instate proper use of refcounts, and then rework the rmfb
      and unref code to no longer serialize fb destruction with the
      mode_config lock. We don't want that since otherwise a compositor
      might end up stalling for a few frames in rmfb.
      
      v2: Don't use kref_get_unless_zero - Greg KH doesn't like that kind of
      interface.
      Reviewed-by: NRob Clark <rob@ti.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      786b99ed