On Wed, Mar 29, 2017 at 03:51:08PM +0200, Maarten Lankhorst wrote: > Op 29-03-17 om 15:31 schreef Boris Brezillon: > > On Wed, 29 Mar 2017 15:26:45 +0200 > > Daniel Vetter <daniel@xxxxxxxx> 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@xxxxxxxxxxxxxxx> > >>> Cc: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx> > >>> Cc: Manasi Navare <manasi.d.navare@xxxxxxxxx> > >> 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? Why can't we supply a ctx always? I didn't see any reason not to in the code ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel