Re: [Intel-gfx] [PATCH v2 1/5] drm/i915/hpd: Replace the loop-within-loop with two independent loops

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

On Wed, 05 Feb 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> No point in looping over all connectors for each hpd pin. Just loop
> over each connector first and deal with each one's hpd pin. Then
> loop over all the hpd pins to mark them as enabled again.
>
> Also get rid of the MST special case as MST encoders simply don't
> have a HPD pin and will get naturally fitered out.

I think 012ee690372c ("drm/i915: Introduce intel_connector_hpd_pin()")
already took care of this part. Please just drop the sentence.

Other than that,

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

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hotplug.c | 39 +++++++++++---------
>  1 file changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index 127a2f28c1ac..531021002e34 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -242,36 +242,39 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
>  		container_of(work, typeof(*dev_priv),
>  			     hotplug.reenable_work.work);
>  	struct drm_device *dev = &dev_priv->drm;
> +	struct drm_connector_list_iter conn_iter;
> +	struct intel_connector *connector;
>  	intel_wakeref_t wakeref;
>  	enum hpd_pin pin;
>  
>  	wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
>  
>  	spin_lock_irq(&dev_priv->irq_lock);
> -	for_each_hpd_pin(pin) {
> -		struct drm_connector_list_iter conn_iter;
> -		struct intel_connector *connector;
>  
> -		if (dev_priv->hotplug.stats[pin].state != HPD_DISABLED)
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	for_each_intel_connector_iter(connector, &conn_iter) {
> +		pin = intel_connector_hpd_pin(connector);
> +		if (pin == HPD_NONE ||
> +		    dev_priv->hotplug.stats[pin].state != HPD_DISABLED)
>  			continue;
>  
> -		dev_priv->hotplug.stats[pin].state = HPD_ENABLED;
> -
> -		drm_connector_list_iter_begin(dev, &conn_iter);
> -		for_each_intel_connector_iter(connector, &conn_iter) {
> -			if (intel_connector_hpd_pin(connector) == pin) {
> -				if (connector->base.polled != connector->polled)
> -					DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n",
> -							 connector->base.name);
> -				connector->base.polled = connector->polled;
> -				if (!connector->base.polled)
> -					connector->base.polled = DRM_CONNECTOR_POLL_HPD;
> -			}
> -		}
> -		drm_connector_list_iter_end(&conn_iter);
> +		if (connector->base.polled != connector->polled)
> +			DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n",
> +					 connector->base.name);
> +		connector->base.polled = connector->polled;
> +		if (!connector->base.polled)
> +			connector->base.polled = DRM_CONNECTOR_POLL_HPD;
> +	}
> +	drm_connector_list_iter_end(&conn_iter);
> +
> +	for_each_hpd_pin(pin) {
> +		if (dev_priv->hotplug.stats[pin].state == HPD_DISABLED)
> +			dev_priv->hotplug.stats[pin].state = HPD_ENABLED;
>  	}
> +
>  	if (dev_priv->display_irqs_enabled && dev_priv->display.hpd_irq_setup)
>  		dev_priv->display.hpd_irq_setup(dev_priv);
> +
>  	spin_unlock_irq(&dev_priv->irq_lock);
>  
>  	intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);

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