diff mbox

[3/4] ALSA: hda - Dispatch incoming HDMI hotplug i915 callback

Message ID 1437465447-8974-4-git-send-email-david.henningsson@canonical.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Henningsson July 21, 2015, 7:57 a.m. UTC
This lets interested codec(s) be notified of HDMI hotplug
events sent from the i915 driver.

Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 include/sound/hdaudio.h |    4 ++++
 sound/hda/hdac_i915.c   |   24 ++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

Comments

Takashi Iwai July 22, 2015, 8:30 a.m. UTC | #1
On Tue, 21 Jul 2015 09:57:26 +0200,
David Henningsson wrote:
> 
> This lets interested codec(s) be notified of HDMI hotplug
> events sent from the i915 driver.
> 
> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
> ---
>  include/sound/hdaudio.h |    4 ++++
>  sound/hda/hdac_i915.c   |   24 ++++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
> index 4caf1fd..ce34182 100644
> --- a/include/sound/hdaudio.h
> +++ b/include/sound/hdaudio.h
> @@ -79,6 +79,10 @@ struct hdac_device {
>  	int (*exec_verb)(struct hdac_device *dev, unsigned int cmd,
>  			 unsigned int flags, unsigned int *res);
>  
> +	/* Used for hotplug notification from i915 driver */
> +	void (*hotplug_notify)(struct hdac_device *,
> +			       const struct i915_audio_hotplug_info *);

Since this callback is specific to HDMI/DP, a more specific name would
be better.  Otherwise this sounds like a generic hotplug handler.

Or, we may make it really generic, e.g. something like
	void (*hotplug_notify)(struct hdac_device *, int event_type,
				const void *data);

and call it with a specific event_type value
	codec->hotplug_notify(codec, HDAC_NOTIFY_I915_DP, hotplug_info);


thanks,

Takashi
Vinod Koul July 22, 2015, 1:56 p.m. UTC | #2
On Wed, Jul 22, 2015 at 10:30:57AM +0200, Takashi Iwai wrote:
> On Tue, 21 Jul 2015 09:57:26 +0200,
> David Henningsson wrote:
> > 
> > This lets interested codec(s) be notified of HDMI hotplug
> > events sent from the i915 driver.
> > 
> > Signed-off-by: David Henningsson <david.henningsson@canonical.com>
> > ---
> >  include/sound/hdaudio.h |    4 ++++
> >  sound/hda/hdac_i915.c   |   24 ++++++++++++++++++++++++
> >  2 files changed, 28 insertions(+)
> > 
> > diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
> > index 4caf1fd..ce34182 100644
> > --- a/include/sound/hdaudio.h
> > +++ b/include/sound/hdaudio.h
> > @@ -79,6 +79,10 @@ struct hdac_device {
> >  	int (*exec_verb)(struct hdac_device *dev, unsigned int cmd,
> >  			 unsigned int flags, unsigned int *res);
> >  
> > +	/* Used for hotplug notification from i915 driver */
> > +	void (*hotplug_notify)(struct hdac_device *,
> > +			       const struct i915_audio_hotplug_info *);
> 
> Since this callback is specific to HDMI/DP, a more specific name would
> be better.  Otherwise this sounds like a generic hotplug handler.
> 
> Or, we may make it really generic, e.g. something like
> 	void (*hotplug_notify)(struct hdac_device *, int event_type,
> 				const void *data);
> 
> and call it with a specific event_type value
> 	codec->hotplug_notify(codec, HDAC_NOTIFY_I915_DP, hotplug_info);
or should this be added to unsol_event handler for the device, we already
are have worker thread for that.
For device it will be single notify for both SW and HW events. Yes adding
event types will help
Takashi Iwai July 22, 2015, 2:01 p.m. UTC | #3
On Wed, 22 Jul 2015 15:56:37 +0200,
Vinod Koul wrote:
> 
> On Wed, Jul 22, 2015 at 10:30:57AM +0200, Takashi Iwai wrote:
> > On Tue, 21 Jul 2015 09:57:26 +0200,
> > David Henningsson wrote:
> > > 
> > > This lets interested codec(s) be notified of HDMI hotplug
> > > events sent from the i915 driver.
> > > 
> > > Signed-off-by: David Henningsson <david.henningsson@canonical.com>
> > > ---
> > >  include/sound/hdaudio.h |    4 ++++
> > >  sound/hda/hdac_i915.c   |   24 ++++++++++++++++++++++++
> > >  2 files changed, 28 insertions(+)
> > > 
> > > diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
> > > index 4caf1fd..ce34182 100644
> > > --- a/include/sound/hdaudio.h
> > > +++ b/include/sound/hdaudio.h
> > > @@ -79,6 +79,10 @@ struct hdac_device {
> > >  	int (*exec_verb)(struct hdac_device *dev, unsigned int cmd,
> > >  			 unsigned int flags, unsigned int *res);
> > >  
> > > +	/* Used for hotplug notification from i915 driver */
> > > +	void (*hotplug_notify)(struct hdac_device *,
> > > +			       const struct i915_audio_hotplug_info *);
> > 
> > Since this callback is specific to HDMI/DP, a more specific name would
> > be better.  Otherwise this sounds like a generic hotplug handler.
> > 
> > Or, we may make it really generic, e.g. something like
> > 	void (*hotplug_notify)(struct hdac_device *, int event_type,
> > 				const void *data);
> > 
> > and call it with a specific event_type value
> > 	codec->hotplug_notify(codec, HDAC_NOTIFY_I915_DP, hotplug_info);
> or should this be added to unsol_event handler for the device, we already
> are have worker thread for that.
> For device it will be single notify for both SW and HW events. Yes adding
> event types will help

Well, the difference from the unsol event is that this gives the
certain amount of data together with the notification.  Of course, it
can be a two-step procedure: first notify via a faked unsol event,
then the driver fetches data actively.


Takashi
diff mbox

Patch

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index 4caf1fd..ce34182 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -79,6 +79,10 @@  struct hdac_device {
 	int (*exec_verb)(struct hdac_device *dev, unsigned int cmd,
 			 unsigned int flags, unsigned int *res);
 
+	/* Used for hotplug notification from i915 driver */
+	void (*hotplug_notify)(struct hdac_device *,
+			       const struct i915_audio_hotplug_info *);
+
 	/* widgets */
 	unsigned int num_nodes;
 	hda_nid_t start_nid, end_nid;
diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
index 5676b84..134ef9c 100644
--- a/sound/hda/hdac_i915.c
+++ b/sound/hda/hdac_i915.c
@@ -118,6 +118,8 @@  static void hdac_component_master_unbind(struct device *dev)
 {
 	struct i915_audio_component *acomp = hdac_acomp;
 
+	acomp->cb_ops = NULL;
+	acomp->hdac_bus = NULL;
 	module_put(acomp->ops->owner);
 	component_unbind_all(dev, acomp);
 	WARN_ON(acomp->ops || acomp->dev);
@@ -128,6 +130,25 @@  static const struct component_master_ops hdac_component_master_ops = {
 	.unbind = hdac_component_master_unbind,
 };
 
+static void i915_audio_component_hotplug_notify(struct hdac_bus *bus,
+	const struct i915_audio_hotplug_info *info)
+{
+	struct hdac_device *d;
+
+	dev_dbg(bus->dev, "Received HDMI hotplug callback (connector = %d-%d, plugged in = %d)",
+		info->connector_type, info->connector_type_id, (int) info->plugged_in);
+
+	list_for_each_entry(d, &bus->codec_list, list) {
+		if (d->hotplug_notify)
+			d->hotplug_notify(d, info);
+	}
+}
+
+static const struct i915_audio_component_cb_ops i915_audio_component_cb_ops = {
+	.owner          = THIS_MODULE,
+	.hotplug_notify = i915_audio_component_hotplug_notify,
+};
+
 static int hdac_component_master_match(struct device *dev, void *data)
 {
 	/* i915 is the only supported component */
@@ -163,6 +184,9 @@  int snd_hdac_i915_init(struct hdac_bus *bus)
 		ret = -ENODEV;
 		goto out_master_del;
 	}
+	acomp->cb_ops = &i915_audio_component_cb_ops;
+	acomp->hdac_bus = bus;
+
 	dev_dbg(dev, "bound to i915 component master\n");
 
 	return 0;