[PATCH 2/2] drm: make unplugged flag specific to udl driver

[David Herrmann]

Hi

On Wed, Feb 10, 2016 at 9:39 PM, Haixia Shi <hshi at 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.

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