Re: [PATCH v2 6/6] drm/i915/dp: Fix duplication of DEVICE_SERVICE_IRQ handling

From: "Souza, Jose" <jose.souza@intel.com>
To: "intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Subject: Re: [PATCH v2 6/6] drm/i915/dp: Fix duplication of DEVICE_SERVICE_IRQ handling
Date: Tue, 25 Sep 2018 21:01:20 +0000	[thread overview]
Message-ID: <2af9869d50ac18d4542fbd2fe240314c852f1a88.camel@intel.com> (raw)
In-Reply-To: <20180924224528.4744-6-dhinakaran.pandiyan@intel.com>

On Mon, 2018-09-24 at 15:45 -0700, Dhinakaran Pandiyan wrote:
> There are two copies of the same code called from long and short
> pulse handlers.


Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 59 ++++++++++++-------------------
> --
>  1 file changed, 22 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index d6ea93e453a2..64c6158feb0b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4038,13 +4038,6 @@ intel_dp_configure_mst(struct intel_dp
> *intel_dp)
>  					intel_dp->is_mst);
>  }
>  
> -static bool
> -intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8
> *sink_irq_vector)
> -{
> -	return drm_dp_dpcd_readb(&intel_dp->aux,
> DP_DEVICE_SERVICE_IRQ_VECTOR,
> -				 sink_irq_vector) == 1;
> -}
> -
>  static bool
>  intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8
> *sink_irq_vector)
>  {
> @@ -4432,6 +4425,26 @@ static bool intel_dp_hotplug(struct
> intel_encoder *encoder,
>  	return changed;
>  }
>  
> +static void intel_dp_check_service_irq(struct intel_dp *intel_dp)
> +{
> +	u8 val;
> +
> +	if (intel_dp->dpcd[DP_DPCD_REV] < 0x11)
> +		return;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux,
> +			      DP_DEVICE_SERVICE_IRQ_VECTOR, &val) != 1
> || !val)
> +		return;
> +
> +	drm_dp_dpcd_writeb(&intel_dp->aux,
> DP_DEVICE_SERVICE_IRQ_VECTOR, val);
> +
> +	if (val & DP_AUTOMATED_TEST_REQUEST)
> +		intel_dp_handle_test_request(intel_dp);
> +
> +	if (val & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
> +		DRM_DEBUG_DRIVER("CP or sink specific irq
> unhandled\n");
> +}
> +
>  /*
>   * According to DP spec
>   * 5.1.2:
> @@ -4449,7 +4462,6 @@ static bool
>  intel_dp_short_pulse(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> -	u8 sink_irq_vector = 0;
>  	u8 old_sink_count = intel_dp->sink_count;
>  	bool ret;
>  
> @@ -4472,20 +4484,7 @@ intel_dp_short_pulse(struct intel_dp
> *intel_dp)
>  		return false;
>  	}
>  
> -	/* Try to read the source of the interrupt */
> -	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> -	    intel_dp_get_sink_irq(intel_dp, &sink_irq_vector) &&
> -	    sink_irq_vector != 0) {
> -		/* Clear interrupt source */
> -		drm_dp_dpcd_writeb(&intel_dp->aux,
> -				   DP_DEVICE_SERVICE_IRQ_VECTOR,
> -				   sink_irq_vector);
> -
> -		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
> -			intel_dp_handle_test_request(intel_dp);
> -		if (sink_irq_vector & (DP_CP_IRQ |
> DP_SINK_SPECIFIC_IRQ))
> -			DRM_DEBUG_DRIVER("CP or sink specific irq
> unhandled\n");
> -	}
> +	intel_dp_check_service_irq(intel_dp);
>  
>  	/* Handle CEC interrupts, if any */
>  	drm_dp_cec_irq(&intel_dp->aux);
> @@ -5020,7 +5019,6 @@ intel_dp_detect(struct drm_connector
> *connector,
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>  	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)-
> >base;
>  	int status;
> -	u8 sink_irq_vector = 0;
>  
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>  		      connector->base.id, connector->name);
> @@ -5103,20 +5101,7 @@ intel_dp_detect(struct drm_connector
> *connector,
>  	    to_intel_connector(connector)->detect_edid)
>  		status = connector_status_connected;
>  
> -	/* Try to read the source of the interrupt */
> -	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> -	    intel_dp_get_sink_irq(intel_dp, &sink_irq_vector) &&
> -	    sink_irq_vector != 0) {
> -		/* Clear interrupt source */
> -		drm_dp_dpcd_writeb(&intel_dp->aux,
> -				   DP_DEVICE_SERVICE_IRQ_VECTOR,
> -				   sink_irq_vector);
> -
> -		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
> -			intel_dp_handle_test_request(intel_dp);
> -		if (sink_irq_vector & (DP_CP_IRQ |
> DP_SINK_SPECIFIC_IRQ))
> -			DRM_DEBUG_DRIVER("CP or sink specific irq
> unhandled\n");
> -	}
> +	intel_dp_check_service_irq(intel_dp);
>  
>  out:
>  	if (status != connector_status_connected && !intel_dp->is_mst)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx