[PATCH 01/13] drm/fb-helper: Push down modeset lock into FB helpers

[Daniel Vetter]

On Thu, Jun 29, 2017 at 11:33 AM, Maarten Lankhorst
<maarten.lankhorst at linux.intel.com> wrote:
> Op 29-06-17 om 11:23 schreef Daniel Vetter:
>> On Thu, Jun 29, 2017 at 11:10 AM, Maarten Lankhorst
>> <maarten.lankhorst at linux.intel.com> wrote:
>>> Op 27-06-17 om 16:59 schreef Daniel Vetter:
>>>> From: Thierry Reding <treding at nvidia.com>
>>>>
>>>> Move the modeset locking from drivers into FB helpers.
>>>>
>>>> v2: Also handle intel_connector_add_to_fbdev.
>>>>
>>>> Tested-by: John Stultz <john.stultz at linaro.org>
>>>> Signed-off-by: Thierry Reding <treding at nvidia.com> (v1)
>>>> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>>>> ---
>>>>  drivers/gpu/drm/drm_fb_helper.c        | 40 +++++++++++++++++++++++++++++-----
>>>>  drivers/gpu/drm/i915/intel_dp_mst.c    |  6 -----
>>>>  drivers/gpu/drm/radeon/radeon_dp_mst.c |  7 ------
>>>>  3 files changed, 34 insertions(+), 19 deletions(-)
>>> I know we suck at DP-MST. But I fear the unregister leaves open a race with mst_port being unset without connection_mutex..
>>>
>>> best_encoder() and mode_valid() appear to use it. It's probably harmless and I have no good solution, so maybe just annotate it in the patch? Might also affect i915_hpd_poll_init_work, though I think the race in itself is harmless.
>> Hm, but this did race even before that already ... Should I just wrap
>> the mst_port = NULL in the connection_mutex? With a big warning that
>> we should have proper refcounting for this instead (both connectors
>> and all the mst things are refcounted already).
>> -Daniel
>
> I think leave open the race, the DP-MST modeset code itself doesn't use the connector->mst_port, just the detect callbacks do. In case of nonblocking modeset mst_port might already fall away so it's not like the race is any worse now.
>
> I don't even know how to solve it, except by not unplugging. :)

Why does grabbing the lock not fix the race? ->detect eventually calls
drm_dp_get_validated_port_ref, which will bail on a zombie mst port..
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch