Hi Luben,
Thanks for reviewing. Was expecting lot from vim auto-indent :/
On 1/22/20 1:07 AM, Luben Tuikov wrote:
On 2020-01-21 11:50 a.m., Nirmoy Das wrote:
- switch (i) {
+ priority = (ctx->override_priority == DRM_SCHED_PRIORITY_UNSET) ?
+ ctx->init_priority : ctx->override_priority;
You don't need parenthesis around the relational equality operator used here.
It has higher precedence than the ternary conditional, in which it is embedded.
Parenthesis makes it more human readable.
+ int r;
+
+ if (priority < 0 || priority >= DRM_SCHED_PRIORITY_MAX)
+ return -EINVAL;
This shouldn't be possible since it is an enum...
What should not be possible ?
But why not do this check in "amdgpu_ctx_priority_permit()"
which is introduced by this patchset and used in the imediately
following line?
Makes sense.
Or perhaps fix up amdgpu_to_sched_priority() where it is massaged
from the ioctl argument which is an integer.
On a side note: I noticed that the enum drm_sched_priority
has no DRM_SCHED_PRIORITY_NONE.
I've found value in setting the first value of an enum to
"_NONE" (unless zero actually has a meaning as set by HW/etc., anyway).
Enum drm_sched_priority starts with "_MIN" and "_LOW" which
are both set to the same (zero) value.
So having DRM_SCHED_PRIORITY_NONE, could be used to denote
that no priority was given and any is fine, or to mean
that if the priority was given out of range, set it to "none",
to mean pick any.
Not sure about if HW accepts 0.
Regards,
Nirmoy
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
