From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 1/4] drm/i915: handle input/output sdvo timings separately in mode_set Date: Tue, 10 Apr 2012 09:17:37 -0700 Message-ID: <20120410091737.4cfb056d@jbarnes-desktop> References: <1334058949-5633-1-git-send-email-daniel.vetter@ffwll.ch> <1334058949-5633-2-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1600507527==" Return-path: Received: from oproxy6-pub.bluehost.com (oproxy6-pub.bluehost.com [67.222.54.6]) by gabe.freedesktop.org (Postfix) with SMTP id C68FE9E7D5 for ; Tue, 10 Apr 2012 09:17:42 -0700 (PDT) In-Reply-To: <1334058949-5633-2-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: DRI@freedesktop.org, Intel Graphics Development , Development List-Id: intel-gfx@lists.freedesktop.org --===============1600507527== Content-Type: multipart/signed; micalg=PGP-SHA1; boundary="Sig_/iz=GBQAD3PPaNYVbsiSVb_9"; protocol="application/pgp-signature" --Sig_/iz=GBQAD3PPaNYVbsiSVb_9 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Tue, 10 Apr 2012 13:55:46 +0200 Daniel Vetter 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. >=20 > 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. >=20 > 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. >=20 > Fix this up when converting a drm mode to an sdvo dtd. >=20 > This regression was introduced in >=20 > commit c74696b9c890074c1e1ee3d7496fc71eb3680ced > Author: Pavel Roskin > Date: Thu Sep 2 14:46:34 2010 -0400 >=20 > i915: revert some checks added by commit 32aad86f >=20 > particularly the following hunk: >=20 > 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, >=20 > /* 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 =3D intel_sdvo->sdvo_flags; > - } else > - intel_sdvo_get_dtd_from_mode(&input_dtd, mode); >=20 > /* If it's a TV, we already set the output timing in mode_fixup. > * Otherwise, the output timing is equal to the input timing. >=20 > Reported-and-Tested-by: Bernard Blackham > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=3D48157 > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_sdvo.c | 34 ++++++++++++++++++-------------= --- > 1 files changed, 18 insertions(+), 16 deletions(-) >=20 > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/int= el_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; > =20 > width =3D mode->crtc_hdisplay; > height =3D mode->crtc_vdisplay; > @@ -747,7 +748,11 @@ static void intel_sdvo_get_dtd_from_mode(struct inte= l_sdvo_dtd *dtd, > h_sync_offset =3D mode->crtc_hsync_start - mode->crtc_hblank_start; > v_sync_offset =3D mode->crtc_vsync_start - mode->crtc_vblank_start; > =20 > - dtd->part1.clock =3D mode->clock / 10; > + mode_clock =3D mode->clock; > + mode_clock /=3D intel_mode_get_pixel_multiplier(mode) ?: 1; > + mode_clock /=3D 10; > + dtd->part1.clock =3D mode_clock; > + > dtd->part1.h_active =3D width & 0xff; > dtd->part1.h_blank =3D h_blank_len & 0xff; > dtd->part1.h_high =3D (((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 =3D 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 =3D intel_mode_get_pixel_multiplier(adjusted_mode); > int rate; > =20 > @@ -1023,20 +1028,13 @@ static void intel_sdvo_mode_set(struct drm_encode= r *encoder, > intel_sdvo->attached_output)) > return; > =20 > - /* 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 =3D 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); > =20 > /* 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; > =20 > + /* 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); > =20 > 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? --=20 Jesse Barnes, Intel Open Source Technology Center --Sig_/iz=GBQAD3PPaNYVbsiSVb_9 Content-Type: application/pgp-signature; name=signature.asc Content-Disposition: attachment; filename=signature.asc -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iQIcBAEBAgAGBQJPhF0hAAoJEIEoDkX4Qk9hgZkP/3oqzV08H+8Mra+pjMoo4lAi CPqe23D//9TH8yZNaSSx8y0hhdkZG23rkbxNs/QgYcJolPl12rtJJLKGqWK2d0Mv w8cAPcElo5CerUIcVUFqphXY/CIBCtdbAqDlhe1sOfvxdXlUhb+Osau5fbPzFRoJ vX9Qpv5Gm9YFWUBb0SPqUHmLxhskU7Tl8EoSrglIqGr2OFdJeBt7Fq3zHEEuoauc o5muIhuKVskY9OKxqRoDooXgV0hKIzc2oND7V2mBwygBBs4oMQQf3r7GLUd44SZZ sZib0nNCwOCH3sbFtxreCuLBgDHxF64ewkisTWVBV0h4w9HUHa205E+FlgDFH36l E0Sf+90GnpZ40wk/P9UMHFf7nkWnJPtRGz5cjuy39UhMYHBSjw4Kcpyw/jPAPNtA e6K71RFZcHffnOtI71dghd7+LJIDBhv6R4Qou6aYAA3KhpkS9Ocmm3UWfVWc6pSb qQuYlOLVeT3Vz/7dGcH/Y8vUanPE5bgphwUe6v5CrDedo7hbwkQ1YEgulhSCVhVq FWJwUCrqq2Y3EvrVehrc9Ev2mSwF99kTIKGKWphEyuk7ju5mciqhhhr7TjmoY0HJ xUyovVKTRJiSZO2C9DcoTqUm0dsmv/flaKnmu/J6hgEfSAu7zZeRfND3iBey7vzn 43jTXMMZXHGWlcIRPZX4 =8x78 -----END PGP SIGNATURE----- --Sig_/iz=GBQAD3PPaNYVbsiSVb_9-- --===============1600507527== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============1600507527==--