On Tue, 09 Dec 2014, Daniel Vetter <daniel@xxxxxxxx> wrote:
> On Tue, Dec 09, 2014 at 11:41:17AM +0200, Imre Deak wrote:
>> Register a component 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.
>>
>> v2:
>> - change roles between the hda and i915 components (Daniel)
>> - add the implementation to a new file (Jani)
>
> I disagree with the name here - intel_component.c is not really
> descriptive since it's not really. Imo it makes much more sense to put
> this into intel_audio.c. After all it's all about how we interact with the
> audio side, which will be even more obvious once we have a dedicated
> subdevice for this.
If we keep this component audio specific, then I guess I agree
intel_audio.c is the better place for it. But that means anything else
(like possibly pmic driver interaction) will need to have a component of
its own.
BR,
Jani.
>
> Also please add a bit of kerneldoc to the _init/_cleanup functions with a
> few words about what this is used for. Or if you want, extract a short
> DOC: section for it even to describe the interactions.
> -Daniel
>
>> - use better namespacing (Jani)
>>
>> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/Makefile | 3 +-
>> drivers/gpu/drm/i915/i915_component.c | 109 ++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/i915_dma.c | 4 ++
>> drivers/gpu/drm/i915/i915_drv.h | 3 +
>> drivers/gpu/drm/i915/intel_drv.h | 4 ++
>> include/drm/i915_component.h | 38 ++++++++++++
>> 6 files changed, 160 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/gpu/drm/i915/i915_component.c
>> create mode 100644 include/drm/i915_component.h
>>
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index e4083e4..6b5b082 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -7,7 +7,8 @@ ccflags-y := -Iinclude/drm
>> # Please keep these build lists sorted!
>>
>> # core driver code
>> -i915-y := i915_drv.o \
>> +i915-y := i915_component.o \
>> + i915_drv.o \
>> i915_params.o \
>> i915_suspend.o \
>> i915_sysfs.o \
>> diff --git a/drivers/gpu/drm/i915/i915_component.c b/drivers/gpu/drm/i915/i915_component.c
>> new file mode 100644
>> index 0000000..4bb49f0
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/i915_component.c
>> @@ -0,0 +1,109 @@
>> +/*
>> + * Copyright © 2014 Intel Corporation
>> + *
>> + * 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, sublicense,
>> + * 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 NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS 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.
>> + */
>> +
>> +#include <linux/component.h>
>> +#include <drm/i915_component.h>
>> +#include "intel_drv.h"
>> +
>> +static void display_component_get_power(struct device *dev)
>> +{
>> + intel_display_power_get(dev_to_i915(dev), POWER_DOMAIN_AUDIO);
>> +}
>> +
>> +static void display_component_put_power(struct device *dev)
>> +{
>> + intel_display_power_put(dev_to_i915(dev), POWER_DOMAIN_AUDIO);
>> +}
>> +
>> +/* Get CDCLK in kHz */
>> +static int display_component_get_cdclk_freq(struct device *dev)
>> +{
>> + struct drm_i915_private *dev_priv = dev_to_i915(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 const struct i915_display_component_ops display_component_ops = {
>> + .owner = THIS_MODULE,
>> + .get_power = display_component_get_power,
>> + .put_power = display_component_put_power,
>> + .get_cdclk_freq = display_component_get_cdclk_freq,
>> +};
>> +
>> +static int display_component_bind(struct device *i915_dev,
>> + struct device *hda_dev, void *data)
>> +{
>> + struct i915_display_component *dcomp = data;
>> +
>> + if (WARN_ON(dcomp->ops || dcomp->dev))
>> + return -EEXIST;
>> +
>> + dcomp->ops = &display_component_ops;
>> + dcomp->dev = i915_dev;
>> +
>> + return 0;
>> +}
>> +
>> +static void display_component_unbind(struct device *i915_dev,
>> + struct device *hda_dev, void *data)
>> +{
>> + struct i915_display_component *dcomp = data;
>> +
>> + dcomp->ops = NULL;
>> + dcomp->dev = NULL;
>> +}
>> +
>> +static const struct component_ops display_component_bind_ops = {
>> + .bind = display_component_bind,
>> + .unbind = display_component_unbind,
>> +};
>> +
>> +void i915_component_init(struct drm_i915_private *dev_priv)
>> +{
>> + int ret;
>> +
>> + ret = component_add(dev_priv->dev->dev, &display_component_bind_ops);
>> + if (ret < 0) {
>> + DRM_ERROR("failed to add display component (%d)\n", ret);
>> + /* continue with reduced functionality */
>> + return;
>> + }
>> +
>> + dev_priv->display_component_registered = true;
>> +}
>> +
>> +void i915_component_cleanup(struct drm_i915_private *dev_priv)
>> +{
>> + if (dev_priv->display_component_registered) {
>> + component_del(dev_priv->dev->dev, &display_component_bind_ops);
>> + dev_priv->display_component_registered = false;
>> + }
>> +}
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index 887d88f..b6238bb 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -830,6 +830,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>>
>> intel_runtime_pm_enable(dev_priv);
>>
>> + i915_component_init(dev_priv);
>> +
>> return 0;
>>
>> out_power_well:
>> @@ -870,6 +872,8 @@ int i915_driver_unload(struct drm_device *dev)
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> int ret;
>>
>> + i915_component_cleanup(dev_priv);
>> +
>> ret = i915_gem_suspend(dev);
>> if (ret) {
>> DRM_ERROR("failed to idle hardware: %d\n", ret);
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index fb2616d..3b7ae14 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1684,6 +1684,9 @@ struct drm_i915_private {
>>
>> bool mchbar_need_disable;
>>
>> + /* hda/i915 display component */
>> + bool display_component_registered;
>> +
>> struct intel_l3_parity l3_parity;
>>
>> /* Cannot be determined by PCIID. You must always read a register. */
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 61a88fa..856709e 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -809,6 +809,10 @@ static inline bool intel_irqs_enabled(struct drm_i915_private *dev_priv)
>> int intel_get_crtc_scanline(struct intel_crtc *crtc);
>> void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv);
>>
>> +/* i915_component.c */
>> +void i915_component_init(struct drm_i915_private *dev_priv);
>> +void i915_component_cleanup(struct drm_i915_private *dev_priv);
>> +
>> /* intel_crt.c */
>> void intel_crt_init(struct drm_device *dev);
>>
>> diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
>> new file mode 100644
>> index 0000000..9c660ca
>> --- /dev/null
>> +++ b/include/drm/i915_component.h
>> @@ -0,0 +1,38 @@
>> +/*
>> + * Copyright © 2014 Intel Corporation
>> + *
>> + * 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, sublicense,
>> + * 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 NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS 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_display_component {
>> + struct device *dev;
>> +
>> + const struct i915_display_component_ops {
>> + struct module *owner;
>> + void (*get_power)(struct device *);
>> + void (*put_power)(struct device *);
>> + int (*get_cdclk_freq)(struct device *);
>> + } *ops;
>> +};
>> +
>> +#endif /* _I915_COMPONENT_H_ */
>> --
>> 1.8.4
>>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
