With the recent addition of locking checks in
commit 62ff94a5492175759546f8bc61383189d6b49122 Author: Daniel Vetter daniel.vetter@ffwll.ch AuthorDate: Thu Jan 23 22:18:47 2014 +0100
drm/crtc-helper: remove LOCKING from kerneldoc
drm_add_edid_modes started to WARN about the mode_config.mutex not being held in the lvds and dp initialization code.
Now since this is init code locking is fairly redudant if it wouldn't be for the drm core registering sysfs files a bit early. And the locking WARNINGs nicely enforce that indeed all access to the mode lists are properly protected. And a full audit shows that only i915 and gma500 touch the modes lists at init time.
Hence I've opted to wrap up this entire mode detection sequence for fixed panels with the mode_config mutex for both lvds and edp outputs.
Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/i915/intel_dp.c | 2 ++ drivers/gpu/drm/i915/intel_lvds.c | 5 +++++ 2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 59ee4dcfebee..6f767e522b3f 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3646,6 +3646,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, /* We now know it's not a ghost, init power sequence regs. */ intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, power_seq);
+ mutex_lock(&dev->mode_config.mutex); edid = drm_get_edid(connector, &intel_dp->aux.ddc); if (edid) { if (drm_add_edid_modes(connector, edid)) { @@ -3676,6 +3677,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, if (fixed_mode) fixed_mode->type |= DRM_MODE_TYPE_PREFERRED; } + mutex_unlock(&dev->mode_config.mutex);
intel_panel_init(&intel_connector->panel, fixed_mode, NULL); intel_panel_setup_backlight(connector); diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index ef5e5661efb2..f1ecf916474a 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -1002,6 +1002,7 @@ void intel_lvds_init(struct drm_device *dev) * Attempt to get the fixed panel mode from DDC. Assume that the * preferred mode is the right one. */ + mutex_lock(&dev->mode_config.mutex); edid = drm_get_edid(connector, intel_gmbus_get_adapter(dev_priv, pin)); if (edid) { if (drm_add_edid_modes(connector, edid)) { @@ -1095,6 +1096,8 @@ void intel_lvds_init(struct drm_device *dev) goto failed;
out: + mutex_unlock(&dev->mode_config.mutex); + lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder); DRM_DEBUG_KMS("detected %s-link lvds configuration\n", lvds_encoder->is_dual_link ? "dual" : "single"); @@ -1123,6 +1126,8 @@ out: return;
failed: + mutex_unlock(&dev->mode_config.mutex); + DRM_DEBUG_KMS("No LVDS modes found, disabling.\n"); drm_connector_cleanup(connector); drm_encoder_cleanup(encoder);
With the recent addition of locking checks in
commit 62ff94a5492175759546f8bc61383189d6b49122 Author: Daniel Vetter daniel.vetter@ffwll.ch AuthorDate: Thu Jan 23 22:18:47 2014 +0100
drm/crtc-helper: remove LOCKING from kerneldoc
drm_add_edid_modes started to WARN about the mode_config.mutex not being held in the lvds and dp initialization code.
Now since this is init code locking is fairly redudant if it wouldn't be for the drm core registering sysfs files a bit early. And the locking WARNINGs nicely enforce that indeed all access to the mode lists are properly protected. And a full audit shows that only i915 and gma500 touch the modes lists at init time.
Hence I've opted to wrap up this entire mode detection sequence for fixed panels with the mode_config mutex for both lvds and edp outputs.
Cc: Patrik Jakobsson patrik.r.jakobsson@gmail.com Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/gma500/cdv_intel_lvds.c | 3 +++ drivers/gpu/drm/gma500/oaktrail_lvds.c | 5 +++++ drivers/gpu/drm/gma500/psb_intel_lvds.c | 3 +++ 3 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c index 20e08e65d46c..66a41c026834 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c +++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c @@ -712,6 +712,7 @@ void cdv_intel_lvds_init(struct drm_device *dev, * Attempt to get the fixed panel mode from DDC. Assume that the * preferred mode is the right one. */ + mutex_lock(&dev->mode_config.mutex); psb_intel_ddc_get_modes(connector, &gma_encoder->ddc_bus->adapter); list_for_each_entry(scan, &connector->probed_modes, head) { @@ -772,10 +773,12 @@ void cdv_intel_lvds_init(struct drm_device *dev, }
out: + mutex_unlock(&dev->mode_config.mutex); drm_sysfs_connector_add(connector); return;
failed_find: + mutex_unlock(&dev->mode_config.mutex); printk(KERN_ERR "Failed find\n"); if (gma_encoder->ddc_bus) psb_intel_i2c_destroy(gma_encoder->ddc_bus); diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c index 5e0697862736..9b099468a5db 100644 --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c @@ -359,6 +359,7 @@ void oaktrail_lvds_init(struct drm_device *dev, * if closed, act like it's not there for now */
+ mutex_lock(&dev->mode_config.mutex); i2c_adap = i2c_get_adapter(dev_priv->ops->i2c_bus); if (i2c_adap == NULL) dev_err(dev->dev, "No ddc adapter available!\n"); @@ -401,10 +402,14 @@ void oaktrail_lvds_init(struct drm_device *dev, }
out: + mutex_unlock(&dev->mode_config.mutex); + drm_sysfs_connector_add(connector); return;
failed_find: + mutex_unlock(&dev->mode_config.mutex); + dev_dbg(dev->dev, "No LVDS modes found, disabling.\n"); if (gma_encoder->ddc_bus) psb_intel_i2c_destroy(gma_encoder->ddc_bus); diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c index 32342f6990d9..891a028a0826 100644 --- a/drivers/gpu/drm/gma500/psb_intel_lvds.c +++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c @@ -777,6 +777,7 @@ void psb_intel_lvds_init(struct drm_device *dev, * Attempt to get the fixed panel mode from DDC. Assume that the * preferred mode is the right one. */ + mutex_lock(&dev->mode_config.mutex); psb_intel_ddc_get_modes(connector, &lvds_priv->ddc_bus->adapter); list_for_each_entry(scan, &connector->probed_modes, head) { if (scan->type & DRM_MODE_TYPE_PREFERRED) { @@ -827,10 +828,12 @@ void psb_intel_lvds_init(struct drm_device *dev, * actually having one. */ out: + mutex_unlock(&dev->mode_config.mutex); drm_sysfs_connector_add(connector); return;
failed_find: + mutex_unlock(&dev->mode_config.mutex); if (lvds_priv->ddc_bus) psb_intel_i2c_destroy(lvds_priv->ddc_bus); failed_ddc:
On Fri, Mar 21, 2014 at 11:22 PM, Daniel Vetter daniel.vetter@ffwll.chwrote:
With the recent addition of locking checks in
commit 62ff94a5492175759546f8bc61383189d6b49122 Author: Daniel Vetter daniel.vetter@ffwll.ch AuthorDate: Thu Jan 23 22:18:47 2014 +0100
drm/crtc-helper: remove LOCKING from kerneldocdrm_add_edid_modes started to WARN about the mode_config.mutex not being held in the lvds and dp initialization code.
Now since this is init code locking is fairly redudant if it wouldn't be for the drm core registering sysfs files a bit early. And the locking WARNINGs nicely enforce that indeed all access to the mode lists are properly protected. And a full audit shows that only i915 and gma500 touch the modes lists at init time.
Hence I've opted to wrap up this entire mode detection sequence for fixed panels with the mode_config mutex for both lvds and edp outputs.
Cc: Patrik Jakobsson patrik.r.jakobsson@gmail.com Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/gma500/cdv_intel_lvds.c | 3 +++ drivers/gpu/drm/gma500/oaktrail_lvds.c | 5 +++++ drivers/gpu/drm/gma500/psb_intel_lvds.c | 3 +++ 3 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c index 20e08e65d46c..66a41c026834 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c +++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c @@ -712,6 +712,7 @@ void cdv_intel_lvds_init(struct drm_device *dev, * Attempt to get the fixed panel mode from DDC. Assume that the * preferred mode is the right one. */
mutex_lock(&dev->mode_config.mutex); psb_intel_ddc_get_modes(connector, &gma_encoder->ddc_bus->adapter); list_for_each_entry(scan, &connector->probed_modes, head) {@@ -772,10 +773,12 @@ void cdv_intel_lvds_init(struct drm_device *dev, }
out:
mutex_unlock(&dev->mode_config.mutex); drm_sysfs_connector_add(connector); return;failed_find:
mutex_unlock(&dev->mode_config.mutex); printk(KERN_ERR "Failed find\n"); if (gma_encoder->ddc_bus) psb_intel_i2c_destroy(gma_encoder->ddc_bus);diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c index 5e0697862736..9b099468a5db 100644 --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c @@ -359,6 +359,7 @@ void oaktrail_lvds_init(struct drm_device *dev, * if closed, act like it's not there for now */
mutex_lock(&dev->mode_config.mutex); i2c_adap = i2c_get_adapter(dev_priv->ops->i2c_bus); if (i2c_adap == NULL) dev_err(dev->dev, "No ddc adapter available!\n");@@ -401,10 +402,14 @@ void oaktrail_lvds_init(struct drm_device *dev, }
out:
mutex_unlock(&dev->mode_config.mutex);drm_sysfs_connector_add(connector); return;failed_find:
mutex_unlock(&dev->mode_config.mutex);dev_dbg(dev->dev, "No LVDS modes found, disabling.\n"); if (gma_encoder->ddc_bus) psb_intel_i2c_destroy(gma_encoder->ddc_bus);diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c index 32342f6990d9..891a028a0826 100644 --- a/drivers/gpu/drm/gma500/psb_intel_lvds.c +++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c @@ -777,6 +777,7 @@ void psb_intel_lvds_init(struct drm_device *dev, * Attempt to get the fixed panel mode from DDC. Assume that the * preferred mode is the right one. */
mutex_lock(&dev->mode_config.mutex); psb_intel_ddc_get_modes(connector, &lvds_priv->ddc_bus->adapter); list_for_each_entry(scan, &connector->probed_modes, head) { if (scan->type & DRM_MODE_TYPE_PREFERRED) {@@ -827,10 +828,12 @@ void psb_intel_lvds_init(struct drm_device *dev, * actually having one. */ out:
mutex_unlock(&dev->mode_config.mutex); drm_sysfs_connector_add(connector); return;failed_find:
mutex_unlock(&dev->mode_config.mutex); if (lvds_priv->ddc_bus) psb_intel_i2c_destroy(lvds_priv->ddc_bus);failed_ddc:
1.8.1.4
Looks good.
Acked-by: Patrik Jakobsson patrik.r.jakobsson@gmail.com
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Patrik Jakobsson