From: sashiko-bot@kernel.org
To: "Albert Esteve" <aesteve@redhat.com>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 1/3] drm/amd/display: replace STATIC_IFN_KUNIT with VISIBLE_IF_KUNIT
Date: Mon, 07 Sep 2026 13:50:02 +0000 [thread overview]
Message-ID: <20260907135003.515A01F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260907-visibility-unification-v1-1-6a962e5b3351@redhat.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing prototypes and global namespace pollution for generic function names when CONFIG_KUNIT is enabled but CONFIG_DRM_AMD_DC_KUNIT_TEST is disabled.
--
commit 559b79e4c5aa67bc448a3cebcac53b59d32d0f66
Author: Albert Esteve <aesteve@redhat.com>
drm/amd/display: replace STATIC_IFN_KUNIT with VISIBLE_IF_KUNIT
This commit switches amdgpu_dm test helpers from the local STATIC_IFN_KUNIT
macro to the generic VISIBLE_IF_KUNIT macro. The helpers become non-static
whenever KUnit is enabled, not just when AMD DC tests are built.
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index ec483276d7538..5b3bebde0b083 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -167,7 +168,7 @@ static inline void amdgpu_dm_exit_ips_for_hw_access(struct dc *dc)
> * @return
> * Counter for vertical blanks
> */
> -STATIC_IFN_KUNIT u32 dm_vblank_get_counter(struct amdgpu_device *adev, int crtc)
> +VISIBLE_IF_KUNIT u32 dm_vblank_get_counter(struct amdgpu_device *adev, int crtc)
[Severity: Low]
If CONFIG_KUNIT is enabled but CONFIG_DRM_AMD_DC_KUNIT_TEST is disabled,
VISIBLE_IF_KUNIT will make this function globally visible. However, if its
prototype in drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h is still
guarded by #if IS_ENABLED(CONFIG_DRM_AMD_DC_KUNIT_TEST), won't this cause a
-Wmissing-prototypes build warning?
[ ... ]
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> index c8a1ab8c3b169..fe310b6d9f603 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> @@ -724,7 +724,7 @@ amdgpu_dm_convert_color_depth_from_display_info(const struct drm_connector *conn
> }
> EXPORT_IF_KUNIT(amdgpu_dm_convert_color_depth_from_display_info);
>
> -STATIC_IFN_KUNIT enum dc_aspect_ratio
> +VISIBLE_IF_KUNIT enum dc_aspect_ratio
> get_aspect_ratio(const struct drm_display_mode *mode_in)
[Severity: Low]
Since VISIBLE_IF_KUNIT makes this function globally visible when CONFIG_KUNIT
is enabled, does the generic name get_aspect_ratio risk namespace collisions
with other display or media drivers?
[ ... ]
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> index 47cbc826ed451..20c1ce8cda446 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> @@ -187,7 +190,7 @@ int psp_set_srm(struct psp_context *psp,
> }
> EXPORT_IF_KUNIT(psp_set_srm);
>
> -STATIC_IFN_KUNIT
> +VISIBLE_IF_KUNIT
> void process_output(struct hdcp_workqueue *hdcp_work)
[Severity: Low]
With VISIBLE_IF_KUNIT, this generic name process_output becomes a global
symbol in vmlinux. Is there a risk of link-time collisions with other
drivers?
Also, similar to the missing prototype issue mentioned earlier, could this
lead to a build warning if the prototype in amdgpu_dm_hdcp.h is still
guarded by CONFIG_DRM_AMD_DC_KUNIT_TEST?
[ ... ]
> @@ -386,7 +389,7 @@ void hdcp_handle_cpirq(struct hdcp_workqueue *hdcp_work, unsigned int link_index
> }
> EXPORT_IF_KUNIT(hdcp_handle_cpirq);
>
> -STATIC_IFN_KUNIT
> +VISIBLE_IF_KUNIT
> void event_callback(struct work_struct *work)
[Severity: Low]
Similar to the above, does exposing the generic name event_callback to the
global namespace risk collisions when CONFIG_KUNIT is enabled?
[ ... ]
> @@ -608,7 +611,7 @@ bool enable_assr(void *handle, struct dc_link *link)
> }
> EXPORT_IF_KUNIT(enable_assr);
>
> -STATIC_IFN_KUNIT
> +VISIBLE_IF_KUNIT
> void update_config(void *handle, struct cp_psp_stream_config *config)
[Severity: Low]
Could making update_config globally visible cause namespace collisions, as it
lacks a subsystem prefix?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-visibility-unification-v1-0-6a962e5b3351@redhat.com?part=1
next prev parent reply other threads:[~2026-09-07 13:50 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-07 13:28 [PATCH 0/3] drm/amd/display: use kunit/visibility.h for test helpers Albert Esteve
2026-09-07 13:28 ` [PATCH 1/3] drm/amd/display: replace STATIC_IFN_KUNIT with VISIBLE_IF_KUNIT Albert Esteve
2026-09-07 13:50 ` sashiko-bot [this message]
2026-09-07 13:28 ` [PATCH 2/3] drm/amd/display: replace EXPORT_IF_KUNIT with EXPORT_SYMBOL_IF_KUNIT Albert Esteve
2026-09-07 13:28 ` [PATCH 3/3] drm/amd/display: drop INLINE_IFN_KUNIT Albert Esteve
2026-09-07 15:39 ` Jani Nikula
2026-09-07 15:24 ` [PATCH 0/3] drm/amd/display: use kunit/visibility.h for test helpers Maxime Ripard
2026-09-11 20:50 ` Alex Hung
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=20260907135003.515A01F00A3A@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=aesteve@redhat.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=sashiko-reviews@lists.linux.dev \
/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: link
Be 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.