* [PATCH] drm/i915/vlv: Take forcewake on media engine writes
@ 2015-12-17 15:14 Mika Kuoppala
2015-12-17 15:26 ` Jesse Barnes
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Mika Kuoppala @ 2015-12-17 15:14 UTC (permalink / raw)
To: intel-gfx; +Cc: Deepak S
Since commit 940aece471bd ("drm/i915/vlv: Valleyview support
for forcewake Individual power wells.") we have only taken
media engine forcewake correctly on reads, but only taken render
engine forcewake on media engine writes and omitted the media
domain.
This asymmetry might have caused unstable behaviour on
media ring access.
Fix is to take media engine forcewake symmetrically to writes.
References: https://bugs.freedesktop.org/show_bug.cgi?id=88012
Cc: Deepak S <deepak.s@intel.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/intel_uncore.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 277e60ae0e47..a2e204088aa5 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -902,6 +902,23 @@ hsw_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool t
GEN6_WRITE_FOOTER; \
}
+#define __vlv_write(x) \
+static void \
+vlv_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \
+ enum forcewake_domains fw_engine = 0; \
+ GEN6_WRITE_HEADER; \
+ if (!NEEDS_FORCE_WAKE(offset)) \
+ fw_engine = 0; \
+ else if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(offset)) \
+ fw_engine = FORCEWAKE_RENDER; \
+ else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(offset)) \
+ fw_engine = FORCEWAKE_MEDIA; \
+ if (fw_engine) \
+ __force_wake_get(dev_priv, fw_engine); \
+ __raw_i915_write##x(dev_priv, reg, val); \
+ GEN6_WRITE_FOOTER; \
+}
+
static const i915_reg_t gen8_shadowed_regs[] = {
FORCEWAKE_MT,
GEN6_RPNSWREQ,
@@ -1019,6 +1036,10 @@ __gen8_write(8)
__gen8_write(16)
__gen8_write(32)
__gen8_write(64)
+__vlv_write(8)
+__vlv_write(16)
+__vlv_write(32)
+__vlv_write(64)
__hsw_write(8)
__hsw_write(16)
__hsw_write(32)
@@ -1031,6 +1052,7 @@ __gen6_write(64)
#undef __gen9_write
#undef __chv_write
#undef __gen8_write
+#undef __vlv_write
#undef __hsw_write
#undef __gen6_write
#undef GEN6_WRITE_FOOTER
@@ -1243,6 +1265,8 @@ void intel_uncore_init(struct drm_device *dev)
case 6:
if (IS_HASWELL(dev)) {
ASSIGN_WRITE_MMIO_VFUNCS(hsw);
+ } else if (IS_VALLEYVIEW(dev)) {
+ ASSIGN_WRITE_MMIO_VFUNCS(vlv);
} else {
ASSIGN_WRITE_MMIO_VFUNCS(gen6);
}
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] drm/i915/vlv: Take forcewake on media engine writes
2015-12-17 15:14 [PATCH] drm/i915/vlv: Take forcewake on media engine writes Mika Kuoppala
@ 2015-12-17 15:26 ` Jesse Barnes
2015-12-17 15:39 ` Ville Syrjälä
2015-12-17 16:20 ` ✗ warning: Fi.CI.BAT Patchwork
2 siblings, 0 replies; 5+ messages in thread
From: Jesse Barnes @ 2015-12-17 15:26 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx; +Cc: Deepak S
On 12/17/2015 07:14 AM, Mika Kuoppala wrote:
> Since commit 940aece471bd ("drm/i915/vlv: Valleyview support
> for forcewake Individual power wells.") we have only taken
> media engine forcewake correctly on reads, but only taken render
> engine forcewake on media engine writes and omitted the media
> domain.
>
> This asymmetry might have caused unstable behaviour on
> media ring access.
>
> Fix is to take media engine forcewake symmetrically to writes.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=88012
> Cc: Deepak S <deepak.s@intel.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
> drivers/gpu/drm/i915/intel_uncore.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 277e60ae0e47..a2e204088aa5 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -902,6 +902,23 @@ hsw_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool t
> GEN6_WRITE_FOOTER; \
> }
>
> +#define __vlv_write(x) \
> +static void \
> +vlv_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \
> + enum forcewake_domains fw_engine = 0; \
> + GEN6_WRITE_HEADER; \
> + if (!NEEDS_FORCE_WAKE(offset)) \
> + fw_engine = 0; \
> + else if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(offset)) \
> + fw_engine = FORCEWAKE_RENDER; \
> + else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(offset)) \
> + fw_engine = FORCEWAKE_MEDIA; \
> + if (fw_engine) \
> + __force_wake_get(dev_priv, fw_engine); \
> + __raw_i915_write##x(dev_priv, reg, val); \
> + GEN6_WRITE_FOOTER; \
> +}
> +
> static const i915_reg_t gen8_shadowed_regs[] = {
> FORCEWAKE_MT,
> GEN6_RPNSWREQ,
> @@ -1019,6 +1036,10 @@ __gen8_write(8)
> __gen8_write(16)
> __gen8_write(32)
> __gen8_write(64)
> +__vlv_write(8)
> +__vlv_write(16)
> +__vlv_write(32)
> +__vlv_write(64)
> __hsw_write(8)
> __hsw_write(16)
> __hsw_write(32)
> @@ -1031,6 +1052,7 @@ __gen6_write(64)
> #undef __gen9_write
> #undef __chv_write
> #undef __gen8_write
> +#undef __vlv_write
> #undef __hsw_write
> #undef __gen6_write
> #undef GEN6_WRITE_FOOTER
> @@ -1243,6 +1265,8 @@ void intel_uncore_init(struct drm_device *dev)
> case 6:
> if (IS_HASWELL(dev)) {
> ASSIGN_WRITE_MMIO_VFUNCS(hsw);
> + } else if (IS_VALLEYVIEW(dev)) {
> + ASSIGN_WRITE_MMIO_VFUNCS(vlv);
> } else {
> ASSIGN_WRITE_MMIO_VFUNCS(gen6);
> }
>
Looks good. Looks like we also have it on chv, so I guess it was just
an oversight.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm/i915/vlv: Take forcewake on media engine writes
2015-12-17 15:14 [PATCH] drm/i915/vlv: Take forcewake on media engine writes Mika Kuoppala
2015-12-17 15:26 ` Jesse Barnes
@ 2015-12-17 15:39 ` Ville Syrjälä
2015-12-17 15:43 ` Chris Wilson
2015-12-17 16:20 ` ✗ warning: Fi.CI.BAT Patchwork
2 siblings, 1 reply; 5+ messages in thread
From: Ville Syrjälä @ 2015-12-17 15:39 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: Deepak S, intel-gfx
On Thu, Dec 17, 2015 at 05:14:54PM +0200, Mika Kuoppala wrote:
> Since commit 940aece471bd ("drm/i915/vlv: Valleyview support
> for forcewake Individual power wells.") we have only taken
> media engine forcewake correctly on reads, but only taken render
> engine forcewake on media engine writes and omitted the media
> domain.
>
> This asymmetry might have caused unstable behaviour on
> media ring access.
>
> Fix is to take media engine forcewake symmetrically to writes.
We don't take any forcewake on writes pre-gen8. That's what the
wake FIFO is for.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=88012
> Cc: Deepak S <deepak.s@intel.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
> drivers/gpu/drm/i915/intel_uncore.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 277e60ae0e47..a2e204088aa5 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -902,6 +902,23 @@ hsw_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool t
> GEN6_WRITE_FOOTER; \
> }
>
> +#define __vlv_write(x) \
> +static void \
> +vlv_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \
> + enum forcewake_domains fw_engine = 0; \
> + GEN6_WRITE_HEADER; \
> + if (!NEEDS_FORCE_WAKE(offset)) \
> + fw_engine = 0; \
> + else if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(offset)) \
> + fw_engine = FORCEWAKE_RENDER; \
> + else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(offset)) \
> + fw_engine = FORCEWAKE_MEDIA; \
> + if (fw_engine) \
> + __force_wake_get(dev_priv, fw_engine); \
> + __raw_i915_write##x(dev_priv, reg, val); \
> + GEN6_WRITE_FOOTER; \
> +}
> +
> static const i915_reg_t gen8_shadowed_regs[] = {
> FORCEWAKE_MT,
> GEN6_RPNSWREQ,
> @@ -1019,6 +1036,10 @@ __gen8_write(8)
> __gen8_write(16)
> __gen8_write(32)
> __gen8_write(64)
> +__vlv_write(8)
> +__vlv_write(16)
> +__vlv_write(32)
> +__vlv_write(64)
> __hsw_write(8)
> __hsw_write(16)
> __hsw_write(32)
> @@ -1031,6 +1052,7 @@ __gen6_write(64)
> #undef __gen9_write
> #undef __chv_write
> #undef __gen8_write
> +#undef __vlv_write
> #undef __hsw_write
> #undef __gen6_write
> #undef GEN6_WRITE_FOOTER
> @@ -1243,6 +1265,8 @@ void intel_uncore_init(struct drm_device *dev)
> case 6:
> if (IS_HASWELL(dev)) {
> ASSIGN_WRITE_MMIO_VFUNCS(hsw);
> + } else if (IS_VALLEYVIEW(dev)) {
> + ASSIGN_WRITE_MMIO_VFUNCS(vlv);
> } else {
> ASSIGN_WRITE_MMIO_VFUNCS(gen6);
> }
> --
> 2.5.0
>
> _______________________________________________
> 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] 5+ messages in thread* Re: [PATCH] drm/i915/vlv: Take forcewake on media engine writes
2015-12-17 15:39 ` Ville Syrjälä
@ 2015-12-17 15:43 ` Chris Wilson
0 siblings, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2015-12-17 15:43 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Deepak S, intel-gfx
On Thu, Dec 17, 2015 at 05:39:39PM +0200, Ville Syrjälä wrote:
> On Thu, Dec 17, 2015 at 05:14:54PM +0200, Mika Kuoppala wrote:
> > Since commit 940aece471bd ("drm/i915/vlv: Valleyview support
> > for forcewake Individual power wells.") we have only taken
> > media engine forcewake correctly on reads, but only taken render
> > engine forcewake on media engine writes and omitted the media
> > domain.
> >
> > This asymmetry might have caused unstable behaviour on
> > media ring access.
> >
> > Fix is to take media engine forcewake symmetrically to writes.
>
> We don't take any forcewake on writes pre-gen8. That's what the
> wake FIFO is for.
On the other hand, if something magically starts working, I think we can
conclude that our FIFO handling or the FIFO is broken. One or the other.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* ✗ warning: Fi.CI.BAT
2015-12-17 15:14 [PATCH] drm/i915/vlv: Take forcewake on media engine writes Mika Kuoppala
2015-12-17 15:26 ` Jesse Barnes
2015-12-17 15:39 ` Ville Syrjälä
@ 2015-12-17 16:20 ` Patchwork
2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2015-12-17 16:20 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
== Summary ==
Built on 8463389e40c3815b2e9b052f34145b1d728975be drm-intel-nightly: 2015y-12m-17d-14h-39m-21s UTC integration manifest
Test igt@kms_flip@basic-flip-vs-wf_vblank on snb-x220t dmesg-warn -> pass
Test igt@kms_flip@basic-flip-vs-wf_vblank on bsw-nuc-2 pass -> dmesg-warn
Test igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence on bdw-nuci7 dmesg-warn -> pass
Test igt@kms_pipe_crc_basic@hang-read-crc-pipe-a on snb-x220t pass -> dmesg-warn
Test igt@kms_flip@basic-plain-flip on bdw-nuci7 pass -> dmesg-warn
Test igt@kms_pipe_crc_basic@read-crc-pipe-a on snb-x220t dmesg-warn -> pass
Test igt@kms_pipe_crc_basic@read-crc-pipe-a on skl-i7k-2 pass -> dmesg-warn
Test igt@kms_pipe_crc_basic@read-crc-pipe-c on skl-i5k-2 dmesg-warn -> pass
Test igt@kms_pipe_crc_basic@read-crc-pipe-b on snb-dellxps dmesg-warn -> pass
Test igt@kms_setmode@basic-clone-single-crtc on snb-dellxps pass -> dmesg-warn
Test igt@kms_flip@basic-flip-vs-modeset on bsw-nuc-2 pass -> dmesg-warn
bdw-nuci7 total:135 pass:124 dwarn:2 dfail:0 fail:0 skip:9
bsw-nuc-2 total:135 pass:113 dwarn:2 dfail:0 fail:0 skip:20
hsw-brixbox total:135 pass:127 dwarn:1 dfail:0 fail:0 skip:7
hsw-gt2 total:135 pass:130 dwarn:1 dfail:0 fail:0 skip:4
ilk-hp8440p total:135 pass:100 dwarn:0 dfail:0 fail:2 skip:33
ivb-t430s total:135 pass:128 dwarn:1 dfail:1 fail:1 skip:4
skl-i5k-2 total:135 pass:123 dwarn:4 dfail:0 fail:0 skip:8
skl-i7k-2 total:135 pass:122 dwarn:5 dfail:0 fail:0 skip:8
snb-dellxps total:135 pass:121 dwarn:2 dfail:0 fail:0 skip:12
snb-x220t total:135 pass:122 dwarn:1 dfail:0 fail:1 skip:11
Results at /archive/results/CI_IGT_test/Patchwork_703/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-12-17 16:20 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-17 15:14 [PATCH] drm/i915/vlv: Take forcewake on media engine writes Mika Kuoppala
2015-12-17 15:26 ` Jesse Barnes
2015-12-17 15:39 ` Ville Syrjälä
2015-12-17 15:43 ` Chris Wilson
2015-12-17 16:20 ` ✗ warning: Fi.CI.BAT Patchwork
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.