On Fri, 2020-09-25 at 19:53 -0400, Ilia Mirkin wrote: > 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. I gotcha-I guess I was just confused because this is already possible with the current code we have (and it was also possible before we started adding these checks into ->mode_valid, which is why I need to get to the max_bpc thing soon). I guess I'll just use the connector's reported bpc for now until we add support for that > > Cheers, > > -ilia > -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
