Quoting Tvrtko Ursulin (2019-03-05 18:17:35)
>
> 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.
I err on the opposite side, keep going at all costs so that the user
isn't scaring at a blank screen of doom. It's not exactly a silent
failure, we tell anyone who asks that preemption is enabled :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
