[PATCH] drm/radeon/kms: remove useless radeon_ddc_dump()
Thomas Reim
reimth at googlemail.com
Mon Nov 28 08:50:20 PST 2011
Dear Alex,
see
http://lists.freedesktop.org/archives/dri-devel/2011-November/016934.html for a proposal.
I went back to the DVI detect function in radeon_connectors.c. The
reason for this is that during initial configuration of the framebuffer,
where we would like to put the log message output, function
drm_helper_probe_single_connector_modes() is called. Log output to the
user is performed already there: e. g. 'CONNECTOR HDMI-A-1' followed by
'HDMI-A-1 is disconnected' in case the (extended) DDC probe fails. The
DDC probe itself is triggered by calling the connector type specific
detect function, e. g. radeon_dvi_detect().
Therefore, I decided to leave the general DRM framebuffer initial
configuration functions untouched and adapt the Radeon connector
specific detect function(s) instead.
Hope you like it.
Best regards
Thomas
> On Tue, Nov 1, 2011 at 7:23 AM, Thomas Reim <reimth at googlemail.com> 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 patch
> > 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.
>
> 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.
>
> >
> > 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?
>
> 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.
>
> >
> > 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 info 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:"1280x1024" 60 108000 1280 1328 1440 1688 1024 1025 1028 1066 0x48 0x5
> >> [ 16.750776] [drm:drm_mode_debug_printmodeline], Modeline 26:"1280x1024" 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).
> >
>
> 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.
>
> > Would this work also for other users, especially the users with eDP, DP,
> > or DP bridge chips?
>
> Yes, it should.
>
> Alex
More information about the dri-devel
mailing list