On Fri, May 04, 2012 at 05:18:20PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <[email protected]>
> 
> This will allow us to disable an infoframe without changing its
> frequency.
> 
> Signed-off-by: Paulo Zanoni <[email protected]>
> Reviewed-by: Eugeni Dodonov <[email protected]>

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
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: [email protected]
Mobile: +41 (0)79 365 57 48
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to