All of lore.kernel.org
 help / color / mirror / Atom feed
* [V3] vga_switcheroo: set audio client id according to bound GPU id
@ 2018-07-17  8:20 Jim Qu
  2018-07-17  8:38 ` Lukas Wunner
  0 siblings, 1 reply; 5+ messages in thread
From: Jim Qu @ 2018-07-17  8:20 UTC (permalink / raw)
  To: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: tiwai-IBi9RG/b67k, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	Jim Qu, lukas-JFq808J9C/izQB+pC5nmwQ, daniel-/w4YWyX8dFk,
	alexander.deucher-5C7GfCeVMHo

On modern laptop, there are more and more platforms
have two GPUs, and each of them maybe have audio codec
for HDMP/DP output. For some dGPU which is no output,
audio codec usually is disabled.

In currect HDA audio driver, it will set all codec as
VGA_SWITCHEROO_DIS, the audio which is binded to UMA
will be suspended if user use debugfs to contorl power

In HDA driver side, it is difficult to know which GPU
the audio has binded to. So set the bound gpu pci dev
to vga_switcheroo.

if the audio client is not the third registration, audio
id will set in vga_switcheroo enable function. if the
audio client is the last registration when vga_switcheroo
_ready() get true, we should get audio client id from bound
GPU directly.

Signed-off-by: Jim Qu <Jim.Qu@amd.com>
---
 drivers/gpu/vga/vga_switcheroo.c | 63 +++++++++++++++++++++++++++++++++-------
 include/linux/vga_switcheroo.h   |  8 ++---
 sound/pci/hda/hda_intel.c        | 11 +++----
 3 files changed, 62 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index fc4adf3..af04e3c 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -103,9 +103,11 @@
  *	runtime pm. If true, writing ON and OFF to the vga_switcheroo debugfs
  *	interface is a no-op so as not to interfere with runtime pm
  * @list: client list
+ * @vga_dev: pci device, indicate which GPU is bound to current audio client
  *
  * Registered client. A client can be either a GPU or an audio device on a GPU.
- * For audio clients, the @fb_info and @active members are bogus.
+ * For audio clients, the @fb_info and @active members are bogus. For GPU
+ * clients, the @vga_dev is bogus.
  */
 struct vga_switcheroo_client {
 	struct pci_dev *pdev;
@@ -116,6 +118,7 @@ struct vga_switcheroo_client {
 	bool active;
 	bool driver_power_control;
 	struct list_head list;
+	struct pci_dev *vga_dev;
 };
 
 /*
@@ -161,9 +164,8 @@ struct vgasr_priv {
 };
 
 #define ID_BIT_AUDIO		0x100
-#define client_is_audio(c)	((c)->id & ID_BIT_AUDIO)
-#define client_is_vga(c)	((c)->id == VGA_SWITCHEROO_UNKNOWN_ID || \
-				 !client_is_audio(c))
+#define client_is_audio(c)		((c)->id & ID_BIT_AUDIO)
+#define client_is_vga(c)		(!client_is_audio(c))
 #define client_id(c)		((c)->id & ~ID_BIT_AUDIO)
 
 static int vga_switcheroo_debugfs_init(struct vgasr_priv *priv);
@@ -192,14 +194,29 @@ static void vga_switcheroo_enable(void)
 		vgasr_priv.handler->init();
 
 	list_for_each_entry(client, &vgasr_priv.clients, list) {
-		if (client->id != VGA_SWITCHEROO_UNKNOWN_ID)
+		if (!client_is_vga(client) ||
+		     client_id(client) != VGA_SWITCHEROO_UNKNOWN_ID)
 			continue;
+
 		ret = vgasr_priv.handler->get_client_id(client->pdev);
 		if (ret < 0)
 			return;
 
 		client->id = ret;
 	}
+
+	list_for_each_entry(client, &vgasr_priv.clients, list) {
+		if (!client_is_audio(client) ||
+		     client_id(client) != VGA_SWITCHEROO_UNKNOWN_ID)
+			continue;
+
+		ret = vgasr_priv.handler->get_client_id(client->vga_dev);
+		if (ret < 0)
+			return;
+
+		client->id = ret | ID_BIT_AUDIO;
+	}
+
 	vga_switcheroo_debugfs_init(&vgasr_priv);
 	vgasr_priv.active = true;
 }
@@ -272,7 +289,9 @@ EXPORT_SYMBOL(vga_switcheroo_handler_flags);
 
 static int register_client(struct pci_dev *pdev,
 			   const struct vga_switcheroo_client_ops *ops,
-			   enum vga_switcheroo_client_id id, bool active,
+			   enum vga_switcheroo_client_id id,
+			   struct pci_dev *vga_dev,
+			   bool active,
 			   bool driver_power_control)
 {
 	struct vga_switcheroo_client *client;
@@ -287,6 +306,7 @@ static int register_client(struct pci_dev *pdev,
 	client->id = id;
 	client->active = active;
 	client->driver_power_control = driver_power_control;
+	client->vga_dev = vga_dev;
 
 	mutex_lock(&vgasr_mutex);
 	list_add_tail(&client->list, &vgasr_priv.clients);
@@ -319,7 +339,7 @@ int vga_switcheroo_register_client(struct pci_dev *pdev,
 				   const struct vga_switcheroo_client_ops *ops,
 				   bool driver_power_control)
 {
-	return register_client(pdev, ops, VGA_SWITCHEROO_UNKNOWN_ID,
+	return register_client(pdev, ops, VGA_SWITCHEROO_UNKNOWN_ID, NULL,
 			       pdev == vga_default_device(),
 			       driver_power_control);
 }
@@ -329,19 +349,40 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
  * vga_switcheroo_register_audio_client - register audio client
  * @pdev: client pci device
  * @ops: client callbacks
- * @id: client identifier
+ * @vga_dev:  pci device which is bound to current audio client
  *
  * Register audio client (audio device on a GPU). The client is assumed
  * to use runtime PM. Beforehand, vga_switcheroo_client_probe_defer()
  * shall be called to ensure that all prerequisites are met.
  *
- * Return: 0 on success, -ENOMEM on memory allocation error.
+ * Return: 0 on success, -ENOMEM on memory allocation error, -EINVAL on getting
+ * client id error.
  */
 int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
 			const struct vga_switcheroo_client_ops *ops,
-			enum vga_switcheroo_client_id id)
+			struct pci_dev *vga_dev)
 {
-	return register_client(pdev, ops, id | ID_BIT_AUDIO, false, true);
+	enum vga_switcheroo_client_id id = VGA_SWITCHEROO_UNKNOWN_ID;
+
+	/*
+	 * if vga_switcheroo has enabled, that mean two GPU clients and also
+	 * handler are registered. Get audio client id from bound GPU client
+	 * id directly, otherwise, set it as VGA_SWITCHEROO_UNKNOWN_ID,
+	 * it will set to correct id in later when vga_switcheroo_enable()
+	 * is called.
+	 */
+	mutex_lock(&vgasr_mutex);
+	if (vgasr_priv.active) {
+		id = vgasr_priv.handler->get_client_id(vga_dev);
+		if (id < 0) {
+			mutex_unlock(&vgasr_mutex);
+			return -EINVAL;
+		}
+	}
+	mutex_unlock(&vgasr_mutex);
+
+	return register_client(pdev, ops, id | ID_BIT_AUDIO, vga_dev,
+			       false, true);
 }
 EXPORT_SYMBOL(vga_switcheroo_register_audio_client);
 
diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
index 77f0f0a..a34539b 100644
--- a/include/linux/vga_switcheroo.h
+++ b/include/linux/vga_switcheroo.h
@@ -84,8 +84,8 @@ enum vga_switcheroo_state {
  * Client identifier. Audio clients use the same identifier & 0x100.
  */
 enum vga_switcheroo_client_id {
-	VGA_SWITCHEROO_UNKNOWN_ID = -1,
-	VGA_SWITCHEROO_IGD,
+	VGA_SWITCHEROO_UNKNOWN_ID = 0x1000,
+	VGA_SWITCHEROO_IGD = 0,
 	VGA_SWITCHEROO_DIS,
 	VGA_SWITCHEROO_MAX_CLIENTS,
 };
@@ -151,7 +151,7 @@ int vga_switcheroo_register_client(struct pci_dev *dev,
 				   bool driver_power_control);
 int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
 					 const struct vga_switcheroo_client_ops *ops,
-					 enum vga_switcheroo_client_id id);
+					 struct pci_dev *vga_dev);
 
 void vga_switcheroo_client_fb_set(struct pci_dev *dev,
 				  struct fb_info *info);
@@ -180,7 +180,7 @@ static inline int vga_switcheroo_register_handler(const struct vga_switcheroo_ha
 		enum vga_switcheroo_handler_flags_t handler_flags) { return 0; }
 static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
 	const struct vga_switcheroo_client_ops *ops,
-	enum vga_switcheroo_client_id id) { return 0; }
+	struct pci_dev *vga_dev) { return 0; }
 static inline void vga_switcheroo_unregister_handler(void) {}
 static inline enum vga_switcheroo_handler_flags_t vga_switcheroo_handler_flags(void) { return 0; }
 static inline int vga_switcheroo_lock_ddc(struct pci_dev *pdev) { return -ENODEV; }
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 14733ff..ead6b7a 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1319,15 +1319,16 @@ static const struct vga_switcheroo_client_ops azx_vs_ops = {
 static int register_vga_switcheroo(struct azx *chip)
 {
 	struct hda_intel *hda = container_of(chip, struct hda_intel, chip);
+	struct pci_dev *p;
 	int err;
 
 	if (!hda->use_vga_switcheroo)
 		return 0;
-	/* FIXME: currently only handling DIS controller
-	 * is there any machine with two switchable HDMI audio controllers?
-	 */
-	err = vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops,
-						   VGA_SWITCHEROO_DIS);
+
+	p = get_bound_vga(chip->pci);
+	err = vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops, p);
+	pci_dev_put(p);
+
 	if (err < 0)
 		return err;
 	hda->vga_switcheroo_registered = 1;
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [V3] vga_switcheroo: set audio client id according to bound GPU id
  2018-07-17  8:20 [V3] vga_switcheroo: set audio client id according to bound GPU id Jim Qu
@ 2018-07-17  8:38 ` Lukas Wunner
       [not found]   ` <20180717083858.GA29241-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: Lukas Wunner @ 2018-07-17  8:38 UTC (permalink / raw)
  To: Jim Qu, tiwai; +Cc: alexander.deucher, alsa-devel, amd-gfx, dri-devel

On Tue, Jul 17, 2018 at 04:20:50PM +0800, Jim Qu wrote:
> On modern laptop, there are more and more platforms
> have two GPUs, and each of them maybe have audio codec
> for HDMP/DP output. For some dGPU which is no output,
> audio codec usually is disabled.
> 
> In currect HDA audio driver, it will set all codec as
> VGA_SWITCHEROO_DIS, the audio which is binded to UMA
> will be suspended if user use debugfs to contorl power
> 
> In HDA driver side, it is difficult to know which GPU
> the audio has binded to. So set the bound gpu pci dev
> to vga_switcheroo.
> 
> if the audio client is not the third registration, audio
> id will set in vga_switcheroo enable function. if the
> audio client is the last registration when vga_switcheroo
> _ready() get true, we should get audio client id from bound
> GPU directly.
> 
> Signed-off-by: Jim Qu <Jim.Qu@amd.com>

Reviewed-by: Lukas Wunner <lukas@wunner.de>

@Takashi, any preference which tree to merge this through?
sound or drm-misc, either way would seem fine to me.  I think
there's going to be one final drm-misc pull sent to Dave this
week, after that it's 4.20.

Thanks,

Lukas
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [V3] vga_switcheroo: set audio client id according to bound GPU id
       [not found]   ` <20180717083858.GA29241-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
@ 2018-07-17  8:52     ` Takashi Iwai
  2018-07-17  8:56       ` jimqu
  0 siblings, 1 reply; 5+ messages in thread
From: Takashi Iwai @ 2018-07-17  8:52 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Jim Qu,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, daniel-/w4YWyX8dFk,
	alexander.deucher-5C7GfCeVMHo

On Tue, 17 Jul 2018 10:38:58 +0200,
Lukas Wunner wrote:
> 
> On Tue, Jul 17, 2018 at 04:20:50PM +0800, Jim Qu wrote:
> > On modern laptop, there are more and more platforms
> > have two GPUs, and each of them maybe have audio codec
> > for HDMP/DP output. For some dGPU which is no output,
> > audio codec usually is disabled.
> > 
> > In currect HDA audio driver, it will set all codec as
> > VGA_SWITCHEROO_DIS, the audio which is binded to UMA
> > will be suspended if user use debugfs to contorl power
> > 
> > In HDA driver side, it is difficult to know which GPU
> > the audio has binded to. So set the bound gpu pci dev
> > to vga_switcheroo.
> > 
> > if the audio client is not the third registration, audio
> > id will set in vga_switcheroo enable function. if the
> > audio client is the last registration when vga_switcheroo
> > _ready() get true, we should get audio client id from bound
> > GPU directly.
> > 
> > Signed-off-by: Jim Qu <Jim.Qu@amd.com>
> 
> Reviewed-by: Lukas Wunner <lukas@wunner.de>
> 
> @Takashi, any preference which tree to merge this through?
> sound or drm-misc, either way would seem fine to me.  I think
> there's going to be one final drm-misc pull sent to Dave this
> week, after that it's 4.20.

Since it's basically an audio problem and I'd love to merge it for
4.19, I'd prefer taking through sound git tree, unless anyone has
objection.


thanks,

Takashi
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [V3] vga_switcheroo: set audio client id according to bound GPU id
  2018-07-17  8:52     ` Takashi Iwai
@ 2018-07-17  8:56       ` jimqu
       [not found]         ` <4fcfa367-4387-7b08-cf29-85b9322da371-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: jimqu @ 2018-07-17  8:56 UTC (permalink / raw)
  To: Takashi Iwai, Lukas Wunner
  Cc: alsa-devel, dri-devel, Jim Qu, amd-gfx, alexander.deucher



On 2018年07月17日 16:52, Takashi Iwai wrote:
> On Tue, 17 Jul 2018 10:38:58 +0200,
> Lukas Wunner wrote:
>> On Tue, Jul 17, 2018 at 04:20:50PM +0800, Jim Qu wrote:
>>> On modern laptop, there are more and more platforms
>>> have two GPUs, and each of them maybe have audio codec
>>> for HDMP/DP output. For some dGPU which is no output,
>>> audio codec usually is disabled.
>>>
>>> In currect HDA audio driver, it will set all codec as
>>> VGA_SWITCHEROO_DIS, the audio which is binded to UMA
>>> will be suspended if user use debugfs to contorl power
>>>
>>> In HDA driver side, it is difficult to know which GPU
>>> the audio has binded to. So set the bound gpu pci dev
>>> to vga_switcheroo.
>>>
>>> if the audio client is not the third registration, audio
>>> id will set in vga_switcheroo enable function. if the
>>> audio client is the last registration when vga_switcheroo
>>> _ready() get true, we should get audio client id from bound
>>> GPU directly.
>>>
>>> Signed-off-by: Jim Qu <Jim.Qu@amd.com>
>> Reviewed-by: Lukas Wunner <lukas@wunner.de>
>>
>> @Takashi, any preference which tree to merge this through?
>> sound or drm-misc, either way would seem fine to me.  I think
>> there's going to be one final drm-misc pull sent to Dave this
>> week, after that it's 4.20.
> Since it's basically an audio problem and I'd love to merge it for
> 4.19, I'd prefer taking through sound git tree, unless anyone has
> objection.

Thanks to Takashi and Lukas great help. Please kindly help merge the 
patch into suitable branch.

Thanks
JimQu

> thanks,
>
> Takashi

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [V3] vga_switcheroo: set audio client id according to bound GPU id
       [not found]         ` <4fcfa367-4387-7b08-cf29-85b9322da371-5C7GfCeVMHo@public.gmane.org>
@ 2018-07-17  9:22           ` Takashi Iwai
  0 siblings, 0 replies; 5+ messages in thread
From: Takashi Iwai @ 2018-07-17  9:22 UTC (permalink / raw)
  To: jimqu
  Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Jim Qu, Lukas Wunner,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, daniel-/w4YWyX8dFk,
	alexander.deucher-5C7GfCeVMHo

On Tue, 17 Jul 2018 10:56:37 +0200,
jimqu wrote:
> 
> 
> 
> On 2018年07月17日 16:52, Takashi Iwai wrote:
> > On Tue, 17 Jul 2018 10:38:58 +0200,
> > Lukas Wunner wrote:
> >> On Tue, Jul 17, 2018 at 04:20:50PM +0800, Jim Qu wrote:
> >>> On modern laptop, there are more and more platforms
> >>> have two GPUs, and each of them maybe have audio codec
> >>> for HDMP/DP output. For some dGPU which is no output,
> >>> audio codec usually is disabled.
> >>>
> >>> In currect HDA audio driver, it will set all codec as
> >>> VGA_SWITCHEROO_DIS, the audio which is binded to UMA
> >>> will be suspended if user use debugfs to contorl power
> >>>
> >>> In HDA driver side, it is difficult to know which GPU
> >>> the audio has binded to. So set the bound gpu pci dev
> >>> to vga_switcheroo.
> >>>
> >>> if the audio client is not the third registration, audio
> >>> id will set in vga_switcheroo enable function. if the
> >>> audio client is the last registration when vga_switcheroo
> >>> _ready() get true, we should get audio client id from bound
> >>> GPU directly.
> >>>
> >>> Signed-off-by: Jim Qu <Jim.Qu@amd.com>
> >> Reviewed-by: Lukas Wunner <lukas@wunner.de>
> >>
> >> @Takashi, any preference which tree to merge this through?
> >> sound or drm-misc, either way would seem fine to me.  I think
> >> there's going to be one final drm-misc pull sent to Dave this
> >> week, after that it's 4.20.
> > Since it's basically an audio problem and I'd love to merge it for
> > 4.19, I'd prefer taking through sound git tree, unless anyone has
> > objection.
> 
> Thanks to Takashi and Lukas great help. Please kindly help merge the
> patch into suitable branch.

I pushed the fix to topic/vga_switcheroo branch of sound git tree now
so that 0day bot can check it.  I'll wait for a while and merge it
later to for-next branch if nothing happens.

The brach is (and will be) based on fresh 4.18-rc5 so that other trees
may merge it cleanly.


thanks,

Takashi
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-07-17  9:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-17  8:20 [V3] vga_switcheroo: set audio client id according to bound GPU id Jim Qu
2018-07-17  8:38 ` Lukas Wunner
     [not found]   ` <20180717083858.GA29241-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
2018-07-17  8:52     ` Takashi Iwai
2018-07-17  8:56       ` jimqu
     [not found]         ` <4fcfa367-4387-7b08-cf29-85b9322da371-5C7GfCeVMHo@public.gmane.org>
2018-07-17  9:22           ` Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.