Re: [PATCH] drm/radeon/kms: remove useless radeon_ddc_dump()

[Thomas Reim <reimth@googlemail.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 8413 bytes --]

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 0000:00:14.2: PCI INT A -> GSI 16 (level, low)
-> IRQ 16
[   15.071645] HDA Intel 0000: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] <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.444242] 
[   16.444248] radeon 0000: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, but call it only for connectors, for
which it works, i. e. not for DP, eDP and DP bridge connectors.

Best regards

Thomas Reim

Am Dienstag, den 25.10.2011, 19:24 -0400 schrieb alexdeucher@gmail.com:

> From: Alex Deucher <alexander.deucher@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.deucher@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,



[-- Attachment #1.1.2: Type: text/html, Size: 10219 bytes --]

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel