• Daniel Vetter's avatar
    drm/i915: handle input/output sdvo timings separately in mode_set · 6651819b
    Daniel Vetter authored
    We seem to have a decent confusion between the output timings and the
    input timings of the sdvo encoder. If I understand the code correctly,
    we use the original mode unchanged for the output timings, safe for
    the lvds case. And we should use the adjusted mode for input timings.
    
    Clarify the situation by adding an explicit output_dtd to the sdvo
    mode_set function and streamline the code-flow by moving the input and
    output mode setting in the sdvo encode together.
    
    Furthermore testing showed that the sdvo input timing needs the
    unadjusted dotclock, the sdvo chip will automatically compute the
    required pixel multiplier to get a dotclock above 100 MHz.
    
    Fix this up when converting a drm mode to an sdvo dtd.
    
    This regression was introduced in
    
    commit c74696b9
    Author: Pavel Roskin <proski@gnu.org>
    Date:   Thu Sep 2 14:46:34 2010 -0400
    
        i915: revert some checks added by commit 32aad86f
    
    particularly the following hunk:
    
    diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
    b/drivers/gpu/drm/i915/intel_sdvo.c
    index 093e914..62d22ae 100644
    --- a/drivers/gpu/drm/i915/intel_sdvo.c
    +++ b/drivers/gpu/drm/i915/intel_sdvo.c
    @@ -1122,11 +1123,9 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
    
         /* We have tried to get input timing in mode_fixup, and filled into
            adjusted_mode */
    -    if (intel_sdvo->is_tv || intel_sdvo->is_lvds) {
    -        intel_sdvo_get_dtd_from_mode(&input_dtd, adjusted_mode);
    +    intel_sdvo_get_dtd_from_mode(&input_dtd, adjusted_mode);
    +    if (intel_sdvo->is_tv || intel_sdvo->is_lvds)
             input_dtd.part2.sdvo_flags = intel_sdvo->sdvo_flags;
    -    } else
    -        intel_sdvo_get_dtd_from_mode(&input_dtd, mode);
    
         /* If it's a TV, we already set the output timing in mode_fixup.
          * Otherwise, the output timing is equal to the input timing.
    
    Due to questions raised in review, below a more elaborate analysis of
    the bug at hand:
    
    Sdvo seems to have two timings, one is the output timing which will be
    sent over whatever is connected on the other side of the sdvo chip (panel,
    hdmi screen, tv), the other is the input timing which will be generated by
    the gmch pipe. It looks like sdvo is expected to scale between the two.
    
    To make things slightly more complicated, we have a bunch of special
    cases:
    - For lvds panel we always use a fixed output timing, namely
      intel_sdvo->sdvo_lvds_fixed_mode, hence that special case.
    - Sdvo has an interface to generate a preferred input timing for a given
      output timing. This is the confusing thing that I've tried to clear up
      with the follow-on patches.
    - A special requirement is that the input pixel clock needs to be between
      100MHz and 200MHz (likely to keep it within the electromechanical design
      range of PCIe), 270MHz on later gen4+. Lower pixel clocks are
      doubled/quadrupled.
    
    The thing this patch tries to fix is that the pipe needs to be
    explicitly instructed to double/quadruple the pixels and needs the
    correspondingly higher pixel clock, whereas the sdvo adaptor seems to
    do that itself and needs the unadjusted pixel clock. For the sdvo
    encode side we already set the pixel mutliplier with a different
    command (0x21).
    
    This patch tries to fix this mess by:
    - Keeping the output mode timing in the unadjusted plain mode, safe
      for the lvds case.
    - Storing the input timing in the adjusted_mode with the adjusted
      pixel clock. This way we don't need to frob around with the core
      crtc mode set code.
    - Fixing up the pixelclock when constructing the sdvo dtd timing
      struct. This is why the first hunk of the patch is an integral part
      of the series.
    - Dropping the is_tv special case because input_dtd is equivalent to
      adjusted_mode after these changes. Follow-up patches clear this up
      further (by simply ripping out intel_sdvo->input_dtd because it's
      not needed).
    
    v2: Extend commit message with an in-depth bug analysis.
    Reported-and-Tested-by: default avatarBernard Blackham <b-linuxgit@largestprime.net>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=48157Reviewed-by: default avatarJesse Barnes <jbarnes@virtuousgeek.org>
    Cc: stable@kernel.org
    Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
    6651819b
intel_sdvo.c 78 KB