All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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.