An error occurred fetching the project authors.
  1. 10 Apr, 2015 3 commits
  2. 24 Mar, 2015 2 commits
    • Imre Deak's avatar
      drm/i915: move clearing of RPS interrupt bits from disable to reset time · 096fad9e
      Imre Deak authored
      The logical place for clearing the RPS latched interrupt bits is when
      resetting the RPS interrupts, so move the corresponding part from the RPS
      disable function to the reset function. During resetting we already
      cleared the IIR bits, so the only thing missing there was clearing pm_iir.
      
      Note that we call gen6_disable_rps_interrupts() also during driver load
      and resume time via intel_uncore_sanitize() when i915 interrupts are
      still not installed. If there are any pending RPS bits at this point
      (which after this patch wouldn't be cleared) they will be cleared by the
      reset code via the interrupt preinstall hooks.
      Signed-off-by: default avatarImre Deak <imre.deak@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      096fad9e
    • Imre Deak's avatar
      drm/i915: fix race when clearing RPS IIR bits · 58072ccb
      Imre Deak authored
      When disabling RPS interrupts there is a race where we disable RPS
      inerrupts while the interrupt handler is running and the handler has
      already latched the pending RPS interrupt from the master IIR register.
      Afterwards the disabling path clears the PM IIR bits, making the state
      of pending interrupts inconsistent from the interrupt handler's point of
      view. This triggers the following warning: "The master control interrupt
      lied (PM)!".
      
      To fix this make sure that any running interrupt handler (which may
      have already latched the master IIR) finishes before clearing the IIR
      bits.
      
      Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87347Signed-off-by: default avatarImre Deak <imre.deak@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      58072ccb
  3. 20 Mar, 2015 2 commits
    • Chris Wilson's avatar
      drm/i915: Use down ei for manual Baytrail RPS calculations · 6f4b12f8
      Chris Wilson authored
      Use both up/down manual ei calcuations for symmetry and greater
      flexibility for reclocking, instead of faking the down interrupt based
      on a fixed integer number of up interrupts.
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Reviewed-by: Deepak S<deepak.s@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      6f4b12f8
    • Chris Wilson's avatar
      drm/i915: Improved w/a for rps on Baytrail · 43cf3bf0
      Chris Wilson authored
      Rewrite commit 31685c25
      Author: Deepak S <deepak.s@linux.intel.com>
      Date:   Thu Jul 3 17:33:01 2014 -0400
      
          drm/i915/vlv: WA for Turbo and RC6 to work together.
      
      Other than code clarity, the major improvement is to disable the extra
      interrupts generated when idle.  However, the reclocking remains rather
      slow under the new manual regime, in particular it fails to downclock as
      quickly as desired. The second major improvement is that for certain
      workloads, like games, we need to combine render+media activity counters
      as the work of displaying the frame is split across the engines and both
      need to be taken into account when deciding the global GPU frequency as
      memory cycles are shared.
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Cc: Deepak S <deepak.s@linux.intel.com>
      Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
      Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
      Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
      Reviewed-by: Deepak S<deepak.s@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      43cf3bf0
  4. 17 Mar, 2015 3 commits
  5. 25 Feb, 2015 1 commit
    • Matt Roper's avatar
      drm/i915: Use enabled value from crtc_state rather than crtc (v2) · 83d65738
      Matt Roper authored
      As vendors transition their drivers from legacy to atomic there's some
      duplication of data between drm_crtc and drm_crtc_state (since
      unconverted drivers likely won't have a state structure).
      
      i915 is partially converted and does have a crtc->state structure, but
      still uses direct crtc fields internally in many places, which causes
      the two sets of data to get out of sync.  As of commit
      
              commit 31c946e8
              Author: Daniel Vetter <daniel.vetter@ffwll.ch>
              Date:   Sun Feb 22 12:24:17 2015 +0100
      
                  drm: If available use atomic state in getcrtc ioctl
      
                  This way drivers fully converted to atomic don't need to update these
                  legacy state variables in their modeset code any more.
      Reviewed-by: default avatarRob Clark <robdclark@gmail.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      
      the DRM core starts assuming that the presence of a ->state structure
      implies that it should make use of the values stored there which, on
      i915, leads to the core code using stale values for CRTC 'enabled'
      status.
      
      Let's switch over to using the state value of 'enable' internally rather
      than using the drm_crtc field.  This ensures that our driver internals
      are working from the same data that the DRM core is, avoiding
      mismatches.
      
      This patch was generated with Coccinelle using the following semantic
      patch:
      
              <smpl>
              @@
              struct drm_crtc C;
              struct drm_crtc *CP;
              @@
              (
              - C.enabled
              + C.state->enable
              |
              - CP->enabled
              + CP->state->enable
              )
      
              // For assignments, we still update the legacy value as well as the state value
              // so add an extra assignment statement for that.
              @@
              struct drm_crtc C;
              struct drm_crtc *CP;
              expression E;
              @@
              (
                C.state->enable = E;
              + C.enabled = E;
              |
                CP->state->enable = E;
              + CP->enabled = E;
              )
              </smpl>
      
      The crtc->mode and crtc->hwmode fields should probably be transitioned
      over as well eventually, but we seem to do an okay job of keeping those
      up-to-date already so I want to minimize the changes that will clash
      with Ander's in-progress atomic work.
      
      v2: Don't remove the assignments to the legacy value when we assign to
          the state value.  A second cocci stanza takes care of adding the
          legacy assignment back where appropriate.  (Daniel)
      
      Cc: Daniel Vetter <daniel@ffwll.ch>
      Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
      Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      83d65738
  6. 24 Feb, 2015 1 commit
    • Imre Deak's avatar
      drm/i915: avoid processing spurious/shared interrupts in low-power states · 2dd2a883
      Imre Deak authored
      Atm, it's possible that the interrupt handler is called when the device
      is in D3 or some other low-power state. It can be due to another device
      that is still in D0 state and shares the interrupt line with i915, or on
      some platforms there could be spurious interrupts even without sharing
      the interrupt line. The latter case was reported by Klaus Ethgen using a
      Lenovo x61p machine (gen 4). He noticed this issue via a system
      suspend/resume hang and bisected it to the following commit:
      
      commit e11aa362
      Author: Jesse Barnes <jbarnes@virtuousgeek.org>
      Date:   Wed Jun 18 09:52:55 2014 -0700
      
          drm/i915: use runtime irq suspend/resume in freeze/thaw
      
      This is a problem, since in low-power states IIR will always read
      0xffffffff resulting in an endless IRQ servicing loop.
      
      Fix this by handling interrupts only when the driver explicitly enables
      them and so it's guaranteed that the interrupt registers return a valid
      value.
      
      Note that this issue existed even before the above commit, since during
      runtime suspend/resume we never unregistered the handler.
      
      v2:
      - clarify the purpose of smp_mb() vs. synchronize_irq() in the
        code comment (Chris)
      
      v3:
      - no need for an explicit smp_mb(), we can assume that synchronize_irq()
        and the mmio read/writes in the install hooks provide for this (Daniel)
      - remove code comment as the remaining synchronize_irq() is self
        explanatory (Daniel)
      
      v4:
      - drm_irq_uninstall() implies synchronize_irq(), so no need to call it
        explicitly (Daniel)
      
      Reference: https://lkml.org/lkml/2015/2/11/205Reported-and-bisected-by: default avatarKlaus Ethgen <Klaus@Ethgen.ch>
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarImre Deak <imre.deak@intel.com>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
      2dd2a883
  7. 23 Feb, 2015 2 commits
  8. 13 Feb, 2015 1 commit
    • Ville Syrjälä's avatar
      drm/i915: Introduce intel_set_rps() · ffe02b40
      Ville Syrjälä authored
      Replace the valleyview_set_rps() and gen6_set_rps() calls with
      intel_set_rps() which itself does the IS_VALLEYVIEW() check. The
      code becomes simpler since the callers don't have to do this check
      themselves.
      
      Most of the change was performe with the following semantic patch:
      @@
      expression E1, E2, E3;
      @@
      - if (IS_VALLEYVIEW(E1)) {
      -  valleyview_set_rps(E2, E3);
      - } else {
      -  gen6_set_rps(E2, E3);
      - }
      + intel_set_rps(E2, E3);
      
      Adding intel_set_rps() and making valleyview_set_rps() and gen6_set_rps()
      static was done manually. Also valleyview_set_rps() had to be moved a
      bit avoid a forward declaration.
      
      v2: Use a less greedy semantic patch
      
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Suggested-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
      Reviewed-by Chris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      ffe02b40
  9. 03 Feb, 2015 1 commit
    • Daniel Vetter's avatar
      drm/i915: Remove bogus locking check in the hangcheck code · b838cbee
      Daniel Vetter authored
      You can _never_ assert that a lock is not held, except in some very
      restricted corner cases where it's guranteed that your code is running
      single-threade (e.g. driver load before you've published any pointers
      leading to that lock).
      
      In addition the early return breaks a bunch of testcases since with
      highly concurrent hangcheck stress tests the reset fails to work and
      the test doesn't recover and time out.
      
      This regression has been introduced in
      
      commit b8d24a06
      Author: Mika Kuoppala <mika.kuoppala@linux.intel.com>
      Date:   Wed Jan 28 17:03:14 2015 +0200
      
          drm/i915: Remove nested work in gpu error handling
      
      Aside: It is possible to check whether a given task doesn't hold a
      lock, but only when lockdep is enabled, using the lockdep_assert_held
      stuff.
      
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Cc: Mika Kuoppala <mika.kuoppala@intel.com>
      Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88908Reviewed-by: default avatarMika Kuoppala <mika.kuoppala@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      b838cbee
  10. 29 Jan, 2015 1 commit
    • Mika Kuoppala's avatar
      drm/i915: Remove nested work in gpu error handling · b8d24a06
      Mika Kuoppala authored
      Now when we declare gpu errors only through our own dedicated
      hangcheck workqueue there is no need to have a separate workqueue
      for handling the resetting and waking up the clients as the deadlock
      concerns are no more.
      
      The only exception is i915_debugfs::i915_set_wedged, which triggers
      error handling through process context. However as this is only used through
      test harness it is responsibility for test harness not to introduce hangs
      through both debug interface and through hangcheck mechanism at the same time.
      
      Remove gpu_error.work and let the hangcheck work do the tasks it used to.
      
      v2: Add a big warning sign into i915_debugfs::i915_set_wedged (Chris)
      
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: default avatarMika Kuoppala <mika.kuoppala@intel.com>
      Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      b8d24a06
  11. 28 Jan, 2015 1 commit
    • Chris Wilson's avatar
      drm/i915: Convert hangcheck from a timer into a delayed work item · 737b1506
      Chris Wilson authored
      When run as a timer, i915_hangcheck_elapsed() must adhere to all the
      rules of running in a softirq context. This is advantageous to us as we
      want to minimise the risk that a driver bug will prevent us from
      detecting a hung GPU. However, that is irrelevant if the driver bug
      prevents us from resetting and recovering. Still it is prudent not to
      rely on mutexes inside the checker, but given the coarseness of
      dev->struct_mutex doing so is extremely hard.
      
      Give in and run from a work queue, i.e. outside of softirq.
      
      v2: Use own workqueue to avoid deadlocks (Daniel)
          Cleanup commit msg and add comment to i915_queue_hangcheck() (Chris)
      
      Cc: Jani Nikula <jani.nikula@intel.com>
      Cc: Daniel Vetter <dnaiel.vetter@ffwll.chm>
      Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
      Signed-off-by: default avatarMika Kuoppala <mika.kuoppala@intel.com>
      [danvet: Remove accidental kerneldoc comment starter, to appease the 0
      day builder.]
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      737b1506
  12. 27 Jan, 2015 3 commits
    • Daniel Vetter's avatar
      drm/i915: Use symbolic irqreturn for ->hpd_pulse · b2c5c181
      Daniel Vetter authored
      Self-explanatory code is better code.
      
      Cc: Dave Airlie <airlied@redhat.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      Reviewed-by: default avatarJani Nikula <jani.nikula@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      b2c5c181
    • Ander Conselvan de Oliveira's avatar
      drm/i915: Make intel_crtc->config a pointer · 6e3c9717
      Ander Conselvan de Oliveira authored
      To match the semantics of drm_crtc->state, which this will eventually
      become. The allocation of the memory for config will be fixed in a
      followup patch. By adding the extra _config field to intel_crtc it was
      possible to generate this entire patch with the cocci script below.
      
      @@ @@
      struct intel_crtc {
      ...
      -struct intel_crtc_state config;
      +struct intel_crtc_state _config;
      +struct intel_crtc_state *config;
      ...
      }
      @@ struct intel_crtc *crtc; @@
      -memset(&crtc->config, 0, sizeof(crtc->config));
      +memset(crtc->config, 0, sizeof(*crtc->config));
      @@ @@
      __intel_set_mode(...) {
      <...
      -to_intel_crtc(crtc)->config = *pipe_config;
      +(*(to_intel_crtc(crtc)->config)) = *pipe_config;
      ...>
      }
      @@ @@
      intel_crtc_init(...) {
      ...
      WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe);
      +intel_crtc->config = &intel_crtc->_config;
      return;
      ...
      }
      @@ struct intel_crtc *crtc; @@
      -&crtc->config
      +crtc->config
      @@ struct intel_crtc *crtc; identifier member; @@
      -crtc->config.member
      +crtc->config->member
      @@ expression E; @@
      -&(to_intel_crtc(E)->config)
      +to_intel_crtc(E)->config
      @@ expression E; identifier member; @@
      -to_intel_crtc(E)->config.member
      +to_intel_crtc(E)->config->member
      
      v2: Clarify manual changes by splitting them into another patch. (Matt)
          Improve cocci script to generate even more of the changes. (Ander)
      Signed-off-by: default avatarAnder Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
      Reviewed-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      6e3c9717
    • Ander Conselvan de Oliveira's avatar
      drm/i915: Embedded struct drm_crtc_state in intel_crtc_state · 2d112de7
      Ander Conselvan de Oliveira authored
      And get rid of the duplicate mode structures. This patch was generated
      with the following semantic patch:
      
      @@ @@
      struct intel_crtc_state {
      +struct drm_crtc_state base;
      +
      ...
      -struct drm_display_mode requested_mode;
      -struct drm_display_mode adjusted_mode;
      ...
      }
      @@ struct intel_crtc_state *state; @@
      -state->adjusted_mode
      +state->base.adjusted_mode
      @@ struct intel_crtc_state *state; @@
      -state->requested_mode
      +state->base.mode
      @@ struct intel_crtc_state state; @@
      -state.adjusted_mode
      +state.base.adjusted_mode
      @@ struct intel_crtc_state state; @@
      -state.requested_mode
      +state.base.mode
      @@ struct drm_crtc *crtc; @@
      -to_intel_crtc(crtc)->config.adjusted_mode
      +to_intel_crtc(crtc)->config.base.adjusted_mode
      @@ identifier member; expression E; @@
      -PIPE_CONF_CHECK_FLAGS(adjusted_mode.member, E);
      +PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.member, E);
      @@ identifier member; @@
      -PIPE_CONF_CHECK_I(adjusted_mode.member);
      +PIPE_CONF_CHECK_I(base.adjusted_mode.member);
      @@ identifier member; @@
      -PIPE_CONF_CHECK_CLOCK_FUZZY(adjusted_mode.member);
      +PIPE_CONF_CHECK_CLOCK_FUZZY(base.adjusted_mode.member);
      
      v2: Completely generate the patch with cocci. (Ander)
      Signed-off-by: default avatarAnder Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
      Reviewed-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      2d112de7
  13. 12 Jan, 2015 4 commits
  14. 18 Dec, 2014 1 commit
    • Ville Syrjälä's avatar
      drm/i915: Don't call intel_prepare_page_flip() multiple times on gen2-4 · 7d47559e
      Ville Syrjälä authored
      The flip stall detector kicks in when pending>=INTEL_FLIP_COMPLETE. That
      means if we first call intel_prepare_page_flip() but don't call
      intel_finish_page_flip(), the next stall check will erroneosly think
      the page flip was somehow stuck.
      
      With enough debug spew emitted from the interrupt handler my 830 hangs
      when this happens. My theory is that the previous vblank interrupt gets
      sufficiently delayed that the handler will see the pending bit set in
      IIR, but ISR still has the bit set as well (ie. the flip was processed
      by CS but didn't complete yet). In this case the handler will proceed
      to call intel_check_page_flip() immediately after
      intel_prepare_page_flip(). It then tries to print a backtrace for the
      stuck flip WARN, which apparetly results in way too much debug spew
      delaying interrupt processing further. That then seems to cause an
      endless loop in the interrupt handler, and the machine is dead until
      the watchdog kicks in and reboots. At least limiting the number of
      iterations of the loop in the interrupt handler also prevented the
      hang.
      
      So it seems better to not call intel_prepare_page_flip() without
      immediately calling intel_finish_page_flip(). The IIR/ISR trickery
      avoids races here so this is a perfectly safe thing to do.
      
      v2: Fix typo in commit message (checkpatch)
      
      Cc: stable@vger.kernel.org
      Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88381
      Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85888Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
      Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
      7d47559e
  15. 15 Dec, 2014 2 commits
    • Imre Deak's avatar
      drm/i915: move RPS PM_IER enabling to gen6_enable_rps_interrupts · 78e68d36
      Imre Deak authored
      Paulo noticed that we don't enable RPS interrupts via PM_IER in
      gen6_enable_rps_interrupts(). This wasn't a problem so far, since the
      only place we disabled RPS interrupts was during system/runtime suspend
      and after that we reenable all interrupts in the IRQ pre/postinstall
      hooks.
      
      In the next patch we'll disable/reenable RPS interrupts during GPU reset
      too, but not call IRQ uninstall, pre/postinstall hooks, so there the
      above wouldn't work. The logical place for programming PM_IER is
      gen6_enable_rps_interrupts() and this also makes the function more
      symmetric with gen6_disable_rps_interrupts(), so move the programming
      there from the postinstall hooks.
      
      Note that these changes don't affect the ILK RPS interrupt code, which
      could be sanitized in a similar way. But that can be done as a
      follow-up.
      
      Credits-to: Paulo Zanoni <paulo.r.zanoni@intel.com>
      Signed-off-by: default avatarImre Deak <imre.deak@intel.com>
      Reviewed-by: default avatarPaulo Zanoni <paulo.r.zanoni@intel.com>
      Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
      78e68d36
    • Daniel Vetter's avatar
      drm/i915: Name the lrc irq handler correctly · 3f7531c3
      Daniel Vetter authored
      We consistently use the _irq_handler postfix for functions called in
      hardirq context. Especially when it's a non-static function hardirq is
      a crazy enough calling context to warrant this level of ocd. So rename
      it.
      
      Cc: Thomas Daniel <thomas.daniel@intel.com>
      Reviewed-by: default avatarThomas Daniel <thomas.daniel@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      3f7531c3
  16. 11 Dec, 2014 1 commit
    • Imre Deak's avatar
      drm/i915: vlv: fix IRQ masking when uninstalling interrupts · c352d1ba
      Imre Deak authored
      irq_mask should include all IRQ bits that we want to mask, but atm we
      set it incorrectly to the inverse of this. If the mask is used
      subsequently to enable/disable some IRQ bits, we may unintentionally
      unmask unrelated IRQs. I can't see any way that this can lead to a real
      problem in the current -nightly code, since the first place the mask
      will be used next (after a suspend/resume cycle) is in
      valleyview_irq_postinstall(), but the mask is reset there to its proper
      value.
      
      This causes a problem in the upstream kernel though, where - due to another
      issue - the mask is used in the above way to disable only the display IRQs.
      This other issue is fixed by:
      
      commit 950eabaf
      Author: Imre Deak <imre.deak@intel.com>
      Date:   Mon Sep 8 15:21:09 2014 +0300
      
          drm/i915: vlv: fix display IRQ enable/disable
      
      Interestingly, even with the above two bugs, we shouldn't in theory have
      any real problems (arguably a famous last sentence:). That's because
      even if we unmask something unintentionally via the VLV_IMR/VLV_IER
      register the master IRQ masking bit in VLV_MASTER_IER is still set and
      should prevent all i915 interrupts. According to my testing on an ASUS
      T100 with DSI output this isn't the case at least with the
      MIPIA_INTERRUPT. Leaving this one unmasked in IMR/IER, while having
      VLV_MASTER_IER set to 0 may lead to a lockup during system suspend as
      shown in the bugzilla ticket below. This fix should get rid of the
      problem reported there in upstream and older kernels.
      
      Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85920
      Cc: stable@vger.kernel.org (v3.15+)
      Signed-off-by: default avatarImre Deak <imre.deak@intel.com>
      Reviewed-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
      Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
      c352d1ba
  17. 08 Dec, 2014 1 commit
    • Daniel Vetter's avatar
      drm/i915: Check mask/bit helper functions · 15a17aae
      Daniel Vetter authored
      After a bit of irc discussion we've concluded that it would be prudent
      to check that callers use the mask/enable paramters correctly. So add
      a WARN_ON.
      
      Spurred by Damien's bugfix which added _MASKED_FIELD.
      
      v2: We use WARN_ON(1) a lot to catch default cases in switch blocks
      which should always be extended. So this doesn't work really. Dunno
      why gcc only started complaining when I've moved the WARN out of the
      static inline helper to address a feedback from Jani.
      
      Cc: Damien Lespiau <damien.lespiau@intel.com>
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Cc: Jani Nikula <jani.nikula@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      15a17aae
  18. 06 Dec, 2014 1 commit
    • John Harrison's avatar
      drm/i915: Additional request structure tracing · bcfcc8ba
      John Harrison authored
      Added the request structure's 'uniq' identifier to the trace information. Also
      renamed the '_complete' trace event to '_notify' as it actually happens in the
      IRQ 'notify_ring()' function. The intention is to add a new '_complete' trace
      event which occurs when a request structure is actually marked as complete.
      However, at the moment the completion status is re-tested every time the query
      is made so there isn't a completion event as such.
      
      v2: New patch added to series.
      
      v3: Rebased to remove completion caching as that is apparently contentious.
      
      Change-Id: Ic9bcde67d175c6c03b96217cdcb6e4cc4aa45d67
      For: VIZ-4377
      Signed-off-by: default avatarJohn Harrison <John.C.Harrison@Intel.com>
      Reviewed-by: default avatarThomas Daniel <Thomas.Daniel@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      bcfcc8ba
  19. 03 Dec, 2014 6 commits
    • John Harrison's avatar
      drm/i915: Convert 'i915_seqno_passed' calls into 'i915_gem_request_completed' · 1b5a433a
      John Harrison authored
      Almost everywhere that caled i915_seqno_passed() was really asking 'has the
      given seqno popped out of the hardware yet?'. Thus it had to query the current
      hardware seqno and then do a signed delta comparison (which copes with wrapping
      around zero but not with seqno values more than 2GB apart, although the latter
      is unlikely!).
      
      Now that the majority of seqno instances have been replaced with request
      structures, it is possible to convert this test to be request based as well.
      There is now a 'i915_gem_request_completed()' function which takes a request and
      returns true or false as appropriate. Note that this currently just wraps up the
      original _passed() test but a later patch in the series will reduce this to
      simply returning a cached internal value, i.e.:
        _completed(req) { return req->completed; }'
      
      This checkin converts almost all _seqno_passed() calls. The only one left is in
      the semaphore code which still requires seqnos not request structures.
      
      For: VIZ-4377
      Signed-off-by: default avatarJohn Harrison <John.C.Harrison@Intel.com>
      Reviewed-by: default avatarThomas Daniel <Thomas.Daniel@intel.com>
      [danvet: Drop hunk touching the trace_irq code since I've dropped the
      patch which converts that, and resolve resulting conflict.]
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      1b5a433a
    • John Harrison's avatar
      drm/i915: Convert 'ring_idle()' to use requests not seqnos · 44cdd6d2
      John Harrison authored
      More seqno value to request structure conversions. Note, this change temporarily
      moves the 'get_seqno()' call inside ring_idle() but this will disappear again in
      a later patch when i915_seqno_passed() itself is converted.
      
      For: VIZ-4377
      Signed-off-by: default avatarJohn Harrison <John.C.Harrison@Intel.com>
      Reviewed-by: default avatarThomas Daniel <Thomas.Daniel@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      44cdd6d2
    • Imre Deak's avatar
      drm/i915: mask RPS IRQs properly when disabling RPS · 9939fba2
      Imre Deak authored
      Atm, igt/gem_reset_stats can trigger the recently added WARN on
      left-over PM_IIR bits in gen6_enable_rps_interrupts(). There are two
      reasons for this:
      1. we call intel_enable_gt_powersave() without a preceeding
         intel_disable_gt_powersave()
      2. gen6_disable_rps_interrupts() doesn't mask interrupts in PM_IMR
      
      1. means RPS interrupts will remain enabled and can be serviced during
      the HW initialization after a GPU reset. 2. means even if we called
      gen6_disable_rps_interrupts() any new RPS interrupt during RPS
      initialization would still propagate to PM_IIR too early (though
      wouldn't be serviced).
      
      This patch solves the 2. issue by also masking interrupts in PM_IMR, the
      following patch fixes 1. getting rid of the WARN. This also makes
      intel_enable_gt_powersave() and intel_disable_gt_powersave() more
      symmetric.
      
      Since gen6_disable_rps_interrupts() is called during driver loading with
      i915 interrupts disabled add a new version of gen6_disable_pm_irq() that
      doesn't WARN for this.
      
      Also while at it, get the irq_lock around the whole PM_IMR/IER/IIR
      programming sequence and make sure that any queued PM_IIR bit is also
      cleared.
      
      The WARN was caught by PRTS after I sent my previous RPS sanitizing
      patchset and I could easily reproduce it on HSW. To actually fix it we
      also need the next patch.
      Reported-by: default avatarHe, Shuang <shuang.he@intel.com>
      Signed-off-by: default avatarImre Deak <imre.deak@intel.com>
      Reviewed-by: default avatarPaulo Zanoni <paulo.r.zanoni@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      9939fba2
    • Daniel Vetter's avatar
      drm/i915: Tune down spurious CRC interrupt warning · 34273620
      Daniel Vetter authored
      We don't really synchronously turn them off from debugfs. We try to
      avoid hitting them too badly by waiting one vblank, but apparently the
      irq handler can still race through that gap.
      
      Since this isn't really all that important for testcases, only for
      debugging CRC issues let's tune it down to a debug message.
      
      Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82602
      Cc: Damien Lespiau <damien.lespiau@intel.com>
      Acked-by: default avatarDamien Lespiau <damien.lespiau@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      34273620
    • Ville Syrjälä's avatar
      drm/i915: Grab modeset locks for GPU rest on pre-ctg · 7514747d
      Ville Syrjälä authored
      On gen4 and earlier the GPU reset also resets the display, so we should
      protect against concurrent modeset operations. Grab all the modeset locks
      around the entire GPU reset dance, remebering first ti dislogde any
      pending page flip to make sure we don't deadlock. Any pageflip coming
      in between these two steps should fail anyway due to reset_in_progress,
      so this should be safe.
      
      This fixes a lot of failed asserts in the modeset code when there's a
      modeset racing with the reset. Naturally the asserts aren't happy when
      the expected state has disappeared.
      
      v2: Drop UMS checks, complete pending flips after the reset (Daniel)
      
      Cc: Daniel Vetter <daniel@ffwll.ch>
      Signed-off-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      7514747d
    • Daniel Vetter's avatar
      drm/i915: Stop gathering error states for CS error interrupts · aaecdf61
      Daniel Vetter authored
      There's quite a few bug reports with error states where the error
      reasons makes just about no sense at all. Like dying on tlbs for a
      display plane that's not even there. Also users don't really report a
      lot of bad side effects generally, just the error states.
      
      Furthermore we don't even enable these interrupts any more on gen5+
      (though the handling code is still there). So this mostly concerns old
      platforms.
      
      Given all that lets make our lives a bit easier and stop capturing
      error states, in the hopes that we can just ignore them. In case
      that's not true and the gpu indeed dies the hangcheck should
      eventually kick in. And I've left some debug log in to make this case
      noticeble. Referenced bug is just an example.
      
      v2: Fix missing \n Jani spotted.
      
      References: https://bugs.freedesktop.org/show_bug.cgi?id=82095
      References: https://bugs.freedesktop.org/show_bug.cgi?id=85944Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      aaecdf61
  20. 20 Nov, 2014 1 commit
  21. 19 Nov, 2014 2 commits
    • Chris Wilson's avatar
      drm/i915: Remove DRI1 ring accessors and API · 5c6c6003
      Chris Wilson authored
      With the deprecation of UMS, and by association DRI1, we have a tough
      choice when updating the ring access routines. We either rewrite the
      DRI1 routines blindly without testing (so likely to be broken) or take
      the liberty of declaring them no longer supported and remove them
      entirely. This takes the latter approach.
      
      v2: Also remove the DRI1 sarea updates
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      [danvet: Fix rebase conflicts.]
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      5c6c6003
    • Imre Deak's avatar
      drm/i915: sanitize rps irq disabling · d4d70aa5
      Imre Deak authored
      When disabling the RPS interrupts there is a tricky dependency between
      the thread disabling the interrupts, the RPS interrupt handler and the
      corresponding RPS work. The RPS work can reenable the interrupts, so
      there is no straightforward order in the disabling thread to (1) make
      sure that any RPS work is flushed and to (2) disable all RPS
      interrupts. Currently this is solved by masking the interrupts using two
      separate mask registers (first level display IMR and PM IMR) and doing
      the disabling when all first level interrupts are disabled.
      
      This works, but the requirement to run with all first level interrupts
      disabled is unnecessary making the suspend / unload time ordering of RPS
      disabling wrt. other unitialization steps difficult and error prone.
      Removing this restriction allows us to disable RPS early during suspend
      / unload and forget about it for the rest of the sequence. By adding a
      more explicit method for avoiding the above race, it also becomes easier
      to prove its correctness. Finally currently we can hit the WARN in
      snb_update_pm_irq(), when a final RPS work runs with the first level
      interrupts already disabled. This won't lead to any problem (due to the
      separate interrupt masks), but with the change in this and the next
      patch we can get rid of the WARN, while leaving it in place for other
      scenarios.
      
      To address the above points, add a new RPS interrupts_enabled flag and
      use this during RPS disabling to avoid requeuing the RPS work and
      reenabling of the RPS interrupts. Since the interrupt disabling happens
      now in intel_suspend_gt_powersave(), we will disable RPS interrupts
      explicitly during suspend (and not just through the first level mask),
      but there is no problem doing so, it's also more consistent and allows
      us to unify more of the RPS disabling during suspend and unload time in
      the next patch.
      
      v2/v3:
      - rebase on patch "drm/i915: move rps irq disable one level up" in the
        patchset
      Signed-off-by: default avatarImre Deak <imre.deak@intel.com>
      Reviewed-by: default avatarPaulo Zanoni <paulo.r.zanoni@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      d4d70aa5