<div class="gmail_quote">On Sat, Apr 14, 2012 at 13:17, Daniel Vetter <span dir="ltr"><<a href="mailto:daniel.vetter@ffwll.ch">daniel.vetter@ffwll.ch</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

This regression has been introduced in<br>
<br>
commit ca9bfa7eed20ea34e862804e62aae10eb159edbb<br>
Author: Daniel Vetter <<a href="mailto:daniel.vetter@ffwll.ch">daniel.vetter@ffwll.ch</a>><br>
Date:   Sat Jan 28 14:49:20 2012 +0100<br>
<br>
    drm/i915: fixup interlaced vertical timings confusion, part 1<br>
<br>
Unfortunately that commit failed to take into account that the lvds<br>
code does some special adjustements to the crtc timings for upscaling<br>
an centering.<br>
<br>
Fix this by explicitly computing crtc timings in the lvds mode fixup<br>
function and setting a special flag in mode->private_flags if the crtc<br>
timings have been adjusted.<br>
<br>
Reported-and-Tested-by: Hans de Bruin <<a href="mailto:jmdebruin@xmsnet.nl">jmdebruin@xmsnet.nl</a>><br>
Bugzilla: <a href="https://bugzilla.kernel.org/show_bug.cgi?id=43071" target="_blank">https://bugzilla.kernel.org/show_bug.cgi?id=43071</a><br>
Signed-Off-by: Daniel Vetter <<a href="mailto:daniel.vetter@ffwll.ch">daniel.vetter@ffwll.ch</a>><br></blockquote><div><br></div><div>As for the patch itself, as far as I can tell, it does what it meant to, so:</div>

<div><br></div><div>Reviewed-by: Eugeni Dodonov <<a href="mailto:eugeni.dodonov@intel.com">eugeni.dodonov@intel.com</a>></div><div><br></div><div>I have one small suggestion in one hunk below, but that's it.</div>

<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">-       /* All interlaced capable intel hw wants timings in frames. */<br>
-       drm_mode_set_crtcinfo(adjusted_mode, 0);<br>
+       /* All interlaced capable intel hw wants timings in frames. Note though<br>
+        * that intel_lvds_mode_fixup does some funny tricks with the crtc<br>
+        * timings, so we need to be careful not to clobber these.*/<br>
+       if (!(adjusted_mode->private_flags & INTEL_MODE_CRTC_TIMINGS_SET))<br>
+               drm_mode_set_crtcinfo(adjusted_mode, 0);<br></blockquote><div><br></div><div>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<span style> </span><span style>intel_crtc_mode_fixup() call?</span></div>

<div><br></div></div>-- <br>Eugeni Dodonov<a href="http://eugeni.dodonov.net/" target="_blank"><br></a><br>