| Message ID | 1661830389-22439-4-git-send-email-quic_abhinavk@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Limit pluggable display modes | expand |
Quoting Abhinav Kumar (2022-08-29 20:33:09) > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c > index bfd0aeff3f0d..8b91d8adf921 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -117,6 +117,7 @@ struct dp_display_private { > > bool wide_bus_en; > > + int max_ext_pclk; Same signed comment as before. > struct dp_audio *audio; > }; > > @@ -986,8 +987,15 @@ enum drm_mode_status dp_bridge_mode_valid(struct drm_bridge *bridge, > if (dp->is_edp) > return MODE_OK; > > - if (mode->clock > DP_MAX_PIXEL_CLK_KHZ) > - return MODE_CLOCK_HIGH; > + /* > + * If DP/eDP supports HPD natively or through a bridge, need to make > + * sure that we filter out the modes with pixel clock higher than the > + * chipset capabilities > + */ > + if ((bridge->ops & DRM_BRIDGE_OP_HPD) || > + (dp->next_bridge && (dp->next_bridge->ops & DRM_BRIDGE_OP_HPD))) Doesn't drm_bridge_chain_mode_valid() already run the mode_valid bridge function for all bridges in the chain? I don't understand why we need to peek at the bridge or the next bridge and only filter in that case. Why can't we always filter modes here? Do we want to allow higher pixel clks than the chip supports? > + if (mode->clock > dp_display->max_ext_pclk) > + return MODE_CLOCK_HIGH; >
On 9/7/2022 6:12 PM, Stephen Boyd wrote: > Quoting Abhinav Kumar (2022-08-29 20:33:09) >> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c >> index bfd0aeff3f0d..8b91d8adf921 100644 >> --- a/drivers/gpu/drm/msm/dp/dp_display.c >> +++ b/drivers/gpu/drm/msm/dp/dp_display.c >> @@ -117,6 +117,7 @@ struct dp_display_private { >> >> bool wide_bus_en; >> >> + int max_ext_pclk; > > Same signed comment as before. This is in Khz. It cannot be negative. I was also thinking of an unsigned int for this but the drm_display_mode's clock is an int so i also kept it like this. 227 struct drm_display_mode { 228 /** 229 * @clock: 230 * 231 * Pixel clock in kHz. 232 */ 233 int clock; /* in kHz */ 234 u16 hdisplay; > >> struct dp_audio *audio; >> }; >> >> @@ -986,8 +987,15 @@ enum drm_mode_status dp_bridge_mode_valid(struct drm_bridge *bridge, >> if (dp->is_edp) >> return MODE_OK; >> >> - if (mode->clock > DP_MAX_PIXEL_CLK_KHZ) >> - return MODE_CLOCK_HIGH; >> + /* >> + * If DP/eDP supports HPD natively or through a bridge, need to make >> + * sure that we filter out the modes with pixel clock higher than the >> + * chipset capabilities >> + */ >> + if ((bridge->ops & DRM_BRIDGE_OP_HPD) || >> + (dp->next_bridge && (dp->next_bridge->ops & DRM_BRIDGE_OP_HPD))) > > Doesn't drm_bridge_chain_mode_valid() already run the mode_valid bridge > function for all bridges in the chain? I don't understand why we need to > peek at the bridge or the next bridge and only filter in that case. Why > can't we always filter modes here? Do we want to allow higher pixel clks > than the chip supports? The next bridge does not know about the max capability of the previous bridge. If the DP is used as a built-in display, we dont want this filter. If the DP is used as the external display either directly or with the next/last bridge as the pluggable one, then we want to apply this filter. The reason we cant always filter modes here is that lets say the DP is being used as a built-in display, then this filter is not needed. Now coming to the HPD part of the next bridge, its not necessary that we use the DP bridge's HPD. We could be using the external bridge's HPD. Like the DSI patch, I should change this to check the last bridge's HPD bridge op. > >> + if (mode->clock > dp_display->max_ext_pclk) >> + return MODE_CLOCK_HIGH; >>
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index e6f7e07fd2a6..7857ce58b615 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -614,7 +614,8 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev, static int _dpu_kms_initialize_displayport(struct drm_device *dev, struct msm_drm_private *priv, - struct dpu_kms *dpu_kms) + struct dpu_kms *dpu_kms, + int max_ext_pclk) { struct drm_encoder *encoder = NULL; struct msm_display_info info; @@ -632,7 +633,7 @@ static int _dpu_kms_initialize_displayport(struct drm_device *dev, } memset(&info, 0, sizeof(info)); - rc = msm_dp_modeset_init(priv->dp[i], dev, encoder); + rc = msm_dp_modeset_init(priv->dp[i], dev, encoder, max_ext_pclk); if (rc) { DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc); drm_encoder_cleanup(encoder); @@ -715,7 +716,7 @@ static int _dpu_kms_setup_displays(struct drm_device *dev, return rc; } - rc = _dpu_kms_initialize_displayport(dev, priv, dpu_kms); + rc = _dpu_kms_initialize_displayport(dev, priv, dpu_kms, max_ext_pclk); if (rc) { DPU_ERROR("initialize_DP failed, rc = %d\n", rc); return rc; diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index bfd0aeff3f0d..8b91d8adf921 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -117,6 +117,7 @@ struct dp_display_private { bool wide_bus_en; + int max_ext_pclk; struct dp_audio *audio; }; @@ -986,8 +987,15 @@ enum drm_mode_status dp_bridge_mode_valid(struct drm_bridge *bridge, if (dp->is_edp) return MODE_OK; - if (mode->clock > DP_MAX_PIXEL_CLK_KHZ) - return MODE_CLOCK_HIGH; + /* + * If DP/eDP supports HPD natively or through a bridge, need to make + * sure that we filter out the modes with pixel clock higher than the + * chipset capabilities + */ + if ((bridge->ops & DRM_BRIDGE_OP_HPD) || + (dp->next_bridge && (dp->next_bridge->ops & DRM_BRIDGE_OP_HPD))) + if (mode->clock > dp_display->max_ext_pclk) + return MODE_CLOCK_HIGH; dp_display = container_of(dp, struct dp_display_private, dp_display); link_info = &dp_display->panel->link_info; @@ -1587,7 +1595,7 @@ static int dp_display_get_next_bridge(struct msm_dp *dp) } int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev, - struct drm_encoder *encoder) + struct drm_encoder *encoder, int max_ext_pclk) { struct msm_drm_private *priv; struct dp_display_private *dp_priv; @@ -1599,6 +1607,8 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev, priv = dev->dev_private; dp_display->drm_dev = dev; + dp_priv->max_ext_pclk = max_ext_pclk; + dp_priv = container_of(dp_display, struct dp_display_private, dp_display); ret = dp_display_request_irq(dp_display); diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h index 866c1a82bf1a..c94b793027a2 100644 --- a/drivers/gpu/drm/msm/dp/dp_parser.h +++ b/drivers/gpu/drm/msm/dp/dp_parser.h @@ -13,7 +13,6 @@ #include "msm_drv.h" #define DP_LABEL "MDSS DP DISPLAY" -#define DP_MAX_PIXEL_CLK_KHZ 675000 #define DP_MAX_NUM_DP_LANES 4 enum dp_pm_type { diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 44d882b04327..39e8cdde6152 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -329,7 +329,7 @@ static inline struct drm_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi *msm_ 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); + struct drm_encoder *encoder, int max_ext_pclk); 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); @@ -346,7 +346,8 @@ static inline void __exit msm_dp_unregister(void) } static inline int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev, - struct drm_encoder *encoder) + struct drm_encoder *encoder, + int max_ext_pclk) { return -EINVAL; }
Filter out DP/eDP modes having an unsupported pixel clock by replacing the current hard-coded limit with the per chipset advertised value. Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 7 ++++--- drivers/gpu/drm/msm/dp/dp_display.c | 16 +++++++++++++--- drivers/gpu/drm/msm/dp/dp_parser.h | 1 - drivers/gpu/drm/msm/msm_drv.h | 5 +++-- 4 files changed, 20 insertions(+), 9 deletions(-)