Our legacy cursor updates are actually mailbox updates.
Ie. the hardware latches things once per frame on start of
vblank, but we issue an number of updates per frame,
withough any attempt to synchronize against the vblank
in software. So in theory only the last update issued
during the frame will latch, and the previous ones are
discarded.
However this can lead to problems with maintaining the
ggtt/iommu mappings as we have no idea which updates
will actually latch.
The problem is exacerbated by the hardware's annoying disarming
behaviour; any non-arming register write will disarm an already
armed update, only to be rearmed later by the arming register
(CURBASE in case of cursors). If a disarming write happens
just before the start of vblank, and the arming write happens
after start of vblank we have effectively prevented the hardware
from latching anything. And if we manage to straddle multiple
sequential vblank starts in this manner we effectively prevent
the hardware from latching any new registers for an arbitrary
amount of time. This provides more time for the (potentially
still in use by the hardware) gtt/iommu mappings to be torn
down.
A partial solution, of course, is to use vblank evasion to
avoid the register writes from spreading on both sides of
the start of vblank.
I've previously highlighted this problem as a general issue
affecting mailbox updates. I even added some notes to the
{i9xx,skl}_crtc_planes_update_arm() to remind us that the noarm
and arm phases both need to pulled into the vblank evasion
critical section if we actually decided to implement mailbox
updates in general. But as I never impelemented the noarm+arm
split for cursors we don't have to worry about that for the
moment.
We've been lucky enough so far that this hasn't really caused
problems. One thing that does help is that Xorg generally
sticks to the same cursor BO. But igt seems pretty good at
hitting this on MTL now, so apparently we have to start
thinking about this.
v2: Wait for PSR exit to avoid the vblank evasion timeout (1ms)
tripping due to PSR exit latency (~5ms typically)
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240116204927.23499-1-ville.syrjala@linux.intel.com
Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
There isn't really any reason to make the caller suffer through
checking the vblank evasion min/max scanlines. If we somehow
ended up with bogus values (which really shouldn't happen)
then just skip the actual vblank evasion loop but otherwise
plow ahead as normal.
The only "real" change is that we now get+put a vblank reference
even if the min/max values are bogus, previously we skipped
directly to the end.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20231213102519.13500-7-ville.syrjala@linux.intel.com
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
On HSW non-ULT (or at least on Dell Latitude E6540) external displays
start to flicker when we enable PSR on the eDP. We observe a much higher
SR and PC6 residency than should be possible with an external display,
and indeen much higher than what we observe with eDP disabled and
only the external display enabled. Looks like the hardware is somehow
ignoring the fact that the external display is active during PSR.
I wasn't able to redproduce this on my HSW ULT machine, or BDW.
So either there's something specific about this particular laptop
(eg. some unknown firmware thing) or the issue is limited to just
non-ULT HSW systems. All known registers that could affect this
look perfectly reasonable on the affected machine.
As a workaround let's unmask the LPSP event to prevent PSR entry
except while in LPSP mode (only pipe A + eDP active). This
will prevent PSR entry entirely when multiple pipes are active.
The one slight downside is that we now also prevent PSR entry
when driving eDP with pipe B or C, but I think that's a reasonable
tradeoff to avoid having to implement a more complex workaround.
Cc: stable@vger.kernel.org
Fixes: 783d8b8087 ("drm/i915/psr: Re-enable PSR1 on hsw/bdw")
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10092
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240118212131.31868-1-ville.syrjala@linux.intel.com
Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
This reverts commit 88b065943c.
Lenovo 82TQ is unhappy if we do the display on sequence this
late. The display output shows severe corruption.
It's unclear if this is a failure on our part (perhaps
something to do with sending commands in LP mode after HS
/video mode transmission has been started? Though the backlight
on command at least seems to work) or simply that there are
some commands in the sequence that are needed to be done
earlier (eg. could be some DSC init stuff?). If the latter
then I don't think the current Windows code would work
either, but maybe this was originally tested with an older
driver, who knows.
Root causing this fully would likely require a lot of
experimentation which isn't really feasible without direct
access to the machine, so let's just accept failure and
go back to the original sequence.
Cc: stable@vger.kernel.org
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10071
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240116210821.30194-1-ville.syrjala@linux.intel.com
Acked-by: Jani Nikula <jani.nikula@intel.com>
Document nested struct members with full names as described in
Documentation/doc-guide/kernel-doc.rst.
intel_guc.h:305: warning: Excess struct member 'lock' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'guc_ids' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'num_guc_ids' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'guc_ids_bitmap' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'guc_id_list' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'guc_ids_in_use' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'destroyed_contexts' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'destroyed_worker' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'reset_fail_worker' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'reset_fail_mask' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'sched_disable_delay_ms' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'sched_disable_gucid_threshold' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'lock' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'gt_stamp' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'ping_delay' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'work' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'shift' description in 'intel_guc'
intel_guc.h:305: warning: Excess struct member 'last_stat_jiffies' description in 'intel_guc'
18 warnings as Errors
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20231226195432.10891-3-rdunlap@infradead.org
(cherry picked from commit e4cf1a70fa)
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
MST connectors don't have a static attached encoder, as their encoder
can change depending on the pipe they use; so the encoder for an MST
connector can't be retrieved using intel_dp_attached_encoder() (which
may return NULL for MST). Most of the PSR debugfs entries depend on a
static connector -> encoder mapping which is only true for eDP and SST
DP connectors and not for MST. These debugfs entries were enabled for
MST connectors as well recently to provide PR information for them, but
handling MST connectors needs more changes.
Fix this by not adding for now the PSR entries on MST connectors. To
make things more uniform add the entries for SST connectors on all
platforms, not just on platforms supporting DP2.0.
v2:
- Keep adding the entries for SST connectors. (Jouni)
- Add a TODO: comment for MST support.
Fixes: ef75c25e8f ("drm/i915/panelreplay: Debugfs support for panel replay")
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9850
Cc: Animesh Manna <animesh.manna@intel.com>
Cc: Jouni Högander <jouni.hogander@intel.com>
Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240103152609.2434100-1-imre.deak@intel.com
(cherry picked from commit 9b0b61c5bc)
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Add pll selection check for C20 as well as
clock state verification0. We have been relying
on sw state to select A or B pll's. This is incorrect
as the hw might see this selection differently. This
patch fixes this shortcoming by reading pll selection
for both sw and hw states and compares if these two
selections match.
Fixes: 59be90248b ("drm/i915/mtl: C20 state verification")
v2: reword commit message and include fix to a
original commit (Imre)
Compare pll selection (Jani)
Signed-off-by: Mika Kahola <mika.kahola@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240102115741.118525-2-mika.kahola@intel.com
(cherry picked from commit f4304beadd)
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
With Xe2_LPD, there were changes to the way CDCLK_CTL must be
programmed. Those were reflected on _bxt_set_cdclk() with commit
3d3696c0fe ("drm/i915/lnl: Start using CDCLK through PLL"), but
bxt_sanitize_cdclk() was left out.
This was causing some issues when loading the driver with a pre-existing
active display configuration: the driver would mistakenly take the
current value of CDCLK_CTL as wrong and the sanitization would be
triggered.
In a scenario where the display was already configured with a high
CDCLKC and had plane(s) enabled, FIFO underrun errors were reported,
because the current sanitization code selects the minimum possible
CDCLK.
Fix that by updating bxt_sanitize_cdclk() to match the changes made in
_bxt_set_cdclk(). Ideally, we would have a common function to derive the
value for CDCLK_CTL, but that can be done in a future change.
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240105140538.183553-2-gustavo.sousa@intel.com
An AUX transfer on any disconnected DP port results in long
timeout/retry delays the same way as this is described for TypeC port in
commit a972cd3f0e ("drm/i915/tc: Abort DP AUX transfer on a disconnected TC port")
Prevent the delay on non-TypeC ports as well by aborting the transfer if
the port is disconnected. For eDP keep the current behavior as the
support for HPD signaling is optional for it.
Link: https://patchwork.freedesktop.org/patch/msgid/20240104083008.2715733-13-imre.deak@intel.com
Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
