On 03/29/2019 11:40 PM, Chris Wilson wrote:
> Quoting Xiaolin Zhang (2019-03-29 13:32:40)
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 2f78829..28e8ee0 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -37,6 +37,7 @@
>> #include "i915_drv.h"
>> #include "i915_trace.h"
>> #include "intel_drv.h"
>> +#include "i915_vgpu.h"
>>
>> /**
>> * DOC: interrupt handling
>> @@ -1470,6 +1471,7 @@ gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir)
>> if (iir & GT_RENDER_USER_INTERRUPT) {
>> intel_engine_breadcrumbs_irq(engine);
>> tasklet |= USES_GUC_SUBMISSION(engine->i915);
>> + tasklet |= USES_PV_SUBMISSION(engine->i915);
> We should move this to an engine->flag.
sure, I will remove this next version.
>
>> }
>>
>> if (tasklet)
>> +static void vgpu_pv_set_default_submission(struct intel_engine_cs *engine)
>> +{
>> + /*
>> + * We inherit a bunch of functions from execlists that we'd like
>> + * to keep using:
>> + *
>> + * engine->submit_request = execlists_submit_request;
>> + * engine->cancel_requests = execlists_cancel_requests;
>> + * engine->schedule = execlists_schedule;
>> + *
>> + * But we need to override the actual submission backend in order
>> + * to talk to the GVT with PV notification message.
>> + */
>> + intel_execlists_set_default_submission(engine);
>> +
>> + engine->execlists.tasklet.func = vgpu_pv_submission_tasklet;
> You need to pin the breadcrumbs irq, or it will not fire on every
> request.
indeed, I should do. thanks your great point.
> I'd push for this to live in intel_pv_submission.c or
> intel_vgpu_submission.c
this will create new file and make a change in Makefile. if this kind
of change is OK,
I will create one to use intel_pv_submission.c name.
>> @@ -401,6 +551,12 @@ void intel_vgpu_config_pv_caps(struct drm_i915_private *dev_priv,
>> ppgtt->vm.insert_entries = gen8_ppgtt_insert_4lvl_pv;
>> ppgtt->vm.clear_range = gen8_ppgtt_clear_4lvl_pv;
>> }
>> +
>> + if (cap == PV_SUBMISSION) {
>> + engine = (struct intel_engine_cs *)data;
>> + engine->set_default_submission = vgpu_pv_set_default_submission;
>> + engine->set_default_submission(engine);
>> + }
>> }
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> index c1b9780..0a66714 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -2352,6 +2352,9 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine)
>> */
>> }
>> engine->emit_bb_start = gen8_emit_bb_start;
>> +
>> + if (intel_vgpu_active(engine->i915))
>> + intel_vgpu_config_pv_caps(engine->i915, PV_SUBMISSION, engine);
> That pair is ugly. Should clean up the engine initialisation so that it
> doesn't involve placing a chunk of code in a foreign class.
> -Chris
>
Yep, will move it to other location.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
