On 08-11-2016 11:06, Emil Velikov wrote: > On 1 November 2016 at 18:47, Mauro Santos <registo.mailling@xxxxxxxxx> wrote: >> On 01-11-2016 18:13, Emil Velikov wrote: >>> From: Emil Velikov <emil.velikov@xxxxxxxxxxxxx> >>> >>> Parsing config sysfs file wakes up the device. The latter of which may >>> be slow and isn't required to begin with. >>> >>> Reading through config is/was required since the revision is not >>> available by other means, although with a kernel patch in the way we can >>> 'cheat' temporarily. >>> >>> That should be fine, since no open-source project has ever used the >>> value. >>> >>> Cc: Michel Dänzer <michel.daenzer@xxxxxxx> >>> Cc: Mauro Santos <registo.mailling@xxxxxxxxx> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98502 >>> Signed-off-by: Emil Velikov <emil.velikov@xxxxxxxxxxxxx> >>> --- >>> Mauro can you apply this against libdrm and rebuild it. You do _not_ >>> need to rebuild mesa afterwords. >>> >>> Thanks >>> --- >>> xf86drm.c | 50 +++++++++++++++++++++++++++++++++++--------------- >>> 1 file changed, 35 insertions(+), 15 deletions(-) >>> >>> diff --git a/xf86drm.c b/xf86drm.c >>> index 52add5e..5a5100c 100644 >>> --- a/xf86drm.c >>> +++ b/xf86drm.c >>> @@ -2950,25 +2950,45 @@ static int drmParsePciDeviceInfo(const char *d_name, >>> drmPciDeviceInfoPtr device) >>> { >>> #ifdef __linux__ >>> +#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0])) >>> + static const char *attrs[] = { >>> + "revision", /* XXX: make sure it's always first, see note below */ >>> + "vendor", >>> + "device", >>> + "subsystem_vendor", >>> + "subsystem_device", >>> + }; >>> char path[PATH_MAX + 1]; >>> - unsigned char config[64]; >>> - int fd, ret; >>> + unsigned int data[ARRAY_SIZE(attrs)]; >>> + FILE *fp; >>> + int ret; >>> >>> - snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/config", d_name); >>> - fd = open(path, O_RDONLY); >>> - if (fd < 0) >>> - return -errno; >>> + for (unsigned i = 0; i < ARRAY_SIZE(attrs); i++) { >>> + snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/%s", >>> + d_name, attrs[i]); >>> + fp = fopen(path, "r"); >>> + if (!fp) { >>> + /* Note: First we check the revision, since older kernels >>> + * may not have it. Default to zero in such cases. */ >>> + if (i == 0) { >>> + data[i] = 0; >>> + continue; >>> + } >>> + return -errno; >>> + } >>> >>> - ret = read(fd, config, sizeof(config)); >>> - close(fd); >>> - if (ret < 0) >>> - return -errno; >>> + ret = fscanf(fp, "%x", &data[i]); >>> + fclose(fp); >>> + if (ret != 1) >>> + return -errno; >>> + >>> + } >>> >>> - device->vendor_id = config[0] | (config[1] << 8); >>> - device->device_id = config[2] | (config[3] << 8); >>> - device->revision_id = config[8]; >>> - device->subvendor_id = config[44] | (config[45] << 8); >>> - device->subdevice_id = config[46] | (config[47] << 8); >>> + device->revision_id = data[0] & 0xff; >>> + device->vendor_id = data[1] & 0xffff; >>> + device->device_id = data[2] & 0xffff; >>> + device->subvendor_id = data[3] & 0xffff; >>> + device->subdevice_id = data[4] & 0xffff; >>> >>> return 0; >>> #else >>> >> >> I have applied this against libdrm 2.4.71 and I don't see any delays >> when starting firefox/chromium/thunderbird/glxgears. >> >> There is also no indication in dmesg that the dGPU is being >> reinitialized when starting the programs where I've detected the problem. >> > Thanks Mauro. Can you give this a try alongside the kernel fix [1] ? > I'd love to get the latter merged soon(ish). > Independent of the kernel side, I might need to go another way for > libdrm/mesa so I'll CC you on future patches. > > Your help is greatly appreciated ! > > Thanks > Emil > > [1] http://patchwork.ozlabs.org/patch/689975/ > I have applied the patch on top of kernel 4.8.6 and I'm using the libdrm with the patch you sent me previously. With this patch things still seem work fine, I don't see any of the problems I've seen before, but I don't know how to confirm that the value from sysfs is now being used by libdrm instead of defaulting to zero. The values in /sys/class/drm/card{0,1}/device/revision match what is reported by lspci for the iGPU and dGPU. I don't know if it's related but the revision file in /sys/class/pci_bus/0000\:03/device/ does show 0xf1 for both GPUs, which is different from what is shown in /sys/class/drm/card{0,1}/device/revision. -- Mauro Santos _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
