[Intel-gfx] [PATCH] drm/i915: don't clobber the special upscaling lvds timings

[Eugeni Dodonov]

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

> This regression has been introduced in
>
> commit ca9bfa7eed20ea34e862804e62aae10eb159edbb
> Author: Daniel Vetter <daniel.vetter at 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 <jmdebruin at xmsnet.nl>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43071
> Signed-Off-by: Daniel Vetter <daniel.vetter at 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.dodonov at 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/>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20120415/e6a8c5c2/attachment.html>