From 52183fb05b8f8db0705e230fc0b8a436512759a1 Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Wed, 4 Jun 2008 09:38:44 -0700 Subject: Fix crash in drm_mode_connector_update_edid_property We need to initialize the edid_blob_ptr to NULL when we init a connector, otherwise drm_mode_connector_update_edid_property may think there's a valid EDID lying around and try to destroy it, causing a crash. --- linux-core/drm_crtc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/linux-core/drm_crtc.c b/linux-core/drm_crtc.c index 4c6afd16..7a049da8 100644 --- a/linux-core/drm_crtc.c +++ b/linux-core/drm_crtc.c @@ -357,6 +357,7 @@ void drm_connector_init(struct drm_device *dev, INIT_LIST_HEAD(&connector->user_modes); INIT_LIST_HEAD(&connector->probed_modes); INIT_LIST_HEAD(&connector->modes); + connector->edid_blob_ptr = NULL; /* randr_connector? */ /* connector_set_monitor(connector)? */ /* check for connector_ignored(connector)? */ -- cgit v1.2.3 From e90716671d7a5dabf13c22a339f750dba77f438a Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Wed, 4 Jun 2008 12:50:03 -0700 Subject: i915: use kzalloc to allocate intel_output for lvds Better to initialize all the struct fields to 0. Also more consistent with other output init routines. --- linux-core/intel_lvds.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux-core/intel_lvds.c b/linux-core/intel_lvds.c index cf9294ee..8d65c161 100644 --- a/linux-core/intel_lvds.c +++ b/linux-core/intel_lvds.c @@ -387,7 +387,7 @@ void intel_lvds_init(struct drm_device *dev) u32 lvds; int pipe; - intel_output = kmalloc(sizeof(struct intel_output), GFP_KERNEL); + intel_output = kzalloc(sizeof(struct intel_output), GFP_KERNEL); if (!intel_output) { return; } -- cgit v1.2.3 From 03bf1fba67413f381d2a548fe08bd634a48fcc48 Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Thu, 5 Jun 2008 15:58:43 -0700 Subject: sysfs registration/teardown fixups A check in drm_sysfs_connector_remove was supposed to allow it to be called even with unregistered objects, to make cleanup paths a little simpler. However, device_is_regsitered didn't always seem to return what we thought it would, so we'd sometimes end up leaving objects lying around rather than unregistering them. Fix this situation up by requiring devices to be registered before being removed. Any problems resulting from this change should be easier to track down than the alternative (which is leaving kobjects registered after unload). --- linux-core/drm_sysfs.c | 32 ++++++++++++++++++++++++++------ linux-core/intel_crt.c | 12 ++++++------ linux-core/intel_lvds.c | 35 +++++++++++++++++------------------ linux-core/intel_sdvo.c | 23 ++++++++++++++--------- 4 files changed, 63 insertions(+), 39 deletions(-) diff --git a/linux-core/drm_sysfs.c b/linux-core/drm_sysfs.c index c148256a..92371c22 100644 --- a/linux-core/drm_sysfs.c +++ b/linux-core/drm_sysfs.c @@ -239,20 +239,26 @@ static struct bin_attribute edid_attr = { * properties (so far, connection status, dpms, mode list & edid) and * generate a hotplug event so userspace knows there's a new connector * available. + * + * Note: + * This routine should only be called *once* for each DRM minor registered. + * A second call for an already registered device will trigger the BUG_ON + * below. */ int drm_sysfs_connector_add(struct drm_connector *connector) { struct drm_device *dev = connector->dev; int ret = 0, i, j; - if (device_is_registered(&connector->kdev)) - return 0; + /* We shouldn't get called more than once for the same connector */ + BUG_ON(device_is_registered(&connector->kdev)); connector->kdev.parent = &dev->primary->kdev; connector->kdev.class = drm_class; connector->kdev.release = drm_sysfs_device_release; - DRM_DEBUG("adding \"%s\" to sysfs\n", drm_get_connector_name(connector)); + DRM_DEBUG("adding \"%s\" to sysfs\n", + drm_get_connector_name(connector)); snprintf(connector->kdev.bus_id, BUS_ID_SIZE, "card%d-%s", dev->primary->index, drm_get_connector_name(connector)); @@ -296,15 +302,19 @@ EXPORT_SYMBOL(drm_sysfs_connector_add); * Remove @connector and its associated attributes from sysfs. Note that * the device model core will take care of sending the "remove" uevent * at this time, so we don't need to do it. + * + * Note: + * This routine should only be called if the connector was previously + * successfully registered. If @connector hasn't been registered yet, + * you'll likely see a panic somewhere deep in sysfs code when called. */ void drm_sysfs_connector_remove(struct drm_connector *connector) { int i; - if (!device_is_registered(&connector->kdev)) - return; + DRM_DEBUG("removing \"%s\" from sysfs\n", + drm_get_connector_name(connector)); - DRM_DEBUG("removing \"%s\" from sysfs\n", drm_get_connector_name(connector)); for (i = 0; i < ARRAY_SIZE(connector_attrs); i++) device_remove_file(&connector->kdev, &connector_attrs[i]); sysfs_remove_bin_file(&connector->kdev.kobj, &edid_attr); @@ -342,6 +352,11 @@ static struct device_attribute dri_attrs[] = { * Add a DRM device to the DRM's device model class. We use @dev's PCI device * as the parent for the Linux device, and make sure it has a file containing * the driver we're using (for userspace compatibility). + * + * Note: + * This routine should only be called *once* for each DRM minor registered. + * A second call for an already registered device will trigger the BUG_ON + * below. */ int drm_sysfs_device_add(struct drm_minor *minor) { @@ -362,6 +377,11 @@ int drm_sysfs_device_add(struct drm_minor *minor) snprintf(minor->kdev.bus_id, BUS_ID_SIZE, minor_str, minor->index); + /* Shouldn't register more than once */ + BUG_ON(device_is_registered(&minor->kdev)); + + DRM_DEBUG("registering DRM device \"%s\"\n", minor->kdev.bus_id); + err = device_register(&minor->kdev); if (err) { DRM_ERROR("device add failed: %d\n", err); diff --git a/linux-core/intel_crt.c b/linux-core/intel_crt.c index e32a9551..b9e8ee63 100644 --- a/linux-core/intel_crt.c +++ b/linux-core/intel_crt.c @@ -268,18 +268,20 @@ void intel_crt_init(struct drm_device *dev) return; connector = &intel_output->base; - drm_connector_init(dev, &intel_output->base, &intel_crt_connector_funcs, DRM_MODE_CONNECTOR_VGA); + drm_connector_init(dev, &intel_output->base, + &intel_crt_connector_funcs, DRM_MODE_CONNECTOR_VGA); - drm_encoder_init(dev, &intel_output->enc, &intel_crt_enc_funcs, DRM_MODE_ENCODER_DAC); + drm_encoder_init(dev, &intel_output->enc, &intel_crt_enc_funcs, + DRM_MODE_ENCODER_DAC); - drm_mode_connector_attach_encoder(&intel_output->base, &intel_output->enc); + drm_mode_connector_attach_encoder(&intel_output->base, + &intel_output->enc); /* Set up the DDC bus. */ intel_output->ddc_bus = intel_i2c_create(dev, GPIOA, "CRTDDC_A"); if (!intel_output->ddc_bus) { dev_printk(KERN_ERR, &dev->pdev->dev, "DDC bus registration " "failed.\n"); - intel_crt_destroy(connector); return; } @@ -291,6 +293,4 @@ void intel_crt_init(struct drm_device *dev) drm_connector_helper_add(connector, &intel_crt_connector_helper_funcs); drm_sysfs_connector_add(connector); - - } diff --git a/linux-core/intel_lvds.c b/linux-core/intel_lvds.c index 8d65c161..04e110e4 100644 --- a/linux-core/intel_lvds.c +++ b/linux-core/intel_lvds.c @@ -329,7 +329,8 @@ static void intel_lvds_destroy(struct drm_connector *connector) { struct intel_output *intel_output = to_intel_output(connector); - intel_i2c_destroy(intel_output->ddc_bus); + if (intel_output->ddc_bus) + intel_i2c_destroy(intel_output->ddc_bus); drm_sysfs_connector_remove(connector); drm_connector_cleanup(connector); kfree(connector); @@ -425,8 +426,7 @@ void intel_lvds_init(struct drm_device *dev) if (!intel_output->ddc_bus) { dev_printk(KERN_ERR, &dev->pdev->dev, "DDC bus registration " "failed.\n"); - intel_lvds_destroy(connector); - return; + goto failed; } /* @@ -470,19 +470,18 @@ void intel_lvds_init(struct drm_device *dev) if (!dev_priv->panel_fixed_mode) goto failed; -#if 0 /* FIXME: detect aopen & mac mini type stuff automatically? */ /* * Blacklist machines with BIOSes that list an LVDS panel without * actually having one. */ - if (dev_priv->PciInfo->chipType == PCI_CHIP_I945_GM) { + if (IS_I945GM(dev)) { /* aopen mini pc */ - if (dev_priv->PciInfo->subsysVendor == 0xa0a0) - goto disable_exit; + if (dev->pdev->subsystem_vendor == 0xa0a0) + goto failed; - if ((dev_priv->PciInfo->subsysVendor == 0x8086) && - (dev_priv->PciInfo->subsysCard == 0x7270)) { + if ((dev->pdev->subsystem_vendor == 0x8086) && + (dev->pdev->subsystem_device == 0x7270)) { /* It's a Mac Mini or Macbook Pro. * * Apple hardware is out to get us. The macbook pro @@ -494,23 +493,23 @@ void intel_lvds_init(struct drm_device *dev) */ if (dev_priv->panel_fixed_mode != NULL && - dev_priv->panel_fixed_mode->HDisplay == 800 && - dev_priv->panel_fixed_mode->VDisplay == 600) - { - xf86DrvMsg(pScrn->scrnIndex, X_INFO, - "Suspected Mac Mini, ignoring the LVDS\n"); - goto disable_exit; + dev_priv->panel_fixed_mode->hdisplay == 800 && + dev_priv->panel_fixed_mode->vdisplay == 600) { + DRM_DEBUG("Suspected Mac Mini, ignoring the LVDS\n"); + goto failed; } } } -#endif out: drm_sysfs_connector_add(connector); return; failed: - DRM_DEBUG("No LVDS modes found, disabling.\n"); - intel_lvds_destroy(connector); + DRM_DEBUG("No LVDS modes found, disabling.\n"); + if (intel_output->ddc_bus) + intel_i2c_destroy(intel_output->ddc_bus); + drm_connector_cleanup(connector); + kfree(connector); } diff --git a/linux-core/intel_sdvo.c b/linux-core/intel_sdvo.c index ef67ef9b..f0a47e2e 100644 --- a/linux-core/intel_sdvo.c +++ b/linux-core/intel_sdvo.c @@ -1036,10 +1036,8 @@ void intel_sdvo_init(struct drm_device *dev, int output_device) else i2cbus = intel_i2c_create(dev, GPIOE, "SDVOCTRL_E for SDVOC"); - if (i2cbus == NULL) { - intel_sdvo_destroy(connector); - return; - } + if (!i2cbus) + goto err_connector; sdvo_priv->i2c_bus = i2cbus; @@ -1061,8 +1059,7 @@ void intel_sdvo_init(struct drm_device *dev, int output_device) if (!intel_sdvo_read_byte(intel_output, i, &ch[i])) { DRM_DEBUG("No SDVO device found on SDVO%c\n", output_device == SDVOB ? 'B' : 'C'); - intel_sdvo_destroy(connector); - return; + goto err_i2c; } } @@ -1107,8 +1104,7 @@ void intel_sdvo_init(struct drm_device *dev, int output_device) DRM_DEBUG("%s: No active RGB or TMDS outputs (0x%02x%02x)\n", SDVO_NAME(sdvo_priv), bytes[0], bytes[1]); - intel_sdvo_destroy(connector); - return; + goto err_i2c; } drm_encoder_init(dev, &intel_output->enc, &intel_sdvo_enc_funcs, encoder_type); @@ -1143,6 +1139,15 @@ void intel_sdvo_init(struct drm_device *dev, int output_device) sdvo_priv->caps.output_flags & (SDVO_OUTPUT_TMDS1 | SDVO_OUTPUT_RGB1) ? 'Y' : 'N'); - intel_output->ddc_bus = i2cbus; + intel_output->ddc_bus = i2cbus; + + return; +err_i2c: + intel_i2c_destroy(intel_output->i2c_bus); +err_connector: + drm_connector_cleanup(connector); + kfree(intel_output); + + return; } -- cgit v1.2.3