Radeon regression in 6.6 kernel

[Alex Deucher]

On Wed, Nov 29, 2023 at 3:10 PM Alex Deucher <alexdeucher at 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 <alexdeucher at gmail.com> wrote:
> >
> > On Wed, Nov 29, 2023 at 11:41 AM Luben Tuikov <ltuikov89 at gmail.com> wrote:
> > >
> > > On 2023-11-29 10:22, Alex Deucher wrote:
> > > > On Wed, Nov 29, 2023 at 8:50 AM Alex Deucher <alexdeucher at gmail.com> wrote:
> > > >>
> > > >> On Tue, Nov 28, 2023 at 11:45 PM Luben Tuikov <ltuikov89 at gmail.com> wrote:
> > > >>>
> > > >>> On 2023-11-28 17:13, Alex Deucher wrote:
> > > >>>> On Mon, Nov 27, 2023 at 6:24 PM Phillip Susi <phill at thesusis.net> wrote:
> > > >>>>>
> > > >>>>> Alex Deucher <alexdeucher at 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.fsf@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-1351f4ba43e2@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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-drm-amdgpu-fix-buffer-funcs-setting-order-on-suspend.patch
Type: text/x-patch
Size: 1441 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20231129/0ba166b5/attachment.bin>