On Mon, Nov 16, 2015 at 05:46:46PM +0000, Russell King - ARM Linux wrote: > On Mon, Nov 16, 2015 at 06:19:53PM +0100, Thierry Reding wrote: > > From: Thierry Reding <treding@xxxxxxxxxx> > > > > An encoder is associated with a connector by the DRM core as a result of > > setting up a configuration. Drivers using the atomic or legacy helpers > > should never set up this link, even if it is a static one. > > > > While at it, try to catch this kind of error in the future by adding a > > WARN_ON() in drm_mode_connector_attach_encoder(). Note that this doesn't > > cover all the cases, since drivers could set this up after attaching. > > Drivers that use the atomic helpers will get a warning later on, though, > > so hopefully the two combined cover enough to help people avoid this in > > the future. > > I'm pretty sure that when I started out writing armada-drm, > pre-initialising the connector's encoder was required, otherwise > DRM would oops. Has something changed which makes a NULL pointer > there at initialisation always safe? The drm core/helpers have always cleared drm_connector->encoder when disabling an output (see drm_crtc_helper_disable). So if you have indeed Oopsed because of this your driver would have likely oopsed after a modeset too. drm_connector->encoder was always just the dynamic association (but I think I've seen some implementation of ->best_encoder which got this wrong and just used drm_connector->encoder). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
