Re: [PATCH v5 4/8] drm/i915: vgpu context submission pv optimization

["Zhang, Xiaolin" <xiaolin.zhang@intel.com>]

On 04/29/2019 06:03 PM, Chris Wilson wrote:
> 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
>
Chris, Thanks your great comments and will be addressed next version
(will remove spin lock, use hw_id,  add ack mechanism and refine patches). 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx