Re: [PATCH] Revert "drm/display/dp_mst: Move all payload info into the atomic state"

["Limonciello, Mario" <mario.limonciello@amd.com>]

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.
>>>