RE: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl

RE: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl

[Hiremath, Vaibhav]

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@nokia.com]
> Sent: Wednesday, November 24, 2010 10:01 PM
> To: Hiremath, Vaibhav
> Cc: Tomi Valkeinen; linux-omap@vger.kernel.org
> Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> 
> On Wed, Nov 24, 2010 at 03:39:44PM +0530, ext Hiremath, Vaibhav wrote:
> >
> > > -----Original Message-----
> > > From: Tomi Valkeinen [mailto:tomi.valkeinen@nokia.com]
> > > Sent: Wednesday, November 24, 2010 2:28 PM
> > > To: Hiremath, Vaibhav
> > > Cc: linux-omap@vger.kernel.org
> > > Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> > >
> > > On Tue, 2010-11-23 at 23:46 +0530, ext Hiremath, Vaibhav wrote:
> > > > Hi,
<snip>
> >
> > > Changing it to WAITFORGO would alter the behaviour. Sometimes it would
> > > not wait at all, sometimes it could wait for multiple vsyncs.
> > [Hiremath, Vaibhav] I am quite not sure, whether it makes sense from
> application point of view to wait for vsync if config is not changed. What
> could be the use-case for such requirement, where application won't change
> anything but still would like to wait on vsync?
> >
> > And as far as wait on multiple vsync is concerned, it does make sense to
> coat "WAITFORVSYNC ioctl makes sure that your change got applied to HW".
> >
> > I am not aware of other architectures, but I believe this is something
> OMAP specific stuff. And for other platforms, WAITFORVSYNC means changes
> applied to HW (why does apps care about whether it is vsync or anything
> else?).
> 
> WAITFORVSYNC waits for the next vsync (or actually vblank in many
> cases). 
[Hiremath, Vaibhav] Please note that this doesn't include VFP (vertical front porch), since you are going to get VSYNC only after VFP.

> That's it. I don't see any point in trying to shoehorn
> other functionality into it.
> 
> If you want to standardise WAITFORGO type of stuff then just add
> another standard ioctl for it.
> 
[Hiremath, Vaibhav] I still believe we should not only look at the name of IOCTL (WAITFORVSYNC) and define what it should do, it may change based on your platform/HW to get the desired functionality out of it.

I am putting fbdev mailing list in CC to get comments from other folks on what is expected from WAITFORVSYNC, especially architectures like
	- s3c-fb.c
	- ps3fb.c
	- atyfb_base.c
	- matroxfb_crtc2.c
	- sh_mobile_lcdcfb.c

Please refer the wiki page which explains the OMAP DSS issue - http://processors.wiki.ti.com/index.php/OMAP35x_Linux_-_DSS_FAQ


I would still argue on, 
In OMAP there is chance of miss-match between user application and Display HW if user uses FBIO_WAITFORVSYNC ioctl in multi-buffer use-case.

while (1) {
	Update/Create the next buffer
	FBIO_PAN
	FBIO_WAITFORVSYNC //assuming HW-SW stay in sync (which is not)
}

There is definitely an issue with above use-case which has been un-doubtfully conformed now.

At least in case of OMAP, WAITFORVSYNC is useless in multiple buffering use-case, application has to use WAITFORGO.

As far as WAITFORGO is concerned, I think GO bit concept is something OMAP notion/term and doesn't make sense to standardize it. Atleast I am not aware of any other architecture having GO bit.

Thanks,
Vaibhav

> --
> Ville Syrjälä

RE: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl

[Hiremath, Vaibhav]

> -----Original Message-----
> From: Hiremath, Vaibhav
> Sent: Friday, November 26, 2010 5:34 PM
> To: 'Måns Rullgård'; linux-omap@vger.kernel.org
> Subject: RE: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> 
> > -----Original Message-----
> > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> > owner@vger.kernel.org] On Behalf Of Måns Rullgård
> > Sent: Friday, November 26, 2010 2:09 PM
> > To: linux-omap@vger.kernel.org
> > Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> >
> > "Hiremath, Vaibhav" <hvaibhav@ti.com> writes:
> >
> > >> -----Original Message-----
> > >> From: Ville Syrjälä [mailto:ville.syrjala@nokia.com]
> > >> Sent: Wednesday, November 24, 2010 10:01 PM
> > >> To: Hiremath, Vaibhav
> > >> Cc: Tomi Valkeinen; linux-omap@vger.kernel.org
> > >> Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> > >>
> > >> On Wed, Nov 24, 2010 at 03:39:44PM +0530, ext Hiremath, Vaibhav
> wrote:
> > >> >
> > >> > > -----Original Message-----
> > >> > > From: Tomi Valkeinen [mailto:tomi.valkeinen@nokia.com]
> > >> > > Sent: Wednesday, November 24, 2010 2:28 PM
> > >> > > To: Hiremath, Vaibhav
> > >> > > Cc: linux-omap@vger.kernel.org
> > >> > > Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> > >> > >
> > >> > > On Tue, 2010-11-23 at 23:46 +0530, ext Hiremath, Vaibhav wrote:
> > >> > > > Hi,
> > > <snip>
> > >> >
<snip..>
> 
> > > As far as WAITFORGO is concerned, I think GO bit concept is
> > > something OMAP notion/term and doesn't make sense to standardize
> > > it. Atleast I am not aware of any other architecture having GO bit.
> >
> > Naming is minor detail.  Feel free to suggest a better one.
> >
> [Hiremath, Vaibhav] If I fail to convince on this, then I think the only
> left option is to make WAITFORGO ioctl generic. And put a disclaimer on
> WAITFORVSYNC, it must not be used in panning use-case.
> 
> 
[Hiremath, Vaibhav] Also let me bring another point here,

If I understand correctly most of the application libraries (DirectFB, X, etc..) does use FBIO_WAITFORVSYNC to synchronize with HW, and manage ping pong mechanism.

With this finding, in case of OMAP3 we have to use OMAPFB_WAITFORGO (breaking standard applications).

Thanks,
Vaibhav

> > --
> > Måns Rullgård
> > mans@mansr.com
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl

[Ville Syrjälä]

On Fri, Nov 26, 2010 at 05:38:11PM +0530, ext Hiremath, Vaibhav wrote:
> 
> > -----Original Message-----
> > From: Hiremath, Vaibhav
> > Sent: Friday, November 26, 2010 5:34 PM
> > To: 'Måns Rullgård'; linux-omap@vger.kernel.org
> > Subject: RE: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> > 
> > > -----Original Message-----
> > > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> > > owner@vger.kernel.org] On Behalf Of Måns Rullgård
> > > Sent: Friday, November 26, 2010 2:09 PM
> > > To: linux-omap@vger.kernel.org
> > > Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> > >
> > > "Hiremath, Vaibhav" <hvaibhav@ti.com> writes:
> > >
> > > >> -----Original Message-----
> > > >> From: Ville Syrjälä [mailto:ville.syrjala@nokia.com]
> > > >> Sent: Wednesday, November 24, 2010 10:01 PM
> > > >> To: Hiremath, Vaibhav
> > > >> Cc: Tomi Valkeinen; linux-omap@vger.kernel.org
> > > >> Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> > > >>
> > > >> On Wed, Nov 24, 2010 at 03:39:44PM +0530, ext Hiremath, Vaibhav
> > wrote:
> > > >> >
> > > >> > > -----Original Message-----
> > > >> > > From: Tomi Valkeinen [mailto:tomi.valkeinen@nokia.com]
> > > >> > > Sent: Wednesday, November 24, 2010 2:28 PM
> > > >> > > To: Hiremath, Vaibhav
> > > >> > > Cc: linux-omap@vger.kernel.org
> > > >> > > Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> > > >> > >
> > > >> > > On Tue, 2010-11-23 at 23:46 +0530, ext Hiremath, Vaibhav wrote:
> > > >> > > > Hi,
> > > > <snip>
> > > >> >
> <snip..>
> > 
> > > > As far as WAITFORGO is concerned, I think GO bit concept is
> > > > something OMAP notion/term and doesn't make sense to standardize
> > > > it. Atleast I am not aware of any other architecture having GO bit.
> > >
> > > Naming is minor detail.  Feel free to suggest a better one.
> > >
> > [Hiremath, Vaibhav] If I fail to convince on this, then I think the only
> > left option is to make WAITFORGO ioctl generic. And put a disclaimer on
> > WAITFORVSYNC, it must not be used in panning use-case.
> > 
> > 
> [Hiremath, Vaibhav] Also let me bring another point here,
> 
> If I understand correctly most of the application libraries (DirectFB, X, etc..) does use FBIO_WAITFORVSYNC to synchronize with HW, and manage ping pong mechanism.

DirectFB uses it also for waiting for vsync.

> With this finding, in case of OMAP3 we have to use OMAPFB_WAITFORGO (breaking standard applications).

Applications using the standard fbdev API won't work with manual update
displays anyway. You need omapfb specific code to handle it so having
another small difference is not a big deal.

In DirectFB the that's trivial since there's already a simple omap
gfxdriver where you could override the default flip functionality with
WAITFORGO based stuff.

Or, as I said, you could add another standard ioctl and fix up userspace
to use it where appropriate and if the kernel driver supports it.

-- 
Ville Syrjälä

Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl

[Felipe Contreras]

On Fri, Nov 26, 2010 at 2:55 PM, Ville Syrjälä <ville.syrjala@nokia.com> wrote:
> On Fri, Nov 26, 2010 at 05:38:11PM +0530, ext Hiremath, Vaibhav wrote:
>> With this finding, in case of OMAP3 we have to use OMAPFB_WAITFORGO (breaking standard applications).
>
> Applications using the standard fbdev API won't work with manual update
> displays anyway. You need omapfb specific code to handle it so having
> another small difference is not a big deal.
>
> In DirectFB the that's trivial since there's already a simple omap
> gfxdriver where you could override the default flip functionality with
> WAITFORGO based stuff.
>
> Or, as I said, you could add another standard ioctl and fix up userspace
> to use it where appropriate and if the kernel driver supports it.

That sounds more appealing to me: there's no point in having
omap-specific interfaces if they can be standard.

-- 
Felipe Contreras

RE: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl

[Hiremath, Vaibhav]

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@nokia.com]
> Sent: Friday, November 26, 2010 6:26 PM
> To: Hiremath, Vaibhav
> Cc: Måns Rullgård; linux-omap@vger.kernel.org; linux-fbdev@vger.kernel.org
> Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> 
> On Fri, Nov 26, 2010 at 05:38:11PM +0530, ext Hiremath, Vaibhav wrote:
> >
> > > -----Original Message-----
> > > From: Hiremath, Vaibhav
> > > Sent: Friday, November 26, 2010 5:34 PM
> > > To: 'Måns Rullgård'; linux-omap@vger.kernel.org
> > > Subject: RE: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> > >
> > > > -----Original Message-----
> > > > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> > > > owner@vger.kernel.org] On Behalf Of Måns Rullgård
> > > > Sent: Friday, November 26, 2010 2:09 PM
> > > > To: linux-omap@vger.kernel.org
> > > > Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> > > >
> > > > "Hiremath, Vaibhav" <hvaibhav@ti.com> writes:
> > > >
> > > > >> -----Original Message-----
> > > > >> From: Ville Syrjälä [mailto:ville.syrjala@nokia.com]
> > > > >> Sent: Wednesday, November 24, 2010 10:01 PM
> > > > >> To: Hiremath, Vaibhav
> > > > >> Cc: Tomi Valkeinen; linux-omap@vger.kernel.org
> > > > >> Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> > > > >>
> > > > >> On Wed, Nov 24, 2010 at 03:39:44PM +0530, ext Hiremath, Vaibhav
> > > wrote:
> > > > >> >
> > > > >> > > -----Original Message-----
> > > > >> > > From: Tomi Valkeinen [mailto:tomi.valkeinen@nokia.com]
> > > > >> > > Sent: Wednesday, November 24, 2010 2:28 PM
> > > > >> > > To: Hiremath, Vaibhav
> > > > >> > > Cc: linux-omap@vger.kernel.org
> > > > >> > > Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> > > > >> > >
> > > > >> > > On Tue, 2010-11-23 at 23:46 +0530, ext Hiremath, Vaibhav
> wrote:
> > > > >> > > > Hi,
> > > > > <snip>
> > > > >> >
> > <snip..>
> > >
> > > > > As far as WAITFORGO is concerned, I think GO bit concept is
> > > > > something OMAP notion/term and doesn't make sense to standardize
> > > > > it. Atleast I am not aware of any other architecture having GO bit.
> > > >
> > > > Naming is minor detail.  Feel free to suggest a better one.
> > > >
> > > [Hiremath, Vaibhav] If I fail to convince on this, then I think the
> only
> > > left option is to make WAITFORGO ioctl generic. And put a disclaimer
> on
> > > WAITFORVSYNC, it must not be used in panning use-case.
> > >
> > >
> > [Hiremath, Vaibhav] Also let me bring another point here,
> >
> > If I understand correctly most of the application libraries (DirectFB, X,
> etc..) does use FBIO_WAITFORVSYNC to synchronize with HW, and manage ping
> pong mechanism.
> 
> DirectFB uses it also for waiting for vsync.
> 
[Hiremath, Vaibhav] Mat,

I am not expert on DirectFb stuff; can you please help me to understand what the use-case is? What DirectFB does/expects on this?

Thanks,
Vaibhav
> > With this finding, in case of OMAP3 we have to use OMAPFB_WAITFORGO
> (breaking standard applications).
> 
> Applications using the standard fbdev API won't work with manual update
> displays anyway. You need omapfb specific code to handle it so having
> another small difference is not a big deal.
> 
> In DirectFB the that's trivial since there's already a simple omap
> gfxdriver where you could override the default flip functionality with
> WAITFORGO based stuff.
> 
> Or, as I said, you could add another standard ioctl and fix up userspace
> to use it where appropriate and if the kernel driver supports it.
> 
> --
> Ville Syrjälä

Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl

[Paul Mundt]

On Fri, Nov 26, 2010 at 02:55:39PM +0200, Ville Syrj?l? wrote:
> On Fri, Nov 26, 2010 at 05:38:11PM +0530, ext Hiremath, Vaibhav wrote:
> > With this finding, in case of OMAP3 we have to use OMAPFB_WAITFORGO
> > (breaking standard applications).
> 
> Applications using the standard fbdev API won't work with manual update
> displays anyway. You need omapfb specific code to handle it so having
> another small difference is not a big deal.
> 
> In DirectFB the that's trivial since there's already a simple omap
> gfxdriver where you could override the default flip functionality with
> WAITFORGO based stuff.
> 
> Or, as I said, you could add another standard ioctl and fix up userspace
> to use it where appropriate and if the kernel driver supports it.
> 
It seems like the WAITFORGO semantics could be layered on top of drivers
using deferred I/O pretty easily, but the question is whether this is
something that userspace plans to make general use of or not. If the only
user of it in userspace code is OMAP-specific, then there's probably not
a lot of point in moving it over to be generic, but if there are
sufficient cases where userspace cares about this information independent
of WAITFORVSYNC for manual update displays, then we can certainly look at
moving it over for those cases.

Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl

[Paul Mundt]

On Tue, Nov 30, 2010 at 03:34:40PM +0900, Paul Mundt wrote:
> On Fri, Nov 26, 2010 at 02:55:39PM +0200, Ville Syrj?l? wrote:
> > On Fri, Nov 26, 2010 at 05:38:11PM +0530, ext Hiremath, Vaibhav wrote:
> > > With this finding, in case of OMAP3 we have to use OMAPFB_WAITFORGO
> > > (breaking standard applications).
> > 
> > Applications using the standard fbdev API won't work with manual update
> > displays anyway. You need omapfb specific code to handle it so having
> > another small difference is not a big deal.
> > 
> > In DirectFB the that's trivial since there's already a simple omap
> > gfxdriver where you could override the default flip functionality with
> > WAITFORGO based stuff.
> > 
> > Or, as I said, you could add another standard ioctl and fix up userspace
> > to use it where appropriate and if the kernel driver supports it.
> > 
> It seems like the WAITFORGO semantics could be layered on top of drivers
> using deferred I/O pretty easily, but the question is whether this is
> something that userspace plans to make general use of or not. If the only
> user of it in userspace code is OMAP-specific, then there's probably not
> a lot of point in moving it over to be generic, but if there are
> sufficient cases where userspace cares about this information independent
> of WAITFORVSYNC for manual update displays, then we can certainly look at
> moving it over for those cases.

Also, WAITFORGO is a pretty abysmal name. It seems like what you want is
a WAITFORHWSYNC or something similar.

RE: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl

[Hiremath, Vaibhav]

> -----Original Message-----
> From: Paul Mundt [mailto:lethal@linux-sh.org]
> Sent: Tuesday, November 30, 2010 12:10 PM
> To: Ville Syrj?l?
> Cc: Hiremath, Vaibhav; M?ns Rullg?rd; linux-omap@vger.kernel.org; linux-
> fbdev@vger.kernel.org
> Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> 
> On Tue, Nov 30, 2010 at 03:34:40PM +0900, Paul Mundt wrote:
> > On Fri, Nov 26, 2010 at 02:55:39PM +0200, Ville Syrj?l? wrote:
> > > On Fri, Nov 26, 2010 at 05:38:11PM +0530, ext Hiremath, Vaibhav wrote:
> > > > With this finding, in case of OMAP3 we have to use OMAPFB_WAITFORGO
> > > > (breaking standard applications).
> > >
> > > Applications using the standard fbdev API won't work with manual
> update
> > > displays anyway. You need omapfb specific code to handle it so having
> > > another small difference is not a big deal.
> > >
> > > In DirectFB the that's trivial since there's already a simple omap
> > > gfxdriver where you could override the default flip functionality with
> > > WAITFORGO based stuff.
> > >
> > > Or, as I said, you could add another standard ioctl and fix up
> userspace
> > > to use it where appropriate and if the kernel driver supports it.
> > >
> > It seems like the WAITFORGO semantics could be layered on top of drivers
> > using deferred I/O pretty easily, but the question is whether this is
> > something that userspace plans to make general use of or not. If the
> only
> > user of it in userspace code is OMAP-specific, then there's probably not
> > a lot of point in moving it over to be generic, but if there are
> > sufficient cases where userspace cares about this information
> independent
> > of WAITFORVSYNC for manual update displays, then we can certainly look
> at
> > moving it over for those cases.
> 
> Also, WAITFORGO is a pretty abysmal name. It seems like what you want is
> a WAITFORHWSYNC or something similar.
[Hiremath, Vaibhav] Yes exactly, this name fits to the use-case and sounds ok to me. 

Tomi,
Any comments?

Thanks,
Vaibhav

RE: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl

[Tomi Valkeinen]

On Tue, 2010-11-30 at 12:29 +0530, ext Hiremath, Vaibhav wrote:
> > -----Original Message-----
> > From: Paul Mundt [mailto:lethal@linux-sh.org]
> > Sent: Tuesday, November 30, 2010 12:10 PM
> > To: Ville Syrj?l?
> > Cc: Hiremath, Vaibhav; M?ns Rullg?rd; linux-omap@vger.kernel.org; linux-
> > fbdev@vger.kernel.org
> > Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> > 
> > On Tue, Nov 30, 2010 at 03:34:40PM +0900, Paul Mundt wrote:
> > > On Fri, Nov 26, 2010 at 02:55:39PM +0200, Ville Syrj?l? wrote:
> > > > On Fri, Nov 26, 2010 at 05:38:11PM +0530, ext Hiremath, Vaibhav wrote:
> > > > > With this finding, in case of OMAP3 we have to use OMAPFB_WAITFORGO
> > > > > (breaking standard applications).
> > > >
> > > > Applications using the standard fbdev API won't work with manual
> > update
> > > > displays anyway. You need omapfb specific code to handle it so having
> > > > another small difference is not a big deal.
> > > >
> > > > In DirectFB the that's trivial since there's already a simple omap
> > > > gfxdriver where you could override the default flip functionality with
> > > > WAITFORGO based stuff.
> > > >
> > > > Or, as I said, you could add another standard ioctl and fix up
> > userspace
> > > > to use it where appropriate and if the kernel driver supports it.
> > > >
> > > It seems like the WAITFORGO semantics could be layered on top of drivers
> > > using deferred I/O pretty easily, but the question is whether this is
> > > something that userspace plans to make general use of or not. If the
> > only
> > > user of it in userspace code is OMAP-specific, then there's probably not
> > > a lot of point in moving it over to be generic, but if there are
> > > sufficient cases where userspace cares about this information
> > independent
> > > of WAITFORVSYNC for manual update displays, then we can certainly look
> > at
> > > moving it over for those cases.
> > 
> > Also, WAITFORGO is a pretty abysmal name. It seems like what you want is
> > a WAITFORHWSYNC or something similar.
> [Hiremath, Vaibhav] Yes exactly, this name fits to the use-case and sounds ok to me. 
> 
> Tomi,
> Any comments?

Ah, I seem to have been dropped out from this mail thread...

Yes, WAITFORHWSYNC sounds much better.

But there seems to be some misunderstanding of what this is about.
Deferred IO or manual update displays are not directly related to this.
This is about configuration registers, ie. address, size, etc. of the
framebuffer, not about the contents of the framebuffer.

The reason why WAITFORGO was implemented is: 

OMAP has user writeable shadow registers and hidden real registers for
display controller. The shadow registers are latched to real registers
on VFP, but only if GO bit has been set. The GO bit is cleared by the HW
when latching has been done.

If the GO bit is set, we cannot touch the shadow registers because we
don't know when the VFP  will happen. That's why there's additionally a
SW cache for the config, so that we don't need to block when the GO bit
is up and the user wants to change the config. The driver "flushes" the
SW config to real registers in VSYNC interrupt handler.

This is why the user may need to wait multiple vsyncs until the config
has really been written to the real HW registers, and WAITFORGO waits
for this. But if there has not been any config changes, WAITFORGO
doesn't wait at all.

So WAITFORVSYNC and WAITFORGO do quite a different thing on OMAP. It is
true what Hiremath says, WAITFORVSYNC is difficult (impossible?) to use
properly on OMAP as if the config write happens between VFP and VSYNC,
the config is not actually yet in the real registers.

But I'm still not comfortable with just moving WAITFORGO over
WAITFORVSYNC. At least we should then change WAITFORGO to always wait at
least for the next vsync, so that it wouldn't return immediately when
there are no pending changes. This would make WAITFORGO act like
WAITFORVSYNC in many cases, but not always.

And to add to the confusion, there are multiple overlays on OMAP. They
may be currently shared by multiple users (for example omapfb and v4l2).
If the user uses WAITFORVSYNC, the call will return for every vsync, as
expected. If he uses WAITFORGO, the call will return in random number of
vsyncs from the user's point of view, because the other user may be
competing from the same resource.

One could still argue that always using WAITFORGO is better, as there's
the problem with WAITFORVSYNC that Hiremath described. But then again,
WAITFORGO acts differently than what (I think) WAITFORVSYNC should do.

So summa summarum, I didn't know how to solve this perfectly earlier,
and thus I implemented WAITFORGO, and I still don't know what would be
the perfect solution.

 Tomi

Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl

[Jonghun Han]

Tomi Valkeinen wrote:

2010/11/30 Tomi Valkeinen <tomi.valkeinen@nokia.com>:
> On Tue, 2010-11-30 at 12:29 +0530, ext Hiremath, Vaibhav wrote:
>> > -----Original Message-----
>> > From: Paul Mundt [mailto:lethal@linux-sh.org]
>> > Sent: Tuesday, November 30, 2010 12:10 PM
>> > To: Ville Syrj?l?
>> > Cc: Hiremath, Vaibhav; M?ns Rullg?rd; linux-omap@vger.kernel.org; linux-
>> > fbdev@vger.kernel.org
>> > Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
>> >
>> > On Tue, Nov 30, 2010 at 03:34:40PM +0900, Paul Mundt wrote:

<snip>

> OMAP has user writeable shadow registers and hidden real registers for
> display controller. The shadow registers are latched to real registers
> on VFP, but only if GO bit has been set. The GO bit is cleared by the HW
> when latching has been done.
>
> If the GO bit is set, we cannot touch the shadow registers because we
> don't know when the VFP  will happen. That's why there's additionally a
> SW cache for the config, so that we don't need to block when the GO bit
> is up and the user wants to change the config. The driver "flushes" the
> SW config to real registers in VSYNC interrupt handler.

Does the driver flush the config to real register directly not a shadow register
in VSYNC ISR? Does it mean display controller use the config flushed
to the real register from the VSYNC ?

I don't know OMAP in detail. But as I know other SoCs also work like it.

Can Go bit is cleared by SW ? And does each overlay(FB) have its own Go bit ?
If Go bit can be cleared by SW, how do you think about the following scheme ?

When user wants to change the config, clear the Go bit
although Go bit has been already set.
And set the config shadow register and then re-set the go bit.
It can make one Vsync delay for the first user who has wanted to
change the config.
But it can reduce the delay for the second user. And WAITFORGO can be removed.

BTW as I know, Android also uses WAITFORVSYNC for multiple buffering.

<snip>

>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

BRs,

Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl

[Måns Rullgård]

Jonghun Han <jonghun79.han@gmail.com> writes:

> Tomi Valkeinen wrote:
>
> 2010/11/30 Tomi Valkeinen <tomi.valkeinen@nokia.com>:
>> On Tue, 2010-11-30 at 12:29 +0530, ext Hiremath, Vaibhav wrote:
>>> > -----Original Message-----
>>> > From: Paul Mundt [mailto:lethal@linux-sh.org]
>>> > Sent: Tuesday, November 30, 2010 12:10 PM
>>> > To: Ville Syrj?l?
>>> > Cc: Hiremath, Vaibhav; M?ns Rullg?rd; linux-omap@vger.kernel.org; linux-
>>> > fbdev@vger.kernel.org
>>> > Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
>>> >
>>> > On Tue, Nov 30, 2010 at 03:34:40PM +0900, Paul Mundt wrote:
>
> <snip>
>
>> OMAP has user writeable shadow registers and hidden real registers for
>> display controller. The shadow registers are latched to real registers
>> on VFP, but only if GO bit has been set. The GO bit is cleared by the HW
>> when latching has been done.
>>
>> If the GO bit is set, we cannot touch the shadow registers because we
>> don't know when the VFP  will happen. That's why there's additionally a
>> SW cache for the config, so that we don't need to block when the GO bit
>> is up and the user wants to change the config. The driver "flushes" the
>> SW config to real registers in VSYNC interrupt handler.
>
> Does the driver flush the config to real register directly not a
> shadow register in VSYNC ISR? Does it mean display controller use
> the config flushed to the real register from the VSYNC ?

The hardware latches the shadow registers to the active registers at
start of VFP.

> I don't know OMAP in detail. But as I know other SoCs also work like it.
>
> Can Go bit is cleared by SW?

No.

> And does each overlay(FB) have its own Go bit?

No.  There is one GO bit per video output, i.e. one each for LCD and TV.

-- 
Måns Rullgård
mans@mansr.com

RE: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl

[Hiremath, Vaibhav]

> -----Original Message-----
> From: Tomi Valkeinen [mailto:tomi.valkeinen@nokia.com]
> Sent: Tuesday, November 30, 2010 7:02 PM
> To: Hiremath, Vaibhav
> Cc: Paul Mundt; Ville Syrj?l?; M?ns Rullg?rd; linux-omap@vger.kernel.org;
> linux-fbdev@vger.kernel.org
> Subject: RE: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> 
> On Tue, 2010-11-30 at 12:29 +0530, ext Hiremath, Vaibhav wrote:
> > > -----Original Message-----
> > > From: Paul Mundt [mailto:lethal@linux-sh.org]
> > > Sent: Tuesday, November 30, 2010 12:10 PM
> > > To: Ville Syrj?l?
> > > Cc: Hiremath, Vaibhav; M?ns Rullg?rd; linux-omap@vger.kernel.org;
> linux-
> > > fbdev@vger.kernel.org
> > > Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> > >
> > > On Tue, Nov 30, 2010 at 03:34:40PM +0900, Paul Mundt wrote:
> > > > On Fri, Nov 26, 2010 at 02:55:39PM +0200, Ville Syrj?l? wrote:
> > > > > On Fri, Nov 26, 2010 at 05:38:11PM +0530, ext Hiremath, Vaibhav
> wrote:

<snip>

> > > a WAITFORHWSYNC or something similar.
> > [Hiremath, Vaibhav] Yes exactly, this name fits to the use-case and
> sounds ok to me.
> >
> > Tomi,
> > Any comments?
> 
> Ah, I seem to have been dropped out from this mail thread...
> 
> Yes, WAITFORHWSYNC sounds much better.
> 
> But there seems to be some misunderstanding of what this is about.
> Deferred IO or manual update displays are not directly related to this.
> This is about configuration registers, ie. address, size, etc. of the
> framebuffer, not about the contents of the framebuffer.
> 
> The reason why WAITFORGO was implemented is:
> 
> OMAP has user writeable shadow registers and hidden real registers for
> display controller. The shadow registers are latched to real registers
> on VFP, but only if GO bit has been set. The GO bit is cleared by the HW
> when latching has been done.
> 
> If the GO bit is set, we cannot touch the shadow registers because we
> don't know when the VFP  will happen. That's why there's additionally a
> SW cache for the config, so that we don't need to block when the GO bit
> is up and the user wants to change the config. The driver "flushes" the
> SW config to real registers in VSYNC interrupt handler.
> 
> This is why the user may need to wait multiple vsyncs until the config
> has really been written to the real HW registers, and WAITFORGO waits
> for this. But if there has not been any config changes, WAITFORGO
> doesn't wait at all.
> 
> So WAITFORVSYNC and WAITFORGO do quite a different thing on OMAP. It is
> true what Hiremath says, WAITFORVSYNC is difficult (impossible?) to use
> properly on OMAP as if the config write happens between VFP and VSYNC,
> the config is not actually yet in the real registers.
> 
> But I'm still not comfortable with just moving WAITFORGO over
> WAITFORVSYNC. At least we should then change WAITFORGO to always wait at
> least for the next vsync, so that it wouldn't return immediately when
> there are no pending changes. This would make WAITFORGO act like
> WAITFORVSYNC in many cases, but not always.
> 
> And to add to the confusion, there are multiple overlays on OMAP. They
> may be currently shared by multiple users (for example omapfb and v4l2).
> If the user uses WAITFORVSYNC, the call will return for every vsync, as
> expected. If he uses WAITFORGO, the call will return in random number of
> vsyncs from the user's point of view, because the other user may be
> competing from the same resource.
> 
> One could still argue that always using WAITFORGO is better, as there's
> the problem with WAITFORVSYNC that Hiremath described. But then again,
> WAITFORGO acts differently than what (I think) WAITFORVSYNC should do.
> 
> So summa summarum, I didn't know how to solve this perfectly earlier,
> and thus I implemented WAITFORGO, and I still don't know what would be
> the perfect solution.
> 
[Hiremath, Vaibhav] We did not had a closure on this topic, so summarizing our discussion here,

Let's keep FBIO_WAITFORVSYNC (or OMAPFB_WAITFORVSYNC) ioctl as is, since as of now the way we understand this ioctl is, it should barely wait for next vsync. 

Change the OMAPFB_WAITFORGO to (standard) FBIO_WAITFORHWSYNC, currently this will be used in OMAP2/3 Fb-display driver.

Thanks,
Vaibhav

>  Tomi
> 

RE: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl

[Hiremath, Vaibhav]

> -----Original Message-----
> From: Paul Mundt [mailto:lethal@linux-sh.org]
> Sent: Tuesday, November 30, 2010 12:05 PM
> To: Ville Syrj?l?
> Cc: Hiremath, Vaibhav; M?ns Rullg?rd; linux-omap@vger.kernel.org; linux-
> fbdev@vger.kernel.org
> Subject: Re: OMAP:DSS: possible bug in WAITFOR_VSYNC ioctl
> 
> On Fri, Nov 26, 2010 at 02:55:39PM +0200, Ville Syrj?l? wrote:
> > On Fri, Nov 26, 2010 at 05:38:11PM +0530, ext Hiremath, Vaibhav wrote:
> > > With this finding, in case of OMAP3 we have to use OMAPFB_WAITFORGO
> > > (breaking standard applications).
> >
> > Applications using the standard fbdev API won't work with manual update
> > displays anyway. You need omapfb specific code to handle it so having
> > another small difference is not a big deal.
> >
> > In DirectFB the that's trivial since there's already a simple omap
> > gfxdriver where you could override the default flip functionality with
> > WAITFORGO based stuff.
> >
> > Or, as I said, you could add another standard ioctl and fix up userspace
> > to use it where appropriate and if the kernel driver supports it.
> >
> It seems like the WAITFORGO semantics could be layered on top of drivers
> using deferred I/O pretty easily, but the question is whether this is
> something that userspace plans to make general use of or not. If the only
> user of it in userspace code is OMAP-specific, then there's probably not
> a lot of point in moving it over to be generic, but if there are
> sufficient cases where userspace cares about this information independent
> of WAITFORVSYNC for manual update displays, then we can certainly look at
> moving it over for those cases.
[Hiremath, Vaibhav] As part of this thread, I was referring to couple of other platforms like Samsung S3C, etc... and neither they do have GO bit concept nor OMAP3 like issue (I was referring to). 
In case of S3C, the device is capable of giving interrupt for all VFP, vsync, VBP, etc... instances if timing cycle and I believe driver makes use of vsync under WAITFORVSYNC.

Thanks,
Vaibhav