Add a REG_WRITE_FOOTER macro as a counterpart to the REG_WRITE_HEADER.
The current code has the spin_lock() in the HEADER, but the
spin_unlock() is open coded, which looks rather confusing on the first
glance. A bit of additional symmetry might help.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
When inspecting reports that boot/suspend/resume times are unusual it
would be useful to clearly identify the time we must spend waiting for
the hardware to complete its task. In this case we have a notification
before we start waiting for the panel to change state, but none
afterwards - which would be useful.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Checkpatch tells me
WARNING: __packed is preferred over __attribute__((packed))
so switch over to __packed across the driver before adding new packed
structs.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Check that the N and P dividers don't cause a divide by zero.
This shouldn't happen under normal circumstances, but can
happen eg. under simulation.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
It's a pain for two reasons:
- The vga plane redisablign requires actual legacy vgao i/o to pull
of. The hw engineers really botched this one here :(
- There seem to be some BIOS out there which send out lid events when
unplugging. Together with our broken DP code, which disables the
port when the cable is lost, this results in an immediate modeset
call, which can hang on the wait for outstanding flips.
- Also we don't want to force a modeset on machines where it's not
really needed, see the referenced bug.
We might want to extend this in general to also all machines that
support opregion, since there the BIOS supposedly should manage the
gfx hardware more cooperatively.
v2: Pimp commit message a bit.
Cc: Roland Dreier <roland@kernel.org>
References: https://bugs.freedesktop.org/show_bug.cgi?id=65486
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We're currently misprinting the port name when vlv_wait_port_ready()
times out. Fix it by using port_name().
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We had some mode_valid() vfuncs returning an int, others the enum. Let's
use the latter everywhere.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Atm we call intel_display_power_enabled() from
i915_capture_error_state() in IRQ context and then take a mutex. To fix
this add a new intel_display_power_enabled_sw() which returns the domain
state based on software tracking as opposed to reading the actual HW
state.
Since we use domain_use_count for this without locking on the reader
side make sure we increase the counter only after enabling all required
power wells and decrease it before disabling any of these power wells.
Regression introduced in
commit 1b02383464b4a915627ef3b8fd0ad7f07168c54c
Author: Imre Deak <imre.deak@intel.com>
Date: Tue Sep 24 16:17:09 2013 +0300
drm/i915: support for multiple power wells
Note that atm we depend on the value returned by
intel_display_power_enabled_sw() in i915_capture_error_state() to avoid
unclaimed register access reports. This was never guaranteed though,
since another thread can disable the power concurrently. If this is a
problem we need another explicit way to disable the reporting during
error captures.
v2:
- remove barriers as the caller can't depend on the value
returned from i915_capture_error_state_sw() anyway (Ville)
- dump the state of pipe/transcoder power domain state (Daniel)
Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This should just be a debug. Add another debug msg to the inherit path
while we're at it.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=72098
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Setting this bit restores all ring contexts in parallel rather than
serially. Matches current BWG recommendations.
Tested-by: "Meng, Mengmeng" <mengmeng.meng@intel.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Deepak S <deepak.s@inel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We use timeout mode, and we need to lower the timeout to get good RC6
residency when loads are running. This gets me from 0% residency during
glxgears to 77%, which is a pretty good improvement. This value also
matches the current BWG recommentations.
Tested-by: "Meng, Mengmeng" <mengmeng.meng@intel.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Deepak S <deepak.s@inel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
It leads to a big mess when stuff interleaves. Especially with the new
patch I've submitted for the drm core to no longer artificially split
up debug messages.
v2: The size parameter to snprintf includes the terminating 0, but the
return value does not. Adjust the logic accordingly. Spotted by Mika.
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
It doesn't like that we assign 0 to a pointer, it wants the real NULL.
On closer look that initialization is actually bogus, and the compiler
can easily see that we never use it unitialized. So let's just drop
this.
Cc: Deepak S <deepak.s@intel.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This was fumbled in the conversion to per-engine forcewake.
Cc: Deepak S <deepak.s@intel.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
On VLV the GTFIFOCTL register has other bits besides the number of free
entries in the GT wake FIFO. Apply a mask when we read th register to
make sure we don't misinterpret the number of free FIFO entries.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
[danvet: There's some unclarity about hsw, but brushed off as todays'
Bspec just acting up a bit.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
On VLV GTFIFODBG has more bits. Just report them all.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Forcewake counts for valleyview are not exposed throgh DebugFS.
Exposing with this change.
Signed-off-by: Deepak S <deepak.s@intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Split vlv force wake routines to help individually control Media/Render
well based on the register access.
We've seen power savings in the lower sub-1W range on workloads that
only need on of the power wells, e.g. glbenchmark, media playback
Note: The same split isn't there for the forcewake queue, only the
forcwake domains are split.
Signed-off-by: Deepak S <deepak.s@intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
[danvet: Rebase on top of the removed forcewake hack in the ring irq
get/put code and add a note to add Deepak's answer to Chris question.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Added power well arguments to all the force wake routines
to help us individually control power well based on the
scenario.
Signed-off-by: Deepak S <deepak.s@intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
[danvet: Resolve conflict with the removed forcewake hack and drop one
spurious hunk Jesse noticed.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Due to user fudging (for instance using video=VGA-1:e with FBDEV=n) we can
attempt to reset an inconsistent CRTC that is marked as active but has
no assigned fb. It would be wise to fix this earlier, but the long
term plan is to have primary and secondary planes associated with a
CRTC, in which crtc->fb being NULL will be expected. So for a quick
short term fix with pretensions of grandeur, just check for a NULL fb
during GPU reset and ignore the plane restoration.
This fixes a potential hard hang (a panic in the panic handler)
following a GPU hang.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
[danvet: Add a corresponding fixme comment.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Add a debugfs entry showing the use-count for all power domains of each
power well.
v3: address comments from Paulo:
- simplify power_domain_str() by using a switch table
- move power_well::domain_count to power_domains
- WARN_ON decrementing a 0 refcount
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Paulo Zanoni <paulo.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
So far we distinguished platforms without a dynamic power well with
the HAS_POWER_WELL macro and for such platforms we didn't call any power
domain functions. Instead of doing this check we can add an always-on
power well for these platforms and call the power domain functions
unconditionally. For always-on power wells we only increase/decrease
their refcounts, otherwise they are nop.
This makes high level driver code more readable and as a bonus provides
some idea of the current power domains state for all platforms (once
the relevant debugfs entry is added).
v3: rename intel_power_wells to i9xx_always_on_power_well (Paulo)
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Paulo Zanoni <paulo.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Paulo Zanoni <paulo.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This may need work if other platforms do the same thing, but in the
meantime we should avoid looking at HSW specific bits in this generic
function.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
[added IS_BROADWELL too as that needs the same handling (Imre)]
Signed-off-by: Imre Deak <imre.deak@intel.com>
[danvet: Add Imre's missing sob.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
In intel_display_capture_error_state we use HAS_POWER_WELL to check if
we are running on Haswell/Broadwell when accessing HSW_PWR_WELL_DRIVER
which is specific to these platforms. Future platforms with power wells
don't have this register, so HAS_POWER_WELL won't work there any more.
Use IS_HASWELL/IS_BROADWELL instead.
v3: fix using logical || instead of bitwise | (Paulo)
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Paulo Zanoni <paulo.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Instead of using a separate function to check whether a power domain is
is always on, add an always-on power well covering all these power
domains and do the usual get/put on these unconditionally. Since we
don't assign a .set handler for these the get/put won't have any effect
besides the adjusted refcount.
This makes the code more readable and provides debug info also on the
use of always-on power wells (once the relevant debugfs entry is added.)
v3: make is_always_on to be bool instead of a bit field (Paulo)
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Paulo Zanoni <paulo.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
HW generations so far had only one always-on power well and optionally
one dynamic power well. Upcoming HW gens may have multiple dynamic power
wells, so add some infrastructure to support them.
The idea is to keep the existing power domain API used by the rest of
the driver and create a mapping between these power domains and the
underlying power wells. This mapping can differ from one HW to another
but high level driver code doesn't need to know about this. Through the
existing get/put API it would just ask for a given power domain and the
power domain framework would make sure the relevant power wells get
enabled in the right order.
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Paulo Zanoni <paulo.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This way the code is simpler and can also be used for other platforms
where the audio power domain->power well mapping is different.
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Paulo Zanoni <paulo.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
If we use a stolen buffer, our probe callback shouldn't allocate a new
buffer; we should re-use the one from the BIOS instead if possible.
v2: fix locking (Jesse)
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
I believe, and an evening of i-g-t, that our original workaround for the
missed interrupts on Sandybridge, that of holding forcewake whilst we
wait for an interrupts, is no longer required. This leaves us dependent
on the second workaround of forcing an UC read of the ACTHD before
reading back the seqno from the snooped HWS. Dropping the forcewake
should allow us to conserve a little power, not much as the GPU is meant
to be busy whilst we wait for it!
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Our VM code already has a cleanup function, and this is a nice place to
put the drm_mm_takedown. This should have no functional impact, it just
leaves the unload function a bit cleaer, and is more logical IMO
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This belonged in
commit 07fe0b1280
Author: Ben Widawsky <ben@bwidawsk.net>
Date: Wed Jul 31 17:00:10 2013 -0700
drm/i915: plumb VM into bind/unbind code
But it was somehow missed along the way.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
i915_gem_execbuffer_relocate became defunct in:
commit 27173f1f95
Author: Ben Widawsky <ben@bwidawsk.net>
Date: Wed Aug 14 11:38:36 2013 +0200
drm/i915: Convert execbuf code to use vmas
eb_create: never used?
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
[danvet: The lingering vm parameter to eb_create might have been back
from the days where we didn't yet keep both vmas and obj lists in the
eb struct. But I didn't check really.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Many tests call this ad naseum now (in an infinite loop, very often).
It clutters the logs. Actually, I'd rather drop it completely...
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This would have never worked.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This should really have been added in BDW integration, as well as:
commit 93bd8649db
Author: Ben Widawsky <ben@bwidawsk.net>
Date: Tue Jul 16 16:50:06 2013 -0700
drm/i915: Put the mm in the parent address space
It didn't really matter before, but it will in the future.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
When we fail for some reason on loading the PDPs, it would be wise to
disable the PPGTT in the ring registers. If we do not do this, we have
undefined results.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
SNB has another register where the actual FBC CPU fence number is
stored. The documenation explicitly states that the fence number
in DPFC_CTL must be 0 on SNB. And in fact when it's not zero,
the GTT tracking simply doesn't work.
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>
Oops, makes testing early boot failures in i915.ko a bit more pain, so
let's fix it.
v2: We already have a bit of static storage to track this (Chris).
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
gcc complains that:
drivers/gpu/drm/i915/i915_debugfs.c: In function ‘display_crc_ctl_write’:
drivers/gpu/drm/i915/i915_debugfs.c:2393:2: warning: ‘val’ may be used uninitialized in this function [-Wuninitialized]
drivers/gpu/drm/i915/i915_debugfs.c:2350:6: note: ‘val’ was declared here
but it can't see that we're going to use val only in the success case.
So shut it up.
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: David Airlie <airlied@linux.ie>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We need to hold the pc8 lock around toggling the value of gpu_idle.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We send the primary and cursor plane data through the gamma unit.
In order to get matching output from sprites, also send the sprite
data through the gamma unit.
In the future we should add some properties to control this
explicitly, and also add properties for the per-sprite gamma ramps
what have you, but for now this seems like a reasonable thing to do.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
All the other .enable_fbc() funcs use plane_name(). Make
gen7_enable_fbc() do the same.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
At least since SNB (perhaps even earlier) even the desktop parts
should have FBC.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The spec tells us that we need to emit an SRM after the LRI
to MSG_FBC_REND_STATE.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Don't issue the FBC nuke/cache clean command when invalidate_domains!=0.
That would indicate that we're not being called for the post-batch
flush.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
