Let's do a frontbuffer flush on dirty fb.
To be used for DIRTYFB drm ioctl.
This patch solves the biggest PSR known issue, that is
missed screen updates during boot, mainly when there is a splash
screen involved like Plymouth.
Previously PSR was being invalidated by fbdev and Plymounth
was taking control with PSR yet invalidated and could get screen
updates normally. However with some atomic modeset changes
Pymouth modeset over ioctl was now causing frontbuffer flushes
making PSR gets back to work while it cannot track the
screen updates and exit properly.
By adding this flush on dirtyfb we properly track frontbuffer
writes and properly exit PSR.
Actually all mmap_wc users should call this dirty callback
in order to have a proper frontbuffer tracking.
In the future it can be extended to return 0 if the whole
screen has being flushed or the number of rects flushed
as Chris suggested.
v2: Remove ORIGIN_FB_DIRTY and use ORIGIN_GTT instead since dirty
callback is just called after few screen updates and not on
everyone as pointed by Daniel.
v3: Use flush instead of invalidate since flush means
invalidate + flush and dirty means drawn had finished and
it can be flushed.
v4: Remove PSR from subject since it is purely frontbuffer tracking
change and that can be useful for FBC as well.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
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>
[danvet: Fix alignment as spotted by Paulo.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Since flush actually means invalidate + flush we need to force psr
exit on PSR flush.
On Core platforms there is no way to disable hw tracking and
do the pure sw tracking so we simulate it by fully disable psr and
reschedule a enable back.
So a good idea is to minimize sequential disable/enable in cases we
know that HW tracking like when flush has been originated by a flip.
Also flip had just invalidated it already.
It also uses origin to minimize the a bit the amount of
disable/enabled, mainly when flip already had invalidated.
With this patch in place it is possible to do a flush on dirty areas
properly in a following patch.
v2: Remove duplicated exit on HSW+Sprites as pointed out by Paulo.
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: Daniel Vetter <daniel.vetter@ffwll.ch>
Stolen gets trashed during hibernation, so storing contexts there
is not a very good idea. On my IVB machines this leads to a totally
dead GPU on resume. A reboot is required to resurrect it. So let's
not store contexts where they will get trampled.
This reverts commit 149c86e74f.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The old style of memory interleaving swizzled upto the end of the
first even bank of memory, and then used the remainder as unswizzled on
the unpaired bank - i.e. swizzling is not constant for all memory. This
causes problems when we try to migrate memory and so the kernel prevents
migration at all when we detect L-shaped inconsistent swizzling.
However, this issue also extends to userspace who try to manually detile
into memory as the swizzling for an individual page is unknown (it
depends on its physical address only known to the kernel), userspace
cannot correctly swizzle objects.
v2: Mark the global swizzling as unknown rather than adjust the value
reported to userspace.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91105
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
wa_ctx_emit() depends on the name of a local variable; if the name of that
variable is changed then we get compile errors. In this case it is unlikely
to be changed as this macro is only used in this set of functions but
Kernel coding guidelines doesn't recommend doing this. It was my mistake
as I should have corrected it at the beginning but missed so correct
this before there are more usages of this macro (Bob Beckett).
https://www.kernel.org/doc/Documentation/CodingStyle,
Chapter 12, "Things to avoid when using macros", point 2):
"
2) macros that depend on having a local variable with a magic name:
#define FOO(val) bar(index, val)
might look like a good thing, but it's confusing as hell when one reads the
code and it's prone to breakage from seemingly innocent changes.
"
v2: Optimization to avoid multiple evaluation of 'index' in the macro.
Since we invoke it multiple times, compiler, if it can, should be able to coalesce
them into a single condition and remove multiple WARN_ON checks (Chris).
Suggested-by: Robert Beckett <robert.beckett@intel.com>
Cc: Robert Beckett <robert.beckett@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Writing to PCH_PORT_HOTPLUG for each interrupt is not required.
Handle it only if hpd has actually occurred like we handle other
interrupts.
v2: Make few variables local to if block (Ville)
v3: Add check for ibx/cpt both (Ville).
While at it, remove the redundant check for hotplug_trigger from
pch_get_hpd_pins
v4: Indentation (Ville)
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Since
commit 8c7b5ccb72
Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Date: Tue Apr 21 17:13:19 2015 +0300
drm/i915: Use atomic helpers for computing changed flags
we compute the plane state for a modeset before actually committing
any changes, which means crtc->active won't be correct yet. Looking at
future work in the modeset conversion targetting 4.3 the only places
where crtc_state->active isn't accurate is when disabling other CRTCs
than the one the modeset is for (when stealing connectors). Which
isn't the case here. And that's also confirmed by an audit, we do
unconditionally update crtc_state->active for the current pipe.
We also don't need to update any other plane check functions since we
only ever add the primary state to the modeset update right now.
Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This was lost in
commit ce22dba92d
Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Date: Tue Apr 21 17:12:56 2015 +0300
drm/i915: Move toggling planes out of crtc enable/disable.
and we still need that crtc->active check since the overall modeset
flow doesn't yet take dpms state into account properly. Fixes WARNING
backtraces on at least bdw/hsw due to the ips disabling code being
upset about being run on a switched-off pipe.
We don't need a corresponding change on the enable side since with the
old setCrtc semantics we always force-enable the pipe after a modeset.
And the dpms function intel_crtc_control already checks for ->active.
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
I have two separate refactor ideas that require extracting this to a
separate function. I'm not sure which idea I'll end choosing, but
since both will require extracting this function, let's do this now.
Notice that this is just code moving. Any possible problems with the
current multiple pipes check should be fixed in later commits.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The poor in_dbg_master() check was the only one without a reason
string. Give it a reason string so it won't feel excluded.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Because the cool kids use dev_priv and FBC wants to be cool too.
We've been historically using struct drm_device on the FBC function
arguments, but we only really need it for intel_vgpu_active(): we can
use dev_priv everywhere else. So let's fully switch to dev_priv since
I'm getting tired of adding "struct drm_device *dev = dev_priv->dev"
everywhere.
If I get a NACK here I'll propose the opposite: convert all the
functions that currently take dev_priv to take dev.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Since that's really what we want to test for. Note remove the gen5
case doesn't change anything: In intel_setup_outputs ilk is handled
already in the HAS_PCH_SPLIT case, and the register save/restore code
touches registers which simply doesn't exist anymore at all.
v2: Drop UMS parts.
v3: Update commit message to reflect that the reg save/restore code is
gone (Ville).
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Follow the correct pipe vs port disable sequence for the PCH LVDS
ports, ie. disable the port after the pipe.
Other PCH port were already converted in the following commits:
1ea56e269e drm/i915: Disable CRT port after pipe on PCH platforms
3c65d1d1bb drm/i915: Disable SDVO port after the pipe on PCH platforms
a4790cec3a drm/i915: Disable HDMI port after the pipe on PCH platforms
08aff3fe26 drm/i915: Move DP port disable to post_disable for pch platforms
but LVDS was forgotten.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
intel_atomic_setup_scalers() dereferences 'plane' before the plane has
been assigned. The plane ID assignment doing this dereference is only
needed for debugging messages later in the function, so just move the
assignment farther down the function to a point where plane will no
longer be NULL.
This was introduced in:
commit 133b0d128b
Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Date: Mon Jun 15 12:33:39 2015 +0200
drm/i915: Clean up intel_atomic_setup_scalers slightly.
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Bob Paauwe <bob.j.paauwe@intel.com>
Reported-by: Bob Paauwe <bob.j.paauwe@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Now when we have requests this deep on call chain, we can mark
the elsp being submitted when it actually is. Remove temp variable
and readjust commenting to more closely fit to the code.
v2: Avoid tmp variable and reduce number of writes (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
In preparation to make intel_lr_context_pin|unpin to accept
requests, assign ringbuf into request before we call the pinning.
v2: No need to unset ringbuf on error path (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
It is found that i915 will not reset gpu under execlist mode when
unload module. that will lead to some issues when unload/load module
with different submission mode. e.g. from execlist mode to ring
buffer mode via loading/unloading i915. Because HW is not in a reset
state and registers are not clean under such condition.
Signed-off-by: Niu,Bing <bing.niu@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
In this WA we need to set GEN8_L3SQCREG4[21:21] and reset it after PIPE_CONTROL
instruction but there is a slight complication as this is applied in WA batch
where the values are only initialized once.
Dave identified an issue with the current implementation where the register value
is read once at the beginning and it is reused; this patch corrects this by saving
the register value to memory, update register with the bit of our interest and
restore it back with original value.
This implementation uses MI_LOAD_REGISTER_MEM which is currently only used
by command parser and was using a default length of 0. This is now updated
with correct length and moved to appropriate place.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dave Gordon <david.s.gordon@intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Now all the functions called by other files check whether FBC has been
initialized. This allows us to drop the checks on the static
functions.
v2:
- s/HAS_FBC/dev_priv->display.enable_fbc/ everywhere but the init
function (Chris).
Suggested-by: 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>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Everything is covered either by fbc.lock or mm.stolen_lock, and
intel_fbc.c is already responsible for grabbing the appropriate locks
when it needs them.
Reviewed-by: Chris wilson <chris@chris-wilson.co.uk>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Make sure we're not going to have weird races in really weird cases
where a lot of different CRTCs are doing rendering and modesets at the
same time.
With this change and the stolen_lock from the previous patch, we can
start removing the struct_mutex locking we have around FBC in the next
patches.
v2:
- Rebase (6 months later)
- Also lock debugfs and stolen.
v3:
- Don't lock a single value read (Chris).
- Replace lockdep assertions with WARNs (Daniel).
- Improve commit message.
- Don't forget intel_pre_plane_update() locking.
v4:
- Don't remove struct_mutex at intel_pre_plane_update() (Chris).
- Add comment regarding locking dependencies (Chris).
- Rebase after the stolen code rework.
- Rebase again after drm-intel-nightly changes.
v5:
- Rebase after the new stolen_lock patch.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v4)
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Which should protect dev_priv->mm.stolen usage. This will allow us to
simplify the relationship between stolen memory, FBC and struct_mutex.
v2:
- Rebase after the stolen_remove_node() dev_priv patch move.
- I realized that after we fixed a few things related to the FBC CFB
size checks, we're not reallocating the CFB anymore with FBC
enabled, so we can just move all the locking to i915_gem_stolen.c
and stop worrying about freezing all the stolen alocations while
freeing/rellocating the CFB. This allows us to fix the "Too
coarse" observation from Chris.
Suggested-by: 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>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
With the abstractions created by the last patch, we can move this code
and the only thing inside intel_fbc.c that knows about dev_priv->mm is
the code that reads stolen_base.
We also had to move a call to i915_gem_stolen_cleanup_compression()
- now called intel_fbc_cleanup_cfb() - outside i915_gem_stolen.c.
v2:
- Rebase after the remove_node() changes on the previous patch.
Requested-by: 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>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We want to move the FBC code out of i915_gem_stolen.c, but that code
directly adds/removes stolen memory nodes. Let's create this
abstraction, so i915_gme_stolen.c is still in control of all the
stolen memory handling. The abstraction will also allow us to add
locking assertions later.
v2:
- Add dev_priv as remove_node() argument since we'll need it later
(Chris).
Requested-by: 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>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Clarify that audio enable/disable sequences are part of the modeset
sequence.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drivers/gpu/drm/i915/intel_ddi.c: In function ‘intel_prepare_ddi’:
drivers/gpu/drm/i915/intel_ddi.c:517:6: warning:
‘ddi_translations_fdi’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
if (ddi_translations_fdi)
^
drivers/gpu/drm/i915/intel_ddi.c:446:30: note: ‘ddi_translations_fdi’
was declared here
const struct ddi_buf_trans *ddi_translations_fdi;
^
This line used to be there, but was removed by:
commit f8896f5d58
Author: David Weinehall <david.weinehall@linux.intel.com>
Date: Thu Jun 25 11:11:03 2015 +030
drm/i915/skl: Buffer translation improvements
Cc: David Weinehall <david.weinehall@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Ville noticed that the PLL HW readout code parsed the fractional
divider value as if the fractional divider was always enabled. This may
result in a port clock state check mismatch if the preceeding modeset
disabled the fractional divider, but left a non-zero divider value in
the register.
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Ensures that the batch buffer is executed by the resource streamer.
And will let userspace know whether Resource Streamer is supported in
the kernel.
v2: Don't skip 1<<15 for the exec flags (Jani Nikula)
v3: Use HAS_RESOURCE_STREAMER macro for execbuf validation (Chris Wilson)
(from getparam patch)
v2: Update I915_PARAM_HAS_RESOURCE_STREAMER so it's after
I915_PARAM_HAS_GPU_RESET.
v3: Only advertise RS support for hardware that supports it.
v4: Add HAS_RESOURCE_STREAMER() macro (Chris)
Testcase: igt/gem_exec_params
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
[danvet: squash in getparam patch since it'd break bisect, suggested
by Chris.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
GEN8 and above uses Execlists by default instead of the legacy
ringbuffer for batch execution. This patch enables the resource
streamer bits when required.
Patch is based on the initial work by Minu Mathai <minu.mathai@intel.com>
This version also adds the required bits to enable GEN8 Resource
Streamer context save and restore for Execlists.
Cc: ville.syrjala@linux.intel.com
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Reviewed-by: Arun Siluvery <arun.siluvery@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
