[Intel-gfx] [PATCH 00/11] drm/i915/tc: Add a workaround for an IOM/TCSS firmware hang issue

[Intel-gfx] [PATCH 00/11] drm/i915/tc: Add a workaround for an IOM/TCSS firmware hang issue

[Imre Deak]

If a TypeC/DP-alt output is not disabled in time some IOM/TCSS firmware
component will time-out and block at least the PCI power management
operations for other PCI GFX devices. This patchset adds a workaround
that performs an in-kernel modeset (either disabling or switching to
TBT-alt mode) on such connectors to work around this issue.

Imre Deak (11):
  drm/i915: Fix PIPEDMC disabling for a bigjoiner configuration
  drm/i915: Make the CRTC wrt. CSC state consistent during
    sanitize-disabling
  drm/i915: Update connector atomic state before crtc sanitize-disabling
  drm/i915: Factor out set_encoder_for_connector()
  drm/i915: Add support for disabling any CRTCs during HW
    readout/sanitization
  drm/i915/dp: Add link training debug and error printing helpers
  drm/i915/dp: Convert link training error to debug message on
    disconnected sink
  drm/i915/dp: Prevent link training fallback on disconnected port
  drm/i915/dp: Factor out intel_dp_get_active_pipes()
  drm/i915: Factor out call_with_modeset_ctx()
  drm/i915/tc: Reset TypeC PHYs left enabled in DP-alt mode after the
    sink disconnects

 drivers/gpu/drm/i915/display/intel_ddi.c      |  92 ++++-
 drivers/gpu/drm/i915/display/intel_display.c  |  12 +-
 .../drm/i915/display/intel_display_types.h    |   2 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  71 +++-
 drivers/gpu/drm/i915/display/intel_dp.h       |   4 +
 .../drm/i915/display/intel_dp_link_training.c | 388 +++++++-----------
 .../drm/i915/display/intel_modeset_setup.c    | 202 +++++++--
 drivers/gpu/drm/i915/display/intel_tc.c       |  21 +
 drivers/gpu/drm/i915/display/intel_tc.h       |   1 +
 9 files changed, 488 insertions(+), 305 deletions(-)

-- 
2.37.2

[Intel-gfx] [PATCH 01/11] drm/i915: Fix PIPEDMC disabling for a bigjoiner configuration

[Imre Deak]

For a bigjoiner configuration display->crtc_disable() will be called
first for the slave CRTCs and then for the master CRTC. However slave
CRTCs will be actually disabled only after the master CRTC is disabled
(from the encoder disable hooks called with the master CRTC state).
Hence the slave PIPEDMCs can be disabled only after the master CRTC is
disabled, make this so.

intel_encoders_post_pll_disable() must be called only for the master
CRTC, as for the other two encoder disable hooks. While at it fix this
up as well. This didn't cause a problem, since
intel_encoders_post_pll_disable() will call the corresponding hook only
for an encoder/connector connected to the given CRTC, however slave
CRTCs will have no associated encoder/connector.

Fixes: 3af2ff0840be ("drm/i915: Enable a PIPEDMC whenever its corresponding pipe is enabled")
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index bf391a6cd8d68..722b4c47379d5 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1700,9 +1700,17 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
 
 	intel_disable_shared_dpll(old_crtc_state);
 
-	intel_encoders_post_pll_disable(state, crtc);
+	if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) {
+		struct intel_crtc *slave_crtc;
+
+		intel_encoders_post_pll_disable(state, crtc);
 
-	intel_dmc_disable_pipe(i915, crtc->pipe);
+		intel_dmc_disable_pipe(i915, crtc->pipe);
+
+		for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
+						 intel_crtc_bigjoiner_slave_pipes(old_crtc_state))
+			intel_dmc_disable_pipe(i915, slave_crtc->pipe);
+	}
 }
 
 static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state)
-- 
2.37.2

[Intel-gfx] [PATCH 02/11] drm/i915: Make the CRTC wrt. CSC state consistent during sanitize-disabling

[Imre Deak]

intel_crtc_free_hw_state() frees all the Intel specific CSC blobs in the
CRTC state, but the next memset() will only clear the corresponding
pointers for the ones stored in intel_crtc_state:hw. Clear the ones
stored in intel_crtc_state as well. Also sync the UAPI state with the HW
state after the HW state was reset. This will reset the uapi.active
flag as well, so no need to do this separately. Syncing the state will
create a new umode blob, so move deleting the blob after the sync call.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_modeset_setup.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
index eefa4018dc0c2..57d087de654f8 100644
--- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
+++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
@@ -30,6 +30,8 @@
 #include "intel_wm.h"
 #include "skl_watermark.h"
 
+static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state *crtc_state);
+
 static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
 					struct drm_modeset_acquire_ctx *ctx)
 {
@@ -88,13 +90,17 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
 	crtc->active = false;
 	crtc->base.enabled = false;
 
-	drm_WARN_ON(&i915->drm,
-		    drm_atomic_set_mode_for_crtc(&crtc_state->uapi, NULL) < 0);
-	crtc_state->uapi.active = false;
 	crtc_state->uapi.connector_mask = 0;
 	crtc_state->uapi.encoder_mask = 0;
+
 	intel_crtc_free_hw_state(crtc_state);
 	memset(&crtc_state->hw, 0, sizeof(crtc_state->hw));
+	crtc_state->pre_csc_lut = NULL;
+	crtc_state->post_csc_lut = NULL;
+	intel_crtc_copy_hw_to_uapi_state(crtc_state);
+
+	drm_WARN_ON(&i915->drm,
+		    drm_atomic_set_mode_for_crtc(&crtc_state->uapi, NULL) < 0);
 
 	for_each_encoder_on_crtc(&i915->drm, &crtc->base, encoder)
 		encoder->base.crtc = NULL;
-- 
2.37.2

Re: [Intel-gfx] [PATCH 02/11] drm/i915: Make the CRTC wrt. CSC state consistent during sanitize-disabling

[Ville Syrjälä]

On Wed, Apr 26, 2023 at 07:52:56PM +0300, Imre Deak wrote:
> intel_crtc_free_hw_state() frees all the Intel specific CSC blobs in the
> CRTC state, but the next memset() will only clear the corresponding
> pointers for the ones stored in intel_crtc_state:hw. Clear the ones
> stored in intel_crtc_state as well. Also sync the UAPI state with the HW
> state after the HW state was reset. This will reset the uapi.active
> flag as well, so no need to do this separately. Syncing the state will
> create a new umode blob, so move deleting the blob after the sync call.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_modeset_setup.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> index eefa4018dc0c2..57d087de654f8 100644
> --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> @@ -30,6 +30,8 @@
>  #include "intel_wm.h"
>  #include "skl_watermark.h"
>  
> +static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state *crtc_state);
> +
>  static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
>  					struct drm_modeset_acquire_ctx *ctx)
>  {
> @@ -88,13 +90,17 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
>  	crtc->active = false;
>  	crtc->base.enabled = false;
>  
> -	drm_WARN_ON(&i915->drm,
> -		    drm_atomic_set_mode_for_crtc(&crtc_state->uapi, NULL) < 0);
> -	crtc_state->uapi.active = false;
>  	crtc_state->uapi.connector_mask = 0;
>  	crtc_state->uapi.encoder_mask = 0;
> +
>  	intel_crtc_free_hw_state(crtc_state);
>  	memset(&crtc_state->hw, 0, sizeof(crtc_state->hw));
> +	crtc_state->pre_csc_lut = NULL;
> +	crtc_state->post_csc_lut = NULL;
> +	intel_crtc_copy_hw_to_uapi_state(crtc_state);
> +
> +	drm_WARN_ON(&i915->drm,
> +		    drm_atomic_set_mode_for_crtc(&crtc_state->uapi, NULL) < 0);

Hmm. Is there a reason we can't just do the full destroy+reset
thing that intel_modeset_readout_hw_state() does?

>  
>  	for_each_encoder_on_crtc(&i915->drm, &crtc->base, encoder)
>  		encoder->base.crtc = NULL;
> -- 
> 2.37.2

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 02/11] drm/i915: Make the CRTC wrt. CSC state consistent during sanitize-disabling

[Imre Deak]

On Wed, Apr 26, 2023 at 08:48:57PM +0300, Ville Syrjälä wrote:
> On Wed, Apr 26, 2023 at 07:52:56PM +0300, Imre Deak wrote:
> > intel_crtc_free_hw_state() frees all the Intel specific CSC blobs in the
> > CRTC state, but the next memset() will only clear the corresponding
> > pointers for the ones stored in intel_crtc_state:hw. Clear the ones
> > stored in intel_crtc_state as well. Also sync the UAPI state with the HW
> > state after the HW state was reset. This will reset the uapi.active
> > flag as well, so no need to do this separately. Syncing the state will
> > create a new umode blob, so move deleting the blob after the sync call.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_modeset_setup.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > index eefa4018dc0c2..57d087de654f8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > @@ -30,6 +30,8 @@
> >  #include "intel_wm.h"
> >  #include "skl_watermark.h"
> >  
> > +static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state *crtc_state);
> > +
> >  static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
> >  					struct drm_modeset_acquire_ctx *ctx)
> >  {
> > @@ -88,13 +90,17 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
> >  	crtc->active = false;
> >  	crtc->base.enabled = false;
> >  
> > -	drm_WARN_ON(&i915->drm,
> > -		    drm_atomic_set_mode_for_crtc(&crtc_state->uapi, NULL) < 0);
> > -	crtc_state->uapi.active = false;
> >  	crtc_state->uapi.connector_mask = 0;
> >  	crtc_state->uapi.encoder_mask = 0;
> > +
> >  	intel_crtc_free_hw_state(crtc_state);
> >  	memset(&crtc_state->hw, 0, sizeof(crtc_state->hw));
> > +	crtc_state->pre_csc_lut = NULL;
> > +	crtc_state->post_csc_lut = NULL;
> > +	intel_crtc_copy_hw_to_uapi_state(crtc_state);
> > +
> > +	drm_WARN_ON(&i915->drm,
> > +		    drm_atomic_set_mode_for_crtc(&crtc_state->uapi, NULL) < 0);
> 
> Hmm. Is there a reason we can't just do the full destroy+reset
> thing that intel_modeset_readout_hw_state() does?

Yes, that's better and works. It revealed that the (global) shared_dpll
state needs to be also updated here.

I'd like to use this function to disable bigjoiner configs as well
(later in this patchset), where the slave CRTC state is used during the
master CRTC disabling (in the encoder post_disable hook); so is it ok to
first disable all related CRTCs and then reset the state for all (also
moving the fbc disabling and watermark updating before the state
update)?

Both of the above changes are at
https://github.com/ideak/linux/commit/fc9b011249112369
on top of this patchset.

Also is it ok to keep the full CRTC reset change as a separate patch
(for the purpose of stable backporting the rest).

> 
> >  
> >  	for_each_encoder_on_crtc(&i915->drm, &crtc->base, encoder)
> >  		encoder->base.crtc = NULL;
> > -- 
> > 2.37.2
> 
> -- 
> Ville Syrjälä
> Intel

[Intel-gfx] [PATCH 03/11] drm/i915: Update connector atomic state before crtc sanitize-disabling

[Imre Deak]

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().

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_modeset_setup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
index 57d087de654f8..fe83579529b3a 100644
--- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
+++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
@@ -706,6 +706,8 @@ void intel_modeset_setup_hw_state(struct drm_i915_private *i915,
 	for_each_intel_encoder(&i915->drm, encoder)
 		intel_sanitize_encoder(encoder);
 
+	intel_modeset_update_connector_atomic_state(i915);
+
 	for_each_intel_crtc(&i915->drm, crtc) {
 		struct intel_crtc_state *crtc_state =
 			to_intel_crtc_state(crtc->base.state);
@@ -714,8 +716,6 @@ void intel_modeset_setup_hw_state(struct drm_i915_private *i915,
 		intel_crtc_state_dump(crtc_state, NULL, "setup_hw_state");
 	}
 
-	intel_modeset_update_connector_atomic_state(i915);
-
 	intel_dpll_sanitize_state(i915);
 
 	intel_wm_get_hw_state(i915);
-- 
2.37.2

[Intel-gfx] [PATCH 04/11] drm/i915: Factor out set_encoder_for_connector()

[Imre Deak]

Factor out a function setting the encoder and CRTC in the connector
atomic state, required by a follow up patch.

No functional changes.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 .../drm/i915/display/intel_modeset_setup.c    | 28 +++++++++++++------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
index fe83579529b3a..a66085cf82bab 100644
--- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
+++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
@@ -32,6 +32,24 @@
 
 static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state *crtc_state);
 
+static void set_encoder_for_connector(struct intel_connector *connector,
+				      struct intel_encoder *encoder)
+{
+	struct drm_connector_state *conn_state = connector->base.state;
+
+	if (conn_state->crtc)
+		drm_connector_put(&connector->base);
+
+	if (encoder) {
+		conn_state->best_encoder = &encoder->base;
+		conn_state->crtc = encoder->base.crtc;
+		drm_connector_get(&connector->base);
+	} else {
+		conn_state->best_encoder = NULL;
+		conn_state->crtc = NULL;
+	}
+}
+
 static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
 					struct drm_modeset_acquire_ctx *ctx)
 {
@@ -131,8 +149,7 @@ static void intel_modeset_update_connector_atomic_state(struct drm_i915_private
 		struct intel_encoder *encoder =
 			to_intel_encoder(connector->base.encoder);
 
-		if (conn_state->crtc)
-			drm_connector_put(&connector->base);
+		set_encoder_for_connector(connector, encoder);
 
 		if (encoder) {
 			struct intel_crtc *crtc =
@@ -140,14 +157,7 @@ static void intel_modeset_update_connector_atomic_state(struct drm_i915_private
 			const struct intel_crtc_state *crtc_state =
 				to_intel_crtc_state(crtc->base.state);
 
-			conn_state->best_encoder = &encoder->base;
-			conn_state->crtc = &crtc->base;
 			conn_state->max_bpc = (crtc_state->pipe_bpp ?: 24) / 3;
-
-			drm_connector_get(&connector->base);
-		} else {
-			conn_state->best_encoder = NULL;
-			conn_state->crtc = NULL;
 		}
 	}
 	drm_connector_list_iter_end(&conn_iter);
-- 
2.37.2

[Intel-gfx] [PATCH 05/11] drm/i915: Add support for disabling any CRTCs during HW readout/sanitization

[Imre Deak]

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 an 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.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 .../drm/i915/display/intel_modeset_setup.c    | 124 ++++++++++++++++--
 1 file changed, 115 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
index a66085cf82bab..f613c074187a2 100644
--- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
+++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
@@ -50,10 +50,39 @@ static void set_encoder_for_connector(struct intel_connector *connector,
 	}
 }
 
+static void reset_encoder_connector_state(struct intel_encoder *encoder)
+{
+	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+	struct intel_connector *connector;
+	struct drm_connector_list_iter conn_iter;
+
+	drm_connector_list_iter_begin(&i915->drm, &conn_iter);
+	for_each_intel_connector_iter(connector, &conn_iter) {
+		if (connector->base.encoder != &encoder->base)
+			continue;
+
+		set_encoder_for_connector(connector, NULL);
+
+		connector->base.dpms = DRM_MODE_DPMS_OFF;
+		connector->base.encoder = NULL;
+	}
+	drm_connector_list_iter_end(&conn_iter);
+}
+
+static void reset_crtc_encoder_state(struct intel_crtc *crtc)
+{
+	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
+	struct intel_encoder *encoder;
+
+	for_each_encoder_on_crtc(&i915->drm, &crtc->base, encoder) {
+		reset_encoder_connector_state(encoder);
+		encoder->base.crtc = NULL;
+	}
+}
+
 static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
 					struct drm_modeset_acquire_ctx *ctx)
 {
-	struct intel_encoder *encoder;
 	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
 	struct intel_bw_state *bw_state =
 		to_intel_bw_state(i915->display.bw.obj.state);
@@ -65,9 +94,8 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
 		to_intel_crtc_state(crtc->base.state);
 	struct intel_plane *plane;
 	struct drm_atomic_state *state;
-	struct intel_crtc_state *temp_crtc_state;
+	struct intel_crtc *temp_crtc;
 	enum pipe pipe = crtc->pipe;
-	int ret;
 
 	if (!crtc_state->hw.active)
 		return;
@@ -92,10 +120,17 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
 	to_intel_atomic_state(state)->internal = true;
 
 	/* Everything's already locked, -EDEADLK can't happen. */
-	temp_crtc_state = intel_atomic_get_crtc_state(state, crtc);
-	ret = drm_atomic_add_affected_connectors(state, &crtc->base);
+	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc,
+					 BIT(pipe) |
+					 intel_crtc_bigjoiner_slave_pipes(crtc_state)) {
+		struct intel_crtc_state *temp_crtc_state =
+			intel_atomic_get_crtc_state(state, temp_crtc);
+		int ret;
 
-	drm_WARN_ON(&i915->drm, IS_ERR(temp_crtc_state) || ret);
+		ret = drm_atomic_add_affected_connectors(state, &temp_crtc->base);
+
+		drm_WARN_ON(&i915->drm, IS_ERR(temp_crtc_state) || ret);
+	}
 
 	i915->display.funcs.display->crtc_disable(to_intel_atomic_state(state), crtc);
 
@@ -120,8 +155,7 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
 	drm_WARN_ON(&i915->drm,
 		    drm_atomic_set_mode_for_crtc(&crtc_state->uapi, NULL) < 0);
 
-	for_each_encoder_on_crtc(&i915->drm, &crtc->base, encoder)
-		encoder->base.crtc = NULL;
+	reset_crtc_encoder_state(crtc);
 
 	intel_fbc_disable(crtc);
 	intel_update_watermarks(i915);
@@ -272,6 +306,77 @@ static void intel_sanitize_fifo_underrun_reporting(const struct intel_crtc_state
 					   !HAS_GMCH(i915));
 }
 
+static u32 get_transcoder_pipes(struct drm_i915_private *i915, u32 transcoder_mask)
+{
+	struct intel_crtc *slave_crtc;
+	u32 pipes = 0;
+
+	for_each_intel_crtc(&i915->drm, slave_crtc) {
+		struct intel_crtc_state *slave_crtc_state =
+			to_intel_crtc_state(slave_crtc->base.state);
+
+		if (slave_crtc_state->cpu_transcoder == INVALID_TRANSCODER)
+			continue;
+
+		if (transcoder_mask & BIT(slave_crtc_state->cpu_transcoder))
+			pipes |= BIT(slave_crtc->pipe);
+	}
+
+	return pipes;
+}
+
+static u32 get_bigjoiner_slave_pipes(struct drm_i915_private *i915, u32 master_pipes)
+{
+	struct intel_crtc *master_crtc;
+	u32 pipes = 0;
+
+	for_each_intel_crtc_in_pipe_mask(&i915->drm, master_crtc, master_pipes) {
+		struct intel_crtc_state *master_crtc_state =
+			to_intel_crtc_state(master_crtc->base.state);
+
+		pipes |= intel_crtc_bigjoiner_slave_pipes(master_crtc_state);
+	}
+
+	return pipes;
+}
+
+static void kill_bigjoiner_slave_noatomic(struct intel_crtc *master_crtc)
+{
+	struct drm_i915_private *i915 = to_i915(master_crtc->base.dev);
+	struct intel_crtc_state *master_crtc_state =
+		to_intel_crtc_state(master_crtc->base.state);
+	struct intel_crtc *slave_crtc;
+
+	for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
+					 intel_crtc_bigjoiner_slave_pipes(master_crtc_state)) {
+		struct intel_crtc_state *slave_crtc_state =
+			to_intel_crtc_state(slave_crtc->base.state);
+
+		slave_crtc_state->bigjoiner_pipes = 0;
+	}
+
+	master_crtc_state->bigjoiner_pipes = 0;
+}
+
+static void disable_crtc_with_slaves(struct intel_crtc *crtc,
+				     struct drm_modeset_acquire_ctx *ctx)
+{
+	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
+	struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state);
+	u32 bigjoiner_masters = BIT(crtc->pipe) |
+				get_transcoder_pipes(i915, crtc_state->sync_mode_slaves_mask);
+	u32 bigjoiner_slaves = get_bigjoiner_slave_pipes(i915, bigjoiner_masters);
+	struct intel_crtc *temp_crtc;
+
+	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc, bigjoiner_slaves)
+		intel_crtc_disable_noatomic(temp_crtc, ctx);
+
+	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc, bigjoiner_masters) {
+		intel_crtc_disable_noatomic(temp_crtc, ctx);
+		kill_bigjoiner_slave_noatomic(temp_crtc);
+	}
+}
+
 static void intel_sanitize_crtc(struct intel_crtc *crtc,
 				struct drm_modeset_acquire_ctx *ctx)
 {
@@ -299,10 +404,11 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
 	/*
 	 * Adjust the state of the output pipe according to whether we have
 	 * active connectors/encoders.
+	 * TODO: Add support for MST
 	 */
 	if (crtc_state->hw.active && !intel_crtc_has_encoders(crtc) &&
 	    !intel_crtc_is_bigjoiner_slave(crtc_state))
-		intel_crtc_disable_noatomic(crtc, ctx);
+		disable_crtc_with_slaves(crtc, ctx);
 }
 
 static bool has_bogus_dpll_config(const struct intel_crtc_state *crtc_state)
-- 
2.37.2

Re: [Intel-gfx] [PATCH 05/11] drm/i915: Add support for disabling any CRTCs during HW readout/sanitization

[Ville Syrjälä]

On Wed, Apr 26, 2023 at 07:52:59PM +0300, Imre Deak wrote:
> 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 an 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.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  .../drm/i915/display/intel_modeset_setup.c    | 124 ++++++++++++++++--
>  1 file changed, 115 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> index a66085cf82bab..f613c074187a2 100644
> --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> @@ -50,10 +50,39 @@ static void set_encoder_for_connector(struct intel_connector *connector,
>  	}
>  }
>  
> +static void reset_encoder_connector_state(struct intel_encoder *encoder)
> +{
> +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +	struct intel_connector *connector;
> +	struct drm_connector_list_iter conn_iter;
> +
> +	drm_connector_list_iter_begin(&i915->drm, &conn_iter);
> +	for_each_intel_connector_iter(connector, &conn_iter) {
> +		if (connector->base.encoder != &encoder->base)
> +			continue;
> +
> +		set_encoder_for_connector(connector, NULL);
> +
> +		connector->base.dpms = DRM_MODE_DPMS_OFF;
> +		connector->base.encoder = NULL;
> +	}
> +	drm_connector_list_iter_end(&conn_iter);
> +}
> +
> +static void reset_crtc_encoder_state(struct intel_crtc *crtc)
> +{
> +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	struct intel_encoder *encoder;
> +
> +	for_each_encoder_on_crtc(&i915->drm, &crtc->base, encoder) {
> +		reset_encoder_connector_state(encoder);
> +		encoder->base.crtc = NULL;
> +	}
> +}
> +
>  static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
>  					struct drm_modeset_acquire_ctx *ctx)
>  {
> -	struct intel_encoder *encoder;
>  	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
>  	struct intel_bw_state *bw_state =
>  		to_intel_bw_state(i915->display.bw.obj.state);
> @@ -65,9 +94,8 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
>  		to_intel_crtc_state(crtc->base.state);
>  	struct intel_plane *plane;
>  	struct drm_atomic_state *state;
> -	struct intel_crtc_state *temp_crtc_state;
> +	struct intel_crtc *temp_crtc;
>  	enum pipe pipe = crtc->pipe;
> -	int ret;
>  
>  	if (!crtc_state->hw.active)
>  		return;
> @@ -92,10 +120,17 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
>  	to_intel_atomic_state(state)->internal = true;
>  
>  	/* Everything's already locked, -EDEADLK can't happen. */
> -	temp_crtc_state = intel_atomic_get_crtc_state(state, crtc);
> -	ret = drm_atomic_add_affected_connectors(state, &crtc->base);
> +	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc,
> +					 BIT(pipe) |
> +					 intel_crtc_bigjoiner_slave_pipes(crtc_state)) {
> +		struct intel_crtc_state *temp_crtc_state =
> +			intel_atomic_get_crtc_state(state, temp_crtc);
> +		int ret;
>  
> -	drm_WARN_ON(&i915->drm, IS_ERR(temp_crtc_state) || ret);
> +		ret = drm_atomic_add_affected_connectors(state, &temp_crtc->base);
> +
> +		drm_WARN_ON(&i915->drm, IS_ERR(temp_crtc_state) || ret);
> +	}

It's a bit weird to have this loop inside the function that
otherwise seems to be called individually for each of the
joined pipes. Why do we need this?

>  
>  	i915->display.funcs.display->crtc_disable(to_intel_atomic_state(state), crtc);
>  
> @@ -120,8 +155,7 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
>  	drm_WARN_ON(&i915->drm,
>  		    drm_atomic_set_mode_for_crtc(&crtc_state->uapi, NULL) < 0);
>  
> -	for_each_encoder_on_crtc(&i915->drm, &crtc->base, encoder)
> -		encoder->base.crtc = NULL;
> +	reset_crtc_encoder_state(crtc);
>  
>  	intel_fbc_disable(crtc);
>  	intel_update_watermarks(i915);
> @@ -272,6 +306,77 @@ static void intel_sanitize_fifo_underrun_reporting(const struct intel_crtc_state
>  					   !HAS_GMCH(i915));
>  }
>  
> +static u32 get_transcoder_pipes(struct drm_i915_private *i915, u32 transcoder_mask)
> +{
> +	struct intel_crtc *slave_crtc;
> +	u32 pipes = 0;
> +
> +	for_each_intel_crtc(&i915->drm, slave_crtc) {
> +		struct intel_crtc_state *slave_crtc_state =
> +			to_intel_crtc_state(slave_crtc->base.state);
> +
> +		if (slave_crtc_state->cpu_transcoder == INVALID_TRANSCODER)
> +			continue;
> +
> +		if (transcoder_mask & BIT(slave_crtc_state->cpu_transcoder))
> +			pipes |= BIT(slave_crtc->pipe);
> +	}
> +
> +	return pipes;
> +}
> +
> +static u32 get_bigjoiner_slave_pipes(struct drm_i915_private *i915, u32 master_pipes)
> +{
> +	struct intel_crtc *master_crtc;
> +	u32 pipes = 0;
> +
> +	for_each_intel_crtc_in_pipe_mask(&i915->drm, master_crtc, master_pipes) {
> +		struct intel_crtc_state *master_crtc_state =
> +			to_intel_crtc_state(master_crtc->base.state);
> +
> +		pipes |= intel_crtc_bigjoiner_slave_pipes(master_crtc_state);
> +	}
> +
> +	return pipes;
> +}
> +
> +static void kill_bigjoiner_slave_noatomic(struct intel_crtc *master_crtc)
> +{
> +	struct drm_i915_private *i915 = to_i915(master_crtc->base.dev);
> +	struct intel_crtc_state *master_crtc_state =
> +		to_intel_crtc_state(master_crtc->base.state);
> +	struct intel_crtc *slave_crtc;
> +
> +	for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> +					 intel_crtc_bigjoiner_slave_pipes(master_crtc_state)) {
> +		struct intel_crtc_state *slave_crtc_state =
> +			to_intel_crtc_state(slave_crtc->base.state);
> +
> +		slave_crtc_state->bigjoiner_pipes = 0;
> +	}
> +
> +	master_crtc_state->bigjoiner_pipes = 0;
> +}
> +
> +static void disable_crtc_with_slaves(struct intel_crtc *crtc,
> +				     struct drm_modeset_acquire_ctx *ctx)
> +{
> +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state);
> +	u32 bigjoiner_masters = BIT(crtc->pipe) |
> +				get_transcoder_pipes(i915, crtc_state->sync_mode_slaves_mask);

The resulting bitmask would seem to also include the bigjoiner slaves.

> +	u32 bigjoiner_slaves = get_bigjoiner_slave_pipes(i915, bigjoiner_masters);
> +	struct intel_crtc *temp_crtc;
> +
> +	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc, bigjoiner_slaves)
> +		intel_crtc_disable_noatomic(temp_crtc, ctx);
> +
> +	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc, bigjoiner_masters) {
> +		intel_crtc_disable_noatomic(temp_crtc, ctx);
> +		kill_bigjoiner_slave_noatomic(temp_crtc);
> +	}
> +}
> +
>  static void intel_sanitize_crtc(struct intel_crtc *crtc,
>  				struct drm_modeset_acquire_ctx *ctx)
>  {
> @@ -299,10 +404,11 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
>  	/*
>  	 * Adjust the state of the output pipe according to whether we have
>  	 * active connectors/encoders.
> +	 * TODO: Add support for MST
>  	 */
>  	if (crtc_state->hw.active && !intel_crtc_has_encoders(crtc) &&
>  	    !intel_crtc_is_bigjoiner_slave(crtc_state))
> -		intel_crtc_disable_noatomic(crtc, ctx);
> +		disable_crtc_with_slaves(crtc, ctx);

I'd like to keep the _noatomic() in the name.

>  }
>  
>  static bool has_bogus_dpll_config(const struct intel_crtc_state *crtc_state)
> -- 
> 2.37.2

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 05/11] drm/i915: Add support for disabling any CRTCs during HW readout/sanitization

[Imre Deak]

On Fri, Apr 28, 2023 at 05:02:35PM +0300, Ville Syrjälä wrote:
> On Wed, Apr 26, 2023 at 07:52:59PM +0300, Imre Deak wrote:
> > 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 an 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.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  .../drm/i915/display/intel_modeset_setup.c    | 124 ++++++++++++++++--
> >  1 file changed, 115 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > index a66085cf82bab..f613c074187a2 100644
> > --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > @@ -50,10 +50,39 @@ static void set_encoder_for_connector(struct intel_connector *connector,
> >  	}
> >  }
> >  
> > +static void reset_encoder_connector_state(struct intel_encoder *encoder)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > +	struct intel_connector *connector;
> > +	struct drm_connector_list_iter conn_iter;
> > +
> > +	drm_connector_list_iter_begin(&i915->drm, &conn_iter);
> > +	for_each_intel_connector_iter(connector, &conn_iter) {
> > +		if (connector->base.encoder != &encoder->base)
> > +			continue;
> > +
> > +		set_encoder_for_connector(connector, NULL);
> > +
> > +		connector->base.dpms = DRM_MODE_DPMS_OFF;
> > +		connector->base.encoder = NULL;
> > +	}
> > +	drm_connector_list_iter_end(&conn_iter);
> > +}
> > +
> > +static void reset_crtc_encoder_state(struct intel_crtc *crtc)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> > +	struct intel_encoder *encoder;
> > +
> > +	for_each_encoder_on_crtc(&i915->drm, &crtc->base, encoder) {
> > +		reset_encoder_connector_state(encoder);
> > +		encoder->base.crtc = NULL;
> > +	}
> > +}
> > +
> >  static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
> >  					struct drm_modeset_acquire_ctx *ctx)
> >  {
> > -	struct intel_encoder *encoder;
> >  	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> >  	struct intel_bw_state *bw_state =
> >  		to_intel_bw_state(i915->display.bw.obj.state);
> > @@ -65,9 +94,8 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
> >  		to_intel_crtc_state(crtc->base.state);
> >  	struct intel_plane *plane;
> >  	struct drm_atomic_state *state;
> > -	struct intel_crtc_state *temp_crtc_state;
> > +	struct intel_crtc *temp_crtc;
> >  	enum pipe pipe = crtc->pipe;
> > -	int ret;
> >  
> >  	if (!crtc_state->hw.active)
> >  		return;
> > @@ -92,10 +120,17 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
> >  	to_intel_atomic_state(state)->internal = true;
> >  
> >  	/* Everything's already locked, -EDEADLK can't happen. */
> > -	temp_crtc_state = intel_atomic_get_crtc_state(state, crtc);
> > -	ret = drm_atomic_add_affected_connectors(state, &crtc->base);
> > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc,
> > +					 BIT(pipe) |
> > +					 intel_crtc_bigjoiner_slave_pipes(crtc_state)) {
> > +		struct intel_crtc_state *temp_crtc_state =
> > +			intel_atomic_get_crtc_state(state, temp_crtc);
> > +		int ret;
> >  
> > -	drm_WARN_ON(&i915->drm, IS_ERR(temp_crtc_state) || ret);
> > +		ret = drm_atomic_add_affected_connectors(state, &temp_crtc->base);
> > +
> > +		drm_WARN_ON(&i915->drm, IS_ERR(temp_crtc_state) || ret);
> > +	}
> 
> It's a bit weird to have this loop inside the function that
> otherwise seems to be called individually for each of the
> joined pipes. Why do we need this?

If this is called for the master pipe, calling its encoders, it will go
through the slave pipes using their CRTC states, at least in
intel_ddi_post_disable().

> >  	i915->display.funcs.display->crtc_disable(to_intel_atomic_state(state), crtc);
> >  
> > @@ -120,8 +155,7 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
> >  	drm_WARN_ON(&i915->drm,
> >  		    drm_atomic_set_mode_for_crtc(&crtc_state->uapi, NULL) < 0);
> >  
> > -	for_each_encoder_on_crtc(&i915->drm, &crtc->base, encoder)
> > -		encoder->base.crtc = NULL;
> > +	reset_crtc_encoder_state(crtc);
> >  
> >  	intel_fbc_disable(crtc);
> >  	intel_update_watermarks(i915);
> > @@ -272,6 +306,77 @@ static void intel_sanitize_fifo_underrun_reporting(const struct intel_crtc_state
> >  					   !HAS_GMCH(i915));
> >  }
> >  
> > +static u32 get_transcoder_pipes(struct drm_i915_private *i915, u32 transcoder_mask)
> > +{
> > +	struct intel_crtc *slave_crtc;
> > +	u32 pipes = 0;
> > +
> > +	for_each_intel_crtc(&i915->drm, slave_crtc) {
> > +		struct intel_crtc_state *slave_crtc_state =
> > +			to_intel_crtc_state(slave_crtc->base.state);
> > +
> > +		if (slave_crtc_state->cpu_transcoder == INVALID_TRANSCODER)
> > +			continue;
> > +
> > +		if (transcoder_mask & BIT(slave_crtc_state->cpu_transcoder))
> > +			pipes |= BIT(slave_crtc->pipe);
> > +	}
> > +
> > +	return pipes;
> > +}
> > +
> > +static u32 get_bigjoiner_slave_pipes(struct drm_i915_private *i915, u32 master_pipes)
> > +{
> > +	struct intel_crtc *master_crtc;
> > +	u32 pipes = 0;
> > +
> > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, master_crtc, master_pipes) {
> > +		struct intel_crtc_state *master_crtc_state =
> > +			to_intel_crtc_state(master_crtc->base.state);
> > +
> > +		pipes |= intel_crtc_bigjoiner_slave_pipes(master_crtc_state);
> > +	}
> > +
> > +	return pipes;
> > +}
> > +
> > +static void kill_bigjoiner_slave_noatomic(struct intel_crtc *master_crtc)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(master_crtc->base.dev);
> > +	struct intel_crtc_state *master_crtc_state =
> > +		to_intel_crtc_state(master_crtc->base.state);
> > +	struct intel_crtc *slave_crtc;
> > +
> > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> > +					 intel_crtc_bigjoiner_slave_pipes(master_crtc_state)) {
> > +		struct intel_crtc_state *slave_crtc_state =
> > +			to_intel_crtc_state(slave_crtc->base.state);
> > +
> > +		slave_crtc_state->bigjoiner_pipes = 0;
> > +	}
> > +
> > +	master_crtc_state->bigjoiner_pipes = 0;
> > +}
> > +
> > +static void disable_crtc_with_slaves(struct intel_crtc *crtc,
> > +				     struct drm_modeset_acquire_ctx *ctx)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> > +	struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state);
> > +	u32 bigjoiner_masters = BIT(crtc->pipe) |
> > +				get_transcoder_pipes(i915, crtc_state->sync_mode_slaves_mask);
> 
> The resulting bitmask would seem to also include the bigjoiner slaves.

Hrm, doesn't sync_mode_slaves_mask contain only the transcoder bits for
the port-synced master CRTCs? I assumed that those port-synced master
CRTCs would point to their slave pipes via their
crtc_state->bigjoiner_pipes mask.

> > +	u32 bigjoiner_slaves = get_bigjoiner_slave_pipes(i915, bigjoiner_masters);
> > +	struct intel_crtc *temp_crtc;
> > +
> > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc, bigjoiner_slaves)
> > +		intel_crtc_disable_noatomic(temp_crtc, ctx);
> > +
> > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc, bigjoiner_masters) {
> > +		intel_crtc_disable_noatomic(temp_crtc, ctx);
> > +		kill_bigjoiner_slave_noatomic(temp_crtc);
> > +	}
> > +}
> > +
> >  static void intel_sanitize_crtc(struct intel_crtc *crtc,
> >  				struct drm_modeset_acquire_ctx *ctx)
> >  {
> > @@ -299,10 +404,11 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
> >  	/*
> >  	 * Adjust the state of the output pipe according to whether we have
> >  	 * active connectors/encoders.
> > +	 * TODO: Add support for MST
> >  	 */
> >  	if (crtc_state->hw.active && !intel_crtc_has_encoders(crtc) &&
> >  	    !intel_crtc_is_bigjoiner_slave(crtc_state))
> > -		intel_crtc_disable_noatomic(crtc, ctx);
> > +		disable_crtc_with_slaves(crtc, ctx);
> 
> I'd like to keep the _noatomic() in the name.

Ok, will change this.

> 
> >  }
> >  
> >  static bool has_bogus_dpll_config(const struct intel_crtc_state *crtc_state)
> > -- 
> > 2.37.2
> 
> -- 
> Ville Syrjälä
> Intel

Re: [Intel-gfx] [PATCH 05/11] drm/i915: Add support for disabling any CRTCs during HW readout/sanitization

[Imre Deak]

On Fri, Apr 28, 2023 at 08:22:54PM +0300, Imre Deak wrote:
> On Fri, Apr 28, 2023 at 05:02:35PM +0300, Ville Syrjälä wrote:
> > On Wed, Apr 26, 2023 at 07:52:59PM +0300, Imre Deak wrote:
> > > 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 an 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.
> > > 
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  .../drm/i915/display/intel_modeset_setup.c    | 124 ++++++++++++++++--
> > >  1 file changed, 115 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > > index a66085cf82bab..f613c074187a2 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > > @@ -50,10 +50,39 @@ static void set_encoder_for_connector(struct intel_connector *connector,
> > >  	}
> > >  }
> > >  
> > > +static void reset_encoder_connector_state(struct intel_encoder *encoder)
> > > +{
> > > +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > > +	struct intel_connector *connector;
> > > +	struct drm_connector_list_iter conn_iter;
> > > +
> > > +	drm_connector_list_iter_begin(&i915->drm, &conn_iter);
> > > +	for_each_intel_connector_iter(connector, &conn_iter) {
> > > +		if (connector->base.encoder != &encoder->base)
> > > +			continue;
> > > +
> > > +		set_encoder_for_connector(connector, NULL);
> > > +
> > > +		connector->base.dpms = DRM_MODE_DPMS_OFF;
> > > +		connector->base.encoder = NULL;
> > > +	}
> > > +	drm_connector_list_iter_end(&conn_iter);
> > > +}
> > > +
> > > +static void reset_crtc_encoder_state(struct intel_crtc *crtc)
> > > +{
> > > +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> > > +	struct intel_encoder *encoder;
> > > +
> > > +	for_each_encoder_on_crtc(&i915->drm, &crtc->base, encoder) {
> > > +		reset_encoder_connector_state(encoder);
> > > +		encoder->base.crtc = NULL;
> > > +	}
> > > +}
> > > +
> > >  static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
> > >  					struct drm_modeset_acquire_ctx *ctx)
> > >  {
> > > -	struct intel_encoder *encoder;
> > >  	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> > >  	struct intel_bw_state *bw_state =
> > >  		to_intel_bw_state(i915->display.bw.obj.state);
> > > @@ -65,9 +94,8 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
> > >  		to_intel_crtc_state(crtc->base.state);
> > >  	struct intel_plane *plane;
> > >  	struct drm_atomic_state *state;
> > > -	struct intel_crtc_state *temp_crtc_state;
> > > +	struct intel_crtc *temp_crtc;
> > >  	enum pipe pipe = crtc->pipe;
> > > -	int ret;
> > >  
> > >  	if (!crtc_state->hw.active)
> > >  		return;
> > > @@ -92,10 +120,17 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
> > >  	to_intel_atomic_state(state)->internal = true;
> > >  
> > >  	/* Everything's already locked, -EDEADLK can't happen. */
> > > -	temp_crtc_state = intel_atomic_get_crtc_state(state, crtc);
> > > -	ret = drm_atomic_add_affected_connectors(state, &crtc->base);
> > > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc,
> > > +					 BIT(pipe) |
> > > +					 intel_crtc_bigjoiner_slave_pipes(crtc_state)) {
> > > +		struct intel_crtc_state *temp_crtc_state =
> > > +			intel_atomic_get_crtc_state(state, temp_crtc);
> > > +		int ret;
> > >  
> > > -	drm_WARN_ON(&i915->drm, IS_ERR(temp_crtc_state) || ret);
> > > +		ret = drm_atomic_add_affected_connectors(state, &temp_crtc->base);
> > > +
> > > +		drm_WARN_ON(&i915->drm, IS_ERR(temp_crtc_state) || ret);
> > > +	}
> > 
> > It's a bit weird to have this loop inside the function that
> > otherwise seems to be called individually for each of the
> > joined pipes. Why do we need this?
> 
> If this is called for the master pipe, calling its encoders, it will go
> through the slave pipes using their CRTC states, at least in
> intel_ddi_post_disable().
> 
> > >  	i915->display.funcs.display->crtc_disable(to_intel_atomic_state(state), crtc);
> > >  
> > > @@ -120,8 +155,7 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
> > >  	drm_WARN_ON(&i915->drm,
> > >  		    drm_atomic_set_mode_for_crtc(&crtc_state->uapi, NULL) < 0);
> > >  
> > > -	for_each_encoder_on_crtc(&i915->drm, &crtc->base, encoder)
> > > -		encoder->base.crtc = NULL;
> > > +	reset_crtc_encoder_state(crtc);
> > >  
> > >  	intel_fbc_disable(crtc);
> > >  	intel_update_watermarks(i915);
> > > @@ -272,6 +306,77 @@ static void intel_sanitize_fifo_underrun_reporting(const struct intel_crtc_state
> > >  					   !HAS_GMCH(i915));
> > >  }
> > >  
> > > +static u32 get_transcoder_pipes(struct drm_i915_private *i915, u32 transcoder_mask)
> > > +{
> > > +	struct intel_crtc *slave_crtc;
> > > +	u32 pipes = 0;
> > > +
> > > +	for_each_intel_crtc(&i915->drm, slave_crtc) {
> > > +		struct intel_crtc_state *slave_crtc_state =
> > > +			to_intel_crtc_state(slave_crtc->base.state);
> > > +
> > > +		if (slave_crtc_state->cpu_transcoder == INVALID_TRANSCODER)
> > > +			continue;
> > > +
> > > +		if (transcoder_mask & BIT(slave_crtc_state->cpu_transcoder))
> > > +			pipes |= BIT(slave_crtc->pipe);
> > > +	}
> > > +
> > > +	return pipes;
> > > +}
> > > +
> > > +static u32 get_bigjoiner_slave_pipes(struct drm_i915_private *i915, u32 master_pipes)
> > > +{
> > > +	struct intel_crtc *master_crtc;
> > > +	u32 pipes = 0;
> > > +
> > > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, master_crtc, master_pipes) {
> > > +		struct intel_crtc_state *master_crtc_state =
> > > +			to_intel_crtc_state(master_crtc->base.state);
> > > +
> > > +		pipes |= intel_crtc_bigjoiner_slave_pipes(master_crtc_state);
> > > +	}
> > > +
> > > +	return pipes;
> > > +}
> > > +
> > > +static void kill_bigjoiner_slave_noatomic(struct intel_crtc *master_crtc)
> > > +{
> > > +	struct drm_i915_private *i915 = to_i915(master_crtc->base.dev);
> > > +	struct intel_crtc_state *master_crtc_state =
> > > +		to_intel_crtc_state(master_crtc->base.state);
> > > +	struct intel_crtc *slave_crtc;
> > > +
> > > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> > > +					 intel_crtc_bigjoiner_slave_pipes(master_crtc_state)) {
> > > +		struct intel_crtc_state *slave_crtc_state =
> > > +			to_intel_crtc_state(slave_crtc->base.state);
> > > +
> > > +		slave_crtc_state->bigjoiner_pipes = 0;
> > > +	}
> > > +
> > > +	master_crtc_state->bigjoiner_pipes = 0;
> > > +}
> > > +
> > > +static void disable_crtc_with_slaves(struct intel_crtc *crtc,
> > > +				     struct drm_modeset_acquire_ctx *ctx)
> > > +{
> > > +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> > > +	struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state);
> > > +	u32 bigjoiner_masters = BIT(crtc->pipe) |
> > > +				get_transcoder_pipes(i915, crtc_state->sync_mode_slaves_mask);
> > 
> > The resulting bitmask would seem to also include the bigjoiner slaves.
> 
> Hrm, doesn't sync_mode_slaves_mask contain only the transcoder bits for
> the port-synced master CRTCs? I assumed that those port-synced master
> CRTCs would point to their slave pipes via their
> crtc_state->bigjoiner_pipes mask.

Ah, understood the issue, the port_synced slave CRTCs need to get
disabled before the port_sync master.

So I think the following gives the correct order:

	u32 portsync_slaves = get_transcoder_pipes(i915,
						   crtc_state->sync_mode_slaves_mask);
	u32 bigjoiner_slaves = get_bigjoiner_slave_pipes(i915,
							 BIT(crtc->pipe) |
							 portsync_slaves);

	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc, bigjoiner_slaves)
		intel_crtc_disable_noatomic(temp_crtc, ctx);

	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc, portsync_slaves) {
		intel_crtc_disable_noatomic(temp_crtc, ctx);
		kill_bigjoiner_slave_noatomic(temp_crtc);
	}

	intel_crtc_disable_noatomic(crtc);
	kill_bigjoiner_slave_noatomic(crtc);

?	
	
> > > +	u32 bigjoiner_slaves = get_bigjoiner_slave_pipes(i915, bigjoiner_masters);
> > > +	struct intel_crtc *temp_crtc;
> > > +
> > > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc, bigjoiner_slaves)
> > > +		intel_crtc_disable_noatomic(temp_crtc, ctx);
> > > +
> > > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc, bigjoiner_masters) {
> > > +		intel_crtc_disable_noatomic(temp_crtc, ctx);
> > > +		kill_bigjoiner_slave_noatomic(temp_crtc);
> > > +	}
> > > +}
> > > +
> > >  static void intel_sanitize_crtc(struct intel_crtc *crtc,
> > >  				struct drm_modeset_acquire_ctx *ctx)
> > >  {
> > > @@ -299,10 +404,11 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
> > >  	/*
> > >  	 * Adjust the state of the output pipe according to whether we have
> > >  	 * active connectors/encoders.
> > > +	 * TODO: Add support for MST
> > >  	 */
> > >  	if (crtc_state->hw.active && !intel_crtc_has_encoders(crtc) &&
> > >  	    !intel_crtc_is_bigjoiner_slave(crtc_state))
> > > -		intel_crtc_disable_noatomic(crtc, ctx);
> > > +		disable_crtc_with_slaves(crtc, ctx);
> > 
> > I'd like to keep the _noatomic() in the name.
> 
> Ok, will change this.
> 
> > 
> > >  }
> > >  
> > >  static bool has_bogus_dpll_config(const struct intel_crtc_state *crtc_state)
> > > -- 
> > > 2.37.2
> > 
> > -- 
> > Ville Syrjälä
> > Intel

Re: [Intel-gfx] [PATCH 05/11] drm/i915: Add support for disabling any CRTCs during HW readout/sanitization

[Imre Deak]

On Fri, Apr 28, 2023 at 08:47:23PM +0300, Imre Deak wrote:
> On Fri, Apr 28, 2023 at 08:22:54PM +0300, Imre Deak wrote:
> > On Fri, Apr 28, 2023 at 05:02:35PM +0300, Ville Syrjälä wrote:
> > > On Wed, Apr 26, 2023 at 07:52:59PM +0300, Imre Deak wrote:
> > > > [...] 
> > > > +static u32 get_transcoder_pipes(struct drm_i915_private *i915, u32 transcoder_mask)
> > > > +{
> > > > +	struct intel_crtc *slave_crtc;
> > > > +	u32 pipes = 0;
> > > > +
> > > > +	for_each_intel_crtc(&i915->drm, slave_crtc) {
> > > > +		struct intel_crtc_state *slave_crtc_state =
> > > > +			to_intel_crtc_state(slave_crtc->base.state);
> > > > +
> > > > +		if (slave_crtc_state->cpu_transcoder == INVALID_TRANSCODER)
> > > > +			continue;
> > > > +
> > > > +		if (transcoder_mask & BIT(slave_crtc_state->cpu_transcoder))
> > > > +			pipes |= BIT(slave_crtc->pipe);
> > > > +	}
> > > > +
> > > > +	return pipes;
> > > > +}
> > > > +
> > > > +static u32 get_bigjoiner_slave_pipes(struct drm_i915_private *i915, u32 master_pipes)
> > > > +{
> > > > +	struct intel_crtc *master_crtc;
> > > > +	u32 pipes = 0;
> > > > +
> > > > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, master_crtc, master_pipes) {
> > > > +		struct intel_crtc_state *master_crtc_state =
> > > > +			to_intel_crtc_state(master_crtc->base.state);
> > > > +
> > > > +		pipes |= intel_crtc_bigjoiner_slave_pipes(master_crtc_state);
> > > > +	}
> > > > +
> > > > +	return pipes;
> > > > +}
> > > > +
> > > > +static void kill_bigjoiner_slave_noatomic(struct intel_crtc *master_crtc)
> > > > +{
> > > > +	struct drm_i915_private *i915 = to_i915(master_crtc->base.dev);
> > > > +	struct intel_crtc_state *master_crtc_state =
> > > > +		to_intel_crtc_state(master_crtc->base.state);
> > > > +	struct intel_crtc *slave_crtc;
> > > > +
> > > > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> > > > +					 intel_crtc_bigjoiner_slave_pipes(master_crtc_state)) {
> > > > +		struct intel_crtc_state *slave_crtc_state =
> > > > +			to_intel_crtc_state(slave_crtc->base.state);
> > > > +
> > > > +		slave_crtc_state->bigjoiner_pipes = 0;
> > > > +	}
> > > > +
> > > > +	master_crtc_state->bigjoiner_pipes = 0;
> > > > +}
> > > > +
> > > > +static void disable_crtc_with_slaves(struct intel_crtc *crtc,
> > > > +				     struct drm_modeset_acquire_ctx *ctx)
> > > > +{
> > > > +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> > > > +	struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state);
> > > > +	u32 bigjoiner_masters = BIT(crtc->pipe) |
> > > > +				get_transcoder_pipes(i915, crtc_state->sync_mode_slaves_mask);
> > > 
> > > The resulting bitmask would seem to also include the bigjoiner slaves.
> > 
> > Hrm, doesn't sync_mode_slaves_mask contain only the transcoder bits for
> > the port-synced master CRTCs? I assumed that those port-synced master
> > CRTCs would point to their slave pipes via their
> > crtc_state->bigjoiner_pipes mask.
> 
> Ah, understood the issue, the port_synced slave CRTCs need to get
> disabled before the port_sync master.
> 
> So I think the following gives the correct order:
> 
> 	u32 portsync_slaves = get_transcoder_pipes(i915,
> 						   crtc_state->sync_mode_slaves_mask);
> 	u32 bigjoiner_slaves = get_bigjoiner_slave_pipes(i915,
> 							 BIT(crtc->pipe) |
> 							 portsync_slaves);
> 
> 	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc, bigjoiner_slaves)
> 		intel_crtc_disable_noatomic(temp_crtc, ctx);
> 
> 	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc, portsync_slaves) {
> 		intel_crtc_disable_noatomic(temp_crtc, ctx);
> 		kill_bigjoiner_slave_noatomic(temp_crtc);
> 	}
> 
> 	intel_crtc_disable_noatomic(crtc);
> 	kill_bigjoiner_slave_noatomic(crtc);
> 
> ?

And that a bigjoiner slave pipe's transcoder points to the master
transcoder :/ so those slave pipes need to be filtered out in
get_transcoder_pipes().

> > > > +	u32 bigjoiner_slaves = get_bigjoiner_slave_pipes(i915, bigjoiner_masters);
> > > > +	struct intel_crtc *temp_crtc;
> > > > +
> > > > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc, bigjoiner_slaves)
> > > > +		intel_crtc_disable_noatomic(temp_crtc, ctx);
> > > > +
> > > > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, temp_crtc, bigjoiner_masters) {
> > > > +		intel_crtc_disable_noatomic(temp_crtc, ctx);
> > > > +		kill_bigjoiner_slave_noatomic(temp_crtc);
> > > > +	}
> > > > +}
> > > > +
> > > >  static void intel_sanitize_crtc(struct intel_crtc *crtc,
> > > >  				struct drm_modeset_acquire_ctx *ctx)
> > > >  {
> > > > @@ -299,10 +404,11 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
> > > >  	/*
> > > >  	 * Adjust the state of the output pipe according to whether we have
> > > >  	 * active connectors/encoders.
> > > > +	 * TODO: Add support for MST
> > > >  	 */
> > > >  	if (crtc_state->hw.active && !intel_crtc_has_encoders(crtc) &&
> > > >  	    !intel_crtc_is_bigjoiner_slave(crtc_state))
> > > > -		intel_crtc_disable_noatomic(crtc, ctx);
> > > > +		disable_crtc_with_slaves(crtc, ctx);
> > > 
> > > I'd like to keep the _noatomic() in the name.
> > 
> > Ok, will change this.
> > 
> > > 
> > > >  }
> > > >  
> > > >  static bool has_bogus_dpll_config(const struct intel_crtc_state *crtc_state)
> > > > -- 
> > > > 2.37.2
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel

[Intel-gfx] [PATCH 06/11] drm/i915/dp: Add link training debug and error printing helpers

[Imre Deak]

Add functions for printing link training debug and error messages, both
to prepare for the next patch, which downgrades an error to debug if the
sink is disconnected and to remove some code duplication.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 .../drm/i915/display/intel_dp_link_training.c | 376 +++++++-----------
 1 file changed, 139 insertions(+), 237 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index 6aa4ae5e7ebe3..12f2880e474ed 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -26,6 +26,47 @@
 #include "intel_dp.h"
 #include "intel_dp_link_training.h"
 
+__printf(5, 6)
+static void lt_msg(struct intel_connector *connector, struct intel_dp *intel_dp, enum drm_dp_phy dp_phy,
+		   bool is_error, const char *format, ...)
+{
+	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
+	char conn_str[128] = {};
+	struct va_format vaf;
+	va_list args;
+
+	va_start(args, format);
+	vaf.fmt = format;
+	vaf.va = &args;
+
+	if (connector)
+		snprintf(conn_str, sizeof(conn_str), "[CONNECTOR:%d:%s]",
+			 connector->base.base.id, connector->base.name);
+
+	if (is_error)
+		drm_err(&i915->drm, "%s[ENCODER:%d:%s][%s] %pV\n",
+			conn_str,
+			encoder->base.base.id, encoder->base.name,
+			drm_dp_phy_name(dp_phy),
+			&vaf);
+	else
+		drm_dbg(&i915->drm, "%s[ENCODER:%d:%s][%s] %pV\n",
+			conn_str,
+			encoder->base.base.id, encoder->base.name,
+			drm_dp_phy_name(dp_phy),
+			&vaf);
+}
+
+#define lt_err(intel_dp, dp_phy, format, ...) \
+	lt_msg(NULL, intel_dp, dp_phy, true, format, ## __VA_ARGS__)
+
+#define lt_dbg(intel_dp, dp_phy, format, ...) \
+	lt_msg(NULL, intel_dp, dp_phy, false, format, ## __VA_ARGS__)
+
+#define lt_conn_dbg(connector, intel_dp, dp_phy, format, ...) \
+	lt_msg(connector, intel_dp, dp_phy, false, format, ## __VA_ARGS__)
+
 static void intel_dp_reset_lttpr_common_caps(struct intel_dp *intel_dp)
 {
 	memset(intel_dp->lttpr_common_caps, 0, sizeof(intel_dp->lttpr_common_caps));
@@ -47,29 +88,21 @@ static void intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
 					 const u8 dpcd[DP_RECEIVER_CAP_SIZE],
 					 enum drm_dp_phy dp_phy)
 {
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
 	u8 *phy_caps = intel_dp_lttpr_phy_caps(intel_dp, dp_phy);
 
 	if (drm_dp_read_lttpr_phy_caps(&intel_dp->aux, dpcd, dp_phy, phy_caps) < 0) {
-		drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
-			    "[ENCODER:%d:%s][%s] failed to read the PHY caps\n",
-			    encoder->base.base.id, encoder->base.name,
-			    drm_dp_phy_name(dp_phy));
+		lt_dbg(intel_dp, dp_phy, "failed to read the PHY caps\n");
 		return;
 	}
 
-	drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
-		    "[ENCODER:%d:%s][%s] PHY capabilities: %*ph\n",
-		    encoder->base.base.id, encoder->base.name,
-		    drm_dp_phy_name(dp_phy),
-		    (int)sizeof(intel_dp->lttpr_phy_caps[0]),
-		    phy_caps);
+	lt_dbg(intel_dp, dp_phy, "PHY capabilities: %*ph\n",
+	       (int)sizeof(intel_dp->lttpr_phy_caps[0]),
+	       phy_caps);
 }
 
 static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp,
 					    const u8 dpcd[DP_RECEIVER_CAP_SIZE])
 {
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
 	int ret;
 
 	ret = drm_dp_read_lttpr_common_caps(&intel_dp->aux, dpcd,
@@ -77,11 +110,9 @@ static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp,
 	if (ret < 0)
 		goto reset_caps;
 
-	drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
-		    "[ENCODER:%d:%s] LTTPR common capabilities: %*ph\n",
-		    encoder->base.base.id, encoder->base.name,
-		    (int)sizeof(intel_dp->lttpr_common_caps),
-		    intel_dp->lttpr_common_caps);
+	lt_dbg(intel_dp, DP_PHY_DPRX, "LTTPR common capabilities: %*ph\n",
+	       (int)sizeof(intel_dp->lttpr_common_caps),
+	       intel_dp->lttpr_common_caps);
 
 	/* The minimum value of LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV is 1.4 */
 	if (intel_dp->lttpr_common_caps[0] < 0x14)
@@ -105,8 +136,6 @@ intel_dp_set_lttpr_transparent_mode(struct intel_dp *intel_dp, bool enable)
 
 static int intel_dp_init_lttpr(struct intel_dp *intel_dp, const u8 dpcd[DP_RECEIVER_CAP_SIZE])
 {
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
-	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
 	int lttpr_count;
 	int i;
 
@@ -138,9 +167,8 @@ static int intel_dp_init_lttpr(struct intel_dp *intel_dp, const u8 dpcd[DP_RECEI
 		return 0;
 
 	if (!intel_dp_set_lttpr_transparent_mode(intel_dp, false)) {
-		drm_dbg_kms(&i915->drm,
-			    "[ENCODER:%d:%s] Switching to LTTPR non-transparent LT mode failed, fall-back to transparent mode\n",
-			    encoder->base.base.id, encoder->base.name);
+		lt_dbg(intel_dp, DP_PHY_DPRX,
+		       "Switching to LTTPR non-transparent LT mode failed, fall-back to transparent mode\n");
 
 		intel_dp_set_lttpr_transparent_mode(intel_dp, true);
 		intel_dp_reset_lttpr_count(intel_dp);
@@ -409,26 +437,22 @@ intel_dp_get_adjust_train(struct intel_dp *intel_dp,
 			  enum drm_dp_phy dp_phy,
 			  const u8 link_status[DP_LINK_STATUS_SIZE])
 {
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
-	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
 	int lane;
 
 	if (intel_dp_is_uhbr(crtc_state)) {
-		drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] 128b/132b, lanes: %d, "
-			    "TX FFE request: " TRAIN_REQ_FMT "\n",
-			    encoder->base.base.id, encoder->base.name,
-			    drm_dp_phy_name(dp_phy),
-			    crtc_state->lane_count,
-			    TRAIN_REQ_TX_FFE_ARGS(link_status));
+		lt_dbg(intel_dp, dp_phy,
+		       "128b/132b, lanes: %d, "
+		       "TX FFE request: " TRAIN_REQ_FMT "\n",
+		       crtc_state->lane_count,
+		       TRAIN_REQ_TX_FFE_ARGS(link_status));
 	} else {
-		drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] 8b/10b, lanes: %d, "
-			    "vswing request: " TRAIN_REQ_FMT ", "
-			    "pre-emphasis request: " TRAIN_REQ_FMT "\n",
-			    encoder->base.base.id, encoder->base.name,
-			    drm_dp_phy_name(dp_phy),
-			    crtc_state->lane_count,
-			    TRAIN_REQ_VSWING_ARGS(link_status),
-			    TRAIN_REQ_PREEMPH_ARGS(link_status));
+		lt_dbg(intel_dp, dp_phy,
+		       "8b/10b, lanes: %d, "
+		       "vswing request: " TRAIN_REQ_FMT ", "
+		       "pre-emphasis request: " TRAIN_REQ_FMT "\n",
+		       crtc_state->lane_count,
+		       TRAIN_REQ_VSWING_ARGS(link_status),
+		       TRAIN_REQ_PREEMPH_ARGS(link_status));
 	}
 
 	for (lane = 0; lane < 4; lane++)
@@ -487,16 +511,11 @@ intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
 				       enum drm_dp_phy dp_phy,
 				       u8 dp_train_pat)
 {
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
-	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
 	u8 train_pat = intel_dp_training_pattern_symbol(dp_train_pat);
 
 	if (train_pat != DP_TRAINING_PATTERN_DISABLE)
-		drm_dbg_kms(&i915->drm,
-			    "[ENCODER:%d:%s][%s] Using DP training pattern TPS%c\n",
-			    encoder->base.base.id, encoder->base.name,
-			    drm_dp_phy_name(dp_phy),
-			    dp_training_pattern_name(train_pat));
+		lt_dbg(intel_dp, dp_phy, "Using DP training pattern TPS%c\n",
+		       dp_training_pattern_name(train_pat));
 
 	intel_dp->set_link_train(intel_dp, crtc_state, dp_train_pat);
 }
@@ -531,24 +550,21 @@ void intel_dp_set_signal_levels(struct intel_dp *intel_dp,
 				enum drm_dp_phy dp_phy)
 {
 	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
-	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
 
 	if (intel_dp_is_uhbr(crtc_state)) {
-		drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] 128b/132b, lanes: %d, "
-			    "TX FFE presets: " TRAIN_SET_FMT "\n",
-			    encoder->base.base.id, encoder->base.name,
-			    drm_dp_phy_name(dp_phy),
-			    crtc_state->lane_count,
-			    TRAIN_SET_TX_FFE_ARGS(intel_dp->train_set));
+		lt_dbg(intel_dp, dp_phy,
+		       "128b/132b, lanes: %d, "
+		       "TX FFE presets: " TRAIN_SET_FMT "\n",
+		       crtc_state->lane_count,
+		       TRAIN_SET_TX_FFE_ARGS(intel_dp->train_set));
 	} else {
-		drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] 8b/10b, lanes: %d, "
-			    "vswing levels: " TRAIN_SET_FMT ", "
-			    "pre-emphasis levels: " TRAIN_SET_FMT "\n",
-			    encoder->base.base.id, encoder->base.name,
-			    drm_dp_phy_name(dp_phy),
-			    crtc_state->lane_count,
-			    TRAIN_SET_VSWING_ARGS(intel_dp->train_set),
-			    TRAIN_SET_PREEMPH_ARGS(intel_dp->train_set));
+		lt_dbg(intel_dp, dp_phy,
+		       "8b/10b, lanes: %d, "
+		       "vswing levels: " TRAIN_SET_FMT ", "
+		       "pre-emphasis levels: " TRAIN_SET_FMT "\n",
+		       crtc_state->lane_count,
+		       TRAIN_SET_VSWING_ARGS(intel_dp->train_set),
+		       TRAIN_SET_PREEMPH_ARGS(intel_dp->train_set));
 	}
 
 	if (intel_dp_phy_is_downstream_of_source(intel_dp, dp_phy))
@@ -645,8 +661,6 @@ static bool
 intel_dp_prepare_link_train(struct intel_dp *intel_dp,
 			    const struct intel_crtc_state *crtc_state)
 {
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
-	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
 	u8 link_config[2];
 	u8 link_bw, rate_select;
 
@@ -671,21 +685,19 @@ intel_dp_prepare_link_train(struct intel_dp *intel_dp,
 		struct intel_connector *connector = intel_dp->attached_connector;
 		__le16 sink_rates[DP_MAX_SUPPORTED_RATES];
 
-		drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Reloading eDP link rates\n",
-			    connector->base.base.id, connector->base.name);
+		lt_conn_dbg(connector, intel_dp, DP_PHY_DPRX,
+			    "Reloading eDP link rates\n");
 
 		drm_dp_dpcd_read(&intel_dp->aux, DP_SUPPORTED_LINK_RATES,
 				 sink_rates, sizeof(sink_rates));
 	}
 
 	if (link_bw)
-		drm_dbg_kms(&i915->drm,
-			    "[ENCODER:%d:%s] Using LINK_BW_SET value %02x\n",
-			    encoder->base.base.id, encoder->base.name, link_bw);
+		lt_dbg(intel_dp, DP_PHY_DPRX, "Using LINK_BW_SET value %02x\n",
+		       link_bw);
 	else
-		drm_dbg_kms(&i915->drm,
-			    "[ENCODER:%d:%s] Using LINK_RATE_SET value %02x\n",
-			    encoder->base.base.id, encoder->base.name, rate_select);
+		lt_dbg(intel_dp, DP_PHY_DPRX, "Using LINK_RATE_SET value %02x\n",
+		       rate_select);
 
 	/* Write the link configuration data */
 	link_config[0] = link_bw;
@@ -737,15 +749,10 @@ void
 intel_dp_dump_link_status(struct intel_dp *intel_dp, enum drm_dp_phy dp_phy,
 			  const u8 link_status[DP_LINK_STATUS_SIZE])
 {
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
-	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
-
-	drm_dbg_kms(&i915->drm,
-		    "[ENCODER:%d:%s][%s] ln0_1:0x%x ln2_3:0x%x align:0x%x sink:0x%x adj_req0_1:0x%x adj_req2_3:0x%x\n",
-		    encoder->base.base.id, encoder->base.name,
-		    drm_dp_phy_name(dp_phy),
-		    link_status[0], link_status[1], link_status[2],
-		    link_status[3], link_status[4], link_status[5]);
+	lt_dbg(intel_dp, dp_phy,
+	       "ln0_1:0x%x ln2_3:0x%x align:0x%x sink:0x%x adj_req0_1:0x%x adj_req2_3:0x%x\n",
+	       link_status[0], link_status[1], link_status[2],
+	       link_status[3], link_status[4], link_status[5]);
 }
 
 /*
@@ -757,8 +764,6 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
 				      const struct intel_crtc_state *crtc_state,
 				      enum drm_dp_phy dp_phy)
 {
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
-	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
 	u8 old_link_status[DP_LINK_STATUS_SIZE] = {};
 	int voltage_tries, cr_tries, max_cr_tries;
 	u8 link_status[DP_LINK_STATUS_SIZE];
@@ -773,9 +778,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
 	if (!intel_dp_reset_link_train(intel_dp, crtc_state, dp_phy,
 				       DP_TRAINING_PATTERN_1 |
 				       DP_LINK_SCRAMBLING_DISABLE)) {
-		drm_err(&i915->drm, "[ENCODER:%d:%s][%s] Failed to enable link training\n",
-			encoder->base.base.id, encoder->base.name,
-			drm_dp_phy_name(dp_phy));
+		lt_err(intel_dp, dp_phy, "Failed to enable link training\n");
 		return false;
 	}
 
@@ -798,35 +801,24 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
 
 		if (drm_dp_dpcd_read_phy_link_status(&intel_dp->aux, dp_phy,
 						     link_status) < 0) {
-			drm_err(&i915->drm, "[ENCODER:%d:%s][%s] Failed to get link status\n",
-				encoder->base.base.id, encoder->base.name,
-				drm_dp_phy_name(dp_phy));
+			lt_err(intel_dp, dp_phy, "Failed to get link status\n");
 			return false;
 		}
 
 		if (drm_dp_clock_recovery_ok(link_status, crtc_state->lane_count)) {
-			drm_dbg_kms(&i915->drm,
-				    "[ENCODER:%d:%s][%s] Clock recovery OK\n",
-				    encoder->base.base.id, encoder->base.name,
-				    drm_dp_phy_name(dp_phy));
+			lt_dbg(intel_dp, dp_phy, "Clock recovery OK\n");
 			return true;
 		}
 
 		if (voltage_tries == 5) {
 			intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
-			drm_dbg_kms(&i915->drm,
-				    "[ENCODER:%d:%s][%s] Same voltage tried 5 times\n",
-				    encoder->base.base.id, encoder->base.name,
-				    drm_dp_phy_name(dp_phy));
+			lt_dbg(intel_dp, dp_phy, "Same voltage tried 5 times\n");
 			return false;
 		}
 
 		if (max_vswing_reached) {
 			intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
-			drm_dbg_kms(&i915->drm,
-				    "[ENCODER:%d:%s][%s] Max Voltage Swing reached\n",
-				    encoder->base.base.id, encoder->base.name,
-				    drm_dp_phy_name(dp_phy));
+			lt_dbg(intel_dp, dp_phy, "Max Voltage Swing reached\n");
 			return false;
 		}
 
@@ -834,10 +826,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
 		intel_dp_get_adjust_train(intel_dp, crtc_state, dp_phy,
 					  link_status);
 		if (!intel_dp_update_link_train(intel_dp, crtc_state, dp_phy)) {
-			drm_err(&i915->drm,
-				"[ENCODER:%d:%s][%s] Failed to update link training\n",
-				encoder->base.base.id, encoder->base.name,
-				drm_dp_phy_name(dp_phy));
+			lt_err(intel_dp, dp_phy, "Failed to update link training\n");
 			return false;
 		}
 
@@ -853,10 +842,8 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
 	}
 
 	intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
-	drm_err(&i915->drm,
-		"[ENCODER:%d:%s][%s] Failed clock recovery %d times, giving up!\n",
-		encoder->base.base.id, encoder->base.name,
-		drm_dp_phy_name(dp_phy), max_cr_tries);
+	lt_err(intel_dp, dp_phy, "Failed clock recovery %d times, giving up!\n",
+	       max_cr_tries);
 
 	return false;
 }
@@ -890,11 +877,11 @@ static u32 intel_dp_training_pattern(struct intel_dp *intel_dp,
 		return DP_TRAINING_PATTERN_4;
 	} else if (crtc_state->port_clock == 810000) {
 		if (!source_tps4)
-			drm_dbg_kms(&i915->drm,
-				    "8.1 Gbps link rate without source TPS4 support\n");
+			lt_dbg(intel_dp, dp_phy,
+			       "8.1 Gbps link rate without source TPS4 support\n");
 		if (!sink_tps4)
-			drm_dbg_kms(&i915->drm,
-				    "8.1 Gbps link rate without sink TPS4 support\n");
+			lt_dbg(intel_dp, dp_phy,
+			       "8.1 Gbps link rate without sink TPS4 support\n");
 	}
 
 	/*
@@ -908,11 +895,11 @@ static u32 intel_dp_training_pattern(struct intel_dp *intel_dp,
 		return  DP_TRAINING_PATTERN_3;
 	} else if (crtc_state->port_clock >= 540000) {
 		if (!source_tps3)
-			drm_dbg_kms(&i915->drm,
-				    ">=5.4/6.48 Gbps link rate without source TPS3 support\n");
+			lt_dbg(intel_dp, dp_phy,
+			       ">=5.4/6.48 Gbps link rate without source TPS3 support\n");
 		if (!sink_tps3)
-			drm_dbg_kms(&i915->drm,
-				    ">=5.4/6.48 Gbps link rate without sink TPS3 support\n");
+			lt_dbg(intel_dp, dp_phy,
+			       ">=5.4/6.48 Gbps link rate without sink TPS3 support\n");
 	}
 
 	return DP_TRAINING_PATTERN_2;
@@ -928,8 +915,6 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
 					    const struct intel_crtc_state *crtc_state,
 					    enum drm_dp_phy dp_phy)
 {
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
-	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
 	int tries;
 	u32 training_pattern;
 	u8 link_status[DP_LINK_STATUS_SIZE];
@@ -948,10 +933,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
 	/* channel equalization */
 	if (!intel_dp_set_link_train(intel_dp, crtc_state, dp_phy,
 				     training_pattern)) {
-		drm_err(&i915->drm,
-			"[ENCODER:%d:%s][%s] Failed to start channel equalization\n",
-			encoder->base.base.id, encoder->base.name,
-			drm_dp_phy_name(dp_phy));
+		lt_err(intel_dp, dp_phy, "Failed to start channel equalization\n");
 		return false;
 	}
 
@@ -960,10 +942,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
 
 		if (drm_dp_dpcd_read_phy_link_status(&intel_dp->aux, dp_phy,
 						     link_status) < 0) {
-			drm_err(&i915->drm,
-				"[ENCODER:%d:%s][%s] Failed to get link status\n",
-				encoder->base.base.id, encoder->base.name,
-				drm_dp_phy_name(dp_phy));
+			lt_err(intel_dp, dp_phy, "Failed to get link status\n");
 			break;
 		}
 
@@ -971,21 +950,15 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
 		if (!drm_dp_clock_recovery_ok(link_status,
 					      crtc_state->lane_count)) {
 			intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
-			drm_dbg_kms(&i915->drm,
-				    "[ENCODER:%d:%s][%s] Clock recovery check failed, cannot "
-				    "continue channel equalization\n",
-				    encoder->base.base.id, encoder->base.name,
-				    drm_dp_phy_name(dp_phy));
+			lt_dbg(intel_dp, dp_phy,
+			       "Clock recovery check failed, cannot continue channel equalization\n");
 			break;
 		}
 
 		if (drm_dp_channel_eq_ok(link_status,
 					 crtc_state->lane_count)) {
 			channel_eq = true;
-			drm_dbg_kms(&i915->drm,
-				    "[ENCODER:%d:%s][%s] Channel EQ done. DP Training successful\n",
-				    encoder->base.base.id, encoder->base.name,
-				    drm_dp_phy_name(dp_phy));
+			lt_dbg(intel_dp, dp_phy, "Channel EQ done. DP Training successful\n");
 			break;
 		}
 
@@ -993,10 +966,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
 		intel_dp_get_adjust_train(intel_dp, crtc_state, dp_phy,
 					  link_status);
 		if (!intel_dp_update_link_train(intel_dp, crtc_state, dp_phy)) {
-			drm_err(&i915->drm,
-				"[ENCODER:%d:%s][%s] Failed to update link training\n",
-				encoder->base.base.id, encoder->base.name,
-				drm_dp_phy_name(dp_phy));
+			lt_err(intel_dp, dp_phy, "Failed to update link training\n");
 			break;
 		}
 	}
@@ -1004,10 +974,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
 	/* Try 5 times, else fail and try at lower BW */
 	if (tries == 5) {
 		intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
-		drm_dbg_kms(&i915->drm,
-			    "[ENCODER:%d:%s][%s] Channel equalization failed 5 times\n",
-			    encoder->base.base.id, encoder->base.name,
-			    drm_dp_phy_name(dp_phy));
+		lt_dbg(intel_dp, dp_phy, "Channel equalization failed 5 times\n");
 	}
 
 	return channel_eq;
@@ -1058,9 +1025,6 @@ intel_dp_128b132b_intra_hop(struct intel_dp *intel_dp,
 void intel_dp_stop_link_train(struct intel_dp *intel_dp,
 			      const struct intel_crtc_state *crtc_state)
 {
-	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
-
 	intel_dp->link_trained = true;
 
 	intel_dp_disable_dpcd_training_pattern(intel_dp, DP_PHY_DPRX);
@@ -1069,9 +1033,7 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp,
 
 	if (intel_dp_is_uhbr(crtc_state) &&
 	    wait_for(intel_dp_128b132b_intra_hop(intel_dp, crtc_state) == 0, 500)) {
-		drm_dbg_kms(&i915->drm,
-			    "[ENCODER:%d:%s] 128b/132b intra-hop not clearing\n",
-			    encoder->base.base.id, encoder->base.name);
+		lt_dbg(intel_dp, DP_PHY_DPRX, "128b/132b intra-hop not clearing\n");
 	}
 }
 
@@ -1081,7 +1043,6 @@ intel_dp_link_train_phy(struct intel_dp *intel_dp,
 			enum drm_dp_phy dp_phy)
 {
 	struct intel_connector *connector = intel_dp->attached_connector;
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
 	bool ret = false;
 
 	if (!intel_dp_link_training_clock_recovery(intel_dp, crtc_state, dp_phy))
@@ -1093,11 +1054,8 @@ intel_dp_link_train_phy(struct intel_dp *intel_dp,
 	ret = true;
 
 out:
-	drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
-		    "[CONNECTOR:%d:%s][ENCODER:%d:%s][%s] Link Training %s at link rate = %d, lane count = %d\n",
-		    connector->base.base.id, connector->base.name,
-		    encoder->base.base.id, encoder->base.name,
-		    drm_dp_phy_name(dp_phy),
+	lt_conn_dbg(connector, intel_dp, dp_phy,
+		    "Link Training %s at link rate = %d, lane count = %d\n",
 		    ret ? "passed" : "failed",
 		    crtc_state->port_clock, crtc_state->lane_count);
 
@@ -1108,13 +1066,10 @@ static void intel_dp_schedule_fallback_link_training(struct intel_dp *intel_dp,
 						     const struct intel_crtc_state *crtc_state)
 {
 	struct intel_connector *intel_connector = intel_dp->attached_connector;
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
 
 	if (intel_dp->hobl_active) {
-		drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
-			    "[ENCODER:%d:%s] Link Training failed with HOBL active, "
-			    "not enabling it from now on",
-			    encoder->base.base.id, encoder->base.name);
+		lt_dbg(intel_dp, DP_PHY_DPRX,
+		       "Link Training failed with HOBL active, not enabling it from now on\n");
 		intel_dp->hobl_failed = true;
 	} else if (intel_dp_get_link_train_fallback_values(intel_dp,
 							   crtc_state->port_clock,
@@ -1161,8 +1116,6 @@ static bool
 intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
 			  const struct intel_crtc_state *crtc_state)
 {
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
-	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
 	u8 link_status[DP_LINK_STATUS_SIZE];
 	int delay_us;
 	int try, max_tries = 20;
@@ -1177,9 +1130,7 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
 	 */
 	if (!intel_dp_reset_link_train(intel_dp, crtc_state, DP_PHY_DPRX,
 				       DP_TRAINING_PATTERN_1)) {
-		drm_err(&i915->drm,
-			"[ENCODER:%d:%s] Failed to start 128b/132b TPS1\n",
-			encoder->base.base.id, encoder->base.name);
+		lt_err(intel_dp, DP_PHY_DPRX, "Failed to start 128b/132b TPS1\n");
 		return false;
 	}
 
@@ -1187,27 +1138,21 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
 
 	/* Read the initial TX FFE settings. */
 	if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
-		drm_err(&i915->drm,
-			"[ENCODER:%d:%s] Failed to read TX FFE presets\n",
-			encoder->base.base.id, encoder->base.name);
+		lt_err(intel_dp, DP_PHY_DPRX, "Failed to read TX FFE presets\n");
 		return false;
 	}
 
 	/* Update signal levels and training set as requested. */
 	intel_dp_get_adjust_train(intel_dp, crtc_state, DP_PHY_DPRX, link_status);
 	if (!intel_dp_update_link_train(intel_dp, crtc_state, DP_PHY_DPRX)) {
-		drm_err(&i915->drm,
-			"[ENCODER:%d:%s] Failed to set initial TX FFE settings\n",
-			encoder->base.base.id, encoder->base.name);
+		lt_err(intel_dp, DP_PHY_DPRX, "Failed to set initial TX FFE settings\n");
 		return false;
 	}
 
 	/* Start transmitting 128b/132b TPS2. */
 	if (!intel_dp_set_link_train(intel_dp, crtc_state, DP_PHY_DPRX,
 				     DP_TRAINING_PATTERN_2)) {
-		drm_err(&i915->drm,
-			"[ENCODER:%d:%s] Failed to start 128b/132b TPS2\n",
-			encoder->base.base.id, encoder->base.name);
+		lt_err(intel_dp, DP_PHY_DPRX, "Failed to start 128b/132b TPS2\n");
 		return false;
 	}
 
@@ -1224,32 +1169,25 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
 		delay_us = drm_dp_128b132b_read_aux_rd_interval(&intel_dp->aux);
 
 		if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
-			drm_err(&i915->drm,
-				"[ENCODER:%d:%s] Failed to read link status\n",
-				encoder->base.base.id, encoder->base.name);
+			lt_err(intel_dp, DP_PHY_DPRX, "Failed to read link status\n");
 			return false;
 		}
 
 		if (drm_dp_128b132b_link_training_failed(link_status)) {
 			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
-			drm_err(&i915->drm,
-				"[ENCODER:%d:%s] Downstream link training failure\n",
-				encoder->base.base.id, encoder->base.name);
+			lt_err(intel_dp, DP_PHY_DPRX,
+			       "Downstream link training failure\n");
 			return false;
 		}
 
 		if (drm_dp_128b132b_lane_channel_eq_done(link_status, crtc_state->lane_count)) {
-			drm_dbg_kms(&i915->drm,
-				    "[ENCODER:%d:%s] Lane channel eq done\n",
-				    encoder->base.base.id, encoder->base.name);
+			lt_dbg(intel_dp, DP_PHY_DPRX, "Lane channel eq done\n");
 			break;
 		}
 
 		if (timeout) {
 			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
-			drm_err(&i915->drm,
-				"[ENCODER:%d:%s] Lane channel eq timeout\n",
-				encoder->base.base.id, encoder->base.name);
+			lt_err(intel_dp, DP_PHY_DPRX, "Lane channel eq timeout\n");
 			return false;
 		}
 
@@ -1259,18 +1197,14 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
 		/* Update signal levels and training set as requested. */
 		intel_dp_get_adjust_train(intel_dp, crtc_state, DP_PHY_DPRX, link_status);
 		if (!intel_dp_update_link_train(intel_dp, crtc_state, DP_PHY_DPRX)) {
-			drm_err(&i915->drm,
-				"[ENCODER:%d:%s] Failed to update TX FFE settings\n",
-				encoder->base.base.id, encoder->base.name);
+			lt_err(intel_dp, DP_PHY_DPRX, "Failed to update TX FFE settings\n");
 			return false;
 		}
 	}
 
 	if (try == max_tries) {
 		intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
-		drm_err(&i915->drm,
-			"[ENCODER:%d:%s] Max loop count reached\n",
-			encoder->base.base.id, encoder->base.name);
+		lt_err(intel_dp, DP_PHY_DPRX, "Max loop count reached\n");
 		return false;
 	}
 
@@ -1279,32 +1213,24 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
 			timeout = true; /* try one last time after deadline */
 
 		if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
-			drm_err(&i915->drm,
-				"[ENCODER:%d:%s] Failed to read link status\n",
-				encoder->base.base.id, encoder->base.name);
+			lt_err(intel_dp, DP_PHY_DPRX, "Failed to read link status\n");
 			return false;
 		}
 
 		if (drm_dp_128b132b_link_training_failed(link_status)) {
 			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
-			drm_err(&i915->drm,
-				"[ENCODER:%d:%s] Downstream link training failure\n",
-				encoder->base.base.id, encoder->base.name);
+			lt_err(intel_dp, DP_PHY_DPRX, "Downstream link training failure\n");
 			return false;
 		}
 
 		if (drm_dp_128b132b_eq_interlane_align_done(link_status)) {
-			drm_dbg_kms(&i915->drm,
-				    "[ENCODER:%d:%s] Interlane align done\n",
-				    encoder->base.base.id, encoder->base.name);
+			lt_dbg(intel_dp, DP_PHY_DPRX, "Interlane align done\n");
 			break;
 		}
 
 		if (timeout) {
 			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
-			drm_err(&i915->drm,
-				"[ENCODER:%d:%s] Interlane align timeout\n",
-				encoder->base.base.id, encoder->base.name);
+			lt_err(intel_dp, DP_PHY_DPRX, "Interlane align timeout\n");
 			return false;
 		}
 
@@ -1322,16 +1248,12 @@ intel_dp_128b132b_lane_cds(struct intel_dp *intel_dp,
 			   const struct intel_crtc_state *crtc_state,
 			   int lttpr_count)
 {
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
-	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
 	u8 link_status[DP_LINK_STATUS_SIZE];
 	unsigned long deadline;
 
 	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TRAINING_PATTERN_SET,
 			       DP_TRAINING_PATTERN_2_CDS) != 1) {
-		drm_err(&i915->drm,
-			"[ENCODER:%d:%s] Failed to start 128b/132b TPS2 CDS\n",
-			encoder->base.base.id, encoder->base.name);
+		lt_err(intel_dp, DP_PHY_DPRX, "Failed to start 128b/132b TPS2 CDS\n");
 		return false;
 	}
 
@@ -1347,34 +1269,26 @@ intel_dp_128b132b_lane_cds(struct intel_dp *intel_dp,
 		usleep_range(2000, 3000);
 
 		if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
-			drm_err(&i915->drm,
-				"[ENCODER:%d:%s] Failed to read link status\n",
-				encoder->base.base.id, encoder->base.name);
+			lt_err(intel_dp, DP_PHY_DPRX, "Failed to read link status\n");
 			return false;
 		}
 
 		if (drm_dp_128b132b_eq_interlane_align_done(link_status) &&
 		    drm_dp_128b132b_cds_interlane_align_done(link_status) &&
 		    drm_dp_128b132b_lane_symbol_locked(link_status, crtc_state->lane_count)) {
-			drm_dbg_kms(&i915->drm,
-				    "[ENCODER:%d:%s] CDS interlane align done\n",
-				    encoder->base.base.id, encoder->base.name);
+			lt_err(intel_dp, DP_PHY_DPRX, "CDS interlane align done\n");
 			break;
 		}
 
 		if (drm_dp_128b132b_link_training_failed(link_status)) {
 			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
-			drm_err(&i915->drm,
-				"[ENCODER:%d:%s] Downstream link training failure\n",
-				encoder->base.base.id, encoder->base.name);
+			lt_err(intel_dp, DP_PHY_DPRX, "Downstream link training failure\n");
 			return false;
 		}
 
 		if (timeout) {
 			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
-			drm_err(&i915->drm,
-				"[ENCODER:%d:%s] CDS timeout\n",
-				encoder->base.base.id, encoder->base.name);
+			lt_err(intel_dp, DP_PHY_DPRX, "CDS timeout\n");
 			return false;
 		}
 	}
@@ -1390,15 +1304,11 @@ intel_dp_128b132b_link_train(struct intel_dp *intel_dp,
 			     const struct intel_crtc_state *crtc_state,
 			     int lttpr_count)
 {
-	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
 	struct intel_connector *connector = intel_dp->attached_connector;
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
 	bool passed = false;
 
 	if (wait_for(intel_dp_128b132b_intra_hop(intel_dp, crtc_state) == 0, 500)) {
-		drm_err(&i915->drm,
-			"[ENCODER:%d:%s] 128b/132b intra-hop not clear\n",
-			encoder->base.base.id, encoder->base.name);
+		lt_err(intel_dp, DP_PHY_DPRX, "128b/132b intra-hop not clear\n");
 		return false;
 	}
 
@@ -1406,10 +1316,8 @@ intel_dp_128b132b_link_train(struct intel_dp *intel_dp,
 	    intel_dp_128b132b_lane_cds(intel_dp, crtc_state, lttpr_count))
 		passed = true;
 
-	drm_dbg_kms(&i915->drm,
-		    "[CONNECTOR:%d:%s][ENCODER:%d:%s] 128b/132b Link Training %s at link rate = %d, lane count = %d\n",
-		    connector->base.base.id, connector->base.name,
-		    encoder->base.base.id, encoder->base.name,
+	lt_conn_dbg(connector, intel_dp, DP_PHY_DPRX,
+		    "128b/132b Link Training %s at link rate = %d, lane count = %d\n",
 		    passed ? "passed" : "failed",
 		    crtc_state->port_clock, crtc_state->lane_count);
 
@@ -1431,7 +1339,6 @@ void intel_dp_start_link_train(struct intel_dp *intel_dp,
 {
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
 	struct intel_connector *connector = intel_dp->attached_connector;
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
 	bool passed;
 
 	/*
@@ -1464,10 +1371,7 @@ void intel_dp_start_link_train(struct intel_dp *intel_dp,
 	 * ignore_long_hpd flag can unset from the testcase.
 	 */
 	if (!passed && i915->display.hotplug.ignore_long_hpd) {
-		drm_dbg_kms(&i915->drm,
-			    "[CONNECTOR:%d:%s][ENCODER:%d:%s] Ignore the link failure\n",
-			    connector->base.base.id, connector->base.name,
-			    encoder->base.base.id, encoder->base.name);
+		lt_conn_dbg(connector, intel_dp, DP_PHY_DPRX, "Ignore the link failure\n");
 		return;
 	}
 
@@ -1478,8 +1382,6 @@ void intel_dp_start_link_train(struct intel_dp *intel_dp,
 void intel_dp_128b132b_sdp_crc16(struct intel_dp *intel_dp,
 				 const struct intel_crtc_state *crtc_state)
 {
-	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
-
 	/*
 	 * VIDEO_DIP_CTL register bit 31 should be set to '0' to not
 	 * disable SDP CRC. This is applicable for Display version 13.
@@ -1492,5 +1394,5 @@ void intel_dp_128b132b_sdp_crc16(struct intel_dp *intel_dp,
 				   DP_SDP_ERROR_DETECTION_CONFIGURATION,
 				   DP_SDP_CRC16_128B132B_EN);
 
-	drm_dbg_kms(&i915->drm, "DP2.0 SDP CRC16 for 128b/132b enabled\n");
+	lt_dbg(intel_dp, DP_PHY_DPRX, "DP2.0 SDP CRC16 for 128b/132b enabled\n");
 }
-- 
2.37.2

Re: [Intel-gfx] [PATCH 06/11] drm/i915/dp: Add link training debug and error printing helpers

[Ville Syrjälä]

On Wed, Apr 26, 2023 at 07:53:00PM +0300, Imre Deak wrote:
> Add functions for printing link training debug and error messages, both
> to prepare for the next patch, which downgrades an error to debug if the
> sink is disconnected and to remove some code duplication.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  .../drm/i915/display/intel_dp_link_training.c | 376 +++++++-----------
>  1 file changed, 139 insertions(+), 237 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index 6aa4ae5e7ebe3..12f2880e474ed 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -26,6 +26,47 @@
>  #include "intel_dp.h"
>  #include "intel_dp_link_training.h"
>  
> +__printf(5, 6)
> +static void lt_msg(struct intel_connector *connector, struct intel_dp *intel_dp, enum drm_dp_phy dp_phy,
> +		   bool is_error, const char *format, ...)

I pretty much hate custom debug/error print functions. Makes it
hard togrep/etc. and just generally causes more "wtf is this?"
moments when reading the code. Unfortunateely the macro hell in
drm_print.h seems to make it really hard to do anything generic
directly :(

> +{
> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> +	char conn_str[128] = {};
> +	struct va_format vaf;
> +	va_list args;
> +
> +	va_start(args, format);
> +	vaf.fmt = format;
> +	vaf.va = &args;
> +
> +	if (connector)
> +		snprintf(conn_str, sizeof(conn_str), "[CONNECTOR:%d:%s]",
> +			 connector->base.base.id, connector->base.name);

Are there actually places where we don't have/can't get the
connector?

> +
> +	if (is_error)
> +		drm_err(&i915->drm, "%s[ENCODER:%d:%s][%s] %pV\n",
> +			conn_str,
> +			encoder->base.base.id, encoder->base.name,
> +			drm_dp_phy_name(dp_phy),
> +			&vaf);
> +	else
> +		drm_dbg(&i915->drm, "%s[ENCODER:%d:%s][%s] %pV\n",

That has lost the correct debug category.

> +			conn_str,
> +			encoder->base.base.id, encoder->base.name,
> +			drm_dp_phy_name(dp_phy),
> +			&vaf);

Hmm. ___drm_dev_dbg() already uses this vaf stuff.
Does chaining it like that work correctly?

> +}
> +
> +#define lt_err(intel_dp, dp_phy, format, ...) \
> +	lt_msg(NULL, intel_dp, dp_phy, true, format, ## __VA_ARGS__)
> +
> +#define lt_dbg(intel_dp, dp_phy, format, ...) \
> +	lt_msg(NULL, intel_dp, dp_phy, false, format, ## __VA_ARGS__)
> +
> +#define lt_conn_dbg(connector, intel_dp, dp_phy, format, ...) \
> +	lt_msg(connector, intel_dp, dp_phy, false, format, ## __VA_ARGS__)
> +
>  static void intel_dp_reset_lttpr_common_caps(struct intel_dp *intel_dp)
>  {
>  	memset(intel_dp->lttpr_common_caps, 0, sizeof(intel_dp->lttpr_common_caps));
> @@ -47,29 +88,21 @@ static void intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
>  					 const u8 dpcd[DP_RECEIVER_CAP_SIZE],
>  					 enum drm_dp_phy dp_phy)
>  {
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>  	u8 *phy_caps = intel_dp_lttpr_phy_caps(intel_dp, dp_phy);
>  
>  	if (drm_dp_read_lttpr_phy_caps(&intel_dp->aux, dpcd, dp_phy, phy_caps) < 0) {
> -		drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
> -			    "[ENCODER:%d:%s][%s] failed to read the PHY caps\n",
> -			    encoder->base.base.id, encoder->base.name,
> -			    drm_dp_phy_name(dp_phy));
> +		lt_dbg(intel_dp, dp_phy, "failed to read the PHY caps\n");
>  		return;
>  	}
>  
> -	drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
> -		    "[ENCODER:%d:%s][%s] PHY capabilities: %*ph\n",
> -		    encoder->base.base.id, encoder->base.name,
> -		    drm_dp_phy_name(dp_phy),
> -		    (int)sizeof(intel_dp->lttpr_phy_caps[0]),
> -		    phy_caps);
> +	lt_dbg(intel_dp, dp_phy, "PHY capabilities: %*ph\n",
> +	       (int)sizeof(intel_dp->lttpr_phy_caps[0]),
> +	       phy_caps);
>  }
>  
>  static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp,
>  					    const u8 dpcd[DP_RECEIVER_CAP_SIZE])
>  {
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>  	int ret;
>  
>  	ret = drm_dp_read_lttpr_common_caps(&intel_dp->aux, dpcd,
> @@ -77,11 +110,9 @@ static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp,
>  	if (ret < 0)
>  		goto reset_caps;
>  
> -	drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
> -		    "[ENCODER:%d:%s] LTTPR common capabilities: %*ph\n",
> -		    encoder->base.base.id, encoder->base.name,
> -		    (int)sizeof(intel_dp->lttpr_common_caps),
> -		    intel_dp->lttpr_common_caps);
> +	lt_dbg(intel_dp, DP_PHY_DPRX, "LTTPR common capabilities: %*ph\n",
> +	       (int)sizeof(intel_dp->lttpr_common_caps),
> +	       intel_dp->lttpr_common_caps);
>  
>  	/* The minimum value of LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV is 1.4 */
>  	if (intel_dp->lttpr_common_caps[0] < 0x14)
> @@ -105,8 +136,6 @@ intel_dp_set_lttpr_transparent_mode(struct intel_dp *intel_dp, bool enable)
>  
>  static int intel_dp_init_lttpr(struct intel_dp *intel_dp, const u8 dpcd[DP_RECEIVER_CAP_SIZE])
>  {
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>  	int lttpr_count;
>  	int i;
>  
> @@ -138,9 +167,8 @@ static int intel_dp_init_lttpr(struct intel_dp *intel_dp, const u8 dpcd[DP_RECEI
>  		return 0;
>  
>  	if (!intel_dp_set_lttpr_transparent_mode(intel_dp, false)) {
> -		drm_dbg_kms(&i915->drm,
> -			    "[ENCODER:%d:%s] Switching to LTTPR non-transparent LT mode failed, fall-back to transparent mode\n",
> -			    encoder->base.base.id, encoder->base.name);
> +		lt_dbg(intel_dp, DP_PHY_DPRX,
> +		       "Switching to LTTPR non-transparent LT mode failed, fall-back to transparent mode\n");
>  
>  		intel_dp_set_lttpr_transparent_mode(intel_dp, true);
>  		intel_dp_reset_lttpr_count(intel_dp);
> @@ -409,26 +437,22 @@ intel_dp_get_adjust_train(struct intel_dp *intel_dp,
>  			  enum drm_dp_phy dp_phy,
>  			  const u8 link_status[DP_LINK_STATUS_SIZE])
>  {
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>  	int lane;
>  
>  	if (intel_dp_is_uhbr(crtc_state)) {
> -		drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] 128b/132b, lanes: %d, "
> -			    "TX FFE request: " TRAIN_REQ_FMT "\n",
> -			    encoder->base.base.id, encoder->base.name,
> -			    drm_dp_phy_name(dp_phy),
> -			    crtc_state->lane_count,
> -			    TRAIN_REQ_TX_FFE_ARGS(link_status));
> +		lt_dbg(intel_dp, dp_phy,
> +		       "128b/132b, lanes: %d, "
> +		       "TX FFE request: " TRAIN_REQ_FMT "\n",
> +		       crtc_state->lane_count,
> +		       TRAIN_REQ_TX_FFE_ARGS(link_status));
>  	} else {
> -		drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] 8b/10b, lanes: %d, "
> -			    "vswing request: " TRAIN_REQ_FMT ", "
> -			    "pre-emphasis request: " TRAIN_REQ_FMT "\n",
> -			    encoder->base.base.id, encoder->base.name,
> -			    drm_dp_phy_name(dp_phy),
> -			    crtc_state->lane_count,
> -			    TRAIN_REQ_VSWING_ARGS(link_status),
> -			    TRAIN_REQ_PREEMPH_ARGS(link_status));
> +		lt_dbg(intel_dp, dp_phy,
> +		       "8b/10b, lanes: %d, "
> +		       "vswing request: " TRAIN_REQ_FMT ", "
> +		       "pre-emphasis request: " TRAIN_REQ_FMT "\n",
> +		       crtc_state->lane_count,
> +		       TRAIN_REQ_VSWING_ARGS(link_status),
> +		       TRAIN_REQ_PREEMPH_ARGS(link_status));
>  	}
>  
>  	for (lane = 0; lane < 4; lane++)
> @@ -487,16 +511,11 @@ intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
>  				       enum drm_dp_phy dp_phy,
>  				       u8 dp_train_pat)
>  {
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>  	u8 train_pat = intel_dp_training_pattern_symbol(dp_train_pat);
>  
>  	if (train_pat != DP_TRAINING_PATTERN_DISABLE)
> -		drm_dbg_kms(&i915->drm,
> -			    "[ENCODER:%d:%s][%s] Using DP training pattern TPS%c\n",
> -			    encoder->base.base.id, encoder->base.name,
> -			    drm_dp_phy_name(dp_phy),
> -			    dp_training_pattern_name(train_pat));
> +		lt_dbg(intel_dp, dp_phy, "Using DP training pattern TPS%c\n",
> +		       dp_training_pattern_name(train_pat));
>  
>  	intel_dp->set_link_train(intel_dp, crtc_state, dp_train_pat);
>  }
> @@ -531,24 +550,21 @@ void intel_dp_set_signal_levels(struct intel_dp *intel_dp,
>  				enum drm_dp_phy dp_phy)
>  {
>  	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>  
>  	if (intel_dp_is_uhbr(crtc_state)) {
> -		drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] 128b/132b, lanes: %d, "
> -			    "TX FFE presets: " TRAIN_SET_FMT "\n",
> -			    encoder->base.base.id, encoder->base.name,
> -			    drm_dp_phy_name(dp_phy),
> -			    crtc_state->lane_count,
> -			    TRAIN_SET_TX_FFE_ARGS(intel_dp->train_set));
> +		lt_dbg(intel_dp, dp_phy,
> +		       "128b/132b, lanes: %d, "
> +		       "TX FFE presets: " TRAIN_SET_FMT "\n",
> +		       crtc_state->lane_count,
> +		       TRAIN_SET_TX_FFE_ARGS(intel_dp->train_set));
>  	} else {
> -		drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] 8b/10b, lanes: %d, "
> -			    "vswing levels: " TRAIN_SET_FMT ", "
> -			    "pre-emphasis levels: " TRAIN_SET_FMT "\n",
> -			    encoder->base.base.id, encoder->base.name,
> -			    drm_dp_phy_name(dp_phy),
> -			    crtc_state->lane_count,
> -			    TRAIN_SET_VSWING_ARGS(intel_dp->train_set),
> -			    TRAIN_SET_PREEMPH_ARGS(intel_dp->train_set));
> +		lt_dbg(intel_dp, dp_phy,
> +		       "8b/10b, lanes: %d, "
> +		       "vswing levels: " TRAIN_SET_FMT ", "
> +		       "pre-emphasis levels: " TRAIN_SET_FMT "\n",
> +		       crtc_state->lane_count,
> +		       TRAIN_SET_VSWING_ARGS(intel_dp->train_set),
> +		       TRAIN_SET_PREEMPH_ARGS(intel_dp->train_set));
>  	}
>  
>  	if (intel_dp_phy_is_downstream_of_source(intel_dp, dp_phy))
> @@ -645,8 +661,6 @@ static bool
>  intel_dp_prepare_link_train(struct intel_dp *intel_dp,
>  			    const struct intel_crtc_state *crtc_state)
>  {
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>  	u8 link_config[2];
>  	u8 link_bw, rate_select;
>  
> @@ -671,21 +685,19 @@ intel_dp_prepare_link_train(struct intel_dp *intel_dp,
>  		struct intel_connector *connector = intel_dp->attached_connector;
>  		__le16 sink_rates[DP_MAX_SUPPORTED_RATES];
>  
> -		drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Reloading eDP link rates\n",
> -			    connector->base.base.id, connector->base.name);
> +		lt_conn_dbg(connector, intel_dp, DP_PHY_DPRX,
> +			    "Reloading eDP link rates\n");
>  
>  		drm_dp_dpcd_read(&intel_dp->aux, DP_SUPPORTED_LINK_RATES,
>  				 sink_rates, sizeof(sink_rates));
>  	}
>  
>  	if (link_bw)
> -		drm_dbg_kms(&i915->drm,
> -			    "[ENCODER:%d:%s] Using LINK_BW_SET value %02x\n",
> -			    encoder->base.base.id, encoder->base.name, link_bw);
> +		lt_dbg(intel_dp, DP_PHY_DPRX, "Using LINK_BW_SET value %02x\n",
> +		       link_bw);
>  	else
> -		drm_dbg_kms(&i915->drm,
> -			    "[ENCODER:%d:%s] Using LINK_RATE_SET value %02x\n",
> -			    encoder->base.base.id, encoder->base.name, rate_select);
> +		lt_dbg(intel_dp, DP_PHY_DPRX, "Using LINK_RATE_SET value %02x\n",
> +		       rate_select);
>  
>  	/* Write the link configuration data */
>  	link_config[0] = link_bw;
> @@ -737,15 +749,10 @@ void
>  intel_dp_dump_link_status(struct intel_dp *intel_dp, enum drm_dp_phy dp_phy,
>  			  const u8 link_status[DP_LINK_STATUS_SIZE])
>  {
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> -
> -	drm_dbg_kms(&i915->drm,
> -		    "[ENCODER:%d:%s][%s] ln0_1:0x%x ln2_3:0x%x align:0x%x sink:0x%x adj_req0_1:0x%x adj_req2_3:0x%x\n",
> -		    encoder->base.base.id, encoder->base.name,
> -		    drm_dp_phy_name(dp_phy),
> -		    link_status[0], link_status[1], link_status[2],
> -		    link_status[3], link_status[4], link_status[5]);
> +	lt_dbg(intel_dp, dp_phy,
> +	       "ln0_1:0x%x ln2_3:0x%x align:0x%x sink:0x%x adj_req0_1:0x%x adj_req2_3:0x%x\n",
> +	       link_status[0], link_status[1], link_status[2],
> +	       link_status[3], link_status[4], link_status[5]);
>  }
>  
>  /*
> @@ -757,8 +764,6 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
>  				      const struct intel_crtc_state *crtc_state,
>  				      enum drm_dp_phy dp_phy)
>  {
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>  	u8 old_link_status[DP_LINK_STATUS_SIZE] = {};
>  	int voltage_tries, cr_tries, max_cr_tries;
>  	u8 link_status[DP_LINK_STATUS_SIZE];
> @@ -773,9 +778,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
>  	if (!intel_dp_reset_link_train(intel_dp, crtc_state, dp_phy,
>  				       DP_TRAINING_PATTERN_1 |
>  				       DP_LINK_SCRAMBLING_DISABLE)) {
> -		drm_err(&i915->drm, "[ENCODER:%d:%s][%s] Failed to enable link training\n",
> -			encoder->base.base.id, encoder->base.name,
> -			drm_dp_phy_name(dp_phy));
> +		lt_err(intel_dp, dp_phy, "Failed to enable link training\n");
>  		return false;
>  	}
>  
> @@ -798,35 +801,24 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
>  
>  		if (drm_dp_dpcd_read_phy_link_status(&intel_dp->aux, dp_phy,
>  						     link_status) < 0) {
> -			drm_err(&i915->drm, "[ENCODER:%d:%s][%s] Failed to get link status\n",
> -				encoder->base.base.id, encoder->base.name,
> -				drm_dp_phy_name(dp_phy));
> +			lt_err(intel_dp, dp_phy, "Failed to get link status\n");
>  			return false;
>  		}
>  
>  		if (drm_dp_clock_recovery_ok(link_status, crtc_state->lane_count)) {
> -			drm_dbg_kms(&i915->drm,
> -				    "[ENCODER:%d:%s][%s] Clock recovery OK\n",
> -				    encoder->base.base.id, encoder->base.name,
> -				    drm_dp_phy_name(dp_phy));
> +			lt_dbg(intel_dp, dp_phy, "Clock recovery OK\n");
>  			return true;
>  		}
>  
>  		if (voltage_tries == 5) {
>  			intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
> -			drm_dbg_kms(&i915->drm,
> -				    "[ENCODER:%d:%s][%s] Same voltage tried 5 times\n",
> -				    encoder->base.base.id, encoder->base.name,
> -				    drm_dp_phy_name(dp_phy));
> +			lt_dbg(intel_dp, dp_phy, "Same voltage tried 5 times\n");
>  			return false;
>  		}
>  
>  		if (max_vswing_reached) {
>  			intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
> -			drm_dbg_kms(&i915->drm,
> -				    "[ENCODER:%d:%s][%s] Max Voltage Swing reached\n",
> -				    encoder->base.base.id, encoder->base.name,
> -				    drm_dp_phy_name(dp_phy));
> +			lt_dbg(intel_dp, dp_phy, "Max Voltage Swing reached\n");
>  			return false;
>  		}
>  
> @@ -834,10 +826,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
>  		intel_dp_get_adjust_train(intel_dp, crtc_state, dp_phy,
>  					  link_status);
>  		if (!intel_dp_update_link_train(intel_dp, crtc_state, dp_phy)) {
> -			drm_err(&i915->drm,
> -				"[ENCODER:%d:%s][%s] Failed to update link training\n",
> -				encoder->base.base.id, encoder->base.name,
> -				drm_dp_phy_name(dp_phy));
> +			lt_err(intel_dp, dp_phy, "Failed to update link training\n");
>  			return false;
>  		}
>  
> @@ -853,10 +842,8 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
>  	}
>  
>  	intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
> -	drm_err(&i915->drm,
> -		"[ENCODER:%d:%s][%s] Failed clock recovery %d times, giving up!\n",
> -		encoder->base.base.id, encoder->base.name,
> -		drm_dp_phy_name(dp_phy), max_cr_tries);
> +	lt_err(intel_dp, dp_phy, "Failed clock recovery %d times, giving up!\n",
> +	       max_cr_tries);
>  
>  	return false;
>  }
> @@ -890,11 +877,11 @@ static u32 intel_dp_training_pattern(struct intel_dp *intel_dp,
>  		return DP_TRAINING_PATTERN_4;
>  	} else if (crtc_state->port_clock == 810000) {
>  		if (!source_tps4)
> -			drm_dbg_kms(&i915->drm,
> -				    "8.1 Gbps link rate without source TPS4 support\n");
> +			lt_dbg(intel_dp, dp_phy,
> +			       "8.1 Gbps link rate without source TPS4 support\n");
>  		if (!sink_tps4)
> -			drm_dbg_kms(&i915->drm,
> -				    "8.1 Gbps link rate without sink TPS4 support\n");
> +			lt_dbg(intel_dp, dp_phy,
> +			       "8.1 Gbps link rate without sink TPS4 support\n");
>  	}
>  
>  	/*
> @@ -908,11 +895,11 @@ static u32 intel_dp_training_pattern(struct intel_dp *intel_dp,
>  		return  DP_TRAINING_PATTERN_3;
>  	} else if (crtc_state->port_clock >= 540000) {
>  		if (!source_tps3)
> -			drm_dbg_kms(&i915->drm,
> -				    ">=5.4/6.48 Gbps link rate without source TPS3 support\n");
> +			lt_dbg(intel_dp, dp_phy,
> +			       ">=5.4/6.48 Gbps link rate without source TPS3 support\n");
>  		if (!sink_tps3)
> -			drm_dbg_kms(&i915->drm,
> -				    ">=5.4/6.48 Gbps link rate without sink TPS3 support\n");
> +			lt_dbg(intel_dp, dp_phy,
> +			       ">=5.4/6.48 Gbps link rate without sink TPS3 support\n");
>  	}
>  
>  	return DP_TRAINING_PATTERN_2;
> @@ -928,8 +915,6 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
>  					    const struct intel_crtc_state *crtc_state,
>  					    enum drm_dp_phy dp_phy)
>  {
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>  	int tries;
>  	u32 training_pattern;
>  	u8 link_status[DP_LINK_STATUS_SIZE];
> @@ -948,10 +933,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
>  	/* channel equalization */
>  	if (!intel_dp_set_link_train(intel_dp, crtc_state, dp_phy,
>  				     training_pattern)) {
> -		drm_err(&i915->drm,
> -			"[ENCODER:%d:%s][%s] Failed to start channel equalization\n",
> -			encoder->base.base.id, encoder->base.name,
> -			drm_dp_phy_name(dp_phy));
> +		lt_err(intel_dp, dp_phy, "Failed to start channel equalization\n");
>  		return false;
>  	}
>  
> @@ -960,10 +942,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
>  
>  		if (drm_dp_dpcd_read_phy_link_status(&intel_dp->aux, dp_phy,
>  						     link_status) < 0) {
> -			drm_err(&i915->drm,
> -				"[ENCODER:%d:%s][%s] Failed to get link status\n",
> -				encoder->base.base.id, encoder->base.name,
> -				drm_dp_phy_name(dp_phy));
> +			lt_err(intel_dp, dp_phy, "Failed to get link status\n");
>  			break;
>  		}
>  
> @@ -971,21 +950,15 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
>  		if (!drm_dp_clock_recovery_ok(link_status,
>  					      crtc_state->lane_count)) {
>  			intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
> -			drm_dbg_kms(&i915->drm,
> -				    "[ENCODER:%d:%s][%s] Clock recovery check failed, cannot "
> -				    "continue channel equalization\n",
> -				    encoder->base.base.id, encoder->base.name,
> -				    drm_dp_phy_name(dp_phy));
> +			lt_dbg(intel_dp, dp_phy,
> +			       "Clock recovery check failed, cannot continue channel equalization\n");
>  			break;
>  		}
>  
>  		if (drm_dp_channel_eq_ok(link_status,
>  					 crtc_state->lane_count)) {
>  			channel_eq = true;
> -			drm_dbg_kms(&i915->drm,
> -				    "[ENCODER:%d:%s][%s] Channel EQ done. DP Training successful\n",
> -				    encoder->base.base.id, encoder->base.name,
> -				    drm_dp_phy_name(dp_phy));
> +			lt_dbg(intel_dp, dp_phy, "Channel EQ done. DP Training successful\n");
>  			break;
>  		}
>  
> @@ -993,10 +966,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
>  		intel_dp_get_adjust_train(intel_dp, crtc_state, dp_phy,
>  					  link_status);
>  		if (!intel_dp_update_link_train(intel_dp, crtc_state, dp_phy)) {
> -			drm_err(&i915->drm,
> -				"[ENCODER:%d:%s][%s] Failed to update link training\n",
> -				encoder->base.base.id, encoder->base.name,
> -				drm_dp_phy_name(dp_phy));
> +			lt_err(intel_dp, dp_phy, "Failed to update link training\n");
>  			break;
>  		}
>  	}
> @@ -1004,10 +974,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
>  	/* Try 5 times, else fail and try at lower BW */
>  	if (tries == 5) {
>  		intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
> -		drm_dbg_kms(&i915->drm,
> -			    "[ENCODER:%d:%s][%s] Channel equalization failed 5 times\n",
> -			    encoder->base.base.id, encoder->base.name,
> -			    drm_dp_phy_name(dp_phy));
> +		lt_dbg(intel_dp, dp_phy, "Channel equalization failed 5 times\n");
>  	}
>  
>  	return channel_eq;
> @@ -1058,9 +1025,6 @@ intel_dp_128b132b_intra_hop(struct intel_dp *intel_dp,
>  void intel_dp_stop_link_train(struct intel_dp *intel_dp,
>  			      const struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> -
>  	intel_dp->link_trained = true;
>  
>  	intel_dp_disable_dpcd_training_pattern(intel_dp, DP_PHY_DPRX);
> @@ -1069,9 +1033,7 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp,
>  
>  	if (intel_dp_is_uhbr(crtc_state) &&
>  	    wait_for(intel_dp_128b132b_intra_hop(intel_dp, crtc_state) == 0, 500)) {
> -		drm_dbg_kms(&i915->drm,
> -			    "[ENCODER:%d:%s] 128b/132b intra-hop not clearing\n",
> -			    encoder->base.base.id, encoder->base.name);
> +		lt_dbg(intel_dp, DP_PHY_DPRX, "128b/132b intra-hop not clearing\n");
>  	}
>  }
>  
> @@ -1081,7 +1043,6 @@ intel_dp_link_train_phy(struct intel_dp *intel_dp,
>  			enum drm_dp_phy dp_phy)
>  {
>  	struct intel_connector *connector = intel_dp->attached_connector;
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>  	bool ret = false;
>  
>  	if (!intel_dp_link_training_clock_recovery(intel_dp, crtc_state, dp_phy))
> @@ -1093,11 +1054,8 @@ intel_dp_link_train_phy(struct intel_dp *intel_dp,
>  	ret = true;
>  
>  out:
> -	drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
> -		    "[CONNECTOR:%d:%s][ENCODER:%d:%s][%s] Link Training %s at link rate = %d, lane count = %d\n",
> -		    connector->base.base.id, connector->base.name,
> -		    encoder->base.base.id, encoder->base.name,
> -		    drm_dp_phy_name(dp_phy),
> +	lt_conn_dbg(connector, intel_dp, dp_phy,
> +		    "Link Training %s at link rate = %d, lane count = %d\n",
>  		    ret ? "passed" : "failed",
>  		    crtc_state->port_clock, crtc_state->lane_count);
>  
> @@ -1108,13 +1066,10 @@ static void intel_dp_schedule_fallback_link_training(struct intel_dp *intel_dp,
>  						     const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_connector *intel_connector = intel_dp->attached_connector;
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>  
>  	if (intel_dp->hobl_active) {
> -		drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
> -			    "[ENCODER:%d:%s] Link Training failed with HOBL active, "
> -			    "not enabling it from now on",
> -			    encoder->base.base.id, encoder->base.name);
> +		lt_dbg(intel_dp, DP_PHY_DPRX,
> +		       "Link Training failed with HOBL active, not enabling it from now on\n");
>  		intel_dp->hobl_failed = true;
>  	} else if (intel_dp_get_link_train_fallback_values(intel_dp,
>  							   crtc_state->port_clock,
> @@ -1161,8 +1116,6 @@ static bool
>  intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
>  			  const struct intel_crtc_state *crtc_state)
>  {
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>  	u8 link_status[DP_LINK_STATUS_SIZE];
>  	int delay_us;
>  	int try, max_tries = 20;
> @@ -1177,9 +1130,7 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
>  	 */
>  	if (!intel_dp_reset_link_train(intel_dp, crtc_state, DP_PHY_DPRX,
>  				       DP_TRAINING_PATTERN_1)) {
> -		drm_err(&i915->drm,
> -			"[ENCODER:%d:%s] Failed to start 128b/132b TPS1\n",
> -			encoder->base.base.id, encoder->base.name);
> +		lt_err(intel_dp, DP_PHY_DPRX, "Failed to start 128b/132b TPS1\n");
>  		return false;
>  	}
>  
> @@ -1187,27 +1138,21 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
>  
>  	/* Read the initial TX FFE settings. */
>  	if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
> -		drm_err(&i915->drm,
> -			"[ENCODER:%d:%s] Failed to read TX FFE presets\n",
> -			encoder->base.base.id, encoder->base.name);
> +		lt_err(intel_dp, DP_PHY_DPRX, "Failed to read TX FFE presets\n");
>  		return false;
>  	}
>  
>  	/* Update signal levels and training set as requested. */
>  	intel_dp_get_adjust_train(intel_dp, crtc_state, DP_PHY_DPRX, link_status);
>  	if (!intel_dp_update_link_train(intel_dp, crtc_state, DP_PHY_DPRX)) {
> -		drm_err(&i915->drm,
> -			"[ENCODER:%d:%s] Failed to set initial TX FFE settings\n",
> -			encoder->base.base.id, encoder->base.name);
> +		lt_err(intel_dp, DP_PHY_DPRX, "Failed to set initial TX FFE settings\n");
>  		return false;
>  	}
>  
>  	/* Start transmitting 128b/132b TPS2. */
>  	if (!intel_dp_set_link_train(intel_dp, crtc_state, DP_PHY_DPRX,
>  				     DP_TRAINING_PATTERN_2)) {
> -		drm_err(&i915->drm,
> -			"[ENCODER:%d:%s] Failed to start 128b/132b TPS2\n",
> -			encoder->base.base.id, encoder->base.name);
> +		lt_err(intel_dp, DP_PHY_DPRX, "Failed to start 128b/132b TPS2\n");
>  		return false;
>  	}
>  
> @@ -1224,32 +1169,25 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
>  		delay_us = drm_dp_128b132b_read_aux_rd_interval(&intel_dp->aux);
>  
>  		if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
> -			drm_err(&i915->drm,
> -				"[ENCODER:%d:%s] Failed to read link status\n",
> -				encoder->base.base.id, encoder->base.name);
> +			lt_err(intel_dp, DP_PHY_DPRX, "Failed to read link status\n");
>  			return false;
>  		}
>  
>  		if (drm_dp_128b132b_link_training_failed(link_status)) {
>  			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
> -			drm_err(&i915->drm,
> -				"[ENCODER:%d:%s] Downstream link training failure\n",
> -				encoder->base.base.id, encoder->base.name);
> +			lt_err(intel_dp, DP_PHY_DPRX,
> +			       "Downstream link training failure\n");
>  			return false;
>  		}
>  
>  		if (drm_dp_128b132b_lane_channel_eq_done(link_status, crtc_state->lane_count)) {
> -			drm_dbg_kms(&i915->drm,
> -				    "[ENCODER:%d:%s] Lane channel eq done\n",
> -				    encoder->base.base.id, encoder->base.name);
> +			lt_dbg(intel_dp, DP_PHY_DPRX, "Lane channel eq done\n");
>  			break;
>  		}
>  
>  		if (timeout) {
>  			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
> -			drm_err(&i915->drm,
> -				"[ENCODER:%d:%s] Lane channel eq timeout\n",
> -				encoder->base.base.id, encoder->base.name);
> +			lt_err(intel_dp, DP_PHY_DPRX, "Lane channel eq timeout\n");
>  			return false;
>  		}
>  
> @@ -1259,18 +1197,14 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
>  		/* Update signal levels and training set as requested. */
>  		intel_dp_get_adjust_train(intel_dp, crtc_state, DP_PHY_DPRX, link_status);
>  		if (!intel_dp_update_link_train(intel_dp, crtc_state, DP_PHY_DPRX)) {
> -			drm_err(&i915->drm,
> -				"[ENCODER:%d:%s] Failed to update TX FFE settings\n",
> -				encoder->base.base.id, encoder->base.name);
> +			lt_err(intel_dp, DP_PHY_DPRX, "Failed to update TX FFE settings\n");
>  			return false;
>  		}
>  	}
>  
>  	if (try == max_tries) {
>  		intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
> -		drm_err(&i915->drm,
> -			"[ENCODER:%d:%s] Max loop count reached\n",
> -			encoder->base.base.id, encoder->base.name);
> +		lt_err(intel_dp, DP_PHY_DPRX, "Max loop count reached\n");
>  		return false;
>  	}
>  
> @@ -1279,32 +1213,24 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
>  			timeout = true; /* try one last time after deadline */
>  
>  		if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
> -			drm_err(&i915->drm,
> -				"[ENCODER:%d:%s] Failed to read link status\n",
> -				encoder->base.base.id, encoder->base.name);
> +			lt_err(intel_dp, DP_PHY_DPRX, "Failed to read link status\n");
>  			return false;
>  		}
>  
>  		if (drm_dp_128b132b_link_training_failed(link_status)) {
>  			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
> -			drm_err(&i915->drm,
> -				"[ENCODER:%d:%s] Downstream link training failure\n",
> -				encoder->base.base.id, encoder->base.name);
> +			lt_err(intel_dp, DP_PHY_DPRX, "Downstream link training failure\n");
>  			return false;
>  		}
>  
>  		if (drm_dp_128b132b_eq_interlane_align_done(link_status)) {
> -			drm_dbg_kms(&i915->drm,
> -				    "[ENCODER:%d:%s] Interlane align done\n",
> -				    encoder->base.base.id, encoder->base.name);
> +			lt_dbg(intel_dp, DP_PHY_DPRX, "Interlane align done\n");
>  			break;
>  		}
>  
>  		if (timeout) {
>  			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
> -			drm_err(&i915->drm,
> -				"[ENCODER:%d:%s] Interlane align timeout\n",
> -				encoder->base.base.id, encoder->base.name);
> +			lt_err(intel_dp, DP_PHY_DPRX, "Interlane align timeout\n");
>  			return false;
>  		}
>  
> @@ -1322,16 +1248,12 @@ intel_dp_128b132b_lane_cds(struct intel_dp *intel_dp,
>  			   const struct intel_crtc_state *crtc_state,
>  			   int lttpr_count)
>  {
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>  	u8 link_status[DP_LINK_STATUS_SIZE];
>  	unsigned long deadline;
>  
>  	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TRAINING_PATTERN_SET,
>  			       DP_TRAINING_PATTERN_2_CDS) != 1) {
> -		drm_err(&i915->drm,
> -			"[ENCODER:%d:%s] Failed to start 128b/132b TPS2 CDS\n",
> -			encoder->base.base.id, encoder->base.name);
> +		lt_err(intel_dp, DP_PHY_DPRX, "Failed to start 128b/132b TPS2 CDS\n");
>  		return false;
>  	}
>  
> @@ -1347,34 +1269,26 @@ intel_dp_128b132b_lane_cds(struct intel_dp *intel_dp,
>  		usleep_range(2000, 3000);
>  
>  		if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
> -			drm_err(&i915->drm,
> -				"[ENCODER:%d:%s] Failed to read link status\n",
> -				encoder->base.base.id, encoder->base.name);
> +			lt_err(intel_dp, DP_PHY_DPRX, "Failed to read link status\n");
>  			return false;
>  		}
>  
>  		if (drm_dp_128b132b_eq_interlane_align_done(link_status) &&
>  		    drm_dp_128b132b_cds_interlane_align_done(link_status) &&
>  		    drm_dp_128b132b_lane_symbol_locked(link_status, crtc_state->lane_count)) {
> -			drm_dbg_kms(&i915->drm,
> -				    "[ENCODER:%d:%s] CDS interlane align done\n",
> -				    encoder->base.base.id, encoder->base.name);
> +			lt_err(intel_dp, DP_PHY_DPRX, "CDS interlane align done\n");
>  			break;
>  		}
>  
>  		if (drm_dp_128b132b_link_training_failed(link_status)) {
>  			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
> -			drm_err(&i915->drm,
> -				"[ENCODER:%d:%s] Downstream link training failure\n",
> -				encoder->base.base.id, encoder->base.name);
> +			lt_err(intel_dp, DP_PHY_DPRX, "Downstream link training failure\n");
>  			return false;
>  		}
>  
>  		if (timeout) {
>  			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
> -			drm_err(&i915->drm,
> -				"[ENCODER:%d:%s] CDS timeout\n",
> -				encoder->base.base.id, encoder->base.name);
> +			lt_err(intel_dp, DP_PHY_DPRX, "CDS timeout\n");
>  			return false;
>  		}
>  	}
> @@ -1390,15 +1304,11 @@ intel_dp_128b132b_link_train(struct intel_dp *intel_dp,
>  			     const struct intel_crtc_state *crtc_state,
>  			     int lttpr_count)
>  {
> -	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>  	struct intel_connector *connector = intel_dp->attached_connector;
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>  	bool passed = false;
>  
>  	if (wait_for(intel_dp_128b132b_intra_hop(intel_dp, crtc_state) == 0, 500)) {
> -		drm_err(&i915->drm,
> -			"[ENCODER:%d:%s] 128b/132b intra-hop not clear\n",
> -			encoder->base.base.id, encoder->base.name);
> +		lt_err(intel_dp, DP_PHY_DPRX, "128b/132b intra-hop not clear\n");
>  		return false;
>  	}
>  
> @@ -1406,10 +1316,8 @@ intel_dp_128b132b_link_train(struct intel_dp *intel_dp,
>  	    intel_dp_128b132b_lane_cds(intel_dp, crtc_state, lttpr_count))
>  		passed = true;
>  
> -	drm_dbg_kms(&i915->drm,
> -		    "[CONNECTOR:%d:%s][ENCODER:%d:%s] 128b/132b Link Training %s at link rate = %d, lane count = %d\n",
> -		    connector->base.base.id, connector->base.name,
> -		    encoder->base.base.id, encoder->base.name,
> +	lt_conn_dbg(connector, intel_dp, DP_PHY_DPRX,
> +		    "128b/132b Link Training %s at link rate = %d, lane count = %d\n",
>  		    passed ? "passed" : "failed",
>  		    crtc_state->port_clock, crtc_state->lane_count);
>  
> @@ -1431,7 +1339,6 @@ void intel_dp_start_link_train(struct intel_dp *intel_dp,
>  {
>  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>  	struct intel_connector *connector = intel_dp->attached_connector;
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>  	bool passed;
>  
>  	/*
> @@ -1464,10 +1371,7 @@ void intel_dp_start_link_train(struct intel_dp *intel_dp,
>  	 * ignore_long_hpd flag can unset from the testcase.
>  	 */
>  	if (!passed && i915->display.hotplug.ignore_long_hpd) {
> -		drm_dbg_kms(&i915->drm,
> -			    "[CONNECTOR:%d:%s][ENCODER:%d:%s] Ignore the link failure\n",
> -			    connector->base.base.id, connector->base.name,
> -			    encoder->base.base.id, encoder->base.name);
> +		lt_conn_dbg(connector, intel_dp, DP_PHY_DPRX, "Ignore the link failure\n");
>  		return;
>  	}
>  
> @@ -1478,8 +1382,6 @@ void intel_dp_start_link_train(struct intel_dp *intel_dp,
>  void intel_dp_128b132b_sdp_crc16(struct intel_dp *intel_dp,
>  				 const struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> -
>  	/*
>  	 * VIDEO_DIP_CTL register bit 31 should be set to '0' to not
>  	 * disable SDP CRC. This is applicable for Display version 13.
> @@ -1492,5 +1394,5 @@ void intel_dp_128b132b_sdp_crc16(struct intel_dp *intel_dp,
>  				   DP_SDP_ERROR_DETECTION_CONFIGURATION,
>  				   DP_SDP_CRC16_128B132B_EN);
>  
> -	drm_dbg_kms(&i915->drm, "DP2.0 SDP CRC16 for 128b/132b enabled\n");
> +	lt_dbg(intel_dp, DP_PHY_DPRX, "DP2.0 SDP CRC16 for 128b/132b enabled\n");
>  }
> -- 
> 2.37.2

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 06/11] drm/i915/dp: Add link training debug and error printing helpers

[Imre Deak]

On Fri, Apr 28, 2023 at 05:21:53PM +0300, Ville Syrjälä wrote:
> On Wed, Apr 26, 2023 at 07:53:00PM +0300, Imre Deak wrote:
> > Add functions for printing link training debug and error messages, both
> > to prepare for the next patch, which downgrades an error to debug if the
> > sink is disconnected and to remove some code duplication.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  .../drm/i915/display/intel_dp_link_training.c | 376 +++++++-----------
> >  1 file changed, 139 insertions(+), 237 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > index 6aa4ae5e7ebe3..12f2880e474ed 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > @@ -26,6 +26,47 @@
> >  #include "intel_dp.h"
> >  #include "intel_dp_link_training.h"
> >  
> > +__printf(5, 6)
> > +static void lt_msg(struct intel_connector *connector, struct intel_dp *intel_dp, enum drm_dp_phy dp_phy,
> > +		   bool is_error, const char *format, ...)
> 
> I pretty much hate custom debug/error print functions. Makes it
> hard togrep/etc. and just generally causes more "wtf is this?"
> moments when reading the code. Unfortunateely the macro hell in
> drm_print.h seems to make it really hard to do anything generic
> directly :(

I guess could use something like drm_printf(&intel_dp->printer, ...)
instead, but that would still need passing the PHY format prefix at each
callsite.

> 
> > +{
> > +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > +	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > +	char conn_str[128] = {};
> > +	struct va_format vaf;
> > +	va_list args;
> > +
> > +	va_start(args, format);
> > +	vaf.fmt = format;
> > +	vaf.va = &args;
> > +
> > +	if (connector)
> > +		snprintf(conn_str, sizeof(conn_str), "[CONNECTOR:%d:%s]",
> > +			 connector->base.base.id, connector->base.name);
> 
> Are there actually places where we don't have/can't get the
> connector?

I guess link training messages could always print
intel_dp->attached_connector (not sure if for MST it's worth / practical
to list all the connectors).

> > +
> > +	if (is_error)
> > +		drm_err(&i915->drm, "%s[ENCODER:%d:%s][%s] %pV\n",
> > +			conn_str,
> > +			encoder->base.base.id, encoder->base.name,
> > +			drm_dp_phy_name(dp_phy),
> > +			&vaf);
> > +	else
> > +		drm_dbg(&i915->drm, "%s[ENCODER:%d:%s][%s] %pV\n",
> 
> That has lost the correct debug category.

Yes, should've been drm_dbg_kms().

> 
> > +			conn_str,
> > +			encoder->base.base.id, encoder->base.name,
> > +			drm_dp_phy_name(dp_phy),
> > +			&vaf);
> 
> Hmm. ___drm_dev_dbg() already uses this vaf stuff.
> Does chaining it like that work correctly?

Didn't think of it, but can't see why not, it's used at least in a few
places like that.

> > +}
> > +
> > +#define lt_err(intel_dp, dp_phy, format, ...) \
> > +	lt_msg(NULL, intel_dp, dp_phy, true, format, ## __VA_ARGS__)
> > +
> > +#define lt_dbg(intel_dp, dp_phy, format, ...) \
> > +	lt_msg(NULL, intel_dp, dp_phy, false, format, ## __VA_ARGS__)
> > +
> > +#define lt_conn_dbg(connector, intel_dp, dp_phy, format, ...) \
> > +	lt_msg(connector, intel_dp, dp_phy, false, format, ## __VA_ARGS__)
> > +
> >  static void intel_dp_reset_lttpr_common_caps(struct intel_dp *intel_dp)
> >  {
> >  	memset(intel_dp->lttpr_common_caps, 0, sizeof(intel_dp->lttpr_common_caps));
> > @@ -47,29 +88,21 @@ static void intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
> >  					 const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> >  					 enum drm_dp_phy dp_phy)
> >  {
> > -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> >  	u8 *phy_caps = intel_dp_lttpr_phy_caps(intel_dp, dp_phy);
> >  
> >  	if (drm_dp_read_lttpr_phy_caps(&intel_dp->aux, dpcd, dp_phy, phy_caps) < 0) {
> > -		drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
> > -			    "[ENCODER:%d:%s][%s] failed to read the PHY caps\n",
> > -			    encoder->base.base.id, encoder->base.name,
> > -			    drm_dp_phy_name(dp_phy));
> > +		lt_dbg(intel_dp, dp_phy, "failed to read the PHY caps\n");
> >  		return;
> >  	}
> >  
> > -	drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
> > -		    "[ENCODER:%d:%s][%s] PHY capabilities: %*ph\n",
> > -		    encoder->base.base.id, encoder->base.name,
> > -		    drm_dp_phy_name(dp_phy),
> > -		    (int)sizeof(intel_dp->lttpr_phy_caps[0]),
> > -		    phy_caps);
> > +	lt_dbg(intel_dp, dp_phy, "PHY capabilities: %*ph\n",
> > +	       (int)sizeof(intel_dp->lttpr_phy_caps[0]),
> > +	       phy_caps);
> >  }
> >  
> >  static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp,
> >  					    const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> >  {
> > -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> >  	int ret;
> >  
> >  	ret = drm_dp_read_lttpr_common_caps(&intel_dp->aux, dpcd,
> > @@ -77,11 +110,9 @@ static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp,
> >  	if (ret < 0)
> >  		goto reset_caps;
> >  
> > -	drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
> > -		    "[ENCODER:%d:%s] LTTPR common capabilities: %*ph\n",
> > -		    encoder->base.base.id, encoder->base.name,
> > -		    (int)sizeof(intel_dp->lttpr_common_caps),
> > -		    intel_dp->lttpr_common_caps);
> > +	lt_dbg(intel_dp, DP_PHY_DPRX, "LTTPR common capabilities: %*ph\n",
> > +	       (int)sizeof(intel_dp->lttpr_common_caps),
> > +	       intel_dp->lttpr_common_caps);
> >  
> >  	/* The minimum value of LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV is 1.4 */
> >  	if (intel_dp->lttpr_common_caps[0] < 0x14)
> > @@ -105,8 +136,6 @@ intel_dp_set_lttpr_transparent_mode(struct intel_dp *intel_dp, bool enable)
> >  
> >  static int intel_dp_init_lttpr(struct intel_dp *intel_dp, const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> >  {
> > -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >  	int lttpr_count;
> >  	int i;
> >  
> > @@ -138,9 +167,8 @@ static int intel_dp_init_lttpr(struct intel_dp *intel_dp, const u8 dpcd[DP_RECEI
> >  		return 0;
> >  
> >  	if (!intel_dp_set_lttpr_transparent_mode(intel_dp, false)) {
> > -		drm_dbg_kms(&i915->drm,
> > -			    "[ENCODER:%d:%s] Switching to LTTPR non-transparent LT mode failed, fall-back to transparent mode\n",
> > -			    encoder->base.base.id, encoder->base.name);
> > +		lt_dbg(intel_dp, DP_PHY_DPRX,
> > +		       "Switching to LTTPR non-transparent LT mode failed, fall-back to transparent mode\n");
> >  
> >  		intel_dp_set_lttpr_transparent_mode(intel_dp, true);
> >  		intel_dp_reset_lttpr_count(intel_dp);
> > @@ -409,26 +437,22 @@ intel_dp_get_adjust_train(struct intel_dp *intel_dp,
> >  			  enum drm_dp_phy dp_phy,
> >  			  const u8 link_status[DP_LINK_STATUS_SIZE])
> >  {
> > -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >  	int lane;
> >  
> >  	if (intel_dp_is_uhbr(crtc_state)) {
> > -		drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] 128b/132b, lanes: %d, "
> > -			    "TX FFE request: " TRAIN_REQ_FMT "\n",
> > -			    encoder->base.base.id, encoder->base.name,
> > -			    drm_dp_phy_name(dp_phy),
> > -			    crtc_state->lane_count,
> > -			    TRAIN_REQ_TX_FFE_ARGS(link_status));
> > +		lt_dbg(intel_dp, dp_phy,
> > +		       "128b/132b, lanes: %d, "
> > +		       "TX FFE request: " TRAIN_REQ_FMT "\n",
> > +		       crtc_state->lane_count,
> > +		       TRAIN_REQ_TX_FFE_ARGS(link_status));
> >  	} else {
> > -		drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] 8b/10b, lanes: %d, "
> > -			    "vswing request: " TRAIN_REQ_FMT ", "
> > -			    "pre-emphasis request: " TRAIN_REQ_FMT "\n",
> > -			    encoder->base.base.id, encoder->base.name,
> > -			    drm_dp_phy_name(dp_phy),
> > -			    crtc_state->lane_count,
> > -			    TRAIN_REQ_VSWING_ARGS(link_status),
> > -			    TRAIN_REQ_PREEMPH_ARGS(link_status));
> > +		lt_dbg(intel_dp, dp_phy,
> > +		       "8b/10b, lanes: %d, "
> > +		       "vswing request: " TRAIN_REQ_FMT ", "
> > +		       "pre-emphasis request: " TRAIN_REQ_FMT "\n",
> > +		       crtc_state->lane_count,
> > +		       TRAIN_REQ_VSWING_ARGS(link_status),
> > +		       TRAIN_REQ_PREEMPH_ARGS(link_status));
> >  	}
> >  
> >  	for (lane = 0; lane < 4; lane++)
> > @@ -487,16 +511,11 @@ intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
> >  				       enum drm_dp_phy dp_phy,
> >  				       u8 dp_train_pat)
> >  {
> > -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >  	u8 train_pat = intel_dp_training_pattern_symbol(dp_train_pat);
> >  
> >  	if (train_pat != DP_TRAINING_PATTERN_DISABLE)
> > -		drm_dbg_kms(&i915->drm,
> > -			    "[ENCODER:%d:%s][%s] Using DP training pattern TPS%c\n",
> > -			    encoder->base.base.id, encoder->base.name,
> > -			    drm_dp_phy_name(dp_phy),
> > -			    dp_training_pattern_name(train_pat));
> > +		lt_dbg(intel_dp, dp_phy, "Using DP training pattern TPS%c\n",
> > +		       dp_training_pattern_name(train_pat));
> >  
> >  	intel_dp->set_link_train(intel_dp, crtc_state, dp_train_pat);
> >  }
> > @@ -531,24 +550,21 @@ void intel_dp_set_signal_levels(struct intel_dp *intel_dp,
> >  				enum drm_dp_phy dp_phy)
> >  {
> >  	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >  
> >  	if (intel_dp_is_uhbr(crtc_state)) {
> > -		drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] 128b/132b, lanes: %d, "
> > -			    "TX FFE presets: " TRAIN_SET_FMT "\n",
> > -			    encoder->base.base.id, encoder->base.name,
> > -			    drm_dp_phy_name(dp_phy),
> > -			    crtc_state->lane_count,
> > -			    TRAIN_SET_TX_FFE_ARGS(intel_dp->train_set));
> > +		lt_dbg(intel_dp, dp_phy,
> > +		       "128b/132b, lanes: %d, "
> > +		       "TX FFE presets: " TRAIN_SET_FMT "\n",
> > +		       crtc_state->lane_count,
> > +		       TRAIN_SET_TX_FFE_ARGS(intel_dp->train_set));
> >  	} else {
> > -		drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s][%s] 8b/10b, lanes: %d, "
> > -			    "vswing levels: " TRAIN_SET_FMT ", "
> > -			    "pre-emphasis levels: " TRAIN_SET_FMT "\n",
> > -			    encoder->base.base.id, encoder->base.name,
> > -			    drm_dp_phy_name(dp_phy),
> > -			    crtc_state->lane_count,
> > -			    TRAIN_SET_VSWING_ARGS(intel_dp->train_set),
> > -			    TRAIN_SET_PREEMPH_ARGS(intel_dp->train_set));
> > +		lt_dbg(intel_dp, dp_phy,
> > +		       "8b/10b, lanes: %d, "
> > +		       "vswing levels: " TRAIN_SET_FMT ", "
> > +		       "pre-emphasis levels: " TRAIN_SET_FMT "\n",
> > +		       crtc_state->lane_count,
> > +		       TRAIN_SET_VSWING_ARGS(intel_dp->train_set),
> > +		       TRAIN_SET_PREEMPH_ARGS(intel_dp->train_set));
> >  	}
> >  
> >  	if (intel_dp_phy_is_downstream_of_source(intel_dp, dp_phy))
> > @@ -645,8 +661,6 @@ static bool
> >  intel_dp_prepare_link_train(struct intel_dp *intel_dp,
> >  			    const struct intel_crtc_state *crtc_state)
> >  {
> > -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >  	u8 link_config[2];
> >  	u8 link_bw, rate_select;
> >  
> > @@ -671,21 +685,19 @@ intel_dp_prepare_link_train(struct intel_dp *intel_dp,
> >  		struct intel_connector *connector = intel_dp->attached_connector;
> >  		__le16 sink_rates[DP_MAX_SUPPORTED_RATES];
> >  
> > -		drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Reloading eDP link rates\n",
> > -			    connector->base.base.id, connector->base.name);
> > +		lt_conn_dbg(connector, intel_dp, DP_PHY_DPRX,
> > +			    "Reloading eDP link rates\n");
> >  
> >  		drm_dp_dpcd_read(&intel_dp->aux, DP_SUPPORTED_LINK_RATES,
> >  				 sink_rates, sizeof(sink_rates));
> >  	}
> >  
> >  	if (link_bw)
> > -		drm_dbg_kms(&i915->drm,
> > -			    "[ENCODER:%d:%s] Using LINK_BW_SET value %02x\n",
> > -			    encoder->base.base.id, encoder->base.name, link_bw);
> > +		lt_dbg(intel_dp, DP_PHY_DPRX, "Using LINK_BW_SET value %02x\n",
> > +		       link_bw);
> >  	else
> > -		drm_dbg_kms(&i915->drm,
> > -			    "[ENCODER:%d:%s] Using LINK_RATE_SET value %02x\n",
> > -			    encoder->base.base.id, encoder->base.name, rate_select);
> > +		lt_dbg(intel_dp, DP_PHY_DPRX, "Using LINK_RATE_SET value %02x\n",
> > +		       rate_select);
> >  
> >  	/* Write the link configuration data */
> >  	link_config[0] = link_bw;
> > @@ -737,15 +749,10 @@ void
> >  intel_dp_dump_link_status(struct intel_dp *intel_dp, enum drm_dp_phy dp_phy,
> >  			  const u8 link_status[DP_LINK_STATUS_SIZE])
> >  {
> > -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > -
> > -	drm_dbg_kms(&i915->drm,
> > -		    "[ENCODER:%d:%s][%s] ln0_1:0x%x ln2_3:0x%x align:0x%x sink:0x%x adj_req0_1:0x%x adj_req2_3:0x%x\n",
> > -		    encoder->base.base.id, encoder->base.name,
> > -		    drm_dp_phy_name(dp_phy),
> > -		    link_status[0], link_status[1], link_status[2],
> > -		    link_status[3], link_status[4], link_status[5]);
> > +	lt_dbg(intel_dp, dp_phy,
> > +	       "ln0_1:0x%x ln2_3:0x%x align:0x%x sink:0x%x adj_req0_1:0x%x adj_req2_3:0x%x\n",
> > +	       link_status[0], link_status[1], link_status[2],
> > +	       link_status[3], link_status[4], link_status[5]);
> >  }
> >  
> >  /*
> > @@ -757,8 +764,6 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
> >  				      const struct intel_crtc_state *crtc_state,
> >  				      enum drm_dp_phy dp_phy)
> >  {
> > -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >  	u8 old_link_status[DP_LINK_STATUS_SIZE] = {};
> >  	int voltage_tries, cr_tries, max_cr_tries;
> >  	u8 link_status[DP_LINK_STATUS_SIZE];
> > @@ -773,9 +778,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
> >  	if (!intel_dp_reset_link_train(intel_dp, crtc_state, dp_phy,
> >  				       DP_TRAINING_PATTERN_1 |
> >  				       DP_LINK_SCRAMBLING_DISABLE)) {
> > -		drm_err(&i915->drm, "[ENCODER:%d:%s][%s] Failed to enable link training\n",
> > -			encoder->base.base.id, encoder->base.name,
> > -			drm_dp_phy_name(dp_phy));
> > +		lt_err(intel_dp, dp_phy, "Failed to enable link training\n");
> >  		return false;
> >  	}
> >  
> > @@ -798,35 +801,24 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
> >  
> >  		if (drm_dp_dpcd_read_phy_link_status(&intel_dp->aux, dp_phy,
> >  						     link_status) < 0) {
> > -			drm_err(&i915->drm, "[ENCODER:%d:%s][%s] Failed to get link status\n",
> > -				encoder->base.base.id, encoder->base.name,
> > -				drm_dp_phy_name(dp_phy));
> > +			lt_err(intel_dp, dp_phy, "Failed to get link status\n");
> >  			return false;
> >  		}
> >  
> >  		if (drm_dp_clock_recovery_ok(link_status, crtc_state->lane_count)) {
> > -			drm_dbg_kms(&i915->drm,
> > -				    "[ENCODER:%d:%s][%s] Clock recovery OK\n",
> > -				    encoder->base.base.id, encoder->base.name,
> > -				    drm_dp_phy_name(dp_phy));
> > +			lt_dbg(intel_dp, dp_phy, "Clock recovery OK\n");
> >  			return true;
> >  		}
> >  
> >  		if (voltage_tries == 5) {
> >  			intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
> > -			drm_dbg_kms(&i915->drm,
> > -				    "[ENCODER:%d:%s][%s] Same voltage tried 5 times\n",
> > -				    encoder->base.base.id, encoder->base.name,
> > -				    drm_dp_phy_name(dp_phy));
> > +			lt_dbg(intel_dp, dp_phy, "Same voltage tried 5 times\n");
> >  			return false;
> >  		}
> >  
> >  		if (max_vswing_reached) {
> >  			intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
> > -			drm_dbg_kms(&i915->drm,
> > -				    "[ENCODER:%d:%s][%s] Max Voltage Swing reached\n",
> > -				    encoder->base.base.id, encoder->base.name,
> > -				    drm_dp_phy_name(dp_phy));
> > +			lt_dbg(intel_dp, dp_phy, "Max Voltage Swing reached\n");
> >  			return false;
> >  		}
> >  
> > @@ -834,10 +826,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
> >  		intel_dp_get_adjust_train(intel_dp, crtc_state, dp_phy,
> >  					  link_status);
> >  		if (!intel_dp_update_link_train(intel_dp, crtc_state, dp_phy)) {
> > -			drm_err(&i915->drm,
> > -				"[ENCODER:%d:%s][%s] Failed to update link training\n",
> > -				encoder->base.base.id, encoder->base.name,
> > -				drm_dp_phy_name(dp_phy));
> > +			lt_err(intel_dp, dp_phy, "Failed to update link training\n");
> >  			return false;
> >  		}
> >  
> > @@ -853,10 +842,8 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
> >  	}
> >  
> >  	intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
> > -	drm_err(&i915->drm,
> > -		"[ENCODER:%d:%s][%s] Failed clock recovery %d times, giving up!\n",
> > -		encoder->base.base.id, encoder->base.name,
> > -		drm_dp_phy_name(dp_phy), max_cr_tries);
> > +	lt_err(intel_dp, dp_phy, "Failed clock recovery %d times, giving up!\n",
> > +	       max_cr_tries);
> >  
> >  	return false;
> >  }
> > @@ -890,11 +877,11 @@ static u32 intel_dp_training_pattern(struct intel_dp *intel_dp,
> >  		return DP_TRAINING_PATTERN_4;
> >  	} else if (crtc_state->port_clock == 810000) {
> >  		if (!source_tps4)
> > -			drm_dbg_kms(&i915->drm,
> > -				    "8.1 Gbps link rate without source TPS4 support\n");
> > +			lt_dbg(intel_dp, dp_phy,
> > +			       "8.1 Gbps link rate without source TPS4 support\n");
> >  		if (!sink_tps4)
> > -			drm_dbg_kms(&i915->drm,
> > -				    "8.1 Gbps link rate without sink TPS4 support\n");
> > +			lt_dbg(intel_dp, dp_phy,
> > +			       "8.1 Gbps link rate without sink TPS4 support\n");
> >  	}
> >  
> >  	/*
> > @@ -908,11 +895,11 @@ static u32 intel_dp_training_pattern(struct intel_dp *intel_dp,
> >  		return  DP_TRAINING_PATTERN_3;
> >  	} else if (crtc_state->port_clock >= 540000) {
> >  		if (!source_tps3)
> > -			drm_dbg_kms(&i915->drm,
> > -				    ">=5.4/6.48 Gbps link rate without source TPS3 support\n");
> > +			lt_dbg(intel_dp, dp_phy,
> > +			       ">=5.4/6.48 Gbps link rate without source TPS3 support\n");
> >  		if (!sink_tps3)
> > -			drm_dbg_kms(&i915->drm,
> > -				    ">=5.4/6.48 Gbps link rate without sink TPS3 support\n");
> > +			lt_dbg(intel_dp, dp_phy,
> > +			       ">=5.4/6.48 Gbps link rate without sink TPS3 support\n");
> >  	}
> >  
> >  	return DP_TRAINING_PATTERN_2;
> > @@ -928,8 +915,6 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
> >  					    const struct intel_crtc_state *crtc_state,
> >  					    enum drm_dp_phy dp_phy)
> >  {
> > -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >  	int tries;
> >  	u32 training_pattern;
> >  	u8 link_status[DP_LINK_STATUS_SIZE];
> > @@ -948,10 +933,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
> >  	/* channel equalization */
> >  	if (!intel_dp_set_link_train(intel_dp, crtc_state, dp_phy,
> >  				     training_pattern)) {
> > -		drm_err(&i915->drm,
> > -			"[ENCODER:%d:%s][%s] Failed to start channel equalization\n",
> > -			encoder->base.base.id, encoder->base.name,
> > -			drm_dp_phy_name(dp_phy));
> > +		lt_err(intel_dp, dp_phy, "Failed to start channel equalization\n");
> >  		return false;
> >  	}
> >  
> > @@ -960,10 +942,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
> >  
> >  		if (drm_dp_dpcd_read_phy_link_status(&intel_dp->aux, dp_phy,
> >  						     link_status) < 0) {
> > -			drm_err(&i915->drm,
> > -				"[ENCODER:%d:%s][%s] Failed to get link status\n",
> > -				encoder->base.base.id, encoder->base.name,
> > -				drm_dp_phy_name(dp_phy));
> > +			lt_err(intel_dp, dp_phy, "Failed to get link status\n");
> >  			break;
> >  		}
> >  
> > @@ -971,21 +950,15 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
> >  		if (!drm_dp_clock_recovery_ok(link_status,
> >  					      crtc_state->lane_count)) {
> >  			intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
> > -			drm_dbg_kms(&i915->drm,
> > -				    "[ENCODER:%d:%s][%s] Clock recovery check failed, cannot "
> > -				    "continue channel equalization\n",
> > -				    encoder->base.base.id, encoder->base.name,
> > -				    drm_dp_phy_name(dp_phy));
> > +			lt_dbg(intel_dp, dp_phy,
> > +			       "Clock recovery check failed, cannot continue channel equalization\n");
> >  			break;
> >  		}
> >  
> >  		if (drm_dp_channel_eq_ok(link_status,
> >  					 crtc_state->lane_count)) {
> >  			channel_eq = true;
> > -			drm_dbg_kms(&i915->drm,
> > -				    "[ENCODER:%d:%s][%s] Channel EQ done. DP Training successful\n",
> > -				    encoder->base.base.id, encoder->base.name,
> > -				    drm_dp_phy_name(dp_phy));
> > +			lt_dbg(intel_dp, dp_phy, "Channel EQ done. DP Training successful\n");
> >  			break;
> >  		}
> >  
> > @@ -993,10 +966,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
> >  		intel_dp_get_adjust_train(intel_dp, crtc_state, dp_phy,
> >  					  link_status);
> >  		if (!intel_dp_update_link_train(intel_dp, crtc_state, dp_phy)) {
> > -			drm_err(&i915->drm,
> > -				"[ENCODER:%d:%s][%s] Failed to update link training\n",
> > -				encoder->base.base.id, encoder->base.name,
> > -				drm_dp_phy_name(dp_phy));
> > +			lt_err(intel_dp, dp_phy, "Failed to update link training\n");
> >  			break;
> >  		}
> >  	}
> > @@ -1004,10 +974,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp,
> >  	/* Try 5 times, else fail and try at lower BW */
> >  	if (tries == 5) {
> >  		intel_dp_dump_link_status(intel_dp, dp_phy, link_status);
> > -		drm_dbg_kms(&i915->drm,
> > -			    "[ENCODER:%d:%s][%s] Channel equalization failed 5 times\n",
> > -			    encoder->base.base.id, encoder->base.name,
> > -			    drm_dp_phy_name(dp_phy));
> > +		lt_dbg(intel_dp, dp_phy, "Channel equalization failed 5 times\n");
> >  	}
> >  
> >  	return channel_eq;
> > @@ -1058,9 +1025,6 @@ intel_dp_128b132b_intra_hop(struct intel_dp *intel_dp,
> >  void intel_dp_stop_link_train(struct intel_dp *intel_dp,
> >  			      const struct intel_crtc_state *crtc_state)
> >  {
> > -	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > -
> >  	intel_dp->link_trained = true;
> >  
> >  	intel_dp_disable_dpcd_training_pattern(intel_dp, DP_PHY_DPRX);
> > @@ -1069,9 +1033,7 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp,
> >  
> >  	if (intel_dp_is_uhbr(crtc_state) &&
> >  	    wait_for(intel_dp_128b132b_intra_hop(intel_dp, crtc_state) == 0, 500)) {
> > -		drm_dbg_kms(&i915->drm,
> > -			    "[ENCODER:%d:%s] 128b/132b intra-hop not clearing\n",
> > -			    encoder->base.base.id, encoder->base.name);
> > +		lt_dbg(intel_dp, DP_PHY_DPRX, "128b/132b intra-hop not clearing\n");
> >  	}
> >  }
> >  
> > @@ -1081,7 +1043,6 @@ intel_dp_link_train_phy(struct intel_dp *intel_dp,
> >  			enum drm_dp_phy dp_phy)
> >  {
> >  	struct intel_connector *connector = intel_dp->attached_connector;
> > -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> >  	bool ret = false;
> >  
> >  	if (!intel_dp_link_training_clock_recovery(intel_dp, crtc_state, dp_phy))
> > @@ -1093,11 +1054,8 @@ intel_dp_link_train_phy(struct intel_dp *intel_dp,
> >  	ret = true;
> >  
> >  out:
> > -	drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
> > -		    "[CONNECTOR:%d:%s][ENCODER:%d:%s][%s] Link Training %s at link rate = %d, lane count = %d\n",
> > -		    connector->base.base.id, connector->base.name,
> > -		    encoder->base.base.id, encoder->base.name,
> > -		    drm_dp_phy_name(dp_phy),
> > +	lt_conn_dbg(connector, intel_dp, dp_phy,
> > +		    "Link Training %s at link rate = %d, lane count = %d\n",
> >  		    ret ? "passed" : "failed",
> >  		    crtc_state->port_clock, crtc_state->lane_count);
> >  
> > @@ -1108,13 +1066,10 @@ static void intel_dp_schedule_fallback_link_training(struct intel_dp *intel_dp,
> >  						     const struct intel_crtc_state *crtc_state)
> >  {
> >  	struct intel_connector *intel_connector = intel_dp->attached_connector;
> > -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> >  
> >  	if (intel_dp->hobl_active) {
> > -		drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
> > -			    "[ENCODER:%d:%s] Link Training failed with HOBL active, "
> > -			    "not enabling it from now on",
> > -			    encoder->base.base.id, encoder->base.name);
> > +		lt_dbg(intel_dp, DP_PHY_DPRX,
> > +		       "Link Training failed with HOBL active, not enabling it from now on\n");
> >  		intel_dp->hobl_failed = true;
> >  	} else if (intel_dp_get_link_train_fallback_values(intel_dp,
> >  							   crtc_state->port_clock,
> > @@ -1161,8 +1116,6 @@ static bool
> >  intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
> >  			  const struct intel_crtc_state *crtc_state)
> >  {
> > -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >  	u8 link_status[DP_LINK_STATUS_SIZE];
> >  	int delay_us;
> >  	int try, max_tries = 20;
> > @@ -1177,9 +1130,7 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
> >  	 */
> >  	if (!intel_dp_reset_link_train(intel_dp, crtc_state, DP_PHY_DPRX,
> >  				       DP_TRAINING_PATTERN_1)) {
> > -		drm_err(&i915->drm,
> > -			"[ENCODER:%d:%s] Failed to start 128b/132b TPS1\n",
> > -			encoder->base.base.id, encoder->base.name);
> > +		lt_err(intel_dp, DP_PHY_DPRX, "Failed to start 128b/132b TPS1\n");
> >  		return false;
> >  	}
> >  
> > @@ -1187,27 +1138,21 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
> >  
> >  	/* Read the initial TX FFE settings. */
> >  	if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
> > -		drm_err(&i915->drm,
> > -			"[ENCODER:%d:%s] Failed to read TX FFE presets\n",
> > -			encoder->base.base.id, encoder->base.name);
> > +		lt_err(intel_dp, DP_PHY_DPRX, "Failed to read TX FFE presets\n");
> >  		return false;
> >  	}
> >  
> >  	/* Update signal levels and training set as requested. */
> >  	intel_dp_get_adjust_train(intel_dp, crtc_state, DP_PHY_DPRX, link_status);
> >  	if (!intel_dp_update_link_train(intel_dp, crtc_state, DP_PHY_DPRX)) {
> > -		drm_err(&i915->drm,
> > -			"[ENCODER:%d:%s] Failed to set initial TX FFE settings\n",
> > -			encoder->base.base.id, encoder->base.name);
> > +		lt_err(intel_dp, DP_PHY_DPRX, "Failed to set initial TX FFE settings\n");
> >  		return false;
> >  	}
> >  
> >  	/* Start transmitting 128b/132b TPS2. */
> >  	if (!intel_dp_set_link_train(intel_dp, crtc_state, DP_PHY_DPRX,
> >  				     DP_TRAINING_PATTERN_2)) {
> > -		drm_err(&i915->drm,
> > -			"[ENCODER:%d:%s] Failed to start 128b/132b TPS2\n",
> > -			encoder->base.base.id, encoder->base.name);
> > +		lt_err(intel_dp, DP_PHY_DPRX, "Failed to start 128b/132b TPS2\n");
> >  		return false;
> >  	}
> >  
> > @@ -1224,32 +1169,25 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
> >  		delay_us = drm_dp_128b132b_read_aux_rd_interval(&intel_dp->aux);
> >  
> >  		if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
> > -			drm_err(&i915->drm,
> > -				"[ENCODER:%d:%s] Failed to read link status\n",
> > -				encoder->base.base.id, encoder->base.name);
> > +			lt_err(intel_dp, DP_PHY_DPRX, "Failed to read link status\n");
> >  			return false;
> >  		}
> >  
> >  		if (drm_dp_128b132b_link_training_failed(link_status)) {
> >  			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
> > -			drm_err(&i915->drm,
> > -				"[ENCODER:%d:%s] Downstream link training failure\n",
> > -				encoder->base.base.id, encoder->base.name);
> > +			lt_err(intel_dp, DP_PHY_DPRX,
> > +			       "Downstream link training failure\n");
> >  			return false;
> >  		}
> >  
> >  		if (drm_dp_128b132b_lane_channel_eq_done(link_status, crtc_state->lane_count)) {
> > -			drm_dbg_kms(&i915->drm,
> > -				    "[ENCODER:%d:%s] Lane channel eq done\n",
> > -				    encoder->base.base.id, encoder->base.name);
> > +			lt_dbg(intel_dp, DP_PHY_DPRX, "Lane channel eq done\n");
> >  			break;
> >  		}
> >  
> >  		if (timeout) {
> >  			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
> > -			drm_err(&i915->drm,
> > -				"[ENCODER:%d:%s] Lane channel eq timeout\n",
> > -				encoder->base.base.id, encoder->base.name);
> > +			lt_err(intel_dp, DP_PHY_DPRX, "Lane channel eq timeout\n");
> >  			return false;
> >  		}
> >  
> > @@ -1259,18 +1197,14 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
> >  		/* Update signal levels and training set as requested. */
> >  		intel_dp_get_adjust_train(intel_dp, crtc_state, DP_PHY_DPRX, link_status);
> >  		if (!intel_dp_update_link_train(intel_dp, crtc_state, DP_PHY_DPRX)) {
> > -			drm_err(&i915->drm,
> > -				"[ENCODER:%d:%s] Failed to update TX FFE settings\n",
> > -				encoder->base.base.id, encoder->base.name);
> > +			lt_err(intel_dp, DP_PHY_DPRX, "Failed to update TX FFE settings\n");
> >  			return false;
> >  		}
> >  	}
> >  
> >  	if (try == max_tries) {
> >  		intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
> > -		drm_err(&i915->drm,
> > -			"[ENCODER:%d:%s] Max loop count reached\n",
> > -			encoder->base.base.id, encoder->base.name);
> > +		lt_err(intel_dp, DP_PHY_DPRX, "Max loop count reached\n");
> >  		return false;
> >  	}
> >  
> > @@ -1279,32 +1213,24 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
> >  			timeout = true; /* try one last time after deadline */
> >  
> >  		if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
> > -			drm_err(&i915->drm,
> > -				"[ENCODER:%d:%s] Failed to read link status\n",
> > -				encoder->base.base.id, encoder->base.name);
> > +			lt_err(intel_dp, DP_PHY_DPRX, "Failed to read link status\n");
> >  			return false;
> >  		}
> >  
> >  		if (drm_dp_128b132b_link_training_failed(link_status)) {
> >  			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
> > -			drm_err(&i915->drm,
> > -				"[ENCODER:%d:%s] Downstream link training failure\n",
> > -				encoder->base.base.id, encoder->base.name);
> > +			lt_err(intel_dp, DP_PHY_DPRX, "Downstream link training failure\n");
> >  			return false;
> >  		}
> >  
> >  		if (drm_dp_128b132b_eq_interlane_align_done(link_status)) {
> > -			drm_dbg_kms(&i915->drm,
> > -				    "[ENCODER:%d:%s] Interlane align done\n",
> > -				    encoder->base.base.id, encoder->base.name);
> > +			lt_dbg(intel_dp, DP_PHY_DPRX, "Interlane align done\n");
> >  			break;
> >  		}
> >  
> >  		if (timeout) {
> >  			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
> > -			drm_err(&i915->drm,
> > -				"[ENCODER:%d:%s] Interlane align timeout\n",
> > -				encoder->base.base.id, encoder->base.name);
> > +			lt_err(intel_dp, DP_PHY_DPRX, "Interlane align timeout\n");
> >  			return false;
> >  		}
> >  
> > @@ -1322,16 +1248,12 @@ intel_dp_128b132b_lane_cds(struct intel_dp *intel_dp,
> >  			   const struct intel_crtc_state *crtc_state,
> >  			   int lttpr_count)
> >  {
> > -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > -	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >  	u8 link_status[DP_LINK_STATUS_SIZE];
> >  	unsigned long deadline;
> >  
> >  	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TRAINING_PATTERN_SET,
> >  			       DP_TRAINING_PATTERN_2_CDS) != 1) {
> > -		drm_err(&i915->drm,
> > -			"[ENCODER:%d:%s] Failed to start 128b/132b TPS2 CDS\n",
> > -			encoder->base.base.id, encoder->base.name);
> > +		lt_err(intel_dp, DP_PHY_DPRX, "Failed to start 128b/132b TPS2 CDS\n");
> >  		return false;
> >  	}
> >  
> > @@ -1347,34 +1269,26 @@ intel_dp_128b132b_lane_cds(struct intel_dp *intel_dp,
> >  		usleep_range(2000, 3000);
> >  
> >  		if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
> > -			drm_err(&i915->drm,
> > -				"[ENCODER:%d:%s] Failed to read link status\n",
> > -				encoder->base.base.id, encoder->base.name);
> > +			lt_err(intel_dp, DP_PHY_DPRX, "Failed to read link status\n");
> >  			return false;
> >  		}
> >  
> >  		if (drm_dp_128b132b_eq_interlane_align_done(link_status) &&
> >  		    drm_dp_128b132b_cds_interlane_align_done(link_status) &&
> >  		    drm_dp_128b132b_lane_symbol_locked(link_status, crtc_state->lane_count)) {
> > -			drm_dbg_kms(&i915->drm,
> > -				    "[ENCODER:%d:%s] CDS interlane align done\n",
> > -				    encoder->base.base.id, encoder->base.name);
> > +			lt_err(intel_dp, DP_PHY_DPRX, "CDS interlane align done\n");
> >  			break;
> >  		}
> >  
> >  		if (drm_dp_128b132b_link_training_failed(link_status)) {
> >  			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
> > -			drm_err(&i915->drm,
> > -				"[ENCODER:%d:%s] Downstream link training failure\n",
> > -				encoder->base.base.id, encoder->base.name);
> > +			lt_err(intel_dp, DP_PHY_DPRX, "Downstream link training failure\n");
> >  			return false;
> >  		}
> >  
> >  		if (timeout) {
> >  			intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
> > -			drm_err(&i915->drm,
> > -				"[ENCODER:%d:%s] CDS timeout\n",
> > -				encoder->base.base.id, encoder->base.name);
> > +			lt_err(intel_dp, DP_PHY_DPRX, "CDS timeout\n");
> >  			return false;
> >  		}
> >  	}
> > @@ -1390,15 +1304,11 @@ intel_dp_128b132b_link_train(struct intel_dp *intel_dp,
> >  			     const struct intel_crtc_state *crtc_state,
> >  			     int lttpr_count)
> >  {
> > -	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> >  	struct intel_connector *connector = intel_dp->attached_connector;
> > -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> >  	bool passed = false;
> >  
> >  	if (wait_for(intel_dp_128b132b_intra_hop(intel_dp, crtc_state) == 0, 500)) {
> > -		drm_err(&i915->drm,
> > -			"[ENCODER:%d:%s] 128b/132b intra-hop not clear\n",
> > -			encoder->base.base.id, encoder->base.name);
> > +		lt_err(intel_dp, DP_PHY_DPRX, "128b/132b intra-hop not clear\n");
> >  		return false;
> >  	}
> >  
> > @@ -1406,10 +1316,8 @@ intel_dp_128b132b_link_train(struct intel_dp *intel_dp,
> >  	    intel_dp_128b132b_lane_cds(intel_dp, crtc_state, lttpr_count))
> >  		passed = true;
> >  
> > -	drm_dbg_kms(&i915->drm,
> > -		    "[CONNECTOR:%d:%s][ENCODER:%d:%s] 128b/132b Link Training %s at link rate = %d, lane count = %d\n",
> > -		    connector->base.base.id, connector->base.name,
> > -		    encoder->base.base.id, encoder->base.name,
> > +	lt_conn_dbg(connector, intel_dp, DP_PHY_DPRX,
> > +		    "128b/132b Link Training %s at link rate = %d, lane count = %d\n",
> >  		    passed ? "passed" : "failed",
> >  		    crtc_state->port_clock, crtc_state->lane_count);
> >  
> > @@ -1431,7 +1339,6 @@ void intel_dp_start_link_train(struct intel_dp *intel_dp,
> >  {
> >  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> >  	struct intel_connector *connector = intel_dp->attached_connector;
> > -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> >  	bool passed;
> >  
> >  	/*
> > @@ -1464,10 +1371,7 @@ void intel_dp_start_link_train(struct intel_dp *intel_dp,
> >  	 * ignore_long_hpd flag can unset from the testcase.
> >  	 */
> >  	if (!passed && i915->display.hotplug.ignore_long_hpd) {
> > -		drm_dbg_kms(&i915->drm,
> > -			    "[CONNECTOR:%d:%s][ENCODER:%d:%s] Ignore the link failure\n",
> > -			    connector->base.base.id, connector->base.name,
> > -			    encoder->base.base.id, encoder->base.name);
> > +		lt_conn_dbg(connector, intel_dp, DP_PHY_DPRX, "Ignore the link failure\n");
> >  		return;
> >  	}
> >  
> > @@ -1478,8 +1382,6 @@ void intel_dp_start_link_train(struct intel_dp *intel_dp,
> >  void intel_dp_128b132b_sdp_crc16(struct intel_dp *intel_dp,
> >  				 const struct intel_crtc_state *crtc_state)
> >  {
> > -	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > -
> >  	/*
> >  	 * VIDEO_DIP_CTL register bit 31 should be set to '0' to not
> >  	 * disable SDP CRC. This is applicable for Display version 13.
> > @@ -1492,5 +1394,5 @@ void intel_dp_128b132b_sdp_crc16(struct intel_dp *intel_dp,
> >  				   DP_SDP_ERROR_DETECTION_CONFIGURATION,
> >  				   DP_SDP_CRC16_128B132B_EN);
> >  
> > -	drm_dbg_kms(&i915->drm, "DP2.0 SDP CRC16 for 128b/132b enabled\n");
> > +	lt_dbg(intel_dp, DP_PHY_DPRX, "DP2.0 SDP CRC16 for 128b/132b enabled\n");
> >  }
> > -- 
> > 2.37.2
> 
> -- 
> Ville Syrjälä
> Intel

[Intel-gfx] [PATCH 07/11] drm/i915/dp: Convert link training error to debug message on disconnected sink

[Imre Deak]

If a sink is disconnected it's expected that link training actions will
fail on it, so downgrade the error messages about such actions to be a
debug message. Such - expected - link training failures are more
frequent after a follow up patch, after which an active TypeC link is
reset after the sink is disconnected which also involves a link
training.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c               |  2 +-
 drivers/gpu/drm/i915/display/intel_dp.h               |  1 +
 drivers/gpu/drm/i915/display/intel_dp_link_training.c | 11 +++++++++--
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 1d28a2560ae01..388e79f74621f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4171,7 +4171,7 @@ static int intel_dp_prep_link_retrain(struct intel_dp *intel_dp,
 	return ret;
 }
 
-static bool intel_dp_is_connected(struct intel_dp *intel_dp)
+bool intel_dp_is_connected(struct intel_dp *intel_dp)
 {
 	struct intel_connector *connector = intel_dp->attached_connector;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index ef39e4f7a329e..488da392fafe5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -42,6 +42,7 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp,
 			      int link_rate, int lane_count);
 int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
 					    int link_rate, u8 lane_count);
+bool intel_dp_is_connected(struct intel_dp *intel_dp);
 int intel_dp_retrain_link(struct intel_encoder *encoder,
 			  struct drm_modeset_acquire_ctx *ctx);
 void intel_dp_set_power(struct intel_dp *intel_dp, u8 mode);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index 12f2880e474ed..a747249c409a0 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -33,6 +33,7 @@ static void lt_msg(struct intel_connector *connector, struct intel_dp *intel_dp,
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
 	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
 	char conn_str[128] = {};
+	const char *discon_str = "";
 	struct va_format vaf;
 	va_list args;
 
@@ -44,6 +45,11 @@ static void lt_msg(struct intel_connector *connector, struct intel_dp *intel_dp,
 		snprintf(conn_str, sizeof(conn_str), "[CONNECTOR:%d:%s]",
 			 connector->base.base.id, connector->base.name);
 
+	if (is_error && !intel_dp_is_connected(intel_dp)) {
+		discon_str = " (sink disconnected)";
+		is_error = false;
+	}
+
 	if (is_error)
 		drm_err(&i915->drm, "%s[ENCODER:%d:%s][%s] %pV\n",
 			conn_str,
@@ -51,11 +57,12 @@ static void lt_msg(struct intel_connector *connector, struct intel_dp *intel_dp,
 			drm_dp_phy_name(dp_phy),
 			&vaf);
 	else
-		drm_dbg(&i915->drm, "%s[ENCODER:%d:%s][%s] %pV\n",
+		drm_dbg(&i915->drm, "%s[ENCODER:%d:%s][%s] %pV%s\n",
 			conn_str,
 			encoder->base.base.id, encoder->base.name,
 			drm_dp_phy_name(dp_phy),
-			&vaf);
+			&vaf,
+			discon_str);
 }
 
 #define lt_err(intel_dp, dp_phy, format, ...) \
-- 
2.37.2

[Intel-gfx] [PATCH 08/11] drm/i915/dp: Prevent link training fallback on disconnected port

[Imre Deak]

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).

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.h               | 1 +
 drivers/gpu/drm/i915/display/intel_dp_link_training.c | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 488da392fafe5..ca12a1733df6f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -102,6 +102,7 @@ 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);
+bool intel_dp_is_connected(struct intel_dp *intel_dp);
 bool intel_digital_port_connected(struct intel_encoder *encoder);
 int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 dsc_max_bpc);
 u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index a747249c409a0..bab95cbcbdfac 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -1074,6 +1074,11 @@ static void intel_dp_schedule_fallback_link_training(struct intel_dp *intel_dp,
 {
 	struct intel_connector *intel_connector = intel_dp->attached_connector;
 
+	if (!intel_dp_is_connected(intel_dp)) {
+		lt_dbg(intel_dp, DP_PHY_DPRX, "Link Training failed on disconnected sink.\n");
+		return;
+	}
+
 	if (intel_dp->hobl_active) {
 		lt_dbg(intel_dp, DP_PHY_DPRX,
 		       "Link Training failed with HOBL active, not enabling it from now on\n");
-- 
2.37.2

[Intel-gfx] [PATCH 09/11] drm/i915/dp: Factor out intel_dp_get_active_pipes()

[Imre Deak]

Factor out a helper used by a follow up patch to reset an active DP
link.

No functional changes.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 388e79f74621f..1e91175506f5d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4118,9 +4118,9 @@ static bool intel_dp_has_connector(struct intel_dp *intel_dp,
 	return false;
 }
 
-static int intel_dp_prep_link_retrain(struct intel_dp *intel_dp,
-				      struct drm_modeset_acquire_ctx *ctx,
-				      u8 *pipe_mask)
+static int intel_dp_get_active_pipes(struct intel_dp *intel_dp,
+				     struct drm_modeset_acquire_ctx *ctx,
+				     u8 *pipe_mask)
 {
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
 	struct drm_connector_list_iter conn_iter;
@@ -4129,9 +4129,6 @@ static int intel_dp_prep_link_retrain(struct intel_dp *intel_dp,
 
 	*pipe_mask = 0;
 
-	if (!intel_dp_needs_link_retrain(intel_dp))
-		return 0;
-
 	drm_connector_list_iter_begin(&i915->drm, &conn_iter);
 	for_each_intel_connector_iter(connector, &conn_iter) {
 		struct drm_connector_state *conn_state =
@@ -4165,9 +4162,6 @@ static int intel_dp_prep_link_retrain(struct intel_dp *intel_dp,
 	}
 	drm_connector_list_iter_end(&conn_iter);
 
-	if (!intel_dp_needs_link_retrain(intel_dp))
-		*pipe_mask = 0;
-
 	return ret;
 }
 
@@ -4196,13 +4190,19 @@ int intel_dp_retrain_link(struct intel_encoder *encoder,
 	if (ret)
 		return ret;
 
-	ret = intel_dp_prep_link_retrain(intel_dp, ctx, &pipe_mask);
+	if (!intel_dp_needs_link_retrain(intel_dp))
+		return 0;
+
+	ret = intel_dp_get_active_pipes(intel_dp, ctx, &pipe_mask);
 	if (ret)
 		return ret;
 
 	if (pipe_mask == 0)
 		return 0;
 
+	if (!intel_dp_needs_link_retrain(intel_dp))
+		return 0;
+
 	drm_dbg_kms(&dev_priv->drm, "[ENCODER:%d:%s] retraining link\n",
 		    encoder->base.base.id, encoder->base.name);
 
-- 
2.37.2

[Intel-gfx] [PATCH 10/11] drm/i915: Factor out call_with_modeset_ctx()

[Imre Deak]

Factor out a helper to call a function with the atomic locks held,
required by a follow-up patch resetting an active DP link.

No functional changes.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 53 ++++++++++++++----------
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 29e4bfab46358..0c8bc32f293b0 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4370,35 +4370,18 @@ static int intel_hdmi_reset_link(struct intel_encoder *encoder,
 	return modeset_pipe(&crtc->base, ctx);
 }
 
-static enum intel_hotplug_state
-intel_ddi_hotplug(struct intel_encoder *encoder,
-		  struct intel_connector *connector)
+static void call_with_modeset_ctx(int (*fn)(struct intel_encoder *encoder,
+					    struct drm_modeset_acquire_ctx *ctx),
+				  struct intel_encoder *encoder)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
-	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
-	struct intel_dp *intel_dp = &dig_port->dp;
-	enum phy phy = intel_port_to_phy(i915, encoder->port);
-	bool is_tc = intel_phy_is_tc(i915, phy);
 	struct drm_modeset_acquire_ctx ctx;
-	enum intel_hotplug_state state;
 	int ret;
 
-	if (intel_dp->compliance.test_active &&
-	    intel_dp->compliance.test_type == DP_TEST_LINK_PHY_TEST_PATTERN) {
-		intel_dp_phy_test(encoder);
-		/* just do the PHY test and nothing else */
-		return INTEL_HOTPLUG_UNCHANGED;
-	}
-
-	state = intel_encoder_hotplug(encoder, connector);
-
 	drm_modeset_acquire_init(&ctx, 0);
 
 	for (;;) {
-		if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA)
-			ret = intel_hdmi_reset_link(encoder, &ctx);
-		else
-			ret = intel_dp_retrain_link(encoder, &ctx);
+		ret = fn(encoder, &ctx);
 
 		if (ret == -EDEADLK) {
 			drm_modeset_backoff(&ctx);
@@ -4410,8 +4393,34 @@ intel_ddi_hotplug(struct intel_encoder *encoder,
 
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
-	drm_WARN(encoder->base.dev, ret,
+	drm_WARN(&i915->drm, ret,
 		 "Acquiring modeset locks failed with %i\n", ret);
+}
+
+static enum intel_hotplug_state
+intel_ddi_hotplug(struct intel_encoder *encoder,
+		  struct intel_connector *connector)
+{
+	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
+	struct intel_dp *intel_dp = &dig_port->dp;
+	enum phy phy = intel_port_to_phy(i915, encoder->port);
+	bool is_tc = intel_phy_is_tc(i915, phy);
+	enum intel_hotplug_state state;
+
+	if (intel_dp->compliance.test_active &&
+	    intel_dp->compliance.test_type == DP_TEST_LINK_PHY_TEST_PATTERN) {
+		intel_dp_phy_test(encoder);
+		/* just do the PHY test and nothing else */
+		return INTEL_HOTPLUG_UNCHANGED;
+	}
+
+	state = intel_encoder_hotplug(encoder, connector);
+
+	if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA)
+		call_with_modeset_ctx(intel_hdmi_reset_link, encoder);
+	else
+		call_with_modeset_ctx(intel_dp_retrain_link, encoder);
 
 	/*
 	 * Unpowered type-c dongles can take some time to boot and be
-- 
2.37.2

Re: [Intel-gfx] [PATCH 10/11] drm/i915: Factor out call_with_modeset_ctx()

[Ville Syrjälä]

On Wed, Apr 26, 2023 at 07:53:04PM +0300, Imre Deak wrote:
> Factor out a helper to call a function with the atomic locks held,
> required by a follow-up patch resetting an active DP link.
> 
> No functional changes.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 53 ++++++++++++++----------
>  1 file changed, 31 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 29e4bfab46358..0c8bc32f293b0 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4370,35 +4370,18 @@ static int intel_hdmi_reset_link(struct intel_encoder *encoder,
>  	return modeset_pipe(&crtc->base, ctx);
>  }
>  
> -static enum intel_hotplug_state
> -intel_ddi_hotplug(struct intel_encoder *encoder,
> -		  struct intel_connector *connector)
> +static void call_with_modeset_ctx(int (*fn)(struct intel_encoder *encoder,
> +					    struct drm_modeset_acquire_ctx *ctx),
> +				  struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> -	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> -	struct intel_dp *intel_dp = &dig_port->dp;
> -	enum phy phy = intel_port_to_phy(i915, encoder->port);
> -	bool is_tc = intel_phy_is_tc(i915, phy);
>  	struct drm_modeset_acquire_ctx ctx;
> -	enum intel_hotplug_state state;
>  	int ret;
>  
> -	if (intel_dp->compliance.test_active &&
> -	    intel_dp->compliance.test_type == DP_TEST_LINK_PHY_TEST_PATTERN) {
> -		intel_dp_phy_test(encoder);
> -		/* just do the PHY test and nothing else */
> -		return INTEL_HOTPLUG_UNCHANGED;
> -	}
> -
> -	state = intel_encoder_hotplug(encoder, connector);
> -
>  	drm_modeset_acquire_init(&ctx, 0);
>  
>  	for (;;) {
> -		if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA)
> -			ret = intel_hdmi_reset_link(encoder, &ctx);
> -		else
> -			ret = intel_dp_retrain_link(encoder, &ctx);
> +		ret = fn(encoder, &ctx);
>  
>  		if (ret == -EDEADLK) {
>  			drm_modeset_backoff(&ctx);
> @@ -4410,8 +4393,34 @@ intel_ddi_hotplug(struct intel_encoder *encoder,
>  
>  	drm_modeset_drop_locks(&ctx);
>  	drm_modeset_acquire_fini(&ctx);
> -	drm_WARN(encoder->base.dev, ret,
> +	drm_WARN(&i915->drm, ret,
>  		 "Acquiring modeset locks failed with %i\n", ret);
> +}

Seems pretty much a less general version of
https://patchwork.freedesktop.org/series/92607/
Unfortuantely that died in the bikeshed.

Maybe we should look into doing something like that
just for i915 initially...

> +
> +static enum intel_hotplug_state
> +intel_ddi_hotplug(struct intel_encoder *encoder,
> +		  struct intel_connector *connector)
> +{
> +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> +	struct intel_dp *intel_dp = &dig_port->dp;
> +	enum phy phy = intel_port_to_phy(i915, encoder->port);
> +	bool is_tc = intel_phy_is_tc(i915, phy);
> +	enum intel_hotplug_state state;
> +
> +	if (intel_dp->compliance.test_active &&
> +	    intel_dp->compliance.test_type == DP_TEST_LINK_PHY_TEST_PATTERN) {
> +		intel_dp_phy_test(encoder);
> +		/* just do the PHY test and nothing else */
> +		return INTEL_HOTPLUG_UNCHANGED;
> +	}
> +
> +	state = intel_encoder_hotplug(encoder, connector);
> +
> +	if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA)
> +		call_with_modeset_ctx(intel_hdmi_reset_link, encoder);
> +	else
> +		call_with_modeset_ctx(intel_dp_retrain_link, encoder);
>  
>  	/*
>  	 * Unpowered type-c dongles can take some time to boot and be
> -- 
> 2.37.2

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 10/11] drm/i915: Factor out call_with_modeset_ctx()

[Imre Deak]

On Fri, Apr 28, 2023 at 05:32:44PM +0300, Ville Syrjälä wrote:
> On Wed, Apr 26, 2023 at 07:53:04PM +0300, Imre Deak wrote:
> > Factor out a helper to call a function with the atomic locks held,
> > required by a follow-up patch resetting an active DP link.
> > 
> > No functional changes.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c | 53 ++++++++++++++----------
> >  1 file changed, 31 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 29e4bfab46358..0c8bc32f293b0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -4370,35 +4370,18 @@ static int intel_hdmi_reset_link(struct intel_encoder *encoder,
> >  	return modeset_pipe(&crtc->base, ctx);
> >  }
> >  
> > -static enum intel_hotplug_state
> > -intel_ddi_hotplug(struct intel_encoder *encoder,
> > -		  struct intel_connector *connector)
> > +static void call_with_modeset_ctx(int (*fn)(struct intel_encoder *encoder,
> > +					    struct drm_modeset_acquire_ctx *ctx),
> > +				  struct intel_encoder *encoder)
> >  {
> >  	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > -	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> > -	struct intel_dp *intel_dp = &dig_port->dp;
> > -	enum phy phy = intel_port_to_phy(i915, encoder->port);
> > -	bool is_tc = intel_phy_is_tc(i915, phy);
> >  	struct drm_modeset_acquire_ctx ctx;
> > -	enum intel_hotplug_state state;
> >  	int ret;
> >  
> > -	if (intel_dp->compliance.test_active &&
> > -	    intel_dp->compliance.test_type == DP_TEST_LINK_PHY_TEST_PATTERN) {
> > -		intel_dp_phy_test(encoder);
> > -		/* just do the PHY test and nothing else */
> > -		return INTEL_HOTPLUG_UNCHANGED;
> > -	}
> > -
> > -	state = intel_encoder_hotplug(encoder, connector);
> > -
> >  	drm_modeset_acquire_init(&ctx, 0);
> >  
> >  	for (;;) {
> > -		if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA)
> > -			ret = intel_hdmi_reset_link(encoder, &ctx);
> > -		else
> > -			ret = intel_dp_retrain_link(encoder, &ctx);
> > +		ret = fn(encoder, &ctx);
> >  
> >  		if (ret == -EDEADLK) {
> >  			drm_modeset_backoff(&ctx);
> > @@ -4410,8 +4393,34 @@ intel_ddi_hotplug(struct intel_encoder *encoder,
> >  
> >  	drm_modeset_drop_locks(&ctx);
> >  	drm_modeset_acquire_fini(&ctx);
> > -	drm_WARN(encoder->base.dev, ret,
> > +	drm_WARN(&i915->drm, ret,
> >  		 "Acquiring modeset locks failed with %i\n", ret);
> > +}
> 
> Seems pretty much a less general version of
> https://patchwork.freedesktop.org/series/92607/
> Unfortuantely that died in the bikeshed.
> 
> Maybe we should look into doing something like that
> just for i915 initially...

Yes, looks better, can adopt an i915 version of that unless you want to
do that.

> 
> > +
> > +static enum intel_hotplug_state
> > +intel_ddi_hotplug(struct intel_encoder *encoder,
> > +		  struct intel_connector *connector)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > +	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> > +	struct intel_dp *intel_dp = &dig_port->dp;
> > +	enum phy phy = intel_port_to_phy(i915, encoder->port);
> > +	bool is_tc = intel_phy_is_tc(i915, phy);
> > +	enum intel_hotplug_state state;
> > +
> > +	if (intel_dp->compliance.test_active &&
> > +	    intel_dp->compliance.test_type == DP_TEST_LINK_PHY_TEST_PATTERN) {
> > +		intel_dp_phy_test(encoder);
> > +		/* just do the PHY test and nothing else */
> > +		return INTEL_HOTPLUG_UNCHANGED;
> > +	}
> > +
> > +	state = intel_encoder_hotplug(encoder, connector);
> > +
> > +	if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA)
> > +		call_with_modeset_ctx(intel_hdmi_reset_link, encoder);
> > +	else
> > +		call_with_modeset_ctx(intel_dp_retrain_link, encoder);
> >  
> >  	/*
> >  	 * Unpowered type-c dongles can take some time to boot and be
> > -- 
> > 2.37.2
> 
> -- 
> Ville Syrjälä
> Intel

[Intel-gfx] [PATCH 11/11] drm/i915/tc: Reset TypeC PHYs left enabled in DP-alt mode after the sink disconnects

[Imre Deak]

If the output on a DP-alt link 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.

To account for a race condition during driver loading where the sink is
disconnected after the above sanitization step and the HPD interrupts
getting 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.

Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5860
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c      | 53 +++++++++++++++++--
 .../drm/i915/display/intel_display_types.h    |  2 +
 drivers/gpu/drm/i915/display/intel_dp.c       | 49 +++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dp.h       |  2 +
 .../drm/i915/display/intel_modeset_setup.c    | 34 +++++++-----
 drivers/gpu/drm/i915/display/intel_tc.c       | 21 ++++++++
 drivers/gpu/drm/i915/display/intel_tc.h       |  1 +
 7 files changed, 145 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 0c8bc32f293b0..a3aaedc97d3eb 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3312,6 +3312,8 @@ static void intel_disable_ddi(struct intel_atomic_state *state,
 			      const struct intel_crtc_state *old_crtc_state,
 			      const struct drm_connector_state *old_conn_state)
 {
+	cancel_delayed_work(&enc_to_dig_port(encoder)->reset_link_work);
+
 	intel_hdcp_disable(to_intel_connector(old_conn_state->connector));
 
 	if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_HDMI))
@@ -4220,9 +4222,29 @@ static void intel_ddi_encoder_reset(struct drm_encoder *encoder)
 		intel_tc_port_init_mode(dig_port);
 }
 
+static bool intel_ddi_tc_port_reset_link(struct intel_encoder *encoder)
+{
+	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
+
+	if (!intel_tc_port_link_needs_reset(dig_port))
+		return false;
+
+	queue_delayed_work(system_unbound_wq, &dig_port->reset_link_work, msecs_to_jiffies(2000));
+
+	return true;
+}
+
+static int intel_ddi_encoder_late_register(struct drm_encoder *_encoder)
+{
+	intel_ddi_tc_port_reset_link(to_intel_encoder(_encoder));
+
+	return 0;
+}
+
 static const struct drm_encoder_funcs intel_ddi_funcs = {
 	.reset = intel_ddi_encoder_reset,
 	.destroy = intel_ddi_encoder_destroy,
+	.late_register = intel_ddi_encoder_late_register,
 };
 
 static struct intel_connector *
@@ -4397,6 +4419,25 @@ static void call_with_modeset_ctx(int (*fn)(struct intel_encoder *encoder,
 		 "Acquiring modeset locks failed with %i\n", ret);
 }
 
+static void intel_ddi_tc_link_reset_work(struct work_struct *work)
+{
+	struct intel_digital_port *dig_port =
+		container_of(work, struct intel_digital_port, reset_link_work.work);
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+	struct intel_encoder *encoder = &dig_port->base;
+
+	mutex_lock(&i915->drm.mode_config.mutex);
+
+	if (intel_tc_port_link_needs_reset(dig_port)) {
+		drm_dbg_kms(&i915->drm,
+			    "[ENCODER:%d:%s] TypeC DP-alt sink disconnected, resetting link\n",
+			    encoder->base.base.id, encoder->base.name);
+		call_with_modeset_ctx(intel_dp_reset_link, &dig_port->base);
+	}
+
+	mutex_unlock(&i915->drm.mode_config.mutex);
+}
+
 static enum intel_hotplug_state
 intel_ddi_hotplug(struct intel_encoder *encoder,
 		  struct intel_connector *connector)
@@ -4417,10 +4458,12 @@ intel_ddi_hotplug(struct intel_encoder *encoder,
 
 	state = intel_encoder_hotplug(encoder, connector);
 
-	if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA)
-		call_with_modeset_ctx(intel_hdmi_reset_link, encoder);
-	else
-		call_with_modeset_ctx(intel_dp_retrain_link, encoder);
+	if (!intel_ddi_tc_port_reset_link(encoder)) {
+		if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA)
+			call_with_modeset_ctx(intel_hdmi_reset_link, encoder);
+		else
+			call_with_modeset_ctx(intel_dp_retrain_link, encoder);
+	}
 
 	/*
 	 * Unpowered type-c dongles can take some time to boot and be
@@ -4956,6 +4999,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 			encoder->pipe_mask = intel_ddi_splitter_pipe_mask(dev_priv);
 	}
 
+	INIT_DELAYED_WORK(&dig_port->reset_link_work, intel_ddi_tc_link_reset_work);
+
 	/*
 	 * In theory we don't need the encoder->type check,
 	 * but leave it just in case we have some really bad VBTs...
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 35c260bd14618..c026940dbeae0 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1803,6 +1803,8 @@ struct intel_digital_port {
 
 	struct intel_tc_port *tc;
 
+	struct delayed_work reset_link_work;
+
 	/* protects num_hdcp_streams reference count, hdcp_port_data and hdcp_auth_status */
 	struct mutex hdcp_mutex;
 	/* the number of pipes using HDCP signalling out of this port */
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 1e91175506f5d..24d0b15d0e925 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4250,6 +4250,55 @@ int intel_dp_retrain_link(struct intel_encoder *encoder,
 	return 0;
 }
 
+int intel_dp_reset_link(struct intel_encoder *encoder,
+			struct drm_modeset_acquire_ctx *ctx)
+{
+	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+	struct drm_atomic_state *_state;
+	struct intel_atomic_state *state;
+	struct intel_crtc *crtc;
+	u8 pipe_mask;
+	int ret = 0;
+
+	ret = drm_modeset_lock(&i915->drm.mode_config.connection_mutex,
+			       ctx);
+	if (ret)
+		return ret;
+
+	ret = intel_dp_get_active_pipes(enc_to_intel_dp(encoder), ctx, &pipe_mask);
+	if (ret)
+		return ret;
+
+	if (!pipe_mask)
+		return 0;
+
+	_state = drm_atomic_state_alloc(&i915->drm);
+	if (!_state)
+		return -ENOMEM;
+	state = to_intel_atomic_state(_state);
+
+	state->base.acquire_ctx = ctx;
+	state->internal = true;
+
+	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask) {
+		struct intel_crtc_state *crtc_state;
+
+		crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
+		if (IS_ERR(crtc_state)) {
+			ret = PTR_ERR(crtc_state);
+			break;
+		}
+
+		crtc_state->uapi.connectors_changed = true;
+	}
+
+	ret = drm_atomic_commit(&state->base);
+
+	drm_atomic_state_put(&state->base);
+
+	return ret;
+}
+
 static int intel_dp_prep_phy_test(struct intel_dp *intel_dp,
 				  struct drm_modeset_acquire_ctx *ctx,
 				  u8 *pipe_mask)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index ca12a1733df6f..02fe28544e775 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -45,6 +45,8 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
 bool intel_dp_is_connected(struct intel_dp *intel_dp);
 int intel_dp_retrain_link(struct intel_encoder *encoder,
 			  struct drm_modeset_acquire_ctx *ctx);
+int intel_dp_reset_link(struct intel_encoder *encoder,
+			struct drm_modeset_acquire_ctx *ctx);
 void intel_dp_set_power(struct intel_dp *intel_dp, u8 mode);
 void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
 					   const struct intel_crtc_state *crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
index f613c074187a2..16120011437f6 100644
--- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
+++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
@@ -26,6 +26,7 @@
 #include "intel_fifo_underrun.h"
 #include "intel_modeset_setup.h"
 #include "intel_pch_display.h"
+#include "intel_tc.h"
 #include "intel_vblank.h"
 #include "intel_wm.h"
 #include "skl_watermark.h"
@@ -253,17 +254,6 @@ intel_sanitize_plane_mapping(struct drm_i915_private *i915)
 	}
 }
 
-static bool intel_crtc_has_encoders(struct intel_crtc *crtc)
-{
-	struct drm_device *dev = crtc->base.dev;
-	struct intel_encoder *encoder;
-
-	for_each_encoder_on_crtc(dev, &crtc->base, encoder)
-		return true;
-
-	return false;
-}
-
 static struct intel_connector *intel_encoder_find_connector(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
@@ -382,6 +372,9 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
 {
 	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
 	struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state);
+	struct intel_encoder *encoder;
+	bool needs_link_reset = false;
+	bool has_encoder = false;
 
 	if (crtc_state->hw.active) {
 		struct intel_plane *plane;
@@ -401,13 +394,28 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
 		intel_color_commit_arm(crtc_state);
 	}
 
+	if (!crtc_state->hw.active ||
+	    intel_crtc_is_bigjoiner_slave(crtc_state))
+		return;
+
+	for_each_encoder_on_crtc(&i915->drm, &crtc->base, encoder) {
+		struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
+
+		has_encoder = true;
+
+		if (!dig_port || !intel_tc_port_link_needs_reset(dig_port))
+			continue;
+
+		needs_link_reset = true;
+		break;
+	}
+
 	/*
 	 * Adjust the state of the output pipe according to whether we have
 	 * active connectors/encoders.
 	 * TODO: Add support for MST
 	 */
-	if (crtc_state->hw.active && !intel_crtc_has_encoders(crtc) &&
-	    !intel_crtc_is_bigjoiner_slave(crtc_state))
+	if (!has_encoder || needs_link_reset)
 		disable_crtc_with_slaves(crtc, ctx);
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index 3b60995e9dfb3..358058c7bb464 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -1335,6 +1335,27 @@ bool intel_tc_port_connected(struct intel_encoder *encoder)
 	return is_connected;
 }
 
+bool intel_tc_port_link_needs_reset(struct intel_digital_port *dig_port)
+{
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
+	struct intel_tc_port *tc = to_tc_port(dig_port);
+	bool ret;
+
+	if (!intel_phy_is_tc(i915, phy))
+		return false;
+
+	mutex_lock(&tc->lock);
+
+	ret = tc->link_refcount &&
+	      intel_tc_port_in_dp_alt_mode(dig_port) &&
+	      intel_tc_port_needs_reset(tc);
+
+	mutex_unlock(&tc->lock);
+
+	return ret;
+}
+
 static void __intel_tc_port_lock(struct intel_tc_port *tc,
 				 int required_lanes)
 {
diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
index dd0810f9ea95e..c4cf1eac54a1c 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.h
+++ b/drivers/gpu/drm/i915/display/intel_tc.h
@@ -34,6 +34,7 @@ void intel_tc_port_flush_work(struct intel_digital_port *dig_port);
 void intel_tc_port_get_link(struct intel_digital_port *dig_port,
 			    int required_lanes);
 void intel_tc_port_put_link(struct intel_digital_port *dig_port);
+bool intel_tc_port_link_needs_reset(struct intel_digital_port *dig_port);
 bool intel_tc_port_ref_held(struct intel_digital_port *dig_port);
 
 int intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy);
-- 
2.37.2

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/tc: Add a workaround for an IOM/TCSS firmware hang issue

[Patchwork]

== Series Details ==

Series: drm/i915/tc: Add a workaround for an IOM/TCSS firmware hang issue
URL   : https://patchwork.freedesktop.org/series/117004/
State : warning

== Summary ==

Error: dim checkpatch failed
95dbb1a42869 drm/i915: Fix PIPEDMC disabling for a bigjoiner configuration
4e1ea394fc72 drm/i915: Make the CRTC wrt. CSC state consistent during sanitize-disabling
d2f3a0d89e2d drm/i915: Update connector atomic state before crtc sanitize-disabling
8ce977b98127 drm/i915: Factor out set_encoder_for_connector()
ba8756ab835a drm/i915: Add support for disabling any CRTCs during HW readout/sanitization
34ddb0bf44b7 drm/i915/dp: Add link training debug and error printing helpers
-:22: WARNING:LONG_LINE: line length of 104 exceeds 100 columns
#22: FILE: drivers/gpu/drm/i915/display/intel_dp_link_training.c:30:
+static void lt_msg(struct intel_connector *connector, struct intel_dp *intel_dp, enum drm_dp_phy dp_phy,

total: 0 errors, 1 warnings, 0 checks, 755 lines checked
e604c2ad4052 drm/i915/dp: Convert link training error to debug message on disconnected sink
0742b270148a drm/i915/dp: Prevent link training fallback on disconnected port
a41dfebc9a94 drm/i915/dp: Factor out intel_dp_get_active_pipes()
47791942c557 drm/i915: Factor out call_with_modeset_ctx()
6d9571d811d4 drm/i915/tc: Reset TypeC PHYs left enabled in DP-alt mode after the sink disconnects
-:28: WARNING:COMMIT_LOG_USE_LINK: Unknown link reference 'Closes:', use 'Link:' instead
#28: 
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5860

total: 0 errors, 1 warnings, 0 checks, 254 lines checked

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/tc: Add a workaround for an IOM/TCSS firmware hang issue

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 7135 bytes --]

== Series Details ==

Series: drm/i915/tc: Add a workaround for an IOM/TCSS firmware hang issue
URL   : https://patchwork.freedesktop.org/series/117004/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13064 -> Patchwork_117004v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/index.html

Participating hosts (41 -> 40)
------------------------------

  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_117004v1:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_selftest@live@hugepages:
    - {bat-mtlp-8}:       NOTRUN -> [FAIL][1] +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/bat-mtlp-8/igt@i915_selftest@live@hugepages.html

  
Known issues
------------

  Here are the changes found in Patchwork_117004v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@hangcheck:
    - bat-dg2-11:         [PASS][2] -> [ABORT][3] ([i915#7913])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/bat-dg2-11/igt@i915_selftest@live@hangcheck.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/bat-dg2-11/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@requests:
    - bat-rplp-1:         [PASS][4] -> [ABORT][5] ([i915#7913] / [i915#7920])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/bat-rplp-1/igt@i915_selftest@live@requests.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/bat-rplp-1/igt@i915_selftest@live@requests.html
    - bat-adlp-6:         [PASS][6] -> [ABORT][7] ([i915#7913])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/bat-adlp-6/igt@i915_selftest@live@requests.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/bat-adlp-6/igt@i915_selftest@live@requests.html

  * igt@i915_selftest@live@reset:
    - bat-rpls-2:         NOTRUN -> [ABORT][8] ([i915#7913])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/bat-rpls-2/igt@i915_selftest@live@reset.html

  * igt@kms_chamelium_hpd@common-hpd-after-suspend:
    - bat-rpls-1:         NOTRUN -> [SKIP][9] ([i915#7828])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/bat-rpls-1/igt@kms_chamelium_hpd@common-hpd-after-suspend.html

  * igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1:
    - bat-dg2-8:          [PASS][10] -> [FAIL][11] ([i915#7932])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1.html

  * igt@kms_pipe_crc_basic@suspend-read-crc:
    - bat-rpls-1:         NOTRUN -> [SKIP][12] ([i915#1845])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/bat-rpls-1/igt@kms_pipe_crc_basic@suspend-read-crc.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3@smem:
    - bat-rpls-1:         [ABORT][13] -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/bat-rpls-1/igt@gem_exec_suspend@basic-s3@smem.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/bat-rpls-1/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@i915_selftest@live@requests:
    - {bat-mtlp-8}:       [ABORT][15] ([i915#7920]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/bat-mtlp-8/igt@i915_selftest@live@requests.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/bat-mtlp-8/igt@i915_selftest@live@requests.html
    - bat-rpls-2:         [ABORT][17] ([i915#7913]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/bat-rpls-2/igt@i915_selftest@live@requests.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/bat-rpls-2/igt@i915_selftest@live@requests.html

  * igt@i915_selftest@live@slpc:
    - bat-rpls-1:         [FAIL][19] ([i915#6997]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/bat-rpls-1/igt@i915_selftest@live@slpc.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/bat-rpls-1/igt@i915_selftest@live@slpc.html

  
#### Warnings ####

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-tgl-1115g4:      [INCOMPLETE][21] ([i915#8102]) -> [INCOMPLETE][22] ([i915#7443] / [i915#8102])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/fi-tgl-1115g4/igt@i915_suspend@basic-s3-without-i915.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/fi-tgl-1115g4/igt@i915_suspend@basic-s3-without-i915.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
  [i915#6997]: https://gitlab.freedesktop.org/drm/intel/issues/6997
  [i915#7443]: https://gitlab.freedesktop.org/drm/intel/issues/7443
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7920]: https://gitlab.freedesktop.org/drm/intel/issues/7920
  [i915#7932]: https://gitlab.freedesktop.org/drm/intel/issues/7932
  [i915#8102]: https://gitlab.freedesktop.org/drm/intel/issues/8102


Build changes
-------------

  * Linux: CI_DRM_13064 -> Patchwork_117004v1

  CI-20190529: 20190529
  CI_DRM_13064: c76d02017c7f00e9ad94af7cb800ab20ded91e68 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7271: d12d7eb92226e14745a80e6bdd95384913a43548 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_117004v1: c76d02017c7f00e9ad94af7cb800ab20ded91e68 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

b4a832d84a1e drm/i915/tc: Reset TypeC PHYs left enabled in DP-alt mode after the sink disconnects
feff97da9aaf drm/i915: Factor out call_with_modeset_ctx()
f4410cf11cbb drm/i915/dp: Factor out intel_dp_get_active_pipes()
a994c1e05006 drm/i915/dp: Prevent link training fallback on disconnected port
e9be91316564 drm/i915/dp: Convert link training error to debug message on disconnected sink
4c06b55c776a drm/i915/dp: Add link training debug and error printing helpers
30a315502f38 drm/i915: Add support for disabling any CRTCs during HW readout/sanitization
2ae346df1d02 drm/i915: Factor out set_encoder_for_connector()
02a01f8c0465 drm/i915: Update connector atomic state before crtc sanitize-disabling
6152ffc6bd7b drm/i915: Make the CRTC wrt. CSC state consistent during sanitize-disabling
c8724a8ccc8c drm/i915: Fix PIPEDMC disabling for a bigjoiner configuration

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/index.html

[-- Attachment #2: Type: text/html, Size: 8454 bytes --]

[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/tc: Add a workaround for an IOM/TCSS firmware hang issue

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 13693 bytes --]

== Series Details ==

Series: drm/i915/tc: Add a workaround for an IOM/TCSS firmware hang issue
URL   : https://patchwork.freedesktop.org/series/117004/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13064_full -> Patchwork_117004v1_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_117004v1_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_117004v1_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_117004v1_full:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@gt_heartbeat:
    - shard-apl:          [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/shard-apl2/igt@i915_selftest@live@gt_heartbeat.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-apl2/igt@i915_selftest@live@gt_heartbeat.html

  
Known issues
------------

  Here are the changes found in Patchwork_117004v1_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fair@basic-none-rrul@rcs0:
    - shard-glk:          NOTRUN -> [FAIL][3] ([i915#2842])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-glk9/igt@gem_exec_fair@basic-none-rrul@rcs0.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-glk:          [PASS][4] -> [FAIL][5] ([i915#2842]) +1 similar issue
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/shard-glk4/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-glk3/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_exec_fair@basic-pace-solo@rcs0:
    - shard-apl:          NOTRUN -> [FAIL][6] ([i915#2842])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-apl4/igt@gem_exec_fair@basic-pace-solo@rcs0.html

  * igt@gem_huc_copy@huc-copy:
    - shard-glk:          NOTRUN -> [SKIP][7] ([fdo#109271] / [i915#2190])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-glk8/igt@gem_huc_copy@huc-copy.html

  * igt@gem_ppgtt@blt-vs-render-ctxn:
    - shard-snb:          [PASS][8] -> [FAIL][9] ([i915#4998] / [i915#8295])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/shard-snb5/igt@gem_ppgtt@blt-vs-render-ctxn.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-snb6/igt@gem_ppgtt@blt-vs-render-ctxn.html

  * igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_mc_ccs:
    - shard-apl:          NOTRUN -> [SKIP][10] ([fdo#109271] / [i915#3886])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-apl4/igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-b-missing-ccs-buffer-y_tiled_gen12_mc_ccs:
    - shard-glk:          NOTRUN -> [SKIP][11] ([fdo#109271] / [i915#3886]) +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-glk8/igt@kms_ccs@pipe-b-missing-ccs-buffer-y_tiled_gen12_mc_ccs.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-glk:          NOTRUN -> [FAIL][12] ([i915#4767])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-glk9/igt@kms_fbcon_fbt@fbc-suspend.html

  * igt@kms_plane_scaling@planes-downscale-factor-0-5-unity-scaling@pipe-b-vga-1:
    - shard-snb:          NOTRUN -> [SKIP][13] ([fdo#109271]) +45 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-snb5/igt@kms_plane_scaling@planes-downscale-factor-0-5-unity-scaling@pipe-b-vga-1.html

  * igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-sf:
    - shard-glk:          NOTRUN -> [SKIP][14] ([fdo#109271] / [i915#658]) +2 similar issues
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-glk8/igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-sf.html

  * igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-sf:
    - shard-apl:          NOTRUN -> [SKIP][15] ([fdo#109271] / [i915#658])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-apl4/igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-sf.html

  * igt@kms_setmode@basic@pipe-a-vga-1:
    - shard-snb:          NOTRUN -> [FAIL][16] ([i915#5465]) +1 similar issue
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-snb4/igt@kms_setmode@basic@pipe-a-vga-1.html

  * igt@v3d/v3d_submit_csd@multi-and-single-sync:
    - shard-apl:          NOTRUN -> [SKIP][17] ([fdo#109271]) +36 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-apl4/igt@v3d/v3d_submit_csd@multi-and-single-sync.html

  * igt@vc4/vc4_perfmon@create-perfmon-exceed:
    - shard-glk:          NOTRUN -> [SKIP][18] ([fdo#109271]) +57 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-glk8/igt@vc4/vc4_perfmon@create-perfmon-exceed.html

  
#### Possible fixes ####

  * igt@gem_barrier_race@remote-request@rcs0:
    - {shard-rkl}:        [ABORT][19] ([i915#7461]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/shard-rkl-3/igt@gem_barrier_race@remote-request@rcs0.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-rkl-3/igt@gem_barrier_race@remote-request@rcs0.html

  * igt@gem_ctx_exec@basic-nohangcheck:
    - {shard-rkl}:        [FAIL][21] ([i915#6268]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/shard-rkl-1/igt@gem_ctx_exec@basic-nohangcheck.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-rkl-6/igt@gem_ctx_exec@basic-nohangcheck.html

  * igt@gem_exec_fair@basic-none@vecs0:
    - {shard-rkl}:        [FAIL][23] ([i915#2842]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/shard-rkl-7/igt@gem_exec_fair@basic-none@vecs0.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-rkl-6/igt@gem_exec_fair@basic-none@vecs0.html

  * igt@gem_exec_fair@basic-pace@rcs0:
    - shard-glk:          [FAIL][25] ([i915#2842]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/shard-glk6/igt@gem_exec_fair@basic-pace@rcs0.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-glk8/igt@gem_exec_fair@basic-pace@rcs0.html

  * igt@gem_lmem_swapping@smem-oom@lmem0:
    - {shard-dg1}:        [TIMEOUT][27] -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/shard-dg1-16/igt@gem_lmem_swapping@smem-oom@lmem0.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-dg1-14/igt@gem_lmem_swapping@smem-oom@lmem0.html

  * igt@i915_pm_rpm@dpms-mode-unset-lpsp:
    - {shard-rkl}:        [SKIP][29] ([i915#1397]) -> [PASS][30] +3 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/shard-rkl-6/igt@i915_pm_rpm@dpms-mode-unset-lpsp.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-rkl-7/igt@i915_pm_rpm@dpms-mode-unset-lpsp.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip-async-flip:
    - {shard-rkl}:        [FAIL][31] ([i915#3743]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/shard-rkl-7/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip-async-flip.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-rkl-4/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip-async-flip.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-glk:          [FAIL][33] ([i915#2346]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/shard-glk2/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-glk5/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_cursor_legacy@single-bo@pipe-b:
    - {shard-rkl}:        [INCOMPLETE][35] ([i915#8011]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/shard-rkl-7/igt@kms_cursor_legacy@single-bo@pipe-b.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-rkl-6/igt@kms_cursor_legacy@single-bo@pipe-b.html

  * igt@kms_flip@flip-vs-suspend@a-dp1:
    - shard-apl:          [ABORT][37] -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/shard-apl3/igt@kms_flip@flip-vs-suspend@a-dp1.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-apl4/igt@kms_flip@flip-vs-suspend@a-dp1.html

  * igt@kms_plane_scaling@i915-max-src-size@pipe-a-hdmi-a-2:
    - {shard-rkl}:        [FAIL][39] ([i915#8292]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/shard-rkl-1/igt@kms_plane_scaling@i915-max-src-size@pipe-a-hdmi-a-2.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-rkl-4/igt@kms_plane_scaling@i915-max-src-size@pipe-a-hdmi-a-2.html

  * igt@perf@oa-exponents@0-rcs0:
    - shard-glk:          [ABORT][41] -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/shard-glk8/igt@perf@oa-exponents@0-rcs0.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-glk9/igt@perf@oa-exponents@0-rcs0.html

  * igt@perf@stress-open-close@0-rcs0:
    - shard-glk:          [ABORT][43] ([i915#7941]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13064/shard-glk3/igt@perf@stress-open-close@0-rcs0.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/shard-glk8/igt@perf@stress-open-close@0-rcs0.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
  [i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#3023]: https://gitlab.freedesktop.org/drm/intel/issues/3023
  [i915#315]: https://gitlab.freedesktop.org/drm/intel/issues/315
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
  [i915#3743]: https://gitlab.freedesktop.org/drm/intel/issues/3743
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4767]: https://gitlab.freedesktop.org/drm/intel/issues/4767
  [i915#4816]: https://gitlab.freedesktop.org/drm/intel/issues/4816
  [i915#4998]: https://gitlab.freedesktop.org/drm/intel/issues/4998
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5461]: https://gitlab.freedesktop.org/drm/intel/issues/5461
  [i915#5465]: https://gitlab.freedesktop.org/drm/intel/issues/5465
  [i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
  [i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
  [i915#7941]: https://gitlab.freedesktop.org/drm/intel/issues/7941
  [i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
  [i915#8292]: https://gitlab.freedesktop.org/drm/intel/issues/8292
  [i915#8295]: https://gitlab.freedesktop.org/drm/intel/issues/8295


Build changes
-------------

  * Linux: CI_DRM_13064 -> Patchwork_117004v1

  CI-20190529: 20190529
  CI_DRM_13064: c76d02017c7f00e9ad94af7cb800ab20ded91e68 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7271: d12d7eb92226e14745a80e6bdd95384913a43548 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_117004v1: c76d02017c7f00e9ad94af7cb800ab20ded91e68 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117004v1/index.html

[-- Attachment #2: Type: text/html, Size: 14118 bytes --]