[platforms/fbdev] Attempt to set the framebuffer color layout on the framebuffer device
ClosedPublic

Authored by nerdopolist on Dec 31 2017, 5:44 AM.

Details

Summary

Weston does something similar
https://cgit.freedesktop.org/wayland/weston/tree/libweston/compositor-fbdev.c#n315
...and it seems that the non-primary framebuffer devices start completely off. (the screen is off on my Displayport device, and the window for the second vga card in qemu is much smaller than the primary one, and all black for my qemu vm) .

In my testing, sending the ioctl FBIOPUT_VSCREENINFO with a *changed* &varinfo allows it to wake up, and turns on the screen.
doing FBIOGET_VSCREENINFO and then FBIOPUT_VSCREENINFO in my testing does not work.

I think really the values that end up getting changed are varinfo.transp.offset and varinfo.transp.length. at least on the qemu system, but in this patch I am aligning all of them for completeness, because the drivers might do it differently for qemu

Test Plan

This causes the window for /dev/fb1 (remote-viewer, and a qemu vm with two "VGA" (bochsdrm) cards) to resize from the smaller size when specifying it as the --fb-device on seat0, and kwin draws on the device.

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
nerdopolist created this revision.Dec 31 2017, 5:44 AM
Restricted Application added a subscriber: kwin. · View Herald TranscriptDec 31 2017, 5:44 AM
nerdopolist requested review of this revision.Dec 31 2017, 5:44 AM

@nerdopolist Please rebase on latest master and update authorship information on Phab:

git fetch
git checkout 50b948134c81
arc patch D9572
git rebase origin/master
# Check if it still compiles
arc diff

@graesslin Friendly ping to review the code afterwards ;)

nerdopolist updated this revision to Diff 31457.Apr 6 2018, 1:56 AM
nerdopolist edited the summary of this revision. (Show Details)

rebase

davidedmundson requested changes to this revision.Apr 19 2018, 10:16 PM
davidedmundson added a subscriber: davidedmundson.

Kinda makes sense, though the weston code also set the resolution and bpp.

I don't see how we have a situation where half the config is correct, but half apparently can't be trusted and should be overwritten (without even checking if it was a valid colour layout first)

plugins/platforms/fbdev/fb_backend.cpp
115

A method called query should not be setting things.

146

why would fixinfo be different?

This revision now requires changes to proceed.Apr 19 2018, 10:16 PM

I might need to research the best way to do this (although I really had bad luck finding info)... This more that secondary framebuffer devices, seem to initialize with the screens off (such as udlfb/displaylink devices) sending the ioctl, seems to be the only way to turn the screen on. (and IIRC, it's due to some attribute changing, (at this moment, I forget the exact one)

Try to adjust for requested changes

rebase the requested changes

nerdopolist marked 2 inline comments as done.Apr 25 2018, 2:59 AM
nerdopolist added inline comments.
plugins/platforms/fbdev/fb_backend.cpp
115

would it be acceptable if I renamed it to
handleScreenInfo
?

nerdopolist marked an inline comment as done.May 2 2018, 12:11 AM

Let me know if I need to make any more changes?

Let me know if I need to change this one?

graesslin accepted this revision.May 20 2018, 5:28 PM
rkflx added a comment.May 23 2018, 9:17 PM

@davidedmundson As you "requested changes", are those implemented now?

I'd like to know if I need to change anything?

Let me know if the change I made was good enough?

rkflx added a comment.Jun 22 2018, 5:45 PM
In D9572#267372, @rkflx wrote:

@davidedmundson As you "requested changes", are those implemented now?

@nerdopolist You could try pinging David (d_ed) on IRC or asking in the #kwin channel.

In D9572#281886, @rkflx wrote:
In D9572#267372, @rkflx wrote:

@davidedmundson As you "requested changes", are those implemented now?

@nerdopolist You could try pinging David (d_ed) on IRC or asking in the #kwin channel.

Ok. I'll try that.

Any chance that this can be reviewed? I tried to address the requested changes

graesslin accepted this revision. May 20 2018, 6:28 PM

Sure, just push.

I don't think I have commit access to push that though? Is there something I need to do?

zzag added a subscriber: zzag.Aug 12 2018, 2:34 PM

Please replace 'framebuffer:' with '[platforms/fbdev]', so the title looks like

[platforms/fbdev] Attempt to set the framebuffer color layout on the framebuffer device

nerdopolist retitled this revision from framebuffer: Attempt to set the framebuffer color layout on the framebuffer device to [platforms/fbdev] Attempt to set the framebuffer color layout on the framebuffer device.Aug 12 2018, 11:21 PM
In D9572#307162, @zzag wrote:

Please replace 'framebuffer:' with '[platforms/fbdev]', so the title looks like

[platforms/fbdev] Attempt to set the framebuffer color layout on the framebuffer device

Thanks. Done.

zzag added a comment.Aug 13 2018, 12:29 PM

I don't think I have commit access to push that though? Is there something I need to do?

I can push this patch for you. Could you please provide you real name?

This revision was not accepted when it landed; it landed in state Needs Review.Aug 13 2018, 2:37 PM
This revision was automatically updated to reflect the committed changes.

Thanks for your patience here, @nerdopolist.

Thanks for merging!