[PATCH 2/2] drm/i915/lspcon: Separate lspcon probe and lspcon init

[Jani Nikula]

On Fri, 22 Mar 2024, Ankit Nautiyal <ankit.k.nautiyal at intel.com> wrote:
> Currently we probe for lspcon, inside lspcon init. Which does 2 things:
> probe the lspcon and set the expected LS/PCON mode.
>
> If there is no lspcon connected, the probe expectedly fails and
> results in error message. This inturn gets propogated to
> lspcon init and we get again error message for lspcon init
> failure.
>
> Separate the probe function and avoid displaying error if probe fails.
> If probe succeeds, only then start lspcon init and set the expected
> LS/PCON mode as first step.
>
> While at it move the drm_err message in lspcon init, instead of the
> caller.
>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c     |  3 +++
>  drivers/gpu/drm/i915/display/intel_lspcon.c | 27 +++++++++++----------
>  drivers/gpu/drm/i915/display/intel_lspcon.h |  1 +
>  3 files changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 94fa34f77cf0..ea8d3e70127e 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5882,6 +5882,9 @@ intel_dp_connector_register(struct drm_connector *connector)
>  	 * ToDo: Clean this up to handle lspcon init and resume more
>  	 * efficiently and streamlined.
>  	 */
> +	if (!lspcon_probe(lspcon))
> +		return ret;
> +
>  	if (lspcon_init(dig_port)) {
>  		lspcon_detect_hdr_capability(lspcon);
>  		if (lspcon->hdr_supported)
> diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c b/drivers/gpu/drm/i915/display/intel_lspcon.c
> index 62159d3ead56..570fde848d00 100644
> --- a/drivers/gpu/drm/i915/display/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
> @@ -266,7 +266,7 @@ static bool lspcon_set_expected_mode(struct intel_lspcon *lspcon)
>  	return true;
>  }
>  
> -static bool lspcon_probe(struct intel_lspcon *lspcon)
> +bool lspcon_probe(struct intel_lspcon *lspcon)
>  {
>  	int retry;
>  	enum drm_dp_dual_mode_type adaptor_type;
> @@ -676,30 +676,31 @@ bool lspcon_init(struct intel_digital_port *dig_port)
>  	lspcon->active = false;
>  	lspcon->mode = DRM_LSPCON_MODE_INVALID;
>  
> -	if (!lspcon_probe(lspcon)) {
> -		drm_err(&i915->drm, "Failed to probe lspcon\n");
> -		return false;
> -	}
> -
>  	if (!lspcon_set_expected_mode(lspcon)) {
>  		drm_err(&i915->drm, "LSPCON Set expected Mode failed\n");
> -		return false;
> +		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;
> +		goto lspcon_init_failed;
>  	}
>  
>  	if (!lspcon_detect_vendor(lspcon)) {
>  		drm_err(&i915->drm, "LSPCON vendor detection failed\n");
> -		return false;
> +		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));

I guess the idea is to reduce dmesg errors, but in this function the
error messages are multiplied.

BR,
Jani.

> +
> +	return false;
>  }
>  
>  u32 intel_lspcon_infoframes_enabled(struct intel_encoder *encoder,
> @@ -721,11 +722,11 @@ void lspcon_resume(struct intel_digital_port *dig_port)
>  		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));
> +		if (!lspcon_probe(lspcon))
> +			return;
> +
> +		if (!lspcon_init(dig_port))
>  			return;
> -		}
>  	}
>  
>  	if (lspcon_wake_native_aux_ch(lspcon)) {
> diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.h b/drivers/gpu/drm/i915/display/intel_lspcon.h
> index e19e10492b05..b156cc6b3a23 100644
> --- a/drivers/gpu/drm/i915/display/intel_lspcon.h
> +++ b/drivers/gpu/drm/i915/display/intel_lspcon.h
> @@ -16,6 +16,7 @@ struct intel_encoder;
>  struct intel_lspcon;
>  
>  bool lspcon_init(struct intel_digital_port *dig_port);
> +bool lspcon_probe(struct intel_lspcon *lspcon);
>  void lspcon_detect_hdr_capability(struct intel_lspcon *lspcon);
>  void lspcon_resume(struct intel_digital_port *dig_port);
>  void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);

-- 
Jani Nikula, Intel