[Intel-gfx] [PATCH 6/8] drm/i915: Introduce execlist_port_* accessors

[Chris Wilson]

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 at intel.com>
> ---
>  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?

> -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); }

What's the impact on object size? (As a quick guide to how much the
compiler can keep the code in check.)
-Chris