* [PATCH] drm/i915: add hotplug activation period to hotplug update mask
@ 2015-10-21 14:22 Jani Nikula
2015-10-21 14:32 ` Ville Syrjälä
2015-10-21 18:28 ` Oleksij Rempel
0 siblings, 2 replies; 8+ messages in thread
From: Jani Nikula @ 2015-10-21 14:22 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, Egbert Eich, Daniel Vetter, Oleksij Rempel
commit 0706f17c307b056ff6f1848320ba82d76945a6ff
Author: Egbert Eich <eich@suse.de>
Date: Wed Sep 23 16:15:27 2015 +0200
drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2
added a check with WARN to ensure only bits within the mask are
enabled. Turns out that doesn't hold for G4X, which spits out:
[ 2.641439] ------------[ cut here ]------------
[ 2.641444] WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/i915/i915_irq.c:182 i915_hotplug_interrupt_update_locked+0x45/0x83()
[ 2.641446] WARN_ON(bits & ~mask)
etc.
Add CRT_HOTPLUG_ACTIVATION_PERIOD_64 to the mask to fix the warning.
Reported-by: Oleksij Rempel <linux@rempel-privat.de>
References: https://bugzilla.kernel.org/show_bug.cgi?id=104991
Fixes: 0706f17c307b ("drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2")
Cc: Egbert Eich <eich@suse.de>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7f91f74961f4..5b9f63d4318b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4236,9 +4236,10 @@ static void i915_hpd_irq_setup(struct drm_device *dev)
/* Ignore TV since it's buggy */
i915_hotplug_interrupt_update_locked(dev_priv,
- (HOTPLUG_INT_EN_MASK
- | CRT_HOTPLUG_VOLTAGE_COMPARE_MASK),
- hotplug_en);
+ HOTPLUG_INT_EN_MASK |
+ CRT_HOTPLUG_VOLTAGE_COMPARE_MASK |
+ CRT_HOTPLUG_ACTIVATION_PERIOD_64,
+ hotplug_en);
}
static irqreturn_t i965_irq_handler(int irq, void *arg)
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] drm/i915: add hotplug activation period to hotplug update mask 2015-10-21 14:22 [PATCH] drm/i915: add hotplug activation period to hotplug update mask Jani Nikula @ 2015-10-21 14:32 ` Ville Syrjälä 2015-10-22 7:46 ` Jani Nikula 2015-10-21 18:28 ` Oleksij Rempel 1 sibling, 1 reply; 8+ messages in thread From: Ville Syrjälä @ 2015-10-21 14:32 UTC (permalink / raw) To: Jani Nikula; +Cc: Egbert Eich, Daniel Vetter, intel-gfx, Oleksij Rempel On Wed, Oct 21, 2015 at 05:22:43PM +0300, Jani Nikula wrote: > commit 0706f17c307b056ff6f1848320ba82d76945a6ff > Author: Egbert Eich <eich@suse.de> > Date: Wed Sep 23 16:15:27 2015 +0200 > > drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2 > > added a check with WARN to ensure only bits within the mask are > enabled. Turns out that doesn't hold for G4X, which spits out: > > [ 2.641439] ------------[ cut here ]------------ > [ 2.641444] WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/i915/i915_irq.c:182 i915_hotplug_interrupt_update_locked+0x45/0x83() > [ 2.641446] WARN_ON(bits & ~mask) > etc. > > Add CRT_HOTPLUG_ACTIVATION_PERIOD_64 to the mask to fix the warning. > > Reported-by: Oleksij Rempel <linux@rempel-privat.de> > References: https://bugzilla.kernel.org/show_bug.cgi?id=104991 > Fixes: 0706f17c307b ("drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2") > Cc: Egbert Eich <eich@suse.de> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 7f91f74961f4..5b9f63d4318b 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -4236,9 +4236,10 @@ static void i915_hpd_irq_setup(struct drm_device *dev) > > /* Ignore TV since it's buggy */ > i915_hotplug_interrupt_update_locked(dev_priv, > - (HOTPLUG_INT_EN_MASK > - | CRT_HOTPLUG_VOLTAGE_COMPARE_MASK), > - hotplug_en); > + HOTPLUG_INT_EN_MASK | > + CRT_HOTPLUG_VOLTAGE_COMPARE_MASK | > + CRT_HOTPLUG_ACTIVATION_PERIOD_64, > + hotplug_en); Or maybe just ~CRT_HOTPLUG_FORCE_DETECT ? > } > > static irqreturn_t i965_irq_handler(int irq, void *arg) > -- > 2.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: add hotplug activation period to hotplug update mask 2015-10-21 14:32 ` Ville Syrjälä @ 2015-10-22 7:46 ` Jani Nikula 2015-10-22 8:03 ` Daniel Vetter 2015-10-22 12:01 ` Ville Syrjälä 0 siblings, 2 replies; 8+ messages in thread From: Jani Nikula @ 2015-10-22 7:46 UTC (permalink / raw) To: Ville Syrjälä Cc: Egbert Eich, Daniel Vetter, intel-gfx, Oleksij Rempel On Wed, 21 Oct 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Wed, Oct 21, 2015 at 05:22:43PM +0300, Jani Nikula wrote: >> commit 0706f17c307b056ff6f1848320ba82d76945a6ff >> Author: Egbert Eich <eich@suse.de> >> Date: Wed Sep 23 16:15:27 2015 +0200 >> >> drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2 >> >> added a check with WARN to ensure only bits within the mask are >> enabled. Turns out that doesn't hold for G4X, which spits out: >> >> [ 2.641439] ------------[ cut here ]------------ >> [ 2.641444] WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/i915/i915_irq.c:182 i915_hotplug_interrupt_update_locked+0x45/0x83() >> [ 2.641446] WARN_ON(bits & ~mask) >> etc. >> >> Add CRT_HOTPLUG_ACTIVATION_PERIOD_64 to the mask to fix the warning. >> >> Reported-by: Oleksij Rempel <linux@rempel-privat.de> >> References: https://bugzilla.kernel.org/show_bug.cgi?id=104991 >> Fixes: 0706f17c307b ("drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2") >> Cc: Egbert Eich <eich@suse.de> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/i915/i915_irq.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c >> index 7f91f74961f4..5b9f63d4318b 100644 >> --- a/drivers/gpu/drm/i915/i915_irq.c >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> @@ -4236,9 +4236,10 @@ static void i915_hpd_irq_setup(struct drm_device *dev) >> >> /* Ignore TV since it's buggy */ >> i915_hotplug_interrupt_update_locked(dev_priv, >> - (HOTPLUG_INT_EN_MASK >> - | CRT_HOTPLUG_VOLTAGE_COMPARE_MASK), >> - hotplug_en); >> + HOTPLUG_INT_EN_MASK | >> + CRT_HOTPLUG_VOLTAGE_COMPARE_MASK | >> + CRT_HOTPLUG_ACTIVATION_PERIOD_64, >> + hotplug_en); > > Or maybe just ~CRT_HOTPLUG_FORCE_DETECT ? This patch already potentially changes behaviour by explicitly setting the activation period to zero (CRT_HOTPLUG_ACTIVATION_PERIOD_32) on non-g4x. Previously it was "don't care". Call me coward, but I am not comfortable with setting the rest of the bits to zero, at least not in the context of this fix, and risking regressions on old machines. BR, Jani. > >> } >> >> static irqreturn_t i965_irq_handler(int irq, void *arg) >> -- >> 2.1.4 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: add hotplug activation period to hotplug update mask 2015-10-22 7:46 ` Jani Nikula @ 2015-10-22 8:03 ` Daniel Vetter 2015-10-22 11:27 ` Jani Nikula 2015-10-22 12:01 ` Ville Syrjälä 1 sibling, 1 reply; 8+ messages in thread From: Daniel Vetter @ 2015-10-22 8:03 UTC (permalink / raw) To: Jani Nikula; +Cc: Egbert Eich, Daniel Vetter, intel-gfx, Oleksij Rempel On Thu, Oct 22, 2015 at 10:46:58AM +0300, Jani Nikula wrote: > On Wed, 21 Oct 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Wed, Oct 21, 2015 at 05:22:43PM +0300, Jani Nikula wrote: > >> commit 0706f17c307b056ff6f1848320ba82d76945a6ff > >> Author: Egbert Eich <eich@suse.de> > >> Date: Wed Sep 23 16:15:27 2015 +0200 > >> > >> drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2 > >> > >> added a check with WARN to ensure only bits within the mask are > >> enabled. Turns out that doesn't hold for G4X, which spits out: > >> > >> [ 2.641439] ------------[ cut here ]------------ > >> [ 2.641444] WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/i915/i915_irq.c:182 i915_hotplug_interrupt_update_locked+0x45/0x83() > >> [ 2.641446] WARN_ON(bits & ~mask) > >> etc. > >> > >> Add CRT_HOTPLUG_ACTIVATION_PERIOD_64 to the mask to fix the warning. > >> > >> Reported-by: Oleksij Rempel <linux@rempel-privat.de> > >> References: https://bugzilla.kernel.org/show_bug.cgi?id=104991 > >> Fixes: 0706f17c307b ("drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2") > >> Cc: Egbert Eich <eich@suse.de> > >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_irq.c | 7 ++++--- > >> 1 file changed, 4 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > >> index 7f91f74961f4..5b9f63d4318b 100644 > >> --- a/drivers/gpu/drm/i915/i915_irq.c > >> +++ b/drivers/gpu/drm/i915/i915_irq.c > >> @@ -4236,9 +4236,10 @@ static void i915_hpd_irq_setup(struct drm_device *dev) > >> > >> /* Ignore TV since it's buggy */ > >> i915_hotplug_interrupt_update_locked(dev_priv, > >> - (HOTPLUG_INT_EN_MASK > >> - | CRT_HOTPLUG_VOLTAGE_COMPARE_MASK), > >> - hotplug_en); > >> + HOTPLUG_INT_EN_MASK | > >> + CRT_HOTPLUG_VOLTAGE_COMPARE_MASK | > >> + CRT_HOTPLUG_ACTIVATION_PERIOD_64, > >> + hotplug_en); > > > > Or maybe just ~CRT_HOTPLUG_FORCE_DETECT ? > > This patch already potentially changes behaviour by explicitly setting > the activation period to zero (CRT_HOTPLUG_ACTIVATION_PERIOD_32) on > non-g4x. Previously it was "don't care". Call me coward, but I am not > comfortable with setting the rest of the bits to zero, at least not in > the context of this fix, and risking regressions on old machines. Might be a notch too paranoid, but we've always had that rmw in there. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > BR, > Jani. > > > > > >> } > >> > >> static irqreturn_t i965_irq_handler(int irq, void *arg) > >> -- > >> 2.1.4 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Ville Syrjälä > > Intel OTC > > -- > Jani Nikula, Intel Open Source Technology Center -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: add hotplug activation period to hotplug update mask 2015-10-22 8:03 ` Daniel Vetter @ 2015-10-22 11:27 ` Jani Nikula 0 siblings, 0 replies; 8+ messages in thread From: Jani Nikula @ 2015-10-22 11:27 UTC (permalink / raw) To: Daniel Vetter; +Cc: Egbert Eich, Daniel Vetter, intel-gfx, Oleksij Rempel On Thu, 22 Oct 2015, Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Oct 22, 2015 at 10:46:58AM +0300, Jani Nikula wrote: >> On Wed, 21 Oct 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >> > On Wed, Oct 21, 2015 at 05:22:43PM +0300, Jani Nikula wrote: >> >> commit 0706f17c307b056ff6f1848320ba82d76945a6ff >> >> Author: Egbert Eich <eich@suse.de> >> >> Date: Wed Sep 23 16:15:27 2015 +0200 >> >> >> >> drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2 >> >> >> >> added a check with WARN to ensure only bits within the mask are >> >> enabled. Turns out that doesn't hold for G4X, which spits out: >> >> >> >> [ 2.641439] ------------[ cut here ]------------ >> >> [ 2.641444] WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/i915/i915_irq.c:182 i915_hotplug_interrupt_update_locked+0x45/0x83() >> >> [ 2.641446] WARN_ON(bits & ~mask) >> >> etc. >> >> >> >> Add CRT_HOTPLUG_ACTIVATION_PERIOD_64 to the mask to fix the warning. >> >> >> >> Reported-by: Oleksij Rempel <linux@rempel-privat.de> >> >> References: https://bugzilla.kernel.org/show_bug.cgi?id=104991 >> >> Fixes: 0706f17c307b ("drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2") >> >> Cc: Egbert Eich <eich@suse.de> >> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> >> --- >> >> drivers/gpu/drm/i915/i915_irq.c | 7 ++++--- >> >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c >> >> index 7f91f74961f4..5b9f63d4318b 100644 >> >> --- a/drivers/gpu/drm/i915/i915_irq.c >> >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> >> @@ -4236,9 +4236,10 @@ static void i915_hpd_irq_setup(struct drm_device *dev) >> >> >> >> /* Ignore TV since it's buggy */ >> >> i915_hotplug_interrupt_update_locked(dev_priv, >> >> - (HOTPLUG_INT_EN_MASK >> >> - | CRT_HOTPLUG_VOLTAGE_COMPARE_MASK), >> >> - hotplug_en); >> >> + HOTPLUG_INT_EN_MASK | >> >> + CRT_HOTPLUG_VOLTAGE_COMPARE_MASK | >> >> + CRT_HOTPLUG_ACTIVATION_PERIOD_64, >> >> + hotplug_en); >> > >> > Or maybe just ~CRT_HOTPLUG_FORCE_DETECT ? >> >> This patch already potentially changes behaviour by explicitly setting >> the activation period to zero (CRT_HOTPLUG_ACTIVATION_PERIOD_32) on >> non-g4x. Previously it was "don't care". Call me coward, but I am not >> comfortable with setting the rest of the bits to zero, at least not in >> the context of this fix, and risking regressions on old machines. > > Might be a notch too paranoid, but we've always had that rmw in there. > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Pushed to drm-intel-next-fixes, thanks for the review. If someone(tm) wants to remove the rmw, it should be through dinq. BR, Jani. > >> >> BR, >> Jani. >> >> >> > >> >> } >> >> >> >> static irqreturn_t i965_irq_handler(int irq, void *arg) >> >> -- >> >> 2.1.4 >> >> >> >> _______________________________________________ >> >> Intel-gfx mailing list >> >> Intel-gfx@lists.freedesktop.org >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> > >> > -- >> > Ville Syrjälä >> > Intel OTC >> >> -- >> Jani Nikula, Intel Open Source Technology Center -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: add hotplug activation period to hotplug update mask 2015-10-22 7:46 ` Jani Nikula 2015-10-22 8:03 ` Daniel Vetter @ 2015-10-22 12:01 ` Ville Syrjälä 2015-10-22 12:43 ` Jani Nikula 1 sibling, 1 reply; 8+ messages in thread From: Ville Syrjälä @ 2015-10-22 12:01 UTC (permalink / raw) To: Jani Nikula; +Cc: Egbert Eich, Daniel Vetter, intel-gfx, Oleksij Rempel On Thu, Oct 22, 2015 at 10:46:58AM +0300, Jani Nikula wrote: > On Wed, 21 Oct 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Wed, Oct 21, 2015 at 05:22:43PM +0300, Jani Nikula wrote: > >> commit 0706f17c307b056ff6f1848320ba82d76945a6ff > >> Author: Egbert Eich <eich@suse.de> > >> Date: Wed Sep 23 16:15:27 2015 +0200 > >> > >> drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2 > >> > >> added a check with WARN to ensure only bits within the mask are > >> enabled. Turns out that doesn't hold for G4X, which spits out: > >> > >> [ 2.641439] ------------[ cut here ]------------ > >> [ 2.641444] WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/i915/i915_irq.c:182 i915_hotplug_interrupt_update_locked+0x45/0x83() > >> [ 2.641446] WARN_ON(bits & ~mask) > >> etc. > >> > >> Add CRT_HOTPLUG_ACTIVATION_PERIOD_64 to the mask to fix the warning. > >> > >> Reported-by: Oleksij Rempel <linux@rempel-privat.de> > >> References: https://bugzilla.kernel.org/show_bug.cgi?id=104991 > >> Fixes: 0706f17c307b ("drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2") > >> Cc: Egbert Eich <eich@suse.de> > >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_irq.c | 7 ++++--- > >> 1 file changed, 4 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > >> index 7f91f74961f4..5b9f63d4318b 100644 > >> --- a/drivers/gpu/drm/i915/i915_irq.c > >> +++ b/drivers/gpu/drm/i915/i915_irq.c > >> @@ -4236,9 +4236,10 @@ static void i915_hpd_irq_setup(struct drm_device *dev) > >> > >> /* Ignore TV since it's buggy */ > >> i915_hotplug_interrupt_update_locked(dev_priv, > >> - (HOTPLUG_INT_EN_MASK > >> - | CRT_HOTPLUG_VOLTAGE_COMPARE_MASK), > >> - hotplug_en); > >> + HOTPLUG_INT_EN_MASK | > >> + CRT_HOTPLUG_VOLTAGE_COMPARE_MASK | > >> + CRT_HOTPLUG_ACTIVATION_PERIOD_64, > >> + hotplug_en); > > > > Or maybe just ~CRT_HOTPLUG_FORCE_DETECT ? > > This patch already potentially changes behaviour by explicitly setting > the activation period to zero (CRT_HOTPLUG_ACTIVATION_PERIOD_32) on > non-g4x. Previously it was "don't care". It was already cleared in irq pre/postinstall. > Call me coward, but I am not > comfortable with setting the rest of the bits to zero, at least not in > the context of this fix, and risking regressions on old machines. > > BR, > Jani. > > > > > >> } > >> > >> static irqreturn_t i965_irq_handler(int irq, void *arg) > >> -- > >> 2.1.4 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Ville Syrjälä > > Intel OTC > > -- > Jani Nikula, Intel Open Source Technology Center -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: add hotplug activation period to hotplug update mask 2015-10-22 12:01 ` Ville Syrjälä @ 2015-10-22 12:43 ` Jani Nikula 0 siblings, 0 replies; 8+ messages in thread From: Jani Nikula @ 2015-10-22 12:43 UTC (permalink / raw) To: Ville Syrjälä Cc: Egbert Eich, Daniel Vetter, intel-gfx, Oleksij Rempel On Thu, 22 Oct 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Thu, Oct 22, 2015 at 10:46:58AM +0300, Jani Nikula wrote: >> On Wed, 21 Oct 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >> > On Wed, Oct 21, 2015 at 05:22:43PM +0300, Jani Nikula wrote: >> >> commit 0706f17c307b056ff6f1848320ba82d76945a6ff >> >> Author: Egbert Eich <eich@suse.de> >> >> Date: Wed Sep 23 16:15:27 2015 +0200 >> >> >> >> drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2 >> >> >> >> added a check with WARN to ensure only bits within the mask are >> >> enabled. Turns out that doesn't hold for G4X, which spits out: >> >> >> >> [ 2.641439] ------------[ cut here ]------------ >> >> [ 2.641444] WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/i915/i915_irq.c:182 i915_hotplug_interrupt_update_locked+0x45/0x83() >> >> [ 2.641446] WARN_ON(bits & ~mask) >> >> etc. >> >> >> >> Add CRT_HOTPLUG_ACTIVATION_PERIOD_64 to the mask to fix the warning. >> >> >> >> Reported-by: Oleksij Rempel <linux@rempel-privat.de> >> >> References: https://bugzilla.kernel.org/show_bug.cgi?id=104991 >> >> Fixes: 0706f17c307b ("drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2") >> >> Cc: Egbert Eich <eich@suse.de> >> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> >> --- >> >> drivers/gpu/drm/i915/i915_irq.c | 7 ++++--- >> >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c >> >> index 7f91f74961f4..5b9f63d4318b 100644 >> >> --- a/drivers/gpu/drm/i915/i915_irq.c >> >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> >> @@ -4236,9 +4236,10 @@ static void i915_hpd_irq_setup(struct drm_device *dev) >> >> >> >> /* Ignore TV since it's buggy */ >> >> i915_hotplug_interrupt_update_locked(dev_priv, >> >> - (HOTPLUG_INT_EN_MASK >> >> - | CRT_HOTPLUG_VOLTAGE_COMPARE_MASK), >> >> - hotplug_en); >> >> + HOTPLUG_INT_EN_MASK | >> >> + CRT_HOTPLUG_VOLTAGE_COMPARE_MASK | >> >> + CRT_HOTPLUG_ACTIVATION_PERIOD_64, >> >> + hotplug_en); >> > >> > Or maybe just ~CRT_HOTPLUG_FORCE_DETECT ? >> >> This patch already potentially changes behaviour by explicitly setting >> the activation period to zero (CRT_HOTPLUG_ACTIVATION_PERIOD_32) on >> non-g4x. Previously it was "don't care". > > It was already cleared in irq pre/postinstall. Fair enough. Anyway I'd like the further change to simmer in dinq first; the fix here was for drm-intel-next-fixes. BR, Jani. > >> Call me coward, but I am not >> comfortable with setting the rest of the bits to zero, at least not in >> the context of this fix, and risking regressions on old machines. >> >> BR, >> Jani. >> >> >> > >> >> } >> >> >> >> static irqreturn_t i965_irq_handler(int irq, void *arg) >> >> -- >> >> 2.1.4 >> >> >> >> _______________________________________________ >> >> Intel-gfx mailing list >> >> Intel-gfx@lists.freedesktop.org >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> > >> > -- >> > Ville Syrjälä >> > Intel OTC >> >> -- >> Jani Nikula, Intel Open Source Technology Center -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: add hotplug activation period to hotplug update mask 2015-10-21 14:22 [PATCH] drm/i915: add hotplug activation period to hotplug update mask Jani Nikula 2015-10-21 14:32 ` Ville Syrjälä @ 2015-10-21 18:28 ` Oleksij Rempel 1 sibling, 0 replies; 8+ messages in thread From: Oleksij Rempel @ 2015-10-21 18:28 UTC (permalink / raw) To: Jani Nikula, intel-gfx; +Cc: Egbert Eich, Daniel Vetter [-- Attachment #1.1: Type: text/plain, Size: 2002 bytes --] Hi, no more warnings with this patch. Thank you! Am 21.10.2015 um 16:22 schrieb Jani Nikula: > commit 0706f17c307b056ff6f1848320ba82d76945a6ff > Author: Egbert Eich <eich@suse.de> > Date: Wed Sep 23 16:15:27 2015 +0200 > > drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2 > > added a check with WARN to ensure only bits within the mask are > enabled. Turns out that doesn't hold for G4X, which spits out: > > [ 2.641439] ------------[ cut here ]------------ > [ 2.641444] WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/i915/i915_irq.c:182 i915_hotplug_interrupt_update_locked+0x45/0x83() > [ 2.641446] WARN_ON(bits & ~mask) > etc. > > Add CRT_HOTPLUG_ACTIVATION_PERIOD_64 to the mask to fix the warning. > > Reported-by: Oleksij Rempel <linux@rempel-privat.de> > References: https://bugzilla.kernel.org/show_bug.cgi?id=104991 > Fixes: 0706f17c307b ("drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2") > Cc: Egbert Eich <eich@suse.de> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 7f91f74961f4..5b9f63d4318b 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -4236,9 +4236,10 @@ static void i915_hpd_irq_setup(struct drm_device *dev) > > /* Ignore TV since it's buggy */ > i915_hotplug_interrupt_update_locked(dev_priv, > - (HOTPLUG_INT_EN_MASK > - | CRT_HOTPLUG_VOLTAGE_COMPARE_MASK), > - hotplug_en); > + HOTPLUG_INT_EN_MASK | > + CRT_HOTPLUG_VOLTAGE_COMPARE_MASK | > + CRT_HOTPLUG_ACTIVATION_PERIOD_64, > + hotplug_en); > } > > static irqreturn_t i965_irq_handler(int irq, void *arg) > -- Regards, Oleksij [-- Attachment #1.2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 213 bytes --] [-- Attachment #2: Type: text/plain, Size: 159 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-10-22 12:39 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-10-21 14:22 [PATCH] drm/i915: add hotplug activation period to hotplug update mask Jani Nikula 2015-10-21 14:32 ` Ville Syrjälä 2015-10-22 7:46 ` Jani Nikula 2015-10-22 8:03 ` Daniel Vetter 2015-10-22 11:27 ` Jani Nikula 2015-10-22 12:01 ` Ville Syrjälä 2015-10-22 12:43 ` Jani Nikula 2015-10-21 18:28 ` Oleksij Rempel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox