Re: [igt-dev] [PATCH i-g-t 3/4] tools/intel_reg: Add support for register access via the MCHBAR mirror

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 06 Mar 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Remembering the MCHBAR mirror offset is too much work. Make the tool
> remember it for me.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  tools/intel_reg.c      | 17 +++++++++++++++++
>  tools/intel_reg_spec.c | 15 +++++++++++++++
>  tools/intel_reg_spec.h |  4 ++++
>  3 files changed, 36 insertions(+)
>
> diff --git a/tools/intel_reg.c b/tools/intel_reg.c
> index 7f8ddbd8e927..69af0f5a19ff 100644
> --- a/tools/intel_reg.c
> +++ b/tools/intel_reg.c
> @@ -185,6 +185,9 @@ static bool port_is_mmio(enum port_addr port)
>  	case PORT_MMIO_32:
>  	case PORT_MMIO_16:
>  	case PORT_MMIO_8:
> +	case PORT_MCHBAR_32:
> +	case PORT_MCHBAR_16:
> +	case PORT_MCHBAR_8:
>  		return true;
>  	default:
>  		return false;
> @@ -366,20 +369,34 @@ static int register_srm(struct config *config, struct reg *reg,
>  	return val;
>  }
>  
> +static uint32_t mcbar_offset(uint32_t devid)
> +{
> +	return intel_gen(devid) >= 6 ? 0x140000 : 0x10000;
> +}
> +
>  static int read_register(struct config *config, struct reg *reg, uint32_t *valp)
>  {
>  	uint32_t val = 0;
>  
>  	switch (reg->port_desc.port) {
> +	case PORT_MCHBAR_32:
> +		reg->mmio_offset = mcbar_offset(config->devid);
> +		/* fall through */

I think it would be better to handle this earlier, at parse_port_desc()
or parse_reg() level. I think having this here might screw with the
ability to handle the mchbar "ports" nicely in the reg spec. Or decode
them.

This being devid specific makes it a bit harder I guess.

BR,
Jani.


>  	case PORT_MMIO_32:
>  		if (reg->engine)
>  			val = register_srm(config, reg, NULL);
>  		else
>  			val = INREG(reg->mmio_offset + reg->addr);
>  		break;
> +	case PORT_MCHBAR_16:
> +		reg->mmio_offset = mcbar_offset(config->devid);
> +		/* fall through */
>  	case PORT_MMIO_16:
>  		val = INREG16(reg->mmio_offset + reg->addr);
>  		break;
> +	case PORT_MCHBAR_8:
> +		reg->mmio_offset = mcbar_offset(config->devid);
> +		/* fall through */
>  	case PORT_MMIO_8:
>  		val = INREG8(reg->mmio_offset + reg->addr);
>  		break;
> diff --git a/tools/intel_reg_spec.c b/tools/intel_reg_spec.c
> index 7c1c6ed34a9e..dadd844730f0 100644
> --- a/tools/intel_reg_spec.c
> +++ b/tools/intel_reg_spec.c
> @@ -48,6 +48,21 @@ static const struct port_desc port_descs[] = {
>  		.port = PORT_MMIO_8,
>  		.stride = 1,
>  	},
> +	{
> +		.name = "mchbar",
> +		.port = PORT_MCHBAR_32,
> +		.stride = 4,
> +	},
> +	{
> +		.name = "mchbar16",
> +		.port = PORT_MCHBAR_16,
> +		.stride = 2,
> +	},
> +	{
> +		.name = "mchbar8",
> +		.port = PORT_MCHBAR_8,
> +		.stride = 1,
> +	},
>  	{
>  		.name = "portio",
>  		.port = PORT_PORTIO,
> diff --git a/tools/intel_reg_spec.h b/tools/intel_reg_spec.h
> index 29f4d9ae2a47..fc27a9a62f6f 100644
> --- a/tools/intel_reg_spec.h
> +++ b/tools/intel_reg_spec.h
> @@ -29,6 +29,10 @@ enum port_addr {
>  	PORT_MMIO_16,
>  	PORT_MMIO_8,
>  
> +	PORT_MCHBAR_32,
> +	PORT_MCHBAR_16,
> +	PORT_MCHBAR_8,
> +
>  	PORT_PORTIO,
>  
>  	PORT_NONE = 0,

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev