diff mbox series

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

Message ID 20240322121832.4170061-3-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 March 22, 2024, 12:18 p.m. UTC
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@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(-)

Comments

Jani Nikula March 25, 2024, 3:18 p.m. UTC | #1
On Fri, 22 Mar 2024, Ankit Nautiyal <ankit.k.nautiyal@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@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);
Nautiyal, Ankit K March 26, 2024, 4:26 a.m. UTC | #2
On 3/25/2024 8:48 PM, Jani Nikula wrote:
> On Fri, 22 Mar 2024, Ankit Nautiyal <ankit.k.nautiyal@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@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.

Earlier we used to get the drm_error for init failure, even if the 
LSPCON was not detected, which is printed as a debug print.

Now we'll get the dmesg errors only if we detect lspcon and lspcon init 
indeed fails.

Regards,

Ankit


>
> 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 March 26, 2024, 6:56 a.m. UTC | #3
On Tue, 26 Mar 2024, "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com> wrote:
> On 3/25/2024 8:48 PM, Jani Nikula wrote:
>> On Fri, 22 Mar 2024, Ankit Nautiyal <ankit.k.nautiyal@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@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.
>
> Earlier we used to get the drm_error for init failure, even if the 
> LSPCON was not detected, which is printed as a debug print.
>
> Now we'll get the dmesg errors only if we detect lspcon and lspcon init 
> indeed fails.

I was referring to lspcon_probe() which now has drm_err() on each of the
branches which goto lspcon_init_failed, which has another
drm_err(). There's no need for the extra error message.

BR,
Jani.


>
> Regards,
>
> Ankit
>
>
>>
>> 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);
Nautiyal, Ankit K March 26, 2024, 8:29 a.m. UTC | #4
On 3/26/2024 12:26 PM, Jani Nikula wrote:
> On Tue, 26 Mar 2024, "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com> wrote:
>> On 3/25/2024 8:48 PM, Jani Nikula wrote:
>>> On Fri, 22 Mar 2024, Ankit Nautiyal <ankit.k.nautiyal@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@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.
>> Earlier we used to get the drm_error for init failure, even if the
>> LSPCON was not detected, which is printed as a debug print.
>>
>> Now we'll get the dmesg errors only if we detect lspcon and lspcon init
>> indeed fails.
> I was referring to lspcon_probe() which now has drm_err() on each of the
> branches which goto lspcon_init_failed, which has another
> drm_err(). There's no need for the extra error message.

Ah ok, I got it, I can get rid of extra error message at the end.

Thanks & Regards,

Ankit

> BR,
> Jani.
>
>
>> Regards,
>>
>> Ankit
>>
>>
>>> 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);
diff mbox series

Patch

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));
+
+	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);