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

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

On Thu, 2019-08-29 at 09:06 +0100, Chris Wilson wrote:
> Quoting Kempczynski, Zbigniew (2019-08-29 06:03:37)
> > On Wed, 2019-08-28 at 20:12 +0200, Daniel Mrzyglod wrote:
> > >     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.
> > > 
> > <cut>
> > > -uint16_t INREG16(uint32_t reg);
> > > -uint8_t INREG8(uint32_t reg);
> > > -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 *mmio, uint32_t reg);
> > > +uint16_t INREG16_DEV(void *mmio, uint32_t reg);
> > > +uint8_t INREG8_DEV(void *mmio, uint32_t reg);
> > > +void OUTREG_DEV(void *mmio, uint32_t reg, uint32_t val);
> > > +void OUTREG16_DEV(void *mmio, uint32_t reg, uint16_t val);
> > > +void OUTREG8_DEV(void *mmio, uint32_t reg, uint8_t val);
> > > +
> > > +#define INREG_PTR(n,...) INREG##n##_DEV(__VA_ARGS__)
> > > +#define INREG_NOPTR(n,...) INREG##n##_DEV(igt_global_mmio,
> > > __VA_ARGS__)
> > > +#define INREG_GET_MACRO(_1,_2,NAME,...) NAME
> > > +#define INREGX(n,...) INREG_GET_MACRO(__VA_ARGS__, INREG_PTR,
> > > INREG_NOPTR)(n, __VA_ARGS__)
> > > +#define INREG16(...) INREGX(16, __VA_ARGS__)
> > > +#define INREG8(...) INREGX(8, __VA_ARGS__)
> > > +#define INREG(...) INREGX(,__VA_ARGS__)
> > > +
> > > +#define OUTREG_PTR(n,...) OUTREG##n##_DEV(__VA_ARGS__)
> > > +#define OUTREG_NOPTR(n,...) OUTREG##n##_DEV(igt_global_mmio,
> > > __VA_ARGS__)
> > > +#define OUTREG_GET_MACRO(_1,_2,_3,NAME,...) NAME
> > > +#define OUTREGX(n, ...) OUTREG_GET_MACRO(__VA_ARGS__,
> > > OUTREG_PTR,
> > > OUTREG_NOPTR)(n, __VA_ARGS__)
> > > +#define OUTREG16(...) OUTREGX(16, __VA_ARGS__)
> > > +#define OUTREG8(...) OUTREGX(8, __VA_ARGS__)
> > > +#define OUTREG(...) OUTREGX(,__VA_ARGS__)
> > 
> > You'll likely became my local macro God, but this is overkill IMO.
> > 
> > For sake of readability I would: 
> > 1. make OUTREGX_DEV, INREGX_DEV (X = {,8,16}) inline functions
> > 2. make OUTREGX, INREG (X = {,8,16}) macros with predefined
> > igt_global_mmio
> >    as first argument to *_DEV() function.
> > 
> > If someone needs to use different mmio than igt_global_mmio he 
> > should use *_DEV(). 
> > 
> > Chris, could you speak up about above?
> 
> I would have finished the job by building the inlines with the
> macros...
> -Chris

THX for comments!
I spend some time with my old macros but inlines generated by macros
are probably cleaner and tidier solution!

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