drm/radeon/kms: fix connector edid handling

Based partly on a patch from
Christian Koenig <deathsimple@vodafone.de>

- fix several memory leaks in radeon_connector->edid handling
- store edid in radeon_connector->edid in detect() or get_modes()
- switch hdmi detect code to use radeon_connector->edid
- add support for oem boards multiple connectors that share
a ddc line.
- short circuit lvds_detect() if have a stored edid

Signed-off-by: Alex Deucher <alexdeucher@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>

diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index 729207a..5b6067a 100644 (file)
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -400,7 +400,6 @@ static enum drm_connector_status radeon_lvds_detect(struct drm_connector *connec
        struct radeon_connector *radeon_connector = to_radeon_connector(connector);
        struct drm_encoder *encoder = radeon_best_single_encoder(connector);
        enum drm_connector_status ret = connector_status_disconnected;
-       bool dret;
 
        if (encoder) {
                struct radeon_encoder *radeon_encoder = to_radeon_encoder(encoder);
@@ -413,12 +412,17 @@ static enum drm_connector_status radeon_lvds_detect(struct drm_connector *connec
        }
 
        /* check for edid as well */
-       if (radeon_connector->ddc_bus) {
-               radeon_i2c_do_lock(radeon_connector, 1);
-               dret = radeon_ddc_probe(radeon_connector);
-               radeon_i2c_do_lock(radeon_connector, 0);
-               if (dret)
-                       ret = connector_status_connected;
+       if (radeon_connector->edid)
+               ret = connector_status_connected;
+       else {
+               if (radeon_connector->ddc_bus) {
+                       radeon_i2c_do_lock(radeon_connector, 1);
+                       radeon_connector->edid = drm_get_edid(&radeon_connector->base,
+                                                             &radeon_connector->ddc_bus->adapter);
+                       radeon_i2c_do_lock(radeon_connector, 0);
+                       if (radeon_connector->edid)
+                               ret = connector_status_connected;
+               }
        }
        /* check acpi lid status ??? */
 
@@ -432,6 +436,8 @@ static void radeon_connector_destroy(struct drm_connector *connector)
 
        if (radeon_connector->ddc_bus)
                radeon_i2c_destroy(radeon_connector->ddc_bus);
+       if (radeon_connector->edid)
+               kfree(radeon_connector->edid);
        kfree(radeon_connector->con_priv);
        drm_sysfs_connector_remove(connector);
        drm_connector_cleanup(connector);
@@ -519,9 +525,32 @@ static enum drm_connector_status radeon_vga_detect(struct drm_connector *connect
        radeon_i2c_do_lock(radeon_connector, 1);
        dret = radeon_ddc_probe(radeon_connector);
        radeon_i2c_do_lock(radeon_connector, 0);
-       if (dret)
-               ret = connector_status_connected;
-       else {
+       if (dret) {
+               if (radeon_connector->edid) {
+                       kfree(radeon_connector->edid);
+                       radeon_connector->edid = NULL;
+               }
+               radeon_i2c_do_lock(radeon_connector, 1);
+               radeon_connector->edid = drm_get_edid(&radeon_connector->base, &radeon_connector->ddc_bus->adapter);
+               radeon_i2c_do_lock(radeon_connector, 0);
+
+               if (!radeon_connector->edid) {
+                       DRM_ERROR("DDC responded but not EDID found for %s\n",
+                                 drm_get_connector_name(connector));
+               } else {
+                       radeon_connector->use_digital = !!(radeon_connector->edid->input & DRM_EDID_INPUT_DIGITAL);
+
+                       /* some oems have boards with separate digital and analog connectors
+                        * with a shared ddc line (often vga + hdmi)
+                        */
+                       if (radeon_connector->use_digital && radeon_connector->shared_ddc) {
+                               kfree(radeon_connector->edid);
+                               radeon_connector->edid = NULL;
+                               ret = connector_status_disconnected;
+                       } else
+                               ret = connector_status_connected;
+               }
+       } else {
                if (radeon_connector->dac_load_detect) {
                        encoder_funcs = encoder->helper_private;
                        ret = encoder_funcs->detect(encoder, connector);
@@ -649,6 +678,10 @@ static enum drm_connector_status radeon_dvi_detect(struct drm_connector *connect
        dret = radeon_ddc_probe(radeon_connector);
        radeon_i2c_do_lock(radeon_connector, 0);
        if (dret) {
+               if (radeon_connector->edid) {
+                       kfree(radeon_connector->edid);
+                       radeon_connector->edid = NULL;
+               }
                radeon_i2c_do_lock(radeon_connector, 1);
                radeon_connector->edid = drm_get_edid(&radeon_connector->base, &radeon_connector->ddc_bus->adapter);
                radeon_i2c_do_lock(radeon_connector, 0);
@@ -659,10 +692,15 @@ static enum drm_connector_status radeon_dvi_detect(struct drm_connector *connect
                } else {
                        radeon_connector->use_digital = !!(radeon_connector->edid->input & DRM_EDID_INPUT_DIGITAL);
 
-                       /* if this isn't a digital monitor
-                          then we need to make sure we don't have any
-                          TV conflicts */
-                       ret = connector_status_connected;
+                       /* some oems have boards with separate digital and analog connectors
+                        * with a shared ddc line (often vga + hdmi)
+                        */
+                       if ((!radeon_connector->use_digital) && radeon_connector->shared_ddc) {
+                               kfree(radeon_connector->edid);
+                               radeon_connector->edid = NULL;
+                               ret = connector_status_disconnected;
+                       } else
+                               ret = connector_status_connected;
                }
        }
 
@@ -787,6 +825,7 @@ radeon_add_atom_connector(struct drm_device *dev,
        struct radeon_connector *radeon_connector;
        struct radeon_connector_atom_dig *radeon_dig_connector;
        uint32_t subpixel_order = SubPixelNone;
+       bool shared_ddc = false;
        int ret;
 
        /* fixme - tv/cv/din */
@@ -800,6 +839,13 @@ radeon_add_atom_connector(struct drm_device *dev,
                        radeon_connector->devices |= supported_device;
                        return;
                }
+               if (radeon_connector->ddc_bus && i2c_bus->valid) {
+                       if (memcmp(&radeon_connector->ddc_bus->rec, i2c_bus,
+                                   sizeof(struct radeon_i2c_bus_rec)) == 0) {
+                               radeon_connector->shared_ddc = true;
+                               shared_ddc = true;
+                       }
+               }
        }
 
        radeon_connector = kzalloc(sizeof(struct radeon_connector), GFP_KERNEL);
@@ -810,6 +856,7 @@ radeon_add_atom_connector(struct drm_device *dev,
 
        radeon_connector->connector_id = connector_id;
        radeon_connector->devices = supported_device;
+       radeon_connector->shared_ddc = shared_ddc;
        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 a20bf43..c85df4a 100644 (file)
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -334,27 +334,19 @@ static bool radeon_setup_enc_conn(struct drm_device *dev)
 
 int radeon_ddc_get_modes(struct radeon_connector *radeon_connector)
 {
-       struct edid *edid;
        int ret = 0;
 
        if (!radeon_connector->ddc_bus)
                return -1;
        if (!radeon_connector->edid) {
                radeon_i2c_do_lock(radeon_connector, 1);
-               edid = drm_get_edid(&radeon_connector->base, &radeon_connector->ddc_bus->adapter);
+               radeon_connector->edid = drm_get_edid(&radeon_connector->base, &radeon_connector->ddc_bus->adapter);
                radeon_i2c_do_lock(radeon_connector, 0);
-       } else
-               edid = radeon_connector->edid;
+       }
 
-       if (edid) {
-               /* update digital bits here */
-               if (edid->input & DRM_EDID_INPUT_DIGITAL)
-                       radeon_connector->use_digital = 1;
-               else
-                       radeon_connector->use_digital = 0;
-               drm_mode_connector_update_edid_property(&radeon_connector->base, edid);
-               ret = drm_add_edid_modes(&radeon_connector->base, edid);
-               kfree(edid);
+       if (radeon_connector->edid) {
+               drm_mode_connector_update_edid_property(&radeon_connector->base, radeon_connector->edid);
+               ret = drm_add_edid_modes(&radeon_connector->base, radeon_connector->edid);
                return ret;
        }
        drm_mode_connector_update_edid_property(&radeon_connector->base, NULL);

diff --git a/drivers/gpu/drm/radeon/radeon_encoders.c b/drivers/gpu/drm/radeon/radeon_encoders.c
index 6cdef27..b70440e 100644 (file)
--- a/drivers/gpu/drm/radeon/radeon_encoders.c
+++ b/drivers/gpu/drm/radeon/radeon_encoders.c
@@ -449,7 +449,7 @@ atombios_digital_setup(struct drm_encoder *encoder, int action)
                case 1:
                        args.v1.ucMisc = 0;
                        args.v1.ucAction = action;
-                       if (drm_detect_hdmi_monitor((struct edid *)connector->edid_blob_ptr))
+                       if (drm_detect_hdmi_monitor(radeon_connector->edid))
                                args.v1.ucMisc |= PANEL_ENCODER_MISC_HDMI_TYPE;
                        args.v1.usPixelClock = cpu_to_le16(radeon_encoder->pixel_clock / 10);
                        if (radeon_encoder->devices & (ATOM_DEVICE_LCD_SUPPORT)) {
@@ -474,7 +474,7 @@ atombios_digital_setup(struct drm_encoder *encoder, int action)
                                if (dig->coherent_mode)
                                        args.v2.ucMisc |= PANEL_ENCODER_MISC_COHERENT;
                        }
-                       if (drm_detect_hdmi_monitor((struct edid *)connector->edid_blob_ptr))
+                       if (drm_detect_hdmi_monitor(radeon_connector->edid))
                                args.v2.ucMisc |= PANEL_ENCODER_MISC_HDMI_TYPE;
                        args.v2.usPixelClock = cpu_to_le16(radeon_encoder->pixel_clock / 10);
                        args.v2.ucTruncate = 0;
@@ -532,7 +532,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
        switch (connector->connector_type) {
        case DRM_MODE_CONNECTOR_DVII:
        case DRM_MODE_CONNECTOR_HDMIB: /* HDMI-B is basically DL-DVI; analog works fine */
-               if (drm_detect_hdmi_monitor((struct edid *)connector->edid_blob_ptr))
+               if (drm_detect_hdmi_monitor(radeon_connector->edid))
                        return ATOM_ENCODER_MODE_HDMI;
                else if (radeon_connector->use_digital)
                        return ATOM_ENCODER_MODE_DVI;
@@ -542,7 +542,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
        case DRM_MODE_CONNECTOR_DVID:
        case DRM_MODE_CONNECTOR_HDMIA:
        default:
-               if (drm_detect_hdmi_monitor((struct edid *)connector->edid_blob_ptr))
+               if (drm_detect_hdmi_monitor(radeon_connector->edid))
                        return ATOM_ENCODER_MODE_HDMI;
                else
                        return ATOM_ENCODER_MODE_DVI;
@@ -554,7 +554,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
                /*if (radeon_output->MonType == MT_DP)
                  return ATOM_ENCODER_MODE_DP;
                  else*/
-               if (drm_detect_hdmi_monitor((struct edid *)connector->edid_blob_ptr))
+               if (drm_detect_hdmi_monitor(radeon_connector->edid))
                        return ATOM_ENCODER_MODE_HDMI;
                else
                        return ATOM_ENCODER_MODE_DVI;

diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index 3d2631b..b6868fb 100644 (file)
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -297,6 +297,8 @@ struct radeon_connector {
        uint32_t connector_id;
        uint32_t devices;
        struct radeon_i2c_chan *ddc_bus;
+       /* some systems have a an hdmi and vga port with a shared ddc line */
+       bool shared_ddc;
        bool use_digital;
        /* we need to mind the EDID between detect
           and get modes due to analog/digital/tvencoder */