Re: [PATCH v10 07/15] drm/bridge: add connector luminance operations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] drm/bridge: rebase artifact in drm_bridge.h causes redefinition build failure
--

commit 13f21a6ed9ffad8454f947df0968b7d42361d36f
Author: Mario Limonciello <mario.limonciello@amd.com>

drm/bridge: add connector luminance operations

This commit adds a set_luminance callback, DRM_BRIDGE_OP_LUMINANCE flag, and
relative luminance fields to struct drm_bridge so bridges that control panel
brightness can advertise luminance operations.

> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 1981d24a700d2..95f5c2b8656cd 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h

[ ... ]

> @@ -1522,6 +1542,60 @@ void drm_bridge_hpd_disable(struct drm_bridge *bridge);
>  void drm_bridge_hpd_notify(struct drm_bridge *bridge,
>  			   enum drm_connector_status status);
>  
> +#ifdef CONFIG_DRM_PANEL_BRIDGE
> +bool drm_bridge_is_panel(const struct drm_bridge *bridge);
> +struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel);
> +struct drm_bridge *drm_panel_bridge_add_typed(struct drm_panel *panel,
> +					      u32 connector_type);
> +void drm_panel_bridge_remove(struct drm_bridge *bridge);
> +int drm_panel_bridge_set_orientation(struct drm_connector *connector,
> +				     struct drm_bridge *bridge);
> +struct drm_bridge *devm_drm_panel_bridge_add(struct device *dev,
> +					     struct drm_panel *panel);
> +struct drm_bridge *devm_drm_panel_bridge_add_typed(struct device *dev,
> +						   struct drm_panel *panel,
> +						   u32 connector_type);
> +struct drm_bridge *drmm_panel_bridge_add(struct drm_device *drm,
> +					     struct drm_panel *panel);
> +struct drm_connector *drm_panel_bridge_connector(struct drm_bridge *bridge);
> +#else
> +static inline bool drm_bridge_is_panel(const struct drm_bridge *bridge)
> +{
> +	return false;
> +}
> +
> +static inline int drm_panel_bridge_set_orientation(struct drm_connector *connector,
> +						   struct drm_bridge *bridge)
> +{
> +	return -EINVAL;
> +}
> +
> +#endif
> +
> +#if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL_BRIDGE)
> +struct drm_bridge *devm_drm_of_get_bridge(struct device *dev, struct device_node *node,
> +					  u32 port, u32 endpoint);
> +struct drm_bridge *drmm_of_get_bridge(struct drm_device *drm, struct device_node *node,
> +					  u32 port, u32 endpoint);
> +#else
> +static inline struct drm_bridge *devm_drm_of_get_bridge(struct device *dev,
> +							struct device_node *node,
> +							u32 port,
> +							u32 endpoint)
> +{
> +	return ERR_PTR(-ENODEV);\n> +}
> +
> +static inline struct drm_bridge *drmm_of_get_bridge(struct drm_device *drm,
> +						     struct device_node *node,
> +						     u32 port,
> +						     u32 endpoint)
> +{
> +	return ERR_PTR(-ENODEV);\n> +}
> +#endif
> +
> +void devm_drm_put_bridge(struct device *dev, struct drm_bridge *bridge);

[Severity: Low]
Was this hunk included by mistake during a rebase or patch split?

Prior to this series, commit aa8a8c7e4641 ("drm/bridge: panel: move all code
to drm_panel.c") moved the CONFIG_DRM_PANEL_BRIDGE and CONFIG_OF panel-bridge
declarations and their #else static inline fallback definitions from
include/drm/drm_bridge.h into include/drm/drm_panel.h, and commit 51afe16e33a0
("drm/bridge: remove devm_drm_put_bridge()") removed devm_drm_put_bridge().

Because include/drm/drm_panel.h includes drm/drm_bridge.h and already defines
those same static inline functions in its #else branches:

include/drm/drm_panel.h:
    #ifdef CONFIG_DRM_PANEL_BRIDGE
    bool drm_bridge_is_panel(const struct drm_bridge *bridge);
    ...
    #else
    static inline bool drm_bridge_is_panel(const struct drm_bridge *bridge)
    {
        return false;
    }

    static inline int drm_panel_bridge_set_orientation(struct drm_connector *connector,
                                                       struct drm_bridge *bridge)
    {
        return -EINVAL;
    }
    #endif

    #if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL_BRIDGE)
    ...
    #else
    static inline struct drm_bridge *devm_drm_of_get_bridge(...)
    ...
    static inline struct drm_bridge *drmm_of_get_bridge(...)
    ...
    #endif

does re-adding this block in include/drm/drm_bridge.h cause redefinition build
failures in translation units that include drm/drm_panel.h (such as
drivers/gpu/drm/display/drm_bridge_connector.c, drivers/gpu/drm/drm_panel.c,
and drivers/gpu/drm/drm_of.c) when CONFIG_OF or CONFIG_DRM_PANEL_BRIDGE is
disabled?

Although a subsequent patch in the series, commit 3eac0878c8a6 ("drm/panel:
add connector luminance forwarding"), removes the #else static inline stubs
and the CONFIG_OF block from include/drm/drm_panel.h, this breaks bisection at
this commit and leaves the misplaced panel-bridge and OF bridge declarations
along with the deleted devm_drm_put_bridge() prototype in
include/drm/drm_bridge.h.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-superm1-backlight-property-v10-0-b2a6f24131f5@kernel.org?part=7