> Subject: [RFC PATCH v2 5/5] drm/msm/dp: remove extra wrappers and > public functions > Date: Sat, 12 Feb 2022 01:40:06 +0300 > From: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > To: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>, Rob Clark > <robdclark@xxxxxxxxx>, Sean Paul <sean@xxxxxxxxxx>, Abhinav Kumar > <quic_abhinavk@xxxxxxxxxxx>, Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx> > CC: Stephen Boyd <swboyd@xxxxxxxxxxxx>, David Airlie <airlied@xxxxxxxx>, > Daniel Vetter <daniel@xxxxxxxx>, linux-arm-msm@xxxxxxxxxxxxxxx, dri- > devel@xxxxxxxxxxxxxxxxxxxxx, freedreno@xxxxxxxxxxxxxxxxxxxxx > > dp_bridge's functions are thin wrappers around the msm_dp_display_* > family. Squash dp_bridge callbacks into respective msm_dp_display > functions, removing the latter functions from public space. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> Tested-by: Sankeerth Billakanti <quic_sbillaka@xxxxxxxxxxx> > --- > drivers/gpu/drm/msm/dp/dp_display.c | 54 +++++++++++++++------- > drivers/gpu/drm/msm/dp/dp_display.h | 1 - > drivers/gpu/drm/msm/dp/dp_drm.c | 72 ++--------------------------- > drivers/gpu/drm/msm/dp/dp_drm.h | 22 ++++++++- > drivers/gpu/drm/msm/msm_drv.h | 31 ------------- > 5 files changed, 60 insertions(+), 120 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index 59e5e5b8e5b4..a9b641a68bff 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -10,7 +10,6 @@ > #include <linux/component.h> > #include <linux/of_irq.h> > #include <linux/delay.h> > -#include <drm/drm_panel.h> > #include "msm_drv.h" > #include "msm_kms.h" > @@ -945,18 +944,36 @@ int dp_display_set_plugged_cb(struct msm_dp > *dp_display, > return 0; > } > -int dp_display_validate_mode(struct msm_dp *dp, u32 mode_pclk_khz) > +/** > + * dp_bridge_mode_valid - callback to determine if specified mode is > +valid > + * @bridge: Pointer to drm bridge structure > + * @info: display info > + * @mode: Pointer to drm mode structure > + * Returns: Validity status for specified mode */ enum drm_mode_status > +dp_bridge_mode_valid(struct drm_bridge *bridge, > + const struct drm_display_info *info, > + const struct drm_display_mode > *mode) > { > const u32 num_components = 3, default_bpp = 24; > struct dp_display_private *dp_display; > struct dp_link_info *link_info; > u32 mode_rate_khz = 0, supported_rate_khz = 0, mode_bpp = 0; > + struct msm_dp *dp; > + int mode_pclk_khz = mode->clock; > + > + dp = to_dp_bridge(bridge)->dp_display; > if (!dp || !mode_pclk_khz || !dp->connector) { > DRM_ERROR("invalid params\n"); > return -EINVAL; > } > + if ((dp->max_pclk_khz <= 0) || > + (dp->max_pclk_khz > DP_MAX_PIXEL_CLK_KHZ) || > + (mode->clock > dp->max_pclk_khz)) > + return MODE_BAD; > + > dp_display = container_of(dp, struct dp_display_private, > dp_display); > link_info = &dp_display->panel->link_info; > @@ -1501,7 +1518,7 @@ int msm_dp_modeset_init(struct msm_dp > *dp_display, struct drm_device *dev, > dp_display->encoder = encoder; > - dp_display->bridge = msm_dp_bridge_init(dp_display, dev, > encoder); > + dp_display->bridge = dp_bridge_init(dp_display, dev, encoder); > if (IS_ERR(dp_display->bridge)) { > ret = PTR_ERR(dp_display->bridge); > DRM_DEV_ERROR(dev->dev, > @@ -1528,8 +1545,10 @@ int msm_dp_modeset_init(struct msm_dp > *dp_display, struct drm_device *dev, > return 0; > } > -int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder > *encoder) > +void dp_bridge_enable(struct drm_bridge *drm_bridge) > { > + struct msm_dp_bridge *dp_bridge = to_dp_bridge(drm_bridge); > + struct msm_dp *dp = dp_bridge->dp_display; > int rc = 0; > struct dp_display_private *dp_display; > u32 state; > @@ -1537,7 +1556,7 @@ int msm_dp_display_enable(struct msm_dp *dp, > struct drm_encoder *encoder) > dp_display = container_of(dp, struct dp_display_private, > dp_display); > if (!dp_display->dp_mode.drm_mode.clock) { > DRM_ERROR("invalid params\n"); > - return -EINVAL; > + return; > } > mutex_lock(&dp_display->event_mutex); > @@ -1549,14 +1568,14 @@ int msm_dp_display_enable(struct msm_dp *dp, > struct drm_encoder *encoder) > if (rc) { > DRM_ERROR("Failed to perform a mode set, rc=%d\n", rc); > mutex_unlock(&dp_display->event_mutex); > - return rc; > + return; > } > rc = dp_display_prepare(dp); > if (rc) { > DRM_ERROR("DP display prepare failed, rc=%d\n", rc); > mutex_unlock(&dp_display->event_mutex); > - return rc; > + return; > } > state = dp_display->hpd_state; > @@ -1581,23 +1600,23 @@ int msm_dp_display_enable(struct msm_dp *dp, > struct drm_encoder *encoder) > dp_display->hpd_state = ST_CONNECTED; > mutex_unlock(&dp_display->event_mutex); > - > - return rc; > } > -int msm_dp_display_pre_disable(struct msm_dp *dp, struct drm_encoder > *encoder) > +void dp_bridge_disable(struct drm_bridge *drm_bridge) > { > + struct msm_dp_bridge *dp_bridge = to_dp_bridge(drm_bridge); > + struct msm_dp *dp = dp_bridge->dp_display; > struct dp_display_private *dp_display; > dp_display = container_of(dp, struct dp_display_private, > dp_display); > dp_ctrl_push_idle(dp_display->ctrl); > - > - return 0; > } > -int msm_dp_display_disable(struct msm_dp *dp, struct drm_encoder > *encoder) > +void dp_bridge_post_disable(struct drm_bridge *drm_bridge) > { > + struct msm_dp_bridge *dp_bridge = to_dp_bridge(drm_bridge); > + struct msm_dp *dp = dp_bridge->dp_display; > int rc = 0; > u32 state; > struct dp_display_private *dp_display; @@ -1624,13 +1643,14 @@ > int msm_dp_display_disable(struct msm_dp *dp, struct drm_encoder > *encoder) > } > mutex_unlock(&dp_display->event_mutex); > - return rc; > } > -void msm_dp_display_mode_set(struct msm_dp *dp, struct drm_encoder > *encoder, > - const struct drm_display_mode *mode, > - const struct drm_display_mode > *adjusted_mode) > +void dp_bridge_mode_set(struct drm_bridge *drm_bridge, > + const struct drm_display_mode *mode, > + const struct drm_display_mode *adjusted_mode) > { > + struct msm_dp_bridge *dp_bridge = to_dp_bridge(drm_bridge); > + struct msm_dp *dp = dp_bridge->dp_display; > struct dp_display_private *dp_display; > dp_display = container_of(dp, struct dp_display_private, > dp_display); diff --git a/drivers/gpu/drm/msm/dp/dp_display.h > b/drivers/gpu/drm/msm/dp/dp_display.h > index 7af2b186d2d9..49a1d89681ac 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.h > +++ b/drivers/gpu/drm/msm/dp/dp_display.h > @@ -32,7 +32,6 @@ struct msm_dp { > int dp_display_set_plugged_cb(struct msm_dp *dp_display, > hdmi_codec_plugged_cb fn, struct device *codec_dev); -int > dp_display_validate_mode(struct msm_dp *dp_display, u32 > mode_pclk_khz); > int dp_display_get_modes(struct msm_dp *dp_display, > struct dp_display_mode *dp_mode); > int dp_display_request_irq(struct msm_dp *dp_display); diff --git > a/drivers/gpu/drm/msm/dp/dp_drm.c > b/drivers/gpu/drm/msm/dp/dp_drm.c index 57800b865fe6..7ce1aca48687 > 100644 > --- a/drivers/gpu/drm/msm/dp/dp_drm.c > +++ b/drivers/gpu/drm/msm/dp/dp_drm.c > @@ -13,14 +13,6 @@ > #include "msm_kms.h" > #include "dp_drm.h" > - > -struct msm_dp_bridge { > - struct drm_bridge bridge; > - struct msm_dp *dp_display; > -}; > - > -#define to_dp_display(x) container_of((x), struct msm_dp_bridge, > bridge) > - > /** > * dp_bridge_detect - callback to determine if connector is connected > * @bridge: Pointer to drm bridge structure @@ -30,7 +22,7 @@ static enum > drm_connector_status dp_bridge_detect(struct drm_bridge *bridge) > { > struct msm_dp *dp; > - dp = to_dp_display(bridge)->dp_display; > + dp = to_dp_bridge(bridge)->dp_display; > DRM_DEBUG_DP("is_connected = %s\n", > (dp->is_connected) ? "true" : "false"); @@ -55,7 +47,7 @@ > static int dp_bridge_get_modes(struct drm_bridge *bridge, struct > drm_connector * > if (!connector) > return 0; > - dp = to_dp_display(bridge)->dp_display; > + dp = to_dp_bridge(bridge)->dp_display; > dp_mode = kzalloc(sizeof(*dp_mode), GFP_KERNEL); > if (!dp_mode) > @@ -95,64 +87,6 @@ static int dp_bridge_get_modes(struct drm_bridge > *bridge, struct drm_connector * > return rc; > } > -/** > - * dp_bridge_mode_valid - callback to determine if specified mode is valid > - * @bridge: Pointer to drm bridge structure > - * @info: display info > - * @mode: Pointer to drm mode structure > - * Returns: Validity status for specified mode > - */ > -static enum drm_mode_status dp_bridge_mode_valid( > - struct drm_bridge *bridge, > - const struct drm_display_info *info, > - const struct drm_display_mode *mode) > -{ > - struct msm_dp *dp_disp; > - > - dp_disp = to_dp_display(bridge)->dp_display; > - > - if ((dp_disp->max_pclk_khz <= 0) || > - (dp_disp->max_pclk_khz > > DP_MAX_PIXEL_CLK_KHZ) || > - (mode->clock > dp_disp->max_pclk_khz)) > - return MODE_BAD; > - > - return dp_display_validate_mode(dp_disp, mode->clock); > -} > - > -static void dp_bridge_mode_set(struct drm_bridge *drm_bridge, > - const struct drm_display_mode *mode, > - const struct drm_display_mode > *adjusted_mode) > -{ > - struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge); > - struct msm_dp *dp_display = dp_bridge->dp_display; > - > - msm_dp_display_mode_set(dp_display, drm_bridge->encoder, > mode, > adjusted_mode); > -} > - > -static void dp_bridge_enable(struct drm_bridge *drm_bridge) -{ > - struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge); > - struct msm_dp *dp_display = dp_bridge->dp_display; > - > - msm_dp_display_enable(dp_display, drm_bridge->encoder); > -} > - > -static void dp_bridge_disable(struct drm_bridge *drm_bridge) -{ > - struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge); > - struct msm_dp *dp_display = dp_bridge->dp_display; > - > - msm_dp_display_pre_disable(dp_display, drm_bridge->encoder); > -} > - > -static void dp_bridge_post_disable(struct drm_bridge *drm_bridge) -{ > - struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge); > - struct msm_dp *dp_display = dp_bridge->dp_display; > - > - msm_dp_display_disable(dp_display, drm_bridge->encoder); > -} > - > static const struct drm_bridge_funcs dp_bridge_ops = { > .enable = dp_bridge_enable, > .disable = dp_bridge_disable, > @@ -163,7 +97,7 @@ static const struct drm_bridge_funcs dp_bridge_ops = > { > .detect = dp_bridge_detect, > }; > -struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, > struct drm_device *dev, > +struct drm_bridge *dp_bridge_init(struct msm_dp *dp_display, struct > drm_device *dev, > struct drm_encoder *encoder) > { > int rc; > diff --git a/drivers/gpu/drm/msm/dp/dp_drm.h > b/drivers/gpu/drm/msm/dp/dp_drm.h index c27bfceefdf0..f4b1ed1e24f7 > 100644 > --- a/drivers/gpu/drm/msm/dp/dp_drm.h > +++ b/drivers/gpu/drm/msm/dp/dp_drm.h > @@ -7,12 +7,30 @@ > #define _DP_DRM_H_ > #include <linux/types.h> > -#include <drm/drm_crtc.h> > -#include <drm/drm_crtc_helper.h> > +#include <drm/drm_bridge.h> > #include "msm_drv.h" > #include "dp_display.h" > +struct msm_dp_bridge { > + struct drm_bridge bridge; > + struct msm_dp *dp_display; > +}; > + > +#define to_dp_bridge(x) container_of((x), struct msm_dp_bridge, > bridge) > + > struct drm_connector *dp_drm_connector_init(struct msm_dp > *dp_display); > +struct drm_bridge *dp_bridge_init(struct msm_dp *dp_display, struct > drm_device *dev, > + struct drm_encoder *encoder); > + > +void dp_bridge_enable(struct drm_bridge *drm_bridge); void > +dp_bridge_disable(struct drm_bridge *drm_bridge); void > +dp_bridge_post_disable(struct drm_bridge *drm_bridge); enum > +drm_mode_status dp_bridge_mode_valid(struct drm_bridge *bridge, > + const struct drm_display_info *info, > + const struct drm_display_mode > *mode); void > +dp_bridge_mode_set(struct drm_bridge *drm_bridge, > + const struct drm_display_mode *mode, > + const struct drm_display_mode *adjusted_mode); > #endif /* _DP_DRM_H_ */ > diff --git a/drivers/gpu/drm/msm/msm_drv.h > b/drivers/gpu/drm/msm/msm_drv.h index d7574e6bd4e4..b698a3be6072 > 100644 > --- a/drivers/gpu/drm/msm/msm_drv.h > +++ b/drivers/gpu/drm/msm/msm_drv.h > @@ -385,16 +385,6 @@ int __init msm_dp_register(void); > void __exit msm_dp_unregister(void); > int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device > *dev, > struct drm_encoder *encoder); > -int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder > *encoder); -int msm_dp_display_disable(struct msm_dp *dp, struct > drm_encoder *encoder); -int msm_dp_display_pre_disable(struct msm_dp > *dp, struct drm_encoder *encoder); -void > msm_dp_display_mode_set(struct msm_dp *dp, struct drm_encoder > *encoder, > - const struct drm_display_mode *mode, > - const struct drm_display_mode > *adjusted_mode); > - > -struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, > - struct drm_device *dev, > - struct drm_encoder *encoder); > void msm_dp_irq_postinstall(struct msm_dp *dp_display); > void msm_dp_snapshot(struct msm_disp_state *disp_state, struct > msm_dp *dp_display); > @@ -414,27 +404,6 @@ static inline int msm_dp_modeset_init(struct > msm_dp *dp_display, > { > return -EINVAL; > } > -static inline int msm_dp_display_enable(struct msm_dp *dp, > - struct drm_encoder *encoder) > -{ > - return -EINVAL; > -} > -static inline int msm_dp_display_disable(struct msm_dp *dp, > - struct drm_encoder *encoder) > -{ > - return -EINVAL; > -} > -static inline int msm_dp_display_pre_disable(struct msm_dp *dp, > - struct drm_encoder *encoder) > -{ > - return -EINVAL; > -} > -static inline void msm_dp_display_mode_set(struct msm_dp *dp, > - struct drm_encoder *encoder, > - const struct drm_display_mode *mode, > - const struct drm_display_mode > *adjusted_mode) > -{ > -} > static inline void msm_dp_irq_postinstall(struct msm_dp *dp_display) > { > -- > 2.34.1