Don't even tell the mm allocator to handle the first page of stolen on
the affected platforms. This means that we won't inherit the FB in
case the BIOS decides to put it at the start of stolen. But the BIOS
should not be putting it at the start of stolen since it's going to
get corrupted. I suppose the bug here is that some pixels at the very
top of the screen will be corrupted, so it's not exactly easy to
notice.
We have confirmation that the first page of stolen does actually get
corrupted, so I really think we should do this in order to avoid any
possible future headaches, even if that means losing BIOS framebuffer
inheritance. Let's not use the HW in a way it's not supposed to be
used.
Notice that now ggtt->stolen_usable_size won't reflect the ending
address of the stolen usable range anymore, so we have to fix the
places that rely on this. To simplify, we'll just use U64_MAX.
v2: don't even put the first page on the mm (Chris)
v3: drm_mm_init() takes size instead of end as argument (Ville)
v4: add a comment explaining the reserved ranges (Chris)
use 0 for start and U64_MAX for end when possible (Chris)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1481808235-27607-1-git-send-email-paulo.r.zanoni@intel.com
It only has two checks now, so it makes sense to just move the code to
the caller.
Also take this opportunity to make no_fbc_reason make more sense: now
we'll only list "no suitable CRTC for FBC" instead of maybe giving a
reason why the last CRTC we checked was not selected, and we'll more
consistently set the reason (e.g., if no primary planes are visible).
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1478883461-20201-5-git-send-email-paulo.r.zanoni@intel.com
Backmerge because Chris Wilson needs the very latest&greates of
Gustavo Padovan's sync_file work, specifically the refcounting changes
from:
commit 30cd85dd6e
Author: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Date: Wed Oct 19 15:48:32 2016 -0200
dma-buf/sync_file: hold reference to fence when creating sync_file
Also good to sync in general since git tends to get confused with the
cherry-picking going on.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
- first slice of the gvt device model (Zhenyu et al)
- compression support for gpu error states (Chris)
- sunset clause on gpu errors resulting in dmesg noise telling users
how to report them
- .rodata diet from Tvrtko
- switch over lots of macros to only take dev_priv (Tvrtko)
- underrun suppression for dp link training (Ville)
- lspcon (hmdi 2.0 on skl/bxt) support from Shashank Sharma, polish
from Jani
- gen9 wm fixes from Paulo&Lyude
- updated ddi programming for kbl (Rodrigo)
- respect alternate aux/ddc pins (from vbt) for all ddi ports (Ville)
* tag 'drm-intel-next-2016-10-24' of git://anongit.freedesktop.org/drm-intel: (227 commits)
drm/i915: Update DRIVER_DATE to 20161024
drm/i915: Stop setting SNB min-freq-table 0 on powersave setup
drm/i915/dp: add lane_count check in intel_dp_check_link_status
drm/i915: Fix whitespace issues
drm/i915: Clean up DDI DDC/AUX CH sanitation
drm/i915: Respect alternate_ddc_pin for all DDI ports
drm/i915: Respect alternate_aux_channel for all DDI ports
drm/i915/gen9: Remove WaEnableYV12BugFixInHalfSliceChicken7
drm/i915: KBL - Recommended buffer translation programming for DisplayPort
drm/i915: Move down skl/kbl ddi iboost and n_edp_entires fixup
drm/i915: Add a sunset clause to GPU hang logging
drm/i915: Stop reporting error details in dmesg as well as the error-state
drm/i915/gvt: do not ignore return value of create_scratch_page
drm/i915/gvt: fix spare warnings on odd constant _Bool cast
drm/i915/gvt: mark symbols static where possible
drm/i915/gvt: fix sparse warnings on different address spaces
drm/i915/gvt: properly access enabled intel_engine_cs
drm/i915/gvt: Remove defunct vmap_batch()
drm/i915/gvt: Use common mapping routines for shadow_bb object
drm/i915/gvt: Use common mapping routines for indirect_ctx object
...
Broadwell and newer actually compress up to 2560 lines instead of 2048
(as documented in the FBC_CTL page). If we don't take this into
consideration we end up reserving too little stolen memory for the
CFB, so we may allocate something else (such as a ring) right after
what we reserved, and the hardware will overwrite it with the contents
of the CFB when FBC is active, causing GPU hangs. Another possibility
is that the CFB may be allocated at the very end of the available
space, so the CFB will overlap the reserved stolen area, leading to
FIFO underruns.
This bug has always been a problem on BDW (the only affected platform
where FBC is enabled by default), but it's much easier to reproduce
since the following commit:
commit c58b735fc7
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Thu Aug 18 17:16:57 2016 +0100
drm/i915: Allocate rings from stolen
Of course, you can only reproduce the bug if your screen is taller
than 2048 lines.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98213
Fixes: a98ee79317 ("drm/i915/fbc: enable FBC by default on HSW and BDW")
Cc: <stable@vger.kernel.org> # v4.6+
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1477065346-13736-1-git-send-email-paulo.r.zanoni@intel.com
We have intel_rotation_90_or_270() in i915 to check if the rotation is
90 or 270 degrees. Similar checks are elsewhere in drm, so let's move
the helper into a central place and use it everwhere.
v2: Drop the BIT()
Convert all new intel_rotation_90_or_270() calls
Deal with superfluous code shuffling
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1474907460-10717-2-git-send-email-ville.syrjala@linux.intel.com
Ever since I started working on FBC I was already aware that FBC can
really amplify the FIFO underrun symptoms. On systems where FIFO
underruns were harmless error messages, enabling FBC would cause the
underruns to give black screens.
We recently tried to enable FBC on Haswell and got reports of a system
that would hang after some hours of uptime, and the first bad commit
was the one that enabled FBC. We also observed that this system had
FIFO underrun error messages on its dmesg. Although we don't have any
evidence that fixing the underruns would solve the bug and make FBC
work properly on this machine, IMHO it's better if we minimize the
amount of possible problems by just giving up FBC whenever we detect
an underrun.
v2: New version, different implementation and commit message.
v3: Clarify the fact that we run from an IRQ handler (Chris).
v4: Also add the underrun_detected check at can_choose() to avoid
misleading dmesg messages (DK).
v5: Fix Engrish, use READ_ONCE on the unlocked read (Chris).
Cc: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: Lyude <cpaul@redhat.com>
Cc: stevenhoneyman@gmail.com <stevenhoneyman@gmail.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1473773937-19758-1-git-send-email-paulo.r.zanoni@intel.com
This reverts commit 8678fdaf39 ("drm/i915/fbc: Allow on unfenced surfaces,
for recent gen") as Skylake has issues with unfenced FBC tracking (and
yes Skylake doesn't even enable FBC yet). Paulo would like to do a full
review of all existing workarounds to see if any more are missing prior
to allowing FBC on unfenced surfaces. In the meantime lets hope that all
framebuffers are idle and naturally fit within the mappable aperture.
Requested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Fixes: 8678fdaf39 ("drm/i915/fbc: Allow on unfenced surfaces...");
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20160824180053.24239-1-chris@chris-wilson.co.uk
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
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
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
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
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
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
- 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)
...
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
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
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
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
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
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
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
>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
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>
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>
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
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
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>
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>
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
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
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
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
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
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
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
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
