Commit Graph

122 Commits

Author SHA1 Message Date
Chris Wilson 8678fdaf39 drm/i915/fbc: Allow on unfenced surfaces, for recent gen
Only fbc1 is tied to using a fence. Later iterations of fbc are more
flexible and allow operation on unfenced frontbuffers.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: "Zanoni, Paulo R" <paulo.r.zanoni@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20160819155428.1670-3-chris@chris-wilson.co.uk
2016-08-19 17:13:29 +01:00
Chris Wilson 12ecf4b979 drm/i915/fbc: Don't set an illegal fence if unfenced
If the frontbuffer doesn't have an associated fence, it will have a
fence reg of -1. If we attempt to OR in this register into the FBC
control register we end up setting all control bits, oops!

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: "Zanoni, Paulo R" <paulo.r.zanoni@intel.com>
Reviwed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20160819155428.1670-2-chris@chris-wilson.co.uk
2016-08-19 16:59:36 +01:00
Chris Wilson 2efb813d53 drm/i915: Fallback to using unmappable memory for scanout
The existing ABI says that scanouts are pinned into the mappable region
so that legacy clients (e.g. old Xorg or plymouthd) can write directly
into the scanout through a GTT mapping. However if the surface does not
fit into the mappable region, we are better off just trying to fit it
anywhere and hoping for the best. (Any userspace that is capable of
using ginormous scanouts is also likely not to rely on pure GTT
updates.) With the partial vma fault support, we are no longer
restricted to only using scanouts that we can pin (though it is still
preferred for performance reasons and for powersaving features like
FBC).

v2: Skip fence pinning when not mappable.
v3: Add a comment to explain the possible ramifications of not being
    able to use fences for unmappable scanouts.
v4: Rebase to skip over some local patches
v5: Rebase to defer until after we have unmappable GTT fault support

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Deepak S <deepak.s@linux.intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20160818161718.27187-27-chris@chris-wilson.co.uk
2016-08-18 22:36:56 +01:00
Chris Wilson 49ef5294cd drm/i915: Move fence tracking from object to vma
In order to handle tiled partial GTT mmappings, we need to associate the
fence with an individual vma.

v2: A couple of silly drops replaced spotted by Joonas

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20160818161718.27187-21-chris@chris-wilson.co.uk
2016-08-18 22:36:50 +01:00
Chris Wilson 058d88c433 drm/i915: Track pinned VMA
Treat the VMA as the primary struct responsible for tracking bindings
into the GPU's VM. That is we want to treat the VMA returned after we
pin an object into the VM as the cookie we hold and eventually release
when unpinning. Doing so eliminates the ambiguity in pinning the object
and then searching for the relevant pin later.

v2: Joonas' stylistic nitpicks, a fun rebase.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1471254551-25805-27-git-send-email-chris@chris-wilson.co.uk
2016-08-15 11:01:13 +01:00
Dave Airlie fc93ff608b Merge tag 'drm-intel-next-2016-08-08' of git://anongit.freedesktop.org/drm-intel into drm-next
- refactor ddi buffer programming a bit (Ville)
- large-scale renaming to untangle naming in the gem code (Chris)
- rework vma/active tracking for accurately reaping idle mappings of shared
  objects (Chris)
- misc dp sst/mst probing corner case fixes (Ville)
- tons of cleanup&tunings all around in gem
- lockless (rcu-protected) request lookup, plus use it everywhere for
  non(b)locking waits (Chris)
- pipe crc debugfs fixes (Rodrigo)
- random fixes all over

* tag 'drm-intel-next-2016-08-08' of git://anongit.freedesktop.org/drm-intel: (222 commits)
  drm/i915: Update DRIVER_DATE to 20160808
  drm/i915: fix aliasing_ppgtt leak
  drm/i915: Update comment before i915_spin_request
  drm/i915: Use drm official vblank_no_hw_counter callback.
  drm/i915: Fix copy_to_user usage for pipe_crc
  Revert "drm/i915: Track active streams also for DP SST"
  drm/i915: fix WaInsertDummyPushConstPs
  drm/i915: Assert that the request hasn't been retired
  drm/i915: Repack fence tiling mode and stride into a single integer
  drm/i915: Document and reject invalid tiling modes
  drm/i915: Remove locking for get_tiling
  drm/i915: Remove pinned check from madvise ioctl
  drm/i915: Reduce locking inside swfinish ioctl
  drm/i915: Remove (struct_mutex) locking for busy-ioctl
  drm/i915: Remove (struct_mutex) locking for wait-ioctl
  drm/i915: Do a nonblocking wait first in pread/pwrite
  drm/i915: Remove unused no-shrinker-steal
  drm/i915: Tidy generation of the GTT mmap offset
  drm/i915/shrinker: Wait before acquiring struct_mutex under oom
  drm/i915: Simplify do_idling() (Ironlake vt-d w/a)
  ...
2016-08-15 16:53:57 +10:00
Ville Syrjälä 936e71e314 drm/i915: Use drm_plane_state.{src,dst,visible}
Replace the private drm_rects/flags in intel_plane_state
with the ones now living in drm_plane_state.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: http://patchwork.freedesktop.org/patch/msgid/1469549224-1860-5-git-send-email-ville.syrjala@linux.intel.com
2016-08-08 14:19:55 -04:00
Joonas Lahtinen 31ad61e4af drm: BIT(DRM_ROTATE_?) -> DRM_ROTATE_?
Only property creation uses the rotation as an index, so convert the
to figure the index when needed.

v2: Use the new defines to build the _MASK defines (Sean)

Cc: intel-gfx@lists.freedesktop.org
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Cc: malidp@foss.arm.com
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Sean Paul <seanpaul@chromium.org>
Acked-by: Liviu Dudau <Liviu.Dudau@arm.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (v1)
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: http://patchwork.freedesktop.org/patch/msgid/1469771405-17653-1-git-send-email-joonas.lahtinen@linux.intel.com
2016-08-08 14:17:56 -04:00
Chris Wilson 3e510a8e65 drm/i915: Repack fence tiling mode and stride into a single integer
In the previous commit, we moved the obj->tiling_mode out of a bitfield
and into its own integer so that we could safely use READ_ONCE(). Let us
now repair some of that damage by sharing the tiling_mode with its
companion, the fence stride.

v2: New magic

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1470388464-28458-18-git-send-email-chris@chris-wilson.co.uk
2016-08-05 10:54:43 +01:00
Chris Wilson 36dbc4d769 drm/i915/fbc: FBC causes display flicker when VT-d is enabled on Skylake
Erratum SKL075: Display Flicker May Occur When Both VT-d And FBC Are Enabled

"Display flickering may occur when both FBC (Frame Buffer Compression)
and VT - d (Intel® Virtualization Technology for Directed I/O) are enabled
and in use by the display controller."

Ville found the w/a name in the database:
WaFbcTurnOffFbcWhenHyperVisorIsUsed:skl,bxt and also dug out that it
affects Broxton.

v2: Log when the quirk is applied.
v3: Ensure i915.enable_fbc is false when !HAS_FBC()
v4: Fix function name after rebase
v5: Add Broxton to the workaround

Note for backporting to stable, we need to add
  #define mkwrite_device_info(ptr) \
	((struct intel_device_info *)INTEL_INFO(ptr))

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: stable@vger.kernel.org
Link: http://patchwork.freedesktop.org/patch/msgid/1470296633-20388-1-git-send-email-chris@chris-wilson.co.uk
2016-08-04 10:28:05 +01:00
Matthew Auld 4da456168f drm/i915: remove redundant fbc warnings
The fbc enabled/active sanity checks are already done in
__intel_fbc_disable so no need to do them again.

Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1467710914-15146-1-git-send-email-matthew.auld@intel.com
2016-08-02 14:25:46 +03:00
Daniel Vetter 62f90b38f3 drm/i915: Update missing kerneldoc
Not sure why so much slips through when 0day is catching these. Hopefully
the much faster sphinx toolchain helps in unlazying people.

Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1468612088-9721-10-git-send-email-daniel.vetter@ffwll.ch
2016-07-19 10:34:24 +02:00
Chris Wilson 91c8a326a1 drm/i915: Convert dev_priv->dev backpointers to dev_priv->drm
Since drm_i915_private is now a subclass of drm_device we do not need to
chase the drm_i915_private->dev backpointer and can instead simply
access drm_i915_private->drm directly.

   text	   data	    bss	    dec	    hex	filename
1068757	   4565	    416	1073738	 10624a	drivers/gpu/drm/i915/i915.ko
1066949	   4565	    416	1071930	 105b3a	drivers/gpu/drm/i915/i915.ko

Created by the coccinelle script:
@@
struct drm_i915_private *d;
identifier i;
@@
(
- d->dev->i
+ d->drm.i
|
- d->dev
+ &d->drm
)

and for good measure the dev_priv->dev backpointer was removed entirely.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1467711623-2905-4-git-send-email-chris@chris-wilson.co.uk
2016-07-05 11:58:45 +01:00
Chris Wilson fac5e23e3c drm/i915: Mass convert dev->dev_private to to_i915(dev)
Since we now subclass struct drm_device, we can save pointer dances by
noting the equivalence of struct drm_device and struct drm_i915_private,
i.e. by using to_i915().

   text    data     bss     dec     hex filename
1073824    4562     416 1078802  107612 drivers/gpu/drm/i915/i915.ko
1068976    4562     416 1073954  106322 drivers/gpu/drm/i915/i915.ko

Created by the coccinelle script:

@@
expression E;
identifier p;
@@
- struct drm_i915_private *p = E->dev_private;
+ struct drm_i915_private *p = to_i915(E);

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Dave Gordon <david.s.gordon@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1467628477-25379-1-git-send-email-chris@chris-wilson.co.uk
2016-07-04 12:54:07 +01:00
Chris Wilson 8d90dfd5ce drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()
By using the out-of-line intel_wait_for_register() not only do we can
efficiency from using the hybrid wait_for() contained within, but we
avoid code bloat from the numerous inlined loops, in total (all patches):

   text    data     bss     dec     hex filename
1078551    4557     416 1083524  108884 drivers/gpu/drm/i915/i915.ko
1070775    4557     416 1075748  106a24 drivers/gpu/drm/i915/i915.ko

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1467297225-21379-38-git-send-email-chris@chris-wilson.co.uk
2016-06-30 15:42:21 +01:00
Paulo Zanoni 80788a0fbb drm/i915/fbc: sanitize i915.enable_fbc during FBC init
The DDX driver changes its behavior depending on the value it reads
from i915.enable_fbc, so sanitize the value in order to allow it to
know what's going on. It uses this in order to choose the defaults for
the TearFree option. Before this patch, it would read -1 and always
assume that FBC was disabled, so it wouldn't force TearFree.

v2: Extract intel_sanitize_fbc_option() (Chris).
v3: Rebase.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1460574069-14005-1-git-send-email-paulo.r.zanoni@intel.com
2016-06-20 17:47:30 -03:00
Paulo Zanoni ab28a5474a drm/i915/fbc: update busy_bits even for GTT and flip flushes
We ignore ORIGIN_GTT because the hardware tracking can recognize GTT
writes and take care of them. We also ignore ORIGIN_FLIP because we
deal with flips without relying on the frontbuffer tracking
infrastructure. On the other hand, a flush is a flush and means we're
good to go, so we need to update busy_bits in order to reflect that,
even if we're not going to do anything else about it.

How to reproduce the bug fixed by this patch:
 - boot SKL up to the desktop environment
 - stop the display manager
 - run any of the igt/kms_frontbuffer_tracking/*fbc*onoff* subtests
 - the tests will fail

The steps above will create the right conditions for us to lose track
of busy_bits. If you, for example, run the full set of FBC tests, the
onoff subtests will succeed.

Also notice that the "bug" is that we'll just keep FBC disabled on
cases where it could be enabled, so it's not something the users can
perceive, it just affects power consumption numbers on properly
configured machines.

Testcase: igt/kms_frontbuffer_tracking/*fbc*onoff* (see above)
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1459804638-3588-2-git-send-email-paulo.r.zanoni@intel.com
2016-06-20 17:47:26 -03:00
Lyude c7f7e2feff drm/i915/fbc: Disable on HSW by default for now
>From https://bugs.freedesktop.org/show_bug.cgi?id=96461 :

This was kind of a difficult bug to track down. If you're using a
Haswell system running GNOME and you have fbc completely enabled and
working, playing videos can result in video artifacts. Steps to
reproduce:

- Run GNOME
- Ensure FBC is enabled and active
- Download a movie, I used the ogg version of Big Buck Bunny for this
- Run `gst-launch-1.0 filesrc location='some_movie.ogg' ! decodebin !
  glimagesink` in a terminal
- Watch for about over a minute, you'll see small horizontal lines go
  down the screen.

For the time being, disable FBC for Haswell by default.

Stefan Richter reported kernel freezes (no video artifacts) when fbc
is on.  (E3-1245 v3 with HD P4600; openbox and some KDE and LXDE
applications, thread begins at https://lkml.org/lkml/2016/4/26/813).
We also got reports from Steven Honeyman on openbox+roxterm.

v2 (From Paulo):
  - Add extra information to the commit message
  - Add Fixes tag
  - Rebase

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96461
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96464
Fixes: a98ee79317 ("drm/i915/fbc: enable FBC by default on HSW and BDW")
Cc: stable@vger.kernel.org
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Lyude <cpaul@redhat.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1465487895-7401-1-git-send-email-cpaul@redhat.com
2016-06-20 17:47:22 -03:00
Maarten Lankhorst faf68d9256 Reapply "drm/i915: Pass atomic states to fbc update, functions."
The patch was reverted as part of the original nonblocking commit
support, but is required for any kind of nonblocking commit.

This is required to let fbc updates run async. It has a lot of
checks whether certain locks are taken, which can be removed when
the relevant states are passed in as pointers.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1463490484-19540-17-git-send-email-maarten.lankhorst@linux.intel.com
Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/28208c38-8738-abdf-0cce-8d8f266b9c28@linux.intel.com
2016-06-16 14:28:35 +02:00
Daniel Vetter 2e7a5701c9 drm/doc: Appease sphinx
Mostly this is unexpected indents. But really it's just a
demonstration for my patch, all these issues have been found&fixed
using the correct source file and line number support I just added.
All line numbers have been perfectly accurate.

One issue looked a bit fishy in intel_lrc.c, where I don't quite grok
what sphinx is unhappy about. But since that file looks like it has
never seen a proper kernel-doc parser I figured better to fix in a
separate path.

v2: Use fancy new &drm_device->struct_mutex linking (Jani).

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: linux-doc@vger.kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
2016-06-02 16:25:20 +02:00
Daniel Vetter e42aeef123 drm/i915: Revert async unpin and nonblocking atomic commit
This reverts the following patches:

d55dbd06bb drm/i915: Allow nonblocking update of pageflips.
15c86bdb76 drm/i915: Check for unpin correctness.
95c2ccdc82 Reapply "drm/i915: Avoid stalling on pending flips for legacy cursor updates"
a6747b7304 drm/i915: Make unpin async.
03f476e1fc drm/i915: Prepare connectors for nonblocking checks.
2099deffef drm/i915: Pass atomic states to fbc update functions.
ee7171af72 drm/i915: Remove reset_counter from intel_crtc.
2ee004f7c5 drm/i915: Remove queue_flip pointer.
b8d2afae55 drm/i915: Remove use_mmio_flip kernel parameter.
8dd634d922 drm/i915: Remove cs based page flip support.
143f73b3bf drm/i915: Rework intel_crtc_page_flip to be almost atomic, v3.
84fc494b64 drm/i915: Add the exclusive fence to plane_state.
6885843ae1 drm/i915: Convert flip_work to a list.
aa420ddd8e drm/i915: Allow mmio updates on all platforms, v2.
afee4d8707 Revert "drm/i915: Avoid stalling on pending flips for legacy cursor updates"

"drm/i915: Allow nonblocking update of pageflips" should have been
split up, misses a proper commit message and seems to cause issues in
the legacy page_flip path as demonstrated by kms_flip.

"drm/i915: Make unpin async" doesn't handle the unthrottled cursor
updates correctly, leading to an apparent pin count leak. This is
caught by the WARN_ON in i915_gem_object_do_pin which screams if we
have more than DRM_I915_GEM_OBJECT_MAX_PIN_COUNT pins.

Unfortuantely we can't just revert these two because this patch series
came with a built-in bisect breakage in the form of temporarily
removing the unthrottled cursor update hack for legacy cursor ioctl.
Therefore there's no other option than to revert the entire pile :(

There's one tiny conflict in intel_drv.h due to other patches, nothing
serious.

Normally I'd wait a bit longer with doing a maintainer revert, but
since the minimal set of patches we need to revert (due to the bisect
breakage) is so big, time is running out fast. And very soon
(especially after a few attempts at fixing issues) it'll be really
hard to revert things cleanly.

Lessons learned:
- Not a good idea to rush the review (done by someone fairly new to
  the area) and not make sure domain experts had a chance to read it.

- Patches should be properly split up. I only looked at the two
  patches that should be reverted in detail, but both look like the
  mix up different things in one patch.

- Patches really should have proper commit messages. Especially when
  doing more than one thing, and especially when touching critical and
  tricky core code.

- Building a patch series and r-b stamping it when it has a built-in
  bisect breakage is not a good idea.

- I also think we need to stop building up technical debt by
  postponing atomic igt testcases even longer. I think it's clear that
  there's enough corner cases in this beast that we really need to
  have the testcases _before_ the next step lands.

(cherry picked from commit 5a21b6650a
from drm-intel-next-queeud)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Dave Airlie <airlied@redhat.com>
Acked-by: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
2016-06-01 09:46:46 +02:00
Maarten Lankhorst 2099deffef drm/i915: Pass atomic states to fbc update functions.
This is required to let fbc updates run async. It has a lot of
checks whether certain locks are taken, which can be removed when
the relevant states are passed in as pointers.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1463490484-19540-17-git-send-email-maarten.lankhorst@linux.intel.com
Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
2016-05-19 14:38:52 +02:00
Tvrtko Ursulin ac657f6461 drm/i915: Introduce IS_GEN macro
To be used for more efficient Gen range checking.

v2: Remove spurious chunk. (Chris Wilson)
v3: Rebase.
v4: Renamed from INTEL_GEN_RANGE and added GEN_FOREVER.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v3)
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1462874228-6601-1-git-send-email-tvrtko.ursulin@linux.intel.com
2016-05-11 12:27:28 +01:00
Chris Wilson c033666a94 drm/i915: Store a i915 backpointer from engine, and use it
text	   data	    bss	    dec	    hex	filename
6309351	3578714	 696320	10584385	 a18141	vmlinux
6308391	3578714	 696320	10583425	 a17d81	vmlinux

Almost 1KiB of code reduction.

v2: More s/INTEL_INFO()->gen/INTEL_GEN()/ and IS_GENx() conversions

   text	   data	    bss	    dec	    hex	filename
6304579	3578778	 696320	10579677	 a16edd	vmlinux
6303427	3578778	 696320	10578525	 a16a5d	vmlinux

Now over 1KiB!

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1462545621-30125-3-git-send-email-chris@chris-wilson.co.uk
2016-05-09 13:41:24 +01:00
Joonas Lahtinen 72e96d6450 drm/i915: Refer to GGTT {,VM} consistently
Refer to the GGTT VM consistently as "ggtt->base" instead of just "ggtt",
"vm" or indirectly through other variables like "dev_priv->ggtt.base"
to avoid confusion with the i915_ggtt object itself and PPGTT VMs.

Refer to the GGTT as "ggtt" instead of indirectly through chaining.

As a bonus gets rid of the long-standing i915_obj_to_ggtt vs.
i915_gem_obj_to_ggtt conflict, due to removal of i915_obj_to_ggtt!

v2:
- Added some more after grepping sources with Chris

v3:
- Refer to GGTT VM through ggtt->base consistently instead of ggtt_vm
  (Chris)

v4:
- Convert all dev_priv->ggtt->foo accesses to ggtt->foo.

v5:
- Make patch checker happy

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
2016-03-31 17:55:43 +03:00
Joonas Lahtinen 62106b4f6b drm/i915: Rename dev_priv->gtt to dev_priv->ggtt
Refer to Global GTT consistently as GGTT, thus rename dev_priv->gtt
to dev_priv->ggtt and struct i915_gtt to struct i915_ggtt.

Fix a couple of whitespace problems while at it.

v2:
- Fix a typo in commit message.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
2016-03-18 15:18:15 +02:00
Paulo Zanoni a98ee79317 drm/i915/fbc: enable FBC by default on HSW and BDW
These platforms should be fine now.

FBC can allow very significant power savings for screen-on idle
systems, but it is worth mentioning that a lot of people won't get
significant power savings by enabling this feature because they may
have something else preventing the system from getting into the
deepest sleep states. Examples may include a hungry wifi device or a
max_performance SATA link power management policy. You can check your
PC state residencies on the powertop "Idle stats" tab. I recommend
trying to run "sudo powertop --auto-tune" and then seeing if the
residencies improve.

Oh, and in case you - the person reading this commit message - found
this commit through git bisect, please do the following:
 - Check your dmesg and see if there are error messages mentioning
   underruns around the time your problem started happening.
 - Download intel-gpu-tools, compile it, and run:
   $ sudo ./tests/kms_frontbuffer_tracking --run-subtest '*fbc-*' 2>&1 | tee fbc.txt
   Then send us the fbc.txt file, especially if you get a failure.
   This will really maximize your chances of getting the bug fixed
   quickly.
 - Try to find a reliable way to reproduce the problem, and tell us.
 - Boot with drm.debug=0xe, reproduce the problem, then send us the
   dmesg file.

v2: Don't enable by default on SKL.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1455655643-2535-1-git-send-email-paulo.r.zanoni@intel.com
2016-02-19 18:06:16 -02:00
Paulo Zanoni 5375ce9f38 drm/i915/fbc: set fbc->active from the new activation functions
Now that we have top-level gen-independent hw_activate and
hw_deactivate functions, set fbc->active directly from them, removing
the duplicated code.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454101060-23198-3-git-send-email-paulo.r.zanoni@intel.com
2016-02-04 14:18:22 -02:00
Paulo Zanoni 8c40074cb2 drm/i915/fbc: unexport the HW level activation functions
The recent introduction of a new caller of dev_priv->fbc.deactivate()
is a good example of why we need unexport those functions. Anything
outside intel_fbc.c should only call the functions exported by
intel_fbc.c, so in order to enforce that, kill the function pointers
stored inside dev_priv->fbc and replace them with functions that can't
be called from outside intel_fbc.c.

This should make it much harder for new code to call these functions
from outside intel_fbc.c.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454101060-23198-2-git-send-email-paulo.r.zanoni@intel.com
2016-02-04 14:17:45 -02:00
Paulo Zanoni e35be23f31 drm/i915/fbc: refactor some small functions called only once
The FBC fixes we've been doing in the last months required a lot of
refactor, so functions that were once big and called from different
spots are now small and called only once. IMHO now it's better to just
move the contents of these functions to their only callers since this
reduces the number of indirections while reading the code.

While at it, also improve the related comments a little bit.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-26-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:21:10 -02:00
Paulo Zanoni 9b42281f9d drm/i915/fbc: don't store/check a pointer to the FB
We already make sure we run intel_fbc_update_update during modesets
and page flips, and this function takes care of deactivating FBC, so
it shouldn't be possible for us to reach the condition we check at
intel_fbc_work_fn. So instead of grabbing framebuffer references and
adding a lot of code to track when we need to free them, just don't
track anything at all since we shouldn't need to.

v2: Rebase.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-25-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:20:39 -02:00
Paulo Zanoni b20d27526c drm/i915/fbc: don't store the fb_id on reg_params
We don't actually use fb_id anywhere. We already compare all
parameters that matter to the hardware: pixel format, stride,
fence_reg and ggtt_offset. The ID shouldn't make a difference.

Besides, we already update the FBC data at every modeset/flip, so this
can't change behind our backs.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-23-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:18:28 -02:00
Paulo Zanoni 913a3a6aca drm/i915/fbc: don't print no_fbc_reason to dmesg
Our dmesg messages started being misleading after we converted to the
enable+activate model: we always print "Disabling FBC", even when
we're just deactivating it. So, for example, when I boot my machine
and do "dmesg | grep -i fbc", I see:
  [drm:intel_fbc_enable] Enabling FBC on pipe A
  [drm:set_no_fbc_reason] Disabling FBC: framebuffer not tiled or fenced
but then, if I read the debugfs file, I will see:
  $ sudo cat i915_fbc_status
  FBC enabled
  Compressing: yes
so we can conclude that dmesg is misleading, since FBC is actually
enabled. What happened is that we deactivated FBC due to fbcon not
being tiled, but when we silently reactivated it when the display
manager started. We don't print activation messages since there may be
way too many of these operations per second during normal desktop
usage.

One possible solution would be to change set_no_fbc_reason to
correctly differentiate between disable and deactivation, but we
removed support from printing activation/deactivation messages in the
past because they were too frequent. So instead of doing this, let's
just not print anything on dmesg, and leave the debugfs file if the
user needs to investigate something. We already print when we enable
and disable FBC anyway on a given pipe, so this should already help
triaging bugs.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-22-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:18:16 -02:00
Paulo Zanoni 5bc40472de drm/i915/fbc: don't try to deactivate FBC if it's not enabled
During FBC invalidation, don't call intel_fbc_deactivate if it's not
enabled. This doesn't fix any bug, but helps making the interface
saner.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-21-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:17:56 -02:00
Paulo Zanoni 49227c4ae3 drm/i915/fbc: make FBC work with fastboot
Move intel_fbc_enable to a place where it is called regardless of the
"modeset" variable, and make sure intel_fbc_enable can be called
multiple times without intel_fbc_disable being called.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-20-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:17:36 -02:00
Paulo Zanoni 58f9c0bc55 drm/i915/fbc: move intel_fbc_{enable, disable} call one level up
Instead of duplicating the calls for every platform, let's just put
them in the correct places inside intel_atomic_commit. This will also
make it easier for us to move the enable call in order to support
fasbtoot.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-19-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:17:19 -02:00
Paulo Zanoni f51be2e0e3 drm/i915/fbc: choose the new FBC CRTC during atomic check
This opens the possibility of implementing nicer schemes to choose the
CRTC, such as checking the amount of stolen memory available, or
choosing the best pipe on platforms that don't die FBC to pipe or
plane A.

This code was written for another refactor that I ended up discarding,
so I don't actually need it, but I figured this patch would be an
improvement on its own so I kept it on the series.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-18-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:16:45 -02:00
Paulo Zanoni 010cf73d46 drm/i915/fbc: rewrite the multiple_pipes_ok() code for locking
Older FBC platforms have this restriction where FBC can't be enabled
if multiple pipes are enabled. In the current code, we disable FBC
before the second pipe becomes visible.

One of the problems with this code is that the current
multiple_pipes_ok() implementation just iterates through all CRTCs
looking at their states, but it doesn't make sure that the state
locks are grabbed. It also can't just grab the locks for every CRTC
since this would kill one of the biggest advantages of atomic
modesetting.

After the recent FBC changes, we now have the appropriate locks for
the given CRTC, so we can just try to maintain the state of each CRTC
and update it once intel_fbc_pre_update is called.

As a last note, I don't have gen 2/3 machines to test this code. My
current plan is to enable FBC on just the newer platforms, so this
patch is just an attempt to get the gen 2/3 code at least looking
sane, so if one day someone decide to fix FBC on these platforms, they
may have less work to do.

Not-tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (only on HSW+)
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-16-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:15:56 -02:00
Paulo Zanoni 65c7600f07 drm/i915/fbc: make sure we cancel the work function at fbc_disable
Just to be sure nothing will survive a module unload. We need to do
this after the unlock in order to make sure the function won't get
stuck trying to grab the lock we already own while we wait for it to
finish.

Reported-by: Reported-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-15-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:15:43 -02:00
Paulo Zanoni c937ab3e58 drm/i915/fbc: rename the FBC disable functions
Instead of:
 - intel_fbc_disable_crtc(crtc)
 - intel_fbc_disable(dev_priv)
we now have:
 - intel_fbc_disable(crtc)
 - intel_fbc_global_disable(dev_priv)

This is because all the other functions that take a CRTC are called
 - intel_fbc_something(crtc)
Instead of:
 - intel_fbc_something_crtc(crtc)

And I also hope that the word "global" is going to help make it more
explicit that "global" is the unusual case, not the opposite.

Reported-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-14-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:13:33 -02:00
Paulo Zanoni 60eb2cc71c drm/i915/fbc: unexport intel_fbc_deactivate
With the addition and usage of intel_fbc_pre_update,
intel_fbc_deactivate is not used anymore outside intel_fbc.c, so kill
the exported function and rename __intel_fbc_deactivate.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-13-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:12:49 -02:00
Paulo Zanoni 1eb52238a5 drm/i915/fbc: fix the FBC state checking code
We'll now call intel_fbc_pre_update instead of intel_fbc_deactivate
during atomic commits. This will continue to guarantee that we
deactivate FBC and it will also update the state checking structures
at the correct time. Then, later, at the point where we were calling
intel_fbc_update, we'll only need to call intel_fbc_post_update.

Also add the proper warnings in case we don't have the appropriate
locks. Daniel mentioned the warnings will have to be removed for async
commits, but let's keep them here while we can.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-12-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:12:07 -02:00
Paulo Zanoni 212890cfcd drm/i915/fbc: split intel_fbc_update into pre and post update
So now pre_update will be responsible for unconditionally deactivating
FBC and updating the state cache, while post_update will be
responsible for checking if it can be enabled, then enabling it.

This is one more step into proper locking.

Notice that intel_fbc_flush now calls post_update directly. The FBC
flush can only happen for drawing operations - since we explicitly
ignore the flips -, so the FBC state is not expected to have changed
at this point. With this we can just run post_update, which will make
sure we won't deactivate+reactivate FBC as would be the case now if we
called pre_update + post_update.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-11-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:11:48 -02:00
Paulo Zanoni aaf78d276b drm/i915/fbc: introduce struct intel_fbc_state_cache
Per the new atomic locking rules, we need to cache the CRTC, plane and
FB state structures we use so we can access them later without needing
more locks. So do this.

Notice that there are some pieces of the FBC code that look at things
that are only computed during the modeset, so we can't just can't
precompute whether FBC can be activated during the update_state_cache
stage. We may be able to do this later.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-10-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:11:09 -02:00
Paulo Zanoni 261fe99ac2 drm/i915/fbc: don't flush for operations on the wrong frontbuffer
If frontbuffer_bits doesn't match the current frontbuffer, there's no
reason to recompress or update FBC.

There was a plan to make the FBC test suite catch this type of
problem, but it never got implemented due to being low priority.

While at it, also implement Ville's suggestion and use
plane->frontbuffer_bit instead of INTEL_FRONTBUFFER_PRIMARY.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-8-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:10:38 -02:00
Paulo Zanoni 0dd81544c9 drm/i915/fbc: don't use the frontbuffer tracking subsystem for flips
Before this patch, page flips would call intel_frontbuffer_flip() and
intel_frontbuffer_flip_complete(), which would call intel_fbc_flush(),
which would call intel_fbc_update(). The problem is that drawing
operations also trigger intel_fbc_flush() calls, so it's not
guaranteed that we have the CRTC and FB locks grabbed when
intel_fbc_flush() happens, since the call trace may come from the
rendering path.

We're trying to make the FBC code grab the appropriate CRTC/FB locks,
so split the drawing and the flipping logic in order to achieve that
in later patches. So now the frontbuffer tracking code is just going
to be used for frontbuffer drawing, and intel_fbc_update() is going to
be used directly for actual page flips.

As a note, we don't need to call intel_fbc_flip() during the two
places where we call intel_frontbuffer_flip() since in one of them we
already have an intel_fbc_update() call, and in the other we have the
planes disabled.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-7-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:10:20 -02:00
Paulo Zanoni ab34a7e8b5 drm/i915/fbc: replace frequent dev_priv->fbc.x with fbc->x
We say "dev_priv->fbc.something" way too many times in our code while
we could be saying just "fbc->something" with a previous declaration
of fbc. This has been bothering me for a while but I didn't want to
patch it since I wanted to fix the real problems first. But as I add
more code I keep thinking about it, especially since it makes the code
easier to read and it can make us fit 80 columns easier, so let's just
do the change now.

While at it, also rename from i915_fbc to intel_fbc because the whole
FBC code uses intel_fbc.

v2: Rebase after the work_fn changes.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453406763-10400-1-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:09:37 -02:00
Paulo Zanoni b183b3f143 drm/i915/fbc: introduce struct intel_fbc_reg_params
The early return inside __intel_fbc_update does not completely check
all the parameters that affect the FBC register values. For example,
we currently lack looking at crtc->adjusted_y (for the fence Y offset)
and all the parameters that affect the CFB size (for i8xx).

Instead of just adding the missing parameters to the check and hoping
that any changes to the fbc_activate functions also come with a
matching change to the __intel_fbc_update check, introduce a new
structure where we store these parameters and use the structure at the
fbc_activate function. Of course, it's still possible to access
everything from dev_priv in those functions, but IMHO the new code
will be harder to break.

v2: Rebase.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-5-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:08:38 -02:00
Paulo Zanoni 44a8a25708 drm/i915/fbc: extract intel_fbc_can_enable()
Make our enable/activate checking model more explicit, especially
since we now have intel_fbc_can_activate().

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-4-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:08:25 -02:00
Paulo Zanoni 615b40d7e4 drm/i915/fbc: extract intel_fbc_can_activate()
Extract all the code that checks if the FBC configuration is valid to
its own function, making __intel_fbc_update() much simpler.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-3-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:07:38 -02:00