> +/** > + * struct drm_panfrost_wait_bo - ioctl argument for waiting for > + * completion of the last DRM_PANFROST_SUBMIT_CL on a BO.
Nit: Should be plain DRM_PANFROST_SUBMIT, there is no CL for us.
> + __s64 timeout_ns; /* absolute */
Erm, why is this signed? Semantically, what does a negative timestamp
mean? Seems suspect. The comment /* absolute */ seems to underscore that
we really do want an unsigned value, perhaps ascribing a special meaning
to 0/~0 for "nonblocking" and "block indefinitely" if needed. Of course,
"(2^64)-1 ns" is essentially indefinite, so the latter need not be a
special case.
* It's 585 years, according to a back of the envelope calculation.
Panfrost will be obsolete many times over by the time that timeout
elapses ;)
> + struct drm_panfrost_mmap_bo mmap_bo = {0,};
Nit: "{0,} is confusing.
General nit: Spacing is all over the place in pan_drm.h. I'm not one to
particularly care (I think I use 8-space indents...), but it's
inconsistent from line to line which is a little distracting. If you use
vim, I have set:
au BufNewFile,BufRead */mesa/* set expandtab tabstop=8 softtabstop=3
shiftwidth=3
au BufNewFile,BufRead */panfrost/* set expandtab tabstop=8 shiftwidth=8
softtabstop=8
which generally does the right thing. Translating to $EDITOR left as an
exercise to the reader.
---
Overall, this looks really great. I haven't gotten the chance to test
this personally yet, but I think I can nevertheless push at least the
other 7 patches tonight :)
Keep up the awesome work!
Alyssa
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
