Based on the HW team inputs. We can should not wait for the old ack,
Waiting for old ack might fail, when other forcewake came before the
present one is desserted.
for example, if forcewake bit 0 was set and before it could get cleared
forcewake bit 1 got set, HW eventually clear bit 0, when the bit 1
is cleared. i.e, bit 1 is still sent then forcewake bit 0 will still be
set.
Signed-off-by: Deepak S <deepak.s@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
[danvet: Add comment Ville requested.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
IS_GEN8() is a bad check in the forcewake code due to bdw vs. chv
differences. Use IS_BROADWELL() instead.
The only actual bug here is that we currently call
__gen7_gt_force_wake_mt_reset() on chv. On the other places we
have checked for chv before using IS_GEN8(), but change them
to use IS_BROADWELL() anyway to reduce the chance of accidents in the
future.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Deepak S <deepak.s@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
By the time I wrote this patch, it allowed me to catch some problems.
But due to patch reordering - in order to prevent fake "regression"
reports - this patch may be merged after the fixes of the problems
identified by this patch.
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The current code only runs when we do an I915_WRITE operation. It
checks if the unclaimed register flag is set before we do the
operation, and then it checks it again after we do the operation. This
double check allows us to find out if the I915_WRITE operation in
question is the bad one, or if some previous code is the bad one. When
it finds a problem, our code uses DRM_ERROR to signal it.
The good thing about the current code is that it detects the problem,
so at least we can know we did something wrong. The problem is that
even though we find the problem, we don't really have much information
to actually debug it. So whenever I see one of these DRM_ERROR
messages on my systems, the first thing I do is apply a patch to
change the DRM_ERROR to a WARN and also check for unclaimed registers
on I915_READ operations. This local patch makes things even slower,
but it usually helps a lot in finding the bad code.
The first point here is that since the current code is only useful to
detect whether we have a problem or not, but it is not really good to
find the cause of the problem, I don't think we should be checking
both before and after every I915_WRITE operation: just doing the check
once should be enough for us to quickly detect problems. With this
change, the code that runs by default for every single user will only
do 1 read operation for every single I915_WRITE, instead of 2. This
patch does this change.
The second point is that the local patch I have should be upstream,
but since it makes things slower it should be disabled by default. So
I added the i915.mmio_debug option to enable it.
So after this patch, this is what will happen:
- By default, we will try to detect unclaimed registers once after
every I915_WRITE operation. Previously we tried twice for every
I915_WRITE.
- When we find an unclaimed register we will still print a DRM_ERROR
message, but we will now tell the user to try again with
i915.mmio_debug=1.
- When we use i915.mmio_debug=1 we will try to find unclaimed
registers both before and after every I915_READ and I915_WRITE
operation, and we will print stack traces in case we find them.
This should really help locating the exact point of the bad code
(or at least finding out that i915.ko is not the problem).
This commit also opens space for really-slow register debugging
operations on other platforms. In theory we can now add lots and lots
of debug code behind i915.mmio_debug, enable this option on our tests,
and catch more problems.
v2: - Remove not-so-useful comments (Daniel)
- Fix the param definition macros (Rodrigo)
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This is an Execlists preparatory patch, since they make context ID become an
overloaded term:
- In the software, it was used to distinguish which context userspace was
trying to use.
- In the BSpec, the term is used to describe the 20-bits long field the
hardware uses to it to discriminate the contexts that are submitted to
the ELSP and inform the driver about their current status (via Context
Switch Interrupts and Context Status Buffers).
Initially, I tried to make the different meanings converge, but it proved
impossible:
- The software ctx->id is per-filp, while the hardware one needs to be
globally unique.
- Also, we multiplex several backing states objects per intel_context,
and all of them need unique HW IDs.
- I tried adding a per-filp ID and then composing the HW context ID as:
ctx->id + file_priv->id + ring->id, but the fact that the hardware only
uses 20-bits means we have to artificially limit the number of filps or
contexts the userspace can create.
The ctx->user_handle renaming bits are done with this Cocci patch (plus
manual frobbing of the struct declaration):
@@
struct intel_context c;
@@
- (c).id
+ c.user_handle
@@
struct intel_context *c;
@@
- (c)->id
+ c->user_handle
Also, while we are at it, s/DEFAULT_CONTEXT_ID/DEFAULT_CONTEXT_HANDLE and
change the type to unsigned 32 bits.
v2: s/handle/user_handle and change the type to uint32_t as suggested by
Chris Wilson.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> (v1)
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Jesse's SOix work required some patches from acpi-next, so pull it in
through a topic barnch.
Conflicts:
drivers/gpu/drm/i915/i915_drv.c
drivers/gpu/drm/i915/intel_pm.c
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Atm, the forcewake refcount will be incorrectly set to zero during
system suspend if there is any reference held via the
i915_forcewake_user debugfs entry.
Fix this by simply not zeroing the sw counters during suspend and
restoring the original state using them. Note that the only other
places where we zeroed the counters were driver load and unload time,
where it was redundant anyway.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78059
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
If the timer putting the last forcewake refcount was pending and we
canceled it, we'll leak the corresponding forcewake and RPM references.
v2:
- do the ptr casting at the caller instead of adding a separate helper
for this (Chris)
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This allows the system to enter the lowest power mode during system freeze.
v2: delete force wake timer at suspend (Imre)
v3: add GT work suspend function (Imre)
v4: use uncore forcewake reset (Daniel)
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Replaced ever growing switch for gen version with chained conditionals.
Futre gen's only need to add a new one if they require something different.
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Robert Beckett <robert.beckett@intel.com>
[danvet: Picked from internal tree and white-wash commit message.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
CHV uses the gen8 shadow register mechanism so we shouldn't be
checking the GT FIFO status.
This effectively removes the posting read, so add an explicit
posting read using FORCEWAKE_ACK_VLV (which is what use in
vlv_forcewake_reset()).
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Skip __gen6_gt_wait_for_thread_c0() on CHV.
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Support to individually control Media/Render well based on the register access.
Add CHV specific write function to habdle difference between registers
that are sadowed vs those that need forcewake even for writes.
Streamline the CHV forcewake functions just like was done for VLV.
This will also fix a bug in accessing the common well registers,
where we'd end up trying to wake up the wells too many times
since we'd call force_wake_get/put twice per register access, with
FORCEFAKE_ALL both times.
v2: Drop write FIFO for CHV and add comman well forcewake (Ville)
Re-factor CHV/VLV Forcewake offsets (Ben)
v3: Fix for decrementing fw count in chv read/write. (Deepak)
v4: Squash the patches (Mika)
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
[vsyrjala: Move the register range macros into intel_uncore.c]
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Deepak S <deepak.s@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Bunch of stuff for 3.16 still:
> - Mipi dsi panel support for byt. Finally! From Shobhit&others. I've
> squeezed this in since it's a regression compared to vbios and we've
> been ridiculed about it a bit too often ...
> - connection_mutex deadlock fix in get_connector (only affects i915).
> - Core patches from Matt's primary plane from Matt Roper, I've pushed the
> i915 stuff to 3.17.
> - vlv power well sequencing fixes from Jesse.
> - Fix for cursor size changes from Chris.
> - agpbusy fixes from Ville.
> - A few smaller things.
>
* tag 'drm-intel-fixes-2014-06-06' of git://anongit.freedesktop.org/drm-intel: (32 commits)
drm/i915: BDW: Adding missing cursor offsets.
drm: Fix getconnector connection_mutex locking
drm/i915/bdw: Only use 2g GGTT for 32b platforms
drm/i915: Nuke pipe A quirk on i830M
drm/i915: fix display power sw state reporting
drm/i915: Always apply cursor width changes
drm/i915: tell the user if both KMS and UMS are disabled
drm/plane-helper: Add drm_plane_helper_check_update() (v3)
drm: Check CRTC compatibility in setplane
drm/i915: use VBT to determine whether to enumerate the VGA port
drm/i915: Don't WARN about ring idle bit on gen2
drm/i915: Silence the WARN if the user tries to GTT mmap an incoherent object
drm/i915: Move the C3 LP write bit setup to gen3_init_clock_gating() for KMS
drm/i915: Enable interrupt-based AGPBUSY# enable on 85x
drm/i915: Flip the sense of AGPBUSY_DIS bit
drm/i915: Set AGPBUSY# bit in init_clock_gating
drm/i915/vlv: add pll assertion when disabling DPIO common well
drm/i915/vlv: move DPIO common reset de-assert into __vlv_set_power_well
drm/i915/vlv: re-order power wells so DPIO common comes after TX
drm/i915/vlv: move CRI refclk enable into __vlv_set_power_well
...
Merge drm-fixes into drm-next.
Both i915 and radeon need this done for later patches.
Conflicts:
drivers/gpu/drm/drm_crtc_helper.c
drivers/gpu/drm/i915/i915_drv.h
drivers/gpu/drm/i915/i915_gem.c
drivers/gpu/drm/i915/i915_gem_execbuffer.c
drivers/gpu/drm/i915/i915_gem_gtt.c
We do this at runtime and later on now.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Ville figured out that it needs a full display reset since apparently
a lot more goes down than just the GT. Until that's address it's
better to just diable gpu reset.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Up until now, contexts had one (and only one) backing object that was
used by the hardware to save/restore render ring contexts (via the
MI_SET_CONTEXT command). Other rings did not have or need this, so
our i915_hw_context struct had a 1:1 relationship with a a real HW
context.
With Logical Ring Contexts and Execlists, this is not possible anymore:
all rings need a backing object, and it cannot be reused. To prepare
for that, rename our contexts to the more generic term intel_context.
No functional changes.
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Apparently we need to disable VCP unit clock gating around media reset
on g4x.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Clear the reset domain after a succesful GPU reset on ilk. We already
do that on gen4, so let's try to be a bit more consistent. And if
ether render or media reset fails, we might use the leftover value
in the register to pinpoint the culprit.
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>
All the other bits in the GDSR register are read-only, so we don't have
to preserve them when we perform a GPU reset.
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>
We're using the reset domains bits for g4x on ilk. But on ilk those bits
actually shifted by one bit. Fix it up so that we use the correct bits.
We were actually always writing 0x2 to the reset domain bits, which
is a reserved value. In practice it looks like the hardware ignores that
value since nothing happens if I write that value when there's a 3D
workload running. Writing the _correct_ render domain value actually
makes the GPU stop.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We should be waiting for the reset bit to clear, not remain set.
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>
There are comments in the gen4-5 reset functions stating that we can't
reset render and media without also doing a display reset. But that's
exactly what the code does, ie. we don't perform a display reset. Drop
the bogus comments.
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>
Media force wake get hangs the machine when the system is booted without
displays attached. The assumption is that (at least some versions of)
the firmware has skipped some initialization in that case.
Empirical evidence suggests we need to reset the media force wake
request register in addition to the render one to avoid hangs.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75895
Reported-by: Imre Deak <imre.deak@intel.com>
Reported-by: Darren Hart <dvhart@linux.intel.com>
Tested-by: Darren Hart <dvhart@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: stable@vger.kernel.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
The same register exists for querying and programming eDRAM AKA eLLC. So
we can simply use it. For now, use all the same defaults as we had
for Haswell, since like Haswell, I have no further details.
I do not actually have a part with eDRAM, so I cannot test this.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Reviewed-by: Brad Volkin <bradley.d.volkin@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The reg_read whitelist has a gen bitmask to code the gens we're allowing
the register to be read on. Until now, it was a literal, but we can be
a bit more expressive.
To ease the review, a small test program:
$ cat bit-range.c
#include <stdio.h>
#include <stdint.h>
#define U32_C(x) x ## U
#define GENMASK(h, l) (((U32_C(1) << ((h) - (l) + 1)) - 1) << (l))
#define GEN_RANGE(l, h) GENMASK(h, l)
int main(int argc, char **argv)
{
printf("0x%08x\n", GEN_RANGE(1, 1));
printf("0x%08x\n", GEN_RANGE(1, 2));
printf("0x%08x\n", GEN_RANGE(4, 4));
printf("0x%08x\n", GEN_RANGE(4, 5));
printf("0x%08x\n", GEN_RANGE(1, 31));
printf("0x%08x\n", GEN_RANGE(4, 8));
return 0;
}
$ ./bit-range
0x00000002
0x00000006
0x00000010
0x00000030
0xfffffffe
0x000001f0
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
These defines are only used in intel_uncore.c.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
That function isn't used outside this file anymore.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
To avoid WARNs when we call it.
Testcase: igt/pm_pc8/reg-read-ioctl
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75693
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
So far force_wake_timer was only used by gen6_gt_force_wake_put. Since
we always had balanced gen6_gt_force_wake_get/put calls, we could
guarantee balanced calls to intel_runtime_pm_get/put.
Commit 8232644ccf, "drm/i915: Convert
the forcewake worker into a timer func" started scheduling the
force_wake_timer at gen6_read, which resulted in an unbalanced
runtime_pm refcount.
So this commit just reverts to the old behavior until we can find a
proper way to used delayed force_wake from the register read/write
macros without leaving the runtime_pm refcounts unbalanced and without
runtime suspending the driver while forcewake is active.
Testcase: igt/pm_pc8/rte
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76544
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Without this the new drv_suspend/forcewake subtest I've created
doesn't result in immediately visible failures.
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This is a regression introduced in
commit 0294ae7b44
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Thu Mar 13 12:00:29 2014 +0000
drm/i915: Consolidate forcewake resetting to a single function
The reordered setup sequence ended up calling del_timer_sync before
the timer was set up correctly, resulting in endless hilarity when
loading the driver.
Compared to Ben's patch (which moved around the setup_timer call to
sanitize_early) this moves the sanitize_early call around in the
driver load call. This way we avoid calling setup_timer again in the
resume code (where we also call sanitize_early).
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Tested-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76242
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We have two paths that try to reset the forcewake registers back to
known good values, with slightly different semantics and levels of
paranoia. Combine the two by passing a parameter to either restore the
forcewake status or to clear our bookkeeping, and raise the paranoia
level to max.
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>
Now that we regularly defer the forcewake dance to a timer func, it is
likely to fire after we disable the device during suspend. This
generates an oops as we detect inconsistency in the hardware state. So
before suspend, we want to complete the outstanding dance and generally
sanitize the registers before handing back to the BIOS.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
In the upcoming patches we'll need to access the rest of the fields in
the punit power gating register, so prepare for that.
v2:
- add doc reference for the power well subsystem IDs (Jesse)
- remove IDs for non-existant DPIO_RX[23] subsystems (Jesse)
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
As we now have intel_uncore_forcewake_reset() no need
to do explicit put after reset.
v2: rebase
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
as they don't exists.
v2: rename gen6_*_mt_* to gen7_*_mt_* as they never get called
with gen6 (Chris)
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
When we get control from BIOS there might be mt forcewake
bits already set. This causes us to do double mt get
without proper clear/ack sequence.
Fix this by clearing mt forcewake register on init,
like we do with older gens.
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
It occured to me that when we're trying to wake up both render
and media wells on VLV, we might end up calling the low level
force_wake_get/put two times even though one call would be
enough. Make that happen by figuring out which wells really
need to be woken up based on the forcewake counts.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by:Deepak S <deepak.s@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
VLV is the only platform where we increment/decrement the forcewake
count around register access. Drop the inc/dec on VLV to make the
forcewake code a bit more unified.
The inc/dec are not necessary since we hold the uncore lock around
the whole operation.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Deepak S <deepak.s@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Use the render/media specific forcewake counts to properly restore the
forcewake status after a GPU reset on VLV.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Deepak S <deepak.s@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
I could swear this was already happening in the current code...
Also, put the reads and writes in a generic place, so we don't forget
it again when we add runtime PM support to new platforms.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Just to be sure...
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Because we shouldn't be runtime suspended when forcewake is supposed
to be enabled.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
[danvet: Update commit message - no WARN expected since the bugfix for
issues hit with this assert is already in. And resolve conflicts with
the change from worker to timer for the delayed fw release.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
When we call gen6_gt_force_wake_put we don't actually put force_wake,
we just schedule gen6_force_wake_work through mod_delayed_work, and
that will eventually release force_wake.
The problem is that we call intel_runtime_pm_put directly at
gen6_gt_force_wake_put, so most of the times we put our runtime PM
reference before the delayed work happens, so we may runtime suspend
while force_wake is still supposed to be enabled if the graphics
autosuspend_delay_ms is too small.
Now the nice thing about the current code is that after it triggers
the delayed work function it gets a refcount, and it only triggers the
delayed work function if refcount is zero. This guarantees that when
we schedule the funciton, it will run before we try to schedule it
again, which simplifies the problem and allows for the current
solution to work properly (hopefully!).
v2: - Keep the VLV refcounts balanced (Jesse)
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We don't want to suffer scheduling delay when turning off the GPU after
waking it up to touch registers. Ideally, we only want to keep the GPU
awake for the register access sequence, with a single forcewake dance on
the first access and release immediately after the last. We set a timer
on the first access so that we only dance once and on the next scheduler
tick, we drop the forcewake again.
This moves the cleanup routine from the common i915 workqueue to a timer
func so that we don't anger powertop, and drop the forcewake again
quicker.
v2: Enable the deferred force_wake_put for regular register reads as
well.
v3: Beautification and make sure we disable forcewake when shutting
down.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Sometimes generic driver code gets forcewake explicitly by
gen6_gt_force_wake_get(), which check forcewake_count before accessing
hardware. However the register access with gen8_write function access
low level hw accessors directly, ignoring the forcewake_count. This
leads to nested forcewake get from hardware, in ring init and possibly
elsewhere, causing forcewake ack clear errors and/or hangs.
Fix this by checking the forcewake count also in gen8_write
v2: Read side doesn't care about shadowed registers,
Remove __needs_put funkiness from gen8_write. (Ville)
Improved commit message.
References: https://bugs.freedesktop.org/show_bug.cgi?id=74007
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Because whatever.*
* This should contain a fairly long list of issues and still
unresolved resgressions, but I didn't really get a vote.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Nothing's changed here; we just need to bump the generation check.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Deepak S