[RFC PATCH v2 5/5] drm/msm/dp: remove extra wrappers and public functions
Abhinav Kumar
quic_abhinavk at quicinc.com
Thu Apr 14 20:17:09 UTC 2022
On 3/16/2022 9:31 AM, Sankeerth Billakanti (QUIC) wrote:
>> 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 at linaro.org>
>> To: Bjorn Andersson <bjorn.andersson at linaro.org>, Rob Clark
>> <robdclark at gmail.com>, Sean Paul <sean at poorly.run>, Abhinav Kumar
>> <quic_abhinavk at quicinc.com>, Kuogee Hsieh <quic_khsieh at quicinc.com>
>> CC: Stephen Boyd <swboyd at chromium.org>, David Airlie <airlied at linux.ie>,
>> Daniel Vetter <daniel at ffwll.ch>, linux-arm-msm at vger.kernel.org, dri-
>> devel at lists.freedesktop.org, freedreno at lists.freedesktop.org
>>
>> 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 at linaro.org>
>
>
> Tested-by: Sankeerth Billakanti <quic_sbillaka at quicinc.com>
Reviewed-by: Abhinav Kumar <quic_abhinavk at quicinc.com>
>
>
>> ---
>> 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
>
More information about the dri-devel
mailing list