[PATCH 05/18] fbdev: initialize yoffset of varinfo

[David Herrmann]

Hi

On Fri, Jun 21, 2013 at 11:43 AM, Marc Chalain <marc.chalain at gmail.com> wrote:
>
> 2013/6/21 David Herrmann <dh.herrmann at gmail.com>
>>
>> Hi
>>
>> On Fri, Jun 21, 2013 at 10:49 AM, mchalain [marc.chalain at gmail.com]
>> <marc.chalain at gmail.com> wrote:
>> > From: mchalain <marc.chalain at gmail.com>
>> >
>> >  it initializes varinfo.yoffset. varinfo.yoffset has to
>> >  point on the beginning of the video memory.
>> >  The card uses this value to push on the screen a part of
>> >  the video memory when this one is larger than the screen.
>> >
>> > ---
>> >  weston/src/compositor-fbdev.c |    6 ++++++
>> >  1 file changed, 6 insertions(+)
>> >
>> > diff --git a/weston/src/compositor-fbdev.c
>> > b/weston/src/compositor-fbdev.c
>> > index adfb67a..d2aee9b 100644
>> > --- a/weston/src/compositor-fbdev.c
>> > +++ b/weston/src/compositor-fbdev.c
>> > @@ -368,6 +368,11 @@ fbdev_query_screen_info(struct fbdev_output
>> > *output, int fd,
>> >                 return -1;
>> >         }
>> >
>> > +       if (varinfo.yoffset != 0) {
>> > +               varinfo.yoffset = 0;
>> > +               if (ioctl(fd, FBIOPAN_DISPLAY, &varinfo) < 0)
>> > +                       return -1;
>> > +       }
>>
>> Why do you need this? It's unnecessary. We call FBIOPUT_VSCREENINFO
>> after fbdev_query_screen_info(), anyway. Furthermore, not all drivers
>> support panning even though the yoffset may be non-zero (you need
>> FBIOPUT_VSCREENINFO then).
>>
>> I think you can just drop this here but keep the yoffset=0 below.
>>
>
> where do you use  FBIOPUT_VSCREENINFO ? I only find inside
> fbdev_set_screen_info but this function is called only when the output is
> reenable not at the startup.

My bad!
But still, you now call FBIOPAN_DISPLAY on _every_
fbdev_query_screen_info() while it is only needed during initial setup
as VT_ENTER calls fbdev_set_screen_info(), anyway. So instead, I'd
recommend to just call fbdev_set_screen_info() during initial setup as
well.

> It seems strange that FBIOPAN_DISPLAY is not supported by some devices. I
> used this one to not set all the varinfo on the device and to be faster.

See ./drivers/video/fbmem.c fb_pan_display(). Trivial fbdev drivers
might not support it, but user-space can still set the offset via
FBIOPUT_VSCREENINFO. If you want, you can call FBIOPAN_DISPLAY and if
it fails, you use FBIOPUT_VSCREENINFO.

> If yoffset may be non-zero this backend shouldn't work this kind of devices
> and we have to change the painting functions.

yoffset=0; is the correct thing to do.

Cheers
David

>> >         return 1;
>> >  }
>> >
>> > @@ -404,6 +409,7 @@ fbdev_set_screen_info(struct fbdev_output *output,
>> > int fd,
>> >         varinfo.blue.length = 8;
>> >         varinfo.blue.msb_right = 0;
>> >
>> > +       varinfo.yoffset = 0;
>>
>> Yep, this is definitely needed.
>>
>> Regards
>> David
>>
>> >         /* Set the device's screen information. */
>> >         if (ioctl(fd, FBIOPUT_VSCREENINFO, &varinfo) < 0) {
>> >                 return -1;
>> > --
>> > 1.7.9.5
>> >
>> > _______________________________________________
>> > wayland-devel mailing list
>> > wayland-devel at lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
>