Demidlayer i915 loading

Demidlayer i915 loading

[Chris Wilson]

Now that the changes to allow reordering of connector registration have
landed in drm-misc, we can look at proceeding with removing the
dev->driver->load() callback and take control of our initialisation
sequence, with the goal of not exposing driver internals to userspace
before we are ready.

(As with previous series, my ulterior motive is to close a hole in GEM
context vs RPS initialisation which depends upon closing the userspace
hole, and so fix a regression whereby we try to initialise a context
during suspend.)
-Chris

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/8] drm/i915: Move panel's backlight setup next to panel init

[Chris Wilson]

Currently setting up the backlight for a panel is sometimes done
together with initialising the panel, and sometimes after the connector
is registered. The backlight setup does not depend upon connector
registration (i.e. access to sysfs/debugfs and the kobject hierachy) so
perform it consistently just after panel initialisation.

Note the discrepancy here as destroying the panel is done during
connector unregistration...

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h  | 3 ++-
 drivers/gpu/drm/i915/intel_dsi.c  | 3 +--
 drivers/gpu/drm/i915/intel_lvds.c | 3 +--
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 496c962426ac..1a001f9e5cac 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1490,7 +1490,8 @@ void intel_gmch_panel_fitting(struct intel_crtc *crtc,
 			      int fitting_mode);
 void intel_panel_set_backlight_acpi(struct intel_connector *connector,
 				    u32 level, u32 max);
-int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe);
+int intel_panel_setup_backlight(struct drm_connector *connector,
+				enum pipe pipe);
 void intel_panel_enable_backlight(struct intel_connector *connector);
 void intel_panel_disable_backlight(struct intel_connector *connector);
 void intel_panel_destroy_backlight(struct drm_connector *connector);
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index b444d0e35a98..ae2dcaf1b52e 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1587,13 +1587,12 @@ void intel_dsi_init(struct drm_device *dev)
 	connector->display_info.height_mm = fixed_mode->height_mm;
 
 	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
+	intel_panel_setup_backlight(connector, INVALID_PIPE);
 
 	intel_dsi_add_properties(intel_connector);
 
 	drm_connector_register(connector);
 
-	intel_panel_setup_backlight(connector, INVALID_PIPE);
-
 	return;
 
 err:
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index e9082185a375..5dcb1a12458d 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1118,6 +1118,7 @@ out:
 	mutex_unlock(&dev->mode_config.mutex);
 
 	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
+	intel_panel_setup_backlight(connector, INVALID_PIPE);
 
 	lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder);
 	DRM_DEBUG_KMS("detected %s-link lvds configuration\n",
@@ -1132,8 +1133,6 @@ out:
 	}
 	drm_connector_register(connector);
 
-	intel_panel_setup_backlight(connector, INVALID_PIPE);
-
 	return;
 
 failed:
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/8] drm/i915: Move registration actions to connector->late_register

[Chris Wilson]

With the introduction of a connector->func for callback from
drm_connector_register() we can move all the tasks that we want to do
upon registration into that callback. Later, this will allow us to
reorder the registration and defer it until after the device is setup
and ready for userspace.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_dp.c     | 48 ++++++++++++++++---------------------
 drivers/gpu/drm/i915/intel_dp_mst.c |  2 ++
 drivers/gpu/drm/i915/intel_sdvo.c   | 19 ++++++++-------
 3 files changed, 34 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ffa43eca14d3..4171e2170e8d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1180,35 +1180,18 @@ intel_dp_aux_fini(struct intel_dp *intel_dp)
 	kfree(intel_dp->aux.name);
 }
 
-static int
+static void
 intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	enum port port = intel_dig_port->port;
-	int ret;
 
 	intel_aux_reg_init(intel_dp);
+	drm_dp_aux_init(&intel_dp->aux);
 
+	/* Failure to allocate our preferred name is not critical */
 	intel_dp->aux.name = kasprintf(GFP_KERNEL, "DPDDC-%c", port_name(port));
-	if (!intel_dp->aux.name)
-		return -ENOMEM;
-
-	intel_dp->aux.dev = connector->base.kdev;
 	intel_dp->aux.transfer = intel_dp_aux_transfer;
-
-	DRM_DEBUG_KMS("registering %s bus for %s\n",
-		      intel_dp->aux.name,
-		      connector->base.kdev->kobj.name);
-
-	ret = drm_dp_aux_register(&intel_dp->aux);
-	if (ret < 0) {
-		DRM_ERROR("drm_dp_aux_register() for %s failed (%d)\n",
-			  intel_dp->aux.name, ret);
-		kfree(intel_dp->aux.name);
-		return ret;
-	}
-
-	return 0;
 }
 
 static int
@@ -4446,6 +4429,20 @@ done:
 	return 0;
 }
 
+static int
+intel_dp_connector_register(struct drm_connector *connector)
+{
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
+
+	i915_debugfs_connector_add(connector);
+
+	DRM_DEBUG_KMS("registering %s bus for %s\n",
+		      intel_dp->aux.name, connector->kdev->kobj.name);
+
+	intel_dp->aux.dev = connector->kdev;
+	return drm_dp_aux_register(&intel_dp->aux);
+}
+
 static void
 intel_dp_connector_unregister(struct drm_connector *connector)
 {
@@ -4572,6 +4569,7 @@ static const struct drm_connector_funcs intel_dp_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = intel_dp_set_property,
 	.atomic_get_property = intel_connector_atomic_get_property,
+	.late_register = intel_dp_connector_register,
 	.early_unregister = intel_dp_connector_unregister,
 	.destroy = intel_dp_connector_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
@@ -5416,7 +5414,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	struct drm_device *dev = intel_encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum port port = intel_dig_port->port;
-	int type, ret;
+	int type;
 
 	if (WARN(intel_dig_port->max_lanes < 1,
 		 "Not enough lanes (%d) for DP on port %c\n",
@@ -5475,6 +5473,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	connector->interlace_allowed = true;
 	connector->doublescan_allowed = 0;
 
+	intel_dp_aux_init(intel_dp, intel_connector);
+
 	INIT_DELAYED_WORK(&intel_dp->panel_vdd_work,
 			  edp_panel_vdd_work);
 
@@ -5519,10 +5519,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		pps_unlock(intel_dp);
 	}
 
-	ret = intel_dp_aux_init(intel_dp, intel_connector);
-	if (ret)
-		goto fail;
-
 	/* init MST on ports that can support it */
 	if (HAS_DP_MST(dev) &&
 	    (port == PORT_B || port == PORT_C || port == PORT_D))
@@ -5546,8 +5542,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
 	}
 
-	i915_debugfs_connector_add(connector);
-
 	return true;
 
 fail:
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 9646816604be..89e7c98f5693 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -477,9 +477,11 @@ static void intel_dp_register_mst_connector(struct drm_connector *connector)
 {
 	struct intel_connector *intel_connector = to_intel_connector(connector);
 	struct drm_device *dev = connector->dev;
+
 	drm_modeset_lock_all(dev);
 	intel_connector_add_to_fbdev(intel_connector);
 	drm_modeset_unlock_all(dev);
+
 	drm_connector_register(&intel_connector->base);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 02b4a6695528..580cc876a90f 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2177,6 +2177,16 @@ done:
 #undef CHECK_PROPERTY
 }
 
+static int
+intel_sdvo_connector_register(struct drm_connector *connector)
+{
+	struct intel_sdvo *sdvo = intel_attached_sdvo(connector);
+
+	return sysfs_create_link(&connector->kdev->kobj,
+				 &sdvo->ddc.dev.kobj,
+				 sdvo->ddc.dev.kobj.name);
+}
+
 static void
 intel_sdvo_connector_unregister(struct drm_connector *connector)
 {
@@ -2193,6 +2203,7 @@ static const struct drm_connector_funcs intel_sdvo_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = intel_sdvo_set_property,
 	.atomic_get_property = intel_connector_atomic_get_property,
+	.late_register = intel_sdvo_connector_register,
 	.early_unregister = intel_sdvo_connector_unregister,
 	.destroy = intel_sdvo_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
@@ -2384,16 +2395,8 @@ intel_sdvo_connector_init(struct intel_sdvo_connector *connector,
 	if (ret < 0)
 		goto err1;
 
-	ret = sysfs_create_link(&drm_connector->kdev->kobj,
-				&encoder->ddc.dev.kobj,
-				encoder->ddc.dev.kobj.name);
-	if (ret < 0)
-		goto err2;
-
 	return 0;
 
-err2:
-	drm_connector_unregister(drm_connector);
 err1:
 	drm_connector_cleanup(drm_connector);
 
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/8] drm/i915: Move backlight registration to connector registration

[Chris Wilson]

Currently the backlight is being registered in the load phase (before
the display and its objects are registered). Move the backlight
registration into the analogous phase by performing it from the
connector registration, just after its creation.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_crt.c     |  1 +
 drivers/gpu/drm/i915/intel_display.c | 15 ++++++++++++++-
 drivers/gpu/drm/i915/intel_dp.c      |  5 +++++
 drivers/gpu/drm/i915/intel_dp_mst.c  |  1 +
 drivers/gpu/drm/i915/intel_drv.h     |  6 +++++-
 drivers/gpu/drm/i915/intel_dsi.c     |  1 +
 drivers/gpu/drm/i915/intel_dvo.c     |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c    |  1 +
 drivers/gpu/drm/i915/intel_lvds.c    |  1 +
 drivers/gpu/drm/i915/intel_panel.c   | 15 +--------------
 drivers/gpu/drm/i915/intel_sdvo.c    |  5 +++++
 drivers/gpu/drm/i915/intel_tv.c      |  1 +
 13 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7c81757f3a3f..cc8d55af5db6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3727,6 +3727,7 @@ extern void intel_modeset_init_hw(struct drm_device *dev);
 extern void intel_modeset_init(struct drm_device *dev);
 extern void intel_modeset_gem_init(struct drm_device *dev);
 extern void intel_modeset_cleanup(struct drm_device *dev);
+extern int intel_connector_register(struct drm_connector *);
 extern void intel_connector_unregister(struct drm_connector *);
 extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
 extern void intel_display_resume(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index e115bcc6766f..8d7d48c74751 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -743,6 +743,7 @@ static const struct drm_connector_funcs intel_crt_connector_funcs = {
 	.dpms = drm_atomic_helper_connector_dpms,
 	.detect = intel_crt_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
+	.late_register = intel_connector_register,
 	.early_unregister = intel_connector_unregister,
 	.destroy = intel_crt_destroy,
 	.set_property = intel_crt_set_property,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0b2cd669ac05..44ab82d45fb0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -16309,8 +16309,21 @@ void intel_modeset_gem_init(struct drm_device *dev)
 			c->state->plane_mask &= ~(1 << drm_plane_index(c->primary));
 		}
 	}
+}
+
+int intel_connector_register(struct drm_connector *connector)
+{
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+	int ret;
+
+	ret = intel_backlight_device_register(intel_connector);
+	if (ret)
+		goto err;
+
+	return 0;
 
-	intel_backlight_register(dev);
+err:
+	return ret;
 }
 
 void intel_connector_unregister(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4171e2170e8d..ddfdf4750396 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4433,6 +4433,11 @@ static int
 intel_dp_connector_register(struct drm_connector *connector)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
+	int ret;
+
+	ret = intel_connector_register(connector);
+	if (ret)
+		return ret;
 
 	i915_debugfs_connector_add(connector);
 
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 89e7c98f5693..5f88e12575ac 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -336,6 +336,7 @@ static const struct drm_connector_funcs intel_dp_mst_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = intel_dp_mst_set_property,
 	.atomic_get_property = intel_connector_atomic_get_property,
+	.late_register = intel_connector_register,
 	.early_unregister = intel_connector_unregister,
 	.destroy = intel_dp_mst_connector_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1a001f9e5cac..de183fccef44 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1500,11 +1500,15 @@ 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);
 
 #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
+int intel_backlight_device_register(struct intel_connector *connector);
 void intel_backlight_device_unregister(struct intel_connector *connector);
 #else /* CONFIG_BACKLIGHT_CLASS_DEVICE */
+static int intel_backlight_device_register(struct intel_connector *connector)
+{
+	return 0;
+}
 static inline void intel_backlight_device_unregister(struct intel_connector *connector)
 {
 }
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index ae2dcaf1b52e..80cc0f9de6c9 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1390,6 +1390,7 @@ static const struct drm_connector_helper_funcs intel_dsi_connector_helper_funcs
 static const struct drm_connector_funcs intel_dsi_connector_funcs = {
 	.dpms = drm_atomic_helper_connector_dpms,
 	.detect = intel_dsi_detect,
+	.late_register = intel_connector_register,
 	.early_unregister = intel_connector_unregister,
 	.destroy = intel_dsi_connector_destroy,
 	.fill_modes = drm_helper_probe_single_connector_modes,
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 60e4ddf2ec6d..669eae46ffa9 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -341,6 +341,7 @@ static void intel_dvo_destroy(struct drm_connector *connector)
 static const struct drm_connector_funcs intel_dvo_connector_funcs = {
 	.dpms = drm_atomic_helper_connector_dpms,
 	.detect = intel_dvo_detect,
+	.late_register = intel_connector_register,
 	.early_unregister = intel_connector_unregister,
 	.destroy = intel_dvo_destroy,
 	.fill_modes = drm_helper_probe_single_connector_modes,
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index fb21626ada64..48ca48c9faff 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1774,6 +1774,7 @@ static const struct drm_connector_funcs intel_hdmi_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = intel_hdmi_set_property,
 	.atomic_get_property = intel_connector_atomic_get_property,
+	.late_register = intel_connector_register,
 	.early_unregister = intel_connector_unregister,
 	.destroy = intel_hdmi_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 5dcb1a12458d..cffc3eb605c5 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -555,6 +555,7 @@ static const struct drm_connector_funcs intel_lvds_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = intel_lvds_set_property,
 	.atomic_get_property = intel_connector_atomic_get_property,
+	.late_register = intel_connector_register,
 	.early_unregister = intel_connector_unregister,
 	.destroy = intel_lvds_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index bf721781c259..3c0b97f0bfae 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1163,7 +1163,7 @@ static const struct backlight_ops intel_backlight_device_ops = {
 	.get_brightness = intel_backlight_device_get_brightness,
 };
 
-static int intel_backlight_device_register(struct intel_connector *connector)
+int intel_backlight_device_register(struct intel_connector *connector)
 {
 	struct intel_panel *panel = &connector->panel;
 	struct backlight_properties props;
@@ -1225,11 +1225,6 @@ void intel_backlight_device_unregister(struct intel_connector *connector)
 		panel->backlight.device = NULL;
 	}
 }
-#else /* CONFIG_BACKLIGHT_CLASS_DEVICE */
-static int intel_backlight_device_register(struct intel_connector *connector)
-{
-	return 0;
-}
 #endif /* CONFIG_BACKLIGHT_CLASS_DEVICE */
 
 /*
@@ -1809,11 +1804,3 @@ 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;
-
-	for_each_intel_connector(dev, connector)
-		intel_backlight_device_register(connector);
-}
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 580cc876a90f..c0c0a65fabf3 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2181,6 +2181,11 @@ static int
 intel_sdvo_connector_register(struct drm_connector *connector)
 {
 	struct intel_sdvo *sdvo = intel_attached_sdvo(connector);
+	int ret;
+
+	ret = intel_connector_register(connector);
+	if (ret)
+		return ret;
 
 	return sysfs_create_link(&connector->kdev->kobj,
 				 &sdvo->ddc.dev.kobj,
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 4ce70a9f9df2..83fe6706a188 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1501,6 +1501,7 @@ out:
 static const struct drm_connector_funcs intel_tv_connector_funcs = {
 	.dpms = drm_atomic_helper_connector_dpms,
 	.detect = intel_tv_detect,
+	.late_register = intel_connector_register,
 	.early_unregister = intel_connector_unregister,
 	.destroy = intel_tv_destroy,
 	.set_property = intel_tv_set_property,
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/8] drm/i915: Move connector registration to driver registration

[Chris Wilson]

Defer connector registration from during construction to the driver
registration phase. This is important for ordering the action correctly,
e.g. not using debugfs before it is ready.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_dma.c      |  2 ++
 drivers/gpu/drm/i915/i915_drv.h      |  2 ++
 drivers/gpu/drm/i915/intel_crt.c     |  2 --
 drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++--
 drivers/gpu/drm/i915/intel_dp.c      |  2 --
 drivers/gpu/drm/i915/intel_dsi.c     |  2 --
 drivers/gpu/drm/i915/intel_dvo.c     |  1 -
 drivers/gpu/drm/i915/intel_hdmi.c    |  1 -
 drivers/gpu/drm/i915/intel_lvds.c    |  1 -
 drivers/gpu/drm/i915/intel_sdvo.c    | 10 ----------
 drivers/gpu/drm/i915/intel_tv.c      |  1 -
 11 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index e28c0ddc0837..29521c4b87a5 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1398,6 +1398,7 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
 		I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
 
 	i915_setup_sysfs(dev);
+	intel_modeset_register(dev_priv);
 
 	if (INTEL_INFO(dev_priv)->num_pipes) {
 		/* Must be done after probing outputs */
@@ -1430,6 +1431,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 	intel_gpu_ips_teardown();
 	acpi_video_unregister();
 	intel_opregion_unregister(dev_priv);
+	intel_modeset_unregister(dev_priv);
 	i915_teardown_sysfs(dev_priv->dev);
 	i915_gem_shrinker_cleanup(dev_priv);
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cc8d55af5db6..9de864727d4b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3727,6 +3727,8 @@ extern void intel_modeset_init_hw(struct drm_device *dev);
 extern void intel_modeset_init(struct drm_device *dev);
 extern void intel_modeset_gem_init(struct drm_device *dev);
 extern void intel_modeset_cleanup(struct drm_device *dev);
+extern void intel_modeset_register(struct drm_i915_private *dev_priv);
+extern void intel_modeset_unregister(struct drm_i915_private *dev_priv);
 extern int intel_connector_register(struct drm_connector *);
 extern void intel_connector_unregister(struct drm_connector *);
 extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 8d7d48c74751..165e4b901548 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -880,8 +880,6 @@ void intel_crt_init(struct drm_device *dev)
 
 	drm_connector_helper_add(connector, &intel_crt_connector_helper_funcs);
 
-	drm_connector_register(connector);
-
 	if (!I915_HAS_HOTPLUG(dev))
 		intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 44ab82d45fb0..1557b00836fe 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15546,6 +15546,16 @@ void intel_modeset_init_hw(struct drm_device *dev)
 	intel_enable_gt_powersave(dev_priv);
 }
 
+void intel_modeset_register(struct drm_i915_private *dev_priv)
+{
+	drm_connector_register_all(dev_priv->dev);
+}
+
+void intel_modeset_unregister(struct drm_i915_private *dev_priv)
+{
+	drm_connector_unregister_all(dev_priv->dev);
+}
+
 /*
  * Calculate what we think the watermarks should be for the state we've read
  * out of the hardware and then immediately program those watermarks so that
@@ -16360,8 +16370,6 @@ void intel_modeset_cleanup(struct drm_device *dev)
 	/* flush any delayed tasks or pending work */
 	flush_scheduled_work();
 
-	drm_connector_unregister_all(dev);
-
 	drm_mode_config_cleanup(dev);
 
 	intel_cleanup_overlay(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ddfdf4750396..45bcc40204c9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5484,7 +5484,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 			  edp_panel_vdd_work);
 
 	intel_connector_attach_encoder(intel_connector, intel_encoder);
-	drm_connector_register(connector);
 
 	if (HAS_DDI(dev))
 		intel_connector->get_hw_state = intel_ddi_connector_get_hw_state;
@@ -5560,7 +5559,6 @@ fail:
 		edp_panel_vdd_off_sync(intel_dp);
 		pps_unlock(intel_dp);
 	}
-	drm_connector_unregister(connector);
 	drm_connector_cleanup(connector);
 
 	return false;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 80cc0f9de6c9..63b720061bd2 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1592,8 +1592,6 @@ void intel_dsi_init(struct drm_device *dev)
 
 	intel_dsi_add_properties(intel_connector);
 
-	drm_connector_register(connector);
-
 	return;
 
 err:
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 669eae46ffa9..14b1d3f333a4 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -551,7 +551,6 @@ void intel_dvo_init(struct drm_device *dev)
 			intel_dvo->panel_wants_dither = true;
 		}
 
-		drm_connector_register(connector);
 		return;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 48ca48c9faff..86e1fdb70141 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1915,7 +1915,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 	intel_hdmi_add_properties(intel_hdmi, connector);
 
 	intel_connector_attach_encoder(intel_connector, intel_encoder);
-	drm_connector_register(connector);
 	intel_hdmi->attached_connector = intel_connector;
 
 	/* For G4X desktop chip, PEG_BAND_GAP_DATA 3:0 must first be written
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index cffc3eb605c5..31af74a53482 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1132,7 +1132,6 @@ out:
 		DRM_DEBUG_KMS("lid notifier registration failed\n");
 		lvds_connector->lid_notifier.notifier_call = NULL;
 	}
-	drm_connector_register(connector);
 
 	return;
 
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index c0c0a65fabf3..ba28f5141687 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2396,16 +2396,8 @@ intel_sdvo_connector_init(struct intel_sdvo_connector *connector,
 	connector->base.get_hw_state = intel_sdvo_connector_get_hw_state;
 
 	intel_connector_attach_encoder(&connector->base, &encoder->base);
-	ret = drm_connector_register(drm_connector);
-	if (ret < 0)
-		goto err1;
 
 	return 0;
-
-err1:
-	drm_connector_cleanup(drm_connector);
-
-	return ret;
 }
 
 static void
@@ -2532,7 +2524,6 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type)
 	return true;
 
 err:
-	drm_connector_unregister(connector);
 	intel_sdvo_destroy(connector);
 	return false;
 }
@@ -2611,7 +2602,6 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
 	return true;
 
 err:
-	drm_connector_unregister(connector);
 	intel_sdvo_destroy(connector);
 	return false;
 }
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 83fe6706a188..153190475ebd 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1642,5 +1642,4 @@ intel_tv_init(struct drm_device *dev)
 	drm_object_attach_property(&connector->base,
 				   dev->mode_config.tv_bottom_margin_property,
 				   intel_tv->margin[TV_MARGIN_BOTTOM]);
-	drm_connector_register(connector);
 }
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/8] drm/i915: Register debugfs interface last

[Chris Wilson]

Currently debugfs files are created before the driver is even loads.
This gives the opportunity for userspace to open that interface and poke
around before the backing data structures are initialised - with the
possibility of oopsing or worse.

Move the creation of the debugfs files to our registration phase, where
we announce our presence to the world when we are ready, i.e the
sequence changes from

	drm_dev_register()
	 -> drm_minor_register()
	  -> drm_debugfs_init()
	   -> i915_debugfs_init()
	 -> i915_driver_load()

to

	drm_dev_register()
	 -> drm_minor_register()
	  -> drm_debugfs_init()
	 -> i915_driver_load()
	  -> i915_debugfs_register()

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++--
 drivers/gpu/drm/i915/i915_dma.c     | 2 ++
 drivers/gpu/drm/i915/i915_drv.c     | 4 ----
 drivers/gpu/drm/i915/i915_drv.h     | 6 ++++--
 4 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5b7526697838..7cf9b1676376 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -5507,8 +5507,9 @@ void intel_display_crc_init(struct drm_device *dev)
 	}
 }
 
-int i915_debugfs_init(struct drm_minor *minor)
+int i915_debugfs_register(struct drm_i915_private *dev_priv)
 {
+	struct drm_minor *minor = dev_priv->dev->primary;
 	int ret, i;
 
 	ret = i915_forcewake_create(minor->debugfs_root, minor);
@@ -5534,8 +5535,9 @@ int i915_debugfs_init(struct drm_minor *minor)
 					minor->debugfs_root, minor);
 }
 
-void i915_debugfs_cleanup(struct drm_minor *minor)
+void i915_debugfs_unregister(struct drm_i915_private *dev_priv)
 {
+	struct drm_minor *minor = dev_priv->dev->primary;
 	int i;
 
 	drm_debugfs_remove_files(i915_debugfs_list,
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 29521c4b87a5..91623874f9a3 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1397,6 +1397,7 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
 	if (intel_vgpu_active(dev_priv))
 		I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
 
+	i915_debugfs_register(dev_priv);
 	i915_setup_sysfs(dev);
 	intel_modeset_register(dev_priv);
 
@@ -1433,6 +1434,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 	intel_opregion_unregister(dev_priv);
 	intel_modeset_unregister(dev_priv);
 	i915_teardown_sysfs(dev_priv->dev);
+	i915_debugfs_unregister(dev_priv);
 	i915_gem_shrinker_cleanup(dev_priv);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3eb47fbcea73..3ea09bd83a5a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1756,10 +1756,6 @@ static struct drm_driver driver = {
 	.postclose = i915_driver_postclose,
 	.set_busid = drm_pci_set_busid,
 
-#if defined(CONFIG_DEBUG_FS)
-	.debugfs_init = i915_debugfs_init,
-	.debugfs_cleanup = i915_debugfs_cleanup,
-#endif
 	.gem_free_object = i915_gem_free_object,
 	.gem_vm_ops = &i915_gem_vm_ops,
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9de864727d4b..44e6715d8b1d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3596,12 +3596,14 @@ int i915_verify_lists(struct drm_device *dev);
 #endif
 
 /* i915_debugfs.c */
-int i915_debugfs_init(struct drm_minor *minor);
-void i915_debugfs_cleanup(struct drm_minor *minor);
 #ifdef CONFIG_DEBUG_FS
+int i915_debugfs_register(struct drm_i915_private *dev_priv);
+void i915_debugfs_unregister(struct drm_i915_private *dev_priv);
 int i915_debugfs_connector_add(struct drm_connector *connector);
 void intel_display_crc_init(struct drm_device *dev);
 #else
+static inline int i915_debugfs_register(struct drm_i915_private *) {return 0;}
+static inline void i915_debugfs_unregister(struct drm_i915_private *) {}
 static inline int i915_debugfs_connector_add(struct drm_connector *connector)
 { return 0; }
 static inline void intel_display_crc_init(struct drm_device *dev) {}
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/8] drm/i915: Demidlayer driver loading

[Chris Wilson]

Take control over allocating, loading and registering the driver from the
DRM midlayer by performing it manually from i915_pci_probe. This allows
us to carefully control the order of when we setup the hardware vs when
it becomes visible to third parties (including userspace). The current
ordering makes the driver visible to userspace first (in order to
coordinate with removed DRI1 userspace), but that ordering incurs risk.
The risk increases as we strive for more asynchronous loading.

One side effect of controlling the allocation is that we can allocate
both the drm_device + drm_i915_private in one block, the next step
towards subclassing.

Unload is still left as before, a mix of midlayer and driver.

v2: After drm_dev_init(), we should call drm_dev_unref() so that we call
drm_dev_release() and free everything from drm_dev_init().

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 76 ++++++++++++++++++++++++++---------------
 drivers/gpu/drm/i915/i915_drv.c |  3 +-
 drivers/gpu/drm/i915/i915_drv.h |  6 +++-
 3 files changed, 54 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 91623874f9a3..0e43ad511833 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1075,9 +1075,10 @@ static void i915_workqueues_cleanup(struct drm_i915_private *dev_priv)
  * function hooks not requiring accessing the device.
  */
 static int i915_driver_init_early(struct drm_i915_private *dev_priv,
-				  struct drm_device *dev,
-				  struct intel_device_info *info)
+				  const struct pci_device_id *ent)
 {
+	const struct intel_device_info *match_info =
+		(struct intel_device_info *)ent->driver_data;
 	struct intel_device_info *device_info;
 	int ret = 0;
 
@@ -1086,8 +1087,8 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 
 	/* Setup the write-once "constant" device info */
 	device_info = (struct intel_device_info *)&dev_priv->info;
-	memcpy(device_info, info, sizeof(dev_priv->info));
-	device_info->device_id = dev->pdev->device;
+	memcpy(device_info, match_info, sizeof(*device_info));
+	device_info->device_id = dev_priv->drm.pdev->device;
 
 	BUG_ON(device_info->gen > sizeof(device_info->gen_mask) * BITS_PER_BYTE);
 	device_info->gen_mask = BIT(device_info->gen - 1);
@@ -1113,18 +1114,18 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 		goto err_workqueues;
 
 	/* This must be called before any calls to HAS_PCH_* */
-	intel_detect_pch(dev);
+	intel_detect_pch(&dev_priv->drm);
 
-	intel_pm_setup(dev);
+	intel_pm_setup(&dev_priv->drm);
 	intel_init_dpio(dev_priv);
 	intel_power_domains_init(dev_priv);
 	intel_irq_init(dev_priv);
 	intel_init_display_hooks(dev_priv);
 	intel_init_clock_gating_hooks(dev_priv);
 	intel_init_audio_hooks(dev_priv);
-	i915_gem_load_init(dev);
+	i915_gem_load_init(&dev_priv->drm);
 
-	intel_display_crc_init(dev);
+	intel_display_crc_init(&dev_priv->drm);
 
 	i915_dump_device_info(dev_priv);
 
@@ -1132,7 +1133,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 	 * very first ones. Almost everything should work, except for maybe
 	 * suspend/resume. And we don't implement workarounds that affect only
 	 * pre-production machines. */
-	if (IS_HSW_EARLY_SDV(dev))
+	if (IS_HSW_EARLY_SDV(dev_priv))
 		DRM_INFO("This is an early pre-production Haswell machine. "
 			 "It may not be fully functional.\n");
 
@@ -1390,6 +1391,7 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
 	struct drm_device *dev = dev_priv->dev;
 
 	i915_gem_shrinker_init(dev_priv);
+
 	/*
 	 * Notify a valid surface after modesetting,
 	 * when running inside a VM.
@@ -1397,9 +1399,13 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
 	if (intel_vgpu_active(dev_priv))
 		I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
 
-	i915_debugfs_register(dev_priv);
-	i915_setup_sysfs(dev);
-	intel_modeset_register(dev_priv);
+	/* Reveal our presence to userspace */
+	if (drm_dev_register(dev, 0) == 0) {
+		i915_debugfs_register(dev_priv);
+		i915_setup_sysfs(dev);
+		intel_modeset_register(dev_priv);
+	} else
+		DRM_ERROR("Failed to register driver for userspace access!\n");
 
 	if (INTEL_INFO(dev_priv)->num_pipes) {
 		/* Must be done after probing outputs */
@@ -1449,24 +1455,38 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
  *   - allocate initial config memory
  *   - setup the DRM framebuffer with the allocated memory
  */
-int i915_driver_load(struct drm_device *dev, unsigned long flags)
+int i915_driver_load(struct pci_dev *pdev,
+		     const struct pci_device_id *ent,
+		     struct drm_driver *driver)
 {
 	struct drm_i915_private *dev_priv;
-	int ret = 0;
+	int ret;
 
+	ret = 0;
 	dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
-	if (dev_priv == NULL)
+	if (dev_priv)
+		ret = drm_dev_init(&dev_priv->drm, driver, &pdev->dev);
+	if (ret) {
+		dev_printk(KERN_ERR, &pdev->dev,
+			   "[" DRM_NAME ":%s] allocation failed\n", __func__);
+		kfree(dev_priv);
 		return -ENOMEM;
+	}
 
-	dev->dev_private = dev_priv;
 	/* Must be set before calling __i915_printk */
-	dev_priv->dev = dev;
+	dev_priv->drm.pdev = pdev;
+	dev_priv->drm.dev_private = dev_priv;
+	dev_priv->dev = &dev_priv->drm;
 
-	ret = i915_driver_init_early(dev_priv, dev,
-				     (struct intel_device_info *)flags);
+	ret = pci_enable_device(pdev);
+	if (ret)
+		goto out_free_priv;
 
+	pci_set_drvdata(pdev, &dev_priv->drm);
+
+	ret = i915_driver_init_early(dev_priv, ent);
 	if (ret < 0)
-		goto out_free_priv;
+		goto out_pci_disable;
 
 	intel_runtime_pm_get(dev_priv);
 
@@ -1483,13 +1503,14 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	 * of the i915_driver_init_/i915_driver_register functions according
 	 * to the role/effect of the given init step.
 	 */
-	if (INTEL_INFO(dev)->num_pipes) {
-		ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes);
+	if (INTEL_INFO(dev_priv)->num_pipes) {
+		ret = drm_vblank_init(dev_priv->dev,
+				      INTEL_INFO(dev_priv)->num_pipes);
 		if (ret)
 			goto out_cleanup_hw;
 	}
 
-	ret = i915_load_modeset_init(dev);
+	ret = i915_load_modeset_init(dev_priv->dev);
 	if (ret < 0)
 		goto out_cleanup_vblank;
 
@@ -1502,7 +1523,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	return 0;
 
 out_cleanup_vblank:
-	drm_vblank_cleanup(dev);
+	drm_vblank_cleanup(dev_priv->dev);
 out_cleanup_hw:
 	i915_driver_cleanup_hw(dev_priv);
 out_cleanup_mmio:
@@ -1510,11 +1531,11 @@ out_cleanup_mmio:
 out_runtime_pm_put:
 	intel_runtime_pm_put(dev_priv);
 	i915_driver_cleanup_early(dev_priv);
+out_pci_disable:
+	pci_disable_device(pdev);
 out_free_priv:
 	i915_load_error(dev_priv, "Device initialization failed (%d)\n", ret);
-
-	kfree(dev_priv);
-
+	drm_dev_unref(&dev_priv->drm);
 	return ret;
 }
 
@@ -1579,7 +1600,6 @@ int i915_driver_unload(struct drm_device *dev)
 	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
 
 	i915_driver_cleanup_early(dev_priv);
-	kfree(dev_priv);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3ea09bd83a5a..1a335e1a8b62 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1034,7 +1034,7 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (vga_switcheroo_client_probe_defer(pdev))
 		return -EPROBE_DEFER;
 
-	return drm_get_pci_dev(pdev, ent, &driver);
+	return i915_driver_load(pdev, ent, &driver);
 }
 
 static void
@@ -1748,7 +1748,6 @@ static struct drm_driver driver = {
 	.driver_features =
 	    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME |
 	    DRIVER_RENDER | DRIVER_MODESET,
-	.load = i915_driver_load,
 	.unload = i915_driver_unload,
 	.open = i915_driver_open,
 	.lastclose = i915_driver_lastclose,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 44e6715d8b1d..20a82b6a050d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1732,6 +1732,8 @@ struct intel_wm_config {
 };
 
 struct drm_i915_private {
+	struct drm_device drm;
+
 	struct drm_device *dev;
 	struct kmem_cache *objects;
 	struct kmem_cache *vmas;
@@ -2902,7 +2904,9 @@ __i915_printk(struct drm_i915_private *dev_priv, const char *level,
 #define i915_report_error(dev_priv, fmt, ...)				   \
 	__i915_printk(dev_priv, KERN_ERR, fmt, ##__VA_ARGS__)
 
-extern int i915_driver_load(struct drm_device *, unsigned long flags);
+extern int i915_driver_load(struct pci_dev *pdev,
+			    const struct pci_device_id *ent,
+			    struct drm_driver *driver);
 extern int i915_driver_unload(struct drm_device *);
 extern int i915_driver_open(struct drm_device *dev, struct drm_file *file);
 extern void i915_driver_lastclose(struct drm_device * dev);
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 7/8] drm/i915: Demidlayer driver unloading

[Chris Wilson]

To complete the transition to manual control of load/unload, we need to
take over unloading from i915_pci_remove(). This allows us to correctly
order our unregister vs shutdown phases, which currently are inverted
due to the midlayer.

However, the unload sequence is still invalid as we shutdown the driver
with the last reference. Ideally, all we want to do is remove the
userspace access on device removal, deferring the cleanup to the
drm_dev_release() - breaking the reference cycles is then left as an
exercise for the reader.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 4 ++++
 drivers/gpu/drm/i915/i915_drv.c | 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 0e43ad511833..b6e7aab109d6 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1435,12 +1435,16 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
 static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 {
 	i915_audio_component_cleanup(dev_priv);
+
 	intel_gpu_ips_teardown();
 	acpi_video_unregister();
 	intel_opregion_unregister(dev_priv);
+
 	intel_modeset_unregister(dev_priv);
 	i915_teardown_sysfs(dev_priv->dev);
 	i915_debugfs_unregister(dev_priv);
+	drm_dev_unregister(dev_priv->dev);
+
 	i915_gem_shrinker_cleanup(dev_priv);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1a335e1a8b62..9ddae6add9e0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1042,7 +1042,8 @@ i915_pci_remove(struct pci_dev *pdev)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
 
-	drm_put_dev(dev);
+	i915_driver_unload(dev);
+	drm_dev_unref(dev);
 }
 
 static int i915_pm_suspend(struct device *dev)
@@ -1748,7 +1749,6 @@ static struct drm_driver driver = {
 	.driver_features =
 	    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME |
 	    DRIVER_RENDER | DRIVER_MODESET,
-	.unload = i915_driver_unload,
 	.open = i915_driver_open,
 	.lastclose = i915_driver_lastclose,
 	.preclose = i915_driver_preclose,
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 8/8] drm/i915: Remove redundant drm_connector_register_all()

[Chris Wilson]

drm_connector_register_all() is now automatically called by
drm_dev_register(), and so we no longer have to do so ourselves (via
intel_modeset_register() after calling drm_dev_register()). Similarly
for unregistering.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_dma.c      |  2 --
 drivers/gpu/drm/i915/i915_drv.h      |  2 --
 drivers/gpu/drm/i915/intel_display.c | 10 ----------
 3 files changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index b6e7aab109d6..af77c18ee2de 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1403,7 +1403,6 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
 	if (drm_dev_register(dev, 0) == 0) {
 		i915_debugfs_register(dev_priv);
 		i915_setup_sysfs(dev);
-		intel_modeset_register(dev_priv);
 	} else
 		DRM_ERROR("Failed to register driver for userspace access!\n");
 
@@ -1440,7 +1439,6 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 	acpi_video_unregister();
 	intel_opregion_unregister(dev_priv);
 
-	intel_modeset_unregister(dev_priv);
 	i915_teardown_sysfs(dev_priv->dev);
 	i915_debugfs_unregister(dev_priv);
 	drm_dev_unregister(dev_priv->dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 20a82b6a050d..ac4fc5e6f14f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3733,8 +3733,6 @@ extern void intel_modeset_init_hw(struct drm_device *dev);
 extern void intel_modeset_init(struct drm_device *dev);
 extern void intel_modeset_gem_init(struct drm_device *dev);
 extern void intel_modeset_cleanup(struct drm_device *dev);
-extern void intel_modeset_register(struct drm_i915_private *dev_priv);
-extern void intel_modeset_unregister(struct drm_i915_private *dev_priv);
 extern int intel_connector_register(struct drm_connector *);
 extern void intel_connector_unregister(struct drm_connector *);
 extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1557b00836fe..254a5679954f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15546,16 +15546,6 @@ void intel_modeset_init_hw(struct drm_device *dev)
 	intel_enable_gt_powersave(dev_priv);
 }
 
-void intel_modeset_register(struct drm_i915_private *dev_priv)
-{
-	drm_connector_register_all(dev_priv->dev);
-}
-
-void intel_modeset_unregister(struct drm_i915_private *dev_priv)
-{
-	drm_connector_unregister_all(dev_priv->dev);
-}
-
 /*
  * Calculate what we think the watermarks should be for the state we've read
  * out of the hardware and then immediately program those watermarks so that
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Ro.CI.BAT: failure for series starting with [1/8] drm/i915: Move panel's backlight setup next to panel init

[Patchwork]

== Series Details ==

Series: series starting with [1/8] drm/i915: Move panel's backlight setup next to panel init
URL   : https://patchwork.freedesktop.org/series/8968/
State : failure

== Summary ==

Series 8968v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/8968/revisions/1/mbox

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-cmd:
                fail       -> PASS       (ro-byt-n2820)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (ro-snb-i7-2620M)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                skip       -> DMESG-WARN (ro-bdw-i5-5250u)
                pass       -> INCOMPLETE (fi-snb-i7-2600)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> SKIP       (ro-bdw-i7-5557U)

fi-snb-i7-2600   total:194  pass:160  dwarn:0   dfail:0   fail:2   skip:31 
ro-bdw-i5-5250u  total:225  pass:197  dwarn:2   dfail:0   fail:0   skip:26 
ro-bdw-i7-5557U  total:225  pass:198  dwarn:0   dfail:0   fail:0   skip:27 
ro-bdw-i7-5600u  total:225  pass:185  dwarn:0   dfail:0   fail:0   skip:40 
ro-byt-n2820     total:225  pass:174  dwarn:0   dfail:0   fail:2   skip:49 
ro-hsw-i3-4010u  total:225  pass:190  dwarn:0   dfail:0   fail:0   skip:35 
ro-hsw-i7-4770r  total:225  pass:190  dwarn:0   dfail:0   fail:0   skip:35 
ro-ilk-i7-620lm  total:225  pass:150  dwarn:0   dfail:0   fail:1   skip:74 
ro-ilk1-i5-650   total:220  pass:150  dwarn:0   dfail:0   fail:1   skip:69 
ro-ivb-i7-3770   total:225  pass:181  dwarn:0   dfail:0   fail:0   skip:44 
ro-ivb2-i7-3770  total:225  pass:185  dwarn:0   dfail:0   fail:0   skip:40 
ro-skl3-i5-6260u total:225  pass:201  dwarn:1   dfail:0   fail:0   skip:23 
ro-snb-i7-2620M  total:225  pass:174  dwarn:0   dfail:0   fail:1   skip:50 
fi-hsw-i7-4770k failed to connect after reboot
fi-skl-i5-6260u failed to connect after reboot
fi-skl-i7-6700k failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1250/

79c69c3 drm-intel-nightly: 2016y-06m-21d-08h-53m-58s UTC integration manifest
cdaff31 drm/i915: Remove redundant drm_connector_register_all()
66de7c0 drm/i915: Demidlayer driver unloading
2e68a8c drm/i915: Demidlayer driver loading
7070be4 drm/i915: Register debugfs interface last
c4e18fe drm/i915: Move connector registration to driver registration
c9f0528 drm/i915: Move backlight registration to connector registration
a89da08 drm/i915: Move registration actions to connector->late_register
09250cf drm/i915: Move panel's backlight setup next to panel init

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/8] drm/i915: Move connector registration to driver registration

[Daniel Vetter]

On Tue, Jun 21, 2016 at 09:53:44AM +0100, Chris Wilson wrote:
> +void intel_modeset_register(struct drm_i915_private *dev_priv)
> +{
> +	drm_connector_register_all(dev_priv->dev);
> +}
> +
> +void intel_modeset_unregister(struct drm_i915_private *dev_priv)
> +{
> +	drm_connector_unregister_all(dev_priv->dev);
> +}

Given that this will all disappear again I wouldn't bother with the
wrapper functions.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/8] drm/i915: Register debugfs interface last

[Daniel Vetter]

On Tue, Jun 21, 2016 at 09:53:45AM +0100, Chris Wilson wrote:
> Currently debugfs files are created before the driver is even loads.
> This gives the opportunity for userspace to open that interface and poke
> around before the backing data structures are initialised - with the
> possibility of oopsing or worse.
> 
> Move the creation of the debugfs files to our registration phase, where
> we announce our presence to the world when we are ready, i.e the
> sequence changes from
> 
> 	drm_dev_register()
> 	 -> drm_minor_register()
> 	  -> drm_debugfs_init()
> 	   -> i915_debugfs_init()
> 	 -> i915_driver_load()
> 
> to
> 
> 	drm_dev_register()
> 	 -> drm_minor_register()
> 	  -> drm_debugfs_init()
> 	 -> i915_driver_load()
> 	  -> i915_debugfs_register()
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

->debugfs_init/cleanup() is another midlayer thing we probably should nuke
over the next decade or so, like with ->load/unload() ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/8] drm/i915: Demidlayer driver loading

[Daniel Vetter]

On Tue, Jun 21, 2016 at 09:53:46AM +0100, Chris Wilson wrote:
> Take control over allocating, loading and registering the driver from the
> DRM midlayer by performing it manually from i915_pci_probe. This allows
> us to carefully control the order of when we setup the hardware vs when
> it becomes visible to third parties (including userspace). The current
> ordering makes the driver visible to userspace first (in order to
> coordinate with removed DRI1 userspace), but that ordering incurs risk.
> The risk increases as we strive for more asynchronous loading.
> 
> One side effect of controlling the allocation is that we can allocate
> both the drm_device + drm_i915_private in one block, the next step
> towards subclassing.
> 
> Unload is still left as before, a mix of midlayer and driver.
> 
> v2: After drm_dev_init(), we should call drm_dev_unref() so that we call
> drm_dev_release() and free everything from drm_dev_init().
> 
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

A bit much s/dev/dev_priv/ conversion here for my taste, but meh. One real
issue spotted below.

> ---
>  drivers/gpu/drm/i915/i915_dma.c | 76 ++++++++++++++++++++++++++---------------
>  drivers/gpu/drm/i915/i915_drv.c |  3 +-
>  drivers/gpu/drm/i915/i915_drv.h |  6 +++-
>  3 files changed, 54 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 91623874f9a3..0e43ad511833 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1075,9 +1075,10 @@ static void i915_workqueues_cleanup(struct drm_i915_private *dev_priv)
>   * function hooks not requiring accessing the device.
>   */
>  static int i915_driver_init_early(struct drm_i915_private *dev_priv,
> -				  struct drm_device *dev,
> -				  struct intel_device_info *info)
> +				  const struct pci_device_id *ent)
>  {
> +	const struct intel_device_info *match_info =
> +		(struct intel_device_info *)ent->driver_data;
>  	struct intel_device_info *device_info;
>  	int ret = 0;
>  
> @@ -1086,8 +1087,8 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
>  
>  	/* Setup the write-once "constant" device info */
>  	device_info = (struct intel_device_info *)&dev_priv->info;
> -	memcpy(device_info, info, sizeof(dev_priv->info));
> -	device_info->device_id = dev->pdev->device;
> +	memcpy(device_info, match_info, sizeof(*device_info));
> +	device_info->device_id = dev_priv->drm.pdev->device;
>  
>  	BUG_ON(device_info->gen > sizeof(device_info->gen_mask) * BITS_PER_BYTE);
>  	device_info->gen_mask = BIT(device_info->gen - 1);
> @@ -1113,18 +1114,18 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
>  		goto err_workqueues;
>  
>  	/* This must be called before any calls to HAS_PCH_* */
> -	intel_detect_pch(dev);
> +	intel_detect_pch(&dev_priv->drm);
>  
> -	intel_pm_setup(dev);
> +	intel_pm_setup(&dev_priv->drm);
>  	intel_init_dpio(dev_priv);
>  	intel_power_domains_init(dev_priv);
>  	intel_irq_init(dev_priv);
>  	intel_init_display_hooks(dev_priv);
>  	intel_init_clock_gating_hooks(dev_priv);
>  	intel_init_audio_hooks(dev_priv);
> -	i915_gem_load_init(dev);
> +	i915_gem_load_init(&dev_priv->drm);
>  
> -	intel_display_crc_init(dev);
> +	intel_display_crc_init(&dev_priv->drm);
>  
>  	i915_dump_device_info(dev_priv);
>  
> @@ -1132,7 +1133,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
>  	 * very first ones. Almost everything should work, except for maybe
>  	 * suspend/resume. And we don't implement workarounds that affect only
>  	 * pre-production machines. */
> -	if (IS_HSW_EARLY_SDV(dev))
> +	if (IS_HSW_EARLY_SDV(dev_priv))
>  		DRM_INFO("This is an early pre-production Haswell machine. "
>  			 "It may not be fully functional.\n");
>  
> @@ -1390,6 +1391,7 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
>  	struct drm_device *dev = dev_priv->dev;
>  
>  	i915_gem_shrinker_init(dev_priv);
> +
>  	/*
>  	 * Notify a valid surface after modesetting,
>  	 * when running inside a VM.
> @@ -1397,9 +1399,13 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
>  	if (intel_vgpu_active(dev_priv))
>  		I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
>  
> -	i915_debugfs_register(dev_priv);
> -	i915_setup_sysfs(dev);
> -	intel_modeset_register(dev_priv);
> +	/* Reveal our presence to userspace */
> +	if (drm_dev_register(dev, 0) == 0) {
> +		i915_debugfs_register(dev_priv);
> +		i915_setup_sysfs(dev);
> +		intel_modeset_register(dev_priv);
> +	} else
> +		DRM_ERROR("Failed to register driver for userspace access!\n");
>  
>  	if (INTEL_INFO(dev_priv)->num_pipes) {
>  		/* Must be done after probing outputs */
> @@ -1449,24 +1455,38 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
>   *   - allocate initial config memory
>   *   - setup the DRM framebuffer with the allocated memory
>   */
> -int i915_driver_load(struct drm_device *dev, unsigned long flags)
> +int i915_driver_load(struct pci_dev *pdev,
> +		     const struct pci_device_id *ent,
> +		     struct drm_driver *driver)
>  {
>  	struct drm_i915_private *dev_priv;
> -	int ret = 0;
> +	int ret;
>  
> +	ret = 0;
>  	dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
> -	if (dev_priv == NULL)
> +	if (dev_priv)
> +		ret = drm_dev_init(&dev_priv->drm, driver, &pdev->dev);
> +	if (ret) {

This ended up a bit too clever, will fail to spot the failure when
dev_priv == NULL. I guess you wanted a ret = -ENOMEM up there.
-Daniel

> +		dev_printk(KERN_ERR, &pdev->dev,
> +			   "[" DRM_NAME ":%s] allocation failed\n", __func__);
> +		kfree(dev_priv);
>  		return -ENOMEM;
> +	}
>  
> -	dev->dev_private = dev_priv;
>  	/* Must be set before calling __i915_printk */
> -	dev_priv->dev = dev;
> +	dev_priv->drm.pdev = pdev;
> +	dev_priv->drm.dev_private = dev_priv;
> +	dev_priv->dev = &dev_priv->drm;
>  
> -	ret = i915_driver_init_early(dev_priv, dev,
> -				     (struct intel_device_info *)flags);
> +	ret = pci_enable_device(pdev);
> +	if (ret)
> +		goto out_free_priv;
>  
> +	pci_set_drvdata(pdev, &dev_priv->drm);
> +
> +	ret = i915_driver_init_early(dev_priv, ent);
>  	if (ret < 0)
> -		goto out_free_priv;
> +		goto out_pci_disable;
>  
>  	intel_runtime_pm_get(dev_priv);
>  
> @@ -1483,13 +1503,14 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  	 * of the i915_driver_init_/i915_driver_register functions according
>  	 * to the role/effect of the given init step.
>  	 */
> -	if (INTEL_INFO(dev)->num_pipes) {
> -		ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes);
> +	if (INTEL_INFO(dev_priv)->num_pipes) {
> +		ret = drm_vblank_init(dev_priv->dev,
> +				      INTEL_INFO(dev_priv)->num_pipes);
>  		if (ret)
>  			goto out_cleanup_hw;
>  	}
>  
> -	ret = i915_load_modeset_init(dev);
> +	ret = i915_load_modeset_init(dev_priv->dev);
>  	if (ret < 0)
>  		goto out_cleanup_vblank;
>  
> @@ -1502,7 +1523,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  	return 0;
>  
>  out_cleanup_vblank:
> -	drm_vblank_cleanup(dev);
> +	drm_vblank_cleanup(dev_priv->dev);
>  out_cleanup_hw:
>  	i915_driver_cleanup_hw(dev_priv);
>  out_cleanup_mmio:
> @@ -1510,11 +1531,11 @@ out_cleanup_mmio:
>  out_runtime_pm_put:
>  	intel_runtime_pm_put(dev_priv);
>  	i915_driver_cleanup_early(dev_priv);
> +out_pci_disable:
> +	pci_disable_device(pdev);
>  out_free_priv:
>  	i915_load_error(dev_priv, "Device initialization failed (%d)\n", ret);
> -
> -	kfree(dev_priv);
> -
> +	drm_dev_unref(&dev_priv->drm);
>  	return ret;
>  }
>  
> @@ -1579,7 +1600,6 @@ int i915_driver_unload(struct drm_device *dev)
>  	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>  
>  	i915_driver_cleanup_early(dev_priv);
> -	kfree(dev_priv);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 3ea09bd83a5a..1a335e1a8b62 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1034,7 +1034,7 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (vga_switcheroo_client_probe_defer(pdev))
>  		return -EPROBE_DEFER;
>  
> -	return drm_get_pci_dev(pdev, ent, &driver);
> +	return i915_driver_load(pdev, ent, &driver);
>  }
>  
>  static void
> @@ -1748,7 +1748,6 @@ static struct drm_driver driver = {
>  	.driver_features =
>  	    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME |
>  	    DRIVER_RENDER | DRIVER_MODESET,
> -	.load = i915_driver_load,
>  	.unload = i915_driver_unload,
>  	.open = i915_driver_open,
>  	.lastclose = i915_driver_lastclose,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 44e6715d8b1d..20a82b6a050d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1732,6 +1732,8 @@ struct intel_wm_config {
>  };
>  
>  struct drm_i915_private {
> +	struct drm_device drm;
> +
>  	struct drm_device *dev;
>  	struct kmem_cache *objects;
>  	struct kmem_cache *vmas;
> @@ -2902,7 +2904,9 @@ __i915_printk(struct drm_i915_private *dev_priv, const char *level,
>  #define i915_report_error(dev_priv, fmt, ...)				   \
>  	__i915_printk(dev_priv, KERN_ERR, fmt, ##__VA_ARGS__)
>  
> -extern int i915_driver_load(struct drm_device *, unsigned long flags);
> +extern int i915_driver_load(struct pci_dev *pdev,
> +			    const struct pci_device_id *ent,
> +			    struct drm_driver *driver);
>  extern int i915_driver_unload(struct drm_device *);
>  extern int i915_driver_open(struct drm_device *dev, struct drm_file *file);
>  extern void i915_driver_lastclose(struct drm_device * dev);
> -- 
> 2.8.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915: Remove redundant drm_connector_register_all()

[Daniel Vetter]

On Tue, Jun 21, 2016 at 09:53:48AM +0100, Chris Wilson wrote:
> drm_connector_register_all() is now automatically called by
> drm_dev_register(), and so we no longer have to do so ourselves (via
> intel_modeset_register() after calling drm_dev_register()). Similarly
> for unregistering.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Ok, they disappear again. Feel free to ignore my earlier comment. With the
other nitpicks fixed, on the entire series:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/i915_dma.c      |  2 --
>  drivers/gpu/drm/i915/i915_drv.h      |  2 --
>  drivers/gpu/drm/i915/intel_display.c | 10 ----------
>  3 files changed, 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index b6e7aab109d6..af77c18ee2de 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1403,7 +1403,6 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
>  	if (drm_dev_register(dev, 0) == 0) {
>  		i915_debugfs_register(dev_priv);
>  		i915_setup_sysfs(dev);
> -		intel_modeset_register(dev_priv);
>  	} else
>  		DRM_ERROR("Failed to register driver for userspace access!\n");
>  
> @@ -1440,7 +1439,6 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
>  	acpi_video_unregister();
>  	intel_opregion_unregister(dev_priv);
>  
> -	intel_modeset_unregister(dev_priv);
>  	i915_teardown_sysfs(dev_priv->dev);
>  	i915_debugfs_unregister(dev_priv);
>  	drm_dev_unregister(dev_priv->dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 20a82b6a050d..ac4fc5e6f14f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3733,8 +3733,6 @@ extern void intel_modeset_init_hw(struct drm_device *dev);
>  extern void intel_modeset_init(struct drm_device *dev);
>  extern void intel_modeset_gem_init(struct drm_device *dev);
>  extern void intel_modeset_cleanup(struct drm_device *dev);
> -extern void intel_modeset_register(struct drm_i915_private *dev_priv);
> -extern void intel_modeset_unregister(struct drm_i915_private *dev_priv);
>  extern int intel_connector_register(struct drm_connector *);
>  extern void intel_connector_unregister(struct drm_connector *);
>  extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1557b00836fe..254a5679954f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15546,16 +15546,6 @@ void intel_modeset_init_hw(struct drm_device *dev)
>  	intel_enable_gt_powersave(dev_priv);
>  }
>  
> -void intel_modeset_register(struct drm_i915_private *dev_priv)
> -{
> -	drm_connector_register_all(dev_priv->dev);
> -}
> -
> -void intel_modeset_unregister(struct drm_i915_private *dev_priv)
> -{
> -	drm_connector_unregister_all(dev_priv->dev);
> -}
> -
>  /*
>   * Calculate what we think the watermarks should be for the state we've read
>   * out of the hardware and then immediately program those watermarks so that
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/8] drm/i915: Move connector registration to driver registration

[Chris Wilson]

On Tue, Jun 21, 2016 at 03:18:12PM +0200, Daniel Vetter wrote:
> On Tue, Jun 21, 2016 at 09:53:44AM +0100, Chris Wilson wrote:
> > +void intel_modeset_register(struct drm_i915_private *dev_priv)
> > +{
> > +	drm_connector_register_all(dev_priv->dev);
> > +}
> > +
> > +void intel_modeset_unregister(struct drm_i915_private *dev_priv)
> > +{
> > +	drm_connector_unregister_all(dev_priv->dev);
> > +}
> 
> Given that this will all disappear again I wouldn't bother with the
> wrapper functions.

They exist to conform with the intel_modeset_phase(). I thought they
helped tell the story of how we are moving the phases around.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/8] drm/i915: Demidlayer driver loading

[Chris Wilson]

On Tue, Jun 21, 2016 at 03:27:02PM +0200, Daniel Vetter wrote:
> On Tue, Jun 21, 2016 at 09:53:46AM +0100, Chris Wilson wrote:
> > -int i915_driver_load(struct drm_device *dev, unsigned long flags)
> > +int i915_driver_load(struct pci_dev *pdev,
> > +		     const struct pci_device_id *ent,
> > +		     struct drm_driver *driver)
> >  {
> >  	struct drm_i915_private *dev_priv;
> > -	int ret = 0;
> > +	int ret;
> >  
> > +	ret = 0;
> >  	dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
> > -	if (dev_priv == NULL)
> > +	if (dev_priv)
> > +		ret = drm_dev_init(&dev_priv->drm, driver, &pdev->dev);
> > +	if (ret) {
> 
> This ended up a bit too clever, will fail to spot the failure when
> dev_priv == NULL. I guess you wanted a ret = -ENOMEM up there.

Yup, and we should then return ret rather than return -ENOMEM.
Ta,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx