Gwan-gyeong Mun [Thu, 14 May 2020 06:07:27 +0000 (09:07 +0300)]
drm/i915: Add state readout for DP VSC SDP
Added state readout for DP VSC SDP and enabled state validation
for DP VSC SDP.
v2: Minor style fix
v3: Replace a structure name to drm_dp_vsc_sdp from intel_dp_vsc_sdp
v4: Use struct drm_device logging macros
v10: Skip checking of VSC SDP when a crtc config has psr.
Gwan-gyeong Mun [Thu, 14 May 2020 06:07:25 +0000 (09:07 +0300)]
drm/i915: Program DP SDPs with computed configs
In order to use computed config for DP SDPs (DP VSC SDP and DP HDR Metadata
Infoframe SDP), it replaces intel_dp_vsc_enable() function and
intel_dp_hdr_metadata_enable() function to intel_dp_set_infoframes()
function.
And it removes unused functions.
Before:
intel_dp_vsc_enable() and intel_dp_hdr_metadata_enable() compute sdp
configs and program sdp registers on enable callback of encoder.
After:
It separates computing of sdp configs and programming of sdp register.
The compute config callback of encoder calls computing sdp configs.
The enable callback of encoder calls programming sdp register.
Gwan-gyeong Mun [Thu, 14 May 2020 06:07:20 +0000 (09:07 +0300)]
drm/i915/dp: Read out DP SDPs
It adds code to read the DP SDPs from the video DIP and unpack them into
the crtc state.
It adds routines that read out DP VSC SDP and DP HDR Metadata Infoframe SDP
In order to unpack DP VSC SDP, it adds intel_dp_vsc_sdp_unpack() function.
It follows DP 1.4a spec. [Table 2-116: VSC SDP Header Bytes] and
[Table 2-117: VSC SDP Payload for DB16 through DB18]
In order to unpack DP HDR Metadata Infoframe SDP, it adds
intel_dp_hdr_metadata_infoframe_sdp_unpack(). And it follows DP 1.4a spec.
([Table 2-125: INFOFRAME SDP v1.2 Header Bytes] and
[Table 2-126: INFOFRAME SDP v1.2 Payload Data Bytes - DB0 through DB31])
and CTA-861-G spec. [Table-42 Dynamic Range and Mastering InfoFrame].
A naming rule and style of intel_read_dp_sdp() function references
intel_read_infoframe() function of intel_hdmi.c
v2: Minor style fix
v3: Replace a structure name to drm_dp_vsc_sdp from intel_dp_vsc_sdp
v4: Use struct drm_device logging macros
v5: Addressed review comments from Uma
- Polish commit message and comments
- Combine the if checks of sdp.HB2 and sdp.HB3
- Add 6bpc to unpacking of VSC SDP
Gwan-gyeong Mun [Thu, 14 May 2020 06:07:19 +0000 (09:07 +0300)]
video/hdmi: Add Unpack only function for DRM infoframe
It adds an unpack only function for DRM infoframe for dynamic range and
mastering infoframe readout.
It unpacks the information data block contained in the binary buffer into
a structured frame of the HDMI Dynamic Range and Mastering (DRM)
information frame.
In contrast to hdmi_drm_infoframe_unpack() function, it does not verify
a checksum.
It can be used for unpacking a DP HDR Metadata Infoframe SDP case.
DP HDR Metadata Infoframe SDP uses the same Dynamic Range and Mastering
(DRM) information (CTA-861-G spec.) such as HDMI DRM infoframe.
But DP SDP header and payload structure are different from HDMI DRM
Infoframe. Therefore unpacking DRM infoframe for DP requires skipping of
a verifying checksum.
v9: Add clear comments to hdmi_drm_infoframe_unpack_only() and
hdmi_drm_infoframe_unpack() (Laurent Pinchart)
Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> Reviewed-by: Uma Shankar <uma.shankar@intel.com> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200514060732.3378396-2-gwan-gyeong.mun@intel.com
Chris Wilson [Wed, 13 May 2020 07:47:46 +0000 (08:47 +0100)]
drm/i915/gt: Transfer old virtual breadcrumbs to irq_worker
The second try at staging the transfer of the breadcrumb. In part one,
we realised we could not simply move to the second engine as we were
only holding the breadcrumb lock on the first. So in commit b20f0c77105d
("drm/i915/gt: Stage the transfer of the virtual breadcrumb"), we
removed it from the first engine and marked up this request to reattach
the signaling on the new engine. However, this failed to take into
account that we only attach the breadcrumb if the new request is added
at the start of the queue, which if we are transferring, it is because
we know there to be a request to be signaled (and hence we would not be
attached).
In this attempt, we try to transfer the completed requests to the
irq_worker on its rq->engine->breadcrumbs. This preserves the coupling
between the rq and its breadcrumbs, so that
i915_request_cancel_breadcrumb() does not attempt to manipulate the list
under the wrong lock.
v2: Code sharing is fun.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1862 Fixes: b20f0c77105d ("drm/i915/gt: Stage the transfer of the virtual breadcrumb") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200513074809.18194-1-chris@chris-wilson.co.uk
Chris Wilson [Thu, 14 May 2020 06:29:05 +0000 (07:29 +0100)]
drm/i915: Show per-engine default property values in sysfs
By providing the default values configured into the kernel via sysfs, it
is much more convenient for userspace to restore those sane defaults, or
at least know what are considered good baseline. This is useful, for
example, to cleanup after any failed userspace prior to commencing new
jobs.
Chris Wilson [Wed, 13 May 2020 17:35:04 +0000 (18:35 +0100)]
drm/i915: Drop no-semaphore boosting
Now that we have fast timeslicing on semaphores, we no longer need to
prioritise none-semaphore work as we will yield any work blocked on a
semaphore to the next in the queue. Previously with no timeslicing,
blocking on the semaphore caused extremely bad scheduling with multiple
clients utilising multiple rings. Now, there is no impact and we can
remove the complication.
Chris Wilson [Wed, 13 May 2020 07:47:59 +0000 (08:47 +0100)]
drm/i915: Drop I915_RESET_TIMEOUT and friends
These were used to set various timeouts for the reset procedure
(deciding when the engine was dead, and even if the reset itself was not
making forward progress). No longer used.
Chris Wilson [Wed, 13 May 2020 16:59:32 +0000 (17:59 +0100)]
drm/i915: Mark the addition of the initial-breadcrumb in the request
The initial-breadcrumb is used to mark the end of the awaiting and the
beginning of the user payload. We verify that we do not start the user
payload before all signaler are completed, checking our semaphore setup
by looking for the initial breadcrumb being written too early. We also
want to ensure that we do not add semaphore waits after we have already
closed the semaphore section, an issue for later deferred waits.
Chris Wilson [Wed, 13 May 2020 12:28:26 +0000 (13:28 +0100)]
drm/i915/gt: Suspend tasklets before resume sanitization
It is possible for a residual tasklet to be pending execution as we
resume (whether that's some prior test kicking off the tasklet, or if we
are in a suspend/resume stress test). As such, we do not want that
tasklet to execute in the middle of our sanitization, such that it sees
the poisoned state. For example,
As part of the reset preparation, engine->reset.prepare() prevents the
tasklet from running, so pull the sanitization inside the critical
section for reset.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1812 Fixes: 4b9820ada0d3 ("drm/i915/gt: Scrub execlists state on resume") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200513122826.27484-1-chris@chris-wilson.co.uk
For future Gen12 SAGV implementation we need to
seemlessly alter wm levels calculated, depending
on whether we are allowed to enable SAGV or not.
So this accessor will give additional flexibility
to do that.
Currently this accessor is still simply working
as "pass-through" function. This will be changed
in next coming patches from this series.
v2: - plane_id -> plane->id(Ville Syrjälä)
- Moved wm_level var to have more local scope
(Ville Syrjälä)
- Renamed yuv to color_plane(Ville Syrjälä) in
skl_plane_wm_level
v3: - plane->id -> plane_id(this time for real, Ville Syrjälä)
- Changed colorplane id type from boolean to int as index
(Ville Syrjälä)
- Moved crtc_state param so that it is first now
(Ville Syrjälä)
- Moved wm_level declaration to tigher scope in
skl_write_plane_wm(Ville Syrjälä)
v4: - Started to use enum values for color plane
- Do sizeof for a type what we are memset'ing
- Zero out wm_uv as well(Ville Syrjälä)
v5: - Fixed rebase conflict caused by COLOR_PLANE_*
enum removal
v6: - Do not use skl_plane_wm_level accessor in skl_allocate_pipe_ddb
v7: - Get rid of wm_uv, which is not used in skl_plane_write_wm(Ville)
Chris Wilson [Wed, 13 May 2020 10:01:20 +0000 (11:01 +0100)]
drm/i915/gt: Reset execlists registers before HWSP
Upon gt resume, we first poison then sanitize the engine. However, our
testing shows that gen9 will very rarely retain the poisoned value from
the HWSP mappings of the execlists status registers. This suggests that
it is reading back from the HWSP, so rejig the register reset.
v2: Maybe RING_CONTEXT_STATUS_PTR is write masked. It is.
Chris Wilson [Sat, 9 May 2020 11:52:17 +0000 (12:52 +0100)]
drm/i915: Handle idling during i915_gem_evict_something busy loops
i915_gem_evict_something() is charged with finding a slot within the GTT
that we may reuse. Since our goal is not to stall, we first look for a
slot that only overlaps idle vma. To this end, on the first pass we move
any active vma to the end of the search list. However, we only stopped
moving active vma after we see the first active vma twice. If during the
search, that first active vma completed, we would not notice and keep on
extending the search list.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1746 Fixes: 587eefbe9921 ("drm/i915: Pull i915_vma_pin under the vm->mutex") Fixes: 1912e1b11c59 ("drm/i915: Coordinate i915_active with its own mutex") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: <stable@vger.kernel.org> # v5.5+ Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200509115217.26853-1-chris@chris-wilson.co.uk
There are still some residual issues with asynchronous binding and
execution, but since commit 043b2372b972 ("drm/i915: Immediately execute
the fenced work") we prefer not to use asynchronous binds, and the
remaining issues do not seem restricted to Cherryview [at least the ones
seen over a few dozen CI runs, less frequent issues are sure to be
discovered!]
These issues seem to be mitigated, if not eliminated entirely, by the
previous commit e289fa368f05 ("drm/i915/gt: Force pte cacheline to main
memory").
Mika Kuoppala [Mon, 11 May 2020 16:08:03 +0000 (19:08 +0300)]
drm/i915/gt: Force pte cacheline to main memory
We have problems of tgl not seeing a valid pte entry when iommu is
enabled. Add heavy handed flushing of entry modification by flushing the
cpu, cacheline and then wcb. This forces the pte out to main memory past
this point regarless of promises of coherency.
This is an evolution of an experimental patch from Chris Wilson of adding
wmb for coherent partners, by adding a clflush to force the cache->memory
step.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1840
Testcase: igt/gem_exec_fence/parallel Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20200511160803.15407-1-mika.kuoppala@linux.intel.com
Chris Wilson [Mon, 11 May 2020 14:13:03 +0000 (15:13 +0100)]
drm/i915/selftests: Always flush before unpining after writing
Be consistent, and even when we know we had used a WC, flush the mapped
object after writing into it. The flush understands the mapping type and
will only clflush if !I915_MAP_WC, but will always insert a wmb [sfence]
so that we can be sure that all writes are visible.
v2: Add the unconditional wmb so we are know that we always flush the
writes to memory/HW at that point.
Chris Wilson [Sun, 10 May 2020 10:24:29 +0000 (11:24 +0100)]
drm/i915: Emit await(batch) before MI_BB_START
Be consistent and ensure that we always emit the asynchronous waits
prior to issuing instructions that use the address. This ensures that if
we do emit GPU commands to do the await, they are before our use!
Ville Syrjälä [Wed, 11 Mar 2020 15:54:20 +0000 (17:54 +0200)]
drm/i915: Turn intel_digital_port_connected() in a vfunc
Let's get rid of the platform if ladders in
intel_digital_port_connected() and make it a vfunc. Now the if
ladders are at the encoder initialization which makes them a bit
less convoluted.
v2: Add forward decl for intel_encoder in intel_tc.h
v3: Duplicate stuff to avoid exposing platform specific
functions across files (Jani)
Ville Syrjälä [Thu, 30 Apr 2020 12:58:21 +0000 (15:58 +0300)]
drm/i915: Fix glk watermark calculations
GLK wants the +1 adjustement for the "blocks per line" value
for x-tile/y-tile, just like cnl+.
Also the x-tile and linear cases are almost identical. The only
difference is this +1 which is always done for glk+, and only
done for linear on skl/bxt. Let's unify it to a single branch
with a special case for the +1, just like we do for y-tile.
Chris Wilson [Mon, 11 May 2020 07:57:07 +0000 (08:57 +0100)]
drm/i915: Tidy awaiting on dma-fences
Just tidy up the return handling for completed dma-fences. While it may
return errors for invalid fence, we already know that we have a good
fence and the only error will be an already signaled fence.
Mika Kuoppala [Mon, 11 May 2020 10:22:01 +0000 (13:22 +0300)]
drm/i915: Make intel_timeline_init static
Commit ca30aaed5228 ("drm/i915/gt: Use the kernel_context to measure the
breadcrumb size") removed the last external user for intel_timeline_init.
Mark it static.
Chris Wilson [Mon, 11 May 2020 07:57:03 +0000 (08:57 +0100)]
drm/i915/gt: Mark up the racy read of execlists->context_tag
Since we are using bitops on context_tag to allow us to reserve and
release inflight tags concurrently, the scan for the next bit is
intentionally racy.
drm/i915: Replace zero-length array with flexible-array
The current codebase makes use of the zero-length array language
extension to the C90 standard, but the preferred mechanism to declare
variable-length types such as these ones is a flexible array member[1][2],
introduced in C99:
struct foo {
int stuff;
struct boo array[];
};
By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last in the structure, which
will help us prevent some kind of undefined behavior bugs from being
inadvertently introduced[3] to the codebase from now on.
Also, notice that, dynamic memory allocations won't be affected by
this change:
"Flexible array members have incomplete type, and so the sizeof operator
may not be applied. As a quirk of the original implementation of
zero-length arrays, sizeof evaluates to zero."[1]
sizeof(flexible-array-member) triggers a warning because flexible array
members have incomplete type[1]. There are some instances of code in
which the sizeof operator is being incorrectly/erroneously applied to
zero-length arrays and the result is zero. Such instances may be hiding
some bugs. So, this work (flexible-array member conversions) will also
help to get completely rid of those sorts of issues.
Chris Wilson [Sat, 9 May 2020 10:50:21 +0000 (11:50 +0100)]
drm/i915: Replace the hardcoded I915_FENCE_TIMEOUT
Expose the hardcoded timeout for unsignaled foreign fences as a Kconfig
option, primarily to allow brave systems to disable the timeout and
solely rely on correct signaling.
Chris Wilson [Fri, 8 May 2020 09:29:27 +0000 (10:29 +0100)]
drm/i915: Prevent using semaphores to chain up to external fences
The downside of using semaphores is that we lose metadata passing
along the signaling chain. This is particularly nasty when we
need to pass along a fatal error such as EFAULT or EDEADLK. For
fatal errors we want to scrub the request before it is executed,
which means that we cannot preload the request onto HW and have
it wait upon a semaphore.
To allow faster engine to engine synchronization, peel the layer of
dma-fence-chain to expose potential i915 fences so that the
i915_request code can emit HW semaphore wait/signal operations in the
ring which is faster than waking up the host to submit unblocked
workloads after interrupt notification.
This is similar to the peeling we do for e.g. dma_fence_array.
Chris Wilson [Fri, 8 May 2020 10:42:20 +0000 (11:42 +0100)]
drm/i915/gt: Improve precision on defer_request assert
The kernel_context does not use initial-breadcrumbs, so when we ask if
its requests have started we do so by comparing against the completion
seqno of the previous request. This is very imprecise, not precise
enough for the defer_request assertion.
Chris Wilson [Fri, 8 May 2020 09:29:26 +0000 (10:29 +0100)]
drm/i915: Pull waiting on an external dma-fence into its routine
As a means for a small code consolidation, but primarily to start
thinking more carefully about internal-vs-external linkage, pull the
pair of i915_sw_fence_await_dma_fence() calls into a common routine.
Chris Wilson [Fri, 8 May 2020 09:29:25 +0000 (10:29 +0100)]
drm/i915: Ignore submit-fences on the same timeline
While we ordinarily do not skip submit-fences due to the accompanying
hook that we want to callback on execution, a submit-fence on the same
timeline is meaningless.
Mika Kuoppala [Thu, 7 May 2020 14:20:45 +0000 (17:20 +0300)]
drm/i915/gen12: Add aux table invalidate for all engines
All engines, exception being blitter as it does not
care about the form, can access compressed surfaces.
So we need to add forced aux table invalidates
for those engines.
v2: virtual instance masking (Chris)
v3: bug on if not found (Chris)
References: 242722ba082f ("drm/i915/gen12: Invalidate aux table entries forcibly")
References bspec#43904, hsdes#1809175790 Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Chuansheng Liu <chuansheng.liu@intel.com> Cc: Rafael Antognolli <rafael.antognolli@intel.com> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Acked-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20200507142045.8668-1-mika.kuoppala@linux.intel.com
Chris Wilson [Thu, 7 May 2020 15:23:38 +0000 (16:23 +0100)]
drm/i915: Remove wait priority boosting
Upon waiting a request (when asked), we gave that request a small
priority boost, not enough for it to cause preemption, but enough for it
to be scheduled next before all equals. We also used that bit to give
new clients a small priority boost, similar to FQ_CODEL, such that we
favoured short interactive tasks ahead of long running streams.
However, this is causing lots of complications with timeslicing where we
both want to honour the boost and yet ignore it. Those complications
cause unexpected user behaviour (tasks not being timesliced and run
concurrently as epxected), and the easiest way to resolve that is to
remove the boost. Hopefully, we can find a compromise again if we need
to, but in theory timeslicing itself and future more advanced schedulers
should give us the interactivity boost we seek.
Chris Wilson [Thu, 7 May 2020 15:51:09 +0000 (16:51 +0100)]
drm/i915: Mark concurrent submissions with a weak-dependency
We recorded the dependencies for WAIT_FOR_SUBMIT in order that we could
correctly perform priority inheritance from the parallel branches to the
common trunk. However, for the purpose of timeslicing and reset
handling, the dependency is weak -- as we the pair of requests are
allowed to run in parallel and not in strict succession.
The real significance though is that this allows us to rearrange
groups of WAIT_FOR_SUBMIT linked requests along the single engine, and
so can resolve user level inter-batch scheduling dependencies from user
semaphores.
Fixes: 5478de461342 ("drm/i915: Copy across scheduler behaviour flags across submit fences")
Testcase: igt/gem_exec_fence/submit Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: <stable@vger.kernel.org> # v5.6+ Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200507155109.8892-1-chris@chris-wilson.co.uk
Mika Kuoppala [Wed, 6 May 2020 16:53:10 +0000 (19:53 +0300)]
drm/i915/gen12: Invalidate aux table entries forcibly
Aux table invalidation can fail on update. So
next access may cause memory access to be into stale entry.
Proposed workaround is to invalidate entries between
all batchbuffers.
v2: correct register address (Yang)
v3: respect the order (Chris)
References bspec#43904, hsdes#1809175790 Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Chuansheng Liu <chuansheng.liu@intel.com> Cc: Rafael Antognolli <rafael.antognolli@intel.com> Cc: Yang A Shi <yang.a.shi@intel.com> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Acked-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20200506165310.1239-1-mika.kuoppala@linux.intel.com
Chris Wilson [Mon, 4 May 2020 18:07:45 +0000 (19:07 +0100)]
drm/i915/gt: Stop holding onto the pinned_default_state
As we only restore the default context state upon banning a context, we
only need enough of the state to run the ring and nothing more. That is
we only need our bare protocontext.
Chris Wilson [Tue, 5 May 2020 08:46:29 +0000 (09:46 +0100)]
drm/i915/execlists: Record the active CCID from before reset
If we cannot trust the reset will flush out the CS event queue such that
process_csb() reports an accurate view of HW, we will need to search the
active and pending contexts to determine which was actually running at
the time we issued the reset.
We need a new PCode request commands and reply codes
to be added as a prepartion patch for QGV points
restricting for new SAGV support.
v2: - Extracted those changes into separate patch
(Ville Syrjälä)
v3: - Moved new PCode masks to another place from
PCode commands(Ville)
v4: - Moved new PCode masks to correspondent PCode
command, with identation(Ville)
- Changed naming to ICL_ instead of GEN11_
to fit more nicely into existing definition
style.
Chris Wilson [Sun, 3 May 2020 18:00:34 +0000 (19:00 +0100)]
drm/i915/display: Warn if the FBC is still writing to stolen on removal
If the FBC is still writing into stolen, it will overwrite any future
users of that stolen region. Check before release, just to ease any
concerns -- we can remove it again later if it is barking up the wrong
tree.
Sultan Alsawaf [Thu, 30 Apr 2020 21:46:54 +0000 (14:46 -0700)]
drm/i915: Don't enable WaIncreaseLatencyIPCEnabled when IPC is disabled
In commit 80d5907556c9, a logical AND was erroneously changed to an OR,
causing WaIncreaseLatencyIPCEnabled to be enabled unconditionally for
kabylake and coffeelake, even when IPC is disabled. Fix the logic so
that WaIncreaseLatencyIPCEnabled is only used when IPC is enabled.
Ville Syrjälä [Wed, 29 Apr 2020 18:54:57 +0000 (21:54 +0300)]
drm/i915: Streamline the artihmetic
All these ROUNDING_FACTORs and whatnot are making this thing hard to
read. Get rid of them. And let's massage some of the fractions to
give us less questionable intermediate results and perhaps less
divisions.
Also looks like a good helping of 64bit math stuff is needed to
avoid some of overflows present in the current code. There
might still be a few overflows, namely when calculating
link_clks_available/samples_room (would require a huge hblank
though), and potentially when calculating hblank_rise (not sure
how large link_clks_active can get).
It looks like we're still not calculating exactly what the spec says
since we truncate tu_data and tu_line early. But I'm too lazy to
figure out if we could avoid that.
Ville Syrjälä [Wed, 29 Apr 2020 18:54:56 +0000 (21:54 +0300)]
drm/i915: Rename variables to be consistent with bspec
Since the code seems insistent on using the variable names from the
bspec formulat, let's be consistent and use those names for all
the things. For some reason 'link_clk' and 'lanes' were left out
in the code until now.
Ville Syrjälä [Wed, 29 Apr 2020 18:54:55 +0000 (21:54 +0300)]
drm/i915: Nuke mode.vrefresh usage
mode.vrefresh is rounded to the nearest integer. You don't want to use
it anywhere that requires precision. Also I want to nuke it.
vtotal*vrefresh == 1000*clock/htotal, so let's use the latter.
Ville Syrjälä [Thu, 30 Apr 2020 12:58:22 +0000 (15:58 +0300)]
drm/i915: Remove cnl pre-prod workarounds
Remove all the stepping dependent cnl workarounds. Bspec lists
more steppings than this so presumably these are classed as
pre-production. And this is cnl after all so no one should
really care anyway.
Ville Syrjälä [Wed, 29 Apr 2020 10:10:23 +0000 (13:10 +0300)]
drm/i915/fbc: Require linear fb stride to be multiple of 512 bytes on gen9/glk
Display WA #1105 says that FBC requires PLANE_STRIDE to be a multiple
of 512 bytes on gen9 and glk.
This is definitely true for glk as certain tests (such as
igt/kms_big_fb/linear-16bpp-rotate-0) are now failing when the
display resolution results in a plane stride which is not a
multiple of 512 bytes.
Curiously I was not able to reproduce this on a KBL. First I
suspected that our use of the FBC override stride explain this,
but after trying to use the override stride on glk the test
still failed. I did try both the old CHICKEN_MISC_4 way and
the new FBC_STRIDE way, neither had any effect on the result.
Anyways, we need this at least on glk. But let's trust the spec
and apply the w/a for all gen9 as well, despite being unable to
reproduce the problem.
That is a preparation patch before next one where we
introduce old_bw_state and a bunch of other changes
as well.
In a review comment it was suggested to split out
at least that renaming into a separate patch, what
is done here.
We need to calculate SAGV mask also in a non-modeset
commit, however currently active_pipes are only calculated
for modesets in global atomic state, thus now we will be
tracking those also in bw_state in order to be able to
properly access global data.
v2: - Removed pre/post plane SAGV updates from modeset(Ville)
- Now tracking active pipes in intel_can_enable_sagv(Ville)
v3: - lock global state if active_pipes change as well(Ville)
drm/i915: Use bw state for per crtc SAGV evaluation
Future platforms require per-crtc SAGV evaluation
and serializing global state when those are changed
from different commits.
v2: - Add has_sagv check to intel_crtc_can_enable_sagv
so that it sets bit in reject mask.
- Use bw_state in intel_pre/post_plane_enable_sagv
instead of atomic state
v3: - Fixed rebase conflict, now using
intel_atomic_crtc_state_for_each_plane_state in
order to call it from atomic check
v4: - Use fb modifier from plane state
v5: - Make intel_has_sagv static again(Ville)
- Removed unnecessary NULL assignments(Ville)
- Removed unnecessary SAGV debug(Ville)
- Call intel_compute_sagv_mask only for modesets(Ville)
- Serialize global state only if sagv results change, but
not mask itself(Ville)
v6: - use lock global state instead of serialize(Ville)
v7: - use both global state lock and serialize depending on
if we need to change only global state or access hw
(Ville)
Chris Wilson [Mon, 4 May 2020 14:06:29 +0000 (15:06 +0100)]
drm/i915/gem: Implement legacy MI_STORE_DATA_IMM
The older arches did not convert MI_STORE_DATA_IMM to using the GTT, but
left them writing to a physical address. The notes suggest that the
primary reason would be so that the writes were cache coherent, as the
CPU cache uses physical tagging. As such we did not implement the
legacy variant of MI_STORE_DATA_IMM and so left all the relocations
synchronous -- but with a small function to convert from the vma address
into the physical address, we can implement asynchronous relocs on these
older arches, fixing up a few tests that require them.
In order to be able to test the legacy paths, refactor the gpu
relocations so that we can hook them up to a selftest.
v2: Use an array of offsets not enum labels for the selftest
v3: Refactor the common igt_hexdump()
Chris Wilson [Mon, 4 May 2020 12:51:49 +0000 (13:51 +0100)]
drm/i915/gem: Specify address type for chained reloc batches
It is required that a chained batch be in the same address domain as its
parent, and also that must be specified in the command for earlier gen
as it is not inferred from the chaining until gen6.
Chris Wilson [Mon, 4 May 2020 04:48:42 +0000 (05:48 +0100)]
drm/i915: Allow some leniency in PCU reads
Extend the timeout for pcode reads to 20ms as they should not be
performed along critical paths, and succeeding after a short delay is
better than failing entirely.
Chris Wilson [Sun, 3 May 2020 17:15:13 +0000 (18:15 +0100)]
drm/i915/gem: Lazily acquire the device wakeref for freeing objects
We only need the device wakeref on freeing the objects if we have to
unbind the object from the global GTT, or otherwise update device
information. If the objects are clean, we never need the wakeref, so
avoid taking until required.
Chris Wilson [Sat, 2 May 2020 17:35:12 +0000 (18:35 +0100)]
drm/i915/gt: Sanitize RPS interrupts upon resume
Currently we clear and disable the RPS pm interrupts on module load, and
presume that they remain disabled forevermore. However, the mask is
cleared on suspend and so after resume they may start showing up again
unexepectedly.
Chris Wilson [Fri, 1 May 2020 19:29:45 +0000 (20:29 +0100)]
drm/i915/gem: Try an alternate engine for relocations
If at first we don't succeed, try try again.
Not all engines may support the MI ops we need to perform asynchronous
relocation patching, and so we end up falling back to a synchronous
operation that has a liability of blocking. However, Tvrtko pointed out
we don't need to use the same engine to perform the relocations as we
are planning to execute the execbuf on, and so if we switch over to a
working engine, we can perform the relocation asynchronously. The user
execbuf will be queued after the relocations by virtue of fencing.
This patch creates a new context per execbuf requiring asynchronous
relocations on an unusable engines. This is perhaps a bit excessive and
can be ameliorated by a small context cache, but for the moment we only
need it for working around a little used engine on Sandybridge, and only
if relocations are actually required to an active batch buffer.
Now we just need to teach the relocation code to handle physical
addressing for gen2/3, and we should then have universal support!
Chris Wilson [Fri, 1 May 2020 19:29:44 +0000 (20:29 +0100)]
drm/i915/gem: Use a single chained reloc batches for a single execbuf
As we can now keep chaining together a relocation batch to process any
number of relocations, we can keep building that relocation batch for
all of the target vma. This avoiding emitting a new request into the
ring for each target, consuming precious ring space and a potential
stall.
v2: Propagate the failure from submitting the relocation batch.
Chris Wilson [Fri, 1 May 2020 19:29:43 +0000 (20:29 +0100)]
drm/i915/gem: Use chained reloc batches
The ring is a precious resource: we anticipate to only use a few hundred
bytes for a request, and only try to reserve that before we start. If we
go beyond our guess in building the request, then instead of waiting at
the start of execbuf before we hold any locks or other resources, we
may trigger a wait inside a critical region. One example is in using gpu
relocations, where currently we emit a new MI_BB_START from the ring
every time we overflow a page of relocation entries. However, instead of
insert the command into the precious ring, we can chain the next page of
relocation entries as MI_BB_START from the end of the previous.
v2: Delay the emit_bb_start until after all the chained vma
synchronisation is complete. Since the buffer pool batches are idle, this
_should_ be a no-op, but one day we may some fancy async GPU bindings
for new vma!
v3: Use pool/batch consitently, once we start thinking in terms of the
batch vma, use batch->obj.
v4: Explain the magic number 4.
Tvrtko spotted that we lose propagation of the error for failing to
submit the relocation request; that's easier to fix up in the next
patch.
Chris Wilson [Fri, 1 May 2020 14:51:20 +0000 (15:51 +0100)]
drm/i915: Implement vm_ops->access for gdb access into mmaps
gdb uses ptrace() to peek and poke bytes of the target's address space.
The driver must implement an vm_ops->access() handler or else gdb will
be unable to inspect the pointer and report it as out-of-bounds.
Worse than useless as it causes immediate suspicion of the valid GTT
pointer, distracting the poor programmer trying to find his bug.
v2: Write-protect readonly objects (Matthew).
Testcase: igt/gem_mmap_gtt/ptrace
Testcase: igt/gem_mmap_offset/ptrace Suggested-by: Kristian H. Kristensen <hoegsberg@google.com> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Matthew Auld <matthew.auld@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Maciej Patelczyk <maciej.patelczyk@intel.com> Cc: Kristian H. Kristensen <hoegsberg@google.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200501145120.18830-1-chris@chris-wilson.co.uk
Chris Wilson [Fri, 1 May 2020 12:22:49 +0000 (13:22 +0100)]
drm/i915/gt: Make timeslicing an explicit engine property
In order to allow userspace to rely on timeslicing to reorder their
batches, we must support preemption of those user batches. Declare
timeslicing as an explicit property that is a combination of having the
kernel support and HW support.
Chris Wilson [Thu, 30 Apr 2020 11:18:12 +0000 (12:18 +0100)]
drm/i915/gt: Move the batch buffer pool from the engine to the gt
Since the introduction of 'soft-rc6', we aim to park the device quickly
and that results in frequent idling of the whole device. Currently upon
idling we free the batch buffer pool, and so this renders the cache
ineffective for many workloads. If we want to have an effective cache of
recently allocated buffers available for reuse, we need to decouple that
cache from the engine powermanagement and make it timer based. As there
is no reason then to keep it within the engine (where it once made
retirement order easier to track), we can move it up the hierarchy to the
owner of the memory allocations.
v2: Hook up to debugfs/drop_caches to clear the cache on demand.
Extend coverage of the blitter client by exercising conversion to and
from tiled sources. In the process we perform spot checks to verify that
the tiling/detiling is being applied correctly, along with position
invariance of the tiling parameters.
Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20200430064957.14942-1-chris@chris-wilson.co.uk
We reduced the clocks slowly after a boost event based on the
observation that the smoothness of animations suffered. However, since
reducing the evalution intervals, we should be able to respond to the
rapidly fluctuating workload of a simple desktop animation and so
restore the more aggressive downclocking.
Chris Wilson [Wed, 29 Apr 2020 20:54:45 +0000 (21:54 +0100)]
drm/i915/gt: Apply the aggressive downclocking to parking
We treat parking as a manual RPS timeout event, and downclock the GPU
for the next unpark and batch execution. However, having restored the
aggressive downclocking and observed that we have very light workloads
whose only interaction is through the manual parking events, carry over
the aggressive downclocking to the fake RPS events.
Chris Wilson [Wed, 29 Apr 2020 20:54:44 +0000 (21:54 +0100)]
drm/i915/gt: Switch to manual evaluation of RPS
As with the realisation for soft-rc6, we respond to idling the engines
within microseconds, far faster than the response times for HW RC6 and
RPS. Furthermore, our fast parking upon idle, prevents HW RPS from
running for many desktop workloads, as the RPS evaluation intervals are
on the order of tens of milliseconds, but the typical workload is just a
couple of milliseconds, but yet we still need to determine the best
frequency for user latency versus power.
Recognising that the HW evaluation intervals are a poor fit, and that
they were deprecated [in bspec at least] from gen10, start to wean
ourselves off them and replace the EI with a timer and our accurate
busy-stats. The principle benefit of manually evaluating RPS intervals
is that we can be more responsive for better performance and powersaving
for both spiky workloads and steady-state.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1698 Fixes: 0b15235dadea ("drm/i915/gt: Treat idling as a RPS downclock event") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Andi Shyti <andi.shyti@intel.com> Reviewed-by: Andi Shyti <andi.shyti@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200429205446.3259-4-chris@chris-wilson.co.uk
Chris Wilson [Wed, 29 Apr 2020 20:54:41 +0000 (21:54 +0100)]
drm/i915/gt: Always enable busy-stats for execlists
In the near future, we will utilize the busy-stats on each engine to
approximate the C0 cycles of each, and use that as an input to a manual
RPS mechanism. That entails having busy-stats always enabled and so we
can remove the enable/disable routines and simplify the pmu setup. As a
consequence of always having the stats enabled, we can also show the
current active time via sysfs/engine/xcs/active_time_ns.
Chris Wilson [Wed, 29 Apr 2020 17:24:29 +0000 (18:24 +0100)]
drm/i915/gt: Keep a no-frills swappable copy of the default context state
We need to keep the default context state around to instantiate new
contexts (aka golden rendercontext), and we also keep it pinned while
the engine is active so that we can quickly reset a hanging context.
However, the default contexts are large enough to merit keeping in
swappable memory as opposed to kernel memory, so we store them inside
shmemfs. Currently, we use the normal GEM objects to create the default
context image, but we can throw away all but the shmemfs file.
This greatly simplifies the tricky power management code which wants to
run underneath the normal GT locking, and we definitely do not want to
use any high level objects that may appear to recurse back into the GT.
Though perhaps the primary advantage of the complex GEM object is that
we aggressively cache the mapping, but here we are recreating the
vm_area everytime time we unpark. At the worst, we add a lightweight
cache, but first find a microbenchmark that is impacted.
Having started to create some utility functions to make working with
shmemfs objects easier, we can start putting them to wider use, where
GEM objects are overkill, such as storing persistent error state.
Dan Carpenter [Wed, 29 Apr 2020 13:24:25 +0000 (16:24 +0300)]
drm/i915/selftests: fix error handling in __live_lrc_indirect_ctx_bb()
If intel_context_create() fails then it leads to an error pointer
dereference. I shuffled things around to make error handling easier.
Fixes: 98c9767946aa ("drm/i915: Add live selftests for indirect ctx batchbuffers") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Andi Shyti <andi.shyti@intel.com> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20200429132425.GE815283@mwanda
Chris Wilson [Tue, 28 Apr 2020 09:02:55 +0000 (10:02 +0100)]
drm/i915: Avoid dereferencing a dead context
Once the intel_context is closed, the GEM context may be freed and so
the link from intel_context.gem_context is invalid.
<3>[ 219.782944] BUG: KASAN: use-after-free in intel_engine_coredump_alloc+0x1bc3/0x2250 [i915]
<3>[ 219.782996] Read of size 8 at addr ffff8881d7dff0b8 by task kworker/0:1/12
drm/i915/gt: Avoid uninitialized use of rpcurupei in frequency_show
When building with clang + -Wuninitialized:
drivers/gpu/drm/i915/gt/debugfs_gt_pm.c:407:7: warning: variable
'rpcurupei' is uninitialized when used here [-Wuninitialized]
rpcurupei,
^~~~~~~~~
drivers/gpu/drm/i915/gt/debugfs_gt_pm.c:304:16: note: initialize the
variable 'rpcurupei' to silence this warning
u32 rpcurupei, rpcurup, rpprevup;
^
= 0
1 warning generated.
rpupei is assigned twice; based on the second argument to
intel_uncore_read, it seems this one should have been assigned to
rpcurupei.
Chris Wilson [Tue, 28 Apr 2020 18:47:50 +0000 (19:47 +0100)]
drm/i915/execlists: Track inflight CCID
The presumption is that by using a circular counter that is twice as
large as the maximum ELSP submission, we would never reuse the same CCID
for two inflight contexts.
However, if we continually preempt an active context such that it always
remains inflight, it can be resubmitted with an arbitrary number of
paired contexts. As each of its paired contexts will use a new CCID,
eventually it will wrap and submit two ELSP with the same CCID.
Rather than use a simple circular counter, switch over to a small bitmap
of inflight ids so we can avoid reusing one that is still potentially
active.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1796 Fixes: fb8e4b80bcc0 ("drm/i915: Remove logical HW ID") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: <stable@vger.kernel.org> # v5.5+ Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200428184751.11257-2-chris@chris-wilson.co.uk
Chris Wilson [Tue, 28 Apr 2020 18:47:49 +0000 (19:47 +0100)]
drm/i915/execlists: Avoid reusing the same logical CCID
The bspec is confusing on the nature of the upper 32bits of the LRC
descriptor. Once upon a time, it said that it uses the upper 32b to
decide if it should perform a lite-restore, and so we must ensure that
each unique context submitted to HW is given a unique CCID [for the
duration of it being on the HW]. Currently, this is achieved by using
a small circular tag, and assigning every context submitted to HW a
new id. However, this tag is being cleared on repinning an inflight
context such that we end up re-using the 0 tag for multiple contexts.
To avoid accidentally clearing the CCID in the upper 32bits of the LRC
descriptor, split the descriptor into two dwords so we can update the
GGTT address separately from the CCID.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1796 Fixes: fb8e4b80bcc0 ("drm/i915: Remove logical HW ID") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: <stable@vger.kernel.org> # v5.5+ Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200428184751.11257-1-chris@chris-wilson.co.uk
Chris Wilson [Tue, 28 Apr 2020 11:43:07 +0000 (12:43 +0100)]
drm/i915/selftests: Tweak the tolerance for clock ticks to 12.5%
Give a small bump for our tolerance on comparing the expected vs
measured clock ticks/time from 10% to 12.5% to accommodate a bad result
on Sandybridge that was off by 10.3%. Hopefully, that is the worst we
will see.
projects / kernel.git / log