On Sat, Apr 14, 2012 at 13:17, Daniel Vetter <daniel.vet...@ffwll.ch> wrote:

> This regression has been introduced in
>
> commit ca9bfa7eed20ea34e862804e62aae10eb159edbb
> Author: Daniel Vetter <daniel.vet...@ffwll.ch>
> Date:   Sat Jan 28 14:49:20 2012 +0100
>
>    drm/i915: fixup interlaced vertical timings confusion, part 1
>
> Unfortunately that commit failed to take into account that the lvds
> code does some special adjustements to the crtc timings for upscaling
> an centering.
>
> Fix this by explicitly computing crtc timings in the lvds mode fixup
> function and setting a special flag in mode->private_flags if the crtc
> timings have been adjusted.
>
> Reported-and-Tested-by: Hans de Bruin <jmdebr...@xmsnet.nl>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43071
> Signed-Off-by: Daniel Vetter <daniel.vet...@ffwll.ch>
>

As for the patch itself, as far as I can tell, it does what it meant to, so:

Reviewed-by: Eugeni Dodonov <eugeni.dodo...@intel.com>

I have one small suggestion in one hunk below, but that's it.

-       /* All interlaced capable intel hw wants timings in frames. */
> -       drm_mode_set_crtcinfo(adjusted_mode, 0);
> +       /* All interlaced capable intel hw wants timings in frames. Note
> though
> +        * that intel_lvds_mode_fixup does some funny tricks with the crtc
> +        * timings, so we need to be careful not to clobber these.*/
> +       if (!(adjusted_mode->private_flags & INTEL_MODE_CRTC_TIMINGS_SET))
> +               drm_mode_set_crtcinfo(adjusted_mode, 0);
>

To simplify the life of next volunteers to touch those paths, perhaps we
should improve the comment either here, or at the
INTEL_MODE_CRTC_TIMING_SET definition, to mention that this flag should be
set by any encoder that does its timing config on its own and does not
wants his custom settings to get overwritten by the generic
intel_crtc_mode_fixup()
call?

-- 
Eugeni Dodonov
<http://eugeni.dodonov.net/>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to