On Fri, Sep 25, 2020 at 6:08 PM Lyude Paul <lyude@xxxxxxxxxx> wrote: > > On Tue, 2020-09-22 at 17:22 -0400, Ilia Mirkin wrote: > > On Tue, Sep 22, 2020 at 5:14 PM Lyude Paul <lyude@xxxxxxxxxx> wrote: > > > On Tue, 2020-09-22 at 17:10 -0400, Ilia Mirkin wrote: > > > > Can we use 6bpc on arbitrary DP monitors, or is there a capability for > > > > it? Maybe only use 6bpc if display_info.bpc == 6 and otherwise use 8? > > > > > > I don't think that display_info.bpc actually implies a minimum bpc, only a > > > maximum bpc iirc (Ville would know the answer to this one). The other thing > > > to > > > note here is that we want to assume the lowest possible bpc here since we're > > > only concerned if the mode passed to ->mode_valid can be set under -any- > > > conditions (including those that require lowering the bpc beyond it's > > > maximum > > > value), so we definitely do want to always use 6bpc here even once we get > > > support for optimizing the bpc based on the available display bandwidth. > > > > Yeah, display_info is the max bpc. But would an average monitor > > support 6bpc? And if it does, does the current link training code even > > try that when display_info.bpc != 6? > > So I did confirm that 6bpc support is mandatory for DP, so yes-6 bpc will always > work. > > But also, your second comment doesn't really apply here. So: to be clear, we're > not really concerned here about whether nouveau will actually use 6bpc or not. > In truth I'm not actually sure either if we have any code that uses 6bpc (iirc > we don't), since we don't current optimize bpc. I think it's very possible for > us to use 6bpc for eDP displays if I recall though, but I'm not sure on that. > > But that's also not the point of this code. ->mode_valid() is only used in two > situations in DRM modesetting: when probing connector modes, and when checking > if a mode is valid or not during the atomic check for atomic modesetting. Its > purpose is only to reject display modes that are physically impossible to set in > hardware due to static hardware constraints. Put another way, we only check the > given mode against constraints which will always remain constant regardless of > the rest of the display state. An example of a static constraint would be the > max pixel clock supported by the hardware, since on sensible hardware this never > changes. A dynamic constraint would be something like how much bandwidth is > currently unused on an MST topology, since that value is entirely dependent on > the rest of the display state. > > So - with that said, bpc is technically a dynamic constraint because while a > sink and source both likely have their own bpc limits, any bpc which is equal or > below that limit can be used depending on what the driver decides - which will > be based on the max_bpc property, and additionally for MST displays it will also > depend on the available bandwidth on the topology. The only non-dynamic thing > about bpc is that at a minimum, it will be 6 - so any mode that doesn't fit on > the link with a bpc of 6 is guaranteed to be a mode that we'll never be able to > set and therefore want to prune. > > So, even if we're not using 6 in the majority of situations, I'm fairly > confident it's the right value here. It's also what i915 does as well (and they > previously had to fix a bug that was the result of assuming a minimum of 8bpc > instead of 6). Here's the situation I'm trying to avoid: 1. Mode is considered "OK" from a bandwidth perspective @6bpc 2. Modesetting logic never tries 6bpc and the modeset fails As long as the two bits of logic agree on what is settable, I'm happy. Cheers, -ilia _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
