If the output on a DP-alt link with its sink disconnected is kept
enabled for too long (about 20 sec), then some IOM/TCSS firmware timeout
will cause havoc on the PCI bus, at least for other GFX devices on it
which will stop powering up. Since user space is not guaranteed to do a
disabling modeset in time, switch such disconnected but active links to
TBT mode - which is without such shortcomings - with a 2 second delay.
If the above condition is detected already during the driver load/system
resume sanitization step disable the output instead, as at that point no
user space or kernel client depends on a consistent output state yet and
because subsequent atomic modeset on such connectors - without the
actual sink capabilities available - can fail.
An active/disconnected port as above will also block the HPD status of
other active/disconnected ports to get updated (stuck in the connected
state), until the former port is disabled, its PHY is disconnected and
a ~10 ms delay has elapsed. This means the link state for all TypeC
ports/CRTCs must be rechecked after a CRTC is disabled due to the above
reason. For this disconnect the PHY synchronously after the CRTC/port is
disabled and recheck all CRTCs for the above condition whenever such a
port is disabled.
To account for a race condition during driver loading where the sink is
disconnected after the above sanitization step and before the HPD
interrupts get enabled, do an explicit check/link reset if needed from
the encoder's late_register hook, which is called after the HPD
interrupts are enabled already.
v2:
- Handle an active/disconnected port blocking the HPD state update of
another active/disconnected port.
- Cancel the delayed work resetting the link also from the encoder
enable/suspend/shutdown hooks.
- Rebase on the earlier intel_modeset_lock_ctx_retry() addition,
fixing here the missed atomic state reset in case of a retry.
- Fix handling of an error return from intel_atomic_get_crtc_state().
- Recheck if the port needs to be reset after all the atomic state
is locked and async commits are waited on.
v3:
- Add intel_crtc_needs_link_reset(), instead of open-coding it,
keep intel_crtc_has_encoders(). (Ville)
- Fix state dumping and use a bitmask to track disabled CRTCs in
intel_sanitize_all_crtcs(). (Ville)
- Set internal in intel_atomic_state right after allocating it.
(Ville)
- Recheck all CRTCs (not yet force-disabled) after a CRTC is
force-disabled for any reason (not only due to a link state)
in intel_sanitize_all_crtcs().
- Reduce delay after CRTC disabling to 20ms, and use the simpler
msleep().
- Clarify code comment about HPD behaviour in
intel_sanitize_all_crtcs().
- Move all the TC link reset logic to intel_tc.c .
- Cancel the link reset work synchronously during system suspend,
driver unload and shutdown.
v4:
- Rebased on previous patch, which allows calling the TC port
suspend/cleanup handlers without modeset locks held; remove the
display driver suspended assert from the link reset work
accordingly.
v5: (Ville)
- Remove reset work canceling from intel_ddi_pre_pll_enable().
- Track a crtc vs. pipe mask in intel_sanitize_all_crtcs().
- Add reset_link_commit() to clarify the
intel_modeset_lock_ctx_retry loop.
Cc: Kai-Heng Feng <email address hidden>
Cc: Ville Syrjälä <email address hidden>
Tested-by: Kai-Heng Feng <email address hidden>
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5860
Reviewed-by: Ville Syrjälä <email address hidden>
Signed-off-by: Imre Deak <email address hidden>
Link: https://patchwork<email address hidden>
(backported from commit c598c335da420715670b1adac846e4f3ebd01e40)
[khfeng: resolve different headers, remove not-yet-introduced variable]
Signed-off-by: Kai-Heng Feng <email address hidden>
Signed-off-by: Timo Aaltonen <email address hidden>
Acked-by: Stefan Bader <email address hidden>
Acked-by: Tim Gardner <email address hidden>
Signed-off-by: Roxana Nicolescu <email address hidden>
Call the TypeC port flush_work and cleanup handlers without the modeset
locks held. These don't require the locks, as the work takes - as it
should be able to at any point in time - any locks it needs and by the
time cleanup is called and after cleanup returns the encoder is not in
use.
This is required by the next patch canceling a TypeC port work
synchronously during encoder suspend and shutdown, where the work can
take modeset locks as well, hence the canceling must be done without
holding the locks.
I also considered moving the modeset locking down to each encoder
suspend()/shutdown() hook instead, however locking the full modeset
state for each encoder separately would be odd, and the bigger change -
affecting all encoders - is beyond the scope of this patchset.
v2:
- Add a TODO: comment to remove modeset locks if no encoder depends
on this. (Ville)
Cc: Ville Syrjälä <email address hidden>
Reviewed-by: Ville Syrjälä <email address hidden>
Signed-off-by: Imre Deak <email address hidden>
Link: https://patchwork<email address hidden>
(cherry picked from commit b61fad5f7e5d859d95a413c3a57f59d007951fa6)
Signed-off-by: Kai-Heng Feng <email address hidden>
Acked-by: Stefan Bader <email address hidden>
Acked-by: Tim Gardner <email address hidden>
Signed-off-by: Roxana Nicolescu <email address hidden>
This patch simplifying the handling of modeset locks and atomic state
for an atomic commit is based on
https://<email address hidden>/
adding the helper to i915. I find this approach preferrable than
open-coding the corresponding steps (fixed for me an atomic
state reset during a DEADLK retry, which I missed in the open-coded
version) and also better than the existing
DRM_MODESET_LOCK_ALL_BEGIN/END macros for the reasons described in the
above original patchset.
This change takes the helper into use only for atomic commits during DDI
hotplug handling, as a preparation for a follow-up patch adding a
similar commit started from the same spot. Other places doing a
driver-internal atomic commit is to be converted by a follow-up
patchset.
Cc: Ville Syrjälä <email address hidden>
Reviewed-by: Ville Syrjälä <email address hidden>
Signed-off-by: Imre Deak <email address hidden>
Link: https://patchwork<email address hidden>
(cherry picked from commit 60ded7cc86f363161e37dc41c548b2ab3e1af5ce)
Signed-off-by: Kai-Heng Feng <email address hidden>
Acked-by: Stefan Bader <email address hidden>
Acked-by: Tim Gardner <email address hidden>
Signed-off-by: Roxana Nicolescu <email address hidden>
Prevent downgrading the link training maximum lane count/rate if the
sink is disconnected - and so the link training failure is expected. In
such cases modeset failures due to the reduced max link params would be
just confusing for user space (instead of which the correct thing it
should act on is the sink disconnect signaled by a hotplug event,
requiring a disabling modeset).
v2:
- Check the actual HPD state to handle the forced connector state case.
(Vinod)
Cc: Ville Syrjälä <email address hidden>
Cc: Vinod Govindapillai <email address hidden>
Reviewed-by: Ville Syrjälä <email address hidden> (v1)
Reviewed-by: Vinod Govindapillai <email address hidden>
Signed-off-by: Imre Deak <email address hidden>
Link: https://patchwork<email address hidden>
(backported from commit f45156ff18bae00ee56ed6aa2a937a8e93e56d7f)
[khfeng: didn't backport new printk macros, stick to the old way]
Signed-off-by: Kai-Heng Feng <email address hidden>
Signed-off-by: Timo Aaltonen <email address hidden>
Acked-by: Stefan Bader <email address hidden>
Acked-by: Tim Gardner <email address hidden>
Signed-off-by: Roxana Nicolescu <email address hidden>
During HW readout/sanitization CRTCs can be disabled only if they don't
have an attached encoder (and so the encoder disable hooks don't need to
be called). An upcoming patch will need to disable CRTCs also with an
attached encoder, so add support for this.
For bigjoiner configs the encoder disabling hooks require the slave CRTC
states, so add these too to the atomic state. Since the connector atomic
state is already up-to-date when the CRTC is disabled the connector
state needs to be updated (reset) after the CRTC is disabled, make this
so. Follow the proper order of disabling first all bigjoiner slaves,
then any port synced CRTC slaves followed by the CRTC originally
requested to be disabled.
v2:
- Fix calculating the bigjoiner_masters mask in a port sync config,
(Ville)
- Keep _noatomic suffix in intel_crtc_disable_noatomic(). (Ville)
- Rebase on full CRTC state reset in this patchset, not requiring
resetting the bigjoiner state separately and (instead) resetting
the full atomic CRTC and related global state after all linked
pipes got disabled.
- Disable portsync slaves before a portsync master.
- Disable a portsync master if a linked portsync slave is disabled.
v3: (Ville)
- Use s/u32/u8 for transcoder and pipe masks.
- Use is_power_of_2() instead of hweight()==1.
Cc: Ville Syrjälä <email address hidden>
Reviewed-by: Ville Syrjälä <email address hidden>
Signed-off-by: Imre Deak <email address hidden>
Link: https://patchwork<email address hidden>
(cherry picked from commit e826839e18b77edb9be622a505d34e883985df48)
Signed-off-by: Kai-Heng Feng <email address hidden>
Signed-off-by: Timo Aaltonen <email address hidden>
Acked-by: Stefan Bader <email address hidden>
Acked-by: Tim Gardner <email address hidden>
Signed-off-by: Roxana Nicolescu <email address hidden>
Split calling the CRTC/encoder disabling hooks and updating the CRTC and
DPLL object states from updating the CRTC and atomic state and other
global state (BW, CDCLK, DBUF) into separate functions. When disabling a
bigjoiner configuration the latter step can be done only after all the
linked pipes are disabled, so this change prepares for that.
No functional changes.
Cc: Ville Syrjälä <email address hidden>
Reviewed-by: Ville Syrjälä <email address hidden>
Signed-off-by: Imre Deak <email address hidden>
Link: https://patchwork<email address hidden>
(cherry picked from commit 3ad41442d7bf5b3af0de927e14ed92b39da68224)
Signed-off-by: Kai-Heng Feng <email address hidden>
Signed-off-by: Timo Aaltonen <email address hidden>
Acked-by: Stefan Bader <email address hidden>
Acked-by: Tim Gardner <email address hidden>
Signed-off-by: Roxana Nicolescu <email address hidden>
During HW state readout/sanitization an up-to-date connector atomic
state will be required by a follow-up patch, which can disable CRTCs
with an encoder (and calling the correct encoder hooks happens via the
connector atomic state encoder pointer). So update the connector state
already before the CRTC sanitize/disable step. For now this doesn't make
a difference, since intel_modeset_update_connector_atomic_state() will
update/enable the atomic state only for connectors that have an enabled
encoder/CRTC. Such CRTCs/encoders will not be affected by
intel_sanitize_crtc().
v2: Add comment about why the connector state needs to be up-to-date.
Cc: Ville Syrjälä <email address hidden>
Reviewed-by: Ville Syrjälä <email address hidden>
Signed-off-by: Imre Deak <email address hidden>
Link: https://patchwork<email address hidden>
(cherry picked from commit db4069fcbdc5c8bc03424934a3395b39b71d9dc6)
Signed-off-by: Kai-Heng Feng <email address hidden>
Signed-off-by: Timo Aaltonen <email address hidden>
Acked-by: Stefan Bader <email address hidden>
Acked-by: Tim Gardner <email address hidden>
Signed-off-by: Roxana Nicolescu <email address hidden>
