[PATCH] drm/amdgpu: use static ids for ACP platform devs

[PATCH] drm/amdgpu: use static ids for ACP platform devs

[Brady Norander]

mfd_add_hotplug_devices() assigns child platform devices with
PLATFORM_DEVID_AUTO, but the ACP machine drivers expect the platform
device names to never change. Use mfd_add_devices() instead and give
each cell a unique id.

Signed-off-by: Brady Norander <bradynorander@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
index deb0785350e8..9c657637d317 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
@@ -302,17 +302,19 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block)
 		adev->acp.acp_res[2].end = adev->acp.acp_res[2].start;
 
 		adev->acp.acp_cell[0].name = "acp_audio_dma";
+		adev->acp.acp_cell[0].id = 0;
 		adev->acp.acp_cell[0].num_resources = 3;
 		adev->acp.acp_cell[0].resources = &adev->acp.acp_res[0];
 		adev->acp.acp_cell[0].platform_data = &adev->asic_type;
 		adev->acp.acp_cell[0].pdata_size = sizeof(adev->asic_type);
 
 		adev->acp.acp_cell[1].name = "designware-i2s";
+		adev->acp.acp_cell[1].id = 1;
 		adev->acp.acp_cell[1].num_resources = 1;
 		adev->acp.acp_cell[1].resources = &adev->acp.acp_res[1];
 		adev->acp.acp_cell[1].platform_data = &i2s_pdata[0];
 		adev->acp.acp_cell[1].pdata_size = sizeof(struct i2s_platform_data);
-		r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell, 2);
+		r = mfd_add_devices(adev->acp.parent, 0, adev->acp.acp_cell, 2, NULL, 0, NULL);
 		if (r)
 			goto failure;
 		r = device_for_each_child(adev->acp.parent, &adev->acp.acp_genpd->gpd,
@@ -410,30 +412,34 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block)
 		adev->acp.acp_res[4].end = adev->acp.acp_res[4].start;
 
 		adev->acp.acp_cell[0].name = "acp_audio_dma";
+		adev->acp.acp_cell[0].id = 0;
 		adev->acp.acp_cell[0].num_resources = 5;
 		adev->acp.acp_cell[0].resources = &adev->acp.acp_res[0];
 		adev->acp.acp_cell[0].platform_data = &adev->asic_type;
 		adev->acp.acp_cell[0].pdata_size = sizeof(adev->asic_type);
 
 		adev->acp.acp_cell[1].name = "designware-i2s";
+		adev->acp.acp_cell[1].id = 1;
 		adev->acp.acp_cell[1].num_resources = 1;
 		adev->acp.acp_cell[1].resources = &adev->acp.acp_res[1];
 		adev->acp.acp_cell[1].platform_data = &i2s_pdata[0];
 		adev->acp.acp_cell[1].pdata_size = sizeof(struct i2s_platform_data);
 
 		adev->acp.acp_cell[2].name = "designware-i2s";
+		adev->acp.acp_cell[2].id = 2;
 		adev->acp.acp_cell[2].num_resources = 1;
 		adev->acp.acp_cell[2].resources = &adev->acp.acp_res[2];
 		adev->acp.acp_cell[2].platform_data = &i2s_pdata[1];
 		adev->acp.acp_cell[2].pdata_size = sizeof(struct i2s_platform_data);
 
 		adev->acp.acp_cell[3].name = "designware-i2s";
+		adev->acp.acp_cell[3].id = 3;
 		adev->acp.acp_cell[3].num_resources = 1;
 		adev->acp.acp_cell[3].resources = &adev->acp.acp_res[3];
 		adev->acp.acp_cell[3].platform_data = &i2s_pdata[2];
 		adev->acp.acp_cell[3].pdata_size = sizeof(struct i2s_platform_data);
 
-		r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell, ACP_DEVS);
+		r = mfd_add_devices(adev->acp.parent, 0, adev->acp.acp_cell, ACP_DEVS, NULL, 0, NULL);
 		if (r)
 			goto failure;
 
-- 
2.48.1

Re: [PATCH] drm/amdgpu: use static ids for ACP platform devs

[Alex Deucher]

On Tue, Mar 25, 2025 at 6:07 PM Brady Norander <bradynorander@gmail.com> wrote:
>
> mfd_add_hotplug_devices() assigns child platform devices with
> PLATFORM_DEVID_AUTO, but the ACP machine drivers expect the platform
> device names to never change. Use mfd_add_devices() instead and give
> each cell a unique id.

While you are at it, can you take a look at
drivers/gpu/drm/amd/amdgpu/isp_v4_1_0.c and
drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c as well?

Alex

>
> Signed-off-by: Brady Norander <bradynorander@gmail.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> index deb0785350e8..9c657637d317 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> @@ -302,17 +302,19 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block)
>                 adev->acp.acp_res[2].end = adev->acp.acp_res[2].start;
>
>                 adev->acp.acp_cell[0].name = "acp_audio_dma";
> +               adev->acp.acp_cell[0].id = 0;
>                 adev->acp.acp_cell[0].num_resources = 3;
>                 adev->acp.acp_cell[0].resources = &adev->acp.acp_res[0];
>                 adev->acp.acp_cell[0].platform_data = &adev->asic_type;
>                 adev->acp.acp_cell[0].pdata_size = sizeof(adev->asic_type);
>
>                 adev->acp.acp_cell[1].name = "designware-i2s";
> +               adev->acp.acp_cell[1].id = 1;
>                 adev->acp.acp_cell[1].num_resources = 1;
>                 adev->acp.acp_cell[1].resources = &adev->acp.acp_res[1];
>                 adev->acp.acp_cell[1].platform_data = &i2s_pdata[0];
>                 adev->acp.acp_cell[1].pdata_size = sizeof(struct i2s_platform_data);
> -               r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell, 2);
> +               r = mfd_add_devices(adev->acp.parent, 0, adev->acp.acp_cell, 2, NULL, 0, NULL);
>                 if (r)
>                         goto failure;
>                 r = device_for_each_child(adev->acp.parent, &adev->acp.acp_genpd->gpd,
> @@ -410,30 +412,34 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block)
>                 adev->acp.acp_res[4].end = adev->acp.acp_res[4].start;
>
>                 adev->acp.acp_cell[0].name = "acp_audio_dma";
> +               adev->acp.acp_cell[0].id = 0;
>                 adev->acp.acp_cell[0].num_resources = 5;
>                 adev->acp.acp_cell[0].resources = &adev->acp.acp_res[0];
>                 adev->acp.acp_cell[0].platform_data = &adev->asic_type;
>                 adev->acp.acp_cell[0].pdata_size = sizeof(adev->asic_type);
>
>                 adev->acp.acp_cell[1].name = "designware-i2s";
> +               adev->acp.acp_cell[1].id = 1;
>                 adev->acp.acp_cell[1].num_resources = 1;
>                 adev->acp.acp_cell[1].resources = &adev->acp.acp_res[1];
>                 adev->acp.acp_cell[1].platform_data = &i2s_pdata[0];
>                 adev->acp.acp_cell[1].pdata_size = sizeof(struct i2s_platform_data);
>
>                 adev->acp.acp_cell[2].name = "designware-i2s";
> +               adev->acp.acp_cell[2].id = 2;
>                 adev->acp.acp_cell[2].num_resources = 1;
>                 adev->acp.acp_cell[2].resources = &adev->acp.acp_res[2];
>                 adev->acp.acp_cell[2].platform_data = &i2s_pdata[1];
>                 adev->acp.acp_cell[2].pdata_size = sizeof(struct i2s_platform_data);
>
>                 adev->acp.acp_cell[3].name = "designware-i2s";
> +               adev->acp.acp_cell[3].id = 3;
>                 adev->acp.acp_cell[3].num_resources = 1;
>                 adev->acp.acp_cell[3].resources = &adev->acp.acp_res[3];
>                 adev->acp.acp_cell[3].platform_data = &i2s_pdata[2];
>                 adev->acp.acp_cell[3].pdata_size = sizeof(struct i2s_platform_data);
>
> -               r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell, ACP_DEVS);
> +               r = mfd_add_devices(adev->acp.parent, 0, adev->acp.acp_cell, ACP_DEVS, NULL, 0, NULL);
>                 if (r)
>                         goto failure;
>
> --
> 2.48.1
>

Re: [PATCH] drm/amdgpu: use static ids for ACP platform devs

[Brady Norander]

On 3/25/25 6:12 PM, Alex Deucher wrote:
> 
> While you are at it, can you take a look at
> drivers/gpu/drm/amd/amdgpu/isp_v4_1_0.c and
> drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c as well?
> 
> Alex

I think it makes more sense to handle that in a separate patch as it is 
an unrelated ip block.

Re: [PATCH] drm/amdgpu: use static ids for ACP platform devs

[Alex Deucher]

On Tue, Mar 25, 2025 at 7:11 PM Brady Norander <bradynorander@gmail.com> wrote:
>
> On 3/25/25 6:12 PM, Alex Deucher wrote:
> >
> > While you are at it, can you take a look at
> > drivers/gpu/drm/amd/amdgpu/isp_v4_1_0.c and
> > drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c as well?
> >
> > Alex
>
> I think it makes more sense to handle that in a separate patch as it is
> an unrelated ip block.

Sure. Can you send a patch to fix those up as well if needed?

Alex

Re: [PATCH] drm/amdgpu: use static ids for ACP platform devs

[Brady Norander]

On 11/10/25 14:14, Alex Deucher wrote:
> On Tue, Mar 25, 2025 at 7:11 PM Brady Norander <bradynorander@gmail.com> wrote:
>>
>> On 3/25/25 6:12 PM, Alex Deucher wrote:
>>>
>>> While you are at it, can you take a look at
>>> drivers/gpu/drm/amd/amdgpu/isp_v4_1_0.c and
>>> drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c as well?
>>>
>>> Alex
>>
>> I think it makes more sense to handle that in a separate patch as it is
>> an unrelated ip block.
> 
> Sure. Can you send a patch to fix those up as well if needed?
> 
> Alex

I don't know if that driver needs this, and I also don't have that 
hardware to test it. If that driver needs this fix, then this can be 
revisited but I don't think it should block this patch being merged now.

Re: [PATCH] drm/amdgpu: use static ids for ACP platform devs

[Brady Norander]

On 11/10/25 14:14, Alex Deucher wrote:
> On Tue, Mar 25, 2025 at 7:11 PM Brady Norander <bradynorander@gmail.com> wrote:
>>
>> On 3/25/25 6:12 PM, Alex Deucher wrote:
>>>
>>> While you are at it, can you take a look at
>>> drivers/gpu/drm/amd/amdgpu/isp_v4_1_0.c and
>>> drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c as well?
>>>
>>> Alex
>>
>> I think it makes more sense to handle that in a separate patch as it is
>> an unrelated ip block.
> 
> Sure. Can you send a patch to fix those up as well if needed?
> 
> Alex

Sorry to nag again, but can we look into getting this patch merged? The 
ASoC patch has already been merged and it would be nice to have audio 
working on this platform.

Re: [PATCH] drm/amdgpu: use static ids for ACP platform devs

[Alex Deucher]

Applied.  Thanks.

Alex

On Sun, Nov 30, 2025 at 8:48 AM Brady Norander <bradynorander@gmail.com> wrote:
>
> On 11/10/25 14:14, Alex Deucher wrote:
> > On Tue, Mar 25, 2025 at 7:11 PM Brady Norander <bradynorander@gmail.com> wrote:
> >>
> >> On 3/25/25 6:12 PM, Alex Deucher wrote:
> >>>
> >>> While you are at it, can you take a look at
> >>> drivers/gpu/drm/amd/amdgpu/isp_v4_1_0.c and
> >>> drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c as well?
> >>>
> >>> Alex
> >>
> >> I think it makes more sense to handle that in a separate patch as it is
> >> an unrelated ip block.
> >
> > Sure. Can you send a patch to fix those up as well if needed?
> >
> > Alex
>
> Sorry to nag again, but can we look into getting this patch merged? The
> ASoC patch has already been merged and it would be nice to have audio
> working on this platform.

Re: [PATCH] drm/amdgpu: use static ids for ACP platform devs

[Oleksandr Natalenko]

[-- Attachment #1: Type: text/plain, Size: 3645 bytes --]

Hello.

On úterý 25. března 2025 22:05:17, středoevropský standardní čas Brady Norander wrote:
> mfd_add_hotplug_devices() assigns child platform devices with
> PLATFORM_DEVID_AUTO, but the ACP machine drivers expect the platform
> device names to never change. Use mfd_add_devices() instead and give
> each cell a unique id.
> 
> Signed-off-by: Brady Norander <bradynorander@gmail.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> index deb0785350e8..9c657637d317 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> @@ -302,17 +302,19 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block)
>  		adev->acp.acp_res[2].end = adev->acp.acp_res[2].start;
>  
>  		adev->acp.acp_cell[0].name = "acp_audio_dma";
> +		adev->acp.acp_cell[0].id = 0;
>  		adev->acp.acp_cell[0].num_resources = 3;
>  		adev->acp.acp_cell[0].resources = &adev->acp.acp_res[0];
>  		adev->acp.acp_cell[0].platform_data = &adev->asic_type;
>  		adev->acp.acp_cell[0].pdata_size = sizeof(adev->asic_type);
>  
>  		adev->acp.acp_cell[1].name = "designware-i2s";
> +		adev->acp.acp_cell[1].id = 1;
>  		adev->acp.acp_cell[1].num_resources = 1;
>  		adev->acp.acp_cell[1].resources = &adev->acp.acp_res[1];
>  		adev->acp.acp_cell[1].platform_data = &i2s_pdata[0];
>  		adev->acp.acp_cell[1].pdata_size = sizeof(struct i2s_platform_data);
> -		r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell, 2);
> +		r = mfd_add_devices(adev->acp.parent, 0, adev->acp.acp_cell, 2, NULL, 0, NULL);
>  		if (r)
>  			goto failure;
>  		r = device_for_each_child(adev->acp.parent, &adev->acp.acp_genpd->gpd,
> @@ -410,30 +412,34 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block)
>  		adev->acp.acp_res[4].end = adev->acp.acp_res[4].start;
>  
>  		adev->acp.acp_cell[0].name = "acp_audio_dma";
> +		adev->acp.acp_cell[0].id = 0;
>  		adev->acp.acp_cell[0].num_resources = 5;
>  		adev->acp.acp_cell[0].resources = &adev->acp.acp_res[0];
>  		adev->acp.acp_cell[0].platform_data = &adev->asic_type;
>  		adev->acp.acp_cell[0].pdata_size = sizeof(adev->asic_type);
>  
>  		adev->acp.acp_cell[1].name = "designware-i2s";
> +		adev->acp.acp_cell[1].id = 1;
>  		adev->acp.acp_cell[1].num_resources = 1;
>  		adev->acp.acp_cell[1].resources = &adev->acp.acp_res[1];
>  		adev->acp.acp_cell[1].platform_data = &i2s_pdata[0];
>  		adev->acp.acp_cell[1].pdata_size = sizeof(struct i2s_platform_data);
>  
>  		adev->acp.acp_cell[2].name = "designware-i2s";
> +		adev->acp.acp_cell[2].id = 2;
>  		adev->acp.acp_cell[2].num_resources = 1;
>  		adev->acp.acp_cell[2].resources = &adev->acp.acp_res[2];
>  		adev->acp.acp_cell[2].platform_data = &i2s_pdata[1];
>  		adev->acp.acp_cell[2].pdata_size = sizeof(struct i2s_platform_data);
>  
>  		adev->acp.acp_cell[3].name = "designware-i2s";
> +		adev->acp.acp_cell[3].id = 3;
>  		adev->acp.acp_cell[3].num_resources = 1;
>  		adev->acp.acp_cell[3].resources = &adev->acp.acp_res[3];
>  		adev->acp.acp_cell[3].platform_data = &i2s_pdata[2];
>  		adev->acp.acp_cell[3].pdata_size = sizeof(struct i2s_platform_data);
>  
> -		r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell, ACP_DEVS);
> +		r = mfd_add_devices(adev->acp.parent, 0, adev->acp.acp_cell, ACP_DEVS, NULL, 0, NULL);
>  		if (r)
>  			goto failure;

Is this patch still supposed to be applied?

Thank you.

-- 
Oleksandr Natalenko, MSE

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]