On Wed, Feb 10, 2016 at 1:38 PM, David Herrmann dh.herrmann@gmail.com wrote:
Hi
On Wed, Feb 10, 2016 at 9:39 PM, Haixia Shi hshi@chromium.org wrote:
if (udl_device_is_unplugged(dev) &&nr != DRM_IOCTL_NR(DRM_IOCTL_MODE_SETCRTC) &&nr != DRM_IOCTL_NR(DRM_IOCTL_MODE_RMFB) &&nr != DRM_IOCTL_NR(DRM_IOCTL_MODE_DESTROY_DUMB))return -ENODEV;Why?
Just do:
if (udl_device_is_unplugged(dev)) return -ENODEV;Why this complex logic here?
Because there are legitimate ioctl calls after UDL is unplugged. See crbug.com/583508 and crbug.com/583758 for some background.
The userspace (Chrome in this case) has allocated FBs and Dumb buffers on the drm device and needs to be given a chance to properly deallocate them (via RMFB and DESTROY_DUMB). In addition, it needs to call SETCRTC with fb_id = 0 to properly release the last refcount on the primary fb.
I initially proposed adding an "UNPLUG_DISALLOW" flag to ioctls so that we can whitelist them on a case-by-case basis but that proposal got shot down as being unnecessary, but you can see my original patch at https://chromium-review.googlesource.com/#/c/326160/
If a device is unplugged, you should consider all your resources to be destroyed. There is no reason to release them manually. User-space *must* be able to deal with asynchronous unplugs.
So the problem if you do that is that things like a buffer's memory pages can disappear from under you. How would you deal with that case? User space certainly can't have a segfault handler catch that just in case :)
Stéphane
If UDL does not release your resources, and you actually hit bugs due to this, then fix UDL to do this. But extending the already broken UNPLUG-hacks we have sounds wrong to me.
Anyway, this change is unrelated to your patch, so please drop it. Feel free to send a separate patch, if you want to continue the discussion.
Thanks David _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel