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@nvidia.com
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