From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> To: "Mukunda,Vijendar" <vijendar.mukunda@amd.com>, broonie@kernel.org, vkoul@kernel.org, alsa-devel@alsa-project.org Cc: Mastan.Katragadda@amd.com, Sunil-kumar.Dommati@amd.com, open list <linux-kernel@vger.kernel.org>, Basavaraj.Hiregoudar@amd.com, Takashi Iwai <tiwai@suse.com>, Liam Girdwood <lgirdwood@gmail.com>, Nathan Chancellor <nathan@kernel.org>, Mario.Limonciello@amd.com, arungopal.kondaveeti@amd.com, Sanyog Kale <sanyog.r.kale@intel.com>, Bard Liao <yung-chuan.liao@linux.intel.com>, Syed Saba Kareem <Syed.SabaKareem@amd.com> Subject: Re: [PATCH 01/19] ASoC: amd: ps: create platform devices based on acp config Date: Tue, 31 Jan 2023 10:00:54 -0600 [thread overview] Message-ID: <fa4cdd91-b430-eb1b-a151-d144f62e827d@linux.intel.com> (raw) In-Reply-To: <87ddd91b-fb5f-4f27-942b-dc439b32ce20@amd.com> On 1/31/23 07:09, Mukunda,Vijendar wrote: > On 16/01/23 13:32, Mukunda,Vijendar wrote: >> On 13/01/23 22:41, Pierre-Louis Bossart wrote: >>>>>> + if (is_dmic_dev && is_sdw_dev) { >>>>>> + switch (acp_data->sdw_master_count) { >>>>>> + case 1: >>>>>> + acp_data->pdev_mask = ACP63_SDW_PDM_DEV_MASK; >>>>>> + acp_data->pdev_count = ACP63_SDW0_PDM_MODE_DEVS; >>>>>> + break; >>>>>> + case 2: >>>>>> + acp_data->pdev_mask = ACP63_SDW_PDM_DEV_MASK; >>>>>> + acp_data->pdev_count = ACP63_SDW0_SDW1_PDM_MODE_DEVS; >>>>>> + break; >>>>> so the cover letter is indeed wrong and confuses two controllers for two >>>>> managers. >>>> ACP IP has two independent manager instances driven by separate controller >>>> each which are connected in different power domains. >>>> >>>> we should create two separate ACPI companion devices for separate >>>> manager instance. Currently we have limitations with BIOS. >>>> we are going with single ACPI companion device. >>>> We will update the changes later. >>> Humm, this is tricky. The BIOS interface isn't something that can be >>> changed at will on the kernel side, you'd have to maintain two solutions >>> with a means to detect which one to use. >>> >>> Or is this is a temporary issue on development devices, then that part >>> should probably not be upstreamed. >> It's a temporary issue on development devices. >> We had discussion with Windows dev team and BIOS team. >> They have agreed to modify ACPI companion device logic. >> We will update the two companion devices logic for two manager >> instances in V2 version. > After experimenting, two ACPI companion devices approach, > we got an update from Windows team, there is a limitation > on windows stack. For current platform, we can't proceed > with two ACPI companion devices. so how would the two controllers be declared then in the DSDT used by Windows? There's a contradiction between having a single companion device and the ability to set the 'manager-number' to one. You probably want to give an example of what you have, otherwise we probably will talk past each other. > > Even on Linux side, if we create two ACPI companion devices > followed by creating a single soundwire manager instance per > Soundwire controller, we have observed an issue in a scenario, > where similar codec parts(UID are also same) are connected on > both soundwire manager instances. We've been handling this case of two identical amplifiers on two different links for the last 3 years. I don't see how this could be a problem, the codecs are declared in the scope of the companion device and the _ADR defines in bits [51..48] which link the codec is connected to. see example below from a TigerLake device with two identical amsp on link 1 and 2. Scope (_SB.PC00.HDAS.SNDW) { Device (SWD1) { Name (_ADR, 0x000131025D131601) // _ADR: Address Device (SWD2) { Name (_ADR, 0x000230025D131601) // _ADR: Address > As per MIPI Disco spec, for single link controllers Link ID should > be set to zero. > If we use Link ID as zero, for the soundwire manager which is on > the second soundwire controller ACPI device scope, then soundwire > framework is not allowing to create peripheral device node as its > duplicate one. I still don't see how it's possible. There is an IDA used in the bus allocation static int sdw_get_id(struct sdw_bus *bus) { int rc = ida_alloc(&sdw_bus_ida, GFP_KERNEL); if (rc < 0) return rc; bus->id = rc; return 0; } and that's used for debugfs /* create the debugfs master-N */ snprintf(name, sizeof(name), "master-%d-%d", bus->id, bus->link_id); as well as in sdw_master_device_add(): dev_set_name(&md->dev, "sdw-master-%d", bus->id); can you clarify what part of the 'SoundWire framework' is problematic? I guess the problem is that you have identical devices with the same _ADR under the same manager, which is problematic indeed, but that's not a SoundWire framework issue, just not a supported configuration. > If we want to support two ACPI companion device approach > on our future platforms, how to proceed? Well how about dealing with a single companion device first, cause that's what you have now and that's already problematic.
WARNING: multiple messages have this Message-ID (diff)
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> To: "Mukunda,Vijendar" <vijendar.mukunda@amd.com>, broonie@kernel.org, vkoul@kernel.org, alsa-devel@alsa-project.org Cc: Mastan.Katragadda@amd.com, Sunil-kumar.Dommati@amd.com, Basavaraj.Hiregoudar@amd.com, Takashi Iwai <tiwai@suse.com>, Liam Girdwood <lgirdwood@gmail.com>, open list <linux-kernel@vger.kernel.org>, Nathan Chancellor <nathan@kernel.org>, Mario.Limonciello@amd.com, arungopal.kondaveeti@amd.com, Sanyog Kale <sanyog.r.kale@intel.com>, Bard Liao <yung-chuan.liao@linux.intel.com>, Syed Saba Kareem <Syed.SabaKareem@amd.com> Subject: Re: [PATCH 01/19] ASoC: amd: ps: create platform devices based on acp config Date: Tue, 31 Jan 2023 10:00:54 -0600 [thread overview] Message-ID: <fa4cdd91-b430-eb1b-a151-d144f62e827d@linux.intel.com> (raw) In-Reply-To: <87ddd91b-fb5f-4f27-942b-dc439b32ce20@amd.com> On 1/31/23 07:09, Mukunda,Vijendar wrote: > On 16/01/23 13:32, Mukunda,Vijendar wrote: >> On 13/01/23 22:41, Pierre-Louis Bossart wrote: >>>>>> + if (is_dmic_dev && is_sdw_dev) { >>>>>> + switch (acp_data->sdw_master_count) { >>>>>> + case 1: >>>>>> + acp_data->pdev_mask = ACP63_SDW_PDM_DEV_MASK; >>>>>> + acp_data->pdev_count = ACP63_SDW0_PDM_MODE_DEVS; >>>>>> + break; >>>>>> + case 2: >>>>>> + acp_data->pdev_mask = ACP63_SDW_PDM_DEV_MASK; >>>>>> + acp_data->pdev_count = ACP63_SDW0_SDW1_PDM_MODE_DEVS; >>>>>> + break; >>>>> so the cover letter is indeed wrong and confuses two controllers for two >>>>> managers. >>>> ACP IP has two independent manager instances driven by separate controller >>>> each which are connected in different power domains. >>>> >>>> we should create two separate ACPI companion devices for separate >>>> manager instance. Currently we have limitations with BIOS. >>>> we are going with single ACPI companion device. >>>> We will update the changes later. >>> Humm, this is tricky. The BIOS interface isn't something that can be >>> changed at will on the kernel side, you'd have to maintain two solutions >>> with a means to detect which one to use. >>> >>> Or is this is a temporary issue on development devices, then that part >>> should probably not be upstreamed. >> It's a temporary issue on development devices. >> We had discussion with Windows dev team and BIOS team. >> They have agreed to modify ACPI companion device logic. >> We will update the two companion devices logic for two manager >> instances in V2 version. > After experimenting, two ACPI companion devices approach, > we got an update from Windows team, there is a limitation > on windows stack. For current platform, we can't proceed > with two ACPI companion devices. so how would the two controllers be declared then in the DSDT used by Windows? There's a contradiction between having a single companion device and the ability to set the 'manager-number' to one. You probably want to give an example of what you have, otherwise we probably will talk past each other. > > Even on Linux side, if we create two ACPI companion devices > followed by creating a single soundwire manager instance per > Soundwire controller, we have observed an issue in a scenario, > where similar codec parts(UID are also same) are connected on > both soundwire manager instances. We've been handling this case of two identical amplifiers on two different links for the last 3 years. I don't see how this could be a problem, the codecs are declared in the scope of the companion device and the _ADR defines in bits [51..48] which link the codec is connected to. see example below from a TigerLake device with two identical amsp on link 1 and 2. Scope (_SB.PC00.HDAS.SNDW) { Device (SWD1) { Name (_ADR, 0x000131025D131601) // _ADR: Address Device (SWD2) { Name (_ADR, 0x000230025D131601) // _ADR: Address > As per MIPI Disco spec, for single link controllers Link ID should > be set to zero. > If we use Link ID as zero, for the soundwire manager which is on > the second soundwire controller ACPI device scope, then soundwire > framework is not allowing to create peripheral device node as its > duplicate one. I still don't see how it's possible. There is an IDA used in the bus allocation static int sdw_get_id(struct sdw_bus *bus) { int rc = ida_alloc(&sdw_bus_ida, GFP_KERNEL); if (rc < 0) return rc; bus->id = rc; return 0; } and that's used for debugfs /* create the debugfs master-N */ snprintf(name, sizeof(name), "master-%d-%d", bus->id, bus->link_id); as well as in sdw_master_device_add(): dev_set_name(&md->dev, "sdw-master-%d", bus->id); can you clarify what part of the 'SoundWire framework' is problematic? I guess the problem is that you have identical devices with the same _ADR under the same manager, which is problematic indeed, but that's not a SoundWire framework issue, just not a supported configuration. > If we want to support two ACPI companion device approach > on our future platforms, how to proceed? Well how about dealing with a single companion device first, cause that's what you have now and that's already problematic.
next prev parent reply other threads:[~2023-01-31 22:19 UTC|newest] Thread overview: 170+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-01-11 9:02 [PATCH 00/19] Add soundwire support for Pink Sardine platform Vijendar Mukunda 2023-01-11 9:02 ` [PATCH 01/19] ASoC: amd: ps: create platform devices based on acp config Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 13:27 ` Amadeusz Sławiński 2023-01-11 13:27 ` Amadeusz Sławiński 2023-01-11 14:13 ` Mukunda,Vijendar 2023-01-11 14:13 ` Mukunda,Vijendar 2023-01-11 13:32 ` Pierre-Louis Bossart 2023-01-11 13:32 ` Pierre-Louis Bossart 2023-01-13 12:36 ` Mukunda,Vijendar 2023-01-13 12:36 ` Mukunda,Vijendar 2023-01-13 17:11 ` Pierre-Louis Bossart 2023-01-13 17:11 ` Pierre-Louis Bossart 2023-01-16 8:02 ` Mukunda,Vijendar 2023-01-16 8:02 ` Mukunda,Vijendar 2023-01-31 13:09 ` Mukunda,Vijendar 2023-01-31 13:09 ` Mukunda,Vijendar 2023-01-31 13:24 ` Mario Limonciello 2023-01-31 13:24 ` Mario Limonciello 2023-01-31 16:00 ` Pierre-Louis Bossart [this message] 2023-01-31 16:00 ` Pierre-Louis Bossart 2023-01-31 22:57 ` Limonciello, Mario 2023-01-31 22:57 ` Limonciello, Mario 2023-02-01 0:51 ` Pierre-Louis Bossart 2023-02-01 0:51 ` Pierre-Louis Bossart 2023-02-01 1:45 ` Mukunda,Vijendar 2023-02-01 1:45 ` Mukunda,Vijendar 2023-02-01 2:03 ` Pierre-Louis Bossart 2023-02-01 2:03 ` Pierre-Louis Bossart 2023-02-01 2:10 ` Mukunda,Vijendar 2023-02-01 2:10 ` Mukunda,Vijendar 2023-02-01 3:52 ` Pierre-Louis Bossart 2023-02-01 6:01 ` Mukunda,Vijendar 2023-02-01 23:08 ` Pierre-Louis Bossart 2023-02-06 6:30 ` Mukunda,Vijendar 2023-02-06 14:50 ` Pierre-Louis Bossart 2023-02-06 16:38 ` Mukunda,Vijendar 2023-01-11 9:02 ` [PATCH 02/19] soundwire: amd: Add support for AMD Master driver Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 13:59 ` Amadeusz Sławiński 2023-01-11 13:59 ` Amadeusz Sławiński 2023-01-11 14:16 ` Mukunda,Vijendar 2023-01-11 14:16 ` Mukunda,Vijendar 2023-01-11 14:37 ` Pierre-Louis Bossart 2023-01-11 14:37 ` Pierre-Louis Bossart 2023-01-13 18:21 ` Mukunda,Vijendar 2023-01-13 18:21 ` Mukunda,Vijendar 2023-01-13 18:41 ` Pierre-Louis Bossart 2023-01-13 18:41 ` Pierre-Louis Bossart 2023-01-16 7:53 ` Mukunda,Vijendar 2023-01-16 7:53 ` Mukunda,Vijendar 2023-01-16 14:57 ` Pierre-Louis Bossart 2023-01-16 14:57 ` Pierre-Louis Bossart 2023-01-17 11:37 ` Mukunda,Vijendar 2023-01-17 11:37 ` Mukunda,Vijendar 2023-01-11 9:02 ` [PATCH 03/19] soundwire: amd: register sdw controller dai ops Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 14:58 ` Pierre-Louis Bossart 2023-01-11 14:58 ` Pierre-Louis Bossart 2023-01-13 11:31 ` Mukunda,Vijendar 2023-01-13 11:31 ` Mukunda,Vijendar 2023-01-13 17:13 ` Pierre-Louis Bossart 2023-01-13 17:13 ` Pierre-Louis Bossart 2023-01-11 14:59 ` Amadeusz Sławiński 2023-01-11 14:59 ` Amadeusz Sławiński 2023-01-11 9:02 ` [PATCH 04/19] soundwire: amd: enable build for AMD soundwire master driver Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-19 18:35 ` kernel test robot 2023-01-19 18:35 ` kernel test robot 2023-01-11 9:02 ` [PATCH 05/19] soundwire: amd: add soundwire interrupt handling Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 15:19 ` Pierre-Louis Bossart 2023-01-11 15:19 ` Pierre-Louis Bossart 2023-01-19 22:00 ` kernel test robot 2023-01-19 22:00 ` kernel test robot 2023-01-11 9:02 ` [PATCH 06/19] ASoC: amd: ps: add support for soundwire interrupts in acp pci driver Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 9:02 ` [PATCH 07/19] ASoC: amd: ps: add soundwire dma driver for pink sardine platform Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 15:22 ` Pierre-Louis Bossart 2023-01-11 15:22 ` Pierre-Louis Bossart 2023-01-12 9:10 ` Mukunda,Vijendar 2023-01-12 9:10 ` Mukunda,Vijendar 2023-01-11 9:02 ` [PATCH 08/19] ASoC: amd: ps: add soundwire dma driver dma ops Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 13:04 ` Mark Brown 2023-01-11 13:04 ` Mark Brown 2023-01-11 14:08 ` Mukunda,Vijendar 2023-01-11 14:08 ` Mukunda,Vijendar 2023-01-11 15:34 ` Pierre-Louis Bossart 2023-01-11 15:34 ` Pierre-Louis Bossart 2023-01-13 11:16 ` Mukunda,Vijendar 2023-01-13 11:16 ` Mukunda,Vijendar 2023-01-13 17:05 ` Pierre-Louis Bossart 2023-01-13 17:05 ` Pierre-Louis Bossart 2023-01-16 6:59 ` Mukunda,Vijendar 2023-01-16 6:59 ` Mukunda,Vijendar 2023-01-11 9:02 ` [PATCH 09/19] ASoC: amd: ps: add support for Soundwire DMA interrupts Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 15:38 ` Pierre-Louis Bossart 2023-01-11 15:38 ` Pierre-Louis Bossart 2023-01-12 10:55 ` Mukunda,Vijendar 2023-01-12 10:55 ` Mukunda,Vijendar 2023-01-11 9:02 ` [PATCH 10/19] ASoC: amd: ps: enable Soundwire DMA driver build Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 9:02 ` [PATCH 11/19] ASoC: amd: update comments in Kconfig file Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 9:02 ` [PATCH 12/19] ASoC: amd: ps: Add soundwire specific checks in pci driver in pm ops Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 9:02 ` [PATCH 13/19] ASoC: amd: ps: add support for runtime pm ops for soundwire dma driver Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 9:02 ` [PATCH 14/19] soundwire: amd: add runtime pm ops for AMD master driver Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 15:47 ` Pierre-Louis Bossart 2023-01-11 15:47 ` Pierre-Louis Bossart 2023-01-12 10:35 ` Mukunda,Vijendar 2023-01-12 10:35 ` Mukunda,Vijendar 2023-01-12 14:47 ` Pierre-Louis Bossart 2023-01-12 14:47 ` Pierre-Louis Bossart 2023-01-11 9:02 ` [PATCH 15/19] soundwire: amd: add startup and shutdown dai ops Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 15:49 ` Pierre-Louis Bossart 2023-01-11 15:49 ` Pierre-Louis Bossart 2023-01-12 10:22 ` Mukunda,Vijendar 2023-01-12 10:22 ` Mukunda,Vijendar 2023-01-11 9:02 ` [PATCH 16/19] soundwire: amd: handle wake enable interrupt Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 15:54 ` Pierre-Louis Bossart 2023-01-11 15:54 ` Pierre-Louis Bossart 2023-01-12 10:21 ` Mukunda,Vijendar 2023-01-12 10:21 ` Mukunda,Vijendar 2023-01-11 9:02 ` [PATCH 17/19] soundwire: amd: add pm_prepare callback and pm ops support Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 15:58 ` Pierre-Louis Bossart 2023-01-11 15:58 ` Pierre-Louis Bossart 2023-01-12 10:14 ` Mukunda,Vijendar 2023-01-12 10:14 ` Mukunda,Vijendar 2023-01-12 14:50 ` Pierre-Louis Bossart 2023-01-12 14:50 ` Pierre-Louis Bossart 2023-01-11 9:02 ` [PATCH 18/19] ASoC: amd: ps: implement system level pm ops for soundwire dma driver Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 9:02 ` [PATCH 19/19] ASoC: amd: ps: increase runtime suspend delay Vijendar Mukunda 2023-01-11 9:02 ` Vijendar Mukunda 2023-01-11 16:02 ` Pierre-Louis Bossart 2023-01-11 16:02 ` Pierre-Louis Bossart 2023-01-12 11:02 ` Mukunda,Vijendar 2023-01-12 11:02 ` Mukunda,Vijendar 2023-01-12 14:54 ` Pierre-Louis Bossart 2023-01-12 14:54 ` Pierre-Louis Bossart 2023-01-12 15:29 ` Limonciello, Mario 2023-01-12 15:29 ` Limonciello, Mario 2023-01-12 16:05 ` Pierre-Louis Bossart 2023-01-13 10:58 ` Mukunda,Vijendar 2023-01-13 17:33 ` Pierre-Louis Bossart 2023-01-13 19:57 ` Mark Brown 2023-01-13 19:57 ` Mark Brown 2023-01-16 8:35 ` Mukunda,Vijendar 2023-01-16 8:35 ` Mukunda,Vijendar 2023-01-16 15:02 ` Pierre-Louis Bossart 2023-01-16 15:02 ` Pierre-Louis Bossart 2023-01-17 11:33 ` Mukunda,Vijendar 2023-01-17 11:33 ` Mukunda,Vijendar 2023-01-17 11:51 ` Pierre-Louis Bossart 2023-01-17 11:51 ` Pierre-Louis Bossart 2023-01-17 12:16 ` Mark Brown 2023-01-17 12:16 ` Mark Brown 2023-01-17 12:36 ` Pierre-Louis Bossart 2023-01-17 12:36 ` Pierre-Louis Bossart 2023-01-11 13:36 ` [PATCH 00/19] Add soundwire support for Pink Sardine platform Pierre-Louis Bossart 2023-01-12 9:08 ` Mukunda,Vijendar
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=fa4cdd91-b430-eb1b-a151-d144f62e827d@linux.intel.com \ --to=pierre-louis.bossart@linux.intel.com \ --cc=Basavaraj.Hiregoudar@amd.com \ --cc=Mario.Limonciello@amd.com \ --cc=Mastan.Katragadda@amd.com \ --cc=Sunil-kumar.Dommati@amd.com \ --cc=Syed.SabaKareem@amd.com \ --cc=alsa-devel@alsa-project.org \ --cc=arungopal.kondaveeti@amd.com \ --cc=broonie@kernel.org \ --cc=lgirdwood@gmail.com \ --cc=linux-kernel@vger.kernel.org \ --cc=nathan@kernel.org \ --cc=sanyog.r.kale@intel.com \ --cc=tiwai@suse.com \ --cc=vijendar.mukunda@amd.com \ --cc=vkoul@kernel.org \ --cc=yung-chuan.liao@linux.intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.