Chris Wilson [Sat, 16 May 2020 19:09:40 +0000 (20:09 +0100)]
drm/i915/display: Return error from dbuf allocation failure
drivers/gpu/drm/i915/intel_pm.c:5920 skl_ddb_add_affected_pipes() error: uninitialized symbol 'ret'.
Fixes: 3cf43cdc63fb ("drm/i915: Introduce proper dbuf state") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200516190940.12675-1-chris@chris-wilson.co.uk
Ville Syrjälä [Mon, 18 May 2020 12:13:54 +0000 (15:13 +0300)]
drm/i915: Fix dbuf slice mask when turning off all the pipes
The current dbuf slice computation only happens when there are
active pipes. If we are turning off all the pipes we just leave
the dbuf slice mask at it's previous value, which may be something
other that BIT(S1). If runtime PM will kick in it will however
turn off everything but S1. Then on the next atomic commit (if
the new dbuf slice mask matches the stale value we left behind)
the code will not turn on the other slices we now need. This will
lead to underruns as the planes are trying to use a dbuf slice
that's not powered up.
To work around let's just just explicitly set the dbuf slice mask
to BIT(S1) when we are turning off all the pipes. Really the code
should just calculate this stuff the same way regardless whether
the pipes are on or off, but we're not quite there yet (need a
bit more work on the dbuf state for that).
Chris Wilson [Mon, 18 May 2020 10:29:11 +0000 (11:29 +0100)]
drm/i915/selftests: Refactor sibling selection
Tvrtko spotted that some selftests were using 'break' not 'continue',
which will fail for discontiguous engine layouts such as on Icelake
(which may have vcs0 and vcs2).
drm/i915: Mark check_shadow_context_ppgtt as maybe unused
When CONFIG_DRM_I915_DEBUG_GEM is not set, clang warns:
drivers/gpu/drm/i915/gvt/scheduler.c:884:1: warning: function
'check_shadow_context_ppgtt' is not needed and will not be emitted
[-Wunneeded-internal-declaration]
check_shadow_context_ppgtt(struct execlist_ring_context *c, struct
intel_vgpu_mm *m)
^
1 warning generated.
This warning is similar to -Wunused-function but rather than warning
that the function is completely unused, it warns that it is used in some
expression within the file but that expression will be evaluated to a
constant or be optimized away in the final assembly, essentially making
it appeared used but really isn't. Usually, this happens when a function
or variable is only used in sizeof, where it will appear to be used but
will be evaluated at compile time and not be required to be emitted.
In this case, the function is only used in GEM_BUG_ON, which is defined
as BUILD_BUG_ON_INVALID, which intentionally follows this pattern. To
fix this warning, add __maybe_unused to make it clear that this is
intentional depending on the configuration.
Chris Wilson [Fri, 15 May 2020 20:00:31 +0000 (21:00 +0100)]
drm/i915/gem: Retry faulthandlers on ENOSPC
As we no longer use the shmemfs allocation directly, we do not expect to
receive -ENOSPC from a backing store allocation. The potential sources
for -ENOSPC are then our own internal eviction code, so the choice is
either to kill the potential application with SIGBUS or to retry the
faulthandler.
In this patch we retry the fault handler, but since this is a should
never happen condition, it is arguable that we gather up copious debug
and kill the application. At worst, we cause an interruptible busy-wait,
stalling the application -- all causes should be transient and the
system should eventually recover. A small stall is hopefully a better
outcome than random oomkiller.
Ville Syrjälä [Tue, 25 Feb 2020 17:11:16 +0000 (19:11 +0200)]
drm/i915: Clean up dbuf debugs during .atomic_check()
Combine the two per-pipe dbuf debugs into one, and use the canonical
[CRTC:%d:%s] style to identify the crtc. Also use the same style as
the plane code uses for the ddb start/end, and prefix bitmask properly
with 0x to make it clear they are in fact bitmasks.
The "how many total slices we are going to use" debug we move to
outside the crtc loop so it gets printed only once at the end.
Ville Syrjälä [Tue, 25 Feb 2020 17:11:15 +0000 (19:11 +0200)]
drm/i915: Move the dbuf pre/post plane update
Encapsulate the dbuf state more by moving the pre/post
plane functions out from intel_display.c. We stick them
into intel_pm.c since that's where the rest of the code
lives for now.
Eventually we should add a new file for this stuff at which
point we also need to decide if it makes sense to even split
the wm code from the ddb code, or to keep them together.
Ville Syrjälä [Tue, 25 Feb 2020 17:11:13 +0000 (19:11 +0200)]
drm/i915: Introduce proper dbuf state
Add a global state to track the dbuf slices. Gets rid of all the nasty
coupling between state->modeset and dbuf recomputation. Also we can now
totally nuke state->active_pipe_changes.
dev_priv->wm.distrust_bios_wm still remains, but that too will get
nuked soon.
Ville Syrjälä [Tue, 25 Feb 2020 17:11:11 +0000 (19:11 +0200)]
drm/i915: Polish some dbuf debugs
Polish some of the dbuf code to give more meaningful debug
messages and whatnot. Also we can switch over to the per-device
debugs/warns at the same time.
Ville Syrjälä [Tue, 25 Feb 2020 17:11:10 +0000 (19:11 +0200)]
drm/i915: Make skl_compute_dbuf_slices() behave consistently for all platforms
Currently skl_compute_dbuf_slices() returns 0 for any inactive pipe on
icl+, but returns BIT(S1) on pre-icl for any pipe (whether it's active or
not). Let's make the behaviour consistent and always return 0 for any
inactive pipe.
After the function is no longer marked 'inline', there
is now a new warning pointing out that the only caller
is inside of an #ifdef:
drivers/gpu/drm/i915/display/intel_panel.c:493:12: warning: 'scale_user_to_hw' defined but not used [-Wunused-function]
493 | static u32 scale_user_to_hw(struct intel_connector *connector,
| ^~~~~~~~~~~~~~~~
Move the function itself into that #ifdef as well.
Ville Syrjälä [Mon, 28 Oct 2019 11:30:35 +0000 (13:30 +0200)]
drm/i915: Enable pipe gamma for the overlay
We pass the plane data through the pipe gamma for all the other
planes. Can't see why we should treat the overlay differently,
so let's enable pipe gamma for it as well.
Ville Syrjälä [Mon, 28 Oct 2019 11:30:34 +0000 (13:30 +0200)]
drm/i915: Configure overlay cc_out precision based on crtc gamma config
Put the overlay color conversion unit into 10bit mode if the
pipe isn't using the 8bit legacy gamma. Not 100% sure this is
what the intention of the bit was but makes at least some sense to
me.
Ville Syrjälä [Mon, 28 Oct 2019 11:30:33 +0000 (13:30 +0200)]
drm/i915: Fix overlay colorkey for 30bpp and 8bpp
As with the video sprites the colorkey is always specified
as 8bpc. For 10bpc primary plane formats we just ignore the
two lsbs of each component. For C8 we'll replicate the same
key to each chanel, which is what the hardware wants.
Ville Syrjälä [Mon, 28 Oct 2019 11:30:32 +0000 (13:30 +0200)]
drm/i915: Fix max cursor size for i915g/gm
Apparently the 128x128 and 256x256 ARGB cursor modes were
only added on LPT/CST.
While the display section of bspec isn't super clear on the
subject, it does highlight these two modes in a different
color, has a few changlog entries indicating the 256x256 mode
was added for a LPT DCN, and that the 128x128 mode was also
added later (though no DCN/platform note there).
The "device dependencies" bspec section does list the 256x256x32
as a new feature for LPT/CST, and goes on to mention that current
hw only has the 64x64x32 mode (which reinforces the notion that
the 128x128 mode was also added at the same time).
Testing on actual hardware confirms all of this. CI shows all
the 128x128 and 256x256 tests failing on GDG, and my ALV
definitely doesn't like them.
So we shall limit GDG/ALV to 64x64 only. And while at it
let's adjust the mobile gen2 case to list the two platforms
explicitly so that the if-ladder looks reasonably uniform.
Joonas Lahtinen [Fri, 30 Aug 2019 10:50:53 +0000 (13:50 +0300)]
drm/i915: Document locking guidelines
To ensure cross-driver locking compatibility, document the expected
guidelines for implementing the GEM locking in i915. Note that this
is a description of how things should end up after being reworked,
and does not reflect the current state of things.
v2: Use rst note:: tag (Rodrigo)
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Dave Airlie <airlied@redhat.com> Cc: Matthew Auld <matthew.auld@intel.com> Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Cc: CQ Tang <cq.tang@intel.com> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Acked-by: Dave Airlie <airlied@redhat.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190830105053.17491-1-joonas.lahtinen@linux.intel.com
Pull the code to do the CS timestamp ns<->ticks conversion into
helpers and use them all over.
The check in i915_perf_noa_delay_set() seems a bit dubious,
so we switch it to do what I assume it wanted to do all along
(ie. make sure the resulting delay in CS timestamp ticks
doesn't exceed 32bits)?
v3: Forbid simultaneous legacy SAGV PCode requests and
restricting qgv points. Put the actual restriction
to commit function, added serialization(thanks to Ville)
to prevent commit being applied out of order in case of
nonblocking and/or nomodeset commits.
v4:
- Minor code refactoring, fixed few typos(thanks to James Ausmus)
- Change the naming of qgv point
masking/unmasking functions(James Ausmus).
- Simplify the masking/unmasking operation itself,
as we don't need to mask only single point per request(James Ausmus)
- Reject and stick to highest bandwidth point if SAGV
can't be enabled(BSpec)
v5:
- Add new mailbox reply codes, which seems to happen during boot
time for TGL and indicate that QGV setting is not yet available.
v6:
- Increase number of supported QGV points to be in sync with BSpec.
v7: - Rebased and resolved conflict to fix build failure.
- Fix NUM_QGV_POINTS to 8 and moved that to header file(James Ausmus)
v8: - Don't report an error if we can't restrict qgv points, as SAGV
can be disabled by BIOS, which is completely legal. So don't
make CI panic. Instead if we detect that there is only 1 QGV
point accessible just analyze if we can fit the required bandwidth
requirements, but no need in restricting.
v9: - Fix wrong QGV transition if we have 0 planes and no SAGV
simultaneously.
v10: - Fix CDCLK corruption, because of global state getting serialized
without modeset, which caused copying of non-calculated cdclk
to be copied to dev_priv(thanks to Ville for the hint).
v11: - Remove unneeded headers and spaces(Matthew Roper)
- Remove unneeded intel_qgv_info qi struct from bw check and zero
out the needed one(Matthew Roper)
- Changed QGV error message to have more clear meaning(Matthew Roper)
- Use state->modeset_set instead of any_ms(Matthew Roper)
- Moved NUM_SAGV_POINTS from i915_reg.h to i915_drv.h where it's used
- Keep using crtc_state->hw.active instead of .enable(Matthew Roper)
- Moved unrelated changes to other patch(using latency as parameter
for plane wm calculation, moved to SAGV refactoring patch)
v12: - Fix rebase conflict with own temporary SAGV/QGV fix.
- Remove unnecessary mask being zero check when unmasking
qgv points as this is completely legal(Matt Roper)
- Check if we are setting the same mask as already being set
in hardware to prevent error from PCode.
- Fix error message when restricting/unrestricting qgv points
to "mask/unmask" which sounds more accurate(Matt Roper)
- Move sagv status setting to icl_get_bw_info from atomic check
as this should be calculated only once.(Matt Roper)
- Edited comments for the case when we can't enable SAGV and
use only 1 QGV point with highest bandwidth to be more
understandable.(Matt Roper)
v13: - Moved max_data_rate in bw check to closer scope(Ville Syrjälä)
- Changed comment for zero new_mask in qgv points masking function
to better reflect reality(Ville Syrjälä)
- Simplified bit mask operation in qgv points masking function
(Ville Syrjälä)
- Moved intel_qgv_points_mask closer to gen11 SAGV disabling,
however this still can't be under modeset condition(Ville Syrjälä)
- Packed qgv_points_mask as u8 and moved closer to pipe_sagv_mask
(Ville Syrjälä)
- Extracted PCode changes to separate patch.(Ville Syrjälä)
- Now treat num_planes 0 same as 1 to avoid confusion and
returning max_bw as 0, which would prevent choosing QGV
point having max bandwidth in case if SAGV is not allowed,
as per BSpec(Ville Syrjälä)
- Do the actual qgv_points_mask swap in the same place as
all other global state parts like cdclk are swapped.
In the next patch, this all will be moved to bw state as
global state, once new global state patch series from Ville
lands
v14: - Now using global state to serialize access to qgv points
- Added global state locking back, otherwise we seem to read
bw state in a wrong way.
v15: - Added TODO comment for near atomic global state locking in
bw code.
v16: - Fixed intel_atomic_bw_* functions to be intel_bw_* as discussed
with Jani Nikula.
- Take bw_state_changed flag into use.
v17: - Moved qgv point related manipulations next to SAGV code, as
those are semantically related(Ville Syrjälä)
- Renamed those into intel_sagv_(pre)|(post)_plane_update
(Ville Syrjälä)
v18: - Move sagv related calls from commit tail into
intel_sagv_(pre)|(post)_plane_update(Ville Syrjälä)
v19: - Use intel_atomic_get_bw_(old)|(new)_state which is intended
for commit tail stage.
v20: - Return max bandwidth for 0 planes(Ville)
- Constify old_bw_state in bw_atomic_check(Ville)
- Removed some debugs(Ville)
- Added data rate to debug print when no QGV points(Ville)
- Removed some comments(Ville)
v21, v22, v23: - Fixed rebase conflict
v24: - Changed PCode mask to use ICL_ prefix
v25: - Resolved rebase conflict
v27: - Use device specific drm_err(Ville)
- Fixed parenthesis ident reported by checkpatch
Line over 100 warns to be fixed together with
existing code style.
Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Cc: Ville Syrjälä <ville.syrjala@intel.com> Cc: James Ausmus <james.ausmus@intel.com>
[vsyrjala: Drop duplicate intel_sagv_{pre,post}_plane_update() prototypes
and drop unused NUM_SAGV_POINTS define] Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200514074853.9508-3-stanislav.lisovskiy@intel.com
Starting from TGL we need to have a separate wm0
values for SAGV and non-SAGV which affects
how calculations are done.
v2: Remove long lines
v3: Removed COLOR_PLANE enum references
v4, v5, v6: Fixed rebase conflict
v7: - Removed skl_plane_wm_level accessor from skl_allocate_pipe_ddb(Ville)
- Removed sagv_uv_wm0(Ville)
- can_sagv->use_sagv_wm(Ville)
v8: - Moved tgl_crtc_can_enable_sagv function up(Ville)
- Changed comment regarding pipe_wm usage(Ville)
- Call intel_can_enable_sagv and tgl_compute_sagv_wm only
for Gen12(Ville)
- Some sagv debugs removed(Ville)
- skl_print_wm_changes improvements(Ville)
- Do assignment instead of memcpy in
skl_pipe_wm_get_hw_state(Ville)
v9: - Removed can_sagv variable(Ville)
- Removed spurious line(Ville)
- Changed u32 to unsigned int as agreed(Ville)
- Assign sagv only for gen12 in
skl_pipe_wm_get_hw_state(Ville)
Joonas Lahtinen [Thu, 14 May 2020 15:02:22 +0000 (18:02 +0300)]
Merge tag 'gvt-next-2020-05-12' of https://github.com/intel/gvt-linux into drm-intel-next-queued
gvt-next-2020-05-12
- Support PPGTT update via LRI cmd (Zhenyu)
- Remove extra kmap for shadow ctx update (Zhenyu)
- Move workload cleanup out of execlist handling code (Zhenyu)
Gwan-gyeong Mun [Thu, 14 May 2020 06:07:32 +0000 (09:07 +0300)]
drm/i915/psr: Use new DP VSC SDP compute routine on PSR
In order to use a common VSC SDP Colorimetry calculating code on PSR,
it uses a new psr vsc sdp compute routine.
Because PSR routine has its own scenario and timings of writing a VSC SDP,
the current PSR routine needs to have its own drm_dp_vsc_sdp structure
member variable on struct i915_psr.
In order to calculate colorimetry information, intel_psr_update()
function and intel_psr_enable() function extend a drm_connector_state
argument.
There are no changes to PSR mechanism.
v3: Replace a structure name to drm_dp_vsc_sdp from intel_dp_vsc_sdp
v4: Rebased
v8: Rebased
v10: When a PSR is enabled, it needs to add DP_SDP_VSC to
infoframes.enable.
It is needed for comparing between HW and pipe_state of VSC_SDP.
v11: If PSR is disabled by flag, it don't enable psr on pipe compute.
v12: Fix an inconsistent indenting
Gwan-gyeong Mun [Thu, 14 May 2020 06:07:31 +0000 (09:07 +0300)]
drm/i915/dp: Add compute routine for DP PSR VSC SDP
In order to use a common VSC SDP Colorimetry calculating code on PSR,
it adds a compute routine for PSR VSC SDP.
As PSR routine can not use infoframes.vsc of crtc state, it also adds new
writing of DP SDPs (Secondary Data Packet) for PSR.
PSR routine has its own scenario and timings of writing a VSC SDP.
v3: Replace a structure name to drm_dp_vsc_sdp from intel_dp_vsc_sdp
v4: Use struct drm_device logging macros
v10: 1) Fix packing of VSC SDP where Pixel Encoding/Colorimetry Format is
not supported.
2) Change a checking of PSR state.
Gwan-gyeong Mun [Thu, 14 May 2020 06:07:29 +0000 (09:07 +0300)]
drm/i915: Program DP SDPs on pipe updates
Call intel_dp_set_infoframes() function on pipe updates to make sure
that we send VSC SDP and HDR Metadata Infoframe SDP (when applicable)
on fastsets.
Gwan-gyeong Mun [Thu, 14 May 2020 06:07:28 +0000 (09:07 +0300)]
drm/i915: Fix enabled infoframe states of lspcon
Compared to implementation of DP and HDMI's encoder->infoframes_enabled,
the lspcon's implementation returns its active state. (we expect enabled
infoframe states of HW.) It leads to pipe state mismatch error
when ddi_get_config is called.
Because the current implementation of lspcon is not ready to support
readout infoframes, we need to return 0 here.
In order to support readout to lspcon, we need to implement read_infoframe
and infoframes_enabled. And set_infoframes also have to set an appropriate
bit on crtc_state->infoframes.enable
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 6c81e21a4742
("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: 6c81e21a4742 ("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: 23122a4d992b ("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: 2850748ef876 ("drm/i915: Pull i915_vma_pin under the vm->mutex") Fixes: b1e3177bd1d8 ("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 92581f9fb99c ("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 84eac0c65940 ("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 fb5970da1b42 ("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.
Zhenyu Wang [Fri, 8 May 2020 03:14:09 +0000 (11:14 +0800)]
drm/i915/gvt: Support PPGTT table load command
The PPGTT in context image can be overridden by LRI cmd with another
PPGTT's pdps. In such case, the load mm is used instead of the one in
the context image. So we need to load its shadow mm in GVT and replace
ppgtt pointers in command.
This feature is used by guest IGD driver to share gfx VM between
different contexts. Verified by IGT "gem_ctx_clone" test.
v4:
- consolidate shadow mm handlers (Yan)
- fix cmd shadow mm pin error path
v3: (Zhenyu Wang)
- Cleanup PDP register offset check
- Add debug check for guest context ppgtt update
- Skip 3-level ppgtt guest handling code. The reason is that all
guests now use 4-level ppgtt table and the only left case for
3-level table is ancient aliasing ppgtt case. But those guest
kernel has no use of PPGTT LRI command. So 3-level ppgtt guest
for this feature becomes simply un-testable.
v2: (Zhenyu Wang)
- Change to list for handling possible multiple ppgtt table loads
in one submission. Make sure shadow mm is to replace for each one.
Zhenyu Wang [Wed, 6 May 2020 09:43:17 +0000 (17:43 +0800)]
drm/i915/gvt: move workload destroy out of execlist complete
To let execlist.c only handle execlist handling and keep other
workload cleanup function in scheduler.c to align with other
workload specific handling there. This doesn't change current
code behavior.
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: d248b371f747 ("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: c81471f5e95c ("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 5a7d202b1574, 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.
projects / kernel.git / log