On 08-11-2016 18:08, Mauro Santos wrote: > On 08-11-2016 17:13, Emil Velikov wrote: >> 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. >> > > If I do 'cat revision' for the dGPU it does not wake up the device so I > guess we can scratch that. > >> Can you pull out the kernel patch and check drmdevice/dmesg with >> patched libdrm ? >> > > Same behavior as before, both versions of drmdevice wake up the dGPU. > Could it be that some other lib called by drmdevice and not called by > other programs is doing something to wake up the dGPU? > >> Thanks >> Emil >> > > Well, I _think_ I might have found the problem and it's not with libdrm if I'm correct. I was now thinking and trying to check if drmdevice might be trying to read some other information that would wake up the dGPU, and it does. If you check the mixed output of drmdevice/dmesg I have posted(?) before you can see that it wakes up the device when this message is shown: Opening device 0 node /dev/dri/card1 Note the path is /dev/dri/card1. I've tried a 'cat /dev/dri/card1' and sure enough the dGPU woke up, so I'd say libdrm and the kernel patch are working just fine as they don't touch anything in /dev but only in /sys. The initial output without the kernel patch says: device[0] available_nodes 0007 nodes nodes[0] /dev/dri/card1 nodes[1] /dev/dri/controlD65 nodes[2] /dev/dri/renderD129 bustype 0000 businfo pci domain 0000 bus 03 dev 00 func 0 deviceinfo pci vendor_id 1002 device_id 6600 subvendor_id 17aa subdevice_id 5049 revision_id 00 If you recheck what I have posted previously after applying the kernel patch it says revision_id 81, so I'd say it's working and we've been following a red herring all along. I suppose I have unintentionally misled you, I'm sorry for that. -- Mauro Santos _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
