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; >