[alsa-devel] [Intel-gfx] [PATCH 2/5] drm/i915: add component support

[Jani Nikula]

On Mon, 08 Dec 2014, Imre Deak <imre.deak at intel.com> wrote:
> Register a component master to be used to interface with the
> snd_hda_intel driver. This is meant to replace the same interface that
> is currently based on module symbol lookup.

I think I'd add a whole new intel_component.c file for this.

>
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 80 +++++++++++++++++++++++++++++++++++++++++
>  include/drm/i915_component.h    | 38 ++++++++++++++++++++
>  2 files changed, 118 insertions(+)
>  create mode 100644 include/drm/i915_component.h
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 887d88f..6e0f3be 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -35,6 +35,8 @@
>  #include <drm/drm_legacy.h>
>  #include "intel_drv.h"
>  #include <drm/i915_drm.h>
> +#include <linux/component.h>
> +#include <drm/i915_component.h>
>  #include "i915_drv.h"
>  #include "i915_trace.h"
>  #include <linux/pci.h>
> @@ -600,6 +602,80 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
>  	}
>  }
>  
> +static void i915_component_get_power(struct device *dev)
> +{
> +	intel_display_power_get(dev_to_i915_priv(dev), POWER_DOMAIN_AUDIO);
> +}
> +
> +static void i915_component_put_power(struct device *dev)
> +{
> +	intel_display_power_put(dev_to_i915_priv(dev), POWER_DOMAIN_AUDIO);
> +}
> +
> +/* Get CDCLK in kHz  */
> +static int i915_component_get_cdclk_freq(struct device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev_to_i915_priv(dev);
> +	int ret;
> +
> +	if (WARN_ON_ONCE(!HAS_DDI(dev_priv)))
> +		return -ENODEV;
> +
> +	intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
> +	ret = intel_ddi_get_cdclk_freq(dev_priv);
> +	intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
> +
> +	return ret;
> +}
> +
> +static struct i915_component_ops component_ops = {
> +	.owner		= THIS_MODULE,
> +	.get_power	= i915_component_get_power,
> +	.put_power	= i915_component_put_power,
> +	.get_cdclk	= i915_component_get_cdclk_freq,

Okay, I'm thinking one step ahead, but I'm not familiar with the
component driver stuff yet. Could we use the same component model for
communicating with, say, the pmic driver in the future? Should we use
the same component ops, or add another one? If the same, then they're
all in the same "namespace", and I think we should use something more
specific than just get_power and put_power.

(The master/slave relationship may be the other way round with the pmic
driver I guess. But nonetheless, we should think about reusing this.)

> +};
> +
> +static int i915_component_master_bind(struct device *dev)
> +{
> +	return component_bind_all(dev, &component_ops);
> +}
> +
> +static void i915_component_master_unbind(struct device *dev)
> +{
> +	return component_unbind_all(dev, &component_ops);
> +}
> +
> +static const struct component_master_ops i915_component_master_ops = {
> +	.bind	= i915_component_master_bind,
> +	.unbind	= i915_component_master_unbind,
> +};
> +
> +static int i915_component_master_match(struct device *dev, void *data)
> +{
> +	/* snd_hda_intel is the only supported component */
> +	return !strcmp(dev->driver->name, "snd_hda_intel");
> +}
> +
> +static void i915_component_master_init(struct drm_i915_private *dev_priv)
> +{
> +	struct device *dev = dev_priv->dev->dev;
> +	struct component_match *match = NULL;
> +	int ret;
> +
> +	component_match_add(dev, &match, i915_component_master_match, NULL);
> +	ret = component_master_add_with_match(dev, &i915_component_master_ops,
> +					      match);
> +	if (ret < 0)
> +		DRM_ERROR("failed to add component master (%d)\n", ret);
> +	/* continue with reduced functionality */
> +}
> +
> +static void i915_component_master_cleanup(struct drm_i915_private *dev_priv)
> +{
> +	/* the following is ok to call even if component_master_add failed */
> +	component_master_del(dev_priv->dev->dev, &i915_component_master_ops);
> +}
> +
>  /**
>   * i915_driver_load - setup chip and create an initial config
>   * @dev: DRM device
> @@ -830,6 +906,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  
>  	intel_runtime_pm_enable(dev_priv);
>  
> +	i915_component_master_init(dev_priv);
> +
>  	return 0;
>  
>  out_power_well:
> @@ -870,6 +948,8 @@ int i915_driver_unload(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int ret;
>  
> +	i915_component_master_cleanup(dev_priv);
> +
>  	ret = i915_gem_suspend(dev);
>  	if (ret) {
>  		DRM_ERROR("failed to idle hardware: %d\n", ret);
> diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
> new file mode 100644
> index 0000000..44542c2
> --- /dev/null
> +++ b/include/drm/i915_component.h
> @@ -0,0 +1,38 @@
> +/**************************************************************************
> + *
> + * Copyright 2014 Intel Inc.
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + **************************************************************************/
> +
> +#ifndef _I915_COMPONENT_H_
> +#define _I915_COMPONENT_H_
> +
> +struct i915_component_ops {
> +	struct module *owner;
> +	void (*get_power)(struct device *);
> +	void (*put_power)(struct device *);
> +	int (*get_cdclk)(struct device *);
> +};
> +
> +#endif				/* _I915_COMPONENT_H_ */
> -- 
> 1.8.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center