On 9/10/2026 3:51 PM, Igor Paunovic wrote: > Hi Chaoyi, > > On 9/10/26 09:16, Chaoyi Chen wrote: >> I don't think this is the correct way to go. >> >> I believe max_bpc should be handled in drm_bridge_connector_init(). >> Take a look at its implementation. HDMI already does max_bpc there, >> so DP should probably do something similar. > > Agreed - that is the question I left open in my reply to the bot. v3 > moves it into drm_bridge_connector_init() and drops this glue patch. > > Three points I would rather ask than guess, since it is shared code: > > 1. How the bridge advertises it. drm_bridge.max_bpc is documented as > "only relevant if DRM_BRIDGE_OP_HDMI is set" and the helper only > reads it inside the OP_HDMI block. I intend to honour it on the > non-HDMI path too, relax that doc sentence, and have dw-dp set > bridge->max_bpc = 10. If you prefer to keep the field HDMI-only, > a separate field is easy - I just did not want to add one unasked. >
Maybe we should consider adding a new "DRM_BRIDGE_OP_DP" :) > 2. The range. HDMI attaches 8..max_bpc. dw-dp's format table goes down > to 6 bpc RGB and i915 attaches 6..N on its DP connectors, so I intend > 6..max_bpc when connector_type is DisplayPort, 8..max_bpc otherwise - > the same gate Dmitry used for the DP subconnector property in his > msm series [1]. Nothing is attached unless a bridge declared > max_bpc, so existing non-HDMI bridge connectors are unchanged. > And once you use DRM_BRIDGE_OP_DP for the check, it shouldn't affect other users. The max_bpc looks reasonable. > 3. Where the default lives. 2/3 restores max_requested_bpc from > connector->max_bpc after drm_mode_config_reset(), but Cristian's > v11 71/74 removes that field [2] (HDMI keeps it in hdmi.funcs). I > intend to keep the value in struct drm_bridge_connector and restore > it from there in drm_bridge_connector_create_state(), which folds > 2/3 into the same patch. 1/3 is unaffected. > > The state still has to exist before the property is attached, so the > non-HDMI path gets the same create-state block drmm_connector_hdmi_init() > has. If you would rather see that block factored into a helper for both > callers, say so and I will do that instead. > Perhaps @Cristian and @Maxime have better ideas? > [1] > https://lore.kernel.org/all/20260722-drm-msm-display-interface-v1-24-368c10fe6...@oss.qualcomm.com/ > [2] > https://lore.kernel.org/all/[email protected]/ > > Thanks, > Igor -- Best, Chaoyi
