On 1 December 2016 at 03:56, Michel Dänzer <michel@xxxxxxxxxxx> wrote: > On 01/12/16 12:09 PM, Michel Dänzer wrote: >> On 01/12/16 05:35 AM, Emil Velikov wrote: >>> From: Emil Velikov <emil.velikov@xxxxxxxxxxxxx> >>> >>> Relative to the original version, here one can provide a flags bitmask. >>> Currently only DRM_DEVICE_IGNORE_PCI_REVISION is supported. >>> >>> Implementation detail: >>> If it's set, we will only parse the separate sysfs files and we won't >>> touch the config one. The latter awakes the device (causing delays) >>> which is the core reason why this API was introduced. >>> >>> Cc: Michel Dänzer <michel@xxxxxxxxxxx> >>> Cc: Nicolai Hähnle <nhaehnle@xxxxxxxxx> >>> Cc: Mauro Santos <registo.mailling@xxxxxxxxx> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98502 >>> Signed-off-by: Emil Velikov <emil.velikov@xxxxxxxxxxxxx> >>> --- >>> Michel, Nicolai any naming suggestions or input in general will be >>> appreciated. >> >> It all looks good to me in general, thanks for doing this! I just have >> a couple of minor suggestions for this patch which might make the code >> clearer, feel free to take them or leave them. Either way, patches 3-5 >> are >> >> Reviewed-by: Michel Dänzer <michel.daenzer@xxxxxxx> > > On further thought, I wonder if maybe drmGetDevice[s]2 should default to > not retrieving the PCI revision, unless a flag is set, say > DRM_DEVICE_GET_PCI_REVISION? That would avoid unnecessarily using the > config file if the caller forgets to set the flag even though it doesn't > need the revision. > Not 100% sold on the reasoning (if someone forgets...) yet making the revision opt-in (as opposed to opt-out) makes sense. > Though in that case, maybe the revision_id field should also be set to > 0xff without the flag, to avoid callers forgetting to set the flag and > getting an incorrect but plausible(?) 0 for the revision. > All suggestions sound amazing, thank you ! Barring any objections, I'll re-spin the series and send one for Mesa later on today. Thanks again, Emil _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
