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 <[email protected]> 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/[email protected]?part=1
