Hi Jeff, I'm thinking out loud so here are a few suggestions/ideas. Please don't take them too seriously although they do make sense from this end. On 24 May 2016 at 18:47, Jeff Mahoney <jeffm@xxxxxxxx> wrote: > The DRM_AMD_ACP option doesn't have any dependencies and selects > MFD_CORE, which results in MFD_CORE=y. Since the code is only called > from DRM_AMDGPU, it should depend on it. Adding the dependency results > in MFD_CORE being selected as a module again if amdgpu is also a module. > > Signed-off-by: Jeff Mahoney <jeffm@xxxxxxxx> > --- > drivers/gpu/drm/amd/acp/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/amd/acp/Kconfig b/drivers/gpu/drm/amd/acp/Kconfig > index ca77ec1..e503e3d 100644 > --- a/drivers/gpu/drm/amd/acp/Kconfig > +++ b/drivers/gpu/drm/amd/acp/Kconfig > @@ -2,6 +2,7 @@ menu "ACP (Audio CoProcessor) Configuration" > > config DRM_AMD_ACP > bool "Enable AMD Audio CoProcessor IP support" > + depends on DRM_AMDGPU > select MFD_CORE > select PM_GENERIC_DOMAINS if PM > help > Afaict ACP/Powerplay doesn't make sense on their own so here is an alternative solution: - create a Kconfig in drm/amd and move/consolidate amdgpu, powerplay, acp in there. amdkfd can be moved as well afaict. - make DRM_AMD_ACP and DRM_AMD_POWERPLAY submenu items for DRM_AMDGPU. AMDKFD on the other hand depends on RADEON || AMDGPU so it should stay separate. - crazy idea: add select and/or depends on SND_SOC_AMD_ACP. since one is useless without the other. Or perhaps make the SOC entry should select/depend on AMD_ACP ? And some future work (cleanups): - fold the acp folder (or inline respective files) within amdgpu. - add forward declarations in acp/include/acp_gfx_if.h and move the cgs* includes where needed (acp/acp_hw.c and amdgpu/amdgpu_acp.c) - apply similar polish for amdgpu/amdgpu_acp.h. - drop the (unneeded ?) include of amdgpu_acp.h include from amdgpu/vi.c - kill off amdgpu_acp::private. Afaict there's not a single place (as of 95306975e9dd38ba2775dd96cb29987ecc7d9360) that actually defines the amd_acp_private struct. Is something broken on my end or this does not build without warnings/errors ? - kill off amdgpu_acp::acp_genpd::cgs_dev (use amdgpu_acp::cgs_device directly) and inline the ::acp_genpd::gpd into amdgpu_acp. Regards, Emil _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
