On 8 November 2016 at 13:38, Mauro Santos <registo.mailling@xxxxxxxxx> wrote: > 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. > Grr my bad. $libdrm_builddir/tests/drmdevice should do it. You might need to explicitly build it (cd tests && make drmdevice) > 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. > That's perfect, thanks. 0000:03 is (in all likelihood) is the PCI bridge. You can check with the device uevent/PCI_SLOT_NAME and cross reference with lspci. Emil _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
