intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Synchronize connectors states when switching from poll to irq
@ 2017-06-26 12:32 Paul Kocialkowski
  2017-06-26 12:51 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Paul Kocialkowski @ 2017-06-26 12:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: David Airlie, linux-kernel, dri-devel, Daniel Vetter

After detecting an IRQ storm, hotplug detection will switch from
irq-based detection to poll-based detection. After a short delay or
when resetting storm detection from debugfs, detection will switch
back to being irq-based.

However, it may occur that polling does not have enough time to detect
the current connector state when that second switch takes place. Thus,
this sets the appropriate hotplug event bits for the concerned
connectors and schedules the hotplug work, that will ensure the
connectors states are in sync when switching back to irq.

Without this, no irq will be triggered and the hpd change will be lost.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_hotplug.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index f1200272a699..29f55480b0bb 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -218,9 +218,13 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
 			struct intel_connector *intel_connector = to_intel_connector(connector);
 
 			if (intel_connector->encoder->hpd_pin == i) {
-				if (connector->polled != intel_connector->polled)
+				if (connector->polled != intel_connector->polled) {
 					DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n",
 							 connector->name);
+
+					dev_priv->hotplug.event_bits |= (1 << i);
+				}
+
 				connector->polled = intel_connector->polled;
 				if (!connector->polled)
 					connector->polled = DRM_CONNECTOR_POLL_HPD;
@@ -232,6 +236,8 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
 		dev_priv->display.hpd_irq_setup(dev_priv);
 	spin_unlock_irq(&dev_priv->irq_lock);
 
+	schedule_work(&dev_priv->hotplug.hotplug_work);
+
 	intel_runtime_pm_put(dev_priv);
 }
 
-- 
2.13.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Synchronize connectors states when switching from poll to irq
  2017-06-26 12:32 [PATCH] drm/i915: Synchronize connectors states when switching from poll to irq Paul Kocialkowski
@ 2017-06-26 12:51 ` Patchwork
  2017-07-18 12:11 ` [PATCH] " Paul Kocialkowski
  2017-07-20  1:04 ` [Intel-gfx] " Pandiyan, Dhinakaran
  2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2017-06-26 12:51 UTC (permalink / raw)
  To: Paul Kocialkowki; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Synchronize connectors states when switching from poll to irq
URL   : https://patchwork.freedesktop.org/series/26361/
State : success

== Summary ==

Series 26361v1 drm/i915: Synchronize connectors states when switching from poll to irq
https://patchwork.freedesktop.org/api/1.0/series/26361/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-kbl-7560u) fdo#100125 +1
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (fi-blb-e6850) fdo#99093
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                dmesg-warn -> PASS       (fi-pnv-d510) fdo#101597 +1

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#99093 https://bugs.freedesktop.org/show_bug.cgi?id=99093
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:441s
fi-bdw-gvtdvm    total:279  pass:257  dwarn:8   dfail:0   fail:0   skip:14  time:432s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:360s
fi-bsw-n3050     total:279  pass:242  dwarn:1   dfail:0   fail:0   skip:36  time:526s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:516s
fi-byt-j1900     total:279  pass:253  dwarn:2   dfail:0   fail:0   skip:24  time:490s
fi-byt-n2820     total:279  pass:249  dwarn:2   dfail:0   fail:0   skip:28  time:482s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:588s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:434s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:416s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:426s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:499s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:474s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:467s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:575s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:580s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:566s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:464s
fi-skl-6700hq    total:279  pass:223  dwarn:1   dfail:0   fail:30  skip:24  time:340s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:473s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:482s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:442s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:538s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:406s

21d74e215ad650f0e8e30de609bd65601f0aa11d drm-tip: 2017y-06m-26d-09h-12m-14s UTC integration manifest
304f83f drm/i915: Synchronize connectors states when switching from poll to irq

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5042/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Synchronize connectors states when switching from poll to irq
  2017-06-26 12:32 [PATCH] drm/i915: Synchronize connectors states when switching from poll to irq Paul Kocialkowski
  2017-06-26 12:51 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-07-18 12:11 ` Paul Kocialkowski
  2017-07-20  6:11   ` Manasi Navare
  2017-07-20  1:04 ` [Intel-gfx] " Pandiyan, Dhinakaran
  2 siblings, 1 reply; 7+ messages in thread
From: Paul Kocialkowski @ 2017-07-18 12:11 UTC (permalink / raw)
  To: intel-gfx; +Cc: David Airlie, linux-kernel, dri-devel, Daniel Vetter

On Mon, 2017-06-26 at 15:32 +0300, Paul Kocialkowski wrote:
> After detecting an IRQ storm, hotplug detection will switch from
> irq-based detection to poll-based detection. After a short delay or
> when resetting storm detection from debugfs, detection will switch
> back to being irq-based.
> 
> However, it may occur that polling does not have enough time to detect
> the current connector state when that second switch takes place. Thus,
> this sets the appropriate hotplug event bits for the concerned
> connectors and schedules the hotplug work, that will ensure the
> connectors states are in sync when switching back to irq.
> 
> Without this, no irq will be triggered and the hpd change will be
> lost.

Does anyone have feedback to provide on this?
It looks like it should be a no-brainer.

Cheers,

Paul

> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hotplug.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index f1200272a699..29f55480b0bb 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -218,9 +218,13 @@ static void
> intel_hpd_irq_storm_reenable_work(struct work_struct *work)
>  			struct intel_connector *intel_connector =
> to_intel_connector(connector);
>  
>  			if (intel_connector->encoder->hpd_pin == i) {
> -				if (connector->polled !=
> intel_connector->polled)
> +				if (connector->polled !=
> intel_connector->polled) {
>  					DRM_DEBUG_DRIVER("Reenabling
> HPD on connector %s\n",
>  							 connector-
> >name);
> +
> +					dev_priv->hotplug.event_bits
> |= (1 << i);
> +				}
> +
>  				connector->polled = intel_connector-
> >polled;
>  				if (!connector->polled)
>  					connector->polled =
> DRM_CONNECTOR_POLL_HPD;
> @@ -232,6 +236,8 @@ static void
> intel_hpd_irq_storm_reenable_work(struct work_struct *work)
>  		dev_priv->display.hpd_irq_setup(dev_priv);
>  	spin_unlock_irq(&dev_priv->irq_lock);
>  
> +	schedule_work(&dev_priv->hotplug.hotplug_work);
> +
>  	intel_runtime_pm_put(dev_priv);
>  }
>  
-- 
Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo, Finland
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Synchronize connectors states when switching from poll to irq
  2017-06-26 12:32 [PATCH] drm/i915: Synchronize connectors states when switching from poll to irq Paul Kocialkowski
  2017-06-26 12:51 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-07-18 12:11 ` [PATCH] " Paul Kocialkowski
@ 2017-07-20  1:04 ` Pandiyan, Dhinakaran
  2017-07-20 14:35   ` Paul Kocialkowski
  2 siblings, 1 reply; 7+ messages in thread
From: Pandiyan, Dhinakaran @ 2017-07-20  1:04 UTC (permalink / raw)
  To: paul.kocialkowski@linux.intel.com
  Cc: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, Vetter, Daniel, airlied@linux.ie


On Mon, 2017-06-26 at 15:32 +0300, Paul Kocialkowski wrote:
> After detecting an IRQ storm, hotplug detection will switch from
> irq-based detection to poll-based detection. After a short delay or
> when resetting storm detection from debugfs, detection will switch
> back to being irq-based.
> 
> However, it may occur that polling does not have enough time to detect
> the current connector state when that second switch takes place. 

Some questions so that I understand this better. How short would this
have to be for detect to not complete? Is there a way I can easily test
this scenario?

> Thus,
> this sets the appropriate hotplug event bits for the concerned
> connectors and schedules the hotplug work, that will ensure the
> connectors states are in sync when switching back to irq.
> 

Not sure I understand this correctly, looks like you are setting the
event_bits even if there was no irq during the polling interval. Is that
right?


> Without this, no irq will be triggered and the hpd change will be lost.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hotplug.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index f1200272a699..29f55480b0bb 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -218,9 +218,13 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
>  			struct intel_connector *intel_connector = to_intel_connector(connector);
>  
>  			if (intel_connector->encoder->hpd_pin == i) {
> -				if (connector->polled != intel_connector->polled)
> +				if (connector->polled != intel_connector->polled) {
>  					DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n",
>  							 connector->name);
> +
> +					dev_priv->hotplug.event_bits |= (1 << i);
> +				}
> +
>  				connector->polled = intel_connector->polled;
>  				if (!connector->polled)
>  					connector->polled = DRM_CONNECTOR_POLL_HPD;
> @@ -232,6 +236,8 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
>  		dev_priv->display.hpd_irq_setup(dev_priv);
>  	spin_unlock_irq(&dev_priv->irq_lock);
>  
> +	schedule_work(&dev_priv->hotplug.hotplug_work);

How does this work with DP connectors? From what I understand, the
event_bits are set for DP based on the return value from
intel_dp_hpd_pulse(). But, doesn't this patch set the bits
unconditionally?

> +
>  	intel_runtime_pm_put(dev_priv);
>  }
>  

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Synchronize connectors states when switching from poll to irq
  2017-07-18 12:11 ` [PATCH] " Paul Kocialkowski
@ 2017-07-20  6:11   ` Manasi Navare
  2017-07-20 14:41     ` Paul Kocialkowski
  0 siblings, 1 reply; 7+ messages in thread
From: Manasi Navare @ 2017-07-20  6:11 UTC (permalink / raw)
  To: Paul Kocialkowski; +Cc: Daniel Vetter, intel-gfx, linux-kernel, dri-devel

On Tue, Jul 18, 2017 at 03:11:42PM +0300, Paul Kocialkowski wrote:
> On Mon, 2017-06-26 at 15:32 +0300, Paul Kocialkowski wrote:
> > After detecting an IRQ storm, hotplug detection will switch from
> > irq-based detection to poll-based detection. After a short delay or
> > when resetting storm detection from debugfs, detection will switch
> > back to being irq-based.
> > 
> > However, it may occur that polling does not have enough time to detect
> > the current connector state when that second switch takes place. Thus,
> > this sets the appropriate hotplug event bits for the concerned
> > connectors and schedules the hotplug work, that will ensure the
> > connectors states are in sync when switching back to irq.
> > 
> > Without this, no irq will be triggered and the hpd change will be
> > lost.
> 
> Does anyone have feedback to provide on this?
> It looks like it should be a no-brainer.
> 
> Cheers,
> 
> Paul
> 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_hotplug.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> > b/drivers/gpu/drm/i915/intel_hotplug.c
> > index f1200272a699..29f55480b0bb 100644
> > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > @@ -218,9 +218,13 @@ static void
> > intel_hpd_irq_storm_reenable_work(struct work_struct *work)
> >  			struct intel_connector *intel_connector =
> > to_intel_connector(connector);
> >  
> >  			if (intel_connector->encoder->hpd_pin == i) {

So if this hpd pin in intel_connector->encoder is set then that
means it got the hpd but because connector->polled is != intel_connector->polled
polling didnt detect that connector.
Is that what you are trying to do here?

Manasi


> > -				if (connector->polled !=
> > intel_connector->polled)
> > +				if (connector->polled !=
> > intel_connector->polled) {
> >  					DRM_DEBUG_DRIVER("Reenabling
> > HPD on connector %s\n",
> >  							 connector-
> > >name);
> > +
> > +					dev_priv->hotplug.event_bits
> > |= (1 << i);
> > +				}
> > +
> >  				connector->polled = intel_connector-
> > >polled;
> >  				if (!connector->polled)
> >  					connector->polled =
> > DRM_CONNECTOR_POLL_HPD;
> > @@ -232,6 +236,8 @@ static void
> > intel_hpd_irq_storm_reenable_work(struct work_struct *work)
> >  		dev_priv->display.hpd_irq_setup(dev_priv);
> >  	spin_unlock_irq(&dev_priv->irq_lock);
> >  
> > +	schedule_work(&dev_priv->hotplug.hotplug_work);
> > +
> >  	intel_runtime_pm_put(dev_priv);
> >  }
> >  
> -- 
> Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo, Finland
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Synchronize connectors states when switching from poll to irq
  2017-07-20  1:04 ` [Intel-gfx] " Pandiyan, Dhinakaran
@ 2017-07-20 14:35   ` Paul Kocialkowski
  0 siblings, 0 replies; 7+ messages in thread
From: Paul Kocialkowski @ 2017-07-20 14:35 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran
  Cc: airlied@linux.ie, Vetter, Daniel, intel-gfx@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org

On Thu, 2017-07-20 at 01:04 +0000, Pandiyan, Dhinakaran wrote:
> On Mon, 2017-06-26 at 15:32 +0300, Paul Kocialkowski wrote:
> > After detecting an IRQ storm, hotplug detection will switch from
> > irq-based detection to poll-based detection. After a short delay or
> > when resetting storm detection from debugfs, detection will switch
> > back to being irq-based.
> > 
> > However, it may occur that polling does not have enough time to
> > detect
> > the current connector state when that second switch takes place. 
> 
> Some questions so that I understand this better. How short would this
> have to be for detect to not complete? Is there a way I can easily
> test this scenario?

Well, it doesn't really matter, the point is that it may happen that the
connector's hpd line changes in the time window between the last poll
(that happens every 100ms) and the moment the irq is enabled back. This
time window, however long it may be, always exists and it may happen
that this is exactly when the hpd event occurs.

It's possible to test this by triggering an hpd storm, then triggering a
regular hpd toggle and directly disabling polling mode via debugfs.
I was able to do this with a chamelium and did see the problem take
place.

> > Thus,
> > this sets the appropriate hotplug event bits for the concerned
> > connectors and schedules the hotplug work, that will ensure the
> > connectors states are in sync when switching back to irq.
> 
> Not sure I understand this correctly, looks like you are setting the
> event_bits even if there was no irq during the polling interval. Is
> that right?

Yes, you are perfectly right. it is necessary to do this because the
event will not be detected either by polling (happening too late) nor by
the irq (happening too early). So we must trigger the hotplug work to
make it check whether the connectors states changed.

> > Without this, no irq will be triggered and the hpd change will be
> > lost.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_hotplug.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> > b/drivers/gpu/drm/i915/intel_hotplug.c
> > index f1200272a699..29f55480b0bb 100644
> > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > @@ -218,9 +218,13 @@ static void
> > intel_hpd_irq_storm_reenable_work(struct work_struct *work)
> >  			struct intel_connector *intel_connector =
> > to_intel_connector(connector);
> >  
> >  			if (intel_connector->encoder->hpd_pin == i)
> > {
> > -				if (connector->polled !=
> > intel_connector->polled)
> > +				if (connector->polled !=
> > intel_connector->polled) {
> >  					DRM_DEBUG_DRIVER("Reenablin
> > g HPD on connector %s\n",
> >  							 connector-
> > >name);
> > +
> > +					dev_priv-
> > >hotplug.event_bits |= (1 << i);
> > +				}
> > +
> >  				connector->polled =
> > intel_connector->polled;
> >  				if (!connector->polled)
> >  					connector->polled =
> > DRM_CONNECTOR_POLL_HPD;
> > @@ -232,6 +236,8 @@ static void
> > intel_hpd_irq_storm_reenable_work(struct work_struct *work)
> >  		dev_priv->display.hpd_irq_setup(dev_priv);
> >  	spin_unlock_irq(&dev_priv->irq_lock);
> >  
> > +	schedule_work(&dev_priv->hotplug.hotplug_work);
> 
> How does this work with DP connectors? From what I understand, the
> event_bits are set for DP based on the return value from
> intel_dp_hpd_pulse(). But, doesn't this patch set the bits
> unconditionally?

It works the same as other connectors, nothing specific there. The
hotplug work will read the connector's state and issue a uevent if its
value has changed.

> > +
> >  	intel_runtime_pm_put(dev_priv);
> >  }
> >  
-- 
Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo, Finland
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Synchronize connectors states when switching from poll to irq
  2017-07-20  6:11   ` Manasi Navare
@ 2017-07-20 14:41     ` Paul Kocialkowski
  0 siblings, 0 replies; 7+ messages in thread
From: Paul Kocialkowski @ 2017-07-20 14:41 UTC (permalink / raw)
  To: Manasi Navare; +Cc: Daniel Vetter, intel-gfx, linux-kernel, dri-devel

On Wed, 2017-07-19 at 23:11 -0700, Manasi Navare wrote:
> On Tue, Jul 18, 2017 at 03:11:42PM +0300, Paul Kocialkowski wrote:
> > On Mon, 2017-06-26 at 15:32 +0300, Paul Kocialkowski wrote:
> > > After detecting an IRQ storm, hotplug detection will switch from
> > > irq-based detection to poll-based detection. After a short delay
> > > or
> > > when resetting storm detection from debugfs, detection will switch
> > > back to being irq-based.
> > > 
> > > However, it may occur that polling does not have enough time to
> > > detect
> > > the current connector state when that second switch takes place.
> > > Thus,
> > > this sets the appropriate hotplug event bits for the concerned
> > > connectors and schedules the hotplug work, that will ensure the
> > > connectors states are in sync when switching back to irq.
> > > 
> > > Without this, no irq will be triggered and the hpd change will be
> > > lost.
> > 
> > Does anyone have feedback to provide on this?
> > It looks like it should be a no-brainer.
> > 
> > Cheers,
> > 
> > Paul
> > 
> > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.co
> > > m>
> > > ---
> > >  drivers/gpu/drm/i915/intel_hotplug.c | 8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> > > b/drivers/gpu/drm/i915/intel_hotplug.c
> > > index f1200272a699..29f55480b0bb 100644
> > > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > > @@ -218,9 +218,13 @@ static void
> > > intel_hpd_irq_storm_reenable_work(struct work_struct *work)
> > >  			struct intel_connector *intel_connector =
> > > to_intel_connector(connector);
> > >  
> > >  			if (intel_connector->encoder->hpd_pin ==
> > > i) {
> 
> So if this hpd pin in intel_connector->encoder is set then that
> means it got the hpd but because connector->polled is !=
> intel_connector->polled
> polling didnt detect that connector.
>
> Is that what you are trying to do here?

The first test is simply a way to match the intel_connector to the drm
connector. The second one indicates whether we are using polling or irq
mode. If they don't match (given the test earlier in the code that only
selects hpd pins that use irq mode), it means that we are switching from
 polling mode to irq mode.

When that happens, it is necessary to assume that the connector state
may have changed (in the time window between the last poll and enabling
irq), so the hotplug work is scheduled. It will check whether a hpd
state change did happen or not and issue a uevent if that is the case.

> > > -				if (connector->polled !=
> > > intel_connector->polled)
> > > +				if (connector->polled !=
> > > intel_connector->polled) {
> > >  					DRM_DEBUG_DRIVER("Reenabl
> > > ing
> > > HPD on connector %s\n",
> > >  							 connecto
> > > r-
> > > > name);
> > > 
> > > +
> > > +					dev_priv-
> > > >hotplug.event_bits
> > > > = (1 << i);
> > > 
> > > +				}
> > > +
> > >  				connector->polled =
> > > intel_connector-
> > > > polled;
> > > 
> > >  				if (!connector->polled)
> > >  					connector->polled =
> > > DRM_CONNECTOR_POLL_HPD;
> > > @@ -232,6 +236,8 @@ static void
> > > intel_hpd_irq_storm_reenable_work(struct work_struct *work)
> > >  		dev_priv->display.hpd_irq_setup(dev_priv);
> > >  	spin_unlock_irq(&dev_priv->irq_lock);
> > >  
> > > +	schedule_work(&dev_priv->hotplug.hotplug_work);
> > > +
> > >  	intel_runtime_pm_put(dev_priv);
> > >  }
> > >  
> > 
> > -- 
> > Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> > Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo,
> > Finland
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- 
Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo, Finland
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-07-20 14:41 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-26 12:32 [PATCH] drm/i915: Synchronize connectors states when switching from poll to irq Paul Kocialkowski
2017-06-26 12:51 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-07-18 12:11 ` [PATCH] " Paul Kocialkowski
2017-07-20  6:11   ` Manasi Navare
2017-07-20 14:41     ` Paul Kocialkowski
2017-07-20  1:04 ` [Intel-gfx] " Pandiyan, Dhinakaran
2017-07-20 14:35   ` Paul Kocialkowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).