1. 07 Oct, 2015 1 commit
  2. 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
  3. 05 Oct, 2015 1 commit
  4. 02 Oct, 2015 10 commits
  5. 01 Oct, 2015 3 commits
  6. 30 Sep, 2015 11 commits
    • Matt Roper's avatar
      drm/i915: Calculate watermark configuration during atomic check (v2) · 76305b1a
      Matt Roper authored
      v2: Don't forget to actually check the cstate->active value when
          tallying up the number of active CRTC's.  (Ander)
      Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      76305b1a
    • Matt Roper's avatar
      drm/i915: Don't set plane visible during HW readout if CRTC is off · a4611e44
      Matt Roper authored
      We already ensure that pstate->visible = false when crtc->active = false
      during runtime programming; make sure we follow the same logic when
      reading out initial hardware state.
      Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      a4611e44
    • Matt Roper's avatar
      drm/i915: Calculate ILK-style watermarks during atomic check (v3) · a28170f3
      Matt Roper authored
      Calculate pipe watermarks during atomic calculation phase, based on the
      contents of the atomic transaction's state structure.  We still program
      the watermarks at the same time we did before, but the computation now
      happens much earlier.
      
      While this patch isn't too exciting by itself, it paves the way for
      future patches.  The eventual goal (which will be realized in future
      patches in this series) is to calculate multiple sets up watermark
      values up front, and then program them at different times (pre- vs
      post-vblank) on the platforms that need a two-step watermark update.
      
      While we're at it, s/intel_compute_pipe_wm/ilk_compute_pipe_wm/ since
      this function only applies to ILK-style watermarks and we have a
      completely different function for SKL-style watermarks.
      
      Note that the original code had a memcmp() in ilk_update_wm() to avoid
      calling ilk_program_watermarks() if the watermarks hadn't changed.  This
      memcmp vanishes here, which means we may do some unnecessary result
      generation and merging in cases where watermarks didn't change, but the
      lower-level function ilk_write_wm_values already makes sure that we
      don't actually try to program the watermark registers again.
      
      v2: Squash a few commits from the original series together; no longer
          leave pre-calculated wm's in a separate temporary structure since
          it's easier to follow the logic if we just cut over to using the
          pre-calculated values directly.
      
      v3:
       - Pass intel_crtc instead of drm_crtc to .compute_pipe_wm() entrypoint
         and use intel_atomic_get_crtc_state() to avoid need for extra
         casting.  (Ander)
       - Drop unused intel_check_crtc() function prototype.  (Ander)
      Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      a28170f3
    • Matt Roper's avatar
      drm/i915: Calculate pipe watermarks into CRTC state (v3) · de4a9f83
      Matt Roper authored
      A future patch will calculate these during the atomic 'check' phase
      rather than at WM programming time, so let's store the watermark
      values we're planning to use in the CRTC state; the values actually
      active on the hardware remains in intel_crtc.
      
      While we're at it, do some minor restructuring to keep ILK and SKL
      values in a union.
      
      v2: Don't move cxsr_allowed to state (Maarten)
      
      v3: Only calculate watermarks in state.  Still keep active watermarks in
          intel_crtc itself.  (Ville)
      Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      de4a9f83
    • Ville Syrjälä's avatar
      drm/i915: Refactor ilk_update_wm (v3) · de165e0b
      Ville Syrjälä authored
      Split ilk_update_wm() into two parts; one doing the programming
      and the other the calculations.
      
      v2: Fix typo in commit message
      
      v3 (by Matt): Heavily rebased for current codebase.
      Signed-off-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
      Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      de165e0b
    • Matt Roper's avatar
      drm/i915: Drop intel_update_sprite_watermarks · 47c99438
      Matt Roper authored
      The only platform that still has an update_sprite_wm entrypoint is SKL;
      on SKL, intel_update_sprite_watermarks just updates intel_plane->wm and
      then performs a regular watermark update.  However intel_plane->wm is
      only used to update a couple fields in intel_wm_config, and those fields
      are never used by the SKL code, so on SKL an update_sprite_wm is
      effectively identical to an update_wm call.  Since we're already
      ensuring that the regular intel_update_wm is called any time we'd try to
      call intel_update_sprite_watermarks, the whole call is redundant and can
      be dropped.
      Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      47c99438
    • Matt Roper's avatar
      drm/i915/ivb: Move WaCxSRDisabledForSpriteScaling w/a to atomic check · 7809e5ae
      Matt Roper authored
      Determine whether we need to apply this workaround at atomic check time
      and just set a flag that will be used by the main watermark update
      routine.
      
      Moving this workaround into the atomic framework reduces
      ilk_update_sprite_wm() to just a standard watermark update, so drop it
      completely and just ensure that ilk_update_wm() is called whenever a
      sprite plane is updated in a way that would affect watermarks.
      Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      7809e5ae
    • Matt Roper's avatar
      drm/i915/skl: Eliminate usage of pipe_wm_parameters from SKL-style WM (v3) · 3a05f5e2
      Matt Roper authored
      Just pull the info out of the state structures rather than staging
      it in an additional set of structures.  To make this more
      straightforward, we change the signature of several internal WM
      functions to take the crtc state as a parameter.
      
      v2:
       - Don't forget to skip cursor planes on a loop in the DDB allocation
         function to match original behavior.  (Ander)
       - Change a use of intel_crtc->active to cstate->active.  They should
         be identical, but it's better to be consistent.  (Ander)
       - Rework more function signatures to pass states rather than crtc for
         consistency. (Ander)
      
      v3:
        - Add missing "+ 1" to skl_wm_plane_id()'s 'overlay' case. (Maarten)
        - Packed formats should pass '0' to drm_format_plane_cpp(), not 1.
          (Maarten)
        - Drop unwanted WARN_ON() for disabled planes when calculating data
          rate for SKL.  (Maarten)
      Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      3a05f5e2
    • Matt Roper's avatar
      drm/i915/skl: Simplify wm structures slightly (v2) · 4969d33e
      Matt Roper authored
      A bunch of SKL watermark-related structures have the cursor plane as a
      separate entry from the rest of the planes.  Since a previous patch
      updated I915_MAX_PLANES such that those plane arrays now have a slot for
      the cursor, update the code to use the new slot in the existing plane
      arrays and kill off the cursor-specific structures.
      
      There shouldn't be any functional change here; this is just shuffling
      around how the data is stored in some of the data structures.  The whole
      patch is generated with Coccinelle via the following semantic patch:
      
              @@ struct skl_pipe_wm_parameters WMP; @@
              - WMP.cursor
              + WMP.plane[PLANE_CURSOR]
      
              @@ struct skl_pipe_wm_parameters *WMP; @@
              - WMP->cursor
              + WMP->plane[PLANE_CURSOR]
      
              @@ @@
              struct skl_pipe_wm_parameters {
              ...
              - struct intel_plane_wm_parameters cursor;
              ...
              };
      
              @@
              struct skl_ddb_allocation DDB;
              expression E;
              @@
              - DDB.cursor[E]
              + DDB.plane[E][PLANE_CURSOR]
      
              @@
              struct skl_ddb_allocation *DDB;
              expression E;
              @@
              - DDB->cursor[E]
              + DDB->plane[E][PLANE_CURSOR]
      
              @@ @@
              struct skl_ddb_allocation {
              ...
              - struct skl_ddb_entry cursor[I915_MAX_PIPES];
              ...
              };
      
              @@
              struct skl_wm_values WMV;
              expression E1, E2;
              @@
              (
              - WMV.cursor[E1][E2]
              + WMV.plane[E1][PLANE_CURSOR][E2]
              |
              - WMV.cursor_trans[E1]
              + WMV.plane_trans[E1][PLANE_CURSOR]
              )
      
              @@
              struct skl_wm_values *WMV;
              expression E1, E2;
              @@
              (
              - WMV->cursor[E1][E2]
              + WMV->plane[E1][PLANE_CURSOR][E2]
              |
              - WMV->cursor_trans[E1]
              + WMV->plane_trans[E1][PLANE_CURSOR]
              )
      
              @@ @@
              struct skl_wm_values {
              ...
              - uint32_t cursor[I915_MAX_PIPES][8];
              ...
              - uint32_t cursor_trans[I915_MAX_PIPES];
              ...
              };
      
              @@ struct skl_wm_level WML; @@
              (
              - WML.cursor_en
              + WML.plane_en[PLANE_CURSOR]
              |
              - WML.cursor_res_b
              + WML.plane_res_b[PLANE_CURSOR]
              |
              - WML.cursor_res_l
              + WML.plane_res_l[PLANE_CURSOR]
              )
      
              @@ struct skl_wm_level *WML; @@
              (
              - WML->cursor_en
              + WML->plane_en[PLANE_CURSOR]
              |
              - WML->cursor_res_b
              + WML->plane_res_b[PLANE_CURSOR]
              |
              - WML->cursor_res_l
              + WML->plane_res_l[PLANE_CURSOR]
              )
      
              @@ @@
              struct skl_wm_level {
              ...
              - bool cursor_en;
              ...
              - uint16_t cursor_res_b;
              - uint8_t cursor_res_l;
              ...
              };
      
      v2: Use a PLANE_CURSOR enum entry rather than making the code reference
          I915_MAX_PLANES or I915_MAX_PLANES+1, which was confusing.  (Ander)
      Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      4969d33e
    • Matt Roper's avatar
      drm/i915: Determine I915_MAX_PLANES from plane enum · 31409e97
      Matt Roper authored
      Let the compiler figure out what I915_MAX_PLANES is from 'enum plane' so
      that we don't need a separate #define.
      
      While we're at it, add the cursor plane to the enum.  This will cause
      I915_MAX_PLANES to now include the cursor plane in its count (it didn't
      previously).   This change is safe since we currently only use this
      value in array declarations (never in the actual code logic); we just
      wind up allocating slightly more memory than we need to.  A followup
      patch will cause various parts of the code to start using the extra
      array element where appropriate.
      
      (This patch probably should have been squashed with the followup patch,
      but I couldn't figure out how to get Coccinelle to modify enum
      declarations...)
      Suggested-by: default avatarAnder Conselvan De Oliveira <conselvan2@gmail.com>
      Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      31409e97
    • Matt Roper's avatar
      drm/i915: Eliminate usage of pipe_wm_parameters from ILK-style WM (v2) · 7221fc33
      Matt Roper authored
      Just pull the info out of the CRTC state structure rather than staging
      it in an additional structure.
      
      Note that we use cstate->active rather than intel_crtc->active which may
      appear to be a change in behavior.  However since we're no longer trying
      to recalculate watermarks during the "pipe off" stage of a modeset,
      intel_crtc->active and cstate->active should always be identical when
      watermarks are calculated (at least for ILK-style platforms).
      
      v2: Clarify reasoning for cstate->active and add a WARN_ON to the code
          to assert that it really is always identical to intel_crtc->active
          as expected.
      Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      7221fc33