[Intel-gfx] [PATCH v5 4/8] drm/i915: vgpu context submission pv optimization

[Chris Wilson]

Quoting Xiaolin Zhang (2019-04-29 04:10:54)
> +static void pv_submit(struct intel_engine_cs *engine)
> +{
> +       struct intel_engine_execlists * const execlists = &engine->execlists;
> +       struct execlist_port *port = execlists->port;
> +       unsigned int n;
> +       struct gvt_shared_page *shared_page = engine->i915->vgpu.shared_page;
> +       u64 descs[2];
> +
> +       for (n = 0; n < execlists_num_ports(execlists); n++) {
> +               struct i915_request *rq;
> +               unsigned int count = 0;
> +
> +               descs[n] = 0;
> +               rq = port_unpack(&port[n], &count);
> +               if (rq && count == 0) {
> +                       port_set(&port[n], port_pack(rq, ++count));
> +                       descs[n] = execlists_update_context(rq);
> +               }
> +       }
> +
> +       spin_lock(&engine->i915->vgpu.shared_page_lock[engine->id]);

Only one engine at a time now accesses that portion of pv_elsp, so the
spin lock is not required per-se, aiui.

The problem is that there is no coordination between pv_submit and the
other side of the pipe, as far as I can see. So it seems very possible
for us to overwrite entries before they have been read. I expect to see
some ack mechanism for VGT_G2V_PV_SUBMISSION.

> +       for (n = 0; n < execlists_num_ports(execlists); n++)
> +               shared_page->pv_elsp[engine->id].descs[n] = descs[n];

I'd recommend not using engine->id, that's just our internal id and may
vary between host/guest. Use engine->hw_id? Or negotiate using pv setup
and store pv_id?

But starting to look more solid over all. We just need to finish
splitting out the various similar-but-quite-different-really submission
backends. :)
-Chris