Re: [PATCH 2/2] drm/i915: Set up SDVO encoder type only at detect

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Apr 16, 2014 at 07:58:48AM +0200, Egbert Eich wrote:
> Daniel Vetter writes:
>  > On Mon, Apr 14, 2014 at 07:26:09PM +0200, Egbert Eich wrote:
>  > > Depending on the SDVO output_flags SDVO may have multiple connectors.
>  > > These are all initialized in intel_sdvo_output_setup(). The connector
>  > > that is initialized later will override the encoder_type that has been
>  > > set up by an earlier connector type initialization. Eventually the
>  > > one that comes last wins.
>  > > Eventually when intel_sdvo_detect() is called the active connector is
>  > > determined.
>  > > Delay encoder type initialization until the active connector is known
>  > > and set it to the type that corresponds to this connector.
>  > > 
>  > > Signed-off-by: Egbert Eich <eich@suse.de>
>  > 
>  > Hm, has this any effect on the code itself? I think if we want to fix this
>  > just for optics we should add a new DRM_MODE_ENCODER_MULTI. At least I
>  > have an sdvo here which has a dac, hdmi and tv-out ...
> 
> With the present logic the last connector in the list matching a flag bit
> will win the encoder type. The encoder type is presently just used for
> (debug) messages, so it is cosmetic.

I'm vary of changing the encoder type at runtime tbh. We use the same hack
in intel_ddi.c to switch between hdmi and dp mode, and the results aren't
pretty imo. For debug output imo adding a new "multi" encoder type is
better.

And if we need the type somehow to set up the right connector for sdvo
encoders with more than one detected output, then we need to track this
piece of state somewhere in our modeset state (either with the
connector->encoder links or with a bit of state in the pipe config
structure). Which also means we need state read-out and cross-check
support to make sure it really does what we want.

Storing such state in global structure tends to lead to subtile bugs and
prevent proper pre-compute/commit semantics for modeset changes.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch