There is a general theory that kzmalloc is better/safer than kmalloc, especially
for interesting data structures. This change updates the request structure
allocation to be zero filled.
This also fixes crashes in the reset code. Quoting Mika's patch:
"Clean the request structure on alloc. Otherwise we might end up
referencing uninitialized fields. This is apparent when we try to
cleanup the preallocated request on ring reset, before any request has
been submitted to the ring. The request->ctx is foobar and we end up
freeing the foobarness."
Note that this fixes a regression introduced in
commit 9eba5d4a1d
Author: John Harrison <John.C.Harrison@Intel.com>
Date: Mon Nov 24 18:49:23 2014 +0000
drm/i915: Ensure OLS & PLR are always in sync
References: https://bugs.freedesktop.org/show_bug.cgi?id=86959
References: https://bugs.freedesktop.org/show_bug.cgi?id=86962
References: https://bugs.freedesktop.org/show_bug.cgi?id=86992
Change-Id: I68715ef758025fab8db763941ef63bf60d7031e2
For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Thomas Daniel <Thomas.Daniel@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Now that sanity prevails and we have the clean split between software
init and starting the engines we can drop all the "have we allocate
this struct already?" nonsense.
Execlist code could benefit quite a bit more still, but that's for
another patch.
Reviewed-by: Dave Gordon <david.s.gordon@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
We can do this.
And now there's finally the clean split between software setup and
hardware setup I kinda wanted since multi-ring support was merged
aeons ago. It only took almost 5 years.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Dave Gordon <david.s.gordon@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
With this all the ->init_hw hooks really only set up hw state needed
to start the ring, all the software state setup and memory/buffer
allocations happen beforehand.
v2: We need to call intel_init_pipe_control after the ring init since
otherwise engine->dev is NULL and it falls over. Currently that's
now after the hw ring is enabled but a) we'll be fine as long as no
one submits a batch b) this will change soon.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Dave Gordon <david.s.gordon@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This is (mostly, some exceptions that need fixing) the hw setup
function which starts the ring. And not the function which allocates
all the resources.
Make this clear by giving it a better name.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Dave Gordon <david.s.gordon@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
There are numerous places in the code where the driver's idea of
how much space is left in a ring is updated using the driver's
latest notions of the positions of 'head' and 'tail' for the ring.
Among them are some that update one or both of these values before
(re)doing the calculation. In particular, there are four different
places in the code where 'last_retired_head' is copied to 'head'
and then set to -1; and two of these do not have a guard to check
that it has actually been updated since last time it was consumed,
leaving the possibility that the dummy -1 can be transferred from
'last_retired_head' to 'head', causing the space calculation to
produce 'impossible' results (previously seen on Android/VLV).
This code therefore consolidates all the calculation and updating of
these values, such that there is only one place where the ring space
is updated, and it ALWAYS uses (and consumes) 'last_retired_head' if
(and ONLY if) it has been updated since the last call.
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The used space in a ring is given by the cyclic distance from the
consumer (HEAD) to the producer (TAIL), i.e. ((tail-head) MOD size);
conversely, the available space in a ring is the cyclic distance
from the producer to the consumer, MINUS the amount reserved for a
"gap" that is supposed to guarantee that the producer never catches
up with or overruns the consumer. Note that some GEN h/w requires
that TAIL never approach to within one cacheline of HEAD, so the gap
is usually set to twice the cacheline size to ensure this.
While the existing code gives the correct answer for correct inputs,
if the producer HAS overrun into the reserved space, the result can
be a value larger than the maximum valid value (size-reserved). We
can improve this by reorganising the calculation, so that in the
event of overrun the result will be negative rather than over-large.
This means that the commonly-used test (available >= required)
will then reject further writes into the ring after an overrun,
giving some chance that we can recover from or at least diagnose
the original problem; whereas allowing more writes would likely both
confuse the h/w and destroy the evidence of what went wrong.
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
It makes a lot more sense (and makes future seqno -> request conversion patches
simpler) to fill in the 'ring' field of the request structure at the point of
creation rather than submission. Given that the request structure is assigned by
ring specific code and thus is locked to a ring from the start, there really is
no reason to defer this assignment.
For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Thomas Daniel <Thomas.Daniel@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
There is no longer any need to retrieve a seqno value from an i915_add_request()
call. The calling code already knows which request structure is being processed
(it can only be ring->OLR). And as the request itself is now used in preference
to the basic seqno value, the latter is now redundant in this situation.
For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Thomas Daniel <Thomas.Daniel@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Updated i915_wait_seqno() to take a request structure instead of a seqno value
and renamed it accordingly. Internally, it just pulls the seqno out of the
request and calls on to __wait_seqno() as before. However, all the code further
up the stack is now simplified as it can just pass the request object straight
through without having to peek inside.
For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Thomas Daniel <Thomas.Daniel@intel.com>
[danvet: Squash in hunk from an earlier patch which was rebased
wrongly.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The OLS value is now obsolete. Exactly the same value is guarateed to be always
available as PLR->seqno. Thus it is safe to remove the OLS completely. And also
to rename the PLR to OLR to keep the 'outstanding lazy ...' naming convention
valid.
For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Thomas Daniel <Thomas.Daniel@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The plan is to use request structures everywhere that seqno values were
previously used. This means saving pointers to structures in places that used to
be simple integers. In turn, that means that the target structure now needs much
more stringent lifetime tracking. That is, it must not be freed while some other
random object still holds a pointer to it.
To achieve this tracking, a reference count needs to be added. Whenever a
pointer to the structure is saved away, the count must be incremented and the
free must only occur when all references have been released.
For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Thomas Daniel <Thomas.Daniel@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The aim is to replace seqno values with request structures. A step along the way
is to switch to using the PLR in preference to the OLS. That requires the PLR to
only be valid when and only when the OLS is also valid. I.e., the two must be
kept in lock step. Then, code which was using the OLS can be safely switched
over to using the PLR instead.
For: VIZ-4377
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Thomas Daniel <Thomas.Daniel@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
With the deprecation of UMS, and by association DRI1, we have a tough
choice when updating the ring access routines. We either rewrite the
DRI1 routines blindly without testing (so likely to be broken) or take
the liberty of declaring them no longer supported and remove them
entirely. This takes the latter approach.
v2: Also remove the DRI1 sarea updates
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
[danvet: Fix rebase conflicts.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Same as with the context, pinning to GGTT regardless is harmful (it
badly fragments the GGTT and can even exhaust it).
Unfortunately, this case is also more complex than the previous one
because we need to map and access the ringbuffer in several places
along the execbuffer path (and we cannot make do by leaving the
default ringbuffer pinned, as before). Also, the context object
itself contains a pointer to the ringbuffer address that we have to
keep updated if we are going to allow the ringbuffer to move around.
v2: Same as with the context pinning, we cannot really do it during
an interrupt. Also, pin the default ringbuffers objects regardless
(makes error capture a lot easier).
v3: Rebased. Take a pin reference of the ringbuffer for each item
in the execlist request queue because the hardware may still be using
the ringbuffer after the MI_USER_INTERRUPT to notify the seqno update
is executed. The ringbuffer must remain pinned until the context save
is complete. No longer pin and unpin ringbuffer in
populate_lr_context() - this transient address is meaningless and the
pinning can cause a sleep while atomic.
v4: Moved ringbuffer pin and unpin into the lr_context_pin functions.
Downgraded pinning check BUG_ONs to WARN_ONs.
v5: Reinstated WARN_ONs for unexpected execlist states. Removed unused
variable.
Issue: VIZ-4277
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
Reviewed-by: Akash Goel <akash.goels@gmail.com>
Reviewed-by: Deepak S<deepak.s@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Following the legacy ring submission example, update the
ring->init_context() hook to support the execlist submission mode.
v2: update to use the new workaround macros and cleanup unused code.
This takes care of both bdw and chv workarounds.
v2.1: Add missing call to init_context() during deferred context creation.
v3: Split init_context (emit) in legacy/lrc modes. For lrc, get the ringbuf
from the context (Mika/Daniel).
v4: Merge init_context interfaces back, the legacy mode only needs the ring,
but the lrc mode needs the ring and context (Mika).
Issue: VIZ-4092
Issue: GMIN-3475
Change-Id: Ie3d093b2542ab0e2a44b90460533e2f979788d6c
Cc: Deepak S <deepak.s@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
[danvet: Align function paramater lists properly.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-WaDisableDopClockGating:chv
-WaDisableSamplerPowerBypass:chv
-WaDisableGunitClockGating:chv
-WaDisableFfDopClockGating:chv
-WaDisableDopClockGating:chv
v2: Remove pre-production WA instead of restricting them
based on revision id (Ville)
For: VIZ-4090
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>
If a ring failed to initialise for any reason then the error path would try to
clean up all rings including those that had not yet been allocated. The ring
clean up code did a check that the ring was valid before starting its work.
Unfortunately, that was after it had already dereferenced the ring to obtain a
dev_private pointer.
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
- suspend/resume/freeze/thaw unification from Imre
- wa list improvements from Mika&Arun
- display pll precomputation from Ander Conselvan, this removed the last
->mode_set callbacks, a big step towards implementing atomic modesets
- more kerneldoc for the interrupt code
- 180 rotation for cursors (Ville&Sonika)
- ULT/ULX feature check macros cleaned up thanks to Damien
- piles and piles of fixes all over, bug team seems to work!
* tag 'drm-intel-next-2014-10-24' of git://anongit.freedesktop.org/drm-intel: (61 commits)
drm/i915: Update DRIVER_DATE to 20141024
drm/i915: add comments on what stage a given PM handler is called
drm/i915: unify switcheroo and legacy suspend/resume handlers
drm/i915: add poweroff_late handler
drm/i915: sanitize suspend/resume helper function names
drm/i915: unify S3 and S4 suspend/resume handlers
drm/i915: disable/re-enable PCI device around S4 freeze/thaw
drm/i915: enable output polling during S4 thaw
drm/i915: check for GT faults in all resume handlers and driver load time
drm/i915: remove unused restore_gtt_mappings optimization during suspend
drm/i915: fix S4 suspend while switcheroo state is off
drm/i915: vlv: fix switcheroo/legacy suspend/resume
drm/i915: propagate error from legacy resume handler
drm/i915: unify legacy S3 suspend and S4 freeze handlers
drm/i915: factor out i915_drm_suspend_late
drm/i915: Emit even number of dwords when emitting LRIs
drm/i915: Add rotation support for cursor plane (v5)
drm/i915: Correctly reject invalid flags for wait_ioctl
drm/i915: use macros to assign mmio access functions
drm/i915: only run hsw_power_well_post_enable when really needed
...
Ok, new attempt, this time around with full ppgtt disabled again.
drm-intel-next-2014-10-03:
- first batch of skl stage 1 enabling
- fixes from Rodrigo to the PSR, fbc and sink crc code
- kerneldoc for the frontbuffer tracking code, runtime pm code and the basic
interrupt enable/disable functions
- smaller stuff all over
drm-intel-next-2014-09-19:
- bunch more i830M fixes from Ville
- full ppgtt now again enabled by default
- more ppgtt fixes from Michel Thierry and Chris Wilson
- plane config work from Gustavo Padovan
- spinlock clarifications
- piles of smaller improvements all over, as usual
* tag 'drm-intel-next-2014-10-03-no-ppgtt' of git://anongit.freedesktop.org/drm-intel: (114 commits)
Revert "drm/i915: Enable full PPGTT on gen7"
drm/i915: Update DRIVER_DATE to 20141003
drm/i915: Remove the duplicated logic between the two shrink phases
drm/i915: kerneldoc for interrupt enable/disable functions
drm/i915: Use dev_priv instead of dev in irq setup functions
drm/i915: s/pm._irqs_disabled/pm.irqs_enabled/
drm/i915: Clear TX FIFO reset master override bits on chv
drm/i915: Make sure hardware uses the correct swing margin/deemph bits on chv
drm/i915: make sink_crc return -EIO on aux read/write failure
drm/i915: Constify send buffer for intel_dp_aux_ch
drm/i915: De-magic the PSR AUX message
drm/i915: Reinstate error level message for non-simulated gpu hangs
drm/i915: Kerneldoc for intel_runtime_pm.c
drm/i915: Call runtime_pm_disable directly
drm/i915: Move intel_display_set_init_power to intel_runtime_pm.c
drm/i915: Bikeshed rpm functions name a bit.
drm/i915: Extract intel_runtime_pm.c
drm/i915: Remove intel_modeset_suspend_hw
drm/i915: spelling fixes for frontbuffer tracking kerneldoc
drm/i915: Tighting frontbuffer tracking around flips
...
The number of DWords should be even when doing ring emits as
command sequences require QWord alignment.
There was some discussion about the maximum length of the MI_LRI
command. Quoting Mika
"I did some test with bdw:
"The maximum is 128 writes, resulting the 8 bit length
field of the command being 0xff, thus following the spec.
The 128'th write went through.
"Perhaps the max command length is then less in older gens?
"Perhaps WARN_ON(x > 128) in MI_LOAD_REGISTER_IMM would be in place
but one needs minor tweak to command parser a bit also then.
#define I915_MAX_WA_REGS 16
keeps us safe for now atleast."
Ville commented that on pre-gen6 the length field seems to be
restricted to 0x3f though. So for all cases we should be ok.
v2: user LRI variant that can write multiple regs in one go (Damien).
We can simply insert one NOP at the end instead of one per register write.
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
[danvet: Add a summary of the MI_LRI length discussion.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
If we build the workaround list in ring initialization
and decouple it from the actual writing of values, we
gain the ability to decide where and how we want to apply
the values.
The advantage of this will become more clear when
we need to initialize workarounds on older gens where
it is not possible to write all the registers through ring
LRIs.
v2: rebase on newest bdw workarounds
Cc: Arun Siluvery <arun.siluvery@linux.intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Arun Siluvery <arun.siluvery@linux.intel.com>
[danvet: Resolve tiny conflict in comments and ocd alignments a bit.]
[danvet2: Remove bogus force_wake_get call spotted by Paulo and QA.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
John Harrison
Michel Thierry
Daniel Vetter
Dave Gordon
Chris Wilson
Thomas Daniel
Arun Siluvery
Dave Airlie
Mika Kuoppala