On 02/06/2021 19:51, Daniel Vetter wrote:
> On Wed, Jun 02, 2021 at 03:06:50PM +0100, Steven Price wrote:
>> On 21/05/2021 10:09, Daniel Vetter wrote:
[...]
>>> +   if (!xa_empty(&job->deps))
>>> +           return xa_erase(&job->deps, job->last_dep++);
>>
>> Rather than tracking last_dep separately this could be written using
>> xa_find():
>>
>>     if (xa_find(&job->deps, &i, ULONG_MAX, XA_PRESENT))
>>      return xa_erase(&job->deps, &i);
> 
> I copypasted this from other drivers, imo consistency is better than
> looking pretty. I think eventually we should stuff this as optional
> helpers into drm/scheduler.
> 
> Also yours walks the xa twice.

Agreed this isn't as efficient (I was somewhat disappointed xarray
doesn't expose a "get and remove the first element" API). Moving this
into drm/scheduler seems like the right long term solution - so matching
other drivers first is a good move.

Thanks,

Steve
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to