1. 16 6月, 2016 1 次提交
  2. 02 6月, 2016 1 次提交
    • D
      drm/doc: Appease sphinx · 2e7a5701
      Daniel Vetter 提交于
      Mostly this is unexpected indents. But really it's just a
      demonstration for my patch, all these issues have been found&fixed
      using the correct source file and line number support I just added.
      All line numbers have been perfectly accurate.
      
      One issue looked a bit fishy in intel_lrc.c, where I don't quite grok
      what sphinx is unhappy about. But since that file looks like it has
      never seen a proper kernel-doc parser I figured better to fix in a
      separate path.
      
      v2: Use fancy new &drm_device->struct_mutex linking (Jani).
      
      Cc: Jani Nikula <jani.nikula@intel.com>
      Cc: linux-doc@vger.kernel.org
      Cc: Jonathan Corbet <corbet@lwn.net>
      Acked-by: NJani Nikula <jani.nikula@intel.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@intel.com>
      2e7a5701
  3. 01 6月, 2016 1 次提交
    • D
      drm/i915: Revert async unpin and nonblocking atomic commit · e42aeef1
      Daniel Vetter 提交于
      This reverts the following patches:
      
      d55dbd06 drm/i915: Allow nonblocking update of pageflips.
      15c86bdb drm/i915: Check for unpin correctness.
      95c2ccdc Reapply "drm/i915: Avoid stalling on pending flips for legacy cursor updates"
      a6747b73 drm/i915: Make unpin async.
      03f476e1 drm/i915: Prepare connectors for nonblocking checks.
      2099deff drm/i915: Pass atomic states to fbc update functions.
      ee7171af drm/i915: Remove reset_counter from intel_crtc.
      2ee004f7 drm/i915: Remove queue_flip pointer.
      b8d2afae drm/i915: Remove use_mmio_flip kernel parameter.
      8dd634d9 drm/i915: Remove cs based page flip support.
      143f73b3 drm/i915: Rework intel_crtc_page_flip to be almost atomic, v3.
      84fc494b drm/i915: Add the exclusive fence to plane_state.
      6885843a drm/i915: Convert flip_work to a list.
      aa420ddd drm/i915: Allow mmio updates on all platforms, v2.
      afee4d87 Revert "drm/i915: Avoid stalling on pending flips for legacy cursor updates"
      
      "drm/i915: Allow nonblocking update of pageflips" should have been
      split up, misses a proper commit message and seems to cause issues in
      the legacy page_flip path as demonstrated by kms_flip.
      
      "drm/i915: Make unpin async" doesn't handle the unthrottled cursor
      updates correctly, leading to an apparent pin count leak. This is
      caught by the WARN_ON in i915_gem_object_do_pin which screams if we
      have more than DRM_I915_GEM_OBJECT_MAX_PIN_COUNT pins.
      
      Unfortuantely we can't just revert these two because this patch series
      came with a built-in bisect breakage in the form of temporarily
      removing the unthrottled cursor update hack for legacy cursor ioctl.
      Therefore there's no other option than to revert the entire pile :(
      
      There's one tiny conflict in intel_drv.h due to other patches, nothing
      serious.
      
      Normally I'd wait a bit longer with doing a maintainer revert, but
      since the minimal set of patches we need to revert (due to the bisect
      breakage) is so big, time is running out fast. And very soon
      (especially after a few attempts at fixing issues) it'll be really
      hard to revert things cleanly.
      
      Lessons learned:
      - Not a good idea to rush the review (done by someone fairly new to
        the area) and not make sure domain experts had a chance to read it.
      
      - Patches should be properly split up. I only looked at the two
        patches that should be reverted in detail, but both look like the
        mix up different things in one patch.
      
      - Patches really should have proper commit messages. Especially when
        doing more than one thing, and especially when touching critical and
        tricky core code.
      
      - Building a patch series and r-b stamping it when it has a built-in
        bisect breakage is not a good idea.
      
      - I also think we need to stop building up technical debt by
        postponing atomic igt testcases even longer. I think it's clear that
        there's enough corner cases in this beast that we really need to
        have the testcases _before_ the next step lands.
      
      (cherry picked from commit 5a21b665
      from drm-intel-next-queeud)
      
      Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
      Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
      Cc: John Harrison <John.C.Harrison@Intel.com>
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Acked-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Acked-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
      Acked-by: NDave Airlie <airlied@redhat.com>
      Acked-by: NJani Nikula <jani.nikula@linux.intel.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@intel.com>
      e42aeef1
  4. 25 5月, 2016 1 次提交
    • D
      drm/i915: Revert async unpin and nonblocking atomic commit · 5a21b665
      Daniel Vetter 提交于
      This reverts the following patches:
      
      d55dbd06 drm/i915: Allow nonblocking update of pageflips.
      15c86bdb drm/i915: Check for unpin correctness.
      95c2ccdc Reapply "drm/i915: Avoid stalling on pending flips for legacy cursor updates"
      a6747b73 drm/i915: Make unpin async.
      03f476e1 drm/i915: Prepare connectors for nonblocking checks.
      2099deff drm/i915: Pass atomic states to fbc update functions.
      ee7171af drm/i915: Remove reset_counter from intel_crtc.
      2ee004f7 drm/i915: Remove queue_flip pointer.
      b8d2afae drm/i915: Remove use_mmio_flip kernel parameter.
      8dd634d9 drm/i915: Remove cs based page flip support.
      143f73b3 drm/i915: Rework intel_crtc_page_flip to be almost atomic, v3.
      84fc494b drm/i915: Add the exclusive fence to plane_state.
      6885843a drm/i915: Convert flip_work to a list.
      aa420ddd drm/i915: Allow mmio updates on all platforms, v2.
      afee4d87 Revert "drm/i915: Avoid stalling on pending flips for legacy cursor updates"
      
      "drm/i915: Allow nonblocking update of pageflips" should have been
      split up, misses a proper commit message and seems to cause issues in
      the legacy page_flip path as demonstrated by kms_flip.
      
      "drm/i915: Make unpin async" doesn't handle the unthrottled cursor
      updates correctly, leading to an apparent pin count leak. This is
      caught by the WARN_ON in i915_gem_object_do_pin which screams if we
      have more than DRM_I915_GEM_OBJECT_MAX_PIN_COUNT pins.
      
      Unfortuantely we can't just revert these two because this patch series
      came with a built-in bisect breakage in the form of temporarily
      removing the unthrottled cursor update hack for legacy cursor ioctl.
      Therefore there's no other option than to revert the entire pile :(
      
      There's one tiny conflict in intel_drv.h due to other patches, nothing
      serious.
      
      Normally I'd wait a bit longer with doing a maintainer revert, but
      since the minimal set of patches we need to revert (due to the bisect
      breakage) is so big, time is running out fast. And very soon
      (especially after a few attempts at fixing issues) it'll be really
      hard to revert things cleanly.
      
      Lessons learned:
      - Not a good idea to rush the review (done by someone fairly new to
        the area) and not make sure domain experts had a chance to read it.
      
      - Patches should be properly split up. I only looked at the two
        patches that should be reverted in detail, but both look like the
        mix up different things in one patch.
      
      - Patches really should have proper commit messages. Especially when
        doing more than one thing, and especially when touching critical and
        tricky core code.
      
      - Building a patch series and r-b stamping it when it has a built-in
        bisect breakage is not a good idea.
      
      - I also think we need to stop building up technical debt by
        postponing atomic igt testcases even longer. I think it's clear that
        there's enough corner cases in this beast that we really need to
        have the testcases _before_ the next step lands.
      
      Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
      Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
      Cc: John Harrison <John.C.Harrison@Intel.com>
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Acked-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Acked-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
      Acked-by: NDave Airlie <airlied@redhat.com>
      Acked-by: NJani Nikula <jani.nikula@linux.intel.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@intel.com>
      5a21b665
  5. 19 5月, 2016 1 次提交
  6. 11 5月, 2016 1 次提交
  7. 09 5月, 2016 1 次提交
  8. 31 3月, 2016 1 次提交
    • J
      drm/i915: Refer to GGTT {,VM} consistently · 72e96d64
      Joonas Lahtinen 提交于
      Refer to the GGTT VM consistently as "ggtt->base" instead of just "ggtt",
      "vm" or indirectly through other variables like "dev_priv->ggtt.base"
      to avoid confusion with the i915_ggtt object itself and PPGTT VMs.
      
      Refer to the GGTT as "ggtt" instead of indirectly through chaining.
      
      As a bonus gets rid of the long-standing i915_obj_to_ggtt vs.
      i915_gem_obj_to_ggtt conflict, due to removal of i915_obj_to_ggtt!
      
      v2:
      - Added some more after grepping sources with Chris
      
      v3:
      - Refer to GGTT VM through ggtt->base consistently instead of ggtt_vm
        (Chris)
      
      v4:
      - Convert all dev_priv->ggtt->foo accesses to ggtt->foo.
      
      v5:
      - Make patch checker happy
      
      Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
      Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: NJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
      Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk>
      72e96d64
  9. 18 3月, 2016 1 次提交
  10. 20 2月, 2016 1 次提交
    • P
      drm/i915/fbc: enable FBC by default on HSW and BDW · a98ee793
      Paulo Zanoni 提交于
      These platforms should be fine now.
      
      FBC can allow very significant power savings for screen-on idle
      systems, but it is worth mentioning that a lot of people won't get
      significant power savings by enabling this feature because they may
      have something else preventing the system from getting into the
      deepest sleep states. Examples may include a hungry wifi device or a
      max_performance SATA link power management policy. You can check your
      PC state residencies on the powertop "Idle stats" tab. I recommend
      trying to run "sudo powertop --auto-tune" and then seeing if the
      residencies improve.
      
      Oh, and in case you - the person reading this commit message - found
      this commit through git bisect, please do the following:
       - Check your dmesg and see if there are error messages mentioning
         underruns around the time your problem started happening.
       - Download intel-gpu-tools, compile it, and run:
         $ sudo ./tests/kms_frontbuffer_tracking --run-subtest '*fbc-*' 2>&1 | tee fbc.txt
         Then send us the fbc.txt file, especially if you get a failure.
         This will really maximize your chances of getting the bug fixed
         quickly.
       - Try to find a reliable way to reproduce the problem, and tell us.
       - Boot with drm.debug=0xe, reproduce the problem, then send us the
         dmesg file.
      
      v2: Don't enable by default on SKL.
      Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com>
      Reviewed-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: http://patchwork.freedesktop.org/patch/msgid/1455655643-2535-1-git-send-email-paulo.r.zanoni@intel.com
      a98ee793
  11. 05 2月, 2016 2 次提交
  12. 30 1月, 2016 22 次提交
  13. 03 12月, 2015 6 次提交
    • P
      drm/i915: only recompress FBC after flushing a drawing operation · ee7d6cfa
      Paulo Zanoni 提交于
      There's no need to stop and restart FBC, which is quite expensive as
      we have to revalidate the CRTC state. After flushing a drawing
      operation we know the CRTC state hasn't changed, so a nuke
      (recompress) should be fine.
      
      v2: Make it simpler (Chris).
      v3: Rewrite the patch again due to patch order changes.
      v4: Rewrite commit message (Chris).
      Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/
      ee7d6cfa
    • P
      drm/i915: get rid of FBC {,de}activation messages · 820bcabb
      Paulo Zanoni 提交于
      When running Cinnamon I see way too many pairs of these messages: many
      per second. Get rid of them as they're just telling us FBC is working
      as expected. We already have the messages for enable/disable, so we
      don't really need messages for activation/deactivation.
      
      v2: Rebase after changing the patch order.
      Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/
      820bcabb
    • P
      drm/i915: kill fbc.uncompressed_size · 559d9135
      Paulo Zanoni 提交于
      Directly call intel_fbc_calculate_cfb_size() in the only place that
      actually needs it, and use the proper check before removing the stolen
      node. IMHO, this change makes our code easier to understand.
      
      v2: Use drm_mm_node_allocated() (Chris).
      Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/
      559d9135
    • P
      drm/i915: use a single intel_fbc_work struct · 128d7356
      Paulo Zanoni 提交于
      This was already on my TODO list, and was requested both by Chris and
      Ville, for different reasons. The advantages are avoiding a frequent
      malloc/free pair, and the locality of having the work structure
      embedded in dev_priv. The maximum used memory is also smaller since
      previously we could have multiple allocated intel_fbc_work structs at
      the same time, and now we'll always have a single one - the one
      embedded on dev_priv. Of course, we're now using a little more memory
      on the cases where there's nothing scheduled.
      
      The biggest challenge here is to keep everything synchronized the way
      it was before.
      
      Currently, when we try to activate FBC, we allocate a new
      intel_fbc_work structure. Then later when we conclude we must delay
      the FBC activation a little more, we allocate a new intel_fbc_work
      struct, and then adjust dev_priv->fbc.fbc_work to point to the new
      struct. So when the old work runs - at intel_fbc_work_fn() - it will
      check that dev_priv->fbc.fbc_work points to something else, so it does
      nothing. Everything is also protected by fbc.lock.
      
      Just cancelling the old delayed work doesn't work because we might
      just cancel it after the work function already started to run, but
      while it is still waiting to grab fbc.lock. That's why we use the
      "dev_priv->fbc.fbc_work == work" check described in the paragraph
      above.
      
      So now that we have a single work struct we have to introduce a new
      way to synchronize everything. So we're making the work function a
      normal work instead of a delayed work, and it will be responsible for
      sleeping the appropriate amount of time itself. This way, after it
      wakes up it can grab the lock, ask "were we delayed or cancelled?" and
      then go back to sleep, enable FBC or give up.
      
      v2:
        - Spelling fixes.
        - Rebase after changing the patch order.
        - Fix ms/jiffies confusion.
      
      Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
      Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/
      128d7356
    • P
      drm/i915: check for FBC planes in the same place as the pipes · e6cd6dc1
      Paulo Zanoni 提交于
      This moves the pre-gen4 check from update() to enable(). The HAS_DDI
      in the original code is not needed since only gen 2/3 have the plane
      swapping code.
      
      v2: Rebase.
      v3: Extract fbc_on_plane_a_only() (Chris).
      Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/
      e6cd6dc1
    • P
      drm/i915: alloc/free the FBC CFB during enable/disable · c5ecd469
      Paulo Zanoni 提交于
      One of the problems with the current code is that it frees the CFB and
      releases its drm_mm node as soon as we flip FBC's enable bit. This is
      bad because after we disable FBC the hardware may still use the CFB
      for the rest of the frame, so in theory we should only release the
      drm_mm node one frame after we disable FBC. Otherwise, a stolen memory
      allocation done right after an FBC disable may result in either
      corrupted memory for the new owner of that memory region or corrupted
      screen/underruns in case the new owner changes it while the hardware
      is still reading it. This case is not exactly easy to reproduce since
      we currently don't do a lot of stolen memory allocations, but I see
      patches on the mailing list trying to expose stolen memory to user
      space, so races will be possible.
      
      I thought about three different approaches to solve this, and they all
      have downsides.
      
      The first approach would be to simply use multiple drm_mm nodes and
      freeing the unused ones only after a frame has passed. The problem
      with this approach is that since stolen memory is rather small,
      there's a risk we just won't be able to allocate a new CFB from stolen
      if the previous one was not freed yet. This could happen in case we
      quickly disable FBC from pipe A and decide to enable it on pipe B, or
      just if we change pipe A's fb stride while FBC is enabled.
      
      The second approach would be similar to the first one, but maintaining
      a single drm_mm node and keeping track of when it can be reused. This
      would remove the disadvantage of not having enough space for two
      nodes, but would create the new problem where we may not be able to
      enable FBC at the point intel_fbc_update() is called, so we would have
      to add more code to retry updating FBC after the time has passed. And
      that can quickly get too complex since we can get invalidate, flush,
      disable and other calls in the middle of the wait.
      
      Both solutions above - and also the current code - have the problem
      that we unnecessarily free+realloc FBC during invalidate+flush
      operations even if the CFB size doesn't change.
      
      The third option would be to move the allocation/deallocation to
      enable/disable. This makes sure that the pipe is always disabled when
      we allocate/deallocate the CFB, so there's no risk that the FBC
      hardware may read or write to the memory right after it is freed from
      drm_mm. The downside is that it is possible for user space to change
      the buffer stride without triggering a disable/enable - only
      deactivate/activate -, so we'll have to handle this case somehow - see
      igt's kms_frontbuffer_tracking test, fbc-stridechange subtest. It
      could be possible to implement a way to free+alloc the CFB during said
      stride change, but it would involve a lot of book-keeping - exactly as
      mentioned above - just for on case, so for now I'll keep it simple and
      just deactivate FBC. Besides, we may not even need to disable FBC
      since we do CFB over-allocation.
      
      Note from Chris: "Starting a fullscreen client that covers a single
      monitor in a multi-monitor setup will trigger a change in stride on
      one of the CRTCs (the monitors will be flipped independently).". It
      shouldn't be a huge problem if we lose FBC on multi-monitor setups
      since these setups already have problems reaching deep PC states
      anyway.
      
      v2: Rebase after changing the patch order.
      v3:
        - Remove references to the stride change case being "uncommon" and
          paste Chris' example.
        - Rebase after a change in a previous patch.
      Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/
      c5ecd469