1. 21 8月, 2013 18 次提交
    • D
      drm/prime: proper locking+refcounting for obj->dma_buf link · 319c933c
      Daniel Vetter 提交于
      The export dma-buf cache is semantically similar to an flink name. So
      semantically it makes sense to treat it the same and remove the name
      (i.e. the dma_buf pointer) and its references when the last gem handle
      disappears.
      
      Again we need to be careful, but double so: Not just could someone
      race and export with a gem close ioctl (so we need to recheck
      obj->handle_count again when assigning the new name), but multiple
      exports can also race against each another. This is prevented by
      holding the dev->object_name_lock across the entire section which
      touches obj->dma_buf.
      
      With the new scheme we also need to reinstate the obj->dma_buf link at
      import time (in case the only reference userspace has held in-between
      was through the dma-buf fd and not through any native gem handle). For
      simplicity we don't check whether it's a native object but
      unconditionally set up that link - with the new scheme of removing the
      obj->dma_buf reference when the last handle disappears we can do that.
      
      To make it clear that this is not just for exported buffers anymore
      als rename it from export_dma_buf to dma_buf.
      
      To make sure that now one can race a fd_to_handle or handle_to_fd with
      gem_close we use the same tricks as in flink of extending the
      dev->object_name_locking critical section. With this change we finally
      have a guaranteed 1:1 relationship (at least for native objects)
      between gem objects and dma-bufs, even accounting for races (which can
      happen since the dma-buf itself holds a reference while in-flight).
      
      This prevent igt/prime_self_import/export-vs-gem_close-race from
      Oopsing the kernel. There is still a leak though since the per-file
      priv dma-buf/handle cache handling is racy. That will be fixed in a
      later patch.
      
      v2: Remove the bogus dma_buf_put from the export_and_register_object
      failure path if we've raced with the handle count dropping to 0.
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      319c933c
    • D
      drm/gem: completely close gem_open vs. gem_close races · 20228c44
      Daniel Vetter 提交于
      The gem flink name holds a reference onto the object itself, and this
      self-reference would prevent an flink'ed object from every being
      freed. To break that loop we remove the flink name when the last
      userspace handle disappears, i.e. when obj->handle_count reaches 0.
      
      Now in gem_open we drop the dev->object_name_lock between the flink
      name lookup and actually adding the handle. This means a concurrent
      gem_close of the last handle could result in the flink name getting
      reaped right inbetween, i.e.
      
      Thread 1		Thread 2
      gem_open		gem_close
      
      flink -> obj lookup
      			handle_count drops to 0
      			remove flink name
      create_handle
      handle_count++
      
      If someone now flinks this object again, we'll get a new flink name.
      
      We can close this race by removing the lock dropping and making the
      entire lookup+handle_create sequence atomic. Unfortunately to still be
      able to share the handle_create logic this requires a
      handle_create_tail function which drops the lock - we can't hold the
      object_name_lock while calling into a driver's ->gem_open callback.
      
      Note that for flink fixing this race isn't really important, since
      racing gem_open against gem_close is clearly a userspace bug. And no
      matter how the race ends, we won't leak any references.
      
      But with dma-buf where the userspace dma-buf fd itself is refcounted
      this is a valid sequence and hence we should fix it. Therefore this
      patch here is just a warm-up exercise (and for consistency between
      flink buffer sharing and dma-buf buffer sharing with self-imports).
      
      Also note that this extension of the critical section in gem_open
      protected by dev->object_name_lock only works because it's now a
      mutex: A spinlock would conflict with the potential memory allocation
      in idr_preload().
      
      This is exercises by igt/gem_flink_race/flink_name.
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      20228c44
    • D
      drm/gem: switch dev->object_name_lock to a mutex · cd4f013f
      Daniel Vetter 提交于
      I want to wrap the creation of a dma-buf from a gem object in it,
      so that the obj->export_dma_buf cache can be atomically filled in.
      
      Instead of creating a new mutex just for that variable I've figured
      I can reuse the existing dev->object_name_lock, especially since
      the new semantics will exactly mirror the flink obj->name already
      protected by that lock.
      
      v2: idr_preload/idr_preload_end is now an atomic section, so need to
      move the mutex locking outside.
      
      [airlied: fix up conflict with patch to make debugfs use lock]
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      cd4f013f
    • D
      drm/prime: clarify logic a bit in drm_gem_prime_fd_to_handle · 84341c28
      Daniel Vetter 提交于
      if (!ret) implies that ret == 0, so no need to clear it again. And
      explicitly check for ret == 0 to indicate that we're checking an errno
      integer.
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      84341c28
    • D
      drm/prime: shrink critical section protected by prime lock · bdf655de
      Daniel Vetter 提交于
      When exporting a gem object as a dma-buf the critical section for the
      per-fd prime lock is just the adding (and in case of errors, removing)
      of the handle to the per-fd lookup cache.
      
      So restrict the critical section to just that part of the function.
      
      This simplifies later reordering.
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      bdf655de
    • D
      drm/prime: use proper pointer in drm_gem_prime_handle_to_fd · 4332bf43
      Daniel Vetter 提交于
      Part of the function uses the properly-typed dmabuf variable, the
      other an untyped void *buf. Kill the later.
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      4332bf43
    • D
      drm/gem: make drm_gem_object_handle_unreference_unlocked static · becee2a5
      Daniel Vetter 提交于
      No one outside of drm should use this, the official interfaces are
      drm_gem_handle_create and drm_gem_handle_delete. The handle refcounting
      is purely an implementation detail of gem.
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      becee2a5
    • D
      drm/prime: fix error path in drm_gem_prime_fd_to_handle · 730c4ff9
      Daniel Vetter 提交于
      handle_unreference only clears up the obj->name and the reference,
      but would leave a dangling handle in the idr. The right thing
      to do is to call handle_delete.
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      730c4ff9
    • D
      drm/gem: fix up flink name create race · a8e11d1c
      Daniel Vetter 提交于
      This is the 2nd attempt, I've always been a bit dissatisified with the
      tricky nature of the first one:
      
      http://lists.freedesktop.org/archives/dri-devel/2012-July/025451.html
      
      The issue is that the flink ioctl can race with calling gem_close on
      the last gem handle. In that case we'll end up with a zero handle
      count, but an flink name (and it's corresponding reference). Which
      results in a neat space leak.
      
      In my first attempt I've solved this by rechecking the handle count.
      But fundamentally the issue is that ->handle_count isn't your usual
      refcount - it can be resurrected from 0 among other things.
      
      For those special beasts atomic_t often suggest way more ordering that
      it actually guarantees. To prevent being tricked by those hairy
      semantics take the easy way out and simply protect the handle with the
      existing dev->object_name_lock.
      
      With that change implemented it's dead easy to fix the flink vs. gem
      close reace: When we try to create the name we simply have to check
      whether there's still officially a gem handle around and if not refuse
      to create the flink name. Since the handle count decrement and flink
      name destruction is now also protected by that lock the reace is gone
      and we can't ever leak the flink reference again.
      
      Outside of the drm core only the exynos driver looks at the handle
      count, and tbh I have no idea why (it's just for debug dmesg output
      luckily).
      
      I've considered inlining the drm_gem_object_handle_free, but I plan to
      add more name-like things (like the exported dma_buf) to this scheme,
      so it's clearer to leave the handle freeing in its own function.
      
      This is exercised by the new gem_flink_race i-g-t testcase, which on
      my snb leaks gem objects at a rate of roughly 1k objects/s.
      
      v2: Fix up the error path handling in handle_create and make it more
      robust by simply calling object_handle_unreference.
      
      v3: Fix up the handle_unreference logic bug - atomic_dec_and_test
      retursn 1 for 0. Oops.
      
      v4: Squash in inlining of drm_gem_object_handle_reference as suggested
      by Dave Airlie and add a note that we now have a testcase.
      
      Cc: Dave Airlie <airlied@gmail.com>
      Cc: Inki Dae <inki.dae@samsung.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      a8e11d1c
    • D
      Merge tag 'drm-intel-next-2013-08-09' of... · 9712def2
      Dave Airlie 提交于
      Merge tag 'drm-intel-next-2013-08-09' of git://people.freedesktop.org/~danvet/drm-intel into drm-next
      
      Daniel writes:
      New pile of stuff for -next:
      - Cleanup of the old crtc helper callbacks, all encoders are now converted
        to the i915 modeset infrastructure.
      - Massive amount of wm patches from Ville for ilk, snb, ivb, hsw, this is
        prep work to eventually get things going for nuclear pageflips where we
        need to adjust watermarks on the fly.
      - More vm/vma patches from Ben. This refactoring isn't yet fully rolled
        out, we miss the execbuf conversion and some of the low-level
        bind/unbind support code.
      - Convert our hdmi infoframe code to use the new common helper functions
        (Damien). This contains some bugfixes for the common infoframe helpers.
      - Some cruft removal from Damien.
      - Various smaller bits&pieces all over, as usual.
      
      * tag 'drm-intel-next-2013-08-09' of git://people.freedesktop.org/~danvet/drm-intel: (105 commits)
        drm/i915: Fix FB WM for HSW
        drm/i915: expose HDMI connectors on port C on BYT
        drm/i915: fix a limit check in hsw_compute_wm_results()
        drm/i915: unbreak i915_gem_object_ggtt_unbind()
        drm/i915: Make intel_set_mode() static
        drm/i915: Remove intel_modeset_disable()
        drm/i915: Make intel_encoder_dpms() static
        drm/i915: Make i915_hangcheck_elapsed() static
        drm/i915: Fix #endif comment
        drm/i915: Remove i915_gem_object_check_coherency()
        drm/i915: Remove stale prototypes
        drm/i915: List objects allocated from stolen memory in debugfs
        drm/i915: Always call intel_update_sprite_watermarks() when disabling a plane
        drm/i915: Pass plane and crtc to intel_update_sprite_watermarks
        drm/i915: Don't try to disable plane if it's already disabled
        drm/i915: Pass crtc to our update/disable_plane hooks
        drm/i915: Split plane watermark parameters into a separate struct
        drm/i915: Pull some watermarks state into a separate structure
        drm/i915: Calculate max watermark levels for ILK+
        drm/i915: Rename hsw_lp_wm_result to intel_wm_level
        ...
      9712def2
    • L
      drm: Make drm_get_platform_dev() static · 66cc8b6b
      Lespiau, Damien 提交于
      It's only used in drm_platform.c.
      Signed-off-by: NDamien Lespiau <damien.lespiau@intel.com>
      Reviewed-by: NAlex Deucher <alexander.deucher@amd.com>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      66cc8b6b
    • L
      15f3b9d9
    • L
      drm: Make drm_fb_cma_describe() static · 2c9c52e8
      Lespiau, Damien 提交于
      This function is only used in drm_fb_cma_helper.c.
      Signed-off-by: NDamien Lespiau <damien.lespiau@intel.com>
      Reviewed-by: NAlex Deucher <alexander.deucher@amd.com>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      2c9c52e8
    • L
      drm: Remove 2 unused defines · a03eb838
      Lespiau, Damien 提交于
      These were introduced in the very first DRM commit:
      
        commit f453ba04
        Author: Dave Airlie <airlied@redhat.com>
        Date:   Fri Nov 7 14:05:41 2008 -0800
      
            DRM: add mode setting support
      
            Add mode setting support to the DRM layer.
      
      But are unused.
      Signed-off-by: NDamien Lespiau <damien.lespiau@intel.com>
      Reviewed-by: NAlex Deucher <alexander.deucher@amd.com>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      a03eb838
    • L
      drm: Make drm_mode_remove() static · 86f422d5
      Lespiau, Damien 提交于
      It's only used in drm_crtc.c.
      Signed-off-by: NDamien Lespiau <damien.lespiau@intel.com>
      Reviewed-by: NAlex Deucher <alexander.deucher@amd.com>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      86f422d5
    • L
      drm: Remove drm_mode_list_concat() · 67587e86
      Lespiau, Damien 提交于
      The last user was removed in
      
        commit 575dc34e
        Author: Dave Airlie <airlied@redhat.com>
        Date:   Mon Sep 7 18:43:26 2009 +1000
      
            drm/kms: remove old std mode fallback code.
      
            The new code adds modes in the helper, which makes more sense
            I disliked the non-driver code adding modes.
      Signed-off-by: NDamien Lespiau <damien.lespiau@intel.com>
      Reviewed-by: NAlex Deucher <alexander.deucher@amd.com>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      67587e86
    • L
      drm: Remove drm_mode_create_dithering_property() · ddecb10c
      Lespiau, Damien 提交于
      This was last used by nouveau, replaced by a driver-specific property
      in:
      
        commit de691855
        Author: Ben Skeggs <bskeggs@redhat.com>
        Date:   Mon Oct 17 12:23:41 2011 +1000
      
            drm/nouveau: improve dithering properties, and implement proper auto mode
      Signed-off-by: NDamien Lespiau <damien.lespiau@intel.com>
      Reviewed-by: NAlex Deucher <alexander.deucher@amd.com>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      ddecb10c
    • L
      drm: Remove stale prototypes · f51607ac
      Lespiau, Damien 提交于
      A few prototypes have been left in the headers, their function friends
      long gone.
      Signed-off-by: NDamien Lespiau <damien.lespiau@intel.com>
      Reviewed-by: NAlex Deucher <alexander.deucher@amd.com>
      Signed-off-by: NDave Airlie <airlied@redhat.com>
      f51607ac
  2. 19 8月, 2013 22 次提交