[v9,05/39] drm/i915: component master at i915 driver load

Submitted by Ramalingam C on Dec. 13, 2018, 4:01 a.m.

Details

Message ID 1544673701-6353-6-git-send-email-ramalingam.c@intel.com
State New
Headers show
Series "drm/i915: Implement HDCP2.2" ( rev: 12 11 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Ramalingam C Dec. 13, 2018, 4:01 a.m.
A generic component master is added to hold the i915 registration
until all required kernel modules are up and active.

This is achieved through following steps:
  - moving the i915 driver registration to the component master's
    bind call
  - all required kernel modules will add one component each to
    component_match of I915 component master.

If no component is added to the I915 component master, due to CONFIG
selection or HW limitation, component master's bind call (i915
registration) will be triggered with no wait.

Similarly when a associated component is removed for some reasons,
I915 will be unregistered through component master unbind.

v2:
  i915_driver_unregister is added to the unbind of master.
v3:
  In master_unbind i915_unregister->drm_atomic_helper_shutdown->
	component_unbind_all [Daniel]

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.c | 86 +++++++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/i915_drv.h |  3 ++
 include/drm/i915_component.h    | 11 ++++++
 3 files changed, 92 insertions(+), 8 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b310a897a4ad..b8a204072e60 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -39,12 +39,14 @@ 
 #include <linux/vgaarb.h>
 #include <linux/vga_switcheroo.h>
 #include <linux/vt.h>
+#include <linux/component.h>
 #include <acpi/video.h>
 
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/i915_drm.h>
+#include <drm/i915_component.h>
 
 #include "i915_drv.h"
 #include "i915_trace.h"
@@ -1577,8 +1579,6 @@  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);
-
 	/*
 	 * Some ports require correctly set-up hpd registers for detection to
 	 * work properly (leading to ghost connected connector status), e.g. VGA
@@ -1609,7 +1609,6 @@  static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 	intel_power_domains_disable(dev_priv);
 
 	intel_fbdev_unregister(dev_priv);
-	intel_audio_deinit(dev_priv);
 
 	/*
 	 * After flushing the fbdev (incl. a late async config which will
@@ -1694,6 +1693,48 @@  static void i915_driver_destroy(struct drm_i915_private *i915)
 	pci_set_drvdata(pdev, NULL);
 }
 
+static void i915_driver_load_tail(struct drm_i915_private *dev_priv)
+{
+	i915_driver_register(dev_priv);
+
+	DRM_INFO("load_tail: I915 driver registered\n");
+}
+
+static void i915_driver_unload_head(struct drm_i915_private *dev_priv)
+{
+	i915_driver_unregister(dev_priv);
+
+	DRM_INFO("unload_head: I915 driver unregistered\n");
+}
+
+static int i915_component_master_bind(struct device *dev)
+{
+	struct drm_i915_private *dev_priv = kdev_to_i915(dev);
+	int ret;
+
+	ret = component_bind_all(dev, dev_priv->comp_master);
+	if (ret < 0)
+		return ret;
+
+	i915_driver_load_tail(dev_priv);
+
+	return 0;
+}
+
+static void i915_component_master_unbind(struct device *dev)
+{
+	struct drm_i915_private *dev_priv = kdev_to_i915(dev);
+
+	i915_driver_unload_head(dev_priv);
+	drm_atomic_helper_shutdown(&dev_priv->drm);
+	component_unbind_all(dev, dev_priv->comp_master);
+}
+
+static const struct component_master_ops i915_component_master_ops = {
+	.bind = i915_component_master_bind,
+	.unbind = i915_component_master_unbind,
+};
+
 /**
  * i915_driver_load - setup chip and create an initial config
  * @pdev: PCI device
@@ -1720,9 +1761,22 @@  int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (!i915_modparams.nuclear_pageflip && match_info->gen < 5)
 		dev_priv->drm.driver_features &= ~DRIVER_ATOMIC;
 
+	dev_priv->comp_master = kzalloc(sizeof(*dev_priv->comp_master),
+					GFP_KERNEL);
+	if (!dev_priv->comp_master) {
+		ret = -ENOMEM;
+		goto out_fini;
+	}
+
+	component_match_alloc(dev_priv->drm.dev, &dev_priv->master_match);
+	if (!dev_priv->master_match) {
+		ret = -ENOMEM;
+		goto out_comp_master_clean;
+	}
+
 	ret = pci_enable_device(pdev);
 	if (ret)
-		goto out_fini;
+		goto out_comp_master_clean;
 
 	ret = i915_driver_init_early(dev_priv);
 	if (ret < 0)
@@ -1742,14 +1796,27 @@  int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (ret < 0)
 		goto out_cleanup_hw;
 
-	i915_driver_register(dev_priv);
+	ret = component_master_add_with_match(dev_priv->drm.dev,
+					      &i915_component_master_ops,
+					      dev_priv->master_match);
+	if (ret < 0) {
+		DRM_DEV_ERROR(&pdev->dev, "Master comp add failed %d\n",
+			      ret);
+		goto out_cleanup_modeset;
+	}
+
+	intel_audio_init(dev_priv);
 
 	enable_rpm_wakeref_asserts(dev_priv);
 
 	i915_welcome_messages(dev_priv);
 
+	DRM_INFO("I915 registration waits for req component(s). if any...\n");
+
 	return 0;
 
+out_cleanup_modeset:
+	intel_modeset_cleanup(&dev_priv->drm);
 out_cleanup_hw:
 	i915_driver_cleanup_hw(dev_priv);
 out_cleanup_mmio:
@@ -1759,6 +1826,8 @@  int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 	i915_driver_cleanup_early(dev_priv);
 out_pci_disable:
 	pci_disable_device(pdev);
+out_comp_master_clean:
+	kfree(dev_priv->comp_master);
 out_fini:
 	i915_load_error(dev_priv, "Device initialization failed (%d)\n", ret);
 	i915_driver_destroy(dev_priv);
@@ -1772,13 +1841,14 @@  void i915_driver_unload(struct drm_device *dev)
 
 	disable_rpm_wakeref_asserts(dev_priv);
 
-	i915_driver_unregister(dev_priv);
+	component_master_del(dev_priv->drm.dev, &i915_component_master_ops);
+	kfree(dev_priv->comp_master);
+
+	intel_audio_deinit(dev_priv);
 
 	if (i915_gem_suspend(dev_priv))
 		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
 
-	drm_atomic_helper_shutdown(dev);
-
 	intel_gvt_cleanup(dev_priv);
 
 	intel_modeset_cleanup(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e70707e79386..25dc3d7a1e3b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2002,6 +2002,9 @@  struct drm_i915_private {
 
 	struct i915_pmu pmu;
 
+	struct i915_component_master *comp_master;
+	struct component_match *master_match;
+
 	/*
 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
 	 * will be rejected. Instead look for a better place.
diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
index fca22d463e1b..6f94ddd3f2c2 100644
--- a/include/drm/i915_component.h
+++ b/include/drm/i915_component.h
@@ -46,4 +46,15 @@  struct i915_audio_component {
 	int aud_sample_rate[MAX_PORTS];
 };
 
+/**
+ * struct i915_component_master - Used for communication between i915
+ *				  and any other drivers for the services
+ *				  of different feature.
+ */
+struct i915_component_master {
+	/*
+	 * Add here the interface details between I915 and interested modules.
+	 */
+};
+
 #endif /* _I915_COMPONENT_H_ */

Comments

On Thu, Dec 13, 2018 at 09:31:07AM +0530, Ramalingam C wrote:
> A generic component master is added to hold the i915 registration
> until all required kernel modules are up and active.
> 
> This is achieved through following steps:
>   - moving the i915 driver registration to the component master's
>     bind call
>   - all required kernel modules will add one component each to
>     component_match of I915 component master.
> 
> If no component is added to the I915 component master, due to CONFIG
> selection or HW limitation, component master's bind call (i915
> registration) will be triggered with no wait.
> 
> Similarly when a associated component is removed for some reasons,
> I915 will be unregistered through component master unbind.
> 
> v2:
>   i915_driver_unregister is added to the unbind of master.
> v3:
>   In master_unbind i915_unregister->drm_atomic_helper_shutdown->
> 	component_unbind_all [Daniel]
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 86 +++++++++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/i915_drv.h |  3 ++
>  include/drm/i915_component.h    | 11 ++++++
>  3 files changed, 92 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b310a897a4ad..b8a204072e60 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -39,12 +39,14 @@
>  #include <linux/vgaarb.h>
>  #include <linux/vga_switcheroo.h>
>  #include <linux/vt.h>
> +#include <linux/component.h>
>  #include <acpi/video.h>
>  
>  #include <drm/drmP.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/i915_drm.h>
> +#include <drm/i915_component.h>
>  
>  #include "i915_drv.h"
>  #include "i915_trace.h"
> @@ -1577,8 +1579,6 @@ 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);
> -
>  	/*
>  	 * Some ports require correctly set-up hpd registers for detection to
>  	 * work properly (leading to ghost connected connector status), e.g. VGA
> @@ -1609,7 +1609,6 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
>  	intel_power_domains_disable(dev_priv);
>  
>  	intel_fbdev_unregister(dev_priv);
> -	intel_audio_deinit(dev_priv);
>  
>  	/*
>  	 * After flushing the fbdev (incl. a late async config which will
> @@ -1694,6 +1693,48 @@ static void i915_driver_destroy(struct drm_i915_private *i915)
>  	pci_set_drvdata(pdev, NULL);
>  }
>  
> +static void i915_driver_load_tail(struct drm_i915_private *dev_priv)
> +{
> +	i915_driver_register(dev_priv);
> +
> +	DRM_INFO("load_tail: I915 driver registered\n");
> +}
> +
> +static void i915_driver_unload_head(struct drm_i915_private *dev_priv)
> +{
> +	i915_driver_unregister(dev_priv);
> +
> +	DRM_INFO("unload_head: I915 driver unregistered\n");
> +}
> +
> +static int i915_component_master_bind(struct device *dev)
> +{
> +	struct drm_i915_private *dev_priv = kdev_to_i915(dev);
> +	int ret;
> +
> +	ret = component_bind_all(dev, dev_priv->comp_master);
> +	if (ret < 0)
> +		return ret;
> +
> +	i915_driver_load_tail(dev_priv);
> +
> +	return 0;
> +}
> +
> +static void i915_component_master_unbind(struct device *dev)
> +{
> +	struct drm_i915_private *dev_priv = kdev_to_i915(dev);
> +
> +	i915_driver_unload_head(dev_priv);
> +	drm_atomic_helper_shutdown(&dev_priv->drm);
> +	component_unbind_all(dev, dev_priv->comp_master);
> +}
> +
> +static const struct component_master_ops i915_component_master_ops = {
> +	.bind = i915_component_master_bind,
> +	.unbind = i915_component_master_unbind,
> +};
> +
>  /**
>   * i915_driver_load - setup chip and create an initial config
>   * @pdev: PCI device
> @@ -1720,9 +1761,22 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (!i915_modparams.nuclear_pageflip && match_info->gen < 5)
>  		dev_priv->drm.driver_features &= ~DRIVER_ATOMIC;
>  
> +	dev_priv->comp_master = kzalloc(sizeof(*dev_priv->comp_master),
> +					GFP_KERNEL);
> +	if (!dev_priv->comp_master) {
> +		ret = -ENOMEM;
> +		goto out_fini;
> +	}
> +
> +	component_match_alloc(dev_priv->drm.dev, &dev_priv->master_match);
> +	if (!dev_priv->master_match) {
> +		ret = -ENOMEM;
> +		goto out_comp_master_clean;
> +	}
> +
>  	ret = pci_enable_device(pdev);
>  	if (ret)
> -		goto out_fini;
> +		goto out_comp_master_clean;
>  
>  	ret = i915_driver_init_early(dev_priv);
>  	if (ret < 0)
> @@ -1742,14 +1796,27 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (ret < 0)
>  		goto out_cleanup_hw;
>  
> -	i915_driver_register(dev_priv);
> +	ret = component_master_add_with_match(dev_priv->drm.dev,
> +					      &i915_component_master_ops,
> +					      dev_priv->master_match);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(&pdev->dev, "Master comp add failed %d\n",
> +			      ret);
> +		goto out_cleanup_modeset;
> +	}
> +

I think a FIXME here would be nice, all we need to do is have a
component_add_locked functions to handle the recursion. Logically there's
not issue really. Same with intel_audio_deinit() below.

Otoh, the component we expose to snd-hda isn't really related to anything
uapi related, it's just a very low-level interface for power domains and a
few other things. Nothing bad happens if we don't register/unregister that
together with all the uapi/kms/fbdev interfaces.

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

> +	intel_audio_init(dev_priv);
>  
>  	enable_rpm_wakeref_asserts(dev_priv);
>  
>  	i915_welcome_messages(dev_priv);
>  
> +	DRM_INFO("I915 registration waits for req component(s). if any...\n");
> +
>  	return 0;
>  
> +out_cleanup_modeset:
> +	intel_modeset_cleanup(&dev_priv->drm);
>  out_cleanup_hw:
>  	i915_driver_cleanup_hw(dev_priv);
>  out_cleanup_mmio:
> @@ -1759,6 +1826,8 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	i915_driver_cleanup_early(dev_priv);
>  out_pci_disable:
>  	pci_disable_device(pdev);
> +out_comp_master_clean:
> +	kfree(dev_priv->comp_master);
>  out_fini:
>  	i915_load_error(dev_priv, "Device initialization failed (%d)\n", ret);
>  	i915_driver_destroy(dev_priv);
> @@ -1772,13 +1841,14 @@ void i915_driver_unload(struct drm_device *dev)
>  
>  	disable_rpm_wakeref_asserts(dev_priv);
>  
> -	i915_driver_unregister(dev_priv);
> +	component_master_del(dev_priv->drm.dev, &i915_component_master_ops);
> +	kfree(dev_priv->comp_master);
> +
> +	intel_audio_deinit(dev_priv);
>  
>  	if (i915_gem_suspend(dev_priv))
>  		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
>  
> -	drm_atomic_helper_shutdown(dev);
> -
>  	intel_gvt_cleanup(dev_priv);
>  
>  	intel_modeset_cleanup(dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e70707e79386..25dc3d7a1e3b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2002,6 +2002,9 @@ struct drm_i915_private {
>  
>  	struct i915_pmu pmu;
>  
> +	struct i915_component_master *comp_master;
> +	struct component_match *master_match;
> +
>  	/*
>  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>  	 * will be rejected. Instead look for a better place.
> diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
> index fca22d463e1b..6f94ddd3f2c2 100644
> --- a/include/drm/i915_component.h
> +++ b/include/drm/i915_component.h
> @@ -46,4 +46,15 @@ struct i915_audio_component {
>  	int aud_sample_rate[MAX_PORTS];
>  };
>  
> +/**
> + * struct i915_component_master - Used for communication between i915
> + *				  and any other drivers for the services
> + *				  of different feature.
> + */
> +struct i915_component_master {
> +	/*
> +	 * Add here the interface details between I915 and interested modules.
> +	 */
> +};
> +
>  #endif /* _I915_COMPONENT_H_ */
> -- 
> 2.7.4
>