I'm going to cc this to dri-devel, because this is something that would be of interest to upstream (you also should be doing the same, since this development should be happening upstream). Anyway, some more comments below: On Mon, 2020-11-09 at 11:49 -0500, Lyude Paul wrote: > On Mon, 2020-11-09 at 15:43 +0800, Koba Ko wrote: > Link: https://patchwork.freedesktop.org/series/77670/ > --- > Hi Lyude, > I'm encountering some issues for the lack of bandwidth on the mst hub. > The valid mode is shown in the edid list but after choosing it and the error > is prompted(Due to NOSPC). > I found your patches should be helpful for this kind of issue. > But I mentioned, > 1. Why does the patch choose "min_bpp=18" but not to choose the crtc_state- > >pipe_bpp > > because I haven't gotten around to adding the proper logic to the MST > helpers to actually optimize the bpp depending on how much bandwidth is > available, along with other constraints such as max_bpc. This is next up on > my todo list (we also need this for nouveau), but I've been a bit busy over > the last two weeks trying to complete some high priority stuff at work. In > general, there's a good number of things that still need to be done for MST: > we also don't support DSC for MST and need to add support for that into the > helpers, and as well we also need to write up support for fallback link > retraining (when we train an MST link at a certain link rate, but that > connection proves to be unreliable so we have to lower the link rate and > recalculate the settings for the display accordingly). > > 2. For the vcpi number, the patch doesn't check it > in intel_dp_mst_mode_valid_ctx(). > Even the mode needs the vcpi greater than 63 and it would be listed in > available EDID. > > Would the PBN check not handle this? Maybe I overlooked VCPI checks when I > wrote this, but generally if there's no possible way a mode could fit in 63 > VCPI, it also shouldn't be able to pass the PBN check. The VCPI checks in > the atomic check are moreso to prevent there from being too much bandwidth > being used on a single hub with multiple displays (which would be dependent > on the current state of other displays), but I'd be interested to know if > you found some situations where we're not filtering out always impossible > modes from intel_dp_mst_mode_valid_ctx(). > > Also, I think I'll have some time a little later this week so I start > writing up support for dynamic bpc selection if this is something you guys > need right now. (hopefully, at least :) actually-I just realized it might take me a little longer to get to it depending on what happens, as I've currently got some other stuff that's higher priority (getting DPCD backlights working on nouveau mainly, which I unfortunately have a deadline for). If you have the time to write the patches sooner I could probably help with reviewing them and other things - although it's a good bit more complicated then it might look, and will require having to pull a lot of logic out of amdgpu. amdgpu currently supports these things, but the agreement they had with upstream is that we'd eventually be moving out code for dynamically chosing bpc/dsc status for MST into the MST helpers so that both i915 and nouveau can benefit. The basic idea would be making it so that a driver first sets up it's initial VCPI allocations during the atomic check, using it's "ideal" settings by calling drm_dp_mst_atomic_find_vcpi_slots() and drm_dp_mst_atomic_release_vcpi_slots(). "Settings" here are vaguely defined as anything on the connector that would affect the bandwidth consumption of the mode we want to set, which in this case would just be the bpc and whether or not dsc is enabled. It probably would also make sense to start piping these to find_vcpi_slots()/release_vcpi_slots() for reasons I'll clarify in a moment. >From there, the driver would figure out what it wants for bpc/dsc as per usual, then run through the rest of the atomic check until the driver calls drm_dp_mst_atomic_check(). At this point if we're using too much bandwidth, drm_dp_mst_atomic_check() would let us know by returning -ENOSPC in order to indicate that we need to retry with more bandwidth conservative settings if possible. So, we'd redo the atomic check over again with a more conservative bpc while maybe also enabling dsc, see if that fits, then try raising the bandwidth usage of each connector gradually until we fail the drm_dp_mst_atomic_check() and come up with the most preferable settings we found. Then we'd trigger modesets on connectors that need to have modesets performed, etc. etc. Keep in mind that this might not be the exact algorithm we end up using, as other approaches such as starting with a display configuration using the smallest bandwidth possible then moving up from there in the atomic check could make more sense. Additionally, remember that dsc would definitely have to be taken into consideration here, since we'd want amdgpu to be using these helpers and we also don't want to suddenly drop DSC support for MST on their side (even if we don't have the code for DSC ready yet in drivers like nouveau). Just in case you're confused on why I keep mentioning it when you originally asked about bpp :). My hope would be that additionally by providing bpc and dsc settings through drm_dp_mst_atomic_find_vcpi_slots() we might be able to also teach the MST helpers to potentially suggest the most optimal display configuration given the bandwidth constraints, so that each driver using the helpers doesn't need to reimplement this. I'm not sure if this is a good idea yet though, and it'll likely be a lot more obvious once I've started writing the code and have more then just one driver using it. As well, there's some resources already for what we might want some of this behavior to look like. 8c20a1ed9b4ff25c67f6c1771a4e1195b5221cce, where amdgpu adds DSC fair share computation, does a pretty good job of describing the algorithm that amdgpu follows. We'll likely want to implement something similar to that, or just reuse the algorithm as-is in the DP MST helpers. Anyway-like I said it's a lot of work, if you're interested in getting a head start I'd be fine with that. But I'll definitely be getting to it in the near future, as it's something my employer needs me to implement :). Also if you do that, please put your WIP code on github or somewhere like that so I can keep up with it. I want to get working on this once I have the time asap, but I don't want to accidentally end up redoing your work. As well, we'd definitely want to make sure we're running our ideas by folks on intel-gfx as well to make sure we do this the right way. > > > Thanks > > -- > Cheers, > Lyude Paul (she/her) > Software Engineer at Red Hat -- 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
