[Intel-gfx] [PATCH] drm/atomic: Acquire connection_mutex lock in drm_helper_probe_single_connector_modes.

[Maarten Lankhorst]

Op 29-03-17 om 15:31 schreef Boris Brezillon:
> On Wed, 29 Mar 2017 15:26:45 +0200
> Daniel Vetter <daniel at ffwll.ch> wrote:
>
>> On Wed, Mar 29, 2017 at 12:16:50PM +0200, Maarten Lankhorst wrote:
>>> mode_valid() and get_modes() called
>>> from drm_helper_probe_single_connector_modes()
>>> may need to look at connector->state because what a valid mode is may
>>> depend on connector properties being set. For example some HDMI modes
>>> might be rejected when a connector property forces the connector
>>> into DVI mode.
>>>
>>> Some implementations of detect() already lock all state,
>>> so we have to pass an acquire_ctx to them to prevent a deadlock.
>>>
>>> This means changing the function signature of detect() slightly,
>>> and passing the acquire_ctx for locking multiple crtc's.
>>> It might be NULL, in which case expensive operations should be avoided.
>>>
>>> intel_dp.c however ignores the force flag, so still lock
>>> connection_mutex there if needed.
>>>
>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>> Cc: Boris Brezillon <boris.brezillon at free-electrons.com>
>>> Cc: Manasi Navare <manasi.d.navare at intel.com>  
>> Hm only noticed this now, but mixing up force with the acquire_ctx sounds
>> like very bad interface design. Yes, the only user of the new hook works
>> like that, but that's really accidental I think. I think just having the
>> ctx everywhere (for atomic drivers at least) would be a lot safer. This is
>> extremely surprising (and undocumented suprise at that).
> Yes, I was about to say the same thing: the interface is not very
> clear, and I don't understand why ctx = NULL implies force = false.

They're the same thing I fear. I could perhaps call it force_ctx instead,
but non-zero ctx implies force, and other way around. Though I guess we could
relax it, and have force = true imply ctx, but not the other way around.

Would that be ok?