Re: [igt-dev] [PATCH i-g-t v7 2/3] lib/intel_mmio: add funtions for read/write register funtions

["Mrzyglod, Daniel T" <daniel.t.mrzyglod@intel.com>]

On Fri, 2019-08-16 at 20:43 +0100, Chris Wilson wrote:
> Quoting Daniel Mrzyglod (2019-08-16 15:53:16)
> >     This patch is first move to extend functionality of intel_mmio
> > library.
> >     There was limitation for 1 device, adding pointer for IO
> > functions to
> >     mmaped area gives us possibility to use those IO functions for
> > other mmaped
> >     devices.
> > 
> >     v4: reword commitmsg, spelling errors
> > 
> >     Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> >     Cc: Daniele Spurio Ceraolo <daniele.ceraolospurio@intel.com>
> >     Cc: Katarzyna Dec <katarzyna.dec@intel.com>
> >     Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >     Cc: Petri Latvala <petri.latvala@intel.com>
> >     Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > 
> >     Reviewed-by: Zbigniew Kempczyński <
> > zbigniew.kempczynski@intel.com>
> >     Signed-off-by: Daniel Mrzyglod <daniel.t.mrzyglod@intel.com>
> > ---
> >  lib/intel_io.h   |   7 ++++
> >  lib/intel_mmio.c | 107
> > +++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 114 insertions(+)
> > 
> > diff --git a/lib/intel_io.h b/lib/intel_io.h
> > index 6014c485..99cb1829 100644
> > --- a/lib/intel_io.h
> > +++ b/lib/intel_io.h
> > @@ -49,6 +49,13 @@ void OUTREG(uint32_t reg, uint32_t val);
> >  void OUTREG16(uint32_t reg, uint16_t val);
> >  void OUTREG8(uint32_t reg, uint8_t val);
> >  
> > +uint32_t INREG_DEV(void *igt_mmio, uint32_t reg);
> > +uint16_t INREG16_DEV(void *igt_mmio, uint32_t reg);
> > +uint8_t INREG8_DEV(void *igt_mmio, uint32_t reg);
> > +void OUTREG_DEV(void *igt_mmio, uint32_t reg, uint32_t val);
> > +void OUTREG16_DEV(void *igt_mmio, uint32_t reg, uint16_t val);
> > +void OUTREG8_DEV(void *igt_mmio, uint32_t reg, uint8_t val);
> > +
> >  /* sideband access functions from intel_iosf.c */
> >  uint32_t intel_dpio_reg_read(uint32_t reg, int phy);
> >  void intel_dpio_reg_write(uint32_t reg, uint32_t val, int phy);
> > diff --git a/lib/intel_mmio.c b/lib/intel_mmio.c
> > index a5458aeb..01411c37 100644
> > --- a/lib/intel_mmio.c
> > +++ b/lib/intel_mmio.c
> > @@ -405,3 +405,110 @@ void OUTREG8(uint32_t reg, uint8_t val)
> >  {
> >         *((volatile uint8_t *)igt_global_mmio + reg) = val;
> >  }
> > +
> > +
> > +/**
> > + * INREG_DEV:
> > + * @igt_mmio maped memory pointer
> > + * @reg: register offset
> > + *
> > + * 32-bit read of the register at offset @reg. This function only
> > works when the
> > + * new register access helper is initialized with
> > intel_register_access_init().
> > + *
> > + * This function directly accesses the igt_mmio without safety
> > checks.
> > + *
> > + * Returns:
> > + * The value read from the register.
> > + */
> > +uint32_t INREG_DEV(void *igt_mmio, uint32_t reg)
> > +{
> > +       return *(volatile uint32_t *)((volatile char *)igt_mmio +
> > reg);
> 
> We allow arithmetic on void*, the first cast is unnecessary.
> 
> Is there a good reason why these are not inlines? The cost of
> function
> call looks to be greater than the body?
> -Chris

So you suggest to move all those functions to intel_io.h/or other
header/ as static inlines? 

I expanded those functions like it was for previous register read write
functions - so maybe this behaviour was for debugging pupose?

Daniel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev