Re: [PATCH 5/8] drm/i915: Introduce iterators for execlist ports

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2017-09-12 09:36:15)
>> Switch to iterators for execlist_port access. This is
>> a preparation for indexing ports from arbitrary location. Which
>> in turn allows us to handle ports in ring like fashion.
>> 
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>
> Something we want to keep an eye as we switch to these macros is code
> generation. As a quick guide, check object size, i.e. run
> scripts/bloat-o-meter.
>
>> -static inline unsigned int
>> -execlist_num_ports(const struct intel_engine_execlist * const el)
>> -{
>> -       return el->port_mask + 1;
>> -}
>> +/* Iterators over elsp ports */
>> +#define __port_idx(start, i, m) (((start) + (i)) & (m))
>> +
>> +#define for_each_execlist_port(el__, port__, n__) \
>> +       for ((n__) = 0; \
>> +            (port__) = &(el__)->port[__port_idx(0, (n__), (el__)->port_mask)], (n__) < (el__)->port_mask + 1; \
>> +            (n__)++)
>
> Using (n__) is misleading. It can't be anything other than a lhv (i.e.
> plain variable and not an expr). checkpatch can complain all it wants.
>
> Probably should keep execlist_num_ports() for another patch.
> That x < y + 1 just keeps on triggering me everytime I see it.

There is only a once callsite per macro if I recall right. We could
just drop these macros and live happier life as no-one really is a fan.

-Mika

>
>> +
>> +#define for_each_execlist_port_reverse(el__, port__, n__) \
>> +       for ((n__) = (el__)->port_mask + 1; \
>> +            (port__) = &(el__)->port[__port_idx((el__)->port_mask, (n__), (el__)->port_mask)], (n__)--;)
>>  
>>  static inline void
>>  execlist_port_complete(struct intel_engine_execlist * const el,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx