On 05/03/2019 18:10, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-03-05 18:07:39)
/*
* Similarly the preempt context must always be available so that
- * we can interrupt the engine at any time.
+ * we can interrupt the engine at any time. However, as preemption
+ * is optional, we allow it to fail.
*/
- if (i915->preempt_context) {
- ce = intel_context_pin(i915->preempt_context, engine);
- if (IS_ERR(ce)) {
- ret = PTR_ERR(ce);
- goto err_unpin_kernel;
- }
- }
+ if (i915->preempt_context)
+ pin_context(i915->preempt_context, engine,
+ &engine->preempt_context);
You lost the failure path here. I suspect deliberately? But I am not
convinced we want to silently lose preemption when keeping the failure
path is so easy.
The failure path kills the module. Whereas we can quite happily survive
without preemption.
Yes it is hard to decide what is worse, modprobe failure which never
happens, or change in performance profile which also never happens. :)
For something so unlikely I'd rather see it fail than silently change
behaviour. Perhaps it has some relevance during development and platform
bringup if nowhere else.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
