Re: [PATCH 14/19] drm/amd/display: Restrict the dc directory root to DC-internal builds

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] drm/amdgpu: build failure with CONFIG_DRM_AMD_DC=n due to
  missing dc/inc path
- [Medium] drm/amd/display: dc root include flag left in
  amdgpu_dm/Makefile
- [Low] drm/amd/display: stale dc.h path in display-manager.rst
  kernel-doc directives
--

commit a3254d30f8e563b4854ad67277856c56f8a96107
Author: Dave Airlie <airlied@redhat.com>

drm/amd/display: Restrict the dc directory root to DC-internal builds

This commit moves public headers from drivers/gpu/drm/amd/display/dc/ into
drivers/gpu/drm/amd/display/dc/inc/ and restricts the dc root include path
to DC-internal objects.

> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 8687afc00c24c..55c89f8d764e2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -39,7 +39,6 @@ ccflags-y := -I$(FULL_AMD_PATH)/include/asic_reg \
>  	-I$(FULL_AMD_DISPLAY_PATH) \
>  	-I$(FULL_AMD_DISPLAY_PATH)/include \
>  	-I$(FULL_AMD_DISPLAY_PATH)/modules/inc \
> -	-I$(FULL_AMD_DISPLAY_PATH)/dc \
>  	-I$(FULL_AMD_DISPLAY_PATH)/amdgpu_dm \
>  	-I$(FULL_AMD_PATH)/amdkfd \
>  	-I$(FULL_AMD_PATH)/ras/ras_mgr

[Severity: High]
Does removing -I$(FULL_AMD_DISPLAY_PATH)/dc without replacing it with
-I$(FULL_AMD_DISPLAY_PATH)/dc/inc break the build when CONFIG_DRM_AMD_DC is
disabled?

-I$(FULL_AMD_DISPLAY_PATH)/dc/inc/ is only added in
drivers/gpu/drm/amd/display/Makefile, which is included conditionally on
CONFIG_DRM_AMD_DC.  However, drivers/gpu/drm/amd/amdgpu/amdgpu.h
unconditionally includes dm_pp_interface.h and amdgpu_dm.h, which pull in
dm_services_types.h, os_types.h, dc_types.h, irq_types.h, and dm_services.h
that this patch moves into dc/inc/.

[ ... ]

> diff --git a/drivers/gpu/drm/amd/display/dc/Makefile b/drivers/gpu/drm/amd/display/dc/Makefile
> index 440f5d3581887..c0fc7f7d241d8 100644
> --- a/drivers/gpu/drm/amd/display/dc/Makefile
> +++ b/drivers/gpu/drm/amd/display/dc/Makefile
> @@ -98,3 +98,13 @@ endif
>  
>  AMD_DISPLAY_FILES += $(addprefix $(AMDDALPATH)/dc/, $(FILES))
>  
> +# The dc directory root holds private headers (dc_priv.h and friends) that
> +# must not be reachable outside DC, so the global include path deliberately
> +# stops at dc/inc. DC-internal objects still need the root on their include
> +# path for the many "subdir/header.h" includes inside DC, so add it per
> +# object instead of through a global ccflags-y entry.
> +DC_INTERNAL_OBJS := $(filter $(AMDDALPATH)/dc/%,$(AMD_DISPLAY_FILES))
> +$(foreach obj,$(DC_INTERNAL_OBJS),\
> +	$(eval CFLAGS_$(obj) += -I$(FULL_AMD_DISPLAY_PATH)/dc))
> +
> +

[Severity: Medium]
Does drivers/gpu/drm/amd/display/amdgpu_dm/Makefile still export the dc
root include path globally?

The commit message notes that -I$(FULL_AMD_DISPLAY_PATH)/dc is removed from
amdgpu_dm/Makefile, but that file is not modified by this patch and still
has:

drivers/gpu/drm/amd/display/amdgpu_dm/Makefile:
    subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc

Because drivers/gpu/drm/amd/display/Makefile includes amdgpu_dm/Makefile in
the top-level amdgpu build, Kbuild adds subdir-ccflags-y to KBUILD_CFLAGS
for all objects in amdgpu, amdgpu_dm, DAL modules, and tests, leaving
dc_priv.h and other dc root headers reachable outside DC.

[ ... ]

> diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/inc/dc.h
> similarity index 99%
> rename from drivers/gpu/drm/amd/display/dc/dc.h
> rename to drivers/gpu/drm/amd/display/dc/inc/dc.h

[Severity: Low]
Should Documentation/gpu/amdgpu/display/display-manager.rst also be updated
for the new path of dc.h?  It has two kernel-doc directives that still
reference drivers/gpu/drm/amd/display/dc/dc.h, which will cause htmldocs
builds to fail to locate the file.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-amdgpu-dc-cleanup-v1-0-0fae41718ad2@redhat.com?part=14