On 1/13/2023 13:28, Lyude Paul wrote:
On Fri, 2023-01-13 at 11:25 +0100, Daniel Vetter wrote:
On Fri, Jan 13, 2023 at 12:16:57PM +0200, Jani Nikula wrote:
Cc: intel-gfx, drm maintainers
Please have the courtesy of Cc'ing us for changes impacting us, and
maybe try to involve us earlier instead of surprising us like
this. Looks like this has been debugged for at least three months, and
the huge revert at this point is going to set us back with what's been
developed on top of it for i915 DP MST and DSC.
tbf I assumed this wont land when I've seen it fly by. It feels a bit much
like living under a rock for half a year and then creating a mess for
everyone else who's been building on top of this is not great.
Like yes it's a regression, but apparently not a blantantly obvious one,
and I think if we just ram this in there's considerable community goodwill
down the drain. Someone needs to get that goodwill up the drain again.
It's a regression, I get that, but this is also going to be really nasty
to deal with. It's a 2500-line commit, plus the dependencies, which I
don't think are accounted for here. (What's the baseline for the revert
anyway?) I don't expect all the dependent commits to be easy to revert
or backport to v6.1 or v6.2.
*sad trombone*
Yeah that's the other thing. 2500 patch revert is not cc stable material.
So this isn't even really helping users all that much.
Unless it also comes with full amounts of backports of the reverts on all
affected drivers for all curent stable trees, fully validated.
The silver lining here is that in terms of how many stable trees this is
broken it's only 6.1.y.
Wayne's revert is against drm-tip.
I found that attempting backporting his revert I run into
conflicts in 6.2-rc3 because of:
831a277ef001 ("Revert "drm/i915: Extract drm_dp_atomic_find_vcpi_slots
cycle to separate function"")
I worked through them and have the result here:
https://gitlab.freedesktop.org/superm1/linux/-/commit/8e926eb77c41e7f32f3d8943cdf7d140ed319b79
and conflicts in 6.1.y from the lack of:
8c7d980da9ba ("drm/nouveau/disp: move DP MST payload config method")
I worked through those as well and the result is here:
https://gitlab.freedesktop.org/superm1/linux/-/commit/2145b4de3fea9908cda6bef0693a797cc7f4ddfc
Affected people in Gitlab #2171 have tested amdgpu works w/ MST again
with 6.1.5 with that applied.
To your point, we do need someone with the appropriate hardware to make
sure we didn't make i915 or nouveau worse by this revert though.
This is bad. I do think we need to have some understanding first of what
"fix this in amdgpu" would look like as plan B. Because plan A does not
look like a happy one at all.
Yeah this whole thing has been a mess, I'm partially to blame here - we should
have reverted earlier, but a lot of this has been me finding out that the
problem here is a lot bigger then I previously imagined - and has not at all
been easy to untangle. I've also dumped so much time into trying to figure it
out that was more or less the only reason I acked this in the first place, I'm
literally just quite tired and exhausted at this point from spinning my wheels
on trying to fix this ;_;.
I am sure there is a real proper fix for this, if anyone wants to help me try
and figure this out I'm happy to setup remote access to the machines I've got
here. I'll also try to push myself to dig further into this next week again.
-Daniel
BR,
Jani.
