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;
