From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Reim Subject: Re: [PATCH] drm/radeon/kms: remove useless radeon_ddc_dump() Date: Tue, 01 Nov 2011 15:53:07 +0100 Message-ID: <1320159187.4247.73.camel@Mark-Aurel.gas.de> References: <1319585047-1529-1-git-send-email-alexdeucher@gmail.com> <1319892930.14718.19.camel@Mark-Aurel.gas.de> <1320074105.4172.51.camel@Mark-Aurel.gas.de> <1320146596.4247.60.camel@Mark-Aurel.gas.de> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0028641707==" Return-path: Received: from mail-ey0-f177.google.com (mail-ey0-f177.google.com [209.85.215.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 8386D9E762 for ; Tue, 1 Nov 2011 07:53:19 -0700 (PDT) Received: by eye3 with SMTP id 3so7153761eye.36 for ; Tue, 01 Nov 2011 07:53:18 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Alex Deucher Cc: Alex Deucher , dri-devel@lists.freedesktop.org, Thomas Reim List-Id: dri-devel@lists.freedesktop.org --===============0028641707== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-RawtOU6fxrMRLfrECXPD" --=-RawtOU6fxrMRLfrECXPD Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Dear Alex, sounds reasonable: it really should be sufficient to have the EDID dumps of to be removed function radeon_ddc_dump() if at all as debug information during fb initialisation. Let's see if there will be a nice solution.=20 Best regards Thomas Am Dienstag, den 01.11.2011, 10:01 -0400 schrieb Alex Deucher: > On Tue, Nov 1, 2011 at 7:23 AM, Thomas Reim wrote= : > > Dear Alex, > > > > I think we've got the point. The users with improperly terminated i2c > > busses suffered a long time from flooded logs and terminals. We solved > > the problem by introducing the extended DDC probe check, which will be > > according to your other patch the default for all implementations. We > > reduced the log entry flood to one entry that shows that the connector > > cannot be used (due to invalid EDID) plus four EDID dumps. Now the patc= h > > here will also remove this one log entry. And we come to the point, > > where there's no such information at all, as for most other users > > (except of those that still use monitors with wrong EDIDs). > > > > Believe me, we still need to get used to this "normal" driver > > behaviour. >=20 > User's will still realize they can't use the monitor since it will > listed as disconnected. Is there really a lot of value in printing a > garbage EDID dump due to an improperly terminated i2c line? Now that > you fixed the probe function, it becomes a moot point I think. > Monitors with bad EDID checksums, etc. will still get logged. I'd > argue that we should still use those too as in most cases the EDID is > still valid, but that is another discussion. >=20 > > > > Nevertheless, I checked drm_fb_helper_initial_config() in more detail. > > Whereas drm_get_edid() logs on kernel error level in case of (EDID) > > problems, the to be removed function radeon_ddc_dump() adds information > > on info level, the functions called by drm_fb_helper_initial_config() > > stay on debug level. I switched on drm.debug and checked the logs. > > You're right, most of the required information is there, but requires > > drm.debug to be enabled. > > > > The question that you also asked in your previous mail and that remains > > is, how important is it to inform the users about the connector status > > during driver load? >=20 > Most users never look at their kernel log unless there is a problem in > which case they'll file a bug report and we can request debug output. >=20 > > > > In the following you find a proposal how the (new) log could look like > > after adding and modifying some logic in the > > drm_fb_helper_initial_config() function: > > > >> [ 14.912386] [drm] Radeon Display Connectors > >> [ 14.912389] [drm] Connector 0: > >> [ 14.912391] [drm] VGA > >> [ 14.912394] [drm] DDC: 0x7e50 0x7e40 0x7e54 0x7e44 0x7e58 0x7e48 = 0x7e5c 0x7e4c > >> [ 14.912397] [drm] Encoders: > >> [ 14.912398] [drm] CRT1: INTERNAL_KLDSCP_DAC1 > >> [ 14.912401] [drm] Connector 1: > >> [ 14.912402] [drm] S-video > >> [ 14.912404] [drm] Encoders: > >> [ 14.912405] [drm] TV1: INTERNAL_KLDSCP_DAC1 > >> [ 14.912407] [drm] Connector 2: > >> [ 14.912409] [drm] HDMI-A > >> [ 14.912410] [drm] HPD2 > >> [ 14.912413] [drm] DDC: 0x7e40 0x7e60 0x7e44 0x7e64 0x7e48 0x7e68 = 0x7e4c 0x7e6c > >> [ 14.912415] [drm] Encoders: > >> [ 14.912417] [drm] DFP2: INTERNAL_DDI > >> [ 14.912419] [drm] Connector 3: > >> [ 14.912421] [drm] DVI-D > >> [ 14.912423] [drm] DDC: 0x7e40 0x7e50 0x7e44 0x7e54 0x7e48 0x7e58 = 0x7e4c 0x7e5c > >> [ 14.912425] [drm] Encoders: > >> [ 14.912427] [drm] DFP3: INTERNAL_LVTM1 > >> > >> ... > > // Information that is already on debug log level changed to kernel inf= o level > >> [ 15.088976] [drm] Probing connector VGA-1 ... > >> [ 15.100040] [drm] VGA-1 is disconnected > >> [ 15.200048] [drm] Probing connector SVIDEO-1 ... > >> [ 15.390040] [drm] SVIDEO-1 is disconnected > >> [ 15.390048] [drm] Probing connector HDMI-A-1 ... > > // NEW: drm_get_edid output in case of EDID problems: > >> [ 15.470734] Raw EDID: > >> [ 15.470745] <3>00 00 00 00 00 00 00 00 00 00 00 07 00 00 00 00 ...= ............. > >> [ 15.470748] <3>00 00 00 00 00 00 00 00 00 00 07 00 00 00 00 00 ...= ............. > >> [ 15.470751] <3>00 00 00 00 00 00 00 00 00 00 7f 00 00 00 00 00 ...= ............. > >> [ 15.470754] <3>00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ...= ............. > >> [ 15.470757] <3>00 00 00 00 1f 00 00 00 00 00 00 00 00 00 00 00 ...= ............. > >> [ 15.470760] <3>00 00 00 00 00 07 00 00 00 00 00 7f 00 00 00 00 ...= ............. > >> [ 15.470762] <3>00 00 00 00 00 00 07 00 00 00 00 00 00 00 00 00 ...= ............. > >> [ 15.470765] <3>00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 ...= ............. > >> [ 15.470767] > >> [ 15.779568] Raw EDID: > >> [ 15.779578] <3>00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ...= ............. > >> [ 15.779581] <3>00 00 00 00 00 7f 00 00 00 00 03 00 00 00 00 00 ...= ............. > >> [ 15.779584] <3>00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ...= ............. > >> [ 15.779587] <3>00 00 00 00 ff 00 00 00 00 00 00 00 00 00 00 00 ...= ............. > >> [ 15.779590] <3>00 00 00 00 00 00 00 00 ff 00 00 00 00 00 00 00 ...= ............. > >> [ 15.779593] <3>00 00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 ...= ............. > >> [ 15.779596] <3>00 00 00 7f 00 00 00 00 00 03 07 00 00 00 00 00 ...= ............. > >> [ 15.779598] <3>00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ...= ............. > >> [ 15.779600] > >> [ 16.151817] Raw EDID: > >> [ 16.151827] <3>00 00 00 00 00 00 1f 00 00 00 00 00 00 00 00 00 ...= ............. > >> [ 16.151830] <3>00 00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 ...= ............. > >> [ 16.151833] <3>00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ...= ............. > >> [ 16.151836] <3>00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ...= ............. > >> [ 16.151839] <3>00 00 1f 00 00 00 00 00 00 00 00 00 00 00 0f 00 ...= ............. > >> [ 16.151842] <3>01 00 00 00 00 00 00 00 01 00 00 00 00 07 00 ff ...= ............. > >> [ 16.151844] <3>00 00 00 00 00 00 ff 00 00 00 00 00 00 00 7f 00 ...= ............. > >> [ 16.151847] <3>00 0f 00 00 00 00 00 00 00 3f 00 00 00 00 00 00 ...= ......?...... > >> [ 16.151849] > >> [ 16.444209] Raw EDID: > >> [ 16.444220] <3>00 07 00 00 00 00 00 00 ff 00 00 00 00 ff 00 00 ...= ............. > >> [ 16.444223] <3>00 00 3f 00 00 00 00 00 00 00 00 00 00 ff 00 00 ..?= ............. > >> [ 16.444226] <3>00 00 00 00 00 00 00 00 00 07 00 00 00 01 0f 00 ...= ............. > >> [ 16.444229] <3>00 07 00 00 00 00 00 00 00 00 01 07 00 00 00 00 ...= ............. > >> [ 16.444231] <3>00 00 00 00 00 00 00 00 7f 00 00 00 00 1f 00 00 ...= ............. > >> [ 16.444234] <3>00 00 03 00 00 00 00 3f 00 03 00 00 00 00 00 00 ...= ....?........ > >> [ 16.444237] <3>7f 00 00 1f 00 00 00 00 00 00 00 00 0f 07 00 00 ...= ............. > >> [ 16.444240] <3>00 00 00 00 00 00 00 00 00 00 03 00 00 00 00 00 ...= ............. > >> [ 16.444248] radeon 0000:01:05.0: HDMI-A-1: EDID block 0 invalid. > > // Existing information on debug level, shifted to info level > >> [ 16.694378] [drm] HDMI-A-1 is disconnected > >> [ 16.694382] [drm] Probing connector DVI-D-1 ... > >> [ 16.750763] [drm] Probed modes for DVI-D-1 > > // If drm.debug is enabled, mode information would follow here > >> [ 16.750769] [drm:drm_mode_debug_printmodeline], Modeline 17:"1280x1= 024" 60 108000 1280 1328 1440 1688 1024 1025 1028 1066 0x48 0x5 > >> [ 16.750776] [drm:drm_mode_debug_printmodeline], Modeline 26:"1280x1= 024" 75 135000 1280 1296 1440 1688 1024 1025 1028 1066 0x40 0x5 > >> > >> ... > >> > >> [ 17.570156] [drm] fb mappable at 0xF0040000 > >> > > > > Continuing from here we would have nothing in the logs, except for the > > case, that we retrieve a wrong EDID with correct EDID header (0x00 0xFF > > 0xFF 0xFF 0xFF 0xFF 0xFF 0x00). > > >=20 > If you want to add better debugging output, that'd be great. While > you are at it, it would be nice to print which modes are rejected by > the driver's mode_valid functions. >=20 > > Would this work also for other users, especially the users with eDP, DP= , > > or DP bridge chips? >=20 > Yes, it should. >=20 > Alex --=-RawtOU6fxrMRLfrECXPD Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) iQEcBAABAgAGBQJOsAfHAAoJEII1bee/l5G+JLkH/2GZy7QFGnn1NwDQwhkUCCVP Im9ShWcGROFXMMVJIL1vqbObYGYV5xoIOYlT8G1pfjetzgjBp4SZdtkAR9nHeeap nkLxubzG3Bn+F6qpUWxRVK24MOuSXsdvAzfnG7Dji32puzxNOuJ+LXYb6A9YzXMq f77QrLbPTPam+7v64xrKUFPsPZ2YV0XwMNSRGeyxcWpXr8TFwJjhmR54FmSRwr1N k/TGLIqkRlSHRtTj6GN5p+qyvHgByFAH9xImTHKsYETilTtjNsbE/WjX3Uo0+kRh mP0D3GGsFhb7DidT9LIe/oV5B9e6iPrGQMrGv9WTG0CU2c8gmLKAJqPQwgvhZzM= =IBj8 -----END PGP SIGNATURE----- --=-RawtOU6fxrMRLfrECXPD-- --===============0028641707== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --===============0028641707==--