Re: [PATCH 2/2] drm/i915: Do not get aux power for disconnected DP ports

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 24 Sep 2018, José Roberto de Souza <jose.souza@intel.com> wrote:
> For ICL type-c ports there is a aux power restriction, it can only be
> enabled while there is sink connected.
>
> BSpec: 21750
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 6b4c19123f2a..48fd38cd4ba4 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5019,19 +5019,27 @@ intel_dp_long_pulse(struct intel_connector *connector,
>  	struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
>  	enum drm_connector_status status;
>  	u8 sink_irq_vector = 0;
> +	bool got_aux_power;
>  
>  	WARN_ON(!drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex));
>  
> +	/* Can't disconnect eDP */
> +	if (!intel_dp_is_edp(intel_dp) &&
> +	    !intel_digital_port_connected(&dp_to_dig_port(intel_dp)->base)) {
> +		status = connector_status_disconnected;
> +		got_aux_power = false;
> +		goto port_disconnected;
> +	}
> +
>  	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> +	got_aux_power = true;
>  
> -	/* Can't disconnect eDP */
>  	if (intel_dp_is_edp(intel_dp))
>  		status = edp_detect(intel_dp);
> -	else if (intel_digital_port_connected(&dp_to_dig_port(intel_dp)->base))
> -		status = intel_dp_detect_dpcd(intel_dp);
>  	else
> -		status = connector_status_disconnected;
> +		status = intel_dp_detect_dpcd(intel_dp);


I think this change makes an already difficult to read function too
complicated. The goto does not help. Please rething the approach.

BR,
Jani.

>  
> +port_disconnected:
>  	if (status == connector_status_disconnected) {
>  		memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
>  
> @@ -5122,7 +5130,8 @@ intel_dp_long_pulse(struct intel_connector *connector,
>  	if (status != connector_status_connected && !intel_dp->is_mst)
>  		intel_dp_unset_edid(intel_dp);
>  
> -	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> +	if (got_aux_power)
> +		intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
>  	return status;
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx