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 {
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;
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);
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
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;
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);
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))
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
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
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
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
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@lists.freedesktop.org
-
Chris Wilson -
José Roberto de Souza -
Rodrigo Vivi -
Souza, Jose