drm/i915: Register the backlight device after the modeset init
authorVille Syrjälä <ville.syrjala@linux.intel.com>
Fri, 7 Nov 2014 13:19:46 +0000 (15:19 +0200)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Fri, 14 Nov 2014 09:29:20 +0000 (10:29 +0100)
Currently we register the backlight device as soon as we register the
connector. That means we can get backlight requests from userspace
already before reading out the current modeset hardware state.

That means we don't yet know the current crtc->encoder->connector mapping,
which causes problems for VLV/CHV which need to know the current pipe in
order to figure out which BLC registers to poke. Currently we just
ignore such requests fairly deep in the backlight code which means the
backlight device brightness property will get out of sync with our
backlight.level and the actual hardware state.

Fix the problem by delaying the backlight device registration until the
entire modeset init has been performed. And we also move the
backlight unregisteration to happen as the first thing during the
modeset cleanup so that we also won't be bothered with userspace
backlight requested during teardown.

This is a real world problem on machines using systemd, because systemd,
for some reason, wants to restore the backlight to the level it used last
time. And that happens as soon as it sees the backlight device appearing
in the system. Sometimes the userspace access makes it through before
the modeset init, sometimes not.

v2: Do not lie to the user in the debug prints (Jani)
    Include connector name in the prints (Jani)
    Fix a typo in the commit message (Jani)

Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drivers/gpu/drm/i915/intel_display.c
drivers/gpu/drm/i915/intel_drv.h
drivers/gpu/drm/i915/intel_panel.c

index 7c2ecbe43ca50585da8c0efb268fcdc0bd609183..983091224f11685f50d3372f848f46e2829794bd 100644 (file)
@@ -13293,6 +13293,8 @@ void intel_modeset_gem_init(struct drm_device *dev)
                }
        }
        mutex_unlock(&dev->struct_mutex);
+
+       intel_backlight_register(dev);
 }
 
 void intel_connector_unregister(struct intel_connector *intel_connector)
@@ -13308,6 +13310,8 @@ void intel_modeset_cleanup(struct drm_device *dev)
        struct drm_i915_private *dev_priv = dev->dev_private;
        struct drm_connector *connector;
 
+       intel_backlight_unregister(dev);
+
        /*
         * Interrupts and polling as the first thing to avoid creating havoc.
         * Too much stuff here (turning of rps, connectors, ...) would
index ef5155739a1ef53e84ef3ca7a5bb314706d92a8a..3c324a806646185b3c82764f4a92715989aebda5 100644 (file)
@@ -1106,6 +1106,9 @@ extern struct drm_display_mode *intel_find_panel_downclock(
                                struct drm_device *dev,
                                struct drm_display_mode *fixed_mode,
                                struct drm_connector *connector);
+void intel_backlight_register(struct drm_device *dev);
+void intel_backlight_unregister(struct drm_device *dev);
+
 
 /* intel_runtime_pm.c */
 int intel_power_domains_init(struct drm_i915_private *);
index 69bbfbaa4680c09e25b48242f997e7bdf164ffcd..708642a9e19baa116f8cad9268eccc52d1741956 100644 (file)
@@ -1041,6 +1041,9 @@ static int intel_backlight_device_register(struct intel_connector *connector)
        if (WARN_ON(panel->backlight.device))
                return -ENODEV;
 
+       if (!panel->backlight.present)
+               return 0;
+
        WARN_ON(panel->backlight.max == 0);
 
        memset(&props, 0, sizeof(props));
@@ -1076,6 +1079,10 @@ static int intel_backlight_device_register(struct intel_connector *connector)
                panel->backlight.device = NULL;
                return -ENODEV;
        }
+
+       DRM_DEBUG_KMS("Connector %s backlight sysfs interface registered\n",
+                     connector->base.name);
+
        return 0;
 }
 
@@ -1302,15 +1309,12 @@ int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe)
                return ret;
        }
 
-       intel_backlight_device_register(intel_connector);
-
        panel->backlight.present = true;
 
-       DRM_DEBUG_KMS("backlight initialized, %s, brightness %u/%u, "
-                     "sysfs interface %sregistered\n",
+       DRM_DEBUG_KMS("Connector %s backlight initialized, %s, brightness %u/%u\n",
+                     connector->name,
                      panel->backlight.enabled ? "enabled" : "disabled",
-                     panel->backlight.level, panel->backlight.max,
-                     panel->backlight.device ? "" : "not ");
+                     panel->backlight.level, panel->backlight.max);
 
        return 0;
 }
@@ -1321,7 +1325,6 @@ void intel_panel_destroy_backlight(struct drm_connector *connector)
        struct intel_panel *panel = &intel_connector->panel;
 
        panel->backlight.present = false;
-       intel_backlight_device_unregister(intel_connector);
 }
 
 /* Set up chip specific backlight functions */
@@ -1384,3 +1387,19 @@ void intel_panel_fini(struct intel_panel *panel)
                drm_mode_destroy(intel_connector->base.dev,
                                panel->downclock_mode);
 }
+
+void intel_backlight_register(struct drm_device *dev)
+{
+       struct intel_connector *connector;
+
+       list_for_each_entry(connector, &dev->mode_config.connector_list, base.head)
+               intel_backlight_device_register(connector);
+}
+
+void intel_backlight_unregister(struct drm_device *dev)
+{
+       struct intel_connector *connector;
+
+       list_for_each_entry(connector, &dev->mode_config.connector_list, base.head)
+               intel_backlight_device_unregister(connector);
+}