On Thu, Oct 20, 2016 at 05:29:36PM +0300, Mika Kuoppala wrote:
> Arkadiusz Hiler <arkadiusz.hiler@xxxxxxxxx> 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@xxxxxxxxx>
> > Cc: Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx>
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
