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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxx>

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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux