diff mbox

[2/4] drm/i915/vlv: disable AVI infoframe emission when writing infoframes

Message ID 1396458534-23108-2-git-send-email-jbarnes@virtuousgeek.org (mailing list archive)
State New, archived
Headers show

Commit Message

Jesse Barnes April 2, 2014, 5:08 p.m. UTC
We also do a disable later when we write a specific infoframe, but here
we do it to prevent sending a stale one before updating the infoframes.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_hdmi.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Ville Syrjälä April 3, 2014, 10:33 a.m. UTC | #1
On Wed, Apr 02, 2014 at 10:08:52AM -0700, Jesse Barnes wrote:
> We also do a disable later when we write a specific infoframe, but here
> we do it to prevent sending a stale one before updating the infoframes.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index ee892a4..3b804fc 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -589,8 +589,8 @@ static void vlv_set_infoframes(struct drm_encoder *encoder,
>  	}
>  
>  	val |= VIDEO_DIP_ENABLE;
> -	val &= ~(VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT |
> -		 VIDEO_DIP_ENABLE_GCP);
> +	val &= ~(VIDEO_DIP_ENABLE_AVI | VIDEO_DIP_ENABLE_VENDOR |
> +		 VIDEO_DIP_ENABLE_GAMUT | VIDEO_DIP_ENABLE_GCP);

This goes against the rule of matching the ENABLE_AVI bit with
the DIP_ENABLE bit. But after reading the spec I think following
that rule is not actually needed as it applies only if we
enable/disable DIP while the port is already enabled. We never
do that AFAICS.

There's another rule that says we aren't even allowed to disable
the AVI infoframe while the port is enabled. The logic we have
in the write_infoframe functions goes agains that rule.

These two rules look like something that are two parts of the same
issue of always having the AVI infoframe enabled while the port is
enabled. And since the logic in the write_infoframe functions already
makes it impossible to update the AVI infoframe correctly on the fly,
I think we should try to change all platforms to do the obvious thing
that you're doing here for VLV. I don't like accumulating differences
between the platforms if we really don't have to.

But I suppose a bit of testing would be needed to make sure such
a change wouldn't break other platforms.

For this patch:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  	I915_WRITE(reg, val);
>  	POSTING_READ(reg);
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index ee892a4..3b804fc 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -589,8 +589,8 @@  static void vlv_set_infoframes(struct drm_encoder *encoder,
 	}
 
 	val |= VIDEO_DIP_ENABLE;
-	val &= ~(VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT |
-		 VIDEO_DIP_ENABLE_GCP);
+	val &= ~(VIDEO_DIP_ENABLE_AVI | VIDEO_DIP_ENABLE_VENDOR |
+		 VIDEO_DIP_ENABLE_GAMUT | VIDEO_DIP_ENABLE_GCP);
 
 	I915_WRITE(reg, val);
 	POSTING_READ(reg);