Re: [PATCH 08/14] drm/i915: break intel_infoframe_flags into _enable and _frequency

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, May 04, 2012 at 05:18:20PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> This will allow us to disable an infoframe without changing its
> frequency.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

I'm a bit unhappy about intel_infoframe_frequency, a 15 line function to
essentially always return VIDEO_DIP_FREQ_VSYNC (at least for all the
cases). Furthermore the intel_ prefix for both these functions is a bit a
misdenomer because hsw will change all this. Are you ok if I smash a
bikeshed on top of your patches that replaces intel_infoframe_freq with
VIDEO_DIP_FREQ_VSYNC?

-Daniel
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c |   32 ++++++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 4ce5f1f..57ab62f 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -94,16 +94,33 @@ static u32 intel_infoframe_index(struct dip_infoframe *frame)
>  	return flags;
>  }
>  
> -static u32 intel_infoframe_flags(struct dip_infoframe *frame)
> +static u32 intel_infoframe_enable(struct dip_infoframe *frame)
>  {
>  	u32 flags = 0;
>  
>  	switch (frame->type) {
>  	case DIP_TYPE_AVI:
> -		flags |= VIDEO_DIP_ENABLE_AVI | VIDEO_DIP_FREQ_VSYNC;
> +		flags |= VIDEO_DIP_ENABLE_AVI;
>  		break;
>  	case DIP_TYPE_SPD:
> -		flags |= VIDEO_DIP_ENABLE_SPD | VIDEO_DIP_FREQ_VSYNC;
> +		flags |= VIDEO_DIP_ENABLE_SPD;
> +		break;
> +	default:
> +		DRM_DEBUG_DRIVER("unknown info frame type %d\n", frame->type);
> +		break;
> +	}
> +
> +	return flags;
> +}
> +
> +static u32 intel_infoframe_frequency(struct dip_infoframe *frame)
> +{
> +	u32 flags = 0;
> +
> +	switch (frame->type) {
> +	case DIP_TYPE_AVI:
> +	case DIP_TYPE_SPD:
> +		flags |= VIDEO_DIP_FREQ_VSYNC;
>  		break;
>  	default:
>  		DRM_DEBUG_DRIVER("unknown info frame type %d\n", frame->type);
> @@ -159,7 +176,8 @@ static void i9xx_write_infoframe(struct drm_encoder *encoder,
>  		data++;
>  	}
>  
> -	val |= intel_infoframe_flags(frame);
> +	val |= intel_infoframe_enable(frame);
> +	val |= intel_infoframe_frequency(frame);
>  
>  	I915_WRITE(VIDEO_DIP_CTL, val);
>  }
> @@ -190,7 +208,8 @@ static void ironlake_write_infoframe(struct drm_encoder *encoder,
>  		data++;
>  	}
>  
> -	val |= intel_infoframe_flags(frame);
> +	val |= intel_infoframe_enable(frame);
> +	val |= intel_infoframe_frequency(frame);
>  
>  	I915_WRITE(reg, val);
>  }
> @@ -221,7 +240,8 @@ static void vlv_write_infoframe(struct drm_encoder *encoder,
>  		data++;
>  	}
>  
> -	val |= intel_infoframe_flags(frame);
> +	val |= intel_infoframe_enable(frame);
> +	val |= intel_infoframe_frequency(frame);
>  
>  	I915_WRITE(reg, val);
>  }
> -- 
> 1.7.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48