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

[Marc Chalain]

2013/6/21 David Herrmann <dh.herrmann at gmail.com>

> 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.
>
>
No I call  FBIOPAN_DISPLAY only at startup after yoffset is set to 0

> 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.
>
> I read the yoffset hasn't to be null in  FB_VMODE_YWRAP . But yoffset is
only used for panning.
It's useless to set it if it doesn't use (fbmem.c uses yoffset only for
panning in your example too)

> 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
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130621/7e614f3f/attachment-0001.html>