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

Kempczynski, Zbigniew
2019-08-29T05:03:39Z

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