[PATCH] drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate

[PATCH] drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate

[Arkadiusz Hiler]

When invalidating RCS TLB the device can enter RC6 state interrupting
the process, therefore the need for render forcewake for the whole
procedure.

This WA is needed for all production SKL SKUs.

References: HSD#2136899, HSD#1404391274
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 drivers/gpu/drm/i915/gvt/render.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c
index f54ab85..f5000ea 100644
--- a/drivers/gpu/drm/i915/gvt/render.c
+++ b/drivers/gpu/drm/i915/gvt/render.c
@@ -134,11 +134,22 @@ static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
 
 	reg = _MMIO(regs[ring_id]);
 
+	/* WaForceWakeRenderDuringMmioTLBInvalidate:skl
+	 * we need to put a forcewake when invalidating RCS TLB caches,
+	 * otherwise device can go to RC6 state and interrupt invalidation
+	 * process */
+	if (IS_SKYLAKE(dev_priv) && ring_id == RCS)
+		intel_uncore_forcewake_get(dev_priv, FORCEWAKE_RENDER);
+
 	I915_WRITE(reg, 0x1);
 
 	if (wait_for_atomic((I915_READ(reg) == 0), 50))
 		gvt_err("timeout in invalidate ring (%d) tlb\n", ring_id);
 
+	if (IS_SKYLAKE(dev_priv) && ring_id == RCS)
+		intel_uncore_forcewake_put(dev_priv, FORCEWAKE_RENDER);
+
+
 	gvt_dbg_core("invalidate TLB for ring %d\n", ring_id);
 }
 
-- 
2.7.4

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

✗ Fi.CI.BAT: warning for drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate

[Patchwork]

== Series Details ==

Series: drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate
URL   : https://patchwork.freedesktop.org/series/14097/
State : warning

== Summary ==

Series 14097v1 drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate
https://patchwork.freedesktop.org/api/1.0/series/14097/revisions/1/mbox/

Test drv_module_reload_basic:
                dmesg-warn -> PASS       (fi-skl-6700hq)
Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> DMESG-WARN (fi-skl-6700hq)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-skl-6700hq)
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-skl-6700hq)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> DMESG-WARN (fi-skl-6700hq)

fi-bdw-5557u     total:246  pass:231  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:246  pass:204  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:246  pass:216  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-j1900     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-byt-n2820     total:246  pass:211  dwarn:0   dfail:0   fail:0   skip:35 
fi-hsw-4770      total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650       total:246  pass:185  dwarn:0   dfail:0   fail:1   skip:60 
fi-ivb-3520m     total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770      total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25 
fi-kbl-7200u     total:246  pass:222  dwarn:0   dfail:0   fail:0   skip:24 
fi-skl-6260u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:246  pass:219  dwarn:4   dfail:0   fail:0   skip:23 
fi-skl-6700k     total:246  pass:221  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hq    total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:246  pass:210  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:246  pass:209  dwarn:0   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2775/

0b12b48da3bc580102665ee9efd7e834f6def00f drm-intel-nightly: 2016y-10m-20d-07h-52m-31s UTC integration manifest
d3ad1f3f drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate

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

Re: ✗ Fi.CI.BAT: warning for drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate

[Arkadiusz Hiler]

On Thu, Oct 20, 2016 at 11:17:09AM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate
> URL   : https://patchwork.freedesktop.org/series/14097/
> State : warning
> 
> == Summary ==
> 
> Series 14097v1 drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate
> https://patchwork.freedesktop.org/api/1.0/series/14097/revisions/1/mbox/
> 
> Test drv_module_reload_basic:
>                 dmesg-warn -> PASS       (fi-skl-6700hq)
> Test gem_exec_suspend:
>         Subgroup basic-s3:
>                 pass       -> DMESG-WARN (fi-skl-6700hq)
> Test kms_pipe_crc_basic:
>         Subgroup suspend-read-crc-pipe-a:
>                 pass       -> DMESG-WARN (fi-skl-6700hq)
>         Subgroup suspend-read-crc-pipe-b:
>                 pass       -> DMESG-WARN (fi-skl-6700hq)
>         Subgroup suspend-read-crc-pipe-c:
>                 pass       -> DMESG-WARN (fi-skl-6700hq)

Seems to not be related to the change made.

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

Re: ✗ Fi.CI.BAT: warning for drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate

[Ville Syrjälä]

On Thu, Oct 20, 2016 at 01:52:32PM +0200, Arkadiusz Hiler wrote:
> On Thu, Oct 20, 2016 at 11:17:09AM +0000, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate
> > URL   : https://patchwork.freedesktop.org/series/14097/
> > State : warning
> > 
> > == Summary ==
> > 
> > Series 14097v1 drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate
> > https://patchwork.freedesktop.org/api/1.0/series/14097/revisions/1/mbox/
> > 
> > Test drv_module_reload_basic:
> >                 dmesg-warn -> PASS       (fi-skl-6700hq)
> > Test gem_exec_suspend:
> >         Subgroup basic-s3:
> >                 pass       -> DMESG-WARN (fi-skl-6700hq)
> > Test kms_pipe_crc_basic:
> >         Subgroup suspend-read-crc-pipe-a:
> >                 pass       -> DMESG-WARN (fi-skl-6700hq)
> >         Subgroup suspend-read-crc-pipe-b:
> >                 pass       -> DMESG-WARN (fi-skl-6700hq)
> >         Subgroup suspend-read-crc-pipe-c:
> >                 pass       -> DMESG-WARN (fi-skl-6700hq)
> 
> Seems to not be related to the change made.

Please quote a bit of the error message(s) so that others might judge
that well, without having to open up the results themselves.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: warning for drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate

[Arkadiusz Hiler]

On Thu, Oct 20, 2016 at 03:04:44PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 20, 2016 at 01:52:32PM +0200, Arkadiusz Hiler wrote:
> > On Thu, Oct 20, 2016 at 11:17:09AM +0000, Patchwork wrote:
> > > == Series Details ==
> > > 
> > > Series: drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate
> > > URL   : https://patchwork.freedesktop.org/series/14097/
> > > State : warning
> > > 
> > > == Summary ==
> > > 
> > > Series 14097v1 drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate
> > > https://patchwork.freedesktop.org/api/1.0/series/14097/revisions/1/mbox/
> > > 
> > > Test drv_module_reload_basic:
> > >                 dmesg-warn -> PASS       (fi-skl-6700hq)

[   36.853518] [drm:lspcon_init [i915]] *ERROR* Failed to probe lspcon
[   36.853598] [drm:intel_ddi_init [i915]] *ERROR* LSPCON init failed on port B
[   36.884272] [Firmware Bug]: ACPI(PEGP) defines _DOD but not _DOS
[   38.004158] Setting dangerous option inject_load_failure - tainting kernel
[   38.150495] Setting dangerous option inject_load_failure - tainting kernel
[   38.332735] Setting dangerous option inject_load_failure - tainting kernel
[   38.514716] Setting dangerous option inject_load_failure - tainting kernel
[   39.600354] [drm:lspcon_init [i915]] *ERROR* Failed to probe lspcon
[   39.600434] [drm:intel_ddi_init [i915]] *ERROR* LSPCON init failed on port B
[   39.629926] [Firmware Bug]: ACPI(PEGP) defines _DOD but not _DOS

> > > Test gem_exec_suspend:
> > >         Subgroup basic-s3:
> > >                 pass       -> DMESG-WARN (fi-skl-6700hq)


[  207.413322] done.
[  207.436017] Suspending console(s) (use no_console_suspend to debug)
[  207.440188] sd 2:0:0:0: [sda] Synchronizing SCSI cache
[  207.440837] sd 2:0:0:0: [sda] Stopping disk
[  209.114203] Broke affinity for irq 124
                {  snip, for different irqs }
[  209.163837]  cache: parent cpu1 should not be sleeping
                {  snip, for all CPUs }
[  209.248422] ACPI : button: The lid device is not compliant to SW_LID.
[  209.344047] sd 2:0:0:0: [sda] Starting disk
[  209.518719] [drm:drm_lspcon_get_mode] *ERROR* LSPCON read(0x80, 0x41) failed
[  209.518733] [drm:lspcon_change_mode.constprop.2 [i915]] *ERROR* Error reading LSPCON mode
[  209.518744] [drm:lspcon_resume [i915]] *ERROR* LSPCON resume failed
[  209.994495] done.

> > > Test kms_pipe_crc_basic:
> > >         Subgroup suspend-read-crc-pipe-a:
> > >                 pass       -> DMESG-WARN (fi-skl-6700hq)
> > >         Subgroup suspend-read-crc-pipe-b:
> > >                 pass       -> DMESG-WARN (fi-skl-6700hq)
> > >         Subgroup suspend-read-crc-pipe-c:
> > >                 pass       -> DMESG-WARN (fi-skl-6700hq)

all three had the exact same as the one lspcon-related above

> > 
> > Seems to not be related to the change made.
> 
> Please quote a bit of the error message(s) so that others might judge
> that well, without having to open up the results themselves.

I'll remember to do that from now on. Thanks.

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

Re: [PATCH] drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate

[Mika Kuoppala]

Arkadiusz Hiler <arkadiusz.hiler@intel.com> writes:

> When invalidating RCS TLB the device can enter RC6 state interrupting
> the process, therefore the need for render forcewake for the whole
> procedure.
>
> This WA is needed for all production SKL SKUs.
>
> References: HSD#2136899, HSD#1404391274
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/render.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c
> index f54ab85..f5000ea 100644
> --- a/drivers/gpu/drm/i915/gvt/render.c
> +++ b/drivers/gpu/drm/i915/gvt/render.c
> @@ -134,11 +134,22 @@ static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
>  
>  	reg = _MMIO(regs[ring_id]);
>

Ok not so familiar with the gvt side but I assume this is the host
side code and thus the vgpu is not active at this stage.

Then you could avoid some of the implicit fw dancing
by:

diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c
index feebb65..93ba156 100644
--- a/drivers/gpu/drm/i915/gvt/render.c
+++ b/drivers/gpu/drm/i915/gvt/render.c
@@ -118,6 +118,7 @@ static u32 gen9_render_mocs_L3[32];
 static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
 {
        struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
+       enum forcewake_domains fw;
        i915_reg_t reg;
        u32 regs[] = {
                [RCS] = 0x4260,
@@ -135,11 +136,21 @@ static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
 
        reg = _MMIO(regs[ring_id]);
 
-       I915_WRITE(reg, 0x1);
+       fw = intel_uncore_forcewake_for_reg(dev_priv, reg,
+                                           FW_REG_READ | FW_REG_WRITE);
 
-       if (wait_for_atomic((I915_READ(reg) == 0), 50))
+       if (ring_id == RCS && IS_SKYLAKE(dev_priv))
+               fw |= FORCEWAKE_RENDER;
+
+       intel_uncore_forcewake_get(dev_priv, fw);
+
+       I915_WRITE_FW(reg, 0x1);
+
+       if (wait_for_atomic((I915_READ_FW(reg) == 0), 50))
                gvt_err("timeout in invalidate ring (%d) tlb\n", ring_id);
 
+       intel_uncore_forcewake_put(dev_priv, fw);
+

-Mika

> +	/* WaForceWakeRenderDuringMmioTLBInvalidate:skl
> +	 * we need to put a forcewake when invalidating RCS TLB caches,
> +	 * otherwise device can go to RC6 state and interrupt invalidation
> +	 * process */
> +	if (IS_SKYLAKE(dev_priv) && ring_id == RCS)
> +		intel_uncore_forcewake_get(dev_priv, FORCEWAKE_RENDER);
> +
>  	I915_WRITE(reg, 0x1);
>  
>  	if (wait_for_atomic((I915_READ(reg) == 0), 50))
>  		gvt_err("timeout in invalidate ring (%d) tlb\n", ring_id);
>  
> +	if (IS_SKYLAKE(dev_priv) && ring_id == RCS)
> +		intel_uncore_forcewake_put(dev_priv, FORCEWAKE_RENDER);
> +
> +
>  	gvt_dbg_core("invalidate TLB for ring %d\n", ring_id);
>  }
>  
> -- 
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate

[Ville Syrjälä]

On Thu, Oct 20, 2016 at 12:57:31PM +0200, Arkadiusz Hiler wrote:
> When invalidating RCS TLB the device can enter RC6 state interrupting
> the process, therefore the need for render forcewake for the whole
> procedure.
> 
> This WA is needed for all production SKL SKUs.
> 
> References: HSD#2136899, HSD#1404391274
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/render.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c
> index f54ab85..f5000ea 100644
> --- a/drivers/gpu/drm/i915/gvt/render.c
> +++ b/drivers/gpu/drm/i915/gvt/render.c
> @@ -134,11 +134,22 @@ static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
>  
>  	reg = _MMIO(regs[ring_id]);

Random drive by comment:
You should add the registers to i915_reg.h properly so that we don't get
this ugly _MMIO() stuff sprinkled all over the place.

>  
> +	/* WaForceWakeRenderDuringMmioTLBInvalidate:skl
> +	 * we need to put a forcewake when invalidating RCS TLB caches,
> +	 * otherwise device can go to RC6 state and interrupt invalidation
> +	 * process */
> +	if (IS_SKYLAKE(dev_priv) && ring_id == RCS)
> +		intel_uncore_forcewake_get(dev_priv, FORCEWAKE_RENDER);
> +
>  	I915_WRITE(reg, 0x1);
>  
>  	if (wait_for_atomic((I915_READ(reg) == 0), 50))
>  		gvt_err("timeout in invalidate ring (%d) tlb\n", ring_id);
>  
> +	if (IS_SKYLAKE(dev_priv) && ring_id == RCS)
> +		intel_uncore_forcewake_put(dev_priv, FORCEWAKE_RENDER);
> +
> +
>  	gvt_dbg_core("invalidate TLB for ring %d\n", ring_id);
>  }
>  
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate

[Arkadiusz Hiler]

On Thu, Oct 20, 2016 at 05:29:36PM +0300, Mika Kuoppala wrote:
> Arkadiusz Hiler <arkadiusz.hiler@intel.com> writes:
> 
> > When invalidating RCS TLB the device can enter RC6 state interrupting
> > the process, therefore the need for render forcewake for the whole
> > procedure.
> >
> > This WA is needed for all production SKL SKUs.
> >
> > References: HSD#2136899, HSD#1404391274
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/render.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c
> > index f54ab85..f5000ea 100644
> > --- a/drivers/gpu/drm/i915/gvt/render.c
> > +++ b/drivers/gpu/drm/i915/gvt/render.c
> > @@ -134,11 +134,22 @@ static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
> >  
> >  	reg = _MMIO(regs[ring_id]);
> >
> 
> Ok not so familiar with the gvt side but I assume this is the host
> side code and thus the vgpu is not active at this stage.

That's my understanding as well. It's a code that is setting up gvt for
further use (shadow context to be exact). It's called indirectly from
intel_gvt_create_vgpu.

We should wait for Zhenyu to verify that.

> Then you could avoid some of the implicit fw dancing
> by:
> 
> diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c
> index feebb65..93ba156 100644
> --- a/drivers/gpu/drm/i915/gvt/render.c
> +++ b/drivers/gpu/drm/i915/gvt/render.c
> @@ -118,6 +118,7 @@ static u32 gen9_render_mocs_L3[32];
>  static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
>  {
>         struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> +       enum forcewake_domains fw;
>         i915_reg_t reg;
>         u32 regs[] = {
>                 [RCS] = 0x4260,
> @@ -135,11 +136,21 @@ static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
>  
>         reg = _MMIO(regs[ring_id]);
>  
> -       I915_WRITE(reg, 0x1);
> +       fw = intel_uncore_forcewake_for_reg(dev_priv, reg,
> +                                           FW_REG_READ | FW_REG_WRITE);
>  
> -       if (wait_for_atomic((I915_READ(reg) == 0), 50))
> +       if (ring_id == RCS && IS_SKYLAKE(dev_priv))
> +               fw |= FORCEWAKE_RENDER;
> +
> +       intel_uncore_forcewake_get(dev_priv, fw);
> +
> +       I915_WRITE_FW(reg, 0x1);
> +
> +       if (wait_for_atomic((I915_READ_FW(reg) == 0), 50))
>                 gvt_err("timeout in invalidate ring (%d) tlb\n", ring_id);
>  
> +       intel_uncore_forcewake_put(dev_priv, fw);
> +
> 

I can go with it, although I do not have strong preference. I think my
version is a little bit easier to follow, but his is less error prone,
as you check for the WA SKU only once, during setting the FW.

Any recommendations?

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

Re: [PATCH] drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 3477 bytes --]

On 2016.10.20 17:20:04 +0200, Arkadiusz Hiler wrote:
> On Thu, Oct 20, 2016 at 05:29:36PM +0300, Mika Kuoppala wrote:
> > Arkadiusz Hiler <arkadiusz.hiler@intel.com> writes:
> > 
> > > When invalidating RCS TLB the device can enter RC6 state interrupting
> > > the process, therefore the need for render forcewake for the whole
> > > procedure.
> > >
> > > This WA is needed for all production SKL SKUs.
> > >
> > > References: HSD#2136899, HSD#1404391274
> > > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/gvt/render.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c
> > > index f54ab85..f5000ea 100644
> > > --- a/drivers/gpu/drm/i915/gvt/render.c
> > > +++ b/drivers/gpu/drm/i915/gvt/render.c
> > > @@ -134,11 +134,22 @@ static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
> > >  
> > >  	reg = _MMIO(regs[ring_id]);
> > >
> > 
> > Ok not so familiar with the gvt side but I assume this is the host
> > side code and thus the vgpu is not active at this stage.
> 
> That's my understanding as well. It's a code that is setting up gvt for
> further use (shadow context to be exact). It's called indirectly from
> intel_gvt_create_vgpu.
> 

yes, it's for host not for vgpu to handle context switch state for vgpu.

> > Then you could avoid some of the implicit fw dancing
> > by:
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c
> > index feebb65..93ba156 100644
> > --- a/drivers/gpu/drm/i915/gvt/render.c
> > +++ b/drivers/gpu/drm/i915/gvt/render.c
> > @@ -118,6 +118,7 @@ static u32 gen9_render_mocs_L3[32];
> >  static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
> >  {
> >         struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> > +       enum forcewake_domains fw;
> >         i915_reg_t reg;
> >         u32 regs[] = {
> >                 [RCS] = 0x4260,
> > @@ -135,11 +136,21 @@ static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
> >  
> >         reg = _MMIO(regs[ring_id]);
> >  
> > -       I915_WRITE(reg, 0x1);
> > +       fw = intel_uncore_forcewake_for_reg(dev_priv, reg,
> > +                                           FW_REG_READ | FW_REG_WRITE);
> >  
> > -       if (wait_for_atomic((I915_READ(reg) == 0), 50))
> > +       if (ring_id == RCS && IS_SKYLAKE(dev_priv))
> > +               fw |= FORCEWAKE_RENDER;
> > +
> > +       intel_uncore_forcewake_get(dev_priv, fw);
> > +
> > +       I915_WRITE_FW(reg, 0x1);
> > +
> > +       if (wait_for_atomic((I915_READ_FW(reg) == 0), 50))
> >                 gvt_err("timeout in invalidate ring (%d) tlb\n", ring_id);
> >  
> > +       intel_uncore_forcewake_put(dev_priv, fw);
> > +
> > 
> 
> I can go with it, although I do not have strong preference. I think my
> version is a little bit easier to follow, but his is less error prone,
> as you check for the WA SKU only once, during setting the FW.
> 
> Any recommendations?
> 

I like this one to be safer. Would Mika like to send another one or I
just take your commit message?

thanks

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 163 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[PATCH v2] drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate

[Arkadiusz Hiler]

When invalidating RCS TLB the device can enter RC6 state interrupting
the process, therefore the need for render forcewake for the whole
procedure.

This WA is needed for all production SKL SKUs.

v2: reworked putting and getting forcewake with help of Mika Kuoppala

References: HSD#2136899, HSD#1404391274
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 drivers/gpu/drm/i915/gvt/render.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c
index feebb65..8e7a80e 100644
--- a/drivers/gpu/drm/i915/gvt/render.c
+++ b/drivers/gpu/drm/i915/gvt/render.c
@@ -118,6 +118,7 @@ static u32 gen9_render_mocs_L3[32];
 static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
 {
 	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
+	enum forcewake_domains fw;
 	i915_reg_t reg;
 	u32 regs[] = {
 		[RCS] = 0x4260,
@@ -135,11 +136,25 @@ static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
 
 	reg = _MMIO(regs[ring_id]);
 
+	/* WaForceWakeRenderDuringMmioTLBInvalidate:skl
+	 * we need to put a forcewake when invalidating RCS TLB caches,
+	 * otherwise device can go to RC6 state and interrupt invalidation
+	 * process
+	 */
+	fw = intel_uncore_forcewake_for_reg(dev_priv, reg,
+					    FW_REG_READ | FW_REG_WRITE);
+	if (ring_id == RCS && IS_SKYLAKE(dev_priv))
+		fw |= FORCEWAKE_RENDER;
+
+	intel_uncore_forcewake_get(dev_priv, fw);
+
 	I915_WRITE(reg, 0x1);
 
 	if (wait_for_atomic((I915_READ(reg) == 0), 50))
 		gvt_err("timeout in invalidate ring (%d) tlb\n", ring_id);
 
+	intel_uncore_forcewake_put(dev_priv, fw);
+
 	gvt_dbg_core("invalidate TLB for ring %d\n", ring_id);
 }
 
-- 
2.7.4

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

Re: [PATCH v2] drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate

[Mika Kuoppala]

Arkadiusz Hiler <arkadiusz.hiler@intel.com> writes:

> When invalidating RCS TLB the device can enter RC6 state interrupting
> the process, therefore the need for render forcewake for the whole
> procedure.
>
> This WA is needed for all production SKL SKUs.
>
> v2: reworked putting and getting forcewake with help of Mika Kuoppala
>
> References: HSD#2136899, HSD#1404391274
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/render.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c
> index feebb65..8e7a80e 100644
> --- a/drivers/gpu/drm/i915/gvt/render.c
> +++ b/drivers/gpu/drm/i915/gvt/render.c
> @@ -118,6 +118,7 @@ static u32 gen9_render_mocs_L3[32];
>  static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
>  {
>  	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> +	enum forcewake_domains fw;
>  	i915_reg_t reg;
>  	u32 regs[] = {
>  		[RCS] = 0x4260,
> @@ -135,11 +136,25 @@ static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
>  
>  	reg = _MMIO(regs[ring_id]);
>  
> +	/* WaForceWakeRenderDuringMmioTLBInvalidate:skl
> +	 * we need to put a forcewake when invalidating RCS TLB caches,
> +	 * otherwise device can go to RC6 state and interrupt invalidation
> +	 * process
> +	 */
> +	fw = intel_uncore_forcewake_for_reg(dev_priv, reg,
> +					    FW_REG_READ | FW_REG_WRITE);
> +	if (ring_id == RCS && IS_SKYLAKE(dev_priv))
> +		fw |= FORCEWAKE_RENDER;
> +
> +	intel_uncore_forcewake_get(dev_priv, fw);
> +
>  	I915_WRITE(reg, 0x1);
>

To take full advantage of the explicit forcewake dance, please
look at I915_WRITE_FW and I915_READ_FW.

-Mika


>  	if (wait_for_atomic((I915_READ(reg) == 0), 50))
>  		gvt_err("timeout in invalidate ring (%d) tlb\n", ring_id);
>  
> +	intel_uncore_forcewake_put(dev_priv, fw);
> +
>  	gvt_dbg_core("invalidate TLB for ring %d\n", ring_id);
>  }
>  
> -- 
> 2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3] drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate

[Arkadiusz Hiler]

When invalidating RCS TLB the device can enter RC6 state interrupting
the process, therefore the need for render forcewake for the whole
procedure.

This WA is needed for all production SKL SKUs.

v2: reworked putting and getting forcewake with help of Mika Kuoppala
v3: use I915_READ_FW and I915_WRITE_FW as we are handling forcewake on
    in the code path

References: HSD#2136899, HSD#1404391274
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 drivers/gpu/drm/i915/gvt/render.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c
index feebb65..be1a7df 100644
--- a/drivers/gpu/drm/i915/gvt/render.c
+++ b/drivers/gpu/drm/i915/gvt/render.c
@@ -118,6 +118,7 @@ static u32 gen9_render_mocs_L3[32];
 static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
 {
 	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
+	enum forcewake_domains fw;
 	i915_reg_t reg;
 	u32 regs[] = {
 		[RCS] = 0x4260,
@@ -135,11 +136,25 @@ static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
 
 	reg = _MMIO(regs[ring_id]);
 
-	I915_WRITE(reg, 0x1);
+	/* WaForceWakeRenderDuringMmioTLBInvalidate:skl
+	 * we need to put a forcewake when invalidating RCS TLB caches,
+	 * otherwise device can go to RC6 state and interrupt invalidation
+	 * process
+	 */
+	fw = intel_uncore_forcewake_for_reg(dev_priv, reg,
+					    FW_REG_READ | FW_REG_WRITE);
+	if (ring_id == RCS && IS_SKYLAKE(dev_priv))
+		fw |= FORCEWAKE_RENDER;
 
-	if (wait_for_atomic((I915_READ(reg) == 0), 50))
+	intel_uncore_forcewake_get(dev_priv, fw);
+
+	I915_WRITE_FW(reg, 0x1);
+
+	if (wait_for_atomic((I915_READ_FW(reg) == 0), 50))
 		gvt_err("timeout in invalidate ring (%d) tlb\n", ring_id);
 
+	intel_uncore_forcewake_put(dev_priv, fw);
+
 	gvt_dbg_core("invalidate TLB for ring %d\n", ring_id);
 }
 
-- 
2.7.4

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

✗ Fi.CI.BAT: warning for drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate (rev3)
URL   : https://patchwork.freedesktop.org/series/14097/
State : warning

== Summary ==

Series 14097v3 drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate
https://patchwork.freedesktop.org/api/1.0/series/14097/revisions/3/mbox/

Test drv_module_reload_basic:
                dmesg-warn -> PASS       (fi-skl-6700hq)
Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> DMESG-WARN (fi-skl-6700hq)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-skl-6700hq)
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-skl-6700hq)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> DMESG-WARN (fi-skl-6700hq)

fi-bdw-5557u     total:246  pass:231  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:246  pass:204  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:246  pass:216  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-j1900     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-byt-n2820     total:246  pass:211  dwarn:0   dfail:0   fail:0   skip:35 
fi-hsw-4770      total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650       total:246  pass:185  dwarn:0   dfail:0   fail:1   skip:60 
fi-ivb-3520m     total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770      total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25 
fi-kbl-7200u     total:246  pass:222  dwarn:0   dfail:0   fail:0   skip:24 
fi-skl-6260u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:246  pass:219  dwarn:4   dfail:0   fail:0   skip:23 
fi-skl-6700k     total:246  pass:221  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hq    total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:246  pass:210  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:246  pass:209  dwarn:0   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2785/

8d34fff02efad9abfc12cace4c347eaa1c3804f7 drm-intel-nightly: 2016y-10m-20d-21h-52m-44s UTC integration manifest
75aa3e3 drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate

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

✓ Fi.CI.BAT: success for drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate (rev4)
URL   : https://patchwork.freedesktop.org/series/14097/
State : success

== Summary ==

Series 14097v4 drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate
https://patchwork.freedesktop.org/api/1.0/series/14097/revisions/4/mbox/


fi-bdw-5557u     total:246  pass:231  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:246  pass:204  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:246  pass:216  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-j1900     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-byt-n2820     total:246  pass:211  dwarn:0   dfail:0   fail:0   skip:35 
fi-hsw-4770      total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650       total:246  pass:185  dwarn:0   dfail:0   fail:1   skip:60 
fi-ivb-3520m     total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770      total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25 
fi-kbl-7200u     total:246  pass:222  dwarn:0   dfail:0   fail:0   skip:24 
fi-skl-6260u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:246  pass:222  dwarn:1   dfail:0   fail:0   skip:23 
fi-skl-6700k     total:246  pass:221  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hq    total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:246  pass:210  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:246  pass:209  dwarn:0   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2786/

8d34fff02efad9abfc12cace4c347eaa1c3804f7 drm-intel-nightly: 2016y-10m-20d-21h-52m-44s UTC integration manifest
081e931 drm/i915/gvt: Implement WaForceWakeRenderDuringMmioTLBInvalidate

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