Skip to content
Snippets Groups Projects

kde/kwin: revert commit to fix blue screen in oneplus-bacon

Closed Imported Administrator requested to merge kwin-framebuffer-fix into master

The reverted commit hardcoded framebuffer parameters causing display problems on oneplus-one
See issue #204 (closed) Reverting this commit fixes the problem.

Edited by Administrator

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Administrator mentioned in issue #204 (closed) · Imported

    mentioned in issue #204 (closed)

    By Federico Amedeo Izzo on 2019-03-08T13:58:02

  • Administrator changed the description · Imported

    changed the description

    By Bart Ribbers on 2019-03-09T18:31:51

  • Author Owner

    Looks good to me, it's amazing that you could figure out what caused the breakage, and were able to rever the related commit so quickly. Thanks a bunch @Nimayer!

    Slightly delaying the merge, because @bshah (who's involved with Plasma Mobile) said he would take a look at this tomorrow.

    By Oliver Smith on 2019-03-09T18:36:28

  • Author Owner

    https://github.com/KDE/kwin/commit/304528e80b935efea05e2d5e3030266e0eddc44c see discussion here, I think it's best if proper fix is upstreamed then random revert here.

    By Bhushan Shah on 2019-03-10T05:12:42

  • Author Owner

    @bshah I agree that a proper fix is better than just reverting the commit.

    From what i understood that commit is writing framebuffer parameters to wake up a secondary screen in QEMU.
    The problem is that doing so, is overwriting the correct framebuffer settings with some hardcoded ones (like RGB32 pixel type instead of RGB_888).

    I think the best option is figure out a different way of fixing the QEMU screen, but I do not know how to do that.

    By Federico Amedeo Izzo on 2019-03-10T09:34:27

  • Author Owner

    I guess, the proper fix is, as mentioned in commit discussion, there - https://cgit.freedesktop.org/wayland/weston/commit/?id=fa2742b3808fc06ccea9cbe9b9fd470fdc4dfec8

    Get var info and reapply it with FB_ACTIVATE_NOW | FB_ACTIVATE_FORCE, if I understand it right.

    By Alexey Min on 2019-03-10T10:03:14

    Edited by Administrator
  • Author Owner

    @minlexx Thank you, that seems right.

    I would prefer to leave the proper implementation to someone with more knowledge about kwin.
    Can we keep the revert patch in pmos for the moment and suggest the change to someone involved with kwin?

    We can remove the revert patch once the problem is fixed upstream

    By Federico Amedeo Izzo on 2019-03-10T13:13:01

  • Author Owner

    I created https://phabricator.kde.org/D19663 to try to correct it this time.

    By n3rdopolis on 2019-03-10T22:27:56

  • Author Owner

    I would prefer to leave the proper implementation to someone with more knowledge about kwin.

    Thanks to @n3rdopolis's fast reaction, there is a good patch now, which will likely land in upstream soon :tada:

    Can we keep the revert patch in pmos for the moment and suggest the change to someone involved with kwin?

    @Nimayer: How about you try the new patch, and if it works, put it into this merge request instead of reverting the old one?

    Here it is in diff format, so it can be saved as .patch file and applied in the package: https://phabricator.kde.org/file/data/ncgrnh3gd53n6gocanua/PHID-FILE-kmpksrdru6b57sk5ank7/D19663.diff

    Then we would remove it with the next version upgrade, which contains the patch.

    By Oliver Smith on 2019-03-11T06:32:35

  • Author Owner

    Thanks @n3rdopolis for the patch! and @ollieparanoid for the diff,
    I will try it on Bacon soon and if it works, will replace the revert patch with the commit from @n3rdopolis

    By Federico Amedeo Izzo on 2019-03-12T11:51:59

  • Author Owner

    np!
    FYI, the patch is now merged into kwin master

    By n3rdopolis on 2019-03-14T00:12:18

  • Author Owner

    Yesterday I tested D19663 and it seems to work, no blue screen any more:

    image image

    But also as it can be seen here red color is swapped with blue, this may be another problem with msm fb driver..)

    By Alexey Min on 2019-03-14T08:01:20

  • Author Owner

    Thanks for trying the patch. Yes, the color swap is a different problem, see here for related information.

    Would you mind updating this merge request to use the upstream patch?

    By Oliver Smith on 2019-03-15T08:00:46

  • Administrator mentioned in merge request !279 (merged) · Imported

    mentioned in merge request !279 (merged)

    By Federico Amedeo Izzo on 2019-03-15T10:51:33

  • Author Owner

    Sorry for the delay.
    I've tried out the patch by @n3rdopolis, and it works fine on oneplus-bacon as @minlexx found out.

    Since I can't modify the source branch for this MR, I've created the MR !279 (merged) with the patch that applies the upstream commit 93b7eea67db418751e7fe4a86bc19430c153588b.

    NOTE: @minlexx I've also created the issue #211 (closed) to investigate the red/blue swapped problem

    Thank you all for the support, I will close this MR and I hope MR !279 (merged) can be merged soon.

    By Federico Amedeo Izzo on 2019-03-15T11:02:01

  • Administrator closed · Imported

    closed

    By Federico Amedeo Izzo on 2019-03-15T11:02:02

Please register or sign in to reply
Loading