Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Decoupling the module state from the core DRM subsystem causes silent feature loss when DRM is built-in and the quirk is a module. - [Medium] Adding `depends on DRM` to `DRM_PANEL_ORIENTATION_QUIRKS` causes Kconfig unmet direct dependency warnings when selected by fbdev drivers. - [Low] Using `#ifdef` for a tristate config causes compiler redefinition errors and strips module metadata for loadable module (`=m`) builds. --
--- Patch [1]: [PATCH] drm: make panel orientation quirks optional --- commit babf1234e204a4c51ca97865c82a0c78512e71b2 Author: Andrew Zhou <[email protected]> drm: make panel orientation quirks optional This commit removes the unconditional selection of DRM_PANEL_ORIENTATION_QUIRKS from the DRM subsystem. It makes the option user-visible, adds a dependency on DRM, and provides fallback stubs for when the option is disabled. > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 323422861e8f6..da4ac94bbf615 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -8,7 +8,6 @@ > menuconfig DRM > tristate "Direct Rendering Manager (XFree86 4.1.0 and higher DRI > support)" > depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && HAS_DMA > - select DRM_PANEL_ORIENTATION_QUIRKS [Severity: High] Does removing this select statement allow the core DRM subsystem to be built-in while DRM_PANEL_ORIENTATION_QUIRKS is configured as a module? If this configuration happens, the built-in DRM core code might evaluate the preprocessor checks for the quirk module as false, which could cause the panel orientation functionality to be silently dropped even though the module is built. > select HDMI > [ ... ] > @@ -369,4 +368,15 @@ endif > > # Separate option because drm_panel_orientation_quirks.c is shared with fbdev > config DRM_PANEL_ORIENTATION_QUIRKS > - tristate > + tristate "Panel orientation quirks for DMI-based detection" > + depends on DRM [Severity: Medium] Can this new dependency cause Kconfig unmet direct dependency warnings? The FB_EFI config option unconditionally selects DRM_PANEL_ORIENTATION_QUIRKS since it shares this code. If a system is configured with CONFIG_FB_EFI=y and CONFIG_DRM=n, the selection from the EFI framebuffer could violate this new dependency on DRM, causing a Kconfig unmet dependency warning. > + help > [ ... ] > diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c > b/drivers/gpu/drm/drm_panel_orientation_quirks.c > index 3a218fb592cef..fb09078a831af 100644 > --- a/drivers/gpu/drm/drm_panel_orientation_quirks.c > +++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c > @@ -589,7 +589,7 @@ EXPORT_SYMBOL(drm_get_panel_orientation_quirk); > > #else > > -/* There are no quirks for non x86 devices yet */ > +/* Fallback stub when DMI or the entire quirk module is disabled. */ > int drm_get_panel_orientation_quirk(int width, int height) > { > return DRM_MODE_PANEL_ORIENTATION_UNKNOWN; > @@ -598,5 +598,7 @@ EXPORT_SYMBOL(drm_get_panel_orientation_quirk); > > #endif > [ ... ] > diff --git a/include/drm/drm_utils.h b/include/drm/drm_utils.h > index 6a46f755daba0..947b8e71260e3 100644 > --- a/include/drm/drm_utils.h > +++ b/include/drm/drm_utils.h > @@ -11,10 +11,18 @@ > #define __DRM_UTILS_H__ > > #include <linux/types.h> > +#include <drm/drm_connector.h> > > struct drm_edid; > > +#ifdef CONFIG_DRM_PANEL_ORIENTATION_QUIRKS > int drm_get_panel_orientation_quirk(int width, int height); > +#else > +static inline int drm_get_panel_orientation_quirk(int width, int height) > +{ > + return DRM_MODE_PANEL_ORIENTATION_UNKNOWN; > +} > +#endif [Severity: Low] Does this preprocessor check properly handle tristate module builds? If CONFIG_DRM_PANEL_ORIENTATION_QUIRKS=m is configured, kbuild defines CONFIG_DRM_PANEL_ORIENTATION_QUIRKS_MODULE instead. This causes the check above to evaluate as false, providing the static inline stub in the header. Meanwhile, drm_panel_orientation_quirks.c evaluates its own matching check as false and defines a non-static fallback stub for the exact same function. Including this header in that source file could result in a compiler redefinition error. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
