On Thu, 2024-01-04 at 10:30 +0200, Imre Deak wrote:
> Add hooks to intel_digital_port to lock and unlock the port and add a
> helper to check the connector's detect status while the port is
> locked
> already. This simplifies checking the connector detect status in
> intel_dp_aux_xfer() and intel_digital_port_connected() in the next
> two
> patches aborting AUX transfers on all DP connectors (except eDP) and
> filtering HPD glitches.
>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
Reviewed-by: Jouni Högander <jouni.hogander@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 3 ++
> .../drm/i915/display/intel_display_types.h | 3 ++
> drivers/gpu/drm/i915/display/intel_dp.c | 34
> +++++++++++++++++--
> drivers/gpu/drm/i915/display/intel_dp.h | 3 ++
> drivers/gpu/drm/i915/display/intel_dp_aux.c | 28 +++++++--------
> drivers/gpu/drm/i915/display/intel_tc.c | 15 +-------
> drivers/gpu/drm/i915/display/intel_tc.h | 1 -
> 7 files changed, 56 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 2746655bcb264..922194b957be2 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -5117,6 +5117,9 @@ void intel_ddi_init(struct drm_i915_private
> *dev_priv,
> encoder->suspend_complete =
> intel_ddi_tc_encoder_suspend_complete;
> encoder->shutdown_complete =
> intel_ddi_tc_encoder_shutdown_complete;
>
> + dig_port->lock = intel_tc_port_lock;
> + dig_port->unlock = intel_tc_port_unlock;
> +
> if (intel_tc_port_init(dig_port, is_legacy) < 0)
> goto err;
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index b9b9d9f2bc0ba..3556ccedbe4c1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1890,6 +1890,9 @@ struct intel_digital_port {
> u32 (*infoframes_enabled)(struct intel_encoder *encoder,
> const struct intel_crtc_state
> *pipe_config);
> bool (*connected)(struct intel_encoder *encoder);
> +
> + void (*lock)(struct intel_digital_port *dig_port);
> + void (*unlock)(struct intel_digital_port *dig_port);
> };
>
> struct intel_dp_mst_encoder {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 61c11f475f54a..f04926d4aa80d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5426,8 +5426,24 @@ edp_detect(struct intel_dp *intel_dp)
> return connector_status_connected;
> }
>
> +void intel_digital_port_lock(struct intel_encoder *encoder)
> +{
> + struct intel_digital_port *dig_port =
> enc_to_dig_port(encoder);
> +
> + if (dig_port->lock)
> + dig_port->lock(dig_port);
> +}
> +
> +void intel_digital_port_unlock(struct intel_encoder *encoder)
> +{
> + struct intel_digital_port *dig_port =
> enc_to_dig_port(encoder);
> +
> + if (dig_port->unlock)
> + dig_port->unlock(dig_port);
> +}
> +
> /*
> - * intel_digital_port_connected - is the specified port connected?
> + * intel_digital_port_connected_locked - is the specified port
> connected?
> * @encoder: intel_encoder
> *
> * In cases where there's a connector physically connected but it
> can't be used
> @@ -5435,9 +5451,12 @@ edp_detect(struct intel_dp *intel_dp)
> * pretty much treat the port as disconnected. This is relevant for
> type-C
> * (starting on ICL) where there's ownership involved.
> *
> + * The caller must hold the lock acquired by calling
> intel_digital_port_lock()
> + * when calling this function.
> + *
> * Return %true if port is connected, %false otherwise.
> */
> -bool intel_digital_port_connected(struct intel_encoder *encoder)
> +bool intel_digital_port_connected_locked(struct intel_encoder
> *encoder)
> {
> struct drm_i915_private *dev_priv = to_i915(encoder-
> >base.dev);
> struct intel_digital_port *dig_port =
> enc_to_dig_port(encoder);
> @@ -5450,6 +5469,17 @@ bool intel_digital_port_connected(struct
> intel_encoder *encoder)
> return is_connected;
> }
>
> +bool intel_digital_port_connected(struct intel_encoder *encoder)
> +{
> + bool ret;
> +
> + intel_digital_port_lock(encoder);
> + ret = intel_digital_port_connected_locked(encoder);
> + intel_digital_port_unlock(encoder);
> +
> + return ret;
> +}
> +
> static const struct drm_edid *
> intel_dp_get_edid(struct intel_dp *intel_dp)
> {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
> b/drivers/gpu/drm/i915/display/intel_dp.h
> index b911706d2e95e..530cc97bc42f4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -115,7 +115,10 @@ void intel_dp_set_infoframes(struct
> intel_encoder *encoder, bool enable,
> void intel_read_dp_sdp(struct intel_encoder *encoder,
> struct intel_crtc_state *crtc_state,
> unsigned int type);
> +void intel_digital_port_lock(struct intel_encoder *encoder);
> +void intel_digital_port_unlock(struct intel_encoder *encoder);
> bool intel_digital_port_connected(struct intel_encoder *encoder);
> +bool intel_digital_port_connected_locked(struct intel_encoder
> *encoder);
> int intel_dp_dsc_compute_max_bpp(const struct intel_connector
> *connector,
> u8 dsc_max_bpc);
> u16 intel_dp_dsc_get_max_compressed_bpp(struct drm_i915_private
> *i915,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> index 2e2af71bcd5a8..b36ef321e835e 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> @@ -9,6 +9,7 @@
> #include "intel_bios.h"
> #include "intel_de.h"
> #include "intel_display_types.h"
> +#include "intel_dp.h"
> #include "intel_dp_aux.h"
> #include "intel_dp_aux_regs.h"
> #include "intel_pps.h"
> @@ -228,6 +229,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> u32 aux_send_ctl_flags)
> {
> struct intel_digital_port *dig_port =
> dp_to_dig_port(intel_dp);
> + struct intel_encoder *encoder = &dig_port->base;
> struct drm_i915_private *i915 = to_i915(dig_port-
> >base.base.dev);
> enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
> bool is_tc_port = intel_phy_is_tc(i915, phy);
> @@ -245,18 +247,17 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> for (i = 0; i < ARRAY_SIZE(ch_data); i++)
> ch_data[i] = intel_dp->aux_ch_data_reg(intel_dp, i);
>
> - if (is_tc_port) {
> - intel_tc_port_lock(dig_port);
> - /*
> - * Abort transfers on a disconnected port as required
> by
> - * DP 1.4a link CTS 4.2.1.5, also avoiding the long
> AUX
> - * timeouts that would otherwise happen.
> - * TODO: abort the transfer on non-TC ports as well.
> - */
> - if (!intel_tc_port_connected_locked(&dig_port->base))
> {
> - ret = -ENXIO;
> - goto out_unlock;
> - }
> + intel_digital_port_lock(encoder);
> + /*
> + * Abort transfers on a disconnected port as required by
> + * DP 1.4a link CTS 4.2.1.5, also avoiding the long AUX
> + * timeouts that would otherwise happen.
> + * TODO: abort the transfer on non-TC ports as well.
> + */
> + if (is_tc_port &&
> + !intel_digital_port_connected_locked(&dig_port->base)) {
> + ret = -ENXIO;
> + goto out_unlock;
> }
>
> aux_domain = intel_aux_power_domain(dig_port);
> @@ -423,8 +424,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> intel_pps_unlock(intel_dp, pps_wakeref);
> intel_display_power_put_async(i915, aux_domain, aux_wakeref);
> out_unlock:
> - if (is_tc_port)
> - intel_tc_port_unlock(dig_port);
> + intel_digital_port_unlock(encoder);
>
> return ret;
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c
> b/drivers/gpu/drm/i915/display/intel_tc.c
> index dcf05e00e5052..80aed9e87927a 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.c
> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> @@ -1590,7 +1590,7 @@ void intel_tc_port_sanitize_mode(struct
> intel_digital_port *dig_port,
> * connected ports are usable, and avoids exposing to the users
> objects they
> * can't really use.
> */
> -bool intel_tc_port_connected_locked(struct intel_encoder *encoder)
> +bool intel_tc_port_connected(struct intel_encoder *encoder)
> {
> struct intel_digital_port *dig_port =
> enc_to_dig_port(encoder);
> struct drm_i915_private *i915 = to_i915(dig_port-
> >base.base.dev);
> @@ -1605,19 +1605,6 @@ bool intel_tc_port_connected_locked(struct
> intel_encoder *encoder)
> return tc_phy_hpd_live_status(tc) & mask;
> }
>
> -bool intel_tc_port_connected(struct intel_encoder *encoder)
> -{
> - struct intel_digital_port *dig_port =
> enc_to_dig_port(encoder);
> - struct intel_tc_port *tc = to_tc_port(dig_port);
> - bool is_connected;
> -
> - mutex_lock(&tc->lock);
> - is_connected = intel_tc_port_connected_locked(encoder);
> - mutex_unlock(&tc->lock);
> -
> - return is_connected;
> -}
> -
> static bool __intel_tc_port_link_needs_reset(struct intel_tc_port
> *tc)
> {
> bool ret;
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.h
> b/drivers/gpu/drm/i915/display/intel_tc.h
> index 80a61e52850ee..936fa2daaa74a 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.h
> +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> @@ -17,7 +17,6 @@ bool intel_tc_port_in_dp_alt_mode(struct
> intel_digital_port *dig_port);
> bool intel_tc_port_in_legacy_mode(struct intel_digital_port
> *dig_port);
>
> bool intel_tc_port_connected(struct intel_encoder *encoder);
> -bool intel_tc_port_connected_locked(struct intel_encoder *encoder);
>
> u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port
> *dig_port);
> int intel_tc_port_max_lane_count(struct intel_digital_port
> *dig_port);
