The workarounds that are applied are exported to a debugfs file;
this is used to verify their state after the test case (reset or
suspend/resume etc). This patch is only required to support i-g-t.
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
For BDW workarounds are currently initialized in init_clock_gating() but
they are lost during reset, suspend/resume etc; this patch moves the WAs
that are part of register state context to render ring init fn otherwise
default context ends up with incorrect values as they don't get initialized
until init_clock_gating fn.
v2: Add workarounds to golden render state
This method has its own issues, first of all this is different for
each gen and it is generated using a tool so adding new workaround
and mainitaining them across gens is not a straightforward process.
v3: Use LRIs to emit these workarounds (Ville)
Instead of modifying the golden render state the same LRIs are
emitted from within the driver.
v4: Use abstract name when exporting gen specific routines (Chris)
For: VIZ-4092
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
According to spec FBC on BDW and HSW are identical without any gaps.
So let's copy the nuke and let FBC really start compressing stuff.
Without this patch we can verify with false color that nothing is being
compressed. With the nuke in place and false color it is possible
to see false color debugs.
Unfortunatelly on some rings like BCS on BDW we have to avoid Bits 22:18 on
LRIs due to a high risk of hung. So, when using Blt ring for frontbuffer rend
cache would never been cleaned and FBC would stop compressing buffer.
One alternative is to cache clean on software frontbuffer tracking.
v2: Fix rebase conflict.
v3: Do not clean cache on BCS ring. Instead use sw frontbuffer tracking.
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Try to avoid confusion with ARRAY_SIZE()/2 and hdmi_level*2.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
[danvet: Resolve silent patch conflict (didn't even fail to build)
with with Sonika's preceding patch to use the
hsw_ddi_translations_fdi table to driver the fdi link training
iteration loop. Also drop the double-write loop Damien spotted.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Renaming the HSW-specific macros for ddi buffer translation slot to denote the
slot and not the vswing/pre-emph values as they are platform-dependent.
This patch is based on top of the patch series for renaming the DP training
vswing/pre-emph defines:
http://lists.freedesktop.org/archives/intel-gfx/2014-August/050407.html
v2: Creating single macro with argument for slot number (Damien)
v3: Adding macro for num of translation entries (Damien)
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Previously, it was possible for the GPU memory accesses to be swizzled
to try to optimize the fetches for tiled buffers. This swizzling was on
top of what the memory controller in the uncore already does.
With broadwell, we drop that GPU side swizzling, and the corresponding
initialization in 3 units (GAM, GT, DE). All those bits are reserved, as
specs put it:
Before Gen8, there was a historical configuration control field to
swizzle address bit[6] for in X/Y tiling modes. This was set in three
different places: TILECTL[1:0], ARB_MODE[5:4], and
DISP_ARB_CTL[14:13]"
For Gen8 the swizzle fields are all reserved, and the CPU's memory
controller performs all address swizzling modifications.
This also means that user space doesn't have to manually swizzle when
accessing tiled buffers from the CPU, and so we always return
I915_BIT_6_SWIZZLE_NONE from i915_gem_detect_bit_6_swizzle(), which
short-circuits the initialization of the registers mentionned above in
i915_gem_init_swizzling().
v2: Refine the explanation a bit more (Daniel)
v3: Make it BDW+ specific (Steve)
Cc: Steve Aarnio <steve.j.aarnio@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
[danvet: Keep the actual code to set the tiling bits for now, in case
some bios escaped to the wild that uses this - we'd need it for
fastboot.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Instead of going through hoops, just put the driver author directly as
DRM_AUTHOR() argument. This will also make it consistent when we add
Intel to the list.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Fix assert_panel_unlocked for vlv/chv, and improve it a bit for
non-LVDS. Also don't pretend it works for DDI. There's still work to do
to get this right for eDP on PCH platforms, but this is a start.
v2: WARN_ON(HAS_DDI)
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The patch introduces fixes for the debugfs attributes emitted by
the i915 driver for GEN8. Currently, it is not emitting the correct
attributes which include the status of RC6 states.
Change-Id: Ib2068a0cac9a5wq3f228e547fa1a097ad369d242df
Signed-off-by: Vedang Patel <vedang.patel@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Rather than describing an object as either "snooped or LLC", we can do
better as we should know what machine we are running on!
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
On BDW we're seeing a problem that after we runtime resume, the
outputs connected to DDI C are not detected: they don't appear in the
SDEISR register and GMBUS transactions don't work. They stop working
at the moment we call intel_opregion_notify_adapter() during runtime
suspend, but they don't go back to work when we call the same function
during runtime resume. They only work after we do a modeset and call
intel_opregion_notify_encoder(), but this point is already too late.
While debugging, I tried to pass PCI_D3hot which is the value that
matches the spec, and it seems to have solved the problem. I couldn't
find any explanation of why this solves the problem, but there's also
no documented explanation - besides our code and git log - of why
Haswell should use PCI_D1, so keep this for now in order to keep BDW
runtime PM working.
Also add a comment to point the fact that there's no spec documenting
all the weirdness involved here.
Cc: kristen.c.accardi@intel.com
Testcase: igt/pm_rpm/drm-resources-equal
Testcase: igt/pm_rpm/i2c
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Because CHV uses cherryview_init_clock_gating instead of
gen8_init_clock_gating.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Because BDW has WPT, which is equivalent to LPT. This is just like the
CPT/PPT case.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Use the correct mask for the unlock bits. In theory this could have lead
to incorrect asserts but this is unlikely in practise.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
These two functions make no sense in an Logical Ring Context & Execlists
world.
v2: We got rid of lrc_enabled and centralized everything in the sanitized
i915.enable_execlists instead.
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
v3: Rebased. Corrected a typo in comment for i915_switch_context and
added a comment that it should not be called in execlist mode. Added
WARN_ON if i915_switch_context is called in execlist mode. Moved check
for execlist mode out of i915_switch_context and into callers. Added
comment in context_reset explaining why nothing is done in execlist
mode.
Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
[danvet: Simplify the patch subject so I can understand it.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Programing GT IER interrupts was fumbled while enabling Interrupts for
gen8
We forgot to program PM IER interrupt in gen8_gt_irq_postinstall based
on the new re-worked interrupt routines.
v2: Kill the loop and init GT interrupts individually (Ville)
Signed-off-by: Deepak S <deepak.s@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
[danvet: Adjust commit message as per discussion with Deepak.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
A pending commit removes synchronous mode from switch_mm. This breaks
execlists because switch_mm will always try to write to the legacy ring
buffer.
Return immediately from i915_ppgtt_init_gw in execlists mode.
No longer check for execlists mode in gen8_ppgtt_enable() because this
will no longer be called in execlists mode.
Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Be sure to always flush a stuck pageflip even if we couldn't possibly
expect one to be there.
References: https://bugs.freedesktop.org/show_bug.cgi?id=82612
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Unfortunately, the gem_obj/vma relationship is not symmetrical; a gem_obj
can look up for the same vma more than once (where the ppgtt refcount is
incremented), but will free the vma only once (i915_gem_free_object).
This difference in refcount get/put means that the ppgtt is not removed
after the context and vma are destroyed, because sometimes the refcount
will never go back to zero.
v2: Just move the ppgtt refcount into vma_create.
OTC-Jira: VIZ-3719
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Less pointless indentation is always nice. There will be a bit more
code in this function once the power sequencer locking is fixed.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
If we force vdd off warn if someone is still using it. With this
change the delayed vdd off work needs to check want_panel_vdd
itself to make sure it doesn't try to turn vdd off when someone
is using it.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Looks nicer.
Not functional change.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
[danvet: Add "No functional change" as requested by Jani.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Add a comment to explain why we care about the current want_panel_vdd
state in intel_dp_aux_ch().
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
edp_* are now the lower level functions and intel_edp_* the higher level
ones. One should use them in pairs.
v2: Don't return void (Jani)
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We want to use the higher level vdd on func here. Not a big deal
yet (we'd just get the warn when things go awry) but when the
locking gets fixed this becomes more important.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Passing the port as a parameter to PANEL_PORT_SELECT_VLV results in
neater code. Sadly the PCH port select bits aren't suitable for the
same treatment and the resulting macro would be much uglier, so
leave those defines as is.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
A previous commit broke aliasing PPGTT for lrc, resulting in a kernel oops
on boot. Add a check so that is full PPGTT is not in use the context is
populated with the aliasing PPGTT.
Issue: VIZ-4278
Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
In the move over to use BIOS connector configs, we lost the ability to
force a specific set of connectors on or off. Try to remedy that by
dropping back to the old behavior if we detect a hard coded connector
config that tries to enable a connector (disabling is easy!).
Based on earlier patches by Jesse Barnes.
v2: Remove Jesse's patch
Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We need do forcewake before Disabling RC6, This is what the BIOS
expects while going into suspend.
v2: updated commit message. (Daniel)
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Deepak S <deepak.s@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Improve the debug message that tells us we've been waiting for a vblank
that never arrived. Printing the pipe could lead a "doh!" moment where
we've been waiting for a vblank on a pipe that was off for instance.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Thomas Wood <thomas.wood@intel.com>
[danvet: Polish commit message a bit.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Chris has decided that enough is enough. It's time to fixup dev Vs
dev_priv. This is a modest contribution to the crusade.
v2: Still use INTEL_INFO(), for the (mythical!) case we want to hardcode
the info struct with defines (Chris)
Rename the macro argument from 'dev' to 'dev_priv' (Jani)
v3: Use names unlikely to be used as macro arguments (Chris)
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Since the ringbuffer does not belong per engine anymore, we have to
make sure that we are always recording the correct ringbuffer.
TODO: This is only a small fix to keep basic error capture working, but
we need to add more information for it to be useful (e.g. dump the
context being executed).
v2: Reorder how the ringbuffer is chosen to clarify the change and
rename the variable, both changes suggested by Chris Wilson. Also,
add the TODO comment to the code, as suggested by Daniel.
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
For cleanliness, i915_error_object_create() was written to handle the
NULL pointer in a central location. The macro that wrapped it and passed
it a num_pages to use, was not safe. As we now never limit the num_pages
to use (we did so at one point to only capture the first page of the
context), we can remove the redundant macro and be NULL safe again.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
For stolen pages, since it is verboten to access them directly on many
architectures, we have to read them through the GTT aperture. If they
are not accessible through the aperture, then we have to abort.
This was complicated by
commit 8b6124a633
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Thu Jan 30 14:38:16 2014 +0000
drm/i915: Don't access snooped pages through the GTT (even for error capture)
and the desire to use stolen memory for ringbuffers, contexts and
batches in the future.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Make backlight class sysfs brightness 0 value switch off the backlight
for connectors that have the backlight_power callback defined. For eDP,
this has the similar caveats regarding power savings as bl_power as only
the power sequencer backlight control is switched off.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed_by: Clinton Taylor <Clinton.A.Taylor@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This lets the userspace switch off the backlight using the backlight
class sysfs bl_power file. The switch is done using the power sequencer;
the backlight PWM, and everything else, remains enabled. The display
backlight won't draw power, but for maximum power savings the encoder
needs to be switched off.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed_by: Clinton Taylor <Clinton.A.Taylor@intel.com>
Tested_by: Clinton Taylor <Clinton.A.Taylor@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Make backlight class sysfs bl_power a sub-state of backlight enabled, if
a backlight power connector callback is defined. It's up to the
connector callback to handle the sub-state, typically in a way that
respects panel power sequencing.
v2: Post the version that does not oops. *facepalm*.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed_by: Clinton Taylor <Clinton.A.Taylor@intel.com>
Tested_by: Clinton Taylor <Clinton.A.Taylor@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Make it possible to change panel power control backlight state without
touching the PWM. No functional changes.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed_by: Clinton Taylor <Clinton.A.Taylor@intel.com>
Tested_by: Clinton Taylor <Clinton.A.Taylor@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The GEN6_PM* registers don't exist on BDW anymore, so when we read
this file we trigger unclaimed register errors. The equivalent BDW
register for PMs is GEN8_GT_I*R(2), so use it.
Testcase: igt/pm_rpm/debugfs-read
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Primary planes support 180 degree rotation. Expose the feature
through rotation drm property.
v2: Calculating linear/tiled offsets based on pipe source width and
height. Added 180 degree rotation support in ironlake_update_plane.
v3: Checking if CRTC is active before issueing update_plane. Added
wait for vblank to make sure we dont overtake page flips. Disabling
FBC since it does not work with rotated planes.
v4: Updated rotation checks for pending flips, fbc disable. Creating
rotation property only for Gen4 onwards. Property resetting as part
of lastclose.
v5: Resetting property in i915_driver_lastclose properly for planes
and crtcs. Fixed linear offset calculation that was off by 1 w.r.t
width in i9xx_update_plane and ironlake_update_plane. Removed tab
based indentation and unnecessary braces in intel_crtc_set_property
and intel_update_fbc. FBC and flip related checks should be done only
for valid crtcs.
v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property
and positioning the disable code in intel_update_fbc.
v7: In case rotation property on inactive crtc is updated, we return
successfully printing debug log as crtc is inactive and only property change
is preserved.
v8: update_plane is changed to update_primary_plane, crtc->fb is changed to
crtc->primary->fb and return value of update_primary_plane is ignored.
v9: added rotation property to primary plane instead of crtc. Removing reset
of rotation property from lastclose. rotation_property is moved to
drm_mode_config, so drm layer will take care of resetting. Adding updation of
fbc when rotation is set to 0. Allowing rotation only if value is
different than old one.
v10: Calling intel_primary_plane_setplane instead of update_primary_plane in
set_property(Daniel).
v11: Using same set_property function for both primary and sprite, Adding
primary plane specific code in the same function (Matt).
v12: Removing disabling/ enabling of fbc from set_property because it is done
from intel_pipe_set_base. Other formatting
v13: we need to call disable_fbc before changing the rotation to 180,
disable_fbc from intel_pipe_set_base gets called very late, that will
be used to re-enable fbc if rotation is set to 0 (Ville).
Testcase: igt/kms_rotation_crc
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
[danvet: Add FIXME to explain why we need the open-coded update_fbc
hunk to disable fbc when rotated 180 degree. And make checkpatch
happier.]
Acked-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This unifies how the primary plane functions work with how the sprite
functions works, which allows us to reuse them to update primary plane
properties.
v2: Moving setting of plane members in the end to take care of failure cases and
not-visible cases (Matt).
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
Acked-by: Matt Roper <matthew.d.roper@intel.com>
[danvet: Add a real commit message.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This patch is to address Daniels concerns over different code during reset:
http://lists.freedesktop.org/archives/intel-gfx/2014-June/047758.html
"The reason for aiming as hard as possible to use the exact same code for
driver load, gpu reset and runtime pm/system resume is that we've simply
seen too many bugs due to slight variations and unintended omissions."
Tested using igt drv_hangman.
V2: Cleaner way of preventing check_wedge returning -EAGAIN
V3: Clean the last_context during reset, to ensure do_switch() does the MI_SET_CONTEXT. As per review.
Signed-off-by: McAulay, Alistair <alistair.mcaulay@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
[danvet: Rebase over ctx->ppgtt rework and extend the comment in
check_wedge a bit.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drm-intel-next-2014-08-22:
- basic code for execlist, which is the fancy new cmd submission on gen8. Still
disabled by default (Ben, Oscar Mateo, Thomas Daniel et al)
- remove the useless usage of console_lock for I915_FBDEV=n (Chris)
- clean up relations between ctx and ppgtt
- clean up ppgtt lifetime handling (Michel Thierry)
- various cursor code improvements from Ville
- execbuffer code cleanups and secure batch fixes (Chris)
- prep work for dev -> dev_priv transition (Chris)
- some of the prep patches for the seqno -> request object transition (Chris)
- various small improvements all over
* tag 'drm-intel-next-2014-09-01' of git://anongit.freedesktop.org/drm-intel: (86 commits)
drm/i915: fix suspend/resume for GENs w/o runtime PM support
drm/i915: Update DRIVER_DATE to 20140822
drm: fix plane rotation when restoring fbdev configuration
drm/i915/bdw: Disable execlists by default
drm/i915/bdw: Enable Logical Ring Contexts (hence, Execlists)
drm/i915/bdw: Document Logical Rings, LR contexts and Execlists
drm/i915/bdw: Print context state in debugfs
drm/i915/bdw: Display context backing obj & ringbuffer info in debugfs
drm/i915/bdw: Display execlists info in debugfs
drm/i915/bdw: Disable semaphores for Execlists
drm/i915/bdw: Make sure gpu reset still works with Execlists
drm/i915/bdw: Don't write PDP in the legacy way when using LRCs
drm/i915: Track cursor changes as frontbuffer tracking flushes
drm/i915/bdw: Help out the ctx switch interrupt handler
drm/i915/bdw: Avoid non-lite-restore preemptions
drm/i915/bdw: Handle context switch events
drm/i915/bdw: Two-stage execlist submit process
drm/i915/bdw: Write the tail pointer, LRC style
drm/i915/bdw: Implement context switching (somewhat)
drm/i915/bdw: Emission of requests with logical rings
...
Conflicts:
drivers/gpu/drm/i915/i915_drv.c
Before sharing common parts between the system and runtime s/r
handlers we WARNed if the runtime s/r handlers were called on GENs that
didn't support RPM. But this WARN is not correct if the same handler is
called from the system s/r path, since that can happen on any platform.
This also broke system s/r on old platforms.
The issue was introduced in
commit 016970beb0
Author: Sagar Kamble <sagar.a.kamble@intel.com>
Date: Wed Aug 13 23:07:06 2014 +0530
v2:
- remove the WARN and depend on the HAS_RUNTIME_PM check in
rutime_suspend/resume instead (Daniel)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82751
Signed-off-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
- Setting dp M2/N2 values plus state checker support (Vandana Kannan)
- chv power well support (Ville)
- DP training pattern 3 support for chv (Ville)
- cleanup of the hsw/bdw ddi pll code, prep work for skl (Damien)
- dsi video burst mode support (Shobhit)
- piles of other chv fixes all over (Ville et. al.)
- cleanup of the ddi translation tables setup code (Damien)
- 180 deg rotation support (Ville & Sonika Jindal)
* tag 'drm-intel-next-2014-08-08' of git://anongit.freedesktop.org/drm-intel: (59 commits)
drm/i915: Update DRIVER_DATE to 20140808
drm/i915: No busy-loop wait_for in the ring init code
drm/i915: Add sprite watermark programming for VLV and CHV
drm/i915: Round-up clock and limit drain latency
drm/i915: Generalize drain latency computation
drm/i915: Free pending page flip events at .preclose()
drm/i915: clean up PPGTT checking logic
drm/i915: Polish the chv cmnlane resrt macros
drm/i915: Hack to tie both common lanes together on chv
drm/i915: Add cherryview_update_wm()
drm/i915: Update DDL only for current CRTC
drm/i915: Parametrize VLV_DDL registers
drm/i915: Fill out the FWx watermark register defines
drm: Resetting rotation property
drm/i915: Add rotation property for sprites
drm: Add rotation_property to mode_config
drm/i915: Make intel_plane_restore() return an error
drm/i915: Add 180 degree sprite rotation support
drm/i915: Introduce a for_each_intel_encoder() macro
drm/i915: Demote the DRRS messages to debug messages
...
We still have a few missing bits and pieces to have execlists enabled by
default eg. the error capture or the render state initialization and so
it wouldn't be wise to enable it by default on BDW just yet.
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Thomas Daniel <thomas.daniel@intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82740
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The time has come, the Walrus said, to talk of many things.
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Add theory of operation notes to intel_lrc.c and comments to externally
visible functions.
v2: Add notes on logical ring context creation.
v3: Use kerneldoc.
v4: Integrate it in the DocBook template.
Signed-off-by: Thomas Daniel <thomas.daniel@intel.com> (v1)
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> (v2, v3)
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
[danvet: Drop hunk about render ring init function since that's not
yet merged.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This has turned out to be really handy in debug so far.
Update:
Since writing this patch, I've gotten similar code upstream for error
state. I've used it quite a bit in debugfs however, and I'd like to keep
it here at least until preemption is working.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
This patch was accidentally dropped in the first Execlists version, and
it has been very useful indeed. Put it back again, but as a standalone
debugfs file.
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
v2: Take the device struct_mutex rather than mode_config mutex for
atomic state capture.
Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
v2: Warn and return if LRCs are not enabled.
v3: Grab the Execlists spinlock (noticed by Daniel Vetter).
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
v4: Lock the struct mutex for atomic state capture
Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
[danvet: Checkpatch.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Up until recently, semaphores weren't enabled in BDW so we didn't care
about them. But then Rodrigo came and enabled them:
commit 521e62e49a
Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
drm/i915: Enable semaphores on BDW
So now we have to explicitly disable them for Execlists until both
features play nicely.
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
If we reset a ring after a hang, we have to make sure that we clear
out all queued Execlists requests.
v2: The ring is, at this point, already being correctly re-programmed
for Execlists, and the hangcheck counters cleared.
v3: Daniel suggests to drop the "if (execlists)" because the Execlists
queue should be empty in legacy mode (which is true, if we do the
INIT_LIST_HEAD).
v4: Do the pending intel_runtime_pm_put
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This is mostly for correctness so that we know we are running the LR
context correctly (this is, the PDPs are contained inside the context
object).
v2: Move the check to inside the enable PPGTT function. The switch
happens in two places: the legacy context switch (that we won't hit
when Execlists are enabled) and the PPGTT enable, which unfortunately
we need. This would look much nicer if the ppgtt->enable was part of
the ring init, where it logically belongs.
v3: Move the check to the start of the enable PPGTT function. None
of the legacy PPGTT enabling is required when using LRCs as the
PPGTT is enabled in the context descriptor and the PDPs are written
in the LRC.
v4: Clarify comment based on review feedback.
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
[danvet: Resolve conflicts with ppgtt_enable rework.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Atm we may retrain the DP link even if the CRTC is inactive through
HPD work->intel_dp_check_link_status(). This in turn can lock up the PHY
(at least on BYT), since the DP port is disabled.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81948
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: stable@vger.kernel.org (3.16+)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Atm we may leave eDP VDD enabled during system suspend after the CRTCs
are disabled through an HPD->DPCD read event. So disable VDD during
suspend at a point when no HPDs can occur.
Note that runtime suspend doesn't have the same problem, since there the
RPM ref held by VDD provides already the needed serialization.
v2:
- add note to commit message about the runtime suspend path (Ville)
- use edp_panel_vdd_off_sync(), so we can keep the WARN in
edp_panel_vdd_off() (Ville)
v3:
- rebased on -fixes (for_each_intel_encoder()->list_for_each_entry())
(Imre)
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (v2)
Cc: stable@vger.kernel.org (3.16+)
[Jani: fix sparse warning reported by Fengguang Wu]
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Make sure these work handlers don't run after we system suspend or
unload the driver. Note that we don't cancel the handlers during runtime
suspend. That could lead to a lockup, since we take a runtime PM ref
from the handlers themselves. Fortunaltely canceling there is not needed
since the RPM ref itself provides for the needed serialization.
v2:
- fix the order of canceling dig_port_work wrt. hotplug_work (Ville)
- zero out {long,short}_hpd_port_mask and hpd_event_bits for speed
(Ville)
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: stable@vger.kernel.org (3.16+)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Atm, the HPD IRQ reenable timer can get rearmed right after it's
canceled. Also to access the HPD IRQ mask registers we need to wake up
the HW.
Solve both issues by converting the reenable timer to a delayed work and
grabbing a runtime PM reference in the work. By this we can also forgo
canceling the timer during runtime suspend, since the only important
thing there is that the HW is awake when we write the registers and
that's ensured by the RPM ref. So do the cancelation only during driver
unload time; this is also a requirement for an upcoming patch where we
want to cancel all HPD related works only during system suspend and
driver unload time, but not during runtime suspend.
Note that there is still a race between the HPD IRQ reenable work and
drm_irq_uninstall() during driver unload, where the work can reenable
the HPD IRQs disabled by drm_irq_uninstall(). This isn't a problem since
the HPD IRQs will still be effectively masked by the first level
interrupt mask.
v2-3:
- unchanged
v4:
- use proper API for changing the expiration time for an already pending
delayed work (Jani)
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (v2)
Cc: stable@vger.kernel.org (3.16+)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Ville noticed that we can call ibx_digital_port_connected() which accesses
the HW without holding any power well/runtime pm reference. Fix this by
holding a display port power domain reference around the whole hpd_pulse
handler.
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Dave Airlie <airlied@redhat.com>
Cc: stable@vger.kernel.org (3.16+)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Make sure the cursor gets fully clipped when enabling it on a disabled
crtc via setplane. This will prevent the lower level code from
attempting to enable the cursor in hardware.
Cc: Paulo Zanoni <przanoni@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: stable@vger.kernel.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
During suspend we turn off the crtcs, but leave the staged config in
place so that we can restore the display(s) to their previous state on
resume.
During resume when we attempt to apply the force pipe A quirk we use the
load detect mechanism. That doesn't check whether there was an already
staged configuration for the crtc since that's not even possible during
normal runtime load detection. But during resume it is possible, and if
we just blindly go and overwrite the staged crtc configuration for the
load detection we can no longer restore the display to the correct
state.
Even worse, we don't even clear all the staged connector->encoder->crtc
links so we may end up using a cloned setup for the load detection, and
after we're done we just clear the links related to the VGA output
leaving the links for the other outputs in place. This will eventually
result in calling intel_set_mode() with mode==NULL but with valid
connector->encoder->crtc links which will result in dereferencing the
NULL mode since the code thinks it will have to a modeset.
To avoid these problems don't use any crtc with new_enabled==true for
load detection.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: stable@vger.kernel.org (for 3.16)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
intel_enable_pipe_a() gets called with all the modeset locks already
held (by drm_modeset_lock_all()), so trying to grab the same
locks using another drm_modeset_acquire_ctx is going to fail miserably.
Move most of the drm_modeset_acquire_ctx handling (init/drop/fini)
out from intel_{get,release}_load_detect_pipe() into the callers
(intel_{crt,tv}_detect()). Only the actual locking and backoff
handling is left in intel_get_load_detect_pipe(). And in
intel_enable_pipe_a() we just share the mode_config.acquire_ctx from
drm_modeset_lock_all() which is already holding all the relevant locks.
It's perfectly legal to lock the same ww_mutex multiple times using the
same ww_acquire_ctx. drm_modeset_lock() will convert the returned
-EALREADY into 0, so the caller doesn't need to do antyhing special.
Fixes a hang on resume on my 830.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: stable@vger.kernel.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
We treat other plane updates in the same fashion. Spotted because
Rodrigo kept reporting a bug in the PSR code where the frontbuffer was
eternally stuck with a dirty cursor bit set.
The psr testcase should have caught this, but that i-g-t is kaputt.
Rodrigo is signed up to fix that.
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Tested-by-and-Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
If we receive a storm of requests for the same context (see gem_storedw_loop_*)
we might end up iterating over too many elements in interrupt time, looking for
contexts to squash together. Instead, share the burden by giving more
intelligence to the queue function. At most, the interrupt will iterate over
three elements.
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
[danvet: Checkpatch.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
In the current Execlists feeding mechanism, full preemption is not
supported yet: only lite-restores are allowed (this is: the GPU
simply samples a new tail pointer for the context currently in
execution).
But we have identified an scenario in which a full preemption occurs:
1) We submit two contexts for execution (A & B).
2) The GPU finishes with the first one (A), switches to the second one
(B) and informs us.
3) We submit B again (hoping to cause a lite restore) together with C,
but in the time we spend writing to the ELSP, the GPU finishes B.
4) The GPU start executing B again (since we told it so).
5) We receive a B finished interrupt and, mistakenly, we submit C (again)
and D, causing a full preemption of B.
The race is avoided by keeping track of how many times a context has been
submitted to the hardware and by better discriminating the received context
switch interrupts: in the example, when we have submitted B twice, we won´t
submit C and D as soon as we receive the notification that B is completed
because we were expecting to get a LITE_RESTORE and we didn´t, so we know a
second completion will be received shortly.
Without this explicit checking, somehow, the batch buffer execution order
gets messed with. This can be verified with the IGT test I sent together with
the series. I don´t know the exact mechanism by which the pre-emption messes
with the execution order but, since other people is working on the Scheduler
+ Preemption on Execlists, I didn´t try to fix it. In these series, only Lite
Restores are supported (other kind of preemptions WARN).
v2: elsp_submitted belongs in the new intel_ctx_submit_request. Several
rebase changes.
v3: Clarify how the race is avoided, as requested by Daniel.
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
[danvet: Align function parameters ...]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Handle all context status events in the context status buffer on every
context switch interrupt. We only remove work from the execlist queue
after a context status buffer reports that it has completed and we only
attempt to schedule new contexts on interrupt when a previously submitted
context completes (unless no contexts are queued, which means the GPU is
free).
We canot call intel_runtime_pm_get() in an interrupt (or with a spinlock
grabbed, FWIW), because it might sleep, which is not a nice thing to do.
Instead, do the runtime_pm get/put together with the create/destroy request,
and handle the forcewake get/put directly.
Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
v2: Unreferencing the context when we are freeing the request might free
the backing bo, which requires the struct_mutex to be grabbed, so defer
unreferencing and freeing to a bottom half.
v3:
- Ack the interrupt inmediately, before trying to handle it (fix for
missing interrupts by Bob Beckett <robert.beckett@intel.com>).
- Update the Context Status Buffer Read Pointer, just in case (spotted
by Damien Lespiau).
v4: New namespace and multiple rebase changes.
v5: Squash with "drm/i915/bdw: Do not call intel_runtime_pm_get() in an
interrupt", as suggested by Daniel.
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
[danvet: Checkpatch ...]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Context switch (and execlist submission) should happen only when
other contexts are not active, otherwise pre-emption occurs.
To assure this, we place context switch requests in a queue and those
request are later consumed when the right context switch interrupt is
received (still TODO).
v2: Use a spinlock, do not remove the requests on unqueue (wait for
context switch completion).
Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
v3: Several rebases and code changes. Use unique ID.
v4:
- Move the queue/lock init to the late ring initialization.
- Damien's kmalloc review comments: check return, use sizeof(*req),
do not cast.
v5:
- Do not reuse drm_i915_gem_request. Instead, create our own.
- New namespace.
Signed-off-by: Michel Thierry <michel.thierry@intel.com> (v1)
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> (v2-v5)
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
[davnet: Checkpatch + wash-up s/BUG_ON/WARN_ON/.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Each logical ring context has the tail pointer in the context object,
so update it before submission.
v2: New namespace.
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
A context switch occurs by submitting a context descriptor to the
ExecList Submission Port. Given that we can now initialize a context,
it's possible to begin implementing the context switch by creating the
descriptor and submitting it to ELSP (actually two, since the ELSP
has two ports).
The context object must be mapped in the GGTT, which means it must exist
in the 0-4GB graphics VA range.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
v2: This code has changed quite a lot in various rebases. Of particular
importance is that now we use the globally unique Submission ID to send
to the hardware. Also, context pages are now pinned unconditionally to
GGTT, so there is no need to bind them.
v3: Use LRCA[31:12] as hwCtxId[19:0]. This guarantees that the HW context
ID we submit to the ELSP is globally unique and != 0 (Bspec requirements
of the software use-only bits of the Context ID in the Context Descriptor
Format) without the hassle of the previous submission Id construction.
Also, re-add the ELSP porting read (it was dropped somewhere during the
rebases).
v4:
- Squash with "drm/i915/bdw: Add forcewake lock around ELSP writes" (BSPEC
says: "SW must set Force Wakeup bit to prevent GT from entering C6 while
ELSP writes are in progress") as noted by Thomas Daniel
(thomas.daniel@intel.com).
- Rename functions and use an execlists/intel_execlists_ namespace.
- The BUG_ON only checked that the LRCA was <32 bits, but it didn't make
sure that it was properly aligned. Spotted by Alistair Mcaulay
<alistair.mcaulay@intel.com>.
v5:
- Improved source code comments as suggested by Chris Wilson.
- No need to abstract submit_ctx away, as pointed by Brad Volkin.
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
[danvet: Checkpatch. Sigh.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
On a previous iteration of this patch, I created an Execlists
version of __i915_add_request and asbtracted it away as a
vfunc. Daniel Vetter wondered then why that was needed:
"with the clean split in command submission I expect every
function to know wether it'll submit to an lrc (everything in
intel_lrc.c) or wether it'll submit to a legacy ring (existing
code), so I don't see a need for an add_request vfunc."
The honest, hairy truth is that this patch is the glue keeping
the whole logical ring puzzle together:
- i915_add_request is used by intel_ring_idle, which in turn is
used by i915_gpu_idle, which in turn is used in several places
inside the eviction and gtt codes.
- Also, it is used by i915_gem_check_olr, which is littered all
over i915_gem.c
- ...
If I were to duplicate all the code that directly or indirectly
uses __i915_add_request, I'll end up creating a separate driver.
To show the differences between the existing legacy version and
the new Execlists one, this time I have special-cased
__i915_add_request instead of adding an add_request vfunc. I
hope this helps to untangle this Gordian knot.
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
[danvet: Adjust to ringbuf->FIXME_lrc_ctx per the discussion with
Thomas Daniel.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The execlist patches have a bit a convoluted and long history and due
to that have the actual submission still misplaced deeply burried in
the low-level ringbuffer handling code. This design goes back to the
legacy ringbuffer code with its tricky lazy request and simple work
submissiion using ring tail writes. For that reason they need a
ring->ctx backpointer.
The goal is to unburry that code and move it up into a level where the
full execlist context is available so that we can ditch this
backpointer. Until that's done make it really obvious that there's
work still to be done.
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Thomas Daniel <thomas.daniel@intel.com>
Acked-by: Thomas Daniel <thomas.daniel@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The current error state harks back to the era of just a single VM. For
full-ppgtt, we capture every bo on every VM. It behoves us to then print
every bo for every VM, which we currently fail to do and so miss vital
information in the error state.
v2: Use the vma address rather than -1!
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
On VLV, post S0i3 during i915_drm_thaw following issue is observed during ring
initialization.
[ 335.604039] [drm:stop_ring] ERROR render ring :timed out trying to stop ring
[ 336.607340] [drm:stop_ring] ERROR render ring :timed out trying to stop ring
[ 336.607345] [drm:init_ring_common] ERROR failed to set render ring head to zero ctl 00000000 head 00000000 tail 00000000 start 00000000
[ 337.610645] [drm:stop_ring] ERROR bsd ring :timed out trying to stop ring
[ 338.613952] [drm:stop_ring] ERROR bsd ring :timed out trying to stop ring
[ 338.613956] [drm:init_ring_common] ERROR failed to set bsd ring head to zero ctl 00000000 head 00000000 tail 00000000 start 00000000
[ 339.617256] [drm:stop_ring] ERROR render ring :timed out trying to stop ring
[ 339.617258] -----------[ cut here ]-----------
[ 339.617267] WARNING: CPU: 0 PID: 6 at drivers/gpu/drm/i915/intel_ringbuffer.c:1666 intel_cleanup_ring+0xe6/0xf0()
[ 339.617396] --[ end trace 5ef5ed1a3c92e2a6 ]--
[ 339.617428] [drm:__i915_drm_thaw] ERROR failed to re-initialize GPU, declaring wedged!
This is happening since wake is not enabled and Gunit registers are not restored.
For this system suspend/resume paths need to follow save/restore and additional
platform specific setup in suspend_complete and resume_prepare.
suspend_complete is shared unconditionaly for VLV, HSW, BDW. resume_prepare for
HSW and BDW has pc8 disabling which is needed during thaw_early so sharing
uncondtionally. For VLV and SNB runtime resume specific sequence exists.
Cc: Imre Deak <imre.deak@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Goel, Akash <akash.goel@intel.com>
Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
With this change, intel_runtime_suspend and intel_runtime_resume functions
become completely platform agnostic. Platform specific suspend/resume
changes are moved to intel_suspend_complete and intel_resume_prepare.
Cc: Imre Deak <imre.deak@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Goel, Akash <akash.goel@intel.com>
Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Rather than take and release the console_lock() around a non-existent
DRM_I915_FBDEV, move the lock acquisation into the callee where it will
be compiled out by the config option entirely. This includes moving the
deferred fb_set_suspend() dance and encapsulating it entirely within
intel_fbdev.c.
v2: Use an integral work item so that we can explicitly flush the work
upon suspend/unload.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
[danvet: Add the flush_work in fbdev_fini per the mailing list
discussion. And s/BUG_ON/WARN_ON/ because.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Ville pointed out the GCCism __builtin_types_compatible_p() that we
could use to replace our heavily casted presumption __I915__ macro that
was based on comparing struct sizes.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
845/865 support different cursor sizes as well, albeit a bit differently
than later platforms. Add the necessary code to make them work.
Untested due to lack of hardware.
v2: Warn but accept invalid stride (Chris)
Rewrite the cursor size checks for other platforms (Chris)
v3: More polish and magic to the cursor size checks (Chris)
v4: Moar polish and a comment (Chris)
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Ever since
commit 5efb3e2838
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
Date: Wed Apr 9 13:28:53 2014 +0300
drm/i915/chv: Add cursor pipe offsets
the only difference between i9xx_update_cursor() and ivb_update_cursor()
was the hsw+ pipe csc handling. Let's unify them and we can rid
outselves of some duplicated code.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
CURSIZE register exists on 845/865 only, so move it to
i845_update_cursor(). Changes to cursor size must be done only when the
cursor is disabled, so do the write just before enabling the cursor.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Make sure the cursor gets fully clipped when enabling it on a disabled
crtc via setplane. This will prevent the lower level code from
attempting to enable the cursor in hardware.
Cc: Paulo Zanoni <przanoni@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Also remove related WARN_ONs which seem to have been hit since a rather
long time. But apperently no one noticed since our module reload is
already WARNING-infested :(
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We want to move the aliasing ppgtt cleanup back into the global
gtt cleanup code for symmetry, but first we need to create such
a place.
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Address space cleanup isn't really a job for the low-level cleanup
callbacks. Without this change we can't reuse the low-level cleanup
callback for the aliasing ppgtt cleanup.
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Now that all the flow is streamlined the rule is simple: We create
a new ppgtt for a new context when we have full ppgtt enabled.
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
There's a bit a confusion since we track the global gtt,
the aliasing and real ppgtt in the ctx->vm pointer. And not
all callers really bother to check for the different cases and just
presume that it points to a real ppgtt.
Now looking closely we don't actually need ->vm to always point at an
address space - the only place that cares actually has fixup code
already to decide whether to look at the per-proces or the global
address space.
So switch to just tracking the ppgtt directly and ditch all the
extraneous code.
v2: Fixup the ppgtt debugfs file to not oops on a NULL ctx->ppgtt.
Also drop the early exit - without aliasing ppgtt we want to dump all
the ppgtts of the contexts if we have full ppgtt.
v3: Actually git add the compile fix.
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Cc: "Thierry, Michel" <michel.thierry@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
OTC-Jira: VIZ-3724
[danvet: Resolve conflicts with execlist patches while applying.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Stuffing this into the context setup code doesn't make a lot of sense.
Also reusing the real ppgtt setup code makes even less sense since the
aliasing ppgtt isn't a real address space. Leaving all that stuff
unitialized will make sure that we catch any abusers promptly.
This is also a prep work to clean up the context->ppgtt link.
v2: Fix up the logic fail, I've fumbled it so badly to completely
disable ppgtt on gen6. Spotted by Ville and Michel. Also move around
the pde write into the gen6 init function, since otherwise it won't
work at all.
v3: Only initialize the aliasing ppgtt when we actually enable it.
Cc: "Thierry, Michel" <michel.thierry@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
[danvet: Squash in fixup from Fengguang Wu.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Currently we abuse the aliasing ppgtt to set up the ppgtt support in
general. Which is a bit backwards since with full ppgtt we don't ever
need the aliasing ppgtt.
So untangle this and separate the ppgtt init from the aliasing
ppgtt. While at it drag it out of the context enabling (which just
does a switch to the default context).
Note that we still have the differentiation between synchronous and
asynchronous ppgtt setup, but that will soon vanish. So also correctly
wire up the return value handling to be prepared for when ->switch_mm
drops the synchronous parameter and could start to fail.
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
A subsequent patch will no longer initialize the aliasing ppgtt if we
have full ppgtt enabled, since we simply don't need that any more.
Unfortunately a few places check for the aliasing ppgtt instead of
checking for ppgtt in general. Fix them up.
One special case are the gtt offset and size macros, which have some
code to remap the aliasing ppgtt to the global gtt. The aliasing ppgtt
is _not_ a logical address space, so passing that in as the vm is
plain and simple a bug. So just WARN about it and carry on - we have a
gracefully fall-through anyway if we can't find the vma.
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We already needs this just as a safety check in case the preallocation
reservation dance fails. But we definitely need this to be able to
move tha aliasing ppgtt setup back out of the context code to this
place, where it belongs.
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Stuff in headers really aught to have this.
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This essentially unbreaks non-ppgtt operation where we'd scribble over
random memory.
While at it give the vm_to_ppgtt function a proper prefix and make it
a bit more paranoid.
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Hardware contexts reference a ppgtt, not the other way round. And the
only user of this (in debugfs) actually only cares about which file
the ppgtt is associated with. So give it what it wants.
While at it give the ppgtt create function a proper name&place.
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
So when reviewing Michel's patch I've noticed a few things and cleaned
them up:
- The early checks in ppgtt_release are now redundant: The inactive
list should always be empty now, so we can ditch these checks. Even
for the aliasing ppgtt (though that's a different confusion) since
we tear that down after all the objects are gone.
- The ppgtt handling functions are splattered all over. Consolidate
them in i915_gem_gtt.c, give them OCD prefixes and add wrappers for
get/put.
- There was a bit a confusion in ppgtt_release about whether it cares
about the active or inactive list. It should care about them both,
so augment the WARNINGs to check for both.
There's still create_vm_for_ctx left to do, put that is blocked on the
removal of ppgtt->ctx. Once that's done we can rename it to
i915_ppgtt_create and move it to its siblings for handling ppgtts.
v2: Move the ppgtt checks into the inline get/put functions as
suggested by Chris.
v3: Inline the now redundant ppgtt local variable.
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
VMAs should take a reference of the address space they use.
Now, when the fd is closed, it will release the ref that the context was
holding, but it will still be referenced by any vmas that are still
active.
ppgtt_release() should then only be called when the last thing referencing
it releases the ref, and it can just call the base cleanup and free the
ppgtt.
Note that with this we will extend the lifetime of ppgtts which
contain shared objects. But all the non-shared objects will get
removed as soon as they drop of the active list and for the shared
ones the shrinker can eventually reap them. Since we currently can't
evict ppgtt pagetables either I don't think that temporary leak is
important.
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
[danvet: Add note about potential ppgtt leak with this approach.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
