[2/2] drm/nouveau: Add HD-audio component notifier support

Submitted by Takashi Iwai on July 22, 2019, 2:38 p.m.

Details

Message ID 20190722143815.7339-3-tiwai@suse.de
State New
Headers show
Series "Audio component support for radeon and nouveau" ( rev: 1 ) in DRI devel

Not browsing as part of any series.

Commit Message

Takashi Iwai July 22, 2019, 2:38 p.m.
This patch adds the support for the notification of HD-audio hotplug
via the already existing drm_audio_component framework.  This allows
us more reliable hotplug notification and ELD transfer without
accessing HD-audio bus; it's more efficient, and more importantly, it
works without waking up the runtime PM.

The implementation is rather simplistic: nouveau driver provides the
get_eld ops for HD-audio, and it notifies the audio hotplug via
pin_eld_notify callback upon each nv50_audio_enable() and _disable()
call.  As the HD-audio pin assignment seems corresponding to the CRTC,
the crtc->index number is passed directly as the zero-based port
number.

The bind and unbind callbacks handle the device-link so that it
assures the PM call order.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/nouveau/Kconfig         |   1 +
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 111 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/nouveau/nouveau_drv.h   |   7 ++
 3 files changed, 119 insertions(+)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index 96b9814e6d06..33ccf11bd70d 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -16,6 +16,7 @@  config DRM_NOUVEAU
 	select INPUT if ACPI && X86
 	select THERMAL if ACPI && X86
 	select ACPI_VIDEO if ACPI && X86
+	select SND_HDA_COMPONENT if SND_HDA_CORE
 	help
 	  Choose this option for open-source NVIDIA support.
 
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 8497768f1b41..919f3d3db161 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -29,6 +29,7 @@ 
 
 #include <linux/dma-mapping.h>
 #include <linux/hdmi.h>
+#include <linux/component.h>
 
 #include <drm/drmP.h>
 #include <drm/drm_atomic_helper.h>
@@ -466,12 +467,113 @@  nv50_dac_create(struct drm_connector *connector, struct dcb_output *dcbe)
 	return 0;
 }
 
+/*
+ * audio component binding for ELD notification
+ */
+static void
+nv50_audio_component_eld_notify(struct drm_audio_component *acomp, int port)
+{
+	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
+		acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
+						 port, -1);
+}
+
+static int
+nv50_audio_component_get_eld(struct device *kdev, int port, int pipe,
+			     bool *enabled, unsigned char *buf, int max_bytes)
+{
+	struct drm_device *drm_dev = dev_get_drvdata(kdev);
+	struct nouveau_drm *drm = nouveau_drm(drm_dev);
+	struct drm_encoder *encoder;
+	struct nouveau_encoder *nv_encoder;
+	struct nouveau_connector *nv_connector;
+	struct nouveau_crtc *nv_crtc;
+	int ret = 0;
+
+	*enabled = false;
+	drm_for_each_encoder(encoder, drm->dev) {
+		nv_encoder = nouveau_encoder(encoder);
+		nv_connector = nouveau_encoder_connector_get(nv_encoder);
+		nv_crtc = nouveau_crtc(encoder->crtc);
+		if (!nv_connector || !nv_crtc || nv_crtc->index != port)
+			continue;
+		*enabled = drm_detect_monitor_audio(nv_connector->edid);
+		if (*enabled) {
+			ret = drm_eld_size(nv_connector->base.eld);
+			memcpy(buf, nv_connector->base.eld,
+			       min(max_bytes, ret));
+		}
+		break;
+	}
+	return ret;
+}
+
+static const struct drm_audio_component_ops nv50_audio_component_ops = {
+	.get_eld = nv50_audio_component_get_eld,
+};
+
+static int
+nv50_audio_component_bind(struct device *kdev, struct device *hda_kdev,
+			  void *data)
+{
+	struct drm_device *drm_dev = dev_get_drvdata(kdev);
+	struct nouveau_drm *drm = nouveau_drm(drm_dev);
+	struct drm_audio_component *acomp = data;
+
+	if (WARN_ON(!device_link_add(hda_kdev, kdev, DL_FLAG_STATELESS)))
+		return -ENOMEM;
+
+	drm_modeset_lock_all(drm_dev);
+	acomp->ops = &nv50_audio_component_ops;
+	acomp->dev = kdev;
+	drm->audio.component = acomp;
+	drm_modeset_unlock_all(drm_dev);
+	return 0;
+}
+
+static void
+nv50_audio_component_unbind(struct device *kdev, struct device *hda_kdev,
+			    void *data)
+{
+	struct drm_device *drm_dev = dev_get_drvdata(kdev);
+	struct nouveau_drm *drm = nouveau_drm(drm_dev);
+	struct drm_audio_component *acomp = data;
+
+	drm_modeset_lock_all(drm_dev);
+	drm->audio.component = NULL;
+	acomp->ops = NULL;
+	acomp->dev = NULL;
+	drm_modeset_unlock_all(drm_dev);
+}
+
+static const struct component_ops nv50_audio_component_bind_ops = {
+	.bind   = nv50_audio_component_bind,
+	.unbind = nv50_audio_component_unbind,
+};
+
+static void
+nv50_audio_component_init(struct nouveau_drm *drm)
+{
+	if (!component_add(drm->dev->dev, &nv50_audio_component_bind_ops))
+		drm->audio.component_registered = true;
+}
+
+static void
+nv50_audio_component_fini(struct nouveau_drm *drm)
+{
+	if (drm->audio.component_registered) {
+		component_del(drm->dev->dev, &nv50_audio_component_bind_ops);
+		drm->audio.component_registered = false;
+	}
+}
+
 /******************************************************************************
  * Audio
  *****************************************************************************/
 static void
 nv50_audio_disable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc)
 {
+	struct nouveau_drm *drm = nouveau_drm(encoder->dev);
 	struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
 	struct nv50_disp *disp = nv50_disp(encoder->dev);
 	struct {
@@ -486,11 +588,14 @@  nv50_audio_disable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc)
 	};
 
 	nvif_mthd(&disp->disp->object, 0, &args, sizeof(args));
+
+	nv50_audio_component_eld_notify(drm->audio.component, nv_crtc->index);
 }
 
 static void
 nv50_audio_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
 {
+	struct nouveau_drm *drm = nouveau_drm(encoder->dev);
 	struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
 	struct nouveau_crtc *nv_crtc = nouveau_crtc(encoder->crtc);
 	struct nouveau_connector *nv_connector;
@@ -517,6 +622,8 @@  nv50_audio_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
 
 	nvif_mthd(&disp->disp->object, 0, &args,
 		  sizeof(args.base) + drm_eld_size(args.data));
+
+	nv50_audio_component_eld_notify(drm->audio.component, nv_crtc->index);
 }
 
 /******************************************************************************
@@ -2281,6 +2388,8 @@  nv50_display_destroy(struct drm_device *dev)
 {
 	struct nv50_disp *disp = nv50_disp(dev);
 
+	nv50_audio_component_fini(nouveau_drm(dev));
+
 	nv50_core_del(&disp->core);
 
 	nouveau_bo_unmap(disp->sync);
@@ -2401,6 +2510,8 @@  nv50_display_create(struct drm_device *dev)
 	/* Disable vblank irqs aggressively for power-saving, safe on nv50+ */
 	dev->vblank_disable_immediate = true;
 
+	nv50_audio_component_init(drm);
+
 out:
 	if (ret)
 		nv50_display_destroy(dev);
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h
index aae035816383..15e4f2aa19bf 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -55,6 +55,8 @@ 
 #include <drm/ttm/ttm_module.h>
 #include <drm/ttm/ttm_page_alloc.h>
 
+#include <drm/drm_audio_component.h>
+
 #include "uapi/drm/nouveau_drm.h"
 
 struct nouveau_channel;
@@ -212,6 +214,11 @@  struct nouveau_drm {
 	struct nouveau_svm *svm;
 
 	struct nouveau_dmem *dmem;
+
+	struct {
+		struct drm_audio_component *component;
+		bool component_registered;
+	} audio;
 };
 
 static inline struct nouveau_drm *

Comments

On Mon, Jul 22, 2019 at 10:38 AM Takashi Iwai <tiwai@suse.de> wrote:
> +static void
> +nv50_audio_component_init(struct nouveau_drm *drm)
> +{
> +       if (!component_add(drm->dev->dev, &nv50_audio_component_bind_ops))
> +               drm->audio.component_registered = true;
> +}

Pardon my ignorance on this topic ... but are there any ill effects
from always doing this? For example, the board in question might not
have ELD support at all (the original G80, covered by dispnv50), or it
might not have any HDMI/DP ports, or it may have them but just nothing
is plugged in.

Could you confirm that this is all OK?

Thanks,

  -ilia
On Mon, 22 Jul 2019 17:00:15 +0200,
Ilia Mirkin wrote:
> 
> On Mon, Jul 22, 2019 at 10:38 AM Takashi Iwai <tiwai@suse.de> wrote:
> > +static void
> > +nv50_audio_component_init(struct nouveau_drm *drm)
> > +{
> > +       if (!component_add(drm->dev->dev, &nv50_audio_component_bind_ops))
> > +               drm->audio.component_registered = true;
> > +}
> 
> Pardon my ignorance on this topic ... but are there any ill effects
> from always doing this? For example, the board in question might not
> have ELD support at all (the original G80, covered by dispnv50), or it
> might not have any HDMI/DP ports, or it may have them but just nothing
> is plugged in.

In general, this change shouldn't do anything worse than what we have
currently.

If the board has no ELD support, the HDMI audio won't work at all, and
HD-audio side would know that it's an invalid ELD.

If the nvidia chips has no audio support and it doesn't have the
corresponding HD-audio audio driver exposed as a PCI device, this
component registration is also harmless, just a placeholder until it's
bound.

> Could you confirm that this is all OK?

At least I tested other way round -- the nvidia component implemented
but without HD-audio master component.


Thanks!

Takashi

> 
> Thanks,
> 
>   -ilia
>
On Mon, 22 Jul 2019 16:38:15 +0200,
Takashi Iwai wrote:
> 
> This patch adds the support for the notification of HD-audio hotplug
> via the already existing drm_audio_component framework.  This allows
> us more reliable hotplug notification and ELD transfer without
> accessing HD-audio bus; it's more efficient, and more importantly, it
> works without waking up the runtime PM.
> 
> The implementation is rather simplistic: nouveau driver provides the
> get_eld ops for HD-audio, and it notifies the audio hotplug via
> pin_eld_notify callback upon each nv50_audio_enable() and _disable()
> call.  As the HD-audio pin assignment seems corresponding to the CRTC,
> the crtc->index number is passed directly as the zero-based port
> number.
> 
> The bind and unbind callbacks handle the device-link so that it
> assures the PM call order.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

Any review / reaction to this patch?

I don't mind dropping the patch for radeon, but nouveau has more
potential benefit by this.


thanks,

Takashi

> ---
>  drivers/gpu/drm/nouveau/Kconfig         |   1 +
>  drivers/gpu/drm/nouveau/dispnv50/disp.c | 111 ++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/nouveau/nouveau_drv.h   |   7 ++
>  3 files changed, 119 insertions(+)
> 
> diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
> index 96b9814e6d06..33ccf11bd70d 100644
> --- a/drivers/gpu/drm/nouveau/Kconfig
> +++ b/drivers/gpu/drm/nouveau/Kconfig
> @@ -16,6 +16,7 @@ config DRM_NOUVEAU
>  	select INPUT if ACPI && X86
>  	select THERMAL if ACPI && X86
>  	select ACPI_VIDEO if ACPI && X86
> +	select SND_HDA_COMPONENT if SND_HDA_CORE
>  	help
>  	  Choose this option for open-source NVIDIA support.
>  
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 8497768f1b41..919f3d3db161 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -29,6 +29,7 @@
>  
>  #include <linux/dma-mapping.h>
>  #include <linux/hdmi.h>
> +#include <linux/component.h>
>  
>  #include <drm/drmP.h>
>  #include <drm/drm_atomic_helper.h>
> @@ -466,12 +467,113 @@ nv50_dac_create(struct drm_connector *connector, struct dcb_output *dcbe)
>  	return 0;
>  }
>  
> +/*
> + * audio component binding for ELD notification
> + */
> +static void
> +nv50_audio_component_eld_notify(struct drm_audio_component *acomp, int port)
> +{
> +	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
> +		acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
> +						 port, -1);
> +}
> +
> +static int
> +nv50_audio_component_get_eld(struct device *kdev, int port, int pipe,
> +			     bool *enabled, unsigned char *buf, int max_bytes)
> +{
> +	struct drm_device *drm_dev = dev_get_drvdata(kdev);
> +	struct nouveau_drm *drm = nouveau_drm(drm_dev);
> +	struct drm_encoder *encoder;
> +	struct nouveau_encoder *nv_encoder;
> +	struct nouveau_connector *nv_connector;
> +	struct nouveau_crtc *nv_crtc;
> +	int ret = 0;
> +
> +	*enabled = false;
> +	drm_for_each_encoder(encoder, drm->dev) {
> +		nv_encoder = nouveau_encoder(encoder);
> +		nv_connector = nouveau_encoder_connector_get(nv_encoder);
> +		nv_crtc = nouveau_crtc(encoder->crtc);
> +		if (!nv_connector || !nv_crtc || nv_crtc->index != port)
> +			continue;
> +		*enabled = drm_detect_monitor_audio(nv_connector->edid);
> +		if (*enabled) {
> +			ret = drm_eld_size(nv_connector->base.eld);
> +			memcpy(buf, nv_connector->base.eld,
> +			       min(max_bytes, ret));
> +		}
> +		break;
> +	}
> +	return ret;
> +}
> +
> +static const struct drm_audio_component_ops nv50_audio_component_ops = {
> +	.get_eld = nv50_audio_component_get_eld,
> +};
> +
> +static int
> +nv50_audio_component_bind(struct device *kdev, struct device *hda_kdev,
> +			  void *data)
> +{
> +	struct drm_device *drm_dev = dev_get_drvdata(kdev);
> +	struct nouveau_drm *drm = nouveau_drm(drm_dev);
> +	struct drm_audio_component *acomp = data;
> +
> +	if (WARN_ON(!device_link_add(hda_kdev, kdev, DL_FLAG_STATELESS)))
> +		return -ENOMEM;
> +
> +	drm_modeset_lock_all(drm_dev);
> +	acomp->ops = &nv50_audio_component_ops;
> +	acomp->dev = kdev;
> +	drm->audio.component = acomp;
> +	drm_modeset_unlock_all(drm_dev);
> +	return 0;
> +}
> +
> +static void
> +nv50_audio_component_unbind(struct device *kdev, struct device *hda_kdev,
> +			    void *data)
> +{
> +	struct drm_device *drm_dev = dev_get_drvdata(kdev);
> +	struct nouveau_drm *drm = nouveau_drm(drm_dev);
> +	struct drm_audio_component *acomp = data;
> +
> +	drm_modeset_lock_all(drm_dev);
> +	drm->audio.component = NULL;
> +	acomp->ops = NULL;
> +	acomp->dev = NULL;
> +	drm_modeset_unlock_all(drm_dev);
> +}
> +
> +static const struct component_ops nv50_audio_component_bind_ops = {
> +	.bind   = nv50_audio_component_bind,
> +	.unbind = nv50_audio_component_unbind,
> +};
> +
> +static void
> +nv50_audio_component_init(struct nouveau_drm *drm)
> +{
> +	if (!component_add(drm->dev->dev, &nv50_audio_component_bind_ops))
> +		drm->audio.component_registered = true;
> +}
> +
> +static void
> +nv50_audio_component_fini(struct nouveau_drm *drm)
> +{
> +	if (drm->audio.component_registered) {
> +		component_del(drm->dev->dev, &nv50_audio_component_bind_ops);
> +		drm->audio.component_registered = false;
> +	}
> +}
> +
>  /******************************************************************************
>   * Audio
>   *****************************************************************************/
>  static void
>  nv50_audio_disable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc)
>  {
> +	struct nouveau_drm *drm = nouveau_drm(encoder->dev);
>  	struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
>  	struct nv50_disp *disp = nv50_disp(encoder->dev);
>  	struct {
> @@ -486,11 +588,14 @@ nv50_audio_disable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc)
>  	};
>  
>  	nvif_mthd(&disp->disp->object, 0, &args, sizeof(args));
> +
> +	nv50_audio_component_eld_notify(drm->audio.component, nv_crtc->index);
>  }
>  
>  static void
>  nv50_audio_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
>  {
> +	struct nouveau_drm *drm = nouveau_drm(encoder->dev);
>  	struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
>  	struct nouveau_crtc *nv_crtc = nouveau_crtc(encoder->crtc);
>  	struct nouveau_connector *nv_connector;
> @@ -517,6 +622,8 @@ nv50_audio_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
>  
>  	nvif_mthd(&disp->disp->object, 0, &args,
>  		  sizeof(args.base) + drm_eld_size(args.data));
> +
> +	nv50_audio_component_eld_notify(drm->audio.component, nv_crtc->index);
>  }
>  
>  /******************************************************************************
> @@ -2281,6 +2388,8 @@ nv50_display_destroy(struct drm_device *dev)
>  {
>  	struct nv50_disp *disp = nv50_disp(dev);
>  
> +	nv50_audio_component_fini(nouveau_drm(dev));
> +
>  	nv50_core_del(&disp->core);
>  
>  	nouveau_bo_unmap(disp->sync);
> @@ -2401,6 +2510,8 @@ nv50_display_create(struct drm_device *dev)
>  	/* Disable vblank irqs aggressively for power-saving, safe on nv50+ */
>  	dev->vblank_disable_immediate = true;
>  
> +	nv50_audio_component_init(drm);
> +
>  out:
>  	if (ret)
>  		nv50_display_destroy(dev);
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h
> index aae035816383..15e4f2aa19bf 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drv.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
> @@ -55,6 +55,8 @@
>  #include <drm/ttm/ttm_module.h>
>  #include <drm/ttm/ttm_page_alloc.h>
>  
> +#include <drm/drm_audio_component.h>
> +
>  #include "uapi/drm/nouveau_drm.h"
>  
>  struct nouveau_channel;
> @@ -212,6 +214,11 @@ struct nouveau_drm {
>  	struct nouveau_svm *svm;
>  
>  	struct nouveau_dmem *dmem;
> +
> +	struct {
> +		struct drm_audio_component *component;
> +		bool component_registered;
> +	} audio;
>  };
>  
>  static inline struct nouveau_drm *
> -- 
> 2.16.4
>