On 8 November 2016 at 16:57, Mauro Santos <registo.mailling@xxxxxxxxx> wrote: > On 08-11-2016 15:57, Emil Velikov wrote: >> On 8 November 2016 at 15:27, Mauro Santos <registo.mailling@xxxxxxxxx> wrote: >>> On 08-11-2016 15:00, Emil Velikov wrote: >>>> 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) >>>> >>> >>> When running drmdevice as my user it still wakes up the dGPU. The >>> correct device revisions are being reported by I suppose that is not >>> being read from sysfs. >>> >> Based on the output you're spot on - doesn't seem like the revision >> sysfs gets used. Most likely drmdevice is linked/using the pre-patch >> (system/local?) libdrm.so ? >> > > I've been using the patched libdrm.so ever since you sent me the patch > for libdrm and I've recompiled libdrm today to get drmdevice so both the > system's and in-tree libdrm.so should have the patch. Arch's PKGBUILD > does have a non default --enable-udev configure parameter, could that > make any difference? > The --enable-udev does not make any difference. The rest does not make sense - the exact same functions are used by drmdevice and mesa, yet it two different results are produced :-\ Or something very funny is happening and reading the device/vendor file does _not_ wake the device, while the revision one does. Can you pull out the kernel patch and check drmdevice/dmesg with patched libdrm ? Thanks Emil _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
