1. 08 Oct, 2015 1 commit
  2. 07 Oct, 2015 16 commits
  3. 06 Oct, 2015 14 commits
    • Chris Wilson's avatar
      drm/i915: Use a task to cancel the userptr on invalidate_range · 380996aa
      Chris Wilson authored
      Whilst discussing possible ways to trigger an invalidate_range on a
      userptr with an aliased GGTT mmapping (and so cause a struct_mutex
      deadlock), the conclusion is that we can, and we must, prevent any
      possible deadlock by avoiding taking the mutex at all during
      invalidate_range. This has numerous advantages all of which stem from
      avoid the sleeping function from inside the unknown context. In
      particular, it simplifies the invalidate_range because we no longer
      have to juggle the spinlock/mutex and can just hold the spinlock
      for the entire walk. To compensate, we have to make get_pages a bit more
      complicated in order to serialise with a pending cancel_userptr worker.
      As we hold the struct_mutex, we have no choice but to return EAGAIN and
      hope that the worker is then flushed before we retry after reacquiring
      the struct_mutex.
      
      The important caveat is that the invalidate_range itself is no longer
      synchronous. There exists a small but definite period in time in which
      the old PTE's page remain accessible via the GPU. Note however that the
      physical pages themselves are not invalidated by the mmu_notifier, just
      the CPU view of the address space. The impact should be limited to a
      delay in pages being flushed, rather than a possibility of writing to
      the wrong pages. The only race condition that this worsens is remapping
      an userptr active on the GPU where fresh work may still reference the
      old pages due to struct_mutex contention. Given that userspace is racing
      with the GPU, it is fair to say that the results are undefined.
      
      v2: Only queue (and importantly only take one refcnt) the worker once.
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Cc: Michał Winiarski <michal.winiarski@intel.com>
      Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
      Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      380996aa
    • Chris Wilson's avatar
      drm/i915: Fix userptr deadlock with aliased GTT mmappings · e4b946bf
      Chris Wilson authored
      Michał Winiarski found a really evil way to trigger a struct_mutex
      deadlock with userptr. He found that if he allocated a userptr bo and
      then GTT mmaped another bo, or even itself, at the same address as the
      userptr using MAP_FIXED, he could then cause a deadlock any time we then
      had to invalidate the GTT mmappings (so at will). Tvrtko then found by
      repeatedly allocating GTT mmappings he could alias with an old userptr
      mmap and also trigger the deadlock.
      
      To counter act the deadlock, we make the observation that we only need
      to take the struct_mutex if the object has any pages to revoke, and that
      before userspace can alias with the userptr address space, it must have
      invalidated the userptr->pages. Thus if we can check for those pages
      outside of the struct_mutex, we can avoid the deadlock. To do so we
      introduce a separate flag for userptr objects that we can inspect from
      the mmu-notifier underneath its spinlock.
      
      The patch makes one eye-catching change. That is the removal serial=0
      after detecting a to-be-freed object inside the invalidate walker. I
      felt setting serial=0 was a questionable pessimisation: it denies us the
      chance to reuse the current iterator for the next loop (before it is
      freed) and being explicit makes the reader question the validity of the
      locking (since the object-free race could occur elsewhere). The
      serialisation of the iterator is through the spinlock, if the object is
      freed before the next loop then the notifier.serial will be incremented
      and we start the walk from the beginning as we detect the invalid cache.
      
      To try and tame the error paths and interactions with the userptr->active
      flag, we have to do a fair amount of rearranging of get_pages_userptr().
      
      v2: Grammar fixes
      v3: Reorder set-active so that it is only set when obj->pages is set
      (and so needs cancellation). Only the order of setting obj->pages and
      the active-flag is crucial. Calling gup after invalidate-range begin
      means the userptr sees the new set of backing storage (and so will not
      need to invalidate its new pages), but we have to be careful not to set
      the active-flag prior to successfully establishing obj->pages.
      v4: Take the active->flag early so we know in the mmu-notifier when we
      have to cancel a pending gup-worker.
      v5: Rearrange the error path so that is not so convoluted
      v6: Set pinned to 0 when negative before calling release_pages()
      Reported-by: default avatarMichał Winiarski <michal.winiarski@intel.com>
      Testcase: igt/gem_userptr_blits/map-fixed*
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Cc: Michał Winiarski <michal.winiarski@intel.com>
      Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
      Cc: stable@vger.kernel.org
      Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      e4b946bf
    • Chris Wilson's avatar
      drm/i915: Only update the current userptr worker · 68d6c840
      Chris Wilson authored
      The userptr worker allows for a slight race condition where upon there
      may two or more threads calling get_user_pages for the same object. When
      we have the array of pages, then we serialise the update of the object.
      However, the worker should only overwrite the obj->userptr.work pointer
      if and only if it is the active one. Currently we clear it for a
      secondary worker with the effect that we may rarely force a second
      lookup.
      
      v2: Rebase and rename a variable to avoid 80cols
      v3: Mention v2
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      68d6c840
    • Michel Thierry's avatar
      drm/i915: prevent out of range pt in the PDE macros (take 3) · 24dfd073
      Michel Thierry authored
      We tried to fix this in commit fdc454c1 ("drm/i915: Prevent out of
      range pt in gen6_for_each_pde").
      
      But the static analyzer still complains that, just before we break due
      to "iter < I915_PDES", we do "pt = (pd)->page_table[iter]" with an
      iter value that is bigger than I915_PDES. Of course, this isn't really
      a problem since no one uses pt outside the macro. Still, every single
      new usage of the macro will create a new issue for us to mark as a
      false positive.
      
      Also, Paulo re-started the discussion a while ago [1], but didn't end up
      implemented.
      
      In order to "solve" this "problem", this patch takes the ideas from
      Chris and Dave, but that check would change the desired behavior of the
      code, because the object (for example pdp->page_directory[iter]) can be
      null during init/alloc, and C would take this as false, breaking the for
      loop immediately.
      
      This has been already verified with "static analysis tools".
      
      [1]http://lists.freedesktop.org/archives/intel-gfx/2015-June/068548.html
      
      v2: Make it a single statement, while preventing the common subexpression
      elimination (Chris)
      
      Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Cc: Dave Gordon <david.s.gordon@intel.com>
      Signed-off-by: default avatarMichel Thierry <michel.thierry@intel.com>
      Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      24dfd073
    • Tvrtko Ursulin's avatar
      drm/i915: Clean up associated VMAs on context destruction · e9f24d5f
      Tvrtko Ursulin authored
      Prevent leaking VMAs and PPGTT VMs when objects are imported
      via flink.
      
      Scenario is that any VMAs created by the importer will be left
      dangling after the importer exits, or destroys the PPGTT context
      with which they are associated.
      
      This is caused by object destruction not running when the
      importer closes the buffer object handle due the reference held
      by the exporter. This also leaks the VM since the VMA has a
      reference on it.
      
      In practice these leaks can be observed by stopping and starting
      the X server on a kernel with fbcon compiled in. Every time
      X server exits another VMA will be leaked against the fbcon's
      frame buffer object.
      
      Also on systems where flink buffer sharing is used extensively,
      like Android, this leak has even more serious consequences.
      
      This version is takes a general approach from the  earlier work
      by Rafael Barbalho (drm/i915: Clean-up PPGTT on context
      destruction) and tries to incorporate the subsequent discussion
      between Chris Wilson and Daniel Vetter.
      
      v2:
      
      Removed immediate cleanup on object retire - it was causing a
      recursive VMA unbind via i915_gem_object_wait_rendering. And
      it is in fact not even needed since by definition context
      cleanup worker runs only after the last context reference has
      been dropped, hence all VMAs against the VM belonging to the
      context are already on the inactive list.
      
      v3:
      
      Previous version could deadlock since VMA unbind waits on any
      rendering on an object to complete. Objects can be busy in a
      different VM which would mean that the cleanup loop would do
      the wait with the struct mutex held.
      
      This is an even simpler approach where we just unbind VMAs
      without waiting since we know all VMAs belonging to this VM
      are idle, and there is nothing in flight, at the point
      context destructor runs.
      
      v4:
      
      Double underscore prefix for __915_vma_unbind_no_wait and a
      commit message typo fix. (Michel Thierry)
      
      Note that this is just a partial/interim fix since we have a bit a
      fundamental issue with cleaning up, e.g.
      
      https://bugs.freedesktop.org/show_bug.cgi?id=87729Signed-off-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
      Testcase: igt/gem_ppgtt.c/flink-and-exit-vma-leak
      Reviewed-by: default avatarMichel Thierry <michel.thierry@intel.com>
      Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Cc: Rafael Barbalho <rafael.barbalho@intel.com>
      Cc: Michel Thierry <michel.thierry@intel.com>
      [danvet: Add a note that this isn't everything.]
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      e9f24d5f
    • kbuild test robot's avatar
    • Ander Conselvan de Oliveira's avatar
      drm/i915: Rename DP link training functions · 2493f21f
      Ander Conselvan de Oliveira authored
      The link training functions had confusing names. The start function
      actually does the clock recovery phase of the link training, and the
      complete function does the channel equalization. So call them that
      instead. Also, every call to intel_dp_start_link_train() was followed
      by a call to intel_dp_complete_link_train(), so add a new start
      function that calls clock_recory and channel_equalization.
      Signed-off-by: default avatarAnder Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      2493f21f
    • Sonika Jindal's avatar
      drm/i915: Add hot_plug hook for hdmi encoder · 0b5e88dc
      Sonika Jindal authored
      This patch adds a separate probe function for HDMI
      EDID read over DDC channel. This function has been
      registered as a .hot_plug handler for HDMI encoder.
      
      The current implementation of hdmi_detect()
      function re-sets the cached HDMI edid (in connector->detect_edid) in
      every detect call.This function gets called many times, sometimes
      directly from userspace probes, forcing drivers to read EDID every
      detect function call.This causes several problems like:
      
      1. Race conditions in multiple hot_plug / unplug cases, between
         interrupts bottom halves and userspace detections.
      2. Many Un-necessary EDID reads for single hotplug/unplug
      3. HDMI complaince failures which expects only one EDID read per hotplug
      
      This function will be serving the purpose of really reading the EDID
      by really probing the DDC channel, and updating the cached EDID.
      
      The plan is to:
      1. i915 IRQ handler bottom half function already calls
         intel_encoder->hotplug() function. Adding This probe function which
         will read the EDID only in case of a hotplug / unplug.
      2. During init_connector this probe will be called to read the edid
      3. Reuse the cached EDID in hdmi_detect() function.
      
      The "< gen7" check is there because this was tested only for >=gen7
      platforms. For older platforms the hotplug/reading edid path remains same.
      
      v2: Calling set_edid instead of hdmi_probe during init.
      Also, for platforms having DDI, intel_encoder for DP and HDMI is same
      (taken from intel_dig_port), so for DP also, hot_plug function gets called
      which is not intended here. So, check for HDMI in intel_hdmi_probe
      Rely on HPD for updating edid only for platforms gen > 8 and also for VLV.
      
      v3: Dropping the gen < 8 || !VLV  check. Now all platforms should rely on
      hotplug or init for updating the edid.(Daniel)
      Also, calling hdmi_probe in init instead of set_edid
      
      v4: Renaming intel_hdmi_probe to intel_hdmi_hot_plug.
      Also calling this hotplug handler from intel_hpd_init to take care of init
      resume scenarios.
      
      v5: Moved the call to encoder hotplug during init to separate patch(Daniel)
      Signed-off-by: default avatarShashank Sharma <shashank.sharma@intel.com>
      Signed-off-by: default avatarSonika Jindal <sonika.jindal@intel.com>
      [danvet: Mark intel_hdmi_hot_plug as static.]
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      0b5e88dc
    • Jordan Justen's avatar
      drm/i915: Add GEN7_GPGPU_DISPATCHDIMX/Y/Z to the register whitelist · 7b9748cb
      Jordan Justen authored
      This is required to support glDispatchComputeIndirect for gen7.
      Signed-off-by: default avatarJordan Justen <jordan.l.justen@intel.com>
      Reviewed-by: default avatarKristian Høgsberg <krh@bitplanet.net>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      7b9748cb
    • Sagar Arun Kamble's avatar
      drm/i915: Update Promotion timer for RC6 TO Mode · 3e7732a0
      Sagar Arun Kamble authored
      When using RC6 timeout mode, the timeout value
      should be written to GEN6_RC6_THRESHOLD.
      
      v2: Updated commit message. (Tom)
      
      v3: Rebase over whitespace differences. (Daniel)
      
      Cc: Tom O'Rourke <Tom.O'Rourke@intel.com>
      Signed-off-by: default avatarSagar Arun Kamble <sagar.a.kamble@intel.com>
      Reviewed-by: default avatarTom O'Rourke <Tom.O'Rourke@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      3e7732a0
    • Alex Dai's avatar
      drm/i915/guc: Add host2guc notification for suspend and resume · a1c41994
      Alex Dai authored
      Add host2guc interface to notify GuC power state changes when
      enter or resume from power saving state.
      
      v3: Move intel_guc_suspend to i915_drm_suspend for consistency.
      
      v2: Add GuC suspend/resume to runtime suspend/resume too
      
      v1: Change to a more flexible way when fill host to GuC scratch
      data in order to remove hard coding.
      Signed-off-by: default avatarAlex Dai <yu.dai@intel.com>
      Reviewed-by: default avatarSagar Arun Kamble <sagar.a.kamble@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      a1c41994
    • Ville Syrjälä's avatar
      drm/i915: Skip CHV PHY asserts until PHY has been fully reset · 3be60de9
      Ville Syrjälä authored
      The BIOS can leave the CHV display PHY in some odd state where
      some of the LDOs/lanes won't power down fully when unused. This
      will trigger a host of asserts that were added in:
      30142273 drm/i915: Add CHV PHY LDO power sanity checks
      6669e39f drm/i915: Add some CHV DPIO lane power state asserts
      
      To avoid that, skip the asserts until the PHY power well has been
      disabled at least once. That will fully reset the PHY, and once
      brought back up, the dynamic power down features will work correctly.
      Signed-off-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
      Reviewed-by: Deepak S<deepak.s@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      3be60de9
    • Ville Syrjälä's avatar
      drm/i915: Don't bypass LRC on CHV · 9571b190
      Ville Syrjälä authored
      The docs are unclear as usual, so it's not clear whether LRC should be
      bypassed, performed normally or GRC code should be used as the LRC code.
      Some old docs stated that LRC bypass ought to be used, more recent ones
      no longer say that. Some docs indicated that we could use GRC as the LRC
      code on CHV, but the BIOS doesn't do that, so let's not do it either.
      
      Besides to enable LRC bypass properly, I believe we should set the bit
      already before deasserting cmnreset.
      Signed-off-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
      Reviewed-by: Deepak S<deepak.s@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      9571b190
    • Sonika Jindal's avatar
      drm/i915: Call encoder hotplug for init and resume cases · 5d250b05
      Sonika Jindal authored
      For all the encoders, call the hot_plug if it is registered.
      This is required for connected boot and resume cases to generate
      fake hpd resulting in reading of edid.
      Removing the initial sdvo hot_plug call too so that it will be called
      just once from this loop.
      Signed-off-by: default avatarSonika Jindal <sonika.jindal@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      5d250b05
  4. 05 Oct, 2015 1 commit
  5. 02 Oct, 2015 8 commits