1. 03 12月, 2015 10 次提交
    • 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
    • P
      drm/i915: introduce intel_fbc_{enable,disable} · d029bcad
      Paulo Zanoni 提交于
      The goal is to call FBC enable/disable only once per modeset, while
      activate/deactivate/update will be called multiple times.
      
      The enable() function will be responsible for deciding if a CRTC will
      have FBC on it and then it will "lock" FBC on this CRTC: it won't be
      possible to change FBC's CRTC until disable(). With this, all checks
      and resource acquisition that only need to be done once per modeset
      can be moved from update() to enable(). And then the update(),
      activate() and deactivate() code will also get simpler since they
      won't need to worry about the CRTC being changed.
      
      The disable() function will do the reverse operation of enable(). One
      of its features is that it should only be called while the pipe is
      already off. This guarantees that FBC is stopped and nothing is
      using the CFB.
      
      With this, the activate() and deactivate() functions just start and
      temporarily stop FBC. They are the ones touching the hardware enable
      bit, so HW state reflects dev_priv->crtc.active.
      
      The last function remaining is update(). A lot of times I thought
      about renaming update() to activate() or try_to_activate() since it's
      called when we want to activate FBC. The thing is that update() may
      not only decide to activate FBC, but also deactivate or keep it on the
      same state, so I'll leave this name for now.
      
      Moving code to enable() and disable() will also help in case we decide
      to move FBC to pipe_config or something else later.
      
      The current patch only puts the very basic code on enable() and
      disable(). The next commits will take care of moving more stuff from
      update() to the new functions.
      
      v2:
        - Rebase.
        - Improve commit message (Chris).
      v3: Rebase after changing the patch order.
      v4: Rebase again after upstream changes.
      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/
      d029bcad
    • P
      drm/i915: introduce is_active/activate/deactivate to the FBC terminology · 0e631adc
      Paulo Zanoni 提交于
      The long term goal is to have enable/disable as the higher level
      functions and activate/deactivate as the lower level functions, just
      like we do for PSR and for the CRTC. This way, we'll run enable and
      disable once per modeset, while update, activate and deactivate will
      be run many times. With this, we can move the checks and code that
      need to run only once per modeset to enable(), making the code simpler
      and possibly a little faster.
      
      This patch is just the first step on the conversion: it starts by
      converting the current low level functions from enable/disable to
      activate/deactivate. This patch by itself has no benefits other than
      making review and rebase easier. Please see the next patches for more
      details on the conversion.
      
      v2:
        - Rebase.
        - Improve commit message (Chris).
      v3: 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/
      0e631adc
    • P
      drm/i915: pass the crtc as an argument to intel_fbc_update() · 754d1133
      Paulo Zanoni 提交于
      There's no need to reevaluate the status of every single crtc when a
      single crtc changes its state.
      
      With this, we're cutting the case where due to a change in pipe B,
      intel_fbc_update() is called, then intel_fbc_find_crtc() concludes FBC
      should be enabled on pipe A, then it completely rechecks the state of
      pipe A only to conclude FBC should remain enabled on pipe A. If any
      change on pipe A triggers a need to recompute whether FBC is valid on
      pipe A, then at some point someone is going to call
      intel_fbc_update(PIPE_A).
      
      The addition of intel_fbc_deactivate() is necessary so we keep track
      of the previously selected CRTC when we do invalidate/flush. We're
      also going to continue the enable/disable/activate/deactivate concept
      in the next patches.
      
      v2: Rebase.
      v3: 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/
      754d1133
    • P
      drm/i915: set dev_priv->fbc.crtc before scheduling the enable work · e9c5fd26
      Paulo Zanoni 提交于
      This thing where we need to get the crtc either from the work
      structure or the fbc structure itself is confusing and unnecessary.
      Set fbc.crtc right when scheduling the enable work so we can always
      use it.
      
      The problem is not what gets passed and how to retrieve it. The
      problem is that when we're in the other parts of the code we always
      have to keep in mind that if FBC is already enabled we have to get the
      CRTC from place A, if FBC is scheduled we have to get the CRTC from
      place B, and if it's disabled there's no CRTC. Having a single place
      to retrieve the CRTC from allows us to treat the "is enabled" and "is
      scheduled" cases as the same case, reducing the mistake surface. I
      guess I should add this to the commit message.
      
      Besides the immediate advantages, this is also going to make one of
      the next commits much simpler. And even later, when we introduce
      enable/disable + activate/deactivate, this will be even simpler as
      we'll set the CRTC at enable time. So all the
      activate/deactivate/update code can just look at the single CRTC
      variable regardless of the current state.
      
      v2: Improve commit message (Chris).
      v3: 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/
      e9c5fd26
    • P
      drm/i915: fix the CFB size check · 90d5234f
      Paulo Zanoni 提交于
      In function find_compression_threshold() we try to over-allocate CFB
      space in order to reduce reallocations and fragmentation, and we're
      not considering that at the CFB size check. Consider it.
      
      There is also a longer-term plan to kill
      dev_priv->fbc.uncompressed_size, but this will come later.
      
      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/
      90d5234f
  2. 10 11月, 2015 12 次提交
  3. 04 11月, 2015 1 次提交
  4. 29 10月, 2015 1 次提交
    • R
      drm/i915/kbl: Introduce Kabylake platform defition. · ef11bdb3
      Rodrigo Vivi 提交于
      Kabylake is a Intel® Processor containing Intel® HD Graphics
      following Skylake.
      
      It is Gen9p5, so it inherits everything from Skylake.
      
      Let's start by adding the platform separated from Skylake
      but reusing most of all features, functions etc. Later we
      rebase the PCI-ID patch without is_skylake=1
      so we don't replace what original Author did there.
      
      Few IS_SKYLAKEs if statements are not being covered by this patch
      on purpose:
         - Workarounds: Kabylake is derivated from Skylake H0 so no
           		  W/As apply here.
         - GuC: A following patch removes Kabylake support with an
           	  explanation: No firmware available yet.
         - DMC/CSR: Done in a separated patch since we need to be carefull
           	      and load the version for revision 7 since
      	      Kabylake is Skylake H0.
      
      v2: relative cleaner commit message and added the missed
          IS_KABYLAKE to intel_i2c.c as pointed out by Jani.
      
      Cc: Jani Nikula <jani.nikula@intel.com>
      Signed-off-by: NRodrigo Vivi <rodrigo.vivi@intel.com>
      Signed-off-by: NJani Nikula <jani.nikula@intel.com>
      ef11bdb3
  5. 09 10月, 2015 2 次提交
    • P
      drm/i915: fix FBC buffer size checks · 856312ae
      Paulo Zanoni 提交于
      According to my experiments (and later confirmation from the hardware
      developers), the maximum sizes mentioned in the specification delimit
      how far in the buffer the hardware tracking can go. And the hardware
      calculates the size based on the plane address we provide - and the
      provided plane address might not be the real x:0,y:0 point due to the
      compute_page_offset() function.
      
      On platforms that do the x/y offset adjustment trick it will be really
      hard to reproduce a bug, but on the current SKL we can reproduce the
      bug with igt/kms_frontbuffer_tracking/fbc-farfromfence. With this
      patch, we'll go from "CRC assertion failure" to "FBC unexpectedly
      disabled", which is still a failure on the test suite but is not a
      perceived user bug - you will just not save as much power as you could
      if FBC is disabled.
      
      v2, rewrite patch after clarification from the Hadware guys:
        - Rename function so it's clear what the check is for.
        - Use the new intel_fbc_get_plane_source_sizes() function in order
          to get the proper sizes as seen by FBC.
      v3:
        - Rebase after the s/sizes/size/ on the previous patch.
        - Adjust comment wording (Ville).
        - s/used_/effective_/ (Ville).
      
      Testcase: igt/kms_frontbuffer_tracking/fbc-farfromfence (SKL)
      Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
      Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      856312ae
    • P
      drm/i915: fix CFB size calculation · c4ffd409
      Paulo Zanoni 提交于
      We were considering the whole framebuffer height, but the spec says we
      should only consider the active display height size. There were still
      some unclear questions based on the spec, but the hardware guys
      clarified them for us. According to them:
      
      - CFB size = CFB stride * Number of lines FBC writes to CFB
      - CFB stride = plane stride / compression limit
      - Number of lines FBC writes to CFB = MIN(plane source height, maximum
        number of lines FBC writes to CFB)
      - Plane source height =
        - pipe source height (PIPE_SRCSZ register) (before SKL)
        - plane size register height (PLANE_SIZE register) (SKL+)
      - Maximum number of lines FBC writes to CFB =
        - plane source height (before HSW)
        - 2048 (HSW+)
      
      For the plane source height, I could just have made our code do
      I915_READ() in order to be more future proof, but since it's not cool
      to do register reads I decided to just recalculate the values we use
      when we actually write to those registers.
      
      With this patch, depending on your machine configuration, a lot of the
      kms_frontbuffer_tracking subtests that used to result in a SKIP due to
      not enough stolen memory still start resulting in a PASS.
      
      v2: Use the clipped src size instead of pipe_src_h (Ville).
      v3: Use the appropriate information provided by the hardware guys.
      v4: Bikesheds: s/sizes/size/, s/fb_cpp/cpp/ (Ville).
      v5: - Don't use crtc->config->pipe_src_x for BDW- (Ville).
          - Fix the register name written in the comment.
      Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com>
      Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      c4ffd409
  6. 30 9月, 2015 2 次提交
  7. 23 9月, 2015 10 次提交
  8. 05 8月, 2015 1 次提交
    • P
      drm/i915: fix FBC frontbuffer tracking flushing code · 6f4551fe
      Paulo Zanoni 提交于
      Due to the way busy_bits was handled, we were not doing any flushes if
      we didn't previously get an invalidate. Since it's possible to get
      flushes without an invalidate first, remove the busy_bits early
      return.
      
      So now that we don't have the busy_bits guard anymore we'll need the
      origin check for the GTT tracking (we were not doing anything on GTT
      flushes due to the GTT check at invalidate()).
      
      As a last detail, since we can get multiple consecutive flushes,
      disable FBC before updating it, otherwise intel_fbc_update() will just
      keep FBC enabled instead of restarting it.
      
      Notice that this does not fix any of the current IGT tests due to the
      fact that we still have a few intel_fbc() calls at points where we
      also have the frontbuffer tracking calls: we didn't fully convert to
      frontbuffer tracking yet. Once we remove those calls and start relying
      only on the frontbuffer tracking infrastructure we'll need this patch.
      Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com>
      Reviewed-by: NRodrigo Vivi <rodrigo.vivi@intel.com>
      Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      6f4551fe
  9. 08 7月, 2015 1 次提交