| Message ID | 1437465447-8974-3-git-send-email-david.henningsson@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 21, 2015 at 09:57:25AM +0200, David Henningsson wrote: > On HDMI hotplug events, notify the audio driver. This will enable > the audio driver to get the information at all times (even when > audio is in different powersave states), and also without reading > it from the hardware. > > Signed-off-by: David Henningsson <david.henningsson@canonical.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_audio.c | 46 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 47 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 542fac6..696624c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1808,6 +1808,7 @@ struct drm_i915_private { > struct drm_property *force_audio_property; > > /* hda/i915 audio component */ > + struct i915_audio_component *audio_component; > bool audio_component_registered; > > uint32_t hw_context_size; > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c > index 3da9b84..eb29e1f 100644 > --- a/drivers/gpu/drm/i915/intel_audio.c > +++ b/drivers/gpu/drm/i915/intel_audio.c > @@ -384,6 +384,44 @@ static void ilk_audio_codec_enable(struct drm_connector *connector, > I915_WRITE(aud_config, tmp); > } > > +static void audio_hotplug_notify(struct drm_i915_private *dev_priv, > + struct drm_connector *connector, > + struct intel_encoder *encoder, > + bool plugged_in) plugged_in is redundant, just check for NULL. Also just derive dev_priv from intel_encoder imo. And finally we tend to put the object that a function operates on first. Since this sends a notify out for the given encoder (well dig port really) I'd put that first. I also think that it would make sense to switch all the audio enable/disable functions from intel_encoder to intel_dig_port. A lot need to do upcasts anyway, and audio really only works on dig ports (which is for hdmi/dp only). -Daniel > +{ > + struct i915_audio_component *acomp = dev_priv->audio_component; > + struct i915_audio_hotplug_info audio_info; > + struct intel_digital_port *intel_dig_port = > + enc_to_dig_port(&encoder->base); > + enum port port = intel_dig_port->port; > + > + if (!acomp || !acomp->cb_ops || !acomp->cb_ops->hotplug_notify) > + return; > + > + if (connector) { > + audio_info.connector_type = connector->connector_type; > + audio_info.connector_type_id = connector->connector_type_id; > + } else { > + audio_info.connector_type = -1; > + audio_info.connector_type_id = -1; > + } > + > + audio_info.port = (int) port; > + /* DP Mst is unsupported for now */ > + audio_info.port_multi_stream_device = 0; > + > + audio_info.plugged_in = plugged_in; > + if (plugged_in && connector) { > + audio_info.eld = connector->eld; > + audio_info.eld_size = drm_eld_size(audio_info.eld); > + } else { > + audio_info.eld = NULL; > + audio_info.eld_size = 0; > + } > + > + acomp->cb_ops->hotplug_notify(acomp->hdac_bus, &audio_info); > +} > + > /** > * intel_audio_codec_enable - Enable the audio codec for HD audio > * @intel_encoder: encoder on which to enable audio > @@ -419,6 +457,8 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder) > > if (dev_priv->display.audio_codec_enable) > dev_priv->display.audio_codec_enable(connector, intel_encoder, mode); > + > + audio_hotplug_notify(dev_priv, connector, intel_encoder, true); > } > > /** > @@ -435,6 +475,8 @@ void intel_audio_codec_disable(struct intel_encoder *encoder) > > if (dev_priv->display.audio_codec_disable) > dev_priv->display.audio_codec_disable(encoder); > + > + audio_hotplug_notify(dev_priv, NULL, encoder, false); > } > > /** > @@ -525,12 +567,14 @@ static int i915_audio_component_bind(struct device *i915_dev, > struct device *hda_dev, void *data) > { > struct i915_audio_component *acomp = data; > + struct drm_i915_private *dev_priv = dev_to_i915(i915_dev); > > if (WARN_ON(acomp->ops || acomp->dev)) > return -EEXIST; > > acomp->ops = &i915_audio_component_ops; > acomp->dev = i915_dev; > + dev_priv->audio_component = acomp; > > return 0; > } > @@ -539,9 +583,11 @@ static void i915_audio_component_unbind(struct device *i915_dev, > struct device *hda_dev, void *data) > { > struct i915_audio_component *acomp = data; > + struct drm_i915_private *dev_priv = dev_to_i915(i915_dev); > > acomp->ops = NULL; > acomp->dev = NULL; > + dev_priv->audio_component = NULL; > } > > static const struct component_ops i915_audio_component_bind_ops = { > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 2015-07-21 11:14, Daniel Vetter wrote: > On Tue, Jul 21, 2015 at 09:57:25AM +0200, David Henningsson wrote: >> On HDMI hotplug events, notify the audio driver. This will enable >> the audio driver to get the information at all times (even when >> audio is in different powersave states), and also without reading >> it from the hardware. >> >> Signed-off-by: David Henningsson <david.henningsson@canonical.com> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 1 + >> drivers/gpu/drm/i915/intel_audio.c | 46 ++++++++++++++++++++++++++++++++++++ >> 2 files changed, 47 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index 542fac6..696624c 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -1808,6 +1808,7 @@ struct drm_i915_private { >> struct drm_property *force_audio_property; >> >> /* hda/i915 audio component */ >> + struct i915_audio_component *audio_component; >> bool audio_component_registered; >> >> uint32_t hw_context_size; >> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c >> index 3da9b84..eb29e1f 100644 >> --- a/drivers/gpu/drm/i915/intel_audio.c >> +++ b/drivers/gpu/drm/i915/intel_audio.c >> @@ -384,6 +384,44 @@ static void ilk_audio_codec_enable(struct drm_connector *connector, >> I915_WRITE(aud_config, tmp); >> } >> >> +static void audio_hotplug_notify(struct drm_i915_private *dev_priv, >> + struct drm_connector *connector, >> + struct intel_encoder *encoder, >> + bool plugged_in) > > plugged_in is redundant, just check for NULL. Also just derive dev_priv > from intel_encoder imo. And finally we tend to put the object that a > function operates on first. Since this sends a notify out for the given > encoder (well dig port really) I'd put that first. Sure, will fix these and send out a v2 (after Takashi has reviewed, too). > I also think that it would make sense to switch all the audio > enable/disable functions from intel_encoder to intel_dig_port. A lot need > to do upcasts anyway, and audio really only works on dig ports (which is > for hdmi/dp only). That's up to you - seems a bit out of scope for this patch set though, I assume. Btw, there are two open questions though that you (or someone else) might be able to share some insight on: 1) The mapping between ports and nid is fixed in patch 4/4, it would be good with an Ack from an Intel engineer on this mapping. 2) Whether to use connector_type / connector_type_id combo or connector_name as identification - it seems like X has a different naming compared to the kernel ("HDMI2" in X vs "HDMI-A-2" in kernel), not sure what Wayland/Mir/others do here...
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 542fac6..696624c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1808,6 +1808,7 @@ struct drm_i915_private { struct drm_property *force_audio_property; /* hda/i915 audio component */ + struct i915_audio_component *audio_component; bool audio_component_registered; uint32_t hw_context_size; diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 3da9b84..eb29e1f 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -384,6 +384,44 @@ static void ilk_audio_codec_enable(struct drm_connector *connector, I915_WRITE(aud_config, tmp); } +static void audio_hotplug_notify(struct drm_i915_private *dev_priv, + struct drm_connector *connector, + struct intel_encoder *encoder, + bool plugged_in) +{ + struct i915_audio_component *acomp = dev_priv->audio_component; + struct i915_audio_hotplug_info audio_info; + struct intel_digital_port *intel_dig_port = + enc_to_dig_port(&encoder->base); + enum port port = intel_dig_port->port; + + if (!acomp || !acomp->cb_ops || !acomp->cb_ops->hotplug_notify) + return; + + if (connector) { + audio_info.connector_type = connector->connector_type; + audio_info.connector_type_id = connector->connector_type_id; + } else { + audio_info.connector_type = -1; + audio_info.connector_type_id = -1; + } + + audio_info.port = (int) port; + /* DP Mst is unsupported for now */ + audio_info.port_multi_stream_device = 0; + + audio_info.plugged_in = plugged_in; + if (plugged_in && connector) { + audio_info.eld = connector->eld; + audio_info.eld_size = drm_eld_size(audio_info.eld); + } else { + audio_info.eld = NULL; + audio_info.eld_size = 0; + } + + acomp->cb_ops->hotplug_notify(acomp->hdac_bus, &audio_info); +} + /** * intel_audio_codec_enable - Enable the audio codec for HD audio * @intel_encoder: encoder on which to enable audio @@ -419,6 +457,8 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder) if (dev_priv->display.audio_codec_enable) dev_priv->display.audio_codec_enable(connector, intel_encoder, mode); + + audio_hotplug_notify(dev_priv, connector, intel_encoder, true); } /** @@ -435,6 +475,8 @@ void intel_audio_codec_disable(struct intel_encoder *encoder) if (dev_priv->display.audio_codec_disable) dev_priv->display.audio_codec_disable(encoder); + + audio_hotplug_notify(dev_priv, NULL, encoder, false); } /** @@ -525,12 +567,14 @@ static int i915_audio_component_bind(struct device *i915_dev, struct device *hda_dev, void *data) { struct i915_audio_component *acomp = data; + struct drm_i915_private *dev_priv = dev_to_i915(i915_dev); if (WARN_ON(acomp->ops || acomp->dev)) return -EEXIST; acomp->ops = &i915_audio_component_ops; acomp->dev = i915_dev; + dev_priv->audio_component = acomp; return 0; } @@ -539,9 +583,11 @@ static void i915_audio_component_unbind(struct device *i915_dev, struct device *hda_dev, void *data) { struct i915_audio_component *acomp = data; + struct drm_i915_private *dev_priv = dev_to_i915(i915_dev); acomp->ops = NULL; acomp->dev = NULL; + dev_priv->audio_component = NULL; } static const struct component_ops i915_audio_component_bind_ops = {
On HDMI hotplug events, notify the audio driver. This will enable the audio driver to get the information at all times (even when audio is in different powersave states), and also without reading it from the hardware. Signed-off-by: David Henningsson <david.henningsson@canonical.com> --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_audio.c | 46 ++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+)