Hi, On Thu, Feb 18, 2016 at 10:39:05PM +0100, Daniel Vetter wrote: > On Thu, Feb 18, 2016 at 9:34 PM, Lukas Wunner <lukas@xxxxxxxxx> wrote: > > > >> Ok, makes sense. I still think adding the check to the client_register > >> function would be good, just as a safety measure. > > > > Hm, the idea of calling vga_switcheroo_client_probe_defer() twice > > causes me pain in the stomach. It's surprising for drivers which > > just don't need it at the moment (amdgpu and snd_hda_intel) and > > it feels like overengineering and pampering driver developers > > beyond reasonable measure. Also while the single existing check is > > cheap, we might later on add checks that take more time and slow > > things down. > > It's motivated by Rusty's API Manifesto: > > http://sweng.the-davies.net/Home/rustys-api-design-manifesto Interesting, thank you. > With the mandatory check in _register we reach level 5 - it'll blow up > at runtime when we try to register it. The manifesto says "5. Do it right or it will always break at runtime". However even if we add a WARN_ON(vga_switcheroo_client_probe_defer(pdev)) to register_client(), it will not *always* spew a stacktrace but only on the machines this concerns (MacBook Pros). Since failure to defer breaks GPU switching, level 5 is already reached. Chances are this won't go unnoticed by the user. > For more context: We have tons of fun with EPROBE_DEFER handling > between i915 and snd-hda I don't understand, there is currently not a single occurrence of EPROBE_DEFER in i915, apart from the one I added. In sound/ there are 88 occurrences of EPROBE_DEFER in soc/, plus 1 in ppc/ and that's it. So not a single one in pci/hda/ where hda_intel.c resides. Is the fun with EPROBE_DEFER handling caused by the lack thereof? Best regards, Lukas _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
