On Thu, Apr 29, 2021 at 11:02:27AM -0500, Jason Ekstrand wrote: > On Thu, Apr 29, 2021 at 7:16 AM Daniel Vetter <daniel@xxxxxxxx> wrote: > > > > On Wed, Apr 28, 2021 at 01:58:17PM -0500, Jason Ekstrand wrote: > > > On Wed, Apr 28, 2021 at 12:18 PM Jason Ekstrand <jason@xxxxxxxxxxxxxx> wrote: > > > > > > > > On Wed, Apr 28, 2021 at 5:13 AM Daniel Vetter <daniel@xxxxxxxx> wrote: > > > > > > > > > > On Tue, Apr 27, 2021 at 08:51:08AM -0500, Jason Ekstrand wrote: > > > > > > I sent a v2 of this patch because it turns out I deleted a bit too > > > > > > much code. This function in particular, has to stay, unfortunately. > > > > > > When a batch is submitted with a SUBMIT_FENCE, this is used to push > > > > > > the work onto a different engine than than the one it's supposed to > > > > > > run in parallel with. This means we can't dead-code this function or > > > > > > the bond_execution function pointer and related stuff. > > > > > > > > > > Uh that's disappointing, since if I understand your point correctly, the > > > > > sibling engines should all be singletons, not load balancing virtual ones. > > > > > So there really should not be any need to pick the right one at execution > > > > > time. > > > > > > > > The media driver itself seems to work fine if I delete all the code. > > > > It's just an IGT testcase that blows up. I'll do more digging to see > > > > if I can better isolate why. > > > > > > I did more digging and I figured out why this test hangs. The test > > > looks at an engine class where there's more than one of that class > > > (currently only vcs) and creates a context where engine[0] is all of > > > the engines of that class bonded together and engine[1-N] is each of > > > those engines individually. It then tests that you can submit a batch > > > to one of the individual engines and then submit with > > > EXEC_FENCE_SUBMIT to the balanced engine and the kernel will sort it > > > out. This doesn't seem like a use-case we care about. > > > > > > If we cared about anything, I would expect it to be submitting to two > > > balanced contexts and expecting "pick any two" behavior. But that's > > > not what the test is testing for. > > > > Yeah ditch it. > > > > Instead make sure that the bonded setparam/ctx validation makes sure that > > 1) no virtual engines are used > > 2) no engine used twice > > 3) anything else stupid you can come up with that we should make sure is > > blocked. > > I've re-introduced the deletion and I'll add nuking that test to my > IGT series. I did it as a separate patch as the FENCE_SUBMIT logic > and the bonding are somewhat separate concerns. > > As far as validation goes, I don't think we need any more for this > case. You used FENCE_SUBMIT and didn't properly isolate things such > that the two run on different engines. Not our problem. Oh I just meant validating the bonded ctx extension thing. Not validating submit fence, that's rather hopeless since it really allows anything you can think of, by design. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
