diff mbox

drm/i915: Fix sdvo connector get_hw_state function

Message ID 1364920777-18426-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter April 2, 2013, 4:39 p.m. UTC
The active output is only the currently selected one, which does not
imply that it's actually enabled. Since we don't use the sdvo encoder
side dpms support, we need to check whether the chip-side sdvo port is
enabled instead.

v2: Fix up Bugzilla links.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=60138
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=63031
Cc: Egbert Eich <eich@pdx.freedesktop.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_sdvo.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Chris Wilson April 2, 2013, 5 p.m. UTC | #1
On Tue, Apr 02, 2013 at 06:39:37PM +0200, Daniel Vetter wrote:
> The active output is only the currently selected one, which does not
> imply that it's actually enabled. Since we don't use the sdvo encoder
> side dpms support, we need to check whether the chip-side sdvo port is
> enabled instead.
> 
> v2: Fix up Bugzilla links.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=60138
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=63031
> Cc: Egbert Eich <eich@pdx.freedesktop.org
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_sdvo.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 33b46d9..0e55bee 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1219,11 +1219,15 @@ 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;
> +	u32 tmp;
>  
> +	tmp = I915_READ(intel_sdvo->sdvo_reg);
>  	intel_sdvo_get_active_outputs(intel_sdvo, &active_outputs);
>  
> -	if (active_outputs & intel_sdvo_connector->output_flag)
> +	if ((active_outputs & intel_sdvo_connector->output_flag) &&
> +	    (tmp & SDVO_ENABLE))
>  		return true;
>  	else
>  		return false;

Would this function be tidier as:

/* Check if this SDVO pipe is on and connected to our output */
if (I915_READ(intel_sdvo->sdvo_reg) & SDVO_ENABLE)
   return false;

intel_sdvo_get_active_outputs(intel_sdvo, &active_outputs);
return (active_outputs & intel_sdvo_connector->output_flag) != 0;

At the very least the patch by shrunk by adjusting it to only add the
earlier check.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 33b46d9..0e55bee 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1219,11 +1219,15 @@  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;
+	u32 tmp;
 
+	tmp = I915_READ(intel_sdvo->sdvo_reg);
 	intel_sdvo_get_active_outputs(intel_sdvo, &active_outputs);
 
-	if (active_outputs & intel_sdvo_connector->output_flag)
+	if ((active_outputs & intel_sdvo_connector->output_flag) &&
+	    (tmp & SDVO_ENABLE))
 		return true;
 	else
 		return false;