On Tue, 10 Apr 2012 13:55:46 +0200
Daniel Vetter <daniel.vetter@xxxxxxxx> wrote:
> 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 c74696b9c890074c1e1ee3d7496fc71eb3680ced
> Author: Pavel Roskin <proski@xxxxxxx>
> 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.
>
> Reported-and-Tested-by: Bernard Blackham <b-linuxgit@xxxxxxxxxxxxxxxx>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=48157
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_sdvo.c | 34 ++++++++++++++++++----------------
> 1 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 6898145..ab47c1e 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -733,6 +733,7 @@ static void intel_sdvo_get_dtd_from_mode(struct intel_sdvo_dtd *dtd,
> uint16_t width, height;
> uint16_t h_blank_len, h_sync_len, v_blank_len, v_sync_len;
> uint16_t h_sync_offset, v_sync_offset;
> + int mode_clock;
>
> width = mode->crtc_hdisplay;
> height = mode->crtc_vdisplay;
> @@ -747,7 +748,11 @@ static void intel_sdvo_get_dtd_from_mode(struct intel_sdvo_dtd *dtd,
> h_sync_offset = mode->crtc_hsync_start - mode->crtc_hblank_start;
> v_sync_offset = mode->crtc_vsync_start - mode->crtc_vblank_start;
>
> - dtd->part1.clock = mode->clock / 10;
> + mode_clock = mode->clock;
> + mode_clock /= intel_mode_get_pixel_multiplier(mode) ?: 1;
> + mode_clock /= 10;
> + dtd->part1.clock = mode_clock;
> +
> dtd->part1.h_active = width & 0xff;
> dtd->part1.h_blank = h_blank_len & 0xff;
> dtd->part1.h_high = (((width >> 8) & 0xf) << 4) |
This hunk looks good.
> @@ -998,7 +1003,7 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
> struct intel_sdvo *intel_sdvo = to_intel_sdvo(encoder);
> u32 sdvox;
> struct intel_sdvo_in_out_map in_out;
> - struct intel_sdvo_dtd input_dtd;
> + struct intel_sdvo_dtd input_dtd, output_dtd;
> int pixel_multiplier = intel_mode_get_pixel_multiplier(adjusted_mode);
> int rate;
>
> @@ -1023,20 +1028,13 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
> intel_sdvo->attached_output))
> return;
>
> - /* We have tried to get input timing in mode_fixup, and filled into
> - * adjusted_mode.
> - */
> - if (intel_sdvo->is_tv || intel_sdvo->is_lvds) {
> - input_dtd = intel_sdvo->input_dtd;
> - } else {
> - /* Set the output timing to the screen */
> - if (!intel_sdvo_set_target_output(intel_sdvo,
> - intel_sdvo->attached_output))
> - return;
> -
> - intel_sdvo_get_dtd_from_mode(&input_dtd, adjusted_mode);
> - (void) intel_sdvo_set_output_timing(intel_sdvo, &input_dtd);
> - }
> + /* lvds has a special fixed output timing. */
> + if (intel_sdvo->is_lvds)
> + intel_sdvo_get_dtd_from_mode(&output_dtd,
> + intel_sdvo->sdvo_lvds_fixed_mode);
> + else
> + intel_sdvo_get_dtd_from_mode(&output_dtd, mode);
> + (void) intel_sdvo_set_output_timing(intel_sdvo, &output_dtd);
>
> /* Set the input timing to the screen. Assume always input 0. */
> if (!intel_sdvo_set_target_input(intel_sdvo))
> @@ -1054,6 +1052,10 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
> !intel_sdvo_set_tv_format(intel_sdvo))
> return;
>
> + /* We have tried to get input timing in mode_fixup, and filled into
> + * adjusted_mode.
> + */
> + intel_sdvo_get_dtd_from_mode(&input_dtd, adjusted_mode);
> (void) intel_sdvo_set_input_timing(intel_sdvo, &input_dtd);
>
> switch (pixel_multiplier) {
But seems mostly separate from this hunk, which I don't really
understand, not being an SDVO expert.
What happened to the tv check? Is input_dtd already set to the right
value here after the change?
--
Jesse Barnes, Intel Open Source Technology Center
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
