Commit graph

10480 commits

Author SHA1 Message Date
Jani Nikula
23a5110dc6 drm/i915/dp: abstract training pattern selection
Make it cleaner to add more checks in the function. No functional
changes.

Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Cc: drm-intel-fixes@lists.freedesktop.org # dependency on the next patch
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454667370-8001-1-git-send-email-jani.nikula@intel.com
2016-02-05 14:38:56 +02:00
Jani Nikula
96afef1d5a drm/i915/dsi: skip gpio element execution when not supported
Skip v3 gpio element because the support is not there, and skip gpio
element on non-vlv because the sideband code is vlv specific.

v2: the gpio stuff is currently only supported on vlv (Ville)

Cc: drm-intel-fixes@lists.freedesktop.org
Fixes: 2a33d93486 ("drm/i915/bios: add support for MIPI sequence block v3")
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454604767-2440-1-git-send-email-jani.nikula@intel.com
2016-02-04 20:34:06 +02:00
Deepak M
dfb19ed20c drm/i915: Extend gpio read/write to other cores
Make the gpio read/write functions more generic iosf sideband read/write
functions, taking the iosf port as argument.

v2: rebase
v3: rebase
v4 by Jani: address Ville's review
v5 by Jani: drop the PCI_DEVFN change (Ville)

Signed-off-by: Deepak M <m.deepak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454604915-17142-1-git-send-email-jani.nikula@intel.com
2016-02-04 19:11:17 +02:00
Jani Nikula
10182e77f5 drm/i915/vlv: drop unused vlv_gps_core_read/write functions
Not needed.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/532150999335216b1374c606e1b3c253a6c9fe9d.1454582914.git.jani.nikula@intel.com
2016-02-04 18:34:08 +02:00
Jani Nikula
4688d45f97 drm/i915: put the IOSF port defines in numerical order
Make it easier to spot duplicates.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/a89f10d2d1954ac1b9a278534cb5209973354caa.1454582914.git.jani.nikula@intel.com
2016-02-04 18:33:44 +02:00
Jani Nikula
4e1c63e376 drm/i915/dsi: don't pass arbitrary data to sideband
Since sequence block v2 the second byte contains flags other than just
pull up/down. Don't pass arbitrary data to the sideband interface.

The rest may or may not work for sequence block v2, but there should be
no harm done.

Cc: stable@vger.kernel.org
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/ebe3c2eee623afc4b3a134533b01f8d591d13f32.1454582914.git.jani.nikula@intel.com
2016-02-04 18:32:57 +02:00
Jani Nikula
5d2d0a12d3 drm/i915/dsi: defend gpio table against out of bounds access
Do not blindly trust the VBT data used for indexing.

Cc: stable@vger.kernel.org
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/cc32d40c2b47f2d2151811855ac2c3dabab1d57d.1454582914.git.jani.nikula@intel.com
2016-02-04 18:30:25 +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
3eb7c38bfe drm/i915: don't deactivate FBC at skylake_disable_primary_plane
FBC is already deactivated at this point.

Besides, nothing should be calling these lower-level function
pointers. A few months ago, the only caller of
dev_priv->fbc.deactivate was intel_pipe_set_base_atomic(), which was
the kgdboc function. But the following commit added it to the SKL
function:

    commit a8d201af68
    Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
    Date:   Thu Jan 7 11:54:11 2016 +0100
        drm/i915: Use plane state for primary plane updates.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
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-1-git-send-email-paulo.r.zanoni@intel.com
2016-02-04 14:17:09 -02:00
Tim Gore
d5165ebd52 drm/i915: implement WaIncreaseDefaultTLBEntries
WaIncreaseDefaultTLBEntries increases the number of TLB
entries available for GPGPU workloads and gives significant
( > 10% ) performance gain for some OCL benchmarks.
Put this in a new function that can be a place for
workarounds that are GT related but not required per ring.
This function is called on driver load and also after a
reset and on resume, so it is safe for workarounds that get
clobbered in these situations. This function currently has
just this one workaround.

v2: This was originally split into 3 patches but following
  review feedback was squashed into 1.
  I have not incorporated some style comments from Chris
  Wilson as I felt that after defining and intialising a
  temporary variable and then adding an additional if block
  to only write the register if the temporary variable had
  been set, this didn't really give a net gain.

v3: Resending in the hope that BAT will run

v4: Change subject line to trigger BAT (please!)

Signed-off-by: Tim Gore <tim.gore@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454586574-2343-1-git-send-email-tim.gore@intel.com
2016-02-04 14:22:31 +00:00
Ramalingam C
58d4d32f43 drm/i915/dsi: Configure DSI after enabling DSI pll
We need to enable DSI PLL before configuring the DSI registers.

This has worked before on BYT/CHV, but BXT is more fussy.

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Mika Kahola <mika.kahola@intel.com>
Tested-by: Mika Kahola <mika.kahola@intel.com> # BXT
Tested-by: Jani Nikula <jani.nikula@intel.com> # BYT
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454503846-12103-1-git-send-email-ramalingam.c@intel.com
2016-02-04 11:07:35 +02:00
Chris Wilson
de4726649b drm/i915: Allow i915_gem_object_get_page() on userptr as well
commit 033908aed5
Author: Dave Gordon <david.s.gordon@intel.com>
Date:   Thu Dec 10 18:51:23 2015 +0000

    drm/i915: mark GEM object pages dirty when mapped & written by the CPU

introduced a check into i915_gem_object_get_dirty_pages() that returned
a NULL pointer when called with a bad object, one that was not backed by
shmemfs. This WARN was too strict as we can work on all struct page
backed objects, and resulted in a WARN + GPF for existing userspace. In
order to differentiate the various types of objects, add a new flags field
to the i915_gem_object_ops struct to describe their capabilities, with
the first flag being whether the object has struct pages.

v2: Drop silly const before an integer in the structure declaration.

Testcase: igt/gem_userptr_blits/relocations
Reported-and-tested-by: Kristian Høgsberg Kristensen <krh@bitplanet.net>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dave Gordon <david.s.gordon@intel.com>
Cc: Kristian Høgsberg Kristensen <krh@bitplanet.net>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Kristian Høgsberg Kristensen <krh@bitplanet.net>
Tested-by: Michal Winiarski <michal.winiarski@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453487551-16799-1-git-send-email-chris@chris-wilson.co.uk
2016-02-03 10:21:24 -08:00
Matt Roper
b2435692db drm/i915: Pretend cursor is always on for ILK-style WM calculations (v2)
Due to our lack of two-step watermark programming, our driver has
historically pretended that the cursor plane is always on for the
purpose of watermark calculations; this helps avoid serious flickering
when the cursor turns off/on (e.g., when the user moves the mouse
pointer to a different screen).  That workaround was accidentally
dropped as we started working toward atomic watermark updates.  Since we
still aren't quite there yet with two-stage updates, we need to
resurrect the workaround and treat the cursor as always active.

v2: Tweak cursor width calculations slightly to more closely match the
    logic we used before the atomic overhaul began.  (Ville)

Cc: simdev11@outlook.com
Cc: manfred.kitzbichler@gmail.com
Cc: drm-intel-fixes@lists.freedesktop.org
Reported-by: simdev11@outlook.com
Reported-by: manfred.kitzbichler@gmail.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93892
Fixes: 43d59eda1 ("drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)")
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454479611-6804-1-git-send-email-matthew.d.roper@intel.com
2016-02-03 05:59:03 -08:00
Matt Roper
ed8d60f450 drm/i915: Check DDI max lanes after applying BXT workaround
In commit bfb9faab8 we added a workaround for some BXT BIOS that fail to
properly initialize the DDI_A_4_LANES bit of the control register (4
lanes is the only valid configuration on BXT since there is no DDI E to
share with).  A recent patch added some additional checks on this
register bit before the workaround gets applied; this breaks eDP on BXT
in some settings.  Some minor code shuffling is all we need to restore
the workaround.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Fixes: 7cd87cb80 ("drm/i915: Check max number of lanes when registering DDI ports")
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454022577-834-1-git-send-email-matthew.d.roper@intel.com
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
2016-02-02 17:52:01 -08:00
Matt Roper
e1ea075423 drm/i915/bxt: Don't save/restore eDP panel power during suspend (v3)
Our attempts save/restore panel power state in i915_suspend.c are
causing unclaimed register warnings on BXT since the registers for this
platform differ from older platforms.

The big hammer suspend/resume shouldn't be necessary for PP since the
connector/encoder hooks should already handle this.  In theory we could
remove this for all platforms, but in practice it's likely that would
cause some regressions since older platforms with LVDS may have
incomplete PP handling.  For now we'll leave the PCH save/restore alone
and change the non-PCH branch to only operate on gen <= 4 so that BXT
and future platforms aren't included.

v2: Typo fix: s/||/&&/

v3: Change non-PCH condition to a gen <= 4 test rather than listing
    VLV/CHV/BXT as specific platforms to exclude; should be more
    future-proof as we add new platforms.  (Daniel)

Cc: Vandana Kannan <vandana.kannan@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: drm-intel-fixes@lists.freedesktop.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1452102821-17190-1-git-send-email-matthew.d.roper@intel.com
2016-02-02 20:12:05 +02:00
Imre Deak
05d25214c4 drm/i915: Properly terminate KMS mode name string during tv init
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: David Weinehall <david.weinehall@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454071949-24677-4-git-send-email-imre.deak@intel.com
2016-02-02 18:28:32 +02:00
Imre Deak
b074eae12f drm/i915: Add debug info for failed MSI enabling
While not being able to enable MSI interrupts may be a normal
circumstance, for debugging it may still be a useful information, so
emit an info about this.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: David Weinehall <david.weinehall@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454071949-24677-3-git-send-email-imre.deak@intel.com
2016-02-02 18:28:28 +02:00
Imre Deak
9c06f6744d drm/i915/chv: Fix error path in GPU freq helpers
Atm we wouldn't catch these errors or on the error path we would end up
with a division-by-zero, fix this up.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: David Weinehall <david.weinehall@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454071949-24677-2-git-send-email-imre.deak@intel.com
2016-02-02 18:28:25 +02:00
Imre Deak
d81a67cc1b drm/i915: Sanity check DP AUX message buffer and size
While we are calling intel_dp_aux_transfer() with msg->size=0 whenever
msg->buffer is NULL, passing NULL to memcpy() is undefined according to
the ISO C standard. I haven't found any notes about this in the GNU C's
or the kernel's documentation of the function and can't imagine what it
would do with the NULL ptr. To better document this use of the
parameters it still make sense to add an explicit check for this to the
code.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: David Weinehall <david.weinehall@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454071949-24677-1-git-send-email-imre.deak@intel.com
2016-02-02 18:28:20 +02:00
Patrik Jakobsson
bf4f2fb05f drm/i915/skl/kbl: Add support for pipe fusing
On SKL and KBL we can have pipe A/B/C disabled by fuse settings. The
pipes must be fused in descending order (e.g. C, B+C, A+B+C). We simply
decrease info->num_pipes if we find a valid fused out config.

v2: Don't store the pipe disabled mask in device info (Damien)

v3: Don't check FUSE_STRAP register for pipe c disabled

Cc: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
[Jani: fixed some checkpatch indentation complaints]
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453300280-10661-1-git-send-email-patrik.jakobsson@linux.intel.com
2016-02-02 17:04:39 +02:00
Rodrigo Vivi
65f61b426d drm/i915: Instrument PSR parameter for debuging with link standby x link off.
Unfortunately we don't know all panels and platforms out there and we
found internal prototypes without VBT proper set but where only
link in standby worked well.

So, before enable PSR by default let's instrument the PSR parameter
in a way that we can identify different panels out there that might
require or work better with link standby mode.

It is also useful to say that for backward compatibility I'm not
changing the meaning of this flag. So "0" still means disabled
and "1" means enabled with full support and maximum power savings.

v2: Use positive value instead of negative for different operation mode
    as suggested by Daniel.

v3: As Paulo suggested use 2 to force link standby and 3 to force link
    fully on. Also split the link_standby introduction in a separated patch.

v4: Use DRM_ERROR for link off request on platforms that don't support and
    Remove the quirk promise.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454356928-19779-1-git-send-email-rodrigo.vivi@intel.com
2016-02-01 12:03:36 -08:00
Rodrigo Vivi
60e5ffe329 drm/i915: Add PSR main link standby support back
Link standby support has been deprecated with 'commit 89251b177
("drm/i915: PSR: deprecate link_standby support for core platforms.")'

The reason for that is that main link in full off offers more power
savings and on HSW and BDW implementations on source side had known
bugs with link standby.

However that same HSD report only mentions BDW and HSW and tells that
a fix was going to new platforms. Since on Skylake link standby
didn't cause the bad blank flickering screens seen on HSW and BDW
let's respect VBT again for this and future platforms.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
2016-02-01 12:03:36 -08:00
Rodrigo Vivi
dc9b5a0c63 drm/i915: PSR simplify port and link standby checks.
Current code not just block link_standby for non DDI platforms but also
block PSR from work on other ports B/C/D/E.

So, besides change any behaviour let's just fix the mess a bit here and
reuse HSW check to block the other ports and reduce the second if only to
link stadnby request.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
2016-02-01 12:03:36 -08:00
Mat Martineau
a41c888259 drm/i915/skl: Fix DMC load on Skylake J0 and K0
The driver does not load firmware for unknown steppings, so these new
steppings must be added to the list.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454023163-25469-1-git-send-email-mathew.j.martineau@linux.intel.com
2016-02-01 08:58:22 -08:00
Rodrigo Vivi
dc5a903714 drm/i915: Sink CRC: tune down error message at stop to debug_kms.
When we stop the sink CRC calculation we wait a while until the counter
is reset to zero and return -ETIMEDOUT. However the sink crc was
calculated already by this point so we just ignore this return at
the main function.

So, let's also ignore the message and put it as a debug message instead
of an error one. The message might still be useful when debuging
test failures so we could be able to know something was not going so
well with sink crc stop.

v2: Improve log message.

Reference: https://bugs.freedesktop.org/show_bug.cgi?id=93694
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1454107499-29678-1-git-send-email-rodrigo.vivi@intel.com
2016-01-29 14:50:42 -08: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
e8216e502a drm/i915/fbc: call intel_fbc_pre_update earlier during page flips
Make sure we do the pre_update - which also deactivates FBC - before
we actually schedule the page flip, just to make sure we don't
flip to the new FB with FBC still activated for the previous FB.

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-24-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:19:00 -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
dd8b3bdbdb drm/i915: simplify struct drm_device access at intel_atomic_check()
We already have a dev variable, there's no need to access state->dev.
Also, I plan to add another dev_priv user here, so declare one for the
current user.

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-17-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:16:27 -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
fcf38d1349 drm/i915/fbc: unconditionally update FBC during atomic commits
We unconditionally disable/update FBC even during the page flip
IOCTLs, and an unconditional disable/update at every atomic commit
touching the primary plane shouldn't impact PC state residency
noticeably. Besides, the code that checks for rotation is a good hint
that we may be forgetting something else, so let's leave all the
decisions to intel_fbc.c, making the code much safer.

Once we have the code to properly make FBC enable/update decisions
based on atomic states, with proper locking, then we'll be able to
evaluate whether it will be worth trying to optimize the cases where a
disable isn't needed.

v2: Upstream moved and now our patch needs to remove dev_priv.

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/1453406837-10511-1-git-send-email-paulo.r.zanoni@intel.com
2016-01-29 18:10:54 -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