Phillip, Can you test this patch? I was not able to repro the issue on the navi2x card I had handy, but I think it should fix it.
Thanks, Alex On Wed, Nov 29, 2023 at 3:49 PM Alex Deucher <alexdeuc...@gmail.com> wrote: > > On Wed, Nov 29, 2023 at 3:10 PM Alex Deucher <alexdeuc...@gmail.com> wrote: > > > > Actually I think I see the problem. I'll try and send out a patch > > later today to test. > > Does the attached patch fix it? > > Alex > > > > > Alex > > > > On Wed, Nov 29, 2023 at 1:52 PM Alex Deucher <alexdeuc...@gmail.com> wrote: > > > > > > On Wed, Nov 29, 2023 at 11:41 AM Luben Tuikov <ltuiko...@gmail.com> wrote: > > > > > > > > On 2023-11-29 10:22, Alex Deucher wrote: > > > > > On Wed, Nov 29, 2023 at 8:50 AM Alex Deucher <alexdeuc...@gmail.com> > > > > > wrote: > > > > >> > > > > >> On Tue, Nov 28, 2023 at 11:45 PM Luben Tuikov <ltuiko...@gmail.com> > > > > >> wrote: > > > > >>> > > > > >>> On 2023-11-28 17:13, Alex Deucher wrote: > > > > >>>> On Mon, Nov 27, 2023 at 6:24 PM Phillip Susi <ph...@thesusis.net> > > > > >>>> wrote: > > > > >>>>> > > > > >>>>> Alex Deucher <alexdeuc...@gmail.com> writes: > > > > >>>>> > > > > >>>>>>> In that case those are the already known problems with the > > > > >>>>>>> scheduler > > > > >>>>>>> changes, aren't they? > > > > >>>>>> > > > > >>>>>> Yes. Those changes went into 6.7 though, not 6.6 AFAIK. Maybe > > > > >>>>>> I'm > > > > >>>>>> misunderstanding what the original report was actually testing. > > > > >>>>>> If it > > > > >>>>>> was 6.7, then try reverting: > > > > >>>>>> 56e449603f0ac580700621a356d35d5716a62ce5 > > > > >>>>>> b70438004a14f4d0f9890b3297cd66248728546c > > > > >>>>> > > > > >>>>> At some point it was suggested that I file a gitlab issue, but I > > > > >>>>> took > > > > >>>>> this to mean it was already known and being worked on. -rc3 came > > > > >>>>> out > > > > >>>>> today and still has the problem. Is there a known issue I could > > > > >>>>> track? > > > > >>>>> > > > > >>>> > > > > >>>> At this point, unless there are any objections, I think we should > > > > >>>> just > > > > >>>> revert the two patches > > > > >>> Uhm, no. > > > > >>> > > > > >>> Why "the two" patches? > > > > >>> > > > > >>> This email, part of this thread, > > > > >>> > > > > >>> https://lore.kernel.org/all/87r0kircdo....@vps.thesusis.net/ > > > > >>> > > > > >>> clearly states that reverting *only* this commit, > > > > >>> 56e449603f0ac5 drm/sched: Convert the GPU scheduler to variable > > > > >>> number of run-queues > > > > >>> *does not* mitigate the failed suspend. (Furthermore, this commit > > > > >>> doesn't really change > > > > >>> anything operational, other than using an allocated array, instead > > > > >>> of a static one, in DRM, > > > > >>> while the 2nd patch is solely contained within the amdgpu driver > > > > >>> code.) > > > > >>> > > > > >>> Leaving us with only this change, > > > > >>> b70438004a14f4 drm/amdgpu: move buffer funcs setting up a level > > > > >>> to be at fault, as the kernel log attached in the linked email > > > > >>> above shows. > > > > >>> > > > > >>> The conclusion is that only b70438004a14f4 needs reverting. > > > > >> > > > > >> b70438004a14f4 was a fix for 56e449603f0ac5. Without b70438004a14f4, > > > > >> 56e449603f0ac5 breaks amdgpu. > > > > > > > > > > We can try and re-enable it in the next kernel. I'm just not sure > > > > > we'll be able to fix this in time for 6.7 with the holidays and all > > > > > and I don't want to cause a lot of scheduler churn at the end of the > > > > > 6.7 cycle if we hold off and try and fix it. Reverting seems like the > > > > > best short term solution. > > > > > > > > A lot of subsequent code has come in since commit 56e449603f0ac5, as it > > > > opened > > > > the opportunity for a 1-to-1 relationship between an entity and a > > > > scheduler. > > > > (Should've always been the case, from the outset. Not sure why it was > > > > coded as > > > > a fixed-size array.) > > > > > > > > Given that commit 56e449603f0ac5 has nothing to do with amdgpu, and the > > > > problem > > > > is wholly contained in amdgpu, and no other driver has this problem, > > > > there is > > > > no reason to have to "churn", i.e. go back and forth in DRM, only to > > > > cover up > > > > an init bug in amdgpu. See the response I just sent in @this thread: > > > > https://lore.kernel.org/r/05007cb0-871e-4dc7-af58-1351f4ba4...@gmail.com > > > > > > > > And it's not like this issue is unknown. I first posted about it on > > > > 2023-10-16. > > > > > > > > Ideally, amdgpu would just fix their init code. > > > > > > You can't make changes to core code that break other drivers. > > > Arguably 56e449603f0ac5 should not have gone in in the first place if > > > it broke amdgpu. b70438004a14f4 was the code to fix amdgpu's init > > > code, but as a side effect it seems to have broken suspend for some > > > users. > > > > > > Alex
