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 <e...@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 ...

This also reminds that I should finally polish of the multi-sdvo fixes I
have around here - currently the last encoder detected wins on a
multi-encoder chip, which means if you have an lvds+hdmi combo and plug in
a screeen you'll never be able to use the lvds again until the hdmi is
unplugged.

Much worse if there's a tv-out detect issue ;-)

Cheers, Daniel

> ---
>  drivers/gpu/drm/i915/intel_sdvo.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c 
> b/drivers/gpu/drm/i915/intel_sdvo.c
> index d27155a..5043f16 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -154,6 +154,9 @@ struct intel_sdvo_connector {
>       /* Mark the type of connector */
>       uint16_t output_flag;
>  
> +     /* store encoder type for convenience */
> +     int encoder_type;
> +
>       enum hdmi_force_audio force_audio;
>  
>       /* This contains all current supported TV format */
> @@ -1746,6 +1749,7 @@ intel_sdvo_detect(struct drm_connector *connector, bool 
> force)
>       if (response == 0)
>               return connector_status_disconnected;
>  
> +     intel_sdvo->base.base.encoder_type = intel_sdvo_connector->encoder_type;
>       intel_sdvo->attached_output = response;
>  
>       intel_sdvo->has_hdmi_monitor = false;
> @@ -2489,7 +2493,9 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int 
> device)
>       } else {
>               intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | 
> DRM_CONNECTOR_POLL_DISCONNECT;
>       }
> -     encoder->encoder_type = DRM_MODE_ENCODER_TMDS;
> +     /* delay encoder_type setting until detection */
> +     intel_sdvo_connector->encoder_type = DRM_MODE_ENCODER_TMDS;
> +     encoder->encoder_type = DRM_MODE_ENCODER_NONE;
>       connector->connector_type = DRM_MODE_CONNECTOR_DVID;
>  
>       if (intel_sdvo_is_hdmi_connector(intel_sdvo, device)) {
> @@ -2524,7 +2530,9 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int 
> type)
>  
>       intel_connector = &intel_sdvo_connector->base;
>       connector = &intel_connector->base;
> -     encoder->encoder_type = DRM_MODE_ENCODER_TVDAC;
> +     /* delay encoder_type setting until detection */
> +     intel_sdvo_connector->encoder_type = DRM_MODE_ENCODER_TVDAC;
> +     encoder->encoder_type = DRM_MODE_ENCODER_NONE;
>       connector->connector_type = DRM_MODE_CONNECTOR_SVIDEO;
>  
>       intel_sdvo->controlled_output |= type;
> @@ -2568,7 +2576,9 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo, 
> int device)
>       intel_connector = &intel_sdvo_connector->base;
>       connector = &intel_connector->base;
>       intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT;
> -     encoder->encoder_type = DRM_MODE_ENCODER_DAC;
> +     /* delay encoder_type setting until detection */
> +     intel_sdvo_connector->encoder_type = DRM_MODE_ENCODER_DAC;
> +     encoder->encoder_type = DRM_MODE_ENCODER_NONE;
>       connector->connector_type = DRM_MODE_CONNECTOR_VGA;
>  
>       if (device == 0) {
> @@ -2603,7 +2613,9 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int 
> device)
>  
>       intel_connector = &intel_sdvo_connector->base;
>       connector = &intel_connector->base;
> -     encoder->encoder_type = DRM_MODE_ENCODER_LVDS;
> +     /* delay encoder_type setting until detection */
> +     intel_sdvo_connector->encoder_type = DRM_MODE_ENCODER_LVDS;
> +     encoder->encoder_type = DRM_MODE_ENCODER_NONE;
>       connector->connector_type = DRM_MODE_CONNECTOR_LVDS;
>  
>       if (device == 0) {
> @@ -2984,7 +2996,8 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t 
> sdvo_reg, bool is_sdvob)
>       /* encoder type will be decided later */
>       intel_encoder = &intel_sdvo->base;
>       intel_encoder->type = INTEL_OUTPUT_SDVO;
> -     drm_encoder_init(dev, &intel_encoder->base, &intel_sdvo_enc_funcs, 0);
> +     drm_encoder_init(dev, &intel_encoder->base, &intel_sdvo_enc_funcs,
> +                      DRM_MODE_ENCODER_NONE);
>  
>       /* Read the regs to test if we can talk to the device */
>       for (i = 0; i < 0x40; i++) {
> -- 
> 1.8.4.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to