Re: [PATCH] drm/radeon/kms: remove useless radeon_ddc_dump()
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 rei...@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] 300 00 00 00 00 00 00 00 00 00 00 07 00 00 00 00 [ 15.470748] 300 00 00 00 00 00 00 00 00 00 07 00 00 00 00 00 [ 15.470751] 300 00 00 00 00 00 00 00 00 00 7f 00 00 00 00 00 [ 15.470754] 300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 15.470757] 300 00 00 00 1f 00 00 00 00 00 00 00 00 00 00 00 [ 15.470760] 300 00 00 00 00 07 00 00 00 00 00 7f 00 00 00 00 [ 15.470762] 300 00 00 00 00 00
Re: [PATCH] drm/radeon/kms: remove useless radeon_ddc_dump()
On Tue, Nov 1, 2011 at 7:23 AM, Thomas Reim rei...@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] 300 00 00 00 00 00 00 00 00 00 00 07 00 00 00 00 [ 15.470748] 300 00 00 00 00 00 00 00 00 00 07 00 00 00 00 00 [ 15.470751] 300 00 00 00 00 00 00 00 00 00 7f 00 00 00 00 00 [ 15.470754] 300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 15.470757] 300 00 00 00 1f 00 00 00 00 00 00 00 00 00 00 00 [ 15.470760] 300 00 00 00 00 07 00 00 00 00 00 7f 00 00 00 00 [ 15.470762] 300 00 00 00 00 00 07 00 00 00 00 00 00 00 00 00 [ 15.470765] 300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 [ 15.470767] [ 15.779568] Raw EDID: [ 15.779578] 300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 15.779581] 300 00 00 00 00 7f 00 00 00 00 03 00 00 00 00 00 [ 15.779584] 300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 15.779587] 300 00 00 00 ff 00 00 00 00 00 00 00 00 00 00 00 [ 15.779590] 300 00 00 00 00 00 00 00 ff 00 00 00 00 00 00 00 [ 15.779593] 300 00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 [ 15.779596] 300 00 00 7f 00 00 00 00 00 03 07 00 00 00 00 00 [ 15.779598] 300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 15.779600] [ 16.151817] Raw EDID: [ 16.151827] 300 00 00 00 00 00 1f 00
Re: [PATCH] drm/radeon/kms: remove useless radeon_ddc_dump()
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. 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 rei...@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] 300 00 00 00 00 00 00 00 00 00 00 07 00 00 00 00 [ 15.470748] 300 00 00 00 00 00 00 00 00 00 07 00 00 00 00 00 [ 15.470751] 300 00 00 00 00 00 00 00 00 00 7f 00 00 00 00 00 [ 15.470754] 300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 15.470757] 300 00 00 00 1f 00 00 00 00 00 00 00 00 00 00 00 [ 15.470760] 300 00 00 00 00 07 00 00 00 00 00 7f 00 00 00 00 [ 15.470762] 300 00 00 00 00 00 07 00 00 00 00 00 00 00 00 00 [ 15.470765] 300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 [ 15.470767] [ 15.779568] Raw EDID: [ 15.779578] 300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 15.779581] 300 00 00 00 00 7f 00 00 00 00 03 00 00 00 00 00 [ 15.779584] 300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 15.779587] 300 00 00 00 ff 00 00 00 00 00 00 00 00 00 00 00
Re: [PATCH] drm/radeon/kms: remove useless radeon_ddc_dump()
Dear Alex,
your reply confuses me:
On Sat, Oct 29, 2011 at 8:55 AM, Thomas Reim rei...@googlemail.com wrote:
Dear Alex,
but we use DDC probing e. g. to identify connectors with improperly
terminated i2c bus. Instead of flooding logs and terminals with EDID dumps,
we decided some months ago to use this function during module loading to
inform the user once (and only once!), which connector has a monitor
connected with valid EDID and which connector has not.
There's nothing in that function that actually prevents the printing
of bad EDIDs.
That's true.
All it does is call drm_get_edid() and report whether
it found an EDID or not. radeon_dvi_detect() already takes into
account the requires_extended_probe flag.
The extended probe flag will prevent the radeon driver from further
useless printing of bad EDIDs every ten seconds:
in radeon_connectors.c:
if (radeon_connector-ddc_bus)
dret = radeon_ddc_probe(radeon_connector,
radeon_connector-requires_extended_probe);
if (dret) {
if (radeon_connector-edid) {
kfree(radeon_connector-edid);
radeon_connector-edid = NULL;
}
radeon_connector-edid = drm_get_edid(radeon_connector-base,
radeon_connector-ddc_bus-adapter);
But the now to be removed function radeon_ddc_dump() took care during
connector setup that at least one (!) dump was in the logs, accompagnied
by the info, that no monitor is connected, or there is a wrong/buggy
EDID.
in radeon_display.c:
radeon_print_display_setup(dev);
list_for_each_entry(drm_connector,
dev-mode_config.connector_list, head)
radeon_ddc_dump(drm_connector);
I cannot imagine why this should confuse users. Having retrieved and
configured connectors plus info plus EDID dump in case of problems
logged next to each other was perfect.
The connectors are probed by
the fb code for the console as well so it just adds to the module load
time. If we want to print what connectors are connected, it should be
done from the fb code so we only have to do it once.
I briefly checked the code, but couldn't find a promising connector
probing function in the fb code. Did you mean function
drm_fb_helper_probe_connector_modes?
Graphic solutions in general have several connectors integrated, and it's
sometimes really difficult to identify, which one of the does not work as
expected, based on the logs. From above log we see, that a monitor is
connected to DVI connector, nothing is connected to the VGA connector, and
we have a problem with the HDMI connector. Without this fuinction, nothing
helpful from radeon module would be in the logs.
We should print the connector name in the generic drm error code then
if we want to print this info at boot time.
Is there a place that is not called every ten seconds?
Maybe we can keep this function, but call it only for connectors, for which
it works, i. e. not for DP, eDP and DP bridge connectors.
That's just as bad. Users won't understand why only certain
connectors are probed.
Best regards
Thomas Reim
Am Dienstag, den 25.10.2011, 19:24 -0400 schrieb alexdeuc...@gmail.com:
From: Alex Deucher alexander.deuc...@amd.com
The function didn't work with DP, eDP, or DP bridge
connectors and thus confused users as it lead them to
believe nothing was connected or the EDID was invalid
when in fact is was, just on the aux bus rather an i2c.
It should also speed up module loading as it avoids a
bunch of extra DDC probing.
Signed-off-by: Alex Deucher alexander.deuc...@amd.com
---
drivers/gpu/drm/radeon/radeon_display.c | 33
---
1 files changed, 0 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_display.c
b/drivers/gpu/drm/radeon/radeon_display.c
index 6adb3e5..4998736 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -33,8 +33,6 @@
#include drm_crtc_helper.h
#include drm_edid.h
-static int radeon_ddc_dump(struct drm_connector *connector);
-
static void avivo_crtc_load_lut(struct drm_crtc *crtc)
{
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
@@ -669,7 +667,6 @@ static void radeon_print_display_setup(struct drm_device
*dev)
static bool radeon_setup_enc_conn(struct drm_device *dev)
{
struct radeon_device *rdev = dev-dev_private;
- struct drm_connector *drm_connector;
bool ret = false;
if (rdev-bios) {
@@ -689,8 +686,6 @@ static bool radeon_setup_enc_conn(struct drm_device
*dev)
if (ret) {
radeon_setup_encoder_clones(dev);
radeon_print_display_setup(dev);
- list_for_each_entry(drm_connector,
dev-mode_config.connector_list,
head)
-
Re: [PATCH] drm/radeon/kms: remove useless radeon_ddc_dump()
On Mon, Oct 31, 2011 at 11:15 AM, Thomas Reim rei...@googlemail.com wrote:
Dear Alex,
your reply confuses me:
On Sat, Oct 29, 2011 at 8:55 AM, Thomas Reim rei...@googlemail.com wrote:
Dear Alex,
but we use DDC probing e. g. to identify connectors with improperly
terminated i2c bus. Instead of flooding logs and terminals with EDID dumps,
we decided some months ago to use this function during module loading to
inform the user once (and only once!), which connector has a monitor
connected with valid EDID and which connector has not.
There's nothing in that function that actually prevents the printing
of bad EDIDs.
That's true.
All it does is call drm_get_edid() and report whether
it found an EDID or not. radeon_dvi_detect() already takes into
account the requires_extended_probe flag.
The extended probe flag will prevent the radeon driver from further
useless printing of bad EDIDs every ten seconds:
Yes, the extended probe check will prevent the spewing of probe
failures, so why do we need to fetch all the edids again at load? It
just adds latency.
in radeon_connectors.c:
if (radeon_connector-ddc_bus)
dret = radeon_ddc_probe(radeon_connector,
radeon_connector-requires_extended_probe);
if (dret) {
if (radeon_connector-edid) {
kfree(radeon_connector-edid);
radeon_connector-edid = NULL;
}
radeon_connector-edid = drm_get_edid(radeon_connector-base,
radeon_connector-ddc_bus-adapter);
But the now to be removed function radeon_ddc_dump() took care during
connector setup that at least one (!) dump was in the logs, accompagnied
by the info, that no monitor is connected, or there is a wrong/buggy
EDID.
Ok, I see what you are saying. I'm not sure how important it is that
we print that out. If we have an improperly terminated i2c line,
you'll either get an edid or garbage and we already cover that with
the extended edid probe check.
in radeon_display.c:
radeon_print_display_setup(dev);
list_for_each_entry(drm_connector,
dev-mode_config.connector_list, head)
radeon_ddc_dump(drm_connector);
I cannot imagine why this should confuse users. Having retrieved and
configured connectors plus info plus EDID dump in case of problems
logged next to each other was perfect.
It's confusing because on systems with eDP, DP, or DP bridge chips
(VGA and LVDS bridges) users get messages saying that the connectors
are disconnected or have a bad edid and then they file bugs that the
driver can''t detect their monitor when the monitor is detected just
fine, it's just on aux rather than i2c. It also adds latency to boot
since you now have two rounds of connector probing; once for the
ddc_dump, and once for the fb code. If we only dump information for
certain connectors it is also confusing since users will not see some
of the connectors checked at boot while others will be. We could add
additional logic for the DP cases, but you'd need to drag in most of
the logic from the dp detect functions in order to determine whether
the DP port is in DP or legacy mode and then decide whether or not to
use aux or i2c; all of which just adds more boot latency considering
we already have to do this for the fb setup.
The connectors are probed by
the fb code for the console as well so it just adds to the module load
time. If we want to print what connectors are connected, it should be
done from the fb code so we only have to do it once.
I briefly checked the code, but couldn't find a promising connector
probing function in the fb code. Did you mean function
drm_fb_helper_probe_connector_modes?
yes, via drm_fb_helper_initial_config()
Graphic solutions in general have several connectors integrated, and it's
sometimes really difficult to identify, which one of the does not work as
expected, based on the logs. From above log we see, that a monitor is
connected to DVI connector, nothing is connected to the VGA connector, and
we have a problem with the HDMI connector. Without this fuinction, nothing
helpful from radeon module would be in the logs.
We should print the connector name in the generic drm error code then
if we want to print this info at boot time.
Is there a place that is not called every ten seconds?
You could add logic to the fb probe code.
Maybe we can keep this function, but call it only for connectors, for which
it works, i. e. not for DP, eDP and DP bridge connectors.
That's just as bad. Users won't understand why only certain
connectors are probed.
Best regards
Thomas Reim
Am Dienstag, den 25.10.2011, 19:24 -0400 schrieb alexdeuc...@gmail.com:
From: Alex Deucher alexander.deuc...@amd.com
The function didn't work with DP, eDP, or DP bridge
connectors and thus confused users as it lead them to
believe nothing was
Re: [PATCH] drm/radeon/kms: remove useless radeon_ddc_dump()
On Sat, Oct 29, 2011 at 8:55 AM, Thomas Reim rei...@googlemail.com wrote:
Dear Alex,
but we use DDC probing e. g. to identify connectors with improperly
terminated i2c bus. Instead of flooding logs and terminals with EDID dumps,
we decided some months ago to use this function during module loading to
inform the user once (and only once!), which connector has a monitor
connected with valid EDID and which connector has not.
There's nothing in that function that actually prevents the printing
of bad EDIDs. All it does is call drm_get_edid() and report whether
it found an EDID or not. radeon_dvi_detect() already takes into
account the requires_extended_probe flag. The connectors are probed by
the fb code for the console as well so it just adds to the module load
time. If we want to print what connectors are connected, it should be
done from the fb code so we only have to do it once.
Graphic solutions in general have several connectors integrated, and it's
sometimes really difficult to identify, which one of the does not work as
expected, based on the logs. From above log we see, that a monitor is
connected to DVI connector, nothing is connected to the VGA connector, and
we have a problem with the HDMI connector. Without this fuinction, nothing
helpful from radeon module would be in the logs.
We should print the connector name in the generic drm error code then
if we want to print this info at boot time.
Maybe we can keep this function, but call it only for connectors, for which
it works, i. e. not for DP, eDP and DP bridge connectors.
That's just as bad. Users won't understand why only certain
connectors are probed.
Best regards
Thomas Reim
Am Dienstag, den 25.10.2011, 19:24 -0400 schrieb alexdeuc...@gmail.com:
From: Alex Deucher alexander.deuc...@amd.com
The function didn't work with DP, eDP, or DP bridge
connectors and thus confused users as it lead them to
believe nothing was connected or the EDID was invalid
when in fact is was, just on the aux bus rather an i2c.
It should also speed up module loading as it avoids a
bunch of extra DDC probing.
Signed-off-by: Alex Deucher alexander.deuc...@amd.com
---
drivers/gpu/drm/radeon/radeon_display.c | 33
---
1 files changed, 0 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_display.c
b/drivers/gpu/drm/radeon/radeon_display.c
index 6adb3e5..4998736 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -33,8 +33,6 @@
#include drm_crtc_helper.h
#include drm_edid.h
-static int radeon_ddc_dump(struct drm_connector *connector);
-
static void avivo_crtc_load_lut(struct drm_crtc *crtc)
{
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
@@ -669,7 +667,6 @@ static void radeon_print_display_setup(struct drm_device
*dev)
static bool radeon_setup_enc_conn(struct drm_device *dev)
{
struct radeon_device *rdev = dev-dev_private;
- struct drm_connector *drm_connector;
bool ret = false;
if (rdev-bios) {
@@ -689,8 +686,6 @@ static bool radeon_setup_enc_conn(struct drm_device
*dev)
if (ret) {
radeon_setup_encoder_clones(dev);
radeon_print_display_setup(dev);
- list_for_each_entry(drm_connector,
dev-mode_config.connector_list,
head)
- radeon_ddc_dump(drm_connector);
}
return ret;
@@ -743,34 +738,6 @@ int radeon_ddc_get_modes(struct radeon_connector
*radeon_connector)
return 0;
}
-static int radeon_ddc_dump(struct drm_connector *connector)
-{
- struct edid *edid;
- struct radeon_connector *radeon_connector =
to_radeon_connector(connector);
- int ret = 0;
-
- /* on hw with routers, select right port */
- if (radeon_connector-router.ddc_valid)
- radeon_router_select_ddc_port(radeon_connector);
-
- if (!radeon_connector-ddc_bus)
- return -1;
- edid = drm_get_edid(connector, radeon_connector-ddc_bus-adapter);
- /* Log EDID retrieval status here. In particular with regard to
- * connectors with requires_extended_probe flag set, that will prevent
- * function radeon_dvi_detect() to fetch EDID on this connector,
- * as long as there is no valid EDID header found */
- if (edid) {
- DRM_INFO(Radeon display connector %s: Found valid EDID,
- drm_get_connector_name(connector));
- kfree(edid);
- } else {
- DRM_INFO(Radeon display connector %s: No monitor connected or
invalid
EDID,
- drm_get_connector_name(connector));
- }
- return ret;
-}
-
/* avivo */
static void avivo_get_fb_div(struct radeon_pll *pll,
u32 target_clock,
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
Re: [PATCH] drm/radeon/kms: remove useless radeon_ddc_dump()
Dear Alex, but we use DDC probing e. g. to identify connectors with improperly terminated i2c bus. Instead of flooding logs and terminals with EDID dumps, we decided some months ago to use this function during module loading to inform the user once (and only once!), which connector has a monitor connected with valid EDID and which connector has not. See the example dmesg log below: [ 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 [ 15.071566] HDA Intel :00:14.2: PCI INT A - GSI 16 (level, low) - IRQ 16 [ 15.071645] HDA Intel :00:14.2: irq 42 for MSI/MSI-X [ 15.072678] [drm] Radeon display connector VGA-1: No display connected or invalid EDID [ 15.470734] Raw EDID: [ 15.470745] 300 00 00 00 00 00 00 00 00 00 00 07 00 00 00 00 [ 15.470748] 300 00 00 00 00 00 00 00 00 00 07 00 00 00 00 00 [ 15.470751] 300 00 00 00 00 00 00 00 00 00 7f 00 00 00 00 00 [ 15.470754] 300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 15.470757] 300 00 00 00 1f 00 00 00 00 00 00 00 00 00 00 00 [ 15.470760] 300 00 00 00 00 07 00 00 00 00 00 7f 00 00 00 00 [ 15.470762] 300 00 00 00 00 00 07 00 00 00 00 00 00 00 00 00 [ 15.470765] 300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 [ 15.470767] [ 15.779568] Raw EDID: [ 15.779578] 300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 15.779581] 300 00 00 00 00 7f 00 00 00 00 03 00 00 00 00 00 [ 15.779584] 300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 15.779587] 300 00 00 00 ff 00 00 00 00 00 00 00 00 00 00 00 [ 15.779590] 300 00 00 00 00 00 00 00 ff 00 00 00 00 00 00 00 [ 15.779593] 300 00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 [ 15.779596] 300 00 00 7f 00 00 00 00 00 03 07 00 00 00 00 00 [ 15.779598] 300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 15.779600] [ 16.151817] Raw EDID: [ 16.151827] 300 00 00 00 00 00 1f 00 00 00 00 00 00 00 00 00 [ 16.151830] 300 00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 [ 16.151833] 300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 16.151836] 300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 16.151839] 300 00 1f 00 00 00 00 00 00 00 00 00 00 00 0f 00 [ 16.151842] 301 00 00 00 00 00 00 00 01 00 00 00 00 07 00 ff [ 16.151844] 300 00 00 00 00 00 ff 00 00 00 00 00 00 00 7f 00 [ 16.151847] 300 0f 00 00 00 00 00 00 00 3f 00 00 00 00 00 00 .?.. [ 16.151849] [ 16.444209] Raw EDID: [ 16.444220] 300 07 00 00 00 00 00 00 ff 00 00 00 00 ff 00 00 [ 16.444223] 300 00 3f 00 00 00 00 00 00 00 00 00 00 ff 00 00 ..?. [ 16.444226] 300 00 00 00 00 00 00 00 00 07 00 00 00 01 0f 00 [ 16.444229] 300 07 00 00 00 00 00 00 00 00 01 07 00 00 00 00 [ 16.444231] 300 00 00 00 00 00 00 00 7f 00 00 00 00 1f 00 00 [ 16.444234] 300 00 03 00 00 00 00 3f 00 03 00 00 00 00 00 00 ...? [ 16.444237] 37f 00 00 1f 00 00 00 00 00 00 00 00 0f 07 00 00 [ 16.444240] 300 00 00 00 00 00 00 00 00 00 03 00 00 00 00 00 [ 16.444242] [ 16.444248] radeon :01:05.0: HDMI-A-1: EDID block 0 invalid. [ 16.444252] [drm] Radeon display connector HDMI-A-1: No display connected or invalid EDID [ 16.762734] [drm] Radeon display connector DVI-D-1: Found valid EDID Graphic solutions in general have several connectors integrated, and it's sometimes really difficult to identify, which one of the does not work as expected, based on the logs. From above log we see, that a monitor is connected to DVI connector, nothing is connected to the VGA connector, and we have a problem with the HDMI connector. Without this fuinction, nothing helpful from radeon module would be in the logs. Maybe we can keep this function,
