| Message ID | 08480c9d2ea6be1671339dc7f74d7c855fff3379.1365768957.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 12 Apr 2013, Jani Nikula <jani.nikula@intel.com> wrote: > Backlight cleanup in the eDP connector destroy callback caused the > backlight device to be removed on some systems that first initialized LVDS > and then attempted to initialize eDP. Prevent multiple backlight > initializations, and ensure backlight cleanup is only done once by moving > it to modeset cleanup. > > A small wrinkle is the introduced asymmetry in backlight > setup/cleanup. This could be solved by adding refcounting, but it seems > overkill considering that there should only ever be one backlight device. > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=55701 Via bugzilla, Tested-by: Peter Verthez <peter.verthez@skynet.be> CC: Stable? > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 3 +++ > drivers/gpu/drm/i915/intel_dp.c | 5 +---- > drivers/gpu/drm/i915/intel_lvds.c | 1 - > drivers/gpu/drm/i915/intel_panel.c | 7 ++++++- > 4 files changed, 10 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 457a0a0..712b0af 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9419,6 +9419,9 @@ void intel_modeset_cleanup(struct drm_device *dev) > /* flush any delayed tasks or pending work */ > flush_scheduled_work(); > > + /* destroy backlight, if any, before the connectors */ > + intel_panel_destroy_backlight(dev); > + > drm_mode_config_cleanup(dev); > > intel_cleanup_overlay(dev); > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 173add1..8845e82 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -2474,17 +2474,14 @@ done: > static void > intel_dp_destroy(struct drm_connector *connector) > { > - struct drm_device *dev = connector->dev; > struct intel_dp *intel_dp = intel_attached_dp(connector); > struct intel_connector *intel_connector = to_intel_connector(connector); > > if (!IS_ERR_OR_NULL(intel_connector->edid)) > kfree(intel_connector->edid); > > - if (is_edp(intel_dp)) { > - intel_panel_destroy_backlight(dev); > + if (is_edp(intel_dp)) > intel_panel_fini(&intel_connector->panel); > - } > > drm_sysfs_connector_remove(connector); > drm_connector_cleanup(connector); > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > index ca2d903..f36f1ba 100644 > --- a/drivers/gpu/drm/i915/intel_lvds.c > +++ b/drivers/gpu/drm/i915/intel_lvds.c > @@ -631,7 +631,6 @@ static void intel_lvds_destroy(struct drm_connector *connector) > if (!IS_ERR_OR_NULL(lvds_connector->base.edid)) > kfree(lvds_connector->base.edid); > > - intel_panel_destroy_backlight(connector->dev); > intel_panel_fini(&lvds_connector->base.panel); > > drm_sysfs_connector_remove(connector); > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > index 464c3d7..5d3e9d7 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -459,6 +459,9 @@ int intel_panel_setup_backlight(struct drm_connector *connector) > > intel_panel_init_backlight(dev); > > + if (WARN_ON(dev_priv->backlight.device)) > + return -ENODEV; > + > memset(&props, 0, sizeof(props)); > props.type = BACKLIGHT_RAW; > props.brightness = dev_priv->backlight.level; > @@ -488,8 +491,10 @@ int intel_panel_setup_backlight(struct drm_connector *connector) > void intel_panel_destroy_backlight(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > - if (dev_priv->backlight.device) > + if (dev_priv->backlight.device) { > backlight_device_unregister(dev_priv->backlight.device); > + dev_priv->backlight.device = NULL; > + } > } > #else > int intel_panel_setup_backlight(struct drm_connector *connector) > -- > 1.7.9.5
On Mon, Apr 15, 2013 at 09:33:13AM +0300, Jani Nikula wrote: > On Fri, 12 Apr 2013, Jani Nikula <jani.nikula@intel.com> wrote: > > Backlight cleanup in the eDP connector destroy callback caused the > > backlight device to be removed on some systems that first initialized LVDS > > and then attempted to initialize eDP. Prevent multiple backlight > > initializations, and ensure backlight cleanup is only done once by moving > > it to modeset cleanup. > > > > A small wrinkle is the introduced asymmetry in backlight > > setup/cleanup. This could be solved by adding refcounting, but it seems > > overkill considering that there should only ever be one backlight device. > > > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=55701 > > Via bugzilla, > > Tested-by: Peter Verthez <peter.verthez@skynet.be> > > CC: Stable? Yeah, agreed. Queued for -next, thanks for the patch. -Daniel > > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 3 +++ > > drivers/gpu/drm/i915/intel_dp.c | 5 +---- > > drivers/gpu/drm/i915/intel_lvds.c | 1 - > > drivers/gpu/drm/i915/intel_panel.c | 7 ++++++- > > 4 files changed, 10 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 457a0a0..712b0af 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -9419,6 +9419,9 @@ void intel_modeset_cleanup(struct drm_device *dev) > > /* flush any delayed tasks or pending work */ > > flush_scheduled_work(); > > > > + /* destroy backlight, if any, before the connectors */ > > + intel_panel_destroy_backlight(dev); > > + > > drm_mode_config_cleanup(dev); > > > > intel_cleanup_overlay(dev); > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 173add1..8845e82 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -2474,17 +2474,14 @@ done: > > static void > > intel_dp_destroy(struct drm_connector *connector) > > { > > - struct drm_device *dev = connector->dev; > > struct intel_dp *intel_dp = intel_attached_dp(connector); > > struct intel_connector *intel_connector = to_intel_connector(connector); > > > > if (!IS_ERR_OR_NULL(intel_connector->edid)) > > kfree(intel_connector->edid); > > > > - if (is_edp(intel_dp)) { > > - intel_panel_destroy_backlight(dev); > > + if (is_edp(intel_dp)) > > intel_panel_fini(&intel_connector->panel); > > - } > > > > drm_sysfs_connector_remove(connector); > > drm_connector_cleanup(connector); > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > > index ca2d903..f36f1ba 100644 > > --- a/drivers/gpu/drm/i915/intel_lvds.c > > +++ b/drivers/gpu/drm/i915/intel_lvds.c > > @@ -631,7 +631,6 @@ static void intel_lvds_destroy(struct drm_connector *connector) > > if (!IS_ERR_OR_NULL(lvds_connector->base.edid)) > > kfree(lvds_connector->base.edid); > > > > - intel_panel_destroy_backlight(connector->dev); > > intel_panel_fini(&lvds_connector->base.panel); > > > > drm_sysfs_connector_remove(connector); > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > > index 464c3d7..5d3e9d7 100644 > > --- a/drivers/gpu/drm/i915/intel_panel.c > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > @@ -459,6 +459,9 @@ int intel_panel_setup_backlight(struct drm_connector *connector) > > > > intel_panel_init_backlight(dev); > > > > + if (WARN_ON(dev_priv->backlight.device)) > > + return -ENODEV; > > + > > memset(&props, 0, sizeof(props)); > > props.type = BACKLIGHT_RAW; > > props.brightness = dev_priv->backlight.level; > > @@ -488,8 +491,10 @@ int intel_panel_setup_backlight(struct drm_connector *connector) > > void intel_panel_destroy_backlight(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > - if (dev_priv->backlight.device) > > + if (dev_priv->backlight.device) { > > backlight_device_unregister(dev_priv->backlight.device); > > + dev_priv->backlight.device = NULL; > > + } > > } > > #else > > int intel_panel_setup_backlight(struct drm_connector *connector) > > -- > > 1.7.9.5 > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 457a0a0..712b0af 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9419,6 +9419,9 @@ void intel_modeset_cleanup(struct drm_device *dev) /* flush any delayed tasks or pending work */ flush_scheduled_work(); + /* destroy backlight, if any, before the connectors */ + intel_panel_destroy_backlight(dev); + drm_mode_config_cleanup(dev); intel_cleanup_overlay(dev); diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 173add1..8845e82 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2474,17 +2474,14 @@ done: static void intel_dp_destroy(struct drm_connector *connector) { - struct drm_device *dev = connector->dev; struct intel_dp *intel_dp = intel_attached_dp(connector); struct intel_connector *intel_connector = to_intel_connector(connector); if (!IS_ERR_OR_NULL(intel_connector->edid)) kfree(intel_connector->edid); - if (is_edp(intel_dp)) { - intel_panel_destroy_backlight(dev); + if (is_edp(intel_dp)) intel_panel_fini(&intel_connector->panel); - } drm_sysfs_connector_remove(connector); drm_connector_cleanup(connector); diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index ca2d903..f36f1ba 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -631,7 +631,6 @@ static void intel_lvds_destroy(struct drm_connector *connector) if (!IS_ERR_OR_NULL(lvds_connector->base.edid)) kfree(lvds_connector->base.edid); - intel_panel_destroy_backlight(connector->dev); intel_panel_fini(&lvds_connector->base.panel); drm_sysfs_connector_remove(connector); diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 464c3d7..5d3e9d7 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -459,6 +459,9 @@ int intel_panel_setup_backlight(struct drm_connector *connector) intel_panel_init_backlight(dev); + if (WARN_ON(dev_priv->backlight.device)) + return -ENODEV; + memset(&props, 0, sizeof(props)); props.type = BACKLIGHT_RAW; props.brightness = dev_priv->backlight.level; @@ -488,8 +491,10 @@ int intel_panel_setup_backlight(struct drm_connector *connector) void intel_panel_destroy_backlight(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - if (dev_priv->backlight.device) + if (dev_priv->backlight.device) { backlight_device_unregister(dev_priv->backlight.device); + dev_priv->backlight.device = NULL; + } } #else int intel_panel_setup_backlight(struct drm_connector *connector)
Backlight cleanup in the eDP connector destroy callback caused the backlight device to be removed on some systems that first initialized LVDS and then attempted to initialize eDP. Prevent multiple backlight initializations, and ensure backlight cleanup is only done once by moving it to modeset cleanup. A small wrinkle is the introduced asymmetry in backlight setup/cleanup. This could be solved by adding refcounting, but it seems overkill considering that there should only ever be one backlight device. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=55701 Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 3 +++ drivers/gpu/drm/i915/intel_dp.c | 5 +---- drivers/gpu/drm/i915/intel_lvds.c | 1 - drivers/gpu/drm/i915/intel_panel.c | 7 ++++++- 4 files changed, 10 insertions(+), 6 deletions(-)