[PATCH 1/6] drm: Let userspace check if driver supports modeset

[PATCH 1/6] drm: Let userspace check if driver supports modeset

[José Roberto de Souza]

GPU accelerators usually don't have display block or the display
driver part can be disable when building driver(for servers it save
some resources) so it is important let userspace check this
capability too.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/drm_ioctl.c | 3 +++
 include/uapi/drm/drm.h      | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index ea10e9a26aad..3a8438ae9b51 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -244,6 +244,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
 	case DRM_CAP_SYNCOBJ:
 		req->value = drm_core_check_feature(dev, DRIVER_SYNCOBJ);
 		return 0;
+	case DRM_CAP_MODESET:
+		req->value = drm_core_check_feature(dev, DRIVER_MODESET);
+		return 0;
 	}
 
 	/* Other caps only work with KMS drivers */
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 300f336633f2..85fae6ddbf48 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -649,6 +649,7 @@ struct drm_gem_open {
 #define DRM_CAP_PAGE_FLIP_TARGET	0x11
 #define DRM_CAP_CRTC_IN_VBLANK_EVENT	0x12
 #define DRM_CAP_SYNCOBJ		0x13
+#define DRM_CAP_MODESET		0x14
 
 /** DRM_IOCTL_GET_CAP ioctl argument type */
 struct drm_get_cap {
-- 
2.18.0

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

[PATCH 2/6] drm/i915: Set PCH as NOP if display is disabled

[José Roberto de Souza]

num_pipes is set to 0 if disable_display is set inside
intel_device_info_runtime_init() but when that happen PCH will
already be set in intel_detect_pch().

i915_driver_load()
	i915_driver_init_early()
		...
		intel_detect_pch()
		...
	...
	i915_driver_init_hw()
		intel_device_info_runtime_init()

Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3834bd758a2e..99792039176f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -287,7 +287,8 @@ static void intel_detect_pch(struct drm_i915_private *dev_priv)
 	 * Use PCH_NOP (PCH but no South Display) for PCH platforms without
 	 * display.
 	 */
-	if (pch && INTEL_INFO(dev_priv)->num_pipes == 0) {
+	if (pch && ((INTEL_INFO(dev_priv)->num_pipes == 0) ||
+	    i915_modparams.disable_display)) {
 		DRM_DEBUG_KMS("Display disabled, reverting to NOP PCH\n");
 		dev_priv->pch_type = PCH_NOP;
 		dev_priv->pch_id = 0;
-- 
2.18.0

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

[PATCH 3/6] drm/i915: Move out non-modeset calls from modeset init and cleanup

[José Roberto de Souza]

i915_load_modeset_init() and intel_modeset_cleanup() was initializing
and cleaning up things that is not modeset only.
This will make easy initialize drive without display part.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      | 56 ++++++++++++++++++----------
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c | 16 +++-----
 3 files changed, 42 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 99792039176f..402ed9b4f29e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -665,25 +665,15 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	/* must happen before intel_power_domains_init_hw() on VLV/CHV */
 	intel_update_rawclk(dev_priv);
 
-	intel_power_domains_init_hw(dev_priv, false);
-
 	intel_csr_ucode_init(dev_priv);
 
-	ret = intel_irq_install(dev_priv);
-	if (ret)
-		goto cleanup_csr;
-
 	intel_setup_gmbus(dev_priv);
 
 	/* Important: The output setup functions called by modeset_init need
 	 * working irqs for e.g. gmbus and dp aux transfers. */
 	ret = intel_modeset_init(dev);
 	if (ret)
-		goto cleanup_irq;
-
-	ret = i915_gem_init(dev_priv);
-	if (ret)
-		goto cleanup_modeset;
+		goto cleanup_gmbus;
 
 	intel_setup_overlay(dev_priv);
 
@@ -692,23 +682,17 @@ static int i915_load_modeset_init(struct drm_device *dev)
 
 	ret = intel_fbdev_init(dev);
 	if (ret)
-		goto cleanup_gem;
+		goto cleanup_modeset;
 
 	/* Only enable hotplug handling once the fbdev is fully set up. */
 	intel_hpd_init(dev_priv);
 
 	return 0;
 
-cleanup_gem:
-	if (i915_gem_suspend(dev_priv))
-		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
-	i915_gem_fini(dev_priv);
 cleanup_modeset:
 	intel_modeset_cleanup(dev);
-cleanup_irq:
-	drm_irq_uninstall(dev);
+cleanup_gmbus:
 	intel_teardown_gmbus(dev_priv);
-cleanup_csr:
 	intel_csr_ucode_fini(dev_priv);
 	intel_power_domains_fini(dev_priv);
 	vga_switcheroo_unregister_client(pdev);
@@ -1395,9 +1379,22 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 			goto out_cleanup_hw;
 	}
 
+	ret = intel_irq_install(dev_priv);
+	if (ret)
+		goto out_cleanup_hw;
+
+	/* i915_gem_init() call chain will call
+	 * intel_display_power_put(i915, POWER_DOMAIN_GT_IRQ);
+	 */
+	intel_power_domains_init_hw(dev_priv, false);
+
+	ret = i915_gem_init(dev_priv);
+	if (ret)
+		goto cleanup_irq;
+
 	ret = i915_load_modeset_init(&dev_priv->drm);
 	if (ret < 0)
-		goto out_cleanup_hw;
+		goto cleanup_gem;
 
 	i915_driver_register(dev_priv);
 
@@ -1411,6 +1408,12 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	return 0;
 
+cleanup_gem:
+	if (i915_gem_suspend(dev_priv))
+		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
+	i915_gem_fini(dev_priv);
+cleanup_irq:
+	drm_irq_uninstall(&dev_priv->drm);
 out_cleanup_hw:
 	i915_driver_cleanup_hw(dev_priv);
 out_cleanup_mmio:
@@ -1445,8 +1448,21 @@ void i915_driver_unload(struct drm_device *dev)
 
 	intel_gvt_cleanup(dev_priv);
 
+	intel_modeset_cleanup_prepare(dev);
+
+	intel_disable_gt_powersave(dev_priv);
+
+	/*
+	 * Interrupts and polling as the first thing to avoid creating havoc.
+	 * Too much stuff here (turning of connectors, ...) would
+	 * experience fancy races otherwise.
+	 */
+	intel_irq_uninstall(dev_priv);
+
 	intel_modeset_cleanup(dev);
 
+	intel_cleanup_gt_powersave(dev_priv);
+
 	intel_bios_cleanup(dev_priv);
 
 	vga_switcheroo_unregister_client(pdev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4fb937399440..51eb48f6b57a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3419,6 +3419,7 @@ mkwrite_device_info(struct drm_i915_private *dev_priv)
 /* modesetting */
 extern void intel_modeset_init_hw(struct drm_device *dev);
 extern int intel_modeset_init(struct drm_device *dev);
+extern void intel_modeset_cleanup_prepare(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 *);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bbf63741ae80..136fb8d51967 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15905,7 +15905,7 @@ static void intel_hpd_poll_fini(struct drm_device *dev)
 	drm_connector_list_iter_end(&conn_iter);
 }
 
-void intel_modeset_cleanup(struct drm_device *dev)
+void intel_modeset_cleanup_prepare(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
@@ -15913,15 +15913,11 @@ void intel_modeset_cleanup(struct drm_device *dev)
 
 	flush_work(&dev_priv->atomic_helper.free_work);
 	WARN_ON(!llist_empty(&dev_priv->atomic_helper.free_list));
+}
 
-	intel_disable_gt_powersave(dev_priv);
-
-	/*
-	 * Interrupts and polling as the first thing to avoid creating havoc.
-	 * Too much stuff here (turning of connectors, ...) would
-	 * experience fancy races otherwise.
-	 */
-	intel_irq_uninstall(dev_priv);
+void intel_modeset_cleanup(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
 
 	/*
 	 * Due to the hpd irq storm handling the hotplug work can re-arm the
@@ -15943,8 +15939,6 @@ void intel_modeset_cleanup(struct drm_device *dev)
 
 	intel_cleanup_overlay(dev_priv);
 
-	intel_cleanup_gt_powersave(dev_priv);
-
 	intel_teardown_gmbus(dev_priv);
 
 	destroy_workqueue(dev_priv->modeset_wq);
-- 
2.18.0

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

Re: [Intel-gfx] [PATCH 3/6] drm/i915: Move out non-modeset calls from modeset init and cleanup

[Chris Wilson]

Quoting José Roberto de Souza (2018-07-16 23:38:38)
> @@ -1395,9 +1379,22 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
>                         goto out_cleanup_hw;
>         }
>  
> +       ret = intel_irq_install(dev_priv);
> +       if (ret)
> +               goto out_cleanup_hw;
> +
> +       /* i915_gem_init() call chain will call
> +        * intel_display_power_put(i915, POWER_DOMAIN_GT_IRQ);
> +        */
> +       intel_power_domains_init_hw(dev_priv, false);
> +
> +       ret = i915_gem_init(dev_priv);
> +       if (ret)
> +               goto cleanup_irq;
> +
>         ret = i915_load_modeset_init(&dev_priv->drm);
>         if (ret < 0)
> -               goto out_cleanup_hw;
> +               goto cleanup_gem;

Bzzt. Order is extremely important. e.g. modeset init needs to reserve
portions of the GTT still in use by the BIOS before we wipe it.
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/6] drm/i915: Move drm_vblank_init() to i915_load_modeset_init()

[José Roberto de Souza]

i915_load_modeset_init() is a more suitable place than
i915_driver_load() as vblank is part of modeset API.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 402ed9b4f29e..aacb467fe3ea 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -643,6 +643,13 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (i915_inject_load_failure())
 		return -ENODEV;
 
+	if (INTEL_INFO(dev_priv)->num_pipes == 0) {
+		ret = drm_vblank_init(&dev_priv->drm,
+				      INTEL_INFO(dev_priv)->num_pipes);
+		if (ret)
+			goto out;
+	}
+
 	intel_bios_init(dev_priv);
 
 	/* If we have > 1 VGA cards, then we need to arbitrate access
@@ -1367,18 +1374,6 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (ret < 0)
 		goto out_cleanup_mmio;
 
-	/*
-	 * TODO: move the vblank init and parts of modeset init steps into one
-	 * of the i915_driver_init_/i915_driver_register functions according
-	 * to the role/effect of the given init step.
-	 */
-	if (INTEL_INFO(dev_priv)->num_pipes) {
-		ret = drm_vblank_init(&dev_priv->drm,
-				      INTEL_INFO(dev_priv)->num_pipes);
-		if (ret)
-			goto out_cleanup_hw;
-	}
-
 	ret = intel_irq_install(dev_priv);
 	if (ret)
 		goto out_cleanup_hw;
-- 
2.18.0

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

[PATCH 5/6] drm/i915: Do not call modeset related functions when display is disabled

[José Roberto de Souza]

No need to run i915_load_modeset_init() when num_pipes == 0 also
kms depends on things initialized in i915_load_modeset_init() so not
initializing it too. fbdev and audio have guards against
num_pipes == 0 but lets move it to the if block to make it explicit
to readers.

Also as planes, CRTCs, encoders and connectors are not being added
it is necessary to unset the MODESET driver feature otherwise it
will crash when registering driver in drm, also disabling ATOMIC as
do not make sense have ATOMIC and do not have MODESET.

There is more modeset/display calls that still needs to be removed,
this is a initial work.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 69 ++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index aacb467fe3ea..e109815cfa51 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1248,23 +1248,26 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
 	if (IS_GEN5(dev_priv))
 		intel_gpu_ips_init(dev_priv);
 
-	intel_audio_init(dev_priv);
+	if (INTEL_INFO(dev_priv)->num_pipes) {
+		intel_audio_init(dev_priv);
 
-	/*
-	 * Some ports require correctly set-up hpd registers for detection to
-	 * work properly (leading to ghost connected connector status), e.g. VGA
-	 * on gm45.  Hence we can only set up the initial fbdev config after hpd
-	 * irqs are fully enabled. We do it last so that the async config
-	 * cannot run before the connectors are registered.
-	 */
-	intel_fbdev_initial_config_async(dev);
+		/*
+		 * Some ports require correctly set-up hpd registers for
+		 * detection to work properly (leading to ghost connected
+		 * connector status), e.g. VGA on gm45.  Hence we can only set
+		 * up the initial fbdev config after hpd irqs are fully enabled.
+		 * We do it last so that the async config cannot run before the
+		 * connectors are registered.
+		 */
+		intel_fbdev_initial_config_async(dev);
 
-	/*
-	 * We need to coordinate the hotplugs with the asynchronous fbdev
-	 * configuration, for which we use the fbdev->async_cookie.
-	 */
-	if (INTEL_INFO(dev_priv)->num_pipes)
+		/*
+		 * We need to coordinate the hotplugs with the asynchronous
+		 * fbdev configuration, for which we use the
+		 * fbdev->async_cookie.
+		 */
 		drm_kms_helper_poll_init(dev);
+	}
 }
 
 /**
@@ -1273,15 +1276,17 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
  */
 static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 {
-	intel_fbdev_unregister(dev_priv);
-	intel_audio_deinit(dev_priv);
+	if (INTEL_INFO(dev_priv)->num_pipes) {
+		intel_fbdev_unregister(dev_priv);
+		intel_audio_deinit(dev_priv);
 
-	/*
-	 * After flushing the fbdev (incl. a late async config which will
-	 * have delayed queuing of a hotplug event), then flush the hotplug
-	 * events.
-	 */
-	drm_kms_helper_poll_fini(&dev_priv->drm);
+		/*
+		 * After flushing the fbdev (incl. a late async config which
+		 * will have delayed queuing of a hotplug event), then flush the
+		 * hotplug events.
+		 */
+		drm_kms_helper_poll_fini(&dev_priv->drm);
+	}
 
 	intel_gpu_ips_teardown();
 	acpi_video_unregister();
@@ -1333,6 +1338,9 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (!i915_modparams.nuclear_pageflip && match_info->gen < 5)
 		driver.driver_features &= ~DRIVER_ATOMIC;
 
+	if (i915_modparams.disable_display)
+		driver.driver_features &= ~(DRIVER_MODESET | DRIVER_ATOMIC);
+
 	ret = -ENOMEM;
 	dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
 	if (dev_priv)
@@ -1387,9 +1395,11 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (ret)
 		goto cleanup_irq;
 
-	ret = i915_load_modeset_init(&dev_priv->drm);
-	if (ret < 0)
-		goto cleanup_gem;
+	if (INTEL_INFO(dev_priv)->num_pipes) {
+		ret = i915_load_modeset_init(&dev_priv->drm);
+		if (ret < 0)
+			goto cleanup_gem;
+	}
 
 	i915_driver_register(dev_priv);
 
@@ -1439,11 +1449,13 @@ void i915_driver_unload(struct drm_device *dev)
 
 	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
 
-	drm_atomic_helper_shutdown(dev);
+	if (INTEL_INFO(dev_priv)->num_pipes)
+		drm_atomic_helper_shutdown(dev);
 
 	intel_gvt_cleanup(dev_priv);
 
-	intel_modeset_cleanup_prepare(dev);
+	if (INTEL_INFO(dev_priv)->num_pipes)
+		intel_modeset_cleanup_prepare(dev);
 
 	intel_disable_gt_powersave(dev_priv);
 
@@ -1454,7 +1466,8 @@ void i915_driver_unload(struct drm_device *dev)
 	 */
 	intel_irq_uninstall(dev_priv);
 
-	intel_modeset_cleanup(dev);
+	if (INTEL_INFO(dev_priv)->num_pipes)
+		intel_modeset_cleanup(dev);
 
 	intel_cleanup_gt_powersave(dev_priv);
 
-- 
2.18.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/6] drm/i915: Remove redundante checks for num_pipes == 0

[José Roberto de Souza]

This 'if's will always be false because of previous changes.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      | 12 +++---------
 drivers/gpu/drm/i915/intel_audio.c   |  3 ---
 drivers/gpu/drm/i915/intel_display.c |  3 ---
 drivers/gpu/drm/i915/intel_i2c.c     |  3 ---
 4 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e109815cfa51..bad7ad0bd5ac 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -643,12 +643,9 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (i915_inject_load_failure())
 		return -ENODEV;
 
-	if (INTEL_INFO(dev_priv)->num_pipes == 0) {
-		ret = drm_vblank_init(&dev_priv->drm,
-				      INTEL_INFO(dev_priv)->num_pipes);
-		if (ret)
-			goto out;
-	}
+	ret = drm_vblank_init(&dev_priv->drm, INTEL_INFO(dev_priv)->num_pipes);
+	if (ret)
+		goto out;
 
 	intel_bios_init(dev_priv);
 
@@ -684,9 +681,6 @@ static int i915_load_modeset_init(struct drm_device *dev)
 
 	intel_setup_overlay(dev_priv);
 
-	if (INTEL_INFO(dev_priv)->num_pipes == 0)
-		return 0;
-
 	ret = intel_fbdev_init(dev);
 	if (ret)
 		goto cleanup_modeset;
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index bb94172ffc07..f02cb211d3e7 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -960,9 +960,6 @@ void i915_audio_component_init(struct drm_i915_private *dev_priv)
 {
 	int ret;
 
-	if (INTEL_INFO(dev_priv)->num_pipes == 0)
-		return;
-
 	ret = component_add(dev_priv->drm.dev, &i915_audio_component_bind_ops);
 	if (ret < 0) {
 		DRM_ERROR("failed to add audio component (%d)\n", ret);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 136fb8d51967..b572151c52fa 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15137,9 +15137,6 @@ int intel_modeset_init(struct drm_device *dev)
 
 	intel_init_pm(dev_priv);
 
-	if (INTEL_INFO(dev_priv)->num_pipes == 0)
-		return 0;
-
 	/*
 	 * There may be no VBT; and if the BIOS enabled SSC we can
 	 * just keep using it to avoid unnecessary flicker.  Whereas if the
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index bef32b7c248e..2f941c5b2e8c 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -819,9 +819,6 @@ int intel_setup_gmbus(struct drm_i915_private *dev_priv)
 	unsigned int pin;
 	int ret;
 
-	if (INTEL_INFO(dev_priv)->num_pipes == 0)
-		return 0;
-
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		dev_priv->gpio_mmio_base = VLV_DISPLAY_BASE;
 	else if (!HAS_GMCH_DISPLAY(dev_priv))
-- 
2.18.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 1/6] drm: Let userspace check if driver supports modeset

[Chris Wilson]

Quoting José Roberto de Souza (2018-07-16 23:38:36)
> GPU accelerators usually don't have display block or the display
> driver part can be disable when building driver(for servers it save
> some resources) so it is important let userspace check this
> capability too.

We currently communicate that by having no modeset resources. What does
another cap bit accomplish that we don't already know?
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/6] drm: Let userspace check if driver supports modeset

[Souza, Jose]

On Tue, 2018-07-17 at 08:28 +0100, Chris Wilson wrote:
> Quoting José Roberto de Souza (2018-07-16 23:38:36)
> > GPU accelerators usually don't have display block or the display
> > driver part can be disable when building driver(for servers it save
> > some resources) so it is important let userspace check this
> > capability too.
> 
> We currently communicate that by having no modeset resources. What
> does
> another cap bit accomplish that we don't already know?

This is a hackish way to check if driver support modeset,
drmModeGetResources()/drm_mode_getresources() can fail and return null
by other reasons and just check for the errno value can be misleading
too.

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

Re: [PATCH 1/6] drm: Let userspace check if driver supports modeset

[Chris Wilson]

Quoting Souza, Jose (2018-07-17 18:02:17)
> On Tue, 2018-07-17 at 08:28 +0100, Chris Wilson wrote:
> > Quoting José Roberto de Souza (2018-07-16 23:38:36)
> > > GPU accelerators usually don't have display block or the display
> > > driver part can be disable when building driver(for servers it save
> > > some resources) so it is important let userspace check this
> > > capability too.
> > 
> > We currently communicate that by having no modeset resources. What
> > does
> > another cap bit accomplish that we don't already know?
> 
> This is a hackish way to check if driver support modeset,
> drmModeGetResources()/drm_mode_getresources() can fail and return null
> by other reasons and just check for the errno value can be misleading
> too.

Do not confuse libdrm with the ioctl. You do not need to allocate
anything for such a check, and it is being used directly without
allocations as a has-kms check.

More to the point existing userspace determines modeset capability
through the reported resources. Your changelog needs to explain why they
are inadequate and how you plan to coordinate your fix with userspace.
As it stands you are introducing uABI with no user...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/6] drm: Let userspace check if driver supports modeset

[Rodrigo Vivi]

On Tue, Jul 17, 2018 at 09:04:45PM +0100, Chris Wilson wrote:
> Quoting Souza, Jose (2018-07-17 18:02:17)
> > On Tue, 2018-07-17 at 08:28 +0100, Chris Wilson wrote:
> > > Quoting José Roberto de Souza (2018-07-16 23:38:36)
> > > > GPU accelerators usually don't have display block or the display
> > > > driver part can be disable when building driver(for servers it save
> > > > some resources) so it is important let userspace check this
> > > > capability too.
> > > 
> > > We currently communicate that by having no modeset resources. What
> > > does
> > > another cap bit accomplish that we don't already know?
> > 
> > This is a hackish way to check if driver support modeset,
> > drmModeGetResources()/drm_mode_getresources() can fail and return null
> > by other reasons and just check for the errno value can be misleading
> > too.
> 
> Do not confuse libdrm with the ioctl. You do not need to allocate
> anything for such a check, and it is being used directly without
> allocations as a has-kms check.
> 
> More to the point existing userspace determines modeset capability
> through the reported resources. Your changelog needs to explain why they
> are inadequate and how you plan to coordinate your fix with userspace.
> As it stands you are introducing uABI with no user...

I agree with Chris here. There is no indication on how this will
affect userspace here and this so far is just a uABI without user
that is a big blocker.

And I also got confused because that modeset capability check
got introduced in a block that is for
"/* Only some caps make sense with UMS/render-only drivers. */"

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

Re: [Intel-gfx] [PATCH 1/6] drm: Let userspace check if driver supports modeset

[Souza, Jose]

On Wed, 2018-08-15 at 14:15 -0700, Rodrigo Vivi wrote:
> On Tue, Jul 17, 2018 at 09:04:45PM +0100, Chris Wilson wrote:
> > Quoting Souza, Jose (2018-07-17 18:02:17)
> > > On Tue, 2018-07-17 at 08:28 +0100, Chris Wilson wrote:
> > > > Quoting José Roberto de Souza (2018-07-16 23:38:36)
> > > > > GPU accelerators usually don't have display block or the
> > > > > display
> > > > > driver part can be disable when building driver(for servers
> > > > > it save
> > > > > some resources) so it is important let userspace check this
> > > > > capability too.
> > > > 
> > > > We currently communicate that by having no modeset resources.
> > > > What
> > > > does
> > > > another cap bit accomplish that we don't already know?
> > > 
> > > This is a hackish way to check if driver support modeset,
> > > drmModeGetResources()/drm_mode_getresources() can fail and return
> > > null
> > > by other reasons and just check for the errno value can be
> > > misleading
> > > too.
> > 
> > Do not confuse libdrm with the ioctl. You do not need to allocate
> > anything for such a check, and it is being used directly without
> > allocations as a has-kms check.
> > 
> > More to the point existing userspace determines modeset capability
> > through the reported resources. Your changelog needs to explain why
> > they
> > are inadequate and how you plan to coordinate your fix with
> > userspace.
> > As it stands you are introducing uABI with no user...
> 
> I agree with Chris here. There is no indication on how this will
> affect userspace here and this so far is just a uABI without user
> that is a big blocker.

The user would be IGT but I would only send the patch after get this
merged.

Okay, I can drop this patch without any changes in this series.

> 
> And I also got confused because that modeset capability check
> got introduced in a block that is for
> "/* Only some caps make sense with UMS/render-only drivers. */"
> 
> > -Chris
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel