[PATCH] intel_reg: add support to hw address port
Dafna Hirschfeld
dafna.hirschfeld at intel.com
Sun May 25 06:50:17 UTC 2025
On 22.05.2025 19:04, Kamil Konieczny wrote:
>Hi Dafna,
>On 2025-05-14 at 21:40:42 +0300, Dafna Hirschfeld wrote:
>
>I will leave review for developers on Cc, here I have few nits.
>
>In subject, please add 'tools/' prefix, so instead of
>
>[PATCH] intel_reg: add support to hw address port
>
>it would be:
>
>[PATCH] tools/intel_reg: add support for hw address port
>
Will do.
>> For some platforms we want to access hw addresses.
>> For that we need a callback per platform that maps the hw address
>> to pci address. Add infrastructure to support that.
>>
>
>Do we need any driver support for this? If yes, will it work
>for both i915 and Xe? In that case could you point to driver
>patches for this?
>Btw it would be good to see a sample implementation for at
>least one GPU, maybe in a separate patch.
actually the purpose of this patch is not to need a driver support.
We convert in userspace the hardware address to pci address so no
driver support is needed. It can could work for any version of either i915/Xe,
Only the mapping callback should be added.
Thanks,
Dafna
>
>Regards,
>Kamil
>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld at intel.com>
>> ---
>> tools/intel_reg.c | 48 +++++++++++++++++++++++++++++++++++-------
>> tools/intel_reg_spec.c | 36 +++++++++++++++++++++++++++++++
>> tools/intel_reg_spec.h | 5 +++++
>> 3 files changed, 81 insertions(+), 8 deletions(-)
>>
>> diff --git a/tools/intel_reg.c b/tools/intel_reg.c
>> index 49afe91c0..28df7904f 100644
>> --- a/tools/intel_reg.c
>> +++ b/tools/intel_reg.c
>> @@ -519,12 +519,17 @@ static int read_register(struct config *config, struct reg *reg, uint32_t *valp)
>> break;
>> case PORT_MCHBAR_16:
>> case PORT_MMIO_16:
>> + case PORT_HWADDR_16:
>> val = INREG16(reg->mmio_offset + reg->addr);
>> break;
>> case PORT_MCHBAR_8:
>> case PORT_MMIO_8:
>> + case PORT_HWADDR_8:
>> val = INREG8(reg->mmio_offset + reg->addr);
>> break;
>> + case PORT_HWADDR_32:
>> + val = INREG(reg->mmio_offset + reg->addr);
>> + break;
>> case PORT_MMIO_VGA_AR:
>> val = vga_ar_read(reg->addr, true);
>> break;
>> @@ -610,6 +615,7 @@ static int write_register(struct config *config, struct reg *reg, uint32_t val)
>> break;
>> case PORT_MCHBAR_16:
>> case PORT_MMIO_16:
>> + case PORT_HWADDR_16:
>> if (val > 0xffff) {
>> fprintf(stderr, "value 0x%08x out of range for port %s\n",
>> val, reg->port_desc.name);
>> @@ -619,6 +625,7 @@ static int write_register(struct config *config, struct reg *reg, uint32_t val)
>> break;
>> case PORT_MCHBAR_8:
>> case PORT_MMIO_8:
>> + case PORT_HWADDR_8:
>> if (val > 0xff) {
>> fprintf(stderr, "value 0x%08x out of range for port %s\n",
>> val, reg->port_desc.name);
>> @@ -626,6 +633,9 @@ static int write_register(struct config *config, struct reg *reg, uint32_t val)
>> }
>> OUTREG8(reg->mmio_offset + reg->addr, val);
>> break;
>> + case PORT_HWADDR_32:
>> + OUTREG(reg->mmio_offset + reg->addr, val);
>> + break;
>> case PORT_MMIO_VGA_AR:
>> if (val > 0xff) {
>> fprintf(stderr, "value 0x%08x out of range for port %s\n",
>> @@ -750,6 +760,14 @@ static int parse_engine(struct reg *reg, const char *s)
>> return reg->engine == NULL;
>> }
>>
>> +/*
>> + * Get codename for a gen5+ platform to be used for finding register spec file.
>> + */
>> +static const char *get_codename(uint32_t devid)
>> +{
>> + return intel_get_device_info(devid)->codename;
>> +}
>> +
>> /* s has [(PORTNAME|PORTNUM|ENGINE|MMIO-OFFSET):](REGNAME|REGADDR) */
>> static int parse_reg(struct config *config, struct reg *reg, const char *s)
>> {
>> @@ -792,6 +810,28 @@ static int parse_reg(struct config *config, struct reg *reg, const char *s)
>> case PORT_MCHBAR_8:
>> reg->mmio_offset = mcbar_offset(config->devid);
>> break;
>> + case PORT_HWADDR_32:
>> + case PORT_HWADDR_16:
>> + case PORT_HWADDR_8:
>> + {
>> + uint64_t dev_addr = strtoull(p, &endp, 16);
>> + uint32_t pci_addr;
>> + const char *codename = get_codename(config->devid);
>> +
>> + ret = hwaddr_to_pciaddr(codename, dev_addr, &pci_addr);
>> + if (ret) {
>> + if (ret == -ENOENT)
>> + fprintf(stderr, "platform %s does not support hwaddr\n",
>> + codename);
>> + else
>> + fprintf(stderr,
>> + "mapping hw addr 0x%llx to pci bar failed (%d)\n",
>> + dev_addr, ret);
>> +
>> + return ret;
>> + }
>> + return set_reg_by_addr(config, reg, pci_addr);
>> + }
>> default:
>> break;
>> }
>> @@ -1077,14 +1117,6 @@ static int intel_reg_help(struct config *config, int argc, char *argv[])
>> return EXIT_SUCCESS;
>> }
>>
>> -/*
>> - * Get codename for a gen5+ platform to be used for finding register spec file.
>> - */
>> -static const char *get_codename(uint32_t devid)
>> -{
>> - return intel_get_device_info(devid)->codename;
>> -}
>> -
>> /*
>> * Get register definitions filename for devid in dir. Return 0 if found,
>> * negative error code otherwise.
>> diff --git a/tools/intel_reg_spec.c b/tools/intel_reg_spec.c
>> index 632db6b9f..f98d51928 100644
>> --- a/tools/intel_reg_spec.c
>> +++ b/tools/intel_reg_spec.c
>> @@ -32,6 +32,26 @@
>>
>> #include "intel_reg_spec.h"
>>
>> +struct hwaddr {
>> + const char *codename;
>> + int (*hwaddr_lookup)(uint64_t dev_addr, uint32_t *pci_addr);
>> +};
>> +
>> +static const struct hwaddr hwaddr_lookups[] = {
>> + { .codename = NULL, /* sentinel */}
>> +};
>> +
>> +int hwaddr_to_pciaddr(const char *codename, uint64_t dev_addr, uint32_t *pci_addr)
>> +{
>> + int i;
>> +
>> + for (i = 0; hwaddr_lookups[i].codename; i++) {
>> + if (strcmp(codename, hwaddr_lookups[i].codename) == 0)
>> + return hwaddr_lookups[i].hwaddr_lookup(dev_addr, pci_addr);
>> + }
>> + return -ENOENT;
>> +}
>> +
>> static const struct port_desc port_descs[] = {
>> {
>> .name = "mmio",
>> @@ -83,6 +103,22 @@ static const struct port_desc port_descs[] = {
>> .port = PORT_MMIO_VGA_CR,
>> .stride = 1,
>> },
>> + {
>> + .name = "hwaddr",
>> + .port = PORT_HWADDR_32,
>> + .stride = 4,
>> + },
>> + {
>> + .name = "hwaddr16",
>> + .port = PORT_HWADDR_16,
>> + .stride = 2,
>> + },
>> + {
>> + .name = "hwaddr8",
>> + .port = PORT_HWADDR_8,
>> + .stride = 1,
>> + },
>> +
>> {
>> .name = "portio",
>> .port = PORT_PORTIO,
>> diff --git a/tools/intel_reg_spec.h b/tools/intel_reg_spec.h
>> index af396a294..ccf4fd366 100644
>> --- a/tools/intel_reg_spec.h
>> +++ b/tools/intel_reg_spec.h
>> @@ -34,6 +34,10 @@ enum port_addr {
>> PORT_MCHBAR_16,
>> PORT_MCHBAR_8,
>>
>> + PORT_HWADDR_32,
>> + PORT_HWADDR_16,
>> + PORT_HWADDR_8,
>> +
>> PORT_MMIO_VGA_AR,
>> PORT_MMIO_VGA_SR,
>> PORT_MMIO_VGA_GR,
>> @@ -86,6 +90,7 @@ static inline void *recalloc(void *ptr, size_t nmemb, size_t size)
>> return realloc(ptr, nmemb * size);
>> }
>>
>> +int hwaddr_to_pciaddr(const char* codename, uint64_t dev_addr, uint32_t *pci_addr);
>> int parse_port_desc(struct reg *reg, const char *s);
>> ssize_t intel_reg_spec_builtin(struct reg **regs, uint32_t devid);
>> ssize_t intel_reg_spec_file(struct reg **regs, const char *filename);
>> --
>> 2.34.1
>>
More information about the igt-dev
mailing list