On Tue, 10 Apr 2012 13:55:46 +0200 Daniel Vetter <daniel.vet...@ffwll.ch> 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 <pro...@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. > > Reported-and-Tested-by: Bernard Blackham <b-linux...@largestprime.net> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=48157 > Signed-off-by: Daniel Vetter <daniel.vet...@ffwll.ch> > --- > 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
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel