Re: [PATCH v6 4/7] drm/i915/perf: reuse intel_lrc ctx regs macro

[Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>]

On 23/05/18 15:57, Tvrtko Ursulin wrote:
> On 22/05/2018 18:59, Lionel Landwerlin wrote:
> > Abstract the context image access a bit.
> >
> > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
> > ---
> >   drivers/gpu/drm/i915/i915_perf.c | 34 +++++++++++++++-----------------
> >   1 file changed, 16 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_perf.c 
> > b/drivers/gpu/drm/i915/i915_perf.c
> > index 805dfc732bba..a5d98bda5c2e 100644
> > --- a/drivers/gpu/drm/i915/i915_perf.c
> > +++ b/drivers/gpu/drm/i915/i915_perf.c
> > @@ -210,6 +210,7 @@
> >   #include "i915_oa_cflgt3.h"
> >   #include "i915_oa_cnl.h"
> >   #include "i915_oa_icl.h"
> > +#include "intel_lrc_reg.h"
> >     /* HW requires this to be a power of two, between 128k and 16M, 
> > though driver
> >    * is currently generally designed assuming the largest 16M size is 
> > used such
> > @@ -1579,27 +1580,25 @@ static void 
> > gen8_update_reg_state_unlocked(struct i915_gem_context *ctx,
> >       u32 ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset;
> >       u32 ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset;
> >       /* The MMIO offsets for Flex EU registers aren't contiguous */
> > -    u32 flex_mmio[] = {
> > -        i915_mmio_reg_offset(EU_PERF_CNTL0),
> > -        i915_mmio_reg_offset(EU_PERF_CNTL1),
> > -        i915_mmio_reg_offset(EU_PERF_CNTL2),
> > -        i915_mmio_reg_offset(EU_PERF_CNTL3),
> > -        i915_mmio_reg_offset(EU_PERF_CNTL4),
> > -        i915_mmio_reg_offset(EU_PERF_CNTL5),
> > -        i915_mmio_reg_offset(EU_PERF_CNTL6),
> > +    i915_reg_t flex_regs[] = {
> > +        EU_PERF_CNTL0,
> > +        EU_PERF_CNTL1,
> > +        EU_PERF_CNTL2,
> > +        EU_PERF_CNTL3,
> > +        EU_PERF_CNTL4,
> > +        EU_PERF_CNTL5,
> > +        EU_PERF_CNTL6,
> >       };
> >       int i;
> >   -    reg_state[ctx_oactxctrl] = 
> > i915_mmio_reg_offset(GEN8_OACTXCONTROL);
> > -    reg_state[ctx_oactxctrl+1] = (dev_priv->perf.oa.period_exponent <<
> > -                      GEN8_OA_TIMER_PERIOD_SHIFT) |
> > -                     (dev_priv->perf.oa.periodic ?
> > -                      GEN8_OA_TIMER_ENABLE : 0) |
> > -                     GEN8_OA_COUNTER_RESUME;
> > +    CTX_REG(reg_state, ctx_oactxctrl, GEN8_OACTXCONTROL,
> > +        (dev_priv->perf.oa.period_exponent << 
> > GEN8_OA_TIMER_PERIOD_SHIFT) |
> > +        (dev_priv->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
> > +        GEN8_OA_COUNTER_RESUME);
> >   -    for (i = 0; i < ARRAY_SIZE(flex_mmio); i++) {
> > +    for (i = 0; i < ARRAY_SIZE(flex_regs); i++) {
> >           u32 state_offset = ctx_flexeu0 + i * 2;
> > -        u32 mmio = flex_mmio[i];
> > +        u32 mmio = i915_mmio_reg_offset(flex_regs[i]);
> >             /*
> >            * This arbitrary default will select the 'EU FPU0 Pipeline
> > @@ -1619,8 +1618,7 @@ static void 
> > gen8_update_reg_state_unlocked(struct i915_gem_context *ctx,
> >               }
> >           }
> >   -        reg_state[state_offset] = mmio;
> > -        reg_state[state_offset+1] = value;
> > +        CTX_REG(reg_state, state_offset, flex_regs[i], value);
>
> Does flex_regs[i] needs to be mmio?

Yeah, the macro uses i915_mmio_reg_offset().

> >       }
> >   }
>
> Regards,
>
> Tvrtko

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