[Intel-gfx] [PATCH 1/2] drm/i915: Only break encoder linked when linked to connector

[Egbert Eich]

Chris Wilson writes:
 > On Mon, Apr 14, 2014 at 07:26:08PM +0200, Egbert Eich wrote:
 > > Depending on the SDVO output_flags SDVO may have multiple connectors
 > > linking to the same encoder (in intel_connector->encoder->base).
 > > Only one of those connectors should be active (ie link to the encoder
 > > thru drm_connector->encoder.
 > > If intel_connector_break_all_links() is called from intel_sanitize_crtc()
 > > we may brake the crtc connection of an encoder thru an inactive connector
 > > in which case intel_connector_break_all_links() will not be called again
 > > for the active connector due to
 > >    if (connector->encoder->base.crtc != &crtc->base)
 > >                                 continue;
 > > in intel_sanitize_crtc().
 > > This will however leave the drm_connector->encoder linkage for this
 > > active connector in place. Subsequently this will cause multiple
 > > warnings in intel_connector_check_state() to trigger and the driver
 > > will eventually die in drm_encoder_crtc_ok() (because of crtc == NULL).
 > > 
 > > To avoid this break the encoder links only if the connector is active
 > > (ie. has drm_connector->encoder set).
 > > 
 > > Signed-off-by: Egbert Eich <eich at suse.de>
 > 
 > This just leaves me with a nagging doubt that not all links will then be
 > broken. Do we have sufficient sanity checks to detect the obverse? Would
 > it be worth adding a second pass over the connector_list to assert that
 > all links are then broken?

Possibly. Maybe we should rework intel_sanitize_encoder() a bit:
loop over all drm_connectors, see if a drm_connector->encoder points
to the encoder in question and make sure that the intel_encoder state
(ie connectors_active and base.crtc) is in sync with the results.

I'll think about it some more ...

Cheers,
	Egbert.