Re: [PATCH] drm/i915: Fix SDVO connector and encoder get_hw_state functions

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Apr 04, 2013 at 04:04:02PM -0400, Egbert Eich wrote:
> From: Egbert Eich <eich@suse.de>
> 
> The connector associated with the encoder is considered active when the
> output associtated with this connector is active on the encoder. The
> encoder itself is considered active when either there is an active
> output on  it or the  respective SDVO channel is active.
> Having active outputs when the SDVO channel is inactive seems to be
> inconsistent: such states can be found when intel_modeset_setup_hw_state()
> collects the hardware state set by the BIOS.
> This inconsistency will be fixed in intel_sanitize_crtc()
> (when intel_crtc_update_dpms() is called), this however only happens
> when the encoder is associated with a crtc.
> 
> This patch also reverts:
> 
>      commit bd6946e87a98fea11907b2a47368e13044458a35
>      Author: Daniel Vetter <daniel.vetter@ffwll.ch>
>      Date:   Tue Apr 2 21:30:34 2013 +0200
> 
>          drm/i915: Fix sdvo connector get_hw_state function
> 
> Signed-off-by: Egbert Eich <eich@suse.de>
> Suggested-By: Daniel Vetter <daniel.vetter@ffwll.ch>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=63031
Queued for -next with cc: stable tag added, thanks for the patch.
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_sdvo.c |    9 +++------
>  1 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 298dc85..f6a9f4a 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1231,12 +1231,8 @@ static bool intel_sdvo_connector_get_hw_state(struct intel_connector *connector)
>  	struct intel_sdvo_connector *intel_sdvo_connector =
>  		to_intel_sdvo_connector(&connector->base);
>  	struct intel_sdvo *intel_sdvo = intel_attached_sdvo(&connector->base);
> -	struct drm_i915_private *dev_priv = intel_sdvo->base.base.dev->dev_private;
>  	u16 active_outputs;
>  
> -	if (!(I915_READ(intel_sdvo->sdvo_reg) & SDVO_ENABLE))
> -		return false;
> -
>  	intel_sdvo_get_active_outputs(intel_sdvo, &active_outputs);
>  
>  	if (active_outputs & intel_sdvo_connector->output_flag)
> @@ -1251,11 +1247,13 @@ static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder,
>  	struct drm_device *dev = encoder->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base);
> +	u16 active_outputs;
>  	u32 tmp;
>  
>  	tmp = I915_READ(intel_sdvo->sdvo_reg);
> +	intel_sdvo_get_active_outputs(intel_sdvo, &active_outputs);
>  
> -	if (!(tmp & SDVO_ENABLE))
> +	if (!(tmp & SDVO_ENABLE) && (active_outputs == 0))
>  		return false;
>  
>  	if (HAS_PCH_CPT(dev))
> @@ -2746,7 +2744,6 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
>  	struct intel_sdvo *intel_sdvo;
>  	u32 hotplug_mask;
>  	int i;
> -
>  	intel_sdvo = kzalloc(sizeof(struct intel_sdvo), GFP_KERNEL);
>  	if (!intel_sdvo)
>  		return false;
> -- 
> 1.7.7
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch