[PATCH] drm: Do not set connector->encoder in drivers

Daniel Vetter daniel at ffwll.ch
Tue Nov 17 02:43:19 PST 2015


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 at 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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the dri-devel mailing list