On Wed, Jul 6, 2011 at 7:30 PM, <reimth at googlemail.com> wrote:
> From: Thomas Reim <rdratlos at yahoo.co.uk>
>
> ? ?Fixes bug https://bugs.launchpad.net/ubuntu/+source/linux/+bug/722806:
> ? ?Some integrated ATI Radeon chipset implementations with add-on HDMI card
> ? ?(e. g. Asus M2A-VM HDMI) indicate the availability of a DDC even
> ? ?when the add-on card is not plugged in or HDMI is disabled in BIOS setup.
> ? ?In this case, drm_get_edid() and drm_edid_block_valid() periodically
> ? ?dump data and kernel errors into system log files and onto terminals.
> ? ?For these chipsets DDC probing is extended by a check for a correct
> ? ?EDID header. Only in case a valid EDID header is also found, the
> ? ?(HDMI) connector will be used by the Radeon driver. This prevents the
> ? ?kernel driver from useless flooding of logs and terminal sessions with
> ? ?EDID dumps and error messages.
> ? ?This patch adds a flag 'requires_extended_probe' to the radeon_connector
> ? ?structure. In function radeon_connector_needs_extended_probe() this flag
> ? ?can be set on a chipset family/vendor/connector type specific basis.
> ? ?In addition, function radeon_ddc_probe() has been adapted to perform
> ? ?extended DDC probing if required by the connector's flag.
> ? ?Requires function drm_edid_header_is_valid() in DRM module provided by
> ? ?[PATCH] drm: Separate EDID Header Check from EDID Block Check.
>
> ? ?Tested for kernel 2.35, 2.38 and 3.0 on Asus M2A-VM HDMI board
>
> Signed-off-by: Thomas Reim <rdratlos at yahoo.co.uk>
Reviewed-by: Alex Deucher <alexdeucher at gmail.com>
> ---
> ?drivers/gpu/drm/radeon/radeon_connectors.c | ? 45
> ++++++++++++++++++++++++++--
> ?drivers/gpu/drm/radeon/radeon_display.c ? ?| ? ?9 +++++
> ?drivers/gpu/drm/radeon/radeon_i2c.c ? ? ? ?| ? 32 +++++++++++++++-----
> ?drivers/gpu/drm/radeon/radeon_mode.h ? ? ? | ? ?6 +++-
> ?4 files changed, 80 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
> b/drivers/gpu/drm/radeon/radeon_connectors.c
> index cbfca3a..2e70be2 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -424,6 +424,36 @@ int radeon_connector_set_property(struct drm_connector
> *connector, struct drm_pr
> ? ? ? ?return 0;
> ?}
>
> +/*
> + * Some integrated ATI Radeon chipset implementations (e. g.
> + * Asus M2A-VM HDMI) may indicate the availability of a DDC,
> + * even when there's no monitor connected. For these connectors
> + * following DDC probe extension will be applied: check also for the
> + * availability of EDID with at least a correct EDID header. Only then,
> + * DDC is assumed to be available. This prevents drm_get_edid() and
> + * drm_edid_block_valid() from periodically dumping data and kernel
> + * errors into the logs and onto the terminal.
> + */
> +static bool radeon_connector_needs_extended_probe(struct radeon_device *dev,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?uint32_t supported_device,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int connector_type)
> +{
> + ? ? ? /* Asus M2A-VM HDMI board sends data to i2c bus even,
> + ? ? ? ?* if HDMI add-on card is not plugged in or HDMI is disabled in
> + ? ? ? ?* BIOS. Valid DDC can only be assumed, if also a valid EDID header
> + ? ? ? ?* can be retrieved via i2c bus during DDC probe */
> + ? ? ? if ((dev->pdev->device == 0x791e) &&
> + ? ? ? ? ? (dev->pdev->subsystem_vendor == 0x1043) &&
> + ? ? ? ? ? (dev->pdev->subsystem_device == 0x826d)) {
> + ? ? ? ? ? ? ? if ((connector_type == DRM_MODE_CONNECTOR_HDMIA) &&
> + ? ? ? ? ? ? ? ? ? (supported_device == ATOM_DEVICE_DFP2_SUPPORT))
> + ? ? ? ? ? ? ? ? ? ? ? return true;
> + ? ? ? }
> +
> + ? ? ? /* Default: no EDID header probe required for DDC probing */
> + ? ? ? return false;
> +}
> +
> ?static void radeon_fixup_lvds_native_mode(struct drm_encoder *encoder,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct drm_connector *connector)
> ?{
> @@ -655,7 +685,8 @@ radeon_vga_detect(struct drm_connector *connector, bool
> force)
> ? ? ? ? ? ? ? ?ret = connector_status_disconnected;
>
> ? ? ? ?if (radeon_connector->ddc_bus)
> - ? ? ? ? ? ? ? dret = radeon_ddc_probe(radeon_connector);
> + ? ? ? ? ? ? ? dret = radeon_ddc_probe(radeon_connector,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?
> radeon_connector->requires_extended_probe);
> ? ? ? ?if (dret) {
> ? ? ? ? ? ? ? ?if (radeon_connector->edid) {
> ? ? ? ? ? ? ? ? ? ? ? ?kfree(radeon_connector->edid);
> @@ -827,7 +858,8 @@ radeon_dvi_detect(struct drm_connector *connector, bool
> force)
> ? ? ? ?bool dret = false;
>
> ? ? ? ?if (radeon_connector->ddc_bus)
> - ? ? ? ? ? ? ? dret = radeon_ddc_probe(radeon_connector);
> + ? ? ? ? ? ? ? dret = radeon_ddc_probe(radeon_connector,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?
> radeon_connector->requires_extended_probe);
> ? ? ? ?if (dret) {
> ? ? ? ? ? ? ? ?if (radeon_connector->edid) {
> ? ? ? ? ? ? ? ? ? ? ? ?kfree(radeon_connector->edid);
> @@ -1245,7 +1277,8 @@ radeon_dp_detect(struct drm_connector *connector, bool
> force)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?if (radeon_dp_getdpcd(radeon_connector))
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ret = connector_status_connected;
> ? ? ? ? ? ? ? ? ? ? ? ?} else {
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? if (radeon_ddc_probe(radeon_connector))
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? if (radeon_ddc_probe(radeon_connector,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?
> ?radeon_connector->requires_extended_probe))
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ret = connector_status_connected;
> ? ? ? ? ? ? ? ? ? ? ? ?}
> ? ? ? ? ? ? ? ?}
> @@ -1400,6 +1433,9 @@ radeon_add_atom_connector(struct drm_device *dev,
> ? ? ? ?radeon_connector->shared_ddc = shared_ddc;
> ? ? ? ?radeon_connector->connector_object_id = connector_object_id;
> ? ? ? ?radeon_connector->hpd = *hpd;
> + ? ? ? radeon_connector->requires_extended_probe =
> + ? ? ? ? ? ? ? radeon_connector_needs_extended_probe(rdev, supported_device,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? connector_type);
> ? ? ? ?radeon_connector->router = *router;
> ? ? ? ?if (router->ddc_valid || router->cd_valid) {
> ? ? ? ? ? ? ? ?radeon_connector->router_bus = radeon_i2c_lookup(rdev,
> &router->i2c_info);
> @@ -1746,6 +1782,9 @@ radeon_add_legacy_connector(struct drm_device *dev,
> ? ? ? ?radeon_connector->devices = supported_device;
> ? ? ? ?radeon_connector->connector_object_id = connector_object_id;
> ? ? ? ?radeon_connector->hpd = *hpd;
> + ? ? ? radeon_connector->requires_extended_probe =
> + ? ? ? ? ? ? ? radeon_connector_needs_extended_probe(rdev, supported_device,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? connector_type);
> ? ? ? ?switch (connector_type) {
> ? ? ? ?case DRM_MODE_CONNECTOR_VGA:
> ? ? ? ? ? ? ? ?drm_connector_init(dev, &radeon_connector->base,
> &radeon_vga_connector_funcs, connector_type);
> diff --git a/drivers/gpu/drm/radeon/radeon_display.c
> b/drivers/gpu/drm/radeon/radeon_display.c
> index 292f73f..ed085ce 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ b/drivers/gpu/drm/radeon/radeon_display.c
> @@ -777,8 +777,17 @@ static int radeon_ddc_dump(struct drm_connector
> *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;
> ?}
> diff --git a/drivers/gpu/drm/radeon/radeon_i2c.c
> b/drivers/gpu/drm/radeon/radeon_i2c.c
> index 781196d..6c111c1 100644
> --- a/drivers/gpu/drm/radeon/radeon_i2c.c
> +++ b/drivers/gpu/drm/radeon/radeon_i2c.c
> @@ -32,17 +32,17 @@
> ?* radeon_ddc_probe
> ?*
> ?*/
> -bool radeon_ddc_probe(struct radeon_connector *radeon_connector)
> +bool radeon_ddc_probe(struct radeon_connector *radeon_connector, bool
> requires_extended_probe)
> ?{
> - ? ? ? u8 out_buf[] = { 0x0, 0x0};
> - ? ? ? u8 buf[2];
> + ? ? ? u8 out = 0x0;
> + ? ? ? u8 buf[8];
> ? ? ? ?int ret;
> ? ? ? ?struct i2c_msg msgs[] = {
> ? ? ? ? ? ? ? ?{
> ? ? ? ? ? ? ? ? ? ? ? ?.addr = 0x50,
> ? ? ? ? ? ? ? ? ? ? ? ?.flags = 0,
> ? ? ? ? ? ? ? ? ? ? ? ?.len = 1,
> - ? ? ? ? ? ? ? ? ? ? ? .buf = out_buf,
> + ? ? ? ? ? ? ? ? ? ? ? .buf = &out,
> ? ? ? ? ? ? ? ?},
> ? ? ? ? ? ? ? ?{
> ? ? ? ? ? ? ? ? ? ? ? ?.addr = 0x50,
> @@ -52,15 +52,31 @@ bool radeon_ddc_probe(struct radeon_connector
> *radeon_connector)
> ? ? ? ? ? ? ? ?}
> ? ? ? ?};
>
> + ? ? ? /* Read 8 bytes from i2c for extended probe of EDID header */
> + ? ? ? if (requires_extended_probe)
> + ? ? ? ? ? ? ? msgs[1].len = 8;
> +
> ? ? ? ?/* on hw with routers, select right port */
> ? ? ? ?if (radeon_connector->router.ddc_valid)
> ? ? ? ? ? ? ? ?radeon_router_select_ddc_port(radeon_connector);
>
> ? ? ? ?ret = i2c_transfer(&radeon_connector->ddc_bus->adapter, msgs, 2);
> - ? ? ? if (ret == 2)
> - ? ? ? ? ? ? ? return true;
> -
> - ? ? ? return false;
> + ? ? ? if (ret != 2)
> + ? ? ? ? ? ? ? /* Couldn't find an accessible DDC on this connector */
> + ? ? ? ? ? ? ? return false;
> + ? ? ? if (requires_extended_probe) {
> + ? ? ? ? ? ? ? /* Probe also for valid EDID header
> + ? ? ? ? ? ? ? ?* EDID header starts with:
> + ? ? ? ? ? ? ? ?* 0x00,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0x00.
> + ? ? ? ? ? ? ? ?* Only the first 6 bytes must be valid as
> + ? ? ? ? ? ? ? ?* drm_edid_block_valid() can fix the last 2 bytes */
> + ? ? ? ? ? ? ? if (drm_edid_header_is_valid(buf) < 6) {
> + ? ? ? ? ? ? ? ? ? ? ? /* Couldn't find an accessible EDID on this
> + ? ? ? ? ? ? ? ? ? ? ? ?* connector */
> + ? ? ? ? ? ? ? ? ? ? ? return false;
> + ? ? ? ? ? ? ? }
> + ? ? ? }
> + ? ? ? return true;
> ?}
>
> ?/* bit banging i2c */
> diff --git a/drivers/gpu/drm/radeon/radeon_mode.h
> b/drivers/gpu/drm/radeon/radeon_mode.h
> index 6df4e3c..d09031c 100644
> --- a/drivers/gpu/drm/radeon/radeon_mode.h
> +++ b/drivers/gpu/drm/radeon/radeon_mode.h
> @@ -438,6 +438,9 @@ struct radeon_connector {
> ? ? ? ?struct radeon_i2c_chan *ddc_bus;
> ? ? ? ?/* some systems have an hdmi and vga port with a shared ddc line */
> ? ? ? ?bool shared_ddc;
> + ? ? ? /* for some Radeon chip families we apply an additional EDID header
> + ? ? ? ? ?check as part of the DDC probe */
> + ? ? ? bool requires_extended_probe;
> ? ? ? ?bool use_digital;
> ? ? ? ?/* we need to mind the EDID between detect
> ? ? ? ? ? and get modes due to analog/digital/tvencoder */
> @@ -514,7 +517,8 @@ extern void radeon_i2c_put_byte(struct radeon_i2c_chan
> *i2c,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?u8 val);
> ?extern void radeon_router_select_ddc_port(struct radeon_connector
> *radeon_connector);
> ?extern void radeon_router_select_cd_port(struct radeon_connector
> *radeon_connector);
> -extern bool radeon_ddc_probe(struct radeon_connector *radeon_connector);
> +extern bool radeon_ddc_probe(struct radeon_connector *radeon_connector,
> + ? ? ? ? ? ? ? ? ? ? ? bool requires_extended_probe);
> ?extern int radeon_ddc_get_modes(struct radeon_connector *radeon_connector);
>
> ?extern struct drm_encoder *radeon_best_encoder(struct drm_connector
> *connector);
> --
> 1.7.1
>
>
