diff mbox series

[3/3] drm/915/lspcon: Reduce dmesg errors during lspcon_init failure

Message ID 20240408050558.865396-4-ankit.k.nautiyal@intel.com (mailing list archive)
State New, archived
Headers show
Series Avoid unwanted lspcon init and probe warnings | expand

Commit Message

Nautiyal, Ankit K April 8, 2024, 5:05 a.m. UTC
Currently lspcon_resume calls lspcon_init and in case of failure we get
error messages from lspcon_init and then again from lspcon_resume.

Just have a single error message in lspcon_init and convert all other
errors as dbg messages.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_lspcon.c | 27 +++++++++++----------
 1 file changed, 14 insertions(+), 13 deletions(-)

Comments

Jani Nikula April 8, 2024, 9:52 a.m. UTC | #1
On Mon, 08 Apr 2024, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
> Currently lspcon_resume calls lspcon_init and in case of failure we get
> error messages from lspcon_init and then again from lspcon_resume.
>
> Just have a single error message in lspcon_init and convert all other
> errors as dbg messages.
>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_lspcon.c | 27 +++++++++++----------
>  1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c b/drivers/gpu/drm/i915/display/intel_lspcon.c
> index 16ee0dc179f7..3c3bc80e32f0 100644
> --- a/drivers/gpu/drm/i915/display/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
> @@ -680,24 +680,30 @@ bool lspcon_init(struct intel_digital_port *dig_port)
>  		return false;
>  
>  	if (!lspcon_set_pcon_mode(lspcon)) {
> -		drm_err(&i915->drm, "LSPCON mode change to PCON failed\n");
> -		return false;
> +		drm_dbg_kms(&i915->drm, "LSPCON mode change to PCON failed\n");
> +		goto lspcon_init_failed;
>  	}
>  
>  	if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd) != 0) {
> -		drm_err(&i915->drm, "LSPCON DPCD read failed\n");
> -		return false;
> +		drm_dbg_kms(&i915->drm, "LSPCON DPCD read failed\n");
> +		goto lspcon_init_failed;
>  	}
>  
>  	if (!lspcon_detect_vendor(lspcon)) {
> -		drm_err(&i915->drm, "LSPCON vendor detection failed\n");
> -		return false;
> +		drm_dbg_kms(&i915->drm, "LSPCON vendor detection failed\n");
> +		goto lspcon_init_failed;

Why not just keep all of the above as drm_err(), adding all the relevant
info there...

>  	}
>  
>  	connector->ycbcr_420_allowed = true;
>  	lspcon->active = true;
>  	drm_dbg_kms(&i915->drm, "Success: LSPCON init\n");
>  	return true;
> +
> +lspcon_init_failed:
> +	drm_err(&i915->drm, "LSPCON init failed on port %c\n",
> +		port_name(dig_port->base.port));
> +
> +	return false;

And dropping this change altogheter? Why would we need both the debug
message and the error? Just have one error message?

With that change,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>  }
>  
>  u32 intel_lspcon_infoframes_enabled(struct intel_encoder *encoder,
> @@ -718,13 +724,8 @@ void lspcon_resume(struct intel_digital_port *dig_port)
>  	if (!intel_bios_encoder_is_lspcon(dig_port->base.devdata))
>  		return;
>  
> -	if (!lspcon->active) {
> -		if (!lspcon_init(dig_port)) {
> -			drm_err(&i915->drm, "LSPCON init failed on port %c\n",
> -				port_name(dig_port->base.port));
> -			return;
> -		}
> -	}
> +	if (!lspcon->active && !lspcon_init(dig_port))
> +		return;
>  
>  	expected_mode = lspcon_get_expected_mode(lspcon);
>  	if (expected_mode == DRM_LSPCON_MODE_PCON)
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c b/drivers/gpu/drm/i915/display/intel_lspcon.c
index 16ee0dc179f7..3c3bc80e32f0 100644
--- a/drivers/gpu/drm/i915/display/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
@@ -680,24 +680,30 @@  bool lspcon_init(struct intel_digital_port *dig_port)
 		return false;
 
 	if (!lspcon_set_pcon_mode(lspcon)) {
-		drm_err(&i915->drm, "LSPCON mode change to PCON failed\n");
-		return false;
+		drm_dbg_kms(&i915->drm, "LSPCON mode change to PCON failed\n");
+		goto lspcon_init_failed;
 	}
 
 	if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd) != 0) {
-		drm_err(&i915->drm, "LSPCON DPCD read failed\n");
-		return false;
+		drm_dbg_kms(&i915->drm, "LSPCON DPCD read failed\n");
+		goto lspcon_init_failed;
 	}
 
 	if (!lspcon_detect_vendor(lspcon)) {
-		drm_err(&i915->drm, "LSPCON vendor detection failed\n");
-		return false;
+		drm_dbg_kms(&i915->drm, "LSPCON vendor detection failed\n");
+		goto lspcon_init_failed;
 	}
 
 	connector->ycbcr_420_allowed = true;
 	lspcon->active = true;
 	drm_dbg_kms(&i915->drm, "Success: LSPCON init\n");
 	return true;
+
+lspcon_init_failed:
+	drm_err(&i915->drm, "LSPCON init failed on port %c\n",
+		port_name(dig_port->base.port));
+
+	return false;
 }
 
 u32 intel_lspcon_infoframes_enabled(struct intel_encoder *encoder,
@@ -718,13 +724,8 @@  void lspcon_resume(struct intel_digital_port *dig_port)
 	if (!intel_bios_encoder_is_lspcon(dig_port->base.devdata))
 		return;
 
-	if (!lspcon->active) {
-		if (!lspcon_init(dig_port)) {
-			drm_err(&i915->drm, "LSPCON init failed on port %c\n",
-				port_name(dig_port->base.port));
-			return;
-		}
-	}
+	if (!lspcon->active && !lspcon_init(dig_port))
+		return;
 
 	expected_mode = lspcon_get_expected_mode(lspcon);
 	if (expected_mode == DRM_LSPCON_MODE_PCON)