Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> Quoting Mika Kuoppala (2017-09-20 15:37:03)
>> Instead of trusting that first available port is at index 0,
>> use accessor to hide this. This is a preparation for a
>> following patches where head can be at arbitrary location
>> in the port array.
>>
>> v2: improved commit message, elsp_ready readability (Chris)
>>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/i915_debugfs.c | 16 +++++++----
>> drivers/gpu/drm/i915/i915_gpu_error.c | 4 +--
>> drivers/gpu/drm/i915/i915_guc_submission.c | 17 ++++++-----
>> drivers/gpu/drm/i915/i915_irq.c | 2 +-
>> drivers/gpu/drm/i915/intel_engine_cs.c | 2 +-
>> drivers/gpu/drm/i915/intel_lrc.c | 42 +++++++++++++++------------
>> drivers/gpu/drm/i915/intel_ringbuffer.h | 46 ++++++++++++++++++++++++++----
>> 7 files changed, 87 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index dbeb6f08ab79..af8cc2eab1b1 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -3348,16 +3348,20 @@ static int i915_engine_info(struct seq_file *m, void *unused)
>>
>> rcu_read_lock();
>> for (idx = 0; idx < execlist_num_ports(el); idx++) {
>> - unsigned int count;
>> + const struct execlist_port *port;
>> + unsigned int count, n;
>>
>> - rq = port_unpack(&el->port[idx], &count);
>> + port = execlist_port_index(el, idx);
>> + n = port_index(port, el);
>
> Bah, execlist_port_index() implies to me that it should return the
> index, not the port. I would just call it execlist_port(). How does that
> look?
It looks much better.
>
>> -static inline void
>> +#define __port_idx(start, index, mask) (((start) + (index)) & (mask))
>> +
>> +static inline struct execlist_port *
>> +execlist_port_head(struct intel_engine_execlist * const el)
>> +{
>> + return &el->port[el->port_head];
>> +}
>> +
>> +/* Index starting from port_head */
>> +static inline struct execlist_port *
>> +execlist_port_index(struct intel_engine_execlist * const el,
>> + const unsigned int n)
>> +{
>> + return &el->port[__port_idx(el->port_head, n, el->port_mask)];
>> +}
>> +
>> +static inline struct execlist_port *
>> +execlist_port_tail(struct intel_engine_execlist * const el)
>> +{
>> + return &el->port[__port_idx(el->port_head, -1, el->port_mask)];
>> +}
>
> Hmm, I was expecting
>
> execlist_port_head() { return execlist_port(el, 0); }
> execlist_port_tail() { return execlist_port(el, -1); }
Seems that I did these on the next patch, moved to here.
>
> What's the impact on object size? (As a quick guide to how much the
> compiler can keep the code in check.)
I can't say what would constitute as a still in check, but things
grow:
add/remove: 0/0 grow/shrink: 6/1 up/down: 277/-26 (251)
function old new delta
intel_lrc_irq_handler 1591 1700 +109
i915_guc_irq_handler 1041 1110 +69
i915_engine_info 1983 2031 +48
insert_request 127 152 +25
intel_engine_is_idle 304 317 +13
gen8_cs_irq_handler 113 126 +13
capture 5454 5428 -26
Total: Before=1144612, After=1144863, chg +0.02%
-Mika
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
