> > > + /* If ->gpu_access is 0, the BO is idle, and if the > > > WRITE flag > > > + * is cleared, that means we only have readers. > > > + */ > > > + if (!bo->gpu_access) > > > + return true; > > > + else if (!(access_type & PAN_BO_GPU_ACCESS_READ) && > > > + !(bo->gpu_access & PAN_BO_GPU_ACCESS_WRITE)) > > > + return true; > > > > The second condition is a little confusing, though I think it's correct. > > Not sure if there's any way to clarify what's meant but just thought I'd > > comment, since inevitably future readers will squint too. > > I can do: > > /* If ->gpu_access is 0, the BO is idle, no need to wait. */ > if (!bo->gpu_access) > return true; > > /* If the caller only wants to wait for writers and no > * writes are pending, we don't have to wait. > */ > if (access_type == PAN_BO_GPU_ACCESS_WRITE && > !(bo->gpu_access & PAN_BO_GPU_ACCESS_WRITE)) > return true; > > instead.
Perfect, thank you :) > > > + /* Update the BO access flags so that panfrost_bo_wait() > > > knows > > > + * about all pending accesses. > > > + */ > > > + bo->gpu_access |= flags & (PAN_BO_GPU_ACCESS_RW); > > > > This looks like black magic. Maybe just clarify in the comment why this > > & is reasonable (relying on bit mask magic). > > It's just here to clear all non-RW flags (we only care about the read/write > information when it comes to BO idleness). I'll add a comment to explain that > part, and maybe another one to explain why we have a '|=' and not just '='. Yeah, I figured it out. Mostly, REing means we have so much unexplained bit magic in the codebase already, we don't need to add more if we can help it :) Hence the comment would be appreciated. > > That aside, as I mentioned it would maybe make more sense to squash this > > into the patch introduce the bo_wait ioctl() in the first place? If > > that's too complicated with merge conflicts and stuff, don't sweat it, > > though :) > > I'm fine with that, I'll re-order things to avoid introducing the bo_wait() > infra before we have the access type info. +1 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
