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

["Kempczynski, Zbigniew" <zbigniew.kempczynski@intel.com>]

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?

Zbigniew


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