Re: [PATCH] amd/display: remove ChromeOS workaround

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Simon,


Am 21.10.21 um 18:08 schrieb Simon Ser:

On Tuesday, October 19th, 2021 at 10:25, Paul Menzel wrote:

Am 19.10.21 um 10:10 schrieb Simon Ser:
On Tuesday, October 19th, 2021 at 01:21, Paul Menzel wrote:

Am 19.10.21 um 01:06 schrieb Simon Ser:
On Tuesday, October 19th, 2021 at 01:03, Paul Menzel wrote:

Excuse my ignorance. Reading the commit message, there was a Linux
kernel change, that broke Chrome OS userspace, right? If so, and we do
not know if there is other userspace using the API incorrectly,
shouldn’t the patch breaking Chrome OS userspace be reverted to adhere
to Linux’ no-regression rule?

No. There was a ChromeOS bug which has been thought to be an amdgpu bug. But
fixing that "bug" breaks other user-space.

Thank you for the explanation. I guess the bug was only surfacing
because Chrome OS device, like Chromebooks, are only using AMD hardware
since a short while (maybe last year).

Reading your message *amdgpu: atomic API and cursor/overlay planes* [1]
again, it says:

Up until now we were using cursor and overlay planes in gamescope [3],
but some changes in the amdgpu driver [1] makes us unable to use planes

So this statement was incorrect? Which changes are that? Or did Chrome
OS ever work correctly with an older Linux kernel or not?

The sequence of events is as follows:

- gamescope can use cursor and overlay planes.
- ChromeOS-specific commit lands, fixing some ChromeOS issues related to video
    playback. This breaks gamescope overlays.

I guess, I am confused, which Chrome OS specific commit that is. Is it
one of the reverted commits below? Which one?

1.  ddab8bd788f5 ("drm/amd/display: Fix two cursor duplication
when using overlay")
2.  e7d9560aeae5 ("Revert "drm/amd/display: Fix overlay validation by
considering cursors"")

ddab8bd788f5 ("drm/amd/display: Fix two cursor duplication when using overlay")
is the commit which introduced the validate_overlay logic fixing ChromeOS and
breaking gamescope.

Thank you for elaborating on this. I guess I mixed up Chrome OS and gamescope, and was especially confused, the commit message of commit ddab8bd788f5 not explicitly listing the problematic userspace. Despite the commit message being well written, this crucial information is missing.

Later, 33f409e60eb0 ("drm/amd/display: Fix overlay validation by considering
cursors") relaxed validate_overlay. This breaks ChromeOS and partially fixes
gamescope (when the overlay is used and the cursor plane is unused).

Finally, e7d9560aeae5 ("Revert "drm/amd/display: Fix overlay validation by
considering cursors"") has reverted that change, fixing ChromeOS (again) and
breaking gamescope completely again.

- Discussion to restrict the ChromeOS-specific logic to ChromeOS, or to revert
    it, either of these fix gamescope.

Given this, I don't see how the quoted statement is incorrect? Maybe I'm
missing something?

Your reply from August 2021 to commit ddab8bd788f5 (drm/amd/display: Fix
two cursor duplication when using overlay) from April 2021 [2]:

Hm. This patch causes a regression for me. I was using primary + overlay
not covering the whole primary plane + cursor before. This patch breaks it.

This patch makes the overlay plane very useless for me, because the primary
plane is always under the overlay plane.

So, I would have thought, everything worked fine before some Linux
kernel commit changed behavior, and regressed userspace.

I've tried to explain the full story above. My user-space went from working to
broken to partially broken to broken. The quoted reply is a complaint that the
commit flipped gamescope from partially broken to completely broken. At the
time I didn't realize that ddab8bd788f5 caused some pain too.

Does that clear things up?

Yes, it does. Thank you very much for taking the time for walking me through this.


Kind regards,

Paul



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux