* [PATCH] drm/i915: GMBUS don't need no forcewake
@ 2016-10-12 11:44 ville.syrjala
2016-10-12 11:58 ` Chris Wilson
2016-10-12 13:49 ` ✗ Fi.CI.BAT: warning for " Patchwork
0 siblings, 2 replies; 6+ messages in thread
From: ville.syrjala @ 2016-10-12 11:44 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
GMBUS is part of the display engine, and thus has no need for
forcewake. Let's not bother trying to grab it then.
I don't recall if the display engine suffers from system hangs
due to multiple accesses to the same "cacheline" in mmio space.
I hope not since we're no longer protected by the uncore lock
since commit 4e6c2d58ba86 ("drm/i915: Take forcewake once for
the entire GMBUS transaction")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: David Weinehall <david.weinehall@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_i2c.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index 79aab9ad6faa..49c7824a4c29 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -468,13 +468,9 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
struct intel_gmbus,
adapter);
struct drm_i915_private *dev_priv = bus->dev_priv;
- const unsigned int fw =
- intel_uncore_forcewake_for_reg(dev_priv, GMBUS0,
- FW_REG_READ | FW_REG_WRITE);
int i = 0, inc, try = 0;
int ret = 0;
- intel_uncore_forcewake_get(dev_priv, fw);
retry:
I915_WRITE_FW(GMBUS0, bus->reg0);
@@ -576,7 +572,6 @@ timeout:
ret = -EAGAIN;
out:
- intel_uncore_forcewake_put(dev_priv, fw);
return ret;
}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: GMBUS don't need no forcewake
2016-10-12 11:44 [PATCH] drm/i915: GMBUS don't need no forcewake ville.syrjala
@ 2016-10-12 11:58 ` Chris Wilson
2016-10-12 12:39 ` Ville Syrjälä
2016-10-12 13:49 ` ✗ Fi.CI.BAT: warning for " Patchwork
1 sibling, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2016-10-12 11:58 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Wed, Oct 12, 2016 at 02:44:47PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> GMBUS is part of the display engine, and thus has no need for
> forcewake. Let's not bother trying to grab it then.
>
> I don't recall if the display engine suffers from system hangs
> due to multiple accesses to the same "cacheline" in mmio space.
> I hope not since we're no longer protected by the uncore lock
> since commit 4e6c2d58ba86 ("drm/i915: Take forcewake once for
> the entire GMBUS transaction")
Only applies to concurrent access to the same cacheline, in this case
should be serialised by the mutex around the gmbus xfer.
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_i2c.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index 79aab9ad6faa..49c7824a4c29 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -468,13 +468,9 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
> struct intel_gmbus,
> adapter);
> struct drm_i915_private *dev_priv = bus->dev_priv;
> - const unsigned int fw =
> - intel_uncore_forcewake_for_reg(dev_priv, GMBUS0,
> - FW_REG_READ | FW_REG_WRITE);
> int i = 0, inc, try = 0;
> int ret = 0;
I915_WARN_ON(intel_uncore_forcewake_for_reg(dev_priv, GMBUS0,
FW_REG_READ |
FW_REG_WRITE));
? Would be good to test the fw handling as well.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: GMBUS don't need no forcewake
2016-10-12 11:58 ` Chris Wilson
@ 2016-10-12 12:39 ` Ville Syrjälä
2016-10-12 12:51 ` Chris Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2016-10-12 12:39 UTC (permalink / raw)
To: Chris Wilson, intel-gfx, David Weinehall
On Wed, Oct 12, 2016 at 12:58:34PM +0100, Chris Wilson wrote:
> On Wed, Oct 12, 2016 at 02:44:47PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > GMBUS is part of the display engine, and thus has no need for
> > forcewake. Let's not bother trying to grab it then.
> >
> > I don't recall if the display engine suffers from system hangs
> > due to multiple accesses to the same "cacheline" in mmio space.
> > I hope not since we're no longer protected by the uncore lock
> > since commit 4e6c2d58ba86 ("drm/i915: Take forcewake once for
> > the entire GMBUS transaction")
>
> Only applies to concurrent access to the same cacheline, in this case
> should be serialised by the mutex around the gmbus xfer.
Hmm. Yeah, I suppose there shouldn't be unrelated stuff nearby. Haven't
double checked though.
>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: David Weinehall <david.weinehall@linux.intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_i2c.c | 5 -----
> > 1 file changed, 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> > index 79aab9ad6faa..49c7824a4c29 100644
> > --- a/drivers/gpu/drm/i915/intel_i2c.c
> > +++ b/drivers/gpu/drm/i915/intel_i2c.c
> > @@ -468,13 +468,9 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
> > struct intel_gmbus,
> > adapter);
> > struct drm_i915_private *dev_priv = bus->dev_priv;
> > - const unsigned int fw =
> > - intel_uncore_forcewake_for_reg(dev_priv, GMBUS0,
> > - FW_REG_READ | FW_REG_WRITE);
> > int i = 0, inc, try = 0;
> > int ret = 0;
>
> I915_WARN_ON(intel_uncore_forcewake_for_reg(dev_priv, GMBUS0,
> FW_REG_READ |
> FW_REG_WRITE));
>
> ? Would be good to test the fw handling as well.
Not sure I'd want to sprinkle forcewake testing into modeset code.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: GMBUS don't need no forcewake
2016-10-12 12:39 ` Ville Syrjälä
@ 2016-10-12 12:51 ` Chris Wilson
0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2016-10-12 12:51 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On Wed, Oct 12, 2016 at 03:39:47PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 12, 2016 at 12:58:34PM +0100, Chris Wilson wrote:
> > On Wed, Oct 12, 2016 at 02:44:47PM +0300, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > GMBUS is part of the display engine, and thus has no need for
> > > forcewake. Let's not bother trying to grab it then.
> > >
> > > I don't recall if the display engine suffers from system hangs
> > > due to multiple accesses to the same "cacheline" in mmio space.
> > > I hope not since we're no longer protected by the uncore lock
> > > since commit 4e6c2d58ba86 ("drm/i915: Take forcewake once for
> > > the entire GMBUS transaction")
> >
> > Only applies to concurrent access to the same cacheline, in this case
> > should be serialised by the mutex around the gmbus xfer.
>
> Hmm. Yeah, I suppose there shouldn't be unrelated stuff nearby. Haven't
> double checked though.
>
> >
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: David Weinehall <david.weinehall@linux.intel.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_i2c.c | 5 -----
> > > 1 file changed, 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> > > index 79aab9ad6faa..49c7824a4c29 100644
> > > --- a/drivers/gpu/drm/i915/intel_i2c.c
> > > +++ b/drivers/gpu/drm/i915/intel_i2c.c
> > > @@ -468,13 +468,9 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
> > > struct intel_gmbus,
> > > adapter);
> > > struct drm_i915_private *dev_priv = bus->dev_priv;
> > > - const unsigned int fw =
> > > - intel_uncore_forcewake_for_reg(dev_priv, GMBUS0,
> > > - FW_REG_READ | FW_REG_WRITE);
> > > int i = 0, inc, try = 0;
> > > int ret = 0;
> >
> > I915_WARN_ON(intel_uncore_forcewake_for_reg(dev_priv, GMBUS0,
> > FW_REG_READ |
> > FW_REG_WRITE));
> >
> > ? Would be good to test the fw handling as well.
>
> Not sure I'd want to sprinkle forcewake testing into modeset code.
You never use registers here? ;)
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* ✗ Fi.CI.BAT: warning for drm/i915: GMBUS don't need no forcewake
2016-10-12 11:44 [PATCH] drm/i915: GMBUS don't need no forcewake ville.syrjala
2016-10-12 11:58 ` Chris Wilson
@ 2016-10-12 13:49 ` Patchwork
2016-10-17 14:12 ` Ville Syrjälä
1 sibling, 1 reply; 6+ messages in thread
From: Patchwork @ 2016-10-12 13:49 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: GMBUS don't need no forcewake
URL : https://patchwork.freedesktop.org/series/13641/
State : warning
== Summary ==
Series 13641v1 drm/i915: GMBUS don't need no forcewake
https://patchwork.freedesktop.org/api/1.0/series/13641/revisions/1/mbox/
Test drv_module_reload_basic:
pass -> DMESG-WARN (fi-ilk-650)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
dmesg-warn -> PASS (fi-byt-j1900)
Test vgem_basic:
Subgroup unload:
skip -> PASS (fi-hsw-4770)
fi-bdw-5557u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16
fi-bsw-n3050 total:248 pass:205 dwarn:0 dfail:0 fail:0 skip:43
fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31
fi-byt-j1900 total:248 pass:214 dwarn:1 dfail:0 fail:1 skip:32
fi-byt-n2820 total:248 pass:211 dwarn:0 dfail:0 fail:1 skip:36
fi-hsw-4770 total:248 pass:225 dwarn:0 dfail:0 fail:0 skip:23
fi-hsw-4770r total:248 pass:225 dwarn:0 dfail:0 fail:0 skip:23
fi-ilk-650 total:248 pass:184 dwarn:1 dfail:0 fail:2 skip:61
fi-ivb-3520m total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26
fi-ivb-3770 total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26
fi-kbl-7200u total:248 pass:223 dwarn:0 dfail:0 fail:0 skip:25
fi-skl-6260u total:248 pass:233 dwarn:0 dfail:0 fail:0 skip:15
fi-skl-6700hq total:248 pass:225 dwarn:0 dfail:0 fail:0 skip:23
fi-skl-6700k total:248 pass:222 dwarn:1 dfail:0 fail:0 skip:25
fi-skl-6770hq total:248 pass:231 dwarn:1 dfail:0 fail:1 skip:15
fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37
fi-snb-2600 total:248 pass:210 dwarn:0 dfail:0 fail:0 skip:38
Results at /archive/results/CI_IGT_test/Patchwork_2685/
46271d41e30090d7fc996e8f5abde6a59f51038b drm-intel-nightly: 2016y-10m-12d-11h-06m-41s UTC integration manifest
3d9fd0b drm/i915: GMBUS don't need no forcewake
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ✗ Fi.CI.BAT: warning for drm/i915: GMBUS don't need no forcewake
2016-10-12 13:49 ` ✗ Fi.CI.BAT: warning for " Patchwork
@ 2016-10-17 14:12 ` Ville Syrjälä
0 siblings, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2016-10-17 14:12 UTC (permalink / raw)
To: intel-gfx
On Wed, Oct 12, 2016 at 01:49:20PM -0000, Patchwork wrote:
> == Series Details ==
>
> Series: drm/i915: GMBUS don't need no forcewake
> URL : https://patchwork.freedesktop.org/series/13641/
> State : warning
>
> == Summary ==
>
> Series 13641v1 drm/i915: GMBUS don't need no forcewake
> https://patchwork.freedesktop.org/api/1.0/series/13641/revisions/1/mbox/
>
> Test drv_module_reload_basic:
> pass -> DMESG-WARN (fi-ilk-650)
[ 44.582062] [drm:intel_enable_pipe [i915]] enabling pipe B
[ 44.582480] [drm:ironlake_fdi_link_train [i915]] FDI_RX_IIR 0x100
[ 44.582505] [drm:ironlake_fdi_link_train [i915]] FDI train 1 done.
[ 44.582684] [drm:ironlake_fdi_link_train [i915]] FDI_RX_IIR 0x600
[ 44.582708] [drm:ironlake_fdi_link_train [i915]] FDI train 2 done.
[ 44.582732] [drm:ironlake_fdi_link_train [i915]] FDI train done
[ 44.582758] [drm:intel_enable_shared_dpll [i915]] enable PCH DPLL B (active 2, on? 0) for crtc 30
[ 44.582781] [drm:intel_enable_shared_dpll [i915]] enabling PCH DPLL B
[ 44.584207] [drm:intel_dp_program_link_training_pattern [i915]] Using DP training pattern TPS1
[ 44.585526] [drm:intel_dp_set_signal_levels [i915]] Using signal levels 00000000
[ 44.585551] [drm:intel_dp_set_signal_levels [i915]] Using vswing level 0
[ 44.585574] [drm:intel_dp_set_signal_levels [i915]] Using pre-emphasis level 0
[ 44.585599] [drm:intel_dp_program_link_training_pattern [i915]] Using DP training pattern TPS1
[ 44.586310] [drm:intel_dp_start_link_train [i915]] clock recovery OK
[ 44.586344] [drm:intel_dp_program_link_training_pattern [i915]] Using DP training pattern TPS2
[ 44.587380] [drm:intel_dp_start_link_train [i915]] Channel EQ done. DP Training successful
[ 44.587587] [drm:intel_enable_dp [i915]] Enabling DP audio on pipe B
[ 44.587614] [drm:intel_audio_codec_enable [i915]] ELD on [CONNECTOR:40:DP-1], [ENCODER:39:DP C]
[ 44.587639] [drm:ilk_audio_codec_enable [i915]] Enable audio codec on port C, pipe B, 36 bytes ELD
[ 44.604682] [drm:intel_pch_fifo_underrun_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun
so this is the non-link retraining case, which could be audio related
(or not).
https://bugs.freedesktop.org/show_bug.cgi?id=98251
Patch pushe to dinq, thanks for the review.
> Test kms_pipe_crc_basic:
> Subgroup suspend-read-crc-pipe-a:
> dmesg-warn -> PASS (fi-byt-j1900)
> Test vgem_basic:
> Subgroup unload:
> skip -> PASS (fi-hsw-4770)
>
> fi-bdw-5557u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16
> fi-bsw-n3050 total:248 pass:205 dwarn:0 dfail:0 fail:0 skip:43
> fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31
> fi-byt-j1900 total:248 pass:214 dwarn:1 dfail:0 fail:1 skip:32
> fi-byt-n2820 total:248 pass:211 dwarn:0 dfail:0 fail:1 skip:36
> fi-hsw-4770 total:248 pass:225 dwarn:0 dfail:0 fail:0 skip:23
> fi-hsw-4770r total:248 pass:225 dwarn:0 dfail:0 fail:0 skip:23
> fi-ilk-650 total:248 pass:184 dwarn:1 dfail:0 fail:2 skip:61
> fi-ivb-3520m total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26
> fi-ivb-3770 total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26
> fi-kbl-7200u total:248 pass:223 dwarn:0 dfail:0 fail:0 skip:25
> fi-skl-6260u total:248 pass:233 dwarn:0 dfail:0 fail:0 skip:15
> fi-skl-6700hq total:248 pass:225 dwarn:0 dfail:0 fail:0 skip:23
> fi-skl-6700k total:248 pass:222 dwarn:1 dfail:0 fail:0 skip:25
> fi-skl-6770hq total:248 pass:231 dwarn:1 dfail:0 fail:1 skip:15
> fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37
> fi-snb-2600 total:248 pass:210 dwarn:0 dfail:0 fail:0 skip:38
>
> Results at /archive/results/CI_IGT_test/Patchwork_2685/
>
> 46271d41e30090d7fc996e8f5abde6a59f51038b drm-intel-nightly: 2016y-10m-12d-11h-06m-41s UTC integration manifest
> 3d9fd0b drm/i915: GMBUS don't need no forcewake
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-17 14:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-12 11:44 [PATCH] drm/i915: GMBUS don't need no forcewake ville.syrjala
2016-10-12 11:58 ` Chris Wilson
2016-10-12 12:39 ` Ville Syrjälä
2016-10-12 12:51 ` Chris Wilson
2016-10-12 13:49 ` ✗ Fi.CI.BAT: warning for " Patchwork
2016-10-17 14:12 ` Ville Syrjälä
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.