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

Brady Norander posted 1 patch 10 months, 2 weeks ago
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
[PATCH] drm/amdgpu: use static ids for ACP platform devs
Posted by Brady Norander 10 months, 2 weeks ago
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
Posted by Oleksandr Natalenko 2 months, 4 weeks ago
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
Re: [PATCH] drm/amdgpu: use static ids for ACP platform devs
Posted by Alex Deucher 10 months, 2 weeks ago
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
Posted by Brady Norander 10 months, 2 weeks ago
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
Posted by Alex Deucher 2 months, 4 weeks ago
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
Posted by Brady Norander 2 months, 1 week ago
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
Posted by Alex Deucher 2 months, 1 week ago
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
Posted by Brady Norander 2 months, 4 weeks ago
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.