Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Wed, Mar 1, 2023 at 7:31 AM Sebastian Wick wrote: > > On Tue, Feb 28, 2023 at 11:52 PM Rob Clark wrote: > > > > On Tue, Feb 28, 2023 at 6:30 AM Sebastian Wick > > wrote: > > > > > > On Tue, Feb 28, 2023 at 12:48 AM Rob Clark wrote: > > > > > > > > On Mon, Feb 27, 2023 at 2:44 PM Sebastian Wick > > > > wrote: > > > > > > > > > > On Mon, Feb 27, 2023 at 11:20 PM Rob Clark > > > > > wrote: > > > > > > > > > > > > On Mon, Feb 27, 2023 at 1:36 PM Rodrigo Vivi > > > > > > wrote: > > > > > > > > > > > > > > On Fri, Feb 24, 2023 at 09:59:57AM -0800, Rob Clark wrote: > > > > > > > > On Fri, Feb 24, 2023 at 7:27 AM Luben Tuikov > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > On 2023-02-24 06:37, Tvrtko Ursulin wrote: > > > > > > > > > > > > > > > > > > > > On 24/02/2023 11:00, Pekka Paalanen wrote: > > > > > > > > > >> On Fri, 24 Feb 2023 10:50:51 + > > > > > > > > > >> Tvrtko Ursulin wrote: > > > > > > > > > >> > > > > > > > > > >>> On 24/02/2023 10:24, Pekka Paalanen wrote: > > > > > > > > > On Fri, 24 Feb 2023 09:41:46 + > > > > > > > > > Tvrtko Ursulin wrote: > > > > > > > > > > > > > > > > > > > On 24/02/2023 09:26, Pekka Paalanen wrote: > > > > > > > > > >> On Thu, 23 Feb 2023 10:51:48 -0800 > > > > > > > > > >> Rob Clark wrote: > > > > > > > > > >> > > > > > > > > > >>> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > > > > > > > > > >>> wrote: > > > > > > > > > > > > > > > > > > On Wed, 22 Feb 2023 07:37:26 -0800 > > > > > > > > > Rob Clark wrote: > > > > > > > > > > > > > > > > > > > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > > > > > > > > > > wrote: > > > > > > > > > >> > > > > > > > > > >> ... > > > > > > > > > >> > > > > > > > > > >> On another matter, if the application uses > > > > > > > > > >> SET_DEADLINE with one > > > > > > > > > >> timestamp, and the compositor uses SET_DEADLINE on > > > > > > > > > >> the same thing with > > > > > > > > > >> another timestamp, what should happen? > > > > > > > > > > > > > > > > > > > > The expectation is that many deadline hints can be > > > > > > > > > > set on a fence. > > > > > > > > > > The fence signaller should track the soonest > > > > > > > > > > deadline. > > > > > > > > > > > > > > > > > > You need to document that as UAPI, since it is > > > > > > > > > observable to userspace. > > > > > > > > > It would be bad if drivers or subsystems would > > > > > > > > > differ in behaviour. > > > > > > > > > > > > > > > > > > >>> > > > > > > > > > >>> It is in the end a hint. It is about giving the > > > > > > > > > >>> driver more > > > > > > > > > >>> information so that it can make better choices. But > > > > > > > > > >>> the driver is > > > > > > > > > >>> even free to ignore it. So maybe "expectation" is > > > > > > > > > >>> too strong of a > > > > > > > > > >>> word. Rather, any other behavior doesn't really make > > > > > > > > > >>> sense. But it > > > > > > > > > >>> could end up being dictated by how the hw and/or fw > > > > > > > > > >>> works. > > > > > > > > > >> > > > > > > > > > >> It will stop being a hint once it has been implemented > > > > > > > > > >> and used in the > > > > > > > > > >> wild long enough. The kernel userspace regression > > > > > > > > > >> rules make sure of > > > > > > > > > >> that. > > > > > > > > > > > > > > > > > > > > Yeah, tricky and maybe a gray area in this case. I > > > > > > > > > > think we eluded > > > > > > > > > > elsewhere in the thread that renaming the thing might > > > > > > > > > > be an option. > > > > > > > > > > > > > > > > > > > > So maybe instead of deadline, which is a very strong > > > > > > > > > > word, use something > > > > > > > > > > along the lines of "present time hint", or "signalled > > > > > > > > > > time hint"? Maybe > > > > > > > > > > reads clumsy. Just throwing some ideas for a start. > > > > > > > > > > > > > > > > > > You can try, but I fear that if it ever changes > > > > > > > > > behaviour and > > > > > > > > > someone notices that, it's labelled as a kernel > > > > > > > > > regression. I don't > > > > > > > > > think documentation has ever been the authoritative > > > > > > > > > definition of UABI > > > > > > > > > in Linux, it just guides drivers and userspace towards a > > > > > > > > > common > > > > > > > > > understanding and common usage patterns. > > > > > > > > > > > > > > > > > > So even if the UABI contract is not documented (ugh), > > > > > > > > > you need to be > > > > > > > > > prepared to set the UABI contract through kernel > > > > > > > > > implementation. > > > > > >
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Mon, Feb 27, 2023 at 02:20:04PM -0800, Rob Clark wrote: > On Mon, Feb 27, 2023 at 1:36 PM Rodrigo Vivi wrote: > > > > On Fri, Feb 24, 2023 at 09:59:57AM -0800, Rob Clark wrote: > > > On Fri, Feb 24, 2023 at 7:27 AM Luben Tuikov wrote: > > > > > > > > On 2023-02-24 06:37, Tvrtko Ursulin wrote: > > > > > > > > > > On 24/02/2023 11:00, Pekka Paalanen wrote: > > > > >> On Fri, 24 Feb 2023 10:50:51 + > > > > >> Tvrtko Ursulin wrote: > > > > >> > > > > >>> On 24/02/2023 10:24, Pekka Paalanen wrote: > > > > On Fri, 24 Feb 2023 09:41:46 + > > > > Tvrtko Ursulin wrote: > > > > > > > > > On 24/02/2023 09:26, Pekka Paalanen wrote: > > > > >> On Thu, 23 Feb 2023 10:51:48 -0800 > > > > >> Rob Clark wrote: > > > > >> > > > > >>> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > > > > >>> wrote: > > > > > > > > On Wed, 22 Feb 2023 07:37:26 -0800 > > > > Rob Clark wrote: > > > > > > > > > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > > > > > wrote: > > > > >> > > > > >> ... > > > > >> > > > > >> On another matter, if the application uses SET_DEADLINE with > > > > >> one > > > > >> timestamp, and the compositor uses SET_DEADLINE on the same > > > > >> thing with > > > > >> another timestamp, what should happen? > > > > > > > > > > The expectation is that many deadline hints can be set on a > > > > > fence. > > > > > The fence signaller should track the soonest deadline. > > > > > > > > You need to document that as UAPI, since it is observable to > > > > userspace. > > > > It would be bad if drivers or subsystems would differ in > > > > behaviour. > > > > > > > > >>> > > > > >>> It is in the end a hint. It is about giving the driver more > > > > >>> information so that it can make better choices. But the driver > > > > >>> is > > > > >>> even free to ignore it. So maybe "expectation" is too strong > > > > >>> of a > > > > >>> word. Rather, any other behavior doesn't really make sense. > > > > >>> But it > > > > >>> could end up being dictated by how the hw and/or fw works. > > > > >> > > > > >> It will stop being a hint once it has been implemented and used > > > > >> in the > > > > >> wild long enough. The kernel userspace regression rules make > > > > >> sure of > > > > >> that. > > > > > > > > > > Yeah, tricky and maybe a gray area in this case. I think we eluded > > > > > elsewhere in the thread that renaming the thing might be an > > > > > option. > > > > > > > > > > So maybe instead of deadline, which is a very strong word, use > > > > > something > > > > > along the lines of "present time hint", or "signalled time hint"? > > > > > Maybe > > > > > reads clumsy. Just throwing some ideas for a start. > > > > > > > > You can try, but I fear that if it ever changes behaviour and > > > > someone notices that, it's labelled as a kernel regression. I don't > > > > think documentation has ever been the authoritative definition of > > > > UABI > > > > in Linux, it just guides drivers and userspace towards a common > > > > understanding and common usage patterns. > > > > > > > > So even if the UABI contract is not documented (ugh), you need to > > > > be > > > > prepared to set the UABI contract through kernel implementation. > > > > >>> > > > > >>> To be the devil's advocate it probably wouldn't be an ABI > > > > >>> regression but > > > > >>> just an regression. Same way as what nice(2) priorities mean hasn't > > > > >>> always been the same over the years, I don't think there is a strict > > > > >>> contract. > > > > >>> > > > > >>> Having said that, it may be different with latency sensitive stuff > > > > >>> such > > > > >>> as UIs though since it is very observable and can be very painful > > > > >>> to users. > > > > >>> > > > > If you do not document the UABI contract, then different drivers > > > > are > > > > likely to implement it differently, leading to differing behaviour. > > > > Also userspace will invent wild ways to abuse the UABI if there is > > > > no > > > > documentation guiding it on proper use. If userspace or end users > > > > observe different behaviour, that's bad even if it's not a > > > > regression. > > > > > > > > I don't like the situation either, but it is what it is. UABI > > > > stability > > > > trumps everything regardless of whether it was documented or not. > > > > > > > > I bet userspace is going to use this as a "make it faster, make it > > > > hotter" button. I would not be surprised if someone wrote a > > > > LD_PRELOAD > > > > library that stamps any and all fences with an
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Tue, Feb 28, 2023 at 11:52 PM Rob Clark wrote: > > On Tue, Feb 28, 2023 at 6:30 AM Sebastian Wick > wrote: > > > > On Tue, Feb 28, 2023 at 12:48 AM Rob Clark wrote: > > > > > > On Mon, Feb 27, 2023 at 2:44 PM Sebastian Wick > > > wrote: > > > > > > > > On Mon, Feb 27, 2023 at 11:20 PM Rob Clark wrote: > > > > > > > > > > On Mon, Feb 27, 2023 at 1:36 PM Rodrigo Vivi > > > > > wrote: > > > > > > > > > > > > On Fri, Feb 24, 2023 at 09:59:57AM -0800, Rob Clark wrote: > > > > > > > On Fri, Feb 24, 2023 at 7:27 AM Luben Tuikov > > > > > > > wrote: > > > > > > > > > > > > > > > > On 2023-02-24 06:37, Tvrtko Ursulin wrote: > > > > > > > > > > > > > > > > > > On 24/02/2023 11:00, Pekka Paalanen wrote: > > > > > > > > >> On Fri, 24 Feb 2023 10:50:51 + > > > > > > > > >> Tvrtko Ursulin wrote: > > > > > > > > >> > > > > > > > > >>> On 24/02/2023 10:24, Pekka Paalanen wrote: > > > > > > > > On Fri, 24 Feb 2023 09:41:46 + > > > > > > > > Tvrtko Ursulin wrote: > > > > > > > > > > > > > > > > > On 24/02/2023 09:26, Pekka Paalanen wrote: > > > > > > > > >> On Thu, 23 Feb 2023 10:51:48 -0800 > > > > > > > > >> Rob Clark wrote: > > > > > > > > >> > > > > > > > > >>> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > > > > > > > > >>> wrote: > > > > > > > > > > > > > > > > On Wed, 22 Feb 2023 07:37:26 -0800 > > > > > > > > Rob Clark wrote: > > > > > > > > > > > > > > > > > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > > > > > > > > > wrote: > > > > > > > > >> > > > > > > > > >> ... > > > > > > > > >> > > > > > > > > >> On another matter, if the application uses > > > > > > > > >> SET_DEADLINE with one > > > > > > > > >> timestamp, and the compositor uses SET_DEADLINE on > > > > > > > > >> the same thing with > > > > > > > > >> another timestamp, what should happen? > > > > > > > > > > > > > > > > > > The expectation is that many deadline hints can be > > > > > > > > > set on a fence. > > > > > > > > > The fence signaller should track the soonest deadline. > > > > > > > > > > > > > > > > You need to document that as UAPI, since it is > > > > > > > > observable to userspace. > > > > > > > > It would be bad if drivers or subsystems would differ > > > > > > > > in behaviour. > > > > > > > > > > > > > > > > >>> > > > > > > > > >>> It is in the end a hint. It is about giving the driver > > > > > > > > >>> more > > > > > > > > >>> information so that it can make better choices. But > > > > > > > > >>> the driver is > > > > > > > > >>> even free to ignore it. So maybe "expectation" is too > > > > > > > > >>> strong of a > > > > > > > > >>> word. Rather, any other behavior doesn't really make > > > > > > > > >>> sense. But it > > > > > > > > >>> could end up being dictated by how the hw and/or fw > > > > > > > > >>> works. > > > > > > > > >> > > > > > > > > >> It will stop being a hint once it has been implemented > > > > > > > > >> and used in the > > > > > > > > >> wild long enough. The kernel userspace regression rules > > > > > > > > >> make sure of > > > > > > > > >> that. > > > > > > > > > > > > > > > > > > Yeah, tricky and maybe a gray area in this case. I think > > > > > > > > > we eluded > > > > > > > > > elsewhere in the thread that renaming the thing might be > > > > > > > > > an option. > > > > > > > > > > > > > > > > > > So maybe instead of deadline, which is a very strong > > > > > > > > > word, use something > > > > > > > > > along the lines of "present time hint", or "signalled > > > > > > > > > time hint"? Maybe > > > > > > > > > reads clumsy. Just throwing some ideas for a start. > > > > > > > > > > > > > > > > You can try, but I fear that if it ever changes behaviour > > > > > > > > and > > > > > > > > someone notices that, it's labelled as a kernel > > > > > > > > regression. I don't > > > > > > > > think documentation has ever been the authoritative > > > > > > > > definition of UABI > > > > > > > > in Linux, it just guides drivers and userspace towards a > > > > > > > > common > > > > > > > > understanding and common usage patterns. > > > > > > > > > > > > > > > > So even if the UABI contract is not documented (ugh), you > > > > > > > > need to be > > > > > > > > prepared to set the UABI contract through kernel > > > > > > > > implementation. > > > > > > > > >>> > > > > > > > > >>> To be the devil's advocate it probably wouldn't be an ABI > > > > > > > > >>> regression but > > > > > > > > >>> just an regression. Same way as what nice(2) priorities > > > > > > > > >>> mean hasn't > > > > > > > > >>> always been the same over the years, I don't
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Tue, Feb 28, 2023 at 6:30 AM Sebastian Wick wrote: > > On Tue, Feb 28, 2023 at 12:48 AM Rob Clark wrote: > > > > On Mon, Feb 27, 2023 at 2:44 PM Sebastian Wick > > wrote: > > > > > > On Mon, Feb 27, 2023 at 11:20 PM Rob Clark wrote: > > > > > > > > On Mon, Feb 27, 2023 at 1:36 PM Rodrigo Vivi > > > > wrote: > > > > > > > > > > On Fri, Feb 24, 2023 at 09:59:57AM -0800, Rob Clark wrote: > > > > > > On Fri, Feb 24, 2023 at 7:27 AM Luben Tuikov > > > > > > wrote: > > > > > > > > > > > > > > On 2023-02-24 06:37, Tvrtko Ursulin wrote: > > > > > > > > > > > > > > > > On 24/02/2023 11:00, Pekka Paalanen wrote: > > > > > > > >> On Fri, 24 Feb 2023 10:50:51 + > > > > > > > >> Tvrtko Ursulin wrote: > > > > > > > >> > > > > > > > >>> On 24/02/2023 10:24, Pekka Paalanen wrote: > > > > > > > On Fri, 24 Feb 2023 09:41:46 + > > > > > > > Tvrtko Ursulin wrote: > > > > > > > > > > > > > > > On 24/02/2023 09:26, Pekka Paalanen wrote: > > > > > > > >> On Thu, 23 Feb 2023 10:51:48 -0800 > > > > > > > >> Rob Clark wrote: > > > > > > > >> > > > > > > > >>> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > > > > > > > >>> wrote: > > > > > > > > > > > > > > On Wed, 22 Feb 2023 07:37:26 -0800 > > > > > > > Rob Clark wrote: > > > > > > > > > > > > > > > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > > > > > > > > wrote: > > > > > > > >> > > > > > > > >> ... > > > > > > > >> > > > > > > > >> On another matter, if the application uses > > > > > > > >> SET_DEADLINE with one > > > > > > > >> timestamp, and the compositor uses SET_DEADLINE on the > > > > > > > >> same thing with > > > > > > > >> another timestamp, what should happen? > > > > > > > > > > > > > > > > The expectation is that many deadline hints can be set > > > > > > > > on a fence. > > > > > > > > The fence signaller should track the soonest deadline. > > > > > > > > > > > > > > You need to document that as UAPI, since it is > > > > > > > observable to userspace. > > > > > > > It would be bad if drivers or subsystems would differ in > > > > > > > behaviour. > > > > > > > > > > > > > > >>> > > > > > > > >>> It is in the end a hint. It is about giving the driver > > > > > > > >>> more > > > > > > > >>> information so that it can make better choices. But the > > > > > > > >>> driver is > > > > > > > >>> even free to ignore it. So maybe "expectation" is too > > > > > > > >>> strong of a > > > > > > > >>> word. Rather, any other behavior doesn't really make > > > > > > > >>> sense. But it > > > > > > > >>> could end up being dictated by how the hw and/or fw works. > > > > > > > >> > > > > > > > >> It will stop being a hint once it has been implemented and > > > > > > > >> used in the > > > > > > > >> wild long enough. The kernel userspace regression rules > > > > > > > >> make sure of > > > > > > > >> that. > > > > > > > > > > > > > > > > Yeah, tricky and maybe a gray area in this case. I think we > > > > > > > > eluded > > > > > > > > elsewhere in the thread that renaming the thing might be an > > > > > > > > option. > > > > > > > > > > > > > > > > So maybe instead of deadline, which is a very strong word, > > > > > > > > use something > > > > > > > > along the lines of "present time hint", or "signalled time > > > > > > > > hint"? Maybe > > > > > > > > reads clumsy. Just throwing some ideas for a start. > > > > > > > > > > > > > > You can try, but I fear that if it ever changes behaviour and > > > > > > > someone notices that, it's labelled as a kernel regression. > > > > > > > I don't > > > > > > > think documentation has ever been the authoritative > > > > > > > definition of UABI > > > > > > > in Linux, it just guides drivers and userspace towards a > > > > > > > common > > > > > > > understanding and common usage patterns. > > > > > > > > > > > > > > So even if the UABI contract is not documented (ugh), you > > > > > > > need to be > > > > > > > prepared to set the UABI contract through kernel > > > > > > > implementation. > > > > > > > >>> > > > > > > > >>> To be the devil's advocate it probably wouldn't be an ABI > > > > > > > >>> regression but > > > > > > > >>> just an regression. Same way as what nice(2) priorities mean > > > > > > > >>> hasn't > > > > > > > >>> always been the same over the years, I don't think there is a > > > > > > > >>> strict > > > > > > > >>> contract. > > > > > > > >>> > > > > > > > >>> Having said that, it may be different with latency sensitive > > > > > > > >>> stuff such > > > > > > > >>> as UIs though since it is very observable and can be very > > > > > > > >>> painful to users. >
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Tue, Feb 28, 2023 at 12:48 AM Rob Clark wrote: > > On Mon, Feb 27, 2023 at 2:44 PM Sebastian Wick > wrote: > > > > On Mon, Feb 27, 2023 at 11:20 PM Rob Clark wrote: > > > > > > On Mon, Feb 27, 2023 at 1:36 PM Rodrigo Vivi > > > wrote: > > > > > > > > On Fri, Feb 24, 2023 at 09:59:57AM -0800, Rob Clark wrote: > > > > > On Fri, Feb 24, 2023 at 7:27 AM Luben Tuikov > > > > > wrote: > > > > > > > > > > > > On 2023-02-24 06:37, Tvrtko Ursulin wrote: > > > > > > > > > > > > > > On 24/02/2023 11:00, Pekka Paalanen wrote: > > > > > > >> On Fri, 24 Feb 2023 10:50:51 + > > > > > > >> Tvrtko Ursulin wrote: > > > > > > >> > > > > > > >>> On 24/02/2023 10:24, Pekka Paalanen wrote: > > > > > > On Fri, 24 Feb 2023 09:41:46 + > > > > > > Tvrtko Ursulin wrote: > > > > > > > > > > > > > On 24/02/2023 09:26, Pekka Paalanen wrote: > > > > > > >> On Thu, 23 Feb 2023 10:51:48 -0800 > > > > > > >> Rob Clark wrote: > > > > > > >> > > > > > > >>> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > > > > > > >>> wrote: > > > > > > > > > > > > On Wed, 22 Feb 2023 07:37:26 -0800 > > > > > > Rob Clark wrote: > > > > > > > > > > > > > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > > > > > > > wrote: > > > > > > >> > > > > > > >> ... > > > > > > >> > > > > > > >> On another matter, if the application uses SET_DEADLINE > > > > > > >> with one > > > > > > >> timestamp, and the compositor uses SET_DEADLINE on the > > > > > > >> same thing with > > > > > > >> another timestamp, what should happen? > > > > > > > > > > > > > > The expectation is that many deadline hints can be set on > > > > > > > a fence. > > > > > > > The fence signaller should track the soonest deadline. > > > > > > > > > > > > You need to document that as UAPI, since it is observable > > > > > > to userspace. > > > > > > It would be bad if drivers or subsystems would differ in > > > > > > behaviour. > > > > > > > > > > > > >>> > > > > > > >>> It is in the end a hint. It is about giving the driver more > > > > > > >>> information so that it can make better choices. But the > > > > > > >>> driver is > > > > > > >>> even free to ignore it. So maybe "expectation" is too > > > > > > >>> strong of a > > > > > > >>> word. Rather, any other behavior doesn't really make > > > > > > >>> sense. But it > > > > > > >>> could end up being dictated by how the hw and/or fw works. > > > > > > >> > > > > > > >> It will stop being a hint once it has been implemented and > > > > > > >> used in the > > > > > > >> wild long enough. The kernel userspace regression rules make > > > > > > >> sure of > > > > > > >> that. > > > > > > > > > > > > > > Yeah, tricky and maybe a gray area in this case. I think we > > > > > > > eluded > > > > > > > elsewhere in the thread that renaming the thing might be an > > > > > > > option. > > > > > > > > > > > > > > So maybe instead of deadline, which is a very strong word, > > > > > > > use something > > > > > > > along the lines of "present time hint", or "signalled time > > > > > > > hint"? Maybe > > > > > > > reads clumsy. Just throwing some ideas for a start. > > > > > > > > > > > > You can try, but I fear that if it ever changes behaviour and > > > > > > someone notices that, it's labelled as a kernel regression. I > > > > > > don't > > > > > > think documentation has ever been the authoritative definition > > > > > > of UABI > > > > > > in Linux, it just guides drivers and userspace towards a common > > > > > > understanding and common usage patterns. > > > > > > > > > > > > So even if the UABI contract is not documented (ugh), you need > > > > > > to be > > > > > > prepared to set the UABI contract through kernel > > > > > > implementation. > > > > > > >>> > > > > > > >>> To be the devil's advocate it probably wouldn't be an ABI > > > > > > >>> regression but > > > > > > >>> just an regression. Same way as what nice(2) priorities mean > > > > > > >>> hasn't > > > > > > >>> always been the same over the years, I don't think there is a > > > > > > >>> strict > > > > > > >>> contract. > > > > > > >>> > > > > > > >>> Having said that, it may be different with latency sensitive > > > > > > >>> stuff such > > > > > > >>> as UIs though since it is very observable and can be very > > > > > > >>> painful to users. > > > > > > >>> > > > > > > If you do not document the UABI contract, then different > > > > > > drivers are > > > > > > likely to implement it differently, leading to differing > > > > > > behaviour. > > > > > > Also userspace will invent wild ways to abuse the UABI if > > > > > >
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Mon, Feb 27, 2023 at 2:44 PM Sebastian Wick wrote: > > On Mon, Feb 27, 2023 at 11:20 PM Rob Clark wrote: > > > > On Mon, Feb 27, 2023 at 1:36 PM Rodrigo Vivi wrote: > > > > > > On Fri, Feb 24, 2023 at 09:59:57AM -0800, Rob Clark wrote: > > > > On Fri, Feb 24, 2023 at 7:27 AM Luben Tuikov > > > > wrote: > > > > > > > > > > On 2023-02-24 06:37, Tvrtko Ursulin wrote: > > > > > > > > > > > > On 24/02/2023 11:00, Pekka Paalanen wrote: > > > > > >> On Fri, 24 Feb 2023 10:50:51 + > > > > > >> Tvrtko Ursulin wrote: > > > > > >> > > > > > >>> On 24/02/2023 10:24, Pekka Paalanen wrote: > > > > > On Fri, 24 Feb 2023 09:41:46 + > > > > > Tvrtko Ursulin wrote: > > > > > > > > > > > On 24/02/2023 09:26, Pekka Paalanen wrote: > > > > > >> On Thu, 23 Feb 2023 10:51:48 -0800 > > > > > >> Rob Clark wrote: > > > > > >> > > > > > >>> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > > > > > >>> wrote: > > > > > > > > > > On Wed, 22 Feb 2023 07:37:26 -0800 > > > > > Rob Clark wrote: > > > > > > > > > > > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > > > > > > wrote: > > > > > >> > > > > > >> ... > > > > > >> > > > > > >> On another matter, if the application uses SET_DEADLINE > > > > > >> with one > > > > > >> timestamp, and the compositor uses SET_DEADLINE on the > > > > > >> same thing with > > > > > >> another timestamp, what should happen? > > > > > > > > > > > > The expectation is that many deadline hints can be set on a > > > > > > fence. > > > > > > The fence signaller should track the soonest deadline. > > > > > > > > > > You need to document that as UAPI, since it is observable to > > > > > userspace. > > > > > It would be bad if drivers or subsystems would differ in > > > > > behaviour. > > > > > > > > > > >>> > > > > > >>> It is in the end a hint. It is about giving the driver more > > > > > >>> information so that it can make better choices. But the > > > > > >>> driver is > > > > > >>> even free to ignore it. So maybe "expectation" is too strong > > > > > >>> of a > > > > > >>> word. Rather, any other behavior doesn't really make sense. > > > > > >>> But it > > > > > >>> could end up being dictated by how the hw and/or fw works. > > > > > >> > > > > > >> It will stop being a hint once it has been implemented and > > > > > >> used in the > > > > > >> wild long enough. The kernel userspace regression rules make > > > > > >> sure of > > > > > >> that. > > > > > > > > > > > > Yeah, tricky and maybe a gray area in this case. I think we > > > > > > eluded > > > > > > elsewhere in the thread that renaming the thing might be an > > > > > > option. > > > > > > > > > > > > So maybe instead of deadline, which is a very strong word, use > > > > > > something > > > > > > along the lines of "present time hint", or "signalled time > > > > > > hint"? Maybe > > > > > > reads clumsy. Just throwing some ideas for a start. > > > > > > > > > > You can try, but I fear that if it ever changes behaviour and > > > > > someone notices that, it's labelled as a kernel regression. I > > > > > don't > > > > > think documentation has ever been the authoritative definition > > > > > of UABI > > > > > in Linux, it just guides drivers and userspace towards a common > > > > > understanding and common usage patterns. > > > > > > > > > > So even if the UABI contract is not documented (ugh), you need > > > > > to be > > > > > prepared to set the UABI contract through kernel implementation. > > > > > >>> > > > > > >>> To be the devil's advocate it probably wouldn't be an ABI > > > > > >>> regression but > > > > > >>> just an regression. Same way as what nice(2) priorities mean > > > > > >>> hasn't > > > > > >>> always been the same over the years, I don't think there is a > > > > > >>> strict > > > > > >>> contract. > > > > > >>> > > > > > >>> Having said that, it may be different with latency sensitive > > > > > >>> stuff such > > > > > >>> as UIs though since it is very observable and can be very painful > > > > > >>> to users. > > > > > >>> > > > > > If you do not document the UABI contract, then different drivers > > > > > are > > > > > likely to implement it differently, leading to differing > > > > > behaviour. > > > > > Also userspace will invent wild ways to abuse the UABI if there > > > > > is no > > > > > documentation guiding it on proper use. If userspace or end users > > > > > observe different behaviour, that's bad even if it's not a > > > > > regression. > > > > > > > > > > I don't like the situation either, but it is what it is. UABI > > >
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Mon, Feb 27, 2023 at 11:20 PM Rob Clark wrote: > > On Mon, Feb 27, 2023 at 1:36 PM Rodrigo Vivi wrote: > > > > On Fri, Feb 24, 2023 at 09:59:57AM -0800, Rob Clark wrote: > > > On Fri, Feb 24, 2023 at 7:27 AM Luben Tuikov wrote: > > > > > > > > On 2023-02-24 06:37, Tvrtko Ursulin wrote: > > > > > > > > > > On 24/02/2023 11:00, Pekka Paalanen wrote: > > > > >> On Fri, 24 Feb 2023 10:50:51 + > > > > >> Tvrtko Ursulin wrote: > > > > >> > > > > >>> On 24/02/2023 10:24, Pekka Paalanen wrote: > > > > On Fri, 24 Feb 2023 09:41:46 + > > > > Tvrtko Ursulin wrote: > > > > > > > > > On 24/02/2023 09:26, Pekka Paalanen wrote: > > > > >> On Thu, 23 Feb 2023 10:51:48 -0800 > > > > >> Rob Clark wrote: > > > > >> > > > > >>> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > > > > >>> wrote: > > > > > > > > On Wed, 22 Feb 2023 07:37:26 -0800 > > > > Rob Clark wrote: > > > > > > > > > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > > > > > wrote: > > > > >> > > > > >> ... > > > > >> > > > > >> On another matter, if the application uses SET_DEADLINE with > > > > >> one > > > > >> timestamp, and the compositor uses SET_DEADLINE on the same > > > > >> thing with > > > > >> another timestamp, what should happen? > > > > > > > > > > The expectation is that many deadline hints can be set on a > > > > > fence. > > > > > The fence signaller should track the soonest deadline. > > > > > > > > You need to document that as UAPI, since it is observable to > > > > userspace. > > > > It would be bad if drivers or subsystems would differ in > > > > behaviour. > > > > > > > > >>> > > > > >>> It is in the end a hint. It is about giving the driver more > > > > >>> information so that it can make better choices. But the driver > > > > >>> is > > > > >>> even free to ignore it. So maybe "expectation" is too strong > > > > >>> of a > > > > >>> word. Rather, any other behavior doesn't really make sense. > > > > >>> But it > > > > >>> could end up being dictated by how the hw and/or fw works. > > > > >> > > > > >> It will stop being a hint once it has been implemented and used > > > > >> in the > > > > >> wild long enough. The kernel userspace regression rules make > > > > >> sure of > > > > >> that. > > > > > > > > > > Yeah, tricky and maybe a gray area in this case. I think we eluded > > > > > elsewhere in the thread that renaming the thing might be an > > > > > option. > > > > > > > > > > So maybe instead of deadline, which is a very strong word, use > > > > > something > > > > > along the lines of "present time hint", or "signalled time hint"? > > > > > Maybe > > > > > reads clumsy. Just throwing some ideas for a start. > > > > > > > > You can try, but I fear that if it ever changes behaviour and > > > > someone notices that, it's labelled as a kernel regression. I don't > > > > think documentation has ever been the authoritative definition of > > > > UABI > > > > in Linux, it just guides drivers and userspace towards a common > > > > understanding and common usage patterns. > > > > > > > > So even if the UABI contract is not documented (ugh), you need to > > > > be > > > > prepared to set the UABI contract through kernel implementation. > > > > >>> > > > > >>> To be the devil's advocate it probably wouldn't be an ABI > > > > >>> regression but > > > > >>> just an regression. Same way as what nice(2) priorities mean hasn't > > > > >>> always been the same over the years, I don't think there is a strict > > > > >>> contract. > > > > >>> > > > > >>> Having said that, it may be different with latency sensitive stuff > > > > >>> such > > > > >>> as UIs though since it is very observable and can be very painful > > > > >>> to users. > > > > >>> > > > > If you do not document the UABI contract, then different drivers > > > > are > > > > likely to implement it differently, leading to differing behaviour. > > > > Also userspace will invent wild ways to abuse the UABI if there is > > > > no > > > > documentation guiding it on proper use. If userspace or end users > > > > observe different behaviour, that's bad even if it's not a > > > > regression. > > > > > > > > I don't like the situation either, but it is what it is. UABI > > > > stability > > > > trumps everything regardless of whether it was documented or not. > > > > > > > > I bet userspace is going to use this as a "make it faster, make it > > > > hotter" button. I would not be surprised if someone wrote a > > > > LD_PRELOAD > > > > library that stamps any and all fences with an expired
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Mon, Feb 27, 2023 at 1:36 PM Rodrigo Vivi wrote: > > On Fri, Feb 24, 2023 at 09:59:57AM -0800, Rob Clark wrote: > > On Fri, Feb 24, 2023 at 7:27 AM Luben Tuikov wrote: > > > > > > On 2023-02-24 06:37, Tvrtko Ursulin wrote: > > > > > > > > On 24/02/2023 11:00, Pekka Paalanen wrote: > > > >> On Fri, 24 Feb 2023 10:50:51 + > > > >> Tvrtko Ursulin wrote: > > > >> > > > >>> On 24/02/2023 10:24, Pekka Paalanen wrote: > > > On Fri, 24 Feb 2023 09:41:46 + > > > Tvrtko Ursulin wrote: > > > > > > > On 24/02/2023 09:26, Pekka Paalanen wrote: > > > >> On Thu, 23 Feb 2023 10:51:48 -0800 > > > >> Rob Clark wrote: > > > >> > > > >>> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > > > >>> wrote: > > > > > > On Wed, 22 Feb 2023 07:37:26 -0800 > > > Rob Clark wrote: > > > > > > > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > > > > wrote: > > > >> > > > >> ... > > > >> > > > >> On another matter, if the application uses SET_DEADLINE with > > > >> one > > > >> timestamp, and the compositor uses SET_DEADLINE on the same > > > >> thing with > > > >> another timestamp, what should happen? > > > > > > > > The expectation is that many deadline hints can be set on a > > > > fence. > > > > The fence signaller should track the soonest deadline. > > > > > > You need to document that as UAPI, since it is observable to > > > userspace. > > > It would be bad if drivers or subsystems would differ in > > > behaviour. > > > > > > >>> > > > >>> It is in the end a hint. It is about giving the driver more > > > >>> information so that it can make better choices. But the driver is > > > >>> even free to ignore it. So maybe "expectation" is too strong of a > > > >>> word. Rather, any other behavior doesn't really make sense. But > > > >>> it > > > >>> could end up being dictated by how the hw and/or fw works. > > > >> > > > >> It will stop being a hint once it has been implemented and used in > > > >> the > > > >> wild long enough. The kernel userspace regression rules make sure > > > >> of > > > >> that. > > > > > > > > Yeah, tricky and maybe a gray area in this case. I think we eluded > > > > elsewhere in the thread that renaming the thing might be an option. > > > > > > > > So maybe instead of deadline, which is a very strong word, use > > > > something > > > > along the lines of "present time hint", or "signalled time hint"? > > > > Maybe > > > > reads clumsy. Just throwing some ideas for a start. > > > > > > You can try, but I fear that if it ever changes behaviour and > > > someone notices that, it's labelled as a kernel regression. I don't > > > think documentation has ever been the authoritative definition of > > > UABI > > > in Linux, it just guides drivers and userspace towards a common > > > understanding and common usage patterns. > > > > > > So even if the UABI contract is not documented (ugh), you need to be > > > prepared to set the UABI contract through kernel implementation. > > > >>> > > > >>> To be the devil's advocate it probably wouldn't be an ABI regression > > > >>> but > > > >>> just an regression. Same way as what nice(2) priorities mean hasn't > > > >>> always been the same over the years, I don't think there is a strict > > > >>> contract. > > > >>> > > > >>> Having said that, it may be different with latency sensitive stuff > > > >>> such > > > >>> as UIs though since it is very observable and can be very painful to > > > >>> users. > > > >>> > > > If you do not document the UABI contract, then different drivers are > > > likely to implement it differently, leading to differing behaviour. > > > Also userspace will invent wild ways to abuse the UABI if there is no > > > documentation guiding it on proper use. If userspace or end users > > > observe different behaviour, that's bad even if it's not a > > > regression. > > > > > > I don't like the situation either, but it is what it is. UABI > > > stability > > > trumps everything regardless of whether it was documented or not. > > > > > > I bet userspace is going to use this as a "make it faster, make it > > > hotter" button. I would not be surprised if someone wrote a > > > LD_PRELOAD > > > library that stamps any and all fences with an expired deadline to > > > just squeeze out a little more through some weird side-effect. > > > > > > Well, that's hopefully overboard in scaring, but in the end, I would > > > like to see UABI documented so I can have a feeling of what it is for > > > and how it was intended to be used. That's all. > > > >>> > > > >>> We share the
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Fri, Feb 24, 2023 at 09:59:57AM -0800, Rob Clark wrote: > On Fri, Feb 24, 2023 at 7:27 AM Luben Tuikov wrote: > > > > On 2023-02-24 06:37, Tvrtko Ursulin wrote: > > > > > > On 24/02/2023 11:00, Pekka Paalanen wrote: > > >> On Fri, 24 Feb 2023 10:50:51 + > > >> Tvrtko Ursulin wrote: > > >> > > >>> On 24/02/2023 10:24, Pekka Paalanen wrote: > > On Fri, 24 Feb 2023 09:41:46 + > > Tvrtko Ursulin wrote: > > > > > On 24/02/2023 09:26, Pekka Paalanen wrote: > > >> On Thu, 23 Feb 2023 10:51:48 -0800 > > >> Rob Clark wrote: > > >> > > >>> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > > >>> wrote: > > > > On Wed, 22 Feb 2023 07:37:26 -0800 > > Rob Clark wrote: > > > > > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > > > wrote: > > >> > > >> ... > > >> > > >> On another matter, if the application uses SET_DEADLINE with one > > >> timestamp, and the compositor uses SET_DEADLINE on the same > > >> thing with > > >> another timestamp, what should happen? > > > > > > The expectation is that many deadline hints can be set on a fence. > > > The fence signaller should track the soonest deadline. > > > > You need to document that as UAPI, since it is observable to > > userspace. > > It would be bad if drivers or subsystems would differ in behaviour. > > > > >>> > > >>> It is in the end a hint. It is about giving the driver more > > >>> information so that it can make better choices. But the driver is > > >>> even free to ignore it. So maybe "expectation" is too strong of a > > >>> word. Rather, any other behavior doesn't really make sense. But it > > >>> could end up being dictated by how the hw and/or fw works. > > >> > > >> It will stop being a hint once it has been implemented and used in > > >> the > > >> wild long enough. The kernel userspace regression rules make sure of > > >> that. > > > > > > Yeah, tricky and maybe a gray area in this case. I think we eluded > > > elsewhere in the thread that renaming the thing might be an option. > > > > > > So maybe instead of deadline, which is a very strong word, use > > > something > > > along the lines of "present time hint", or "signalled time hint"? > > > Maybe > > > reads clumsy. Just throwing some ideas for a start. > > > > You can try, but I fear that if it ever changes behaviour and > > someone notices that, it's labelled as a kernel regression. I don't > > think documentation has ever been the authoritative definition of UABI > > in Linux, it just guides drivers and userspace towards a common > > understanding and common usage patterns. > > > > So even if the UABI contract is not documented (ugh), you need to be > > prepared to set the UABI contract through kernel implementation. > > >>> > > >>> To be the devil's advocate it probably wouldn't be an ABI regression but > > >>> just an regression. Same way as what nice(2) priorities mean hasn't > > >>> always been the same over the years, I don't think there is a strict > > >>> contract. > > >>> > > >>> Having said that, it may be different with latency sensitive stuff such > > >>> as UIs though since it is very observable and can be very painful to > > >>> users. > > >>> > > If you do not document the UABI contract, then different drivers are > > likely to implement it differently, leading to differing behaviour. > > Also userspace will invent wild ways to abuse the UABI if there is no > > documentation guiding it on proper use. If userspace or end users > > observe different behaviour, that's bad even if it's not a regression. > > > > I don't like the situation either, but it is what it is. UABI stability > > trumps everything regardless of whether it was documented or not. > > > > I bet userspace is going to use this as a "make it faster, make it > > hotter" button. I would not be surprised if someone wrote a LD_PRELOAD > > library that stamps any and all fences with an expired deadline to > > just squeeze out a little more through some weird side-effect. > > > > Well, that's hopefully overboard in scaring, but in the end, I would > > like to see UABI documented so I can have a feeling of what it is for > > and how it was intended to be used. That's all. > > >>> > > >>> We share the same concern. If you read elsewhere in these threads you > > >>> will notice I have been calling this an "arms race". If the ability to > > >>> make yourself go faster does not required additional privilege I also > > >>> worry everyone will do it at which point it becomes pointless. So yes, I > > >>> do share this concern about exposing any of this as an unprivileged > > >>> uapi. > > >>> > > >>>
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Mon, Feb 27, 2023 at 1:34 AM Pekka Paalanen wrote: > > On Fri, 24 Feb 2023 11:44:53 -0800 > Rob Clark wrote: > > > On Fri, Feb 24, 2023 at 2:24 AM Pekka Paalanen wrote: > > > > > > On Fri, 24 Feb 2023 09:41:46 + > > > Tvrtko Ursulin wrote: > > > > > > > On 24/02/2023 09:26, Pekka Paalanen wrote: > > > > > On Thu, 23 Feb 2023 10:51:48 -0800 > > > > > Rob Clark wrote: > > > > > > > > > >> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > > > > >> wrote: > > > > >>> > > > > >>> On Wed, 22 Feb 2023 07:37:26 -0800 > > > > >>> Rob Clark wrote: > > > > >>> > > > > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > > > > wrote: > > > > > > > > > > ... > > > > > > > > > > On another matter, if the application uses SET_DEADLINE with one > > > > > timestamp, and the compositor uses SET_DEADLINE on the same thing > > > > > with > > > > > another timestamp, what should happen? > > > > > > > > The expectation is that many deadline hints can be set on a fence. > > > > The fence signaller should track the soonest deadline. > > > > >>> > > > > >>> You need to document that as UAPI, since it is observable to > > > > >>> userspace. > > > > >>> It would be bad if drivers or subsystems would differ in behaviour. > > > > >>> > > > > >> > > > > >> It is in the end a hint. It is about giving the driver more > > > > >> information so that it can make better choices. But the driver is > > > > >> even free to ignore it. So maybe "expectation" is too strong of a > > > > >> word. Rather, any other behavior doesn't really make sense. But it > > > > >> could end up being dictated by how the hw and/or fw works. > > > > > > > > > > It will stop being a hint once it has been implemented and used in the > > > > > wild long enough. The kernel userspace regression rules make sure of > > > > > that. > > > > > > > > Yeah, tricky and maybe a gray area in this case. I think we eluded > > > > elsewhere in the thread that renaming the thing might be an option. > > > > > > > > So maybe instead of deadline, which is a very strong word, use something > > > > along the lines of "present time hint", or "signalled time hint"? Maybe > > > > reads clumsy. Just throwing some ideas for a start. > > > > > > You can try, but I fear that if it ever changes behaviour and > > > someone notices that, it's labelled as a kernel regression. I don't > > > think documentation has ever been the authoritative definition of UABI > > > in Linux, it just guides drivers and userspace towards a common > > > understanding and common usage patterns. > > > > > > So even if the UABI contract is not documented (ugh), you need to be > > > prepared to set the UABI contract through kernel implementation. > > > > > > If you do not document the UABI contract, then different drivers are > > > likely to implement it differently, leading to differing behaviour. > > > Also userspace will invent wild ways to abuse the UABI if there is no > > > documentation guiding it on proper use. If userspace or end users > > > observe different behaviour, that's bad even if it's not a regression. > > > > > > I don't like the situation either, but it is what it is. UABI stability > > > trumps everything regardless of whether it was documented or not. > > > > > > I bet userspace is going to use this as a "make it faster, make it > > > hotter" button. I would not be surprised if someone wrote a LD_PRELOAD > > > library that stamps any and all fences with an expired deadline to > > > just squeeze out a little more through some weird side-effect. > > > > Userspace already has various (driver specific) debugfs/sysfs that it > > can use if it wants to make it hotter and drain batteries faster, so > > I'm not seeing a strong need to cater to the "turn it up to eleven" > > crowd here. And really your point feels like a good reason to _not_ > > document this as anything more than a hint. > > My point is that no matter what you say in documentation or leave > unsaid, people can and will abuse this by the behaviour it provides > anyway, like every other UABI. > > So why not just document what it is supposed to do? It cannot get any > worse. Maybe you get lucky instead and people don't abuse it that much > if they read the docs. > > E.g. can it affect GPU job scheduling, can it affect GPU clocks, can it > affect power consumption, and so on. I expect, potentially, all or any, or none of the above ;-) I guess I could document it as such, just to preempt potential complaints about broken spacebar-heater. The question is, where? I could add something about fence deadline hints in dma-buf.rst, which would I think be useful in general for driver writers. But there isn't really any existing documents other than headerdoc comments for sync_file ioctls, or drm_syncobj related ioctls (the latter are really just for mesa to use, so maybe that is ok). > > > Back in the real world, mobile games are already well aware of the fps > > vs battery-life (and
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Fri, 24 Feb 2023 11:44:53 -0800 Rob Clark wrote: > On Fri, Feb 24, 2023 at 2:24 AM Pekka Paalanen wrote: > > > > On Fri, 24 Feb 2023 09:41:46 + > > Tvrtko Ursulin wrote: > > > > > On 24/02/2023 09:26, Pekka Paalanen wrote: > > > > On Thu, 23 Feb 2023 10:51:48 -0800 > > > > Rob Clark wrote: > > > > > > > >> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > > > >> wrote: > > > >>> > > > >>> On Wed, 22 Feb 2023 07:37:26 -0800 > > > >>> Rob Clark wrote: > > > >>> > > > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > > > wrote: > > > > > > > > ... > > > > > > > > On another matter, if the application uses SET_DEADLINE with one > > > > timestamp, and the compositor uses SET_DEADLINE on the same thing > > > > with > > > > another timestamp, what should happen? > > > > > > The expectation is that many deadline hints can be set on a fence. > > > The fence signaller should track the soonest deadline. > > > >>> > > > >>> You need to document that as UAPI, since it is observable to > > > >>> userspace. > > > >>> It would be bad if drivers or subsystems would differ in behaviour. > > > >>> > > > >> > > > >> It is in the end a hint. It is about giving the driver more > > > >> information so that it can make better choices. But the driver is > > > >> even free to ignore it. So maybe "expectation" is too strong of a > > > >> word. Rather, any other behavior doesn't really make sense. But it > > > >> could end up being dictated by how the hw and/or fw works. > > > > > > > > It will stop being a hint once it has been implemented and used in the > > > > wild long enough. The kernel userspace regression rules make sure of > > > > that. > > > > > > Yeah, tricky and maybe a gray area in this case. I think we eluded > > > elsewhere in the thread that renaming the thing might be an option. > > > > > > So maybe instead of deadline, which is a very strong word, use something > > > along the lines of "present time hint", or "signalled time hint"? Maybe > > > reads clumsy. Just throwing some ideas for a start. > > > > You can try, but I fear that if it ever changes behaviour and > > someone notices that, it's labelled as a kernel regression. I don't > > think documentation has ever been the authoritative definition of UABI > > in Linux, it just guides drivers and userspace towards a common > > understanding and common usage patterns. > > > > So even if the UABI contract is not documented (ugh), you need to be > > prepared to set the UABI contract through kernel implementation. > > > > If you do not document the UABI contract, then different drivers are > > likely to implement it differently, leading to differing behaviour. > > Also userspace will invent wild ways to abuse the UABI if there is no > > documentation guiding it on proper use. If userspace or end users > > observe different behaviour, that's bad even if it's not a regression. > > > > I don't like the situation either, but it is what it is. UABI stability > > trumps everything regardless of whether it was documented or not. > > > > I bet userspace is going to use this as a "make it faster, make it > > hotter" button. I would not be surprised if someone wrote a LD_PRELOAD > > library that stamps any and all fences with an expired deadline to > > just squeeze out a little more through some weird side-effect. > > Userspace already has various (driver specific) debugfs/sysfs that it > can use if it wants to make it hotter and drain batteries faster, so > I'm not seeing a strong need to cater to the "turn it up to eleven" > crowd here. And really your point feels like a good reason to _not_ > document this as anything more than a hint. My point is that no matter what you say in documentation or leave unsaid, people can and will abuse this by the behaviour it provides anyway, like every other UABI. So why not just document what it is supposed to do? It cannot get any worse. Maybe you get lucky instead and people don't abuse it that much if they read the docs. E.g. can it affect GPU job scheduling, can it affect GPU clocks, can it affect power consumption, and so on. > Back in the real world, mobile games are already well aware of the fps > vs battery-life (and therefore gameplay) tradeoff. But what is > missing is a way to inform the kernel of userspace's intentions, so > that gpu dvfs can make intelligent decisions. This series is meant to > bridge that gap. Then document that. As long as you document it properly: what you expect it to be used for and how. Or if this is reserved strictly for Mesa drivers, then document that. You can also stop CC'ing me if you don't want attention to UABI docs. I don't read dri-devel@ unless I'm explicitly CC'd nowadays. Thanks, pq pgpeZOjTsYxQF.pgp Description: OpenPGP digital signature
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Fri, Feb 24, 2023 at 2:24 AM Pekka Paalanen wrote: > > On Fri, 24 Feb 2023 09:41:46 + > Tvrtko Ursulin wrote: > > > On 24/02/2023 09:26, Pekka Paalanen wrote: > > > On Thu, 23 Feb 2023 10:51:48 -0800 > > > Rob Clark wrote: > > > > > >> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > > >> wrote: > > >>> > > >>> On Wed, 22 Feb 2023 07:37:26 -0800 > > >>> Rob Clark wrote: > > >>> > > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > > wrote: > > > > > > ... > > > > > > On another matter, if the application uses SET_DEADLINE with one > > > timestamp, and the compositor uses SET_DEADLINE on the same thing with > > > another timestamp, what should happen? > > > > The expectation is that many deadline hints can be set on a fence. > > The fence signaller should track the soonest deadline. > > >>> > > >>> You need to document that as UAPI, since it is observable to userspace. > > >>> It would be bad if drivers or subsystems would differ in behaviour. > > >>> > > >> > > >> It is in the end a hint. It is about giving the driver more > > >> information so that it can make better choices. But the driver is > > >> even free to ignore it. So maybe "expectation" is too strong of a > > >> word. Rather, any other behavior doesn't really make sense. But it > > >> could end up being dictated by how the hw and/or fw works. > > > > > > It will stop being a hint once it has been implemented and used in the > > > wild long enough. The kernel userspace regression rules make sure of > > > that. > > > > Yeah, tricky and maybe a gray area in this case. I think we eluded > > elsewhere in the thread that renaming the thing might be an option. > > > > So maybe instead of deadline, which is a very strong word, use something > > along the lines of "present time hint", or "signalled time hint"? Maybe > > reads clumsy. Just throwing some ideas for a start. > > You can try, but I fear that if it ever changes behaviour and > someone notices that, it's labelled as a kernel regression. I don't > think documentation has ever been the authoritative definition of UABI > in Linux, it just guides drivers and userspace towards a common > understanding and common usage patterns. > > So even if the UABI contract is not documented (ugh), you need to be > prepared to set the UABI contract through kernel implementation. > > If you do not document the UABI contract, then different drivers are > likely to implement it differently, leading to differing behaviour. > Also userspace will invent wild ways to abuse the UABI if there is no > documentation guiding it on proper use. If userspace or end users > observe different behaviour, that's bad even if it's not a regression. > > I don't like the situation either, but it is what it is. UABI stability > trumps everything regardless of whether it was documented or not. > > I bet userspace is going to use this as a "make it faster, make it > hotter" button. I would not be surprised if someone wrote a LD_PRELOAD > library that stamps any and all fences with an expired deadline to > just squeeze out a little more through some weird side-effect. Userspace already has various (driver specific) debugfs/sysfs that it can use if it wants to make it hotter and drain batteries faster, so I'm not seeing a strong need to cater to the "turn it up to eleven" crowd here. And really your point feels like a good reason to _not_ document this as anything more than a hint. Back in the real world, mobile games are already well aware of the fps vs battery-life (and therefore gameplay) tradeoff. But what is missing is a way to inform the kernel of userspace's intentions, so that gpu dvfs can make intelligent decisions. This series is meant to bridge that gap. BR, -R > Well, that's hopefully overboard in scaring, but in the end, I would > like to see UABI documented so I can have a feeling of what it is for > and how it was intended to be used. That's all. > > > Thanks, > pq
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Fri, Feb 24, 2023 at 7:27 AM Luben Tuikov wrote: > > On 2023-02-24 06:37, Tvrtko Ursulin wrote: > > > > On 24/02/2023 11:00, Pekka Paalanen wrote: > >> On Fri, 24 Feb 2023 10:50:51 + > >> Tvrtko Ursulin wrote: > >> > >>> On 24/02/2023 10:24, Pekka Paalanen wrote: > On Fri, 24 Feb 2023 09:41:46 + > Tvrtko Ursulin wrote: > > > On 24/02/2023 09:26, Pekka Paalanen wrote: > >> On Thu, 23 Feb 2023 10:51:48 -0800 > >> Rob Clark wrote: > >> > >>> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > >>> wrote: > > On Wed, 22 Feb 2023 07:37:26 -0800 > Rob Clark wrote: > > > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > > wrote: > >> > >> ... > >> > >> On another matter, if the application uses SET_DEADLINE with one > >> timestamp, and the compositor uses SET_DEADLINE on the same thing > >> with > >> another timestamp, what should happen? > > > > The expectation is that many deadline hints can be set on a fence. > > The fence signaller should track the soonest deadline. > > You need to document that as UAPI, since it is observable to > userspace. > It would be bad if drivers or subsystems would differ in behaviour. > > >>> > >>> It is in the end a hint. It is about giving the driver more > >>> information so that it can make better choices. But the driver is > >>> even free to ignore it. So maybe "expectation" is too strong of a > >>> word. Rather, any other behavior doesn't really make sense. But it > >>> could end up being dictated by how the hw and/or fw works. > >> > >> It will stop being a hint once it has been implemented and used in the > >> wild long enough. The kernel userspace regression rules make sure of > >> that. > > > > Yeah, tricky and maybe a gray area in this case. I think we eluded > > elsewhere in the thread that renaming the thing might be an option. > > > > So maybe instead of deadline, which is a very strong word, use something > > along the lines of "present time hint", or "signalled time hint"? Maybe > > reads clumsy. Just throwing some ideas for a start. > > You can try, but I fear that if it ever changes behaviour and > someone notices that, it's labelled as a kernel regression. I don't > think documentation has ever been the authoritative definition of UABI > in Linux, it just guides drivers and userspace towards a common > understanding and common usage patterns. > > So even if the UABI contract is not documented (ugh), you need to be > prepared to set the UABI contract through kernel implementation. > >>> > >>> To be the devil's advocate it probably wouldn't be an ABI regression but > >>> just an regression. Same way as what nice(2) priorities mean hasn't > >>> always been the same over the years, I don't think there is a strict > >>> contract. > >>> > >>> Having said that, it may be different with latency sensitive stuff such > >>> as UIs though since it is very observable and can be very painful to > >>> users. > >>> > If you do not document the UABI contract, then different drivers are > likely to implement it differently, leading to differing behaviour. > Also userspace will invent wild ways to abuse the UABI if there is no > documentation guiding it on proper use. If userspace or end users > observe different behaviour, that's bad even if it's not a regression. > > I don't like the situation either, but it is what it is. UABI stability > trumps everything regardless of whether it was documented or not. > > I bet userspace is going to use this as a "make it faster, make it > hotter" button. I would not be surprised if someone wrote a LD_PRELOAD > library that stamps any and all fences with an expired deadline to > just squeeze out a little more through some weird side-effect. > > Well, that's hopefully overboard in scaring, but in the end, I would > like to see UABI documented so I can have a feeling of what it is for > and how it was intended to be used. That's all. > >>> > >>> We share the same concern. If you read elsewhere in these threads you > >>> will notice I have been calling this an "arms race". If the ability to > >>> make yourself go faster does not required additional privilege I also > >>> worry everyone will do it at which point it becomes pointless. So yes, I > >>> do share this concern about exposing any of this as an unprivileged uapi. > >>> > >>> Is it possible to limit access to only compositors in some sane way? > >>> Sounds tricky when dma-fence should be disconnected from DRM.. > >> > >> Maybe it's not that bad in this particular case, because we are talking > >> only about boosting GPU clocks which benefits everyone (except > >> battery
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Fri, Feb 24, 2023 at 2:24 AM Pekka Paalanen wrote: > > On Fri, 24 Feb 2023 09:41:46 + > Tvrtko Ursulin wrote: > > > On 24/02/2023 09:26, Pekka Paalanen wrote: > > > On Thu, 23 Feb 2023 10:51:48 -0800 > > > Rob Clark wrote: > > > > > >> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > > >> wrote: > > >>> > > >>> On Wed, 22 Feb 2023 07:37:26 -0800 > > >>> Rob Clark wrote: > > >>> > > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > > wrote: > > > > > > ... > > > > > > On another matter, if the application uses SET_DEADLINE with one > > > timestamp, and the compositor uses SET_DEADLINE on the same thing with > > > another timestamp, what should happen? > > > > The expectation is that many deadline hints can be set on a fence. > > The fence signaller should track the soonest deadline. > > >>> > > >>> You need to document that as UAPI, since it is observable to userspace. > > >>> It would be bad if drivers or subsystems would differ in behaviour. > > >>> > > >> > > >> It is in the end a hint. It is about giving the driver more > > >> information so that it can make better choices. But the driver is > > >> even free to ignore it. So maybe "expectation" is too strong of a > > >> word. Rather, any other behavior doesn't really make sense. But it > > >> could end up being dictated by how the hw and/or fw works. > > > > > > It will stop being a hint once it has been implemented and used in the > > > wild long enough. The kernel userspace regression rules make sure of > > > that. > > > > Yeah, tricky and maybe a gray area in this case. I think we eluded > > elsewhere in the thread that renaming the thing might be an option. > > > > So maybe instead of deadline, which is a very strong word, use something > > along the lines of "present time hint", or "signalled time hint"? Maybe > > reads clumsy. Just throwing some ideas for a start. > > You can try, but I fear that if it ever changes behaviour and > someone notices that, it's labelled as a kernel regression. I don't > think documentation has ever been the authoritative definition of UABI > in Linux, it just guides drivers and userspace towards a common > understanding and common usage patterns. > > So even if the UABI contract is not documented (ugh), you need to be > prepared to set the UABI contract through kernel implementation. > > If you do not document the UABI contract, then different drivers are > likely to implement it differently, leading to differing behaviour. > Also userspace will invent wild ways to abuse the UABI if there is no > documentation guiding it on proper use. If userspace or end users > observe different behaviour, that's bad even if it's not a regression. > > I don't like the situation either, but it is what it is. UABI stability > trumps everything regardless of whether it was documented or not. > > I bet userspace is going to use this as a "make it faster, make it > hotter" button. I would not be surprised if someone wrote a LD_PRELOAD > library that stamps any and all fences with an expired deadline to > just squeeze out a little more through some weird side-effect. Ok, maybe we can rename the SET_DEADLINE ioctl to SPACEBAR_HEATER ;-) BR, -R > Well, that's hopefully overboard in scaring, but in the end, I would > like to see UABI documented so I can have a feeling of what it is for > and how it was intended to be used. That's all. > > > Thanks, > pq
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On 2023-02-24 06:37, Tvrtko Ursulin wrote: > > On 24/02/2023 11:00, Pekka Paalanen wrote: >> On Fri, 24 Feb 2023 10:50:51 + >> Tvrtko Ursulin wrote: >> >>> On 24/02/2023 10:24, Pekka Paalanen wrote: On Fri, 24 Feb 2023 09:41:46 + Tvrtko Ursulin wrote: > On 24/02/2023 09:26, Pekka Paalanen wrote: >> On Thu, 23 Feb 2023 10:51:48 -0800 >> Rob Clark wrote: >> >>> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen >>> wrote: On Wed, 22 Feb 2023 07:37:26 -0800 Rob Clark wrote: > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > wrote: >> >> ... >> >> On another matter, if the application uses SET_DEADLINE with one >> timestamp, and the compositor uses SET_DEADLINE on the same thing >> with >> another timestamp, what should happen? > > The expectation is that many deadline hints can be set on a fence. > The fence signaller should track the soonest deadline. You need to document that as UAPI, since it is observable to userspace. It would be bad if drivers or subsystems would differ in behaviour. >>> >>> It is in the end a hint. It is about giving the driver more >>> information so that it can make better choices. But the driver is >>> even free to ignore it. So maybe "expectation" is too strong of a >>> word. Rather, any other behavior doesn't really make sense. But it >>> could end up being dictated by how the hw and/or fw works. >> >> It will stop being a hint once it has been implemented and used in the >> wild long enough. The kernel userspace regression rules make sure of >> that. > > Yeah, tricky and maybe a gray area in this case. I think we eluded > elsewhere in the thread that renaming the thing might be an option. > > So maybe instead of deadline, which is a very strong word, use something > along the lines of "present time hint", or "signalled time hint"? Maybe > reads clumsy. Just throwing some ideas for a start. You can try, but I fear that if it ever changes behaviour and someone notices that, it's labelled as a kernel regression. I don't think documentation has ever been the authoritative definition of UABI in Linux, it just guides drivers and userspace towards a common understanding and common usage patterns. So even if the UABI contract is not documented (ugh), you need to be prepared to set the UABI contract through kernel implementation. >>> >>> To be the devil's advocate it probably wouldn't be an ABI regression but >>> just an regression. Same way as what nice(2) priorities mean hasn't >>> always been the same over the years, I don't think there is a strict >>> contract. >>> >>> Having said that, it may be different with latency sensitive stuff such >>> as UIs though since it is very observable and can be very painful to users. >>> If you do not document the UABI contract, then different drivers are likely to implement it differently, leading to differing behaviour. Also userspace will invent wild ways to abuse the UABI if there is no documentation guiding it on proper use. If userspace or end users observe different behaviour, that's bad even if it's not a regression. I don't like the situation either, but it is what it is. UABI stability trumps everything regardless of whether it was documented or not. I bet userspace is going to use this as a "make it faster, make it hotter" button. I would not be surprised if someone wrote a LD_PRELOAD library that stamps any and all fences with an expired deadline to just squeeze out a little more through some weird side-effect. Well, that's hopefully overboard in scaring, but in the end, I would like to see UABI documented so I can have a feeling of what it is for and how it was intended to be used. That's all. >>> >>> We share the same concern. If you read elsewhere in these threads you >>> will notice I have been calling this an "arms race". If the ability to >>> make yourself go faster does not required additional privilege I also >>> worry everyone will do it at which point it becomes pointless. So yes, I >>> do share this concern about exposing any of this as an unprivileged uapi. >>> >>> Is it possible to limit access to only compositors in some sane way? >>> Sounds tricky when dma-fence should be disconnected from DRM.. >> >> Maybe it's not that bad in this particular case, because we are talking >> only about boosting GPU clocks which benefits everyone (except >> battery life) and it does not penalize other programs like e.g. >> job priorities do. > > Apart from efficiency that you mentioned, which does not always favor > higher clocks, sometimes thermal budget is also shared between CPU and > GPU. So
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On 24/02/2023 11:00, Pekka Paalanen wrote: On Fri, 24 Feb 2023 10:50:51 + Tvrtko Ursulin wrote: On 24/02/2023 10:24, Pekka Paalanen wrote: On Fri, 24 Feb 2023 09:41:46 + Tvrtko Ursulin wrote: On 24/02/2023 09:26, Pekka Paalanen wrote: On Thu, 23 Feb 2023 10:51:48 -0800 Rob Clark wrote: On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen wrote: On Wed, 22 Feb 2023 07:37:26 -0800 Rob Clark wrote: On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen wrote: ... On another matter, if the application uses SET_DEADLINE with one timestamp, and the compositor uses SET_DEADLINE on the same thing with another timestamp, what should happen? The expectation is that many deadline hints can be set on a fence. The fence signaller should track the soonest deadline. You need to document that as UAPI, since it is observable to userspace. It would be bad if drivers or subsystems would differ in behaviour. It is in the end a hint. It is about giving the driver more information so that it can make better choices. But the driver is even free to ignore it. So maybe "expectation" is too strong of a word. Rather, any other behavior doesn't really make sense. But it could end up being dictated by how the hw and/or fw works. It will stop being a hint once it has been implemented and used in the wild long enough. The kernel userspace regression rules make sure of that. Yeah, tricky and maybe a gray area in this case. I think we eluded elsewhere in the thread that renaming the thing might be an option. So maybe instead of deadline, which is a very strong word, use something along the lines of "present time hint", or "signalled time hint"? Maybe reads clumsy. Just throwing some ideas for a start. You can try, but I fear that if it ever changes behaviour and someone notices that, it's labelled as a kernel regression. I don't think documentation has ever been the authoritative definition of UABI in Linux, it just guides drivers and userspace towards a common understanding and common usage patterns. So even if the UABI contract is not documented (ugh), you need to be prepared to set the UABI contract through kernel implementation. To be the devil's advocate it probably wouldn't be an ABI regression but just an regression. Same way as what nice(2) priorities mean hasn't always been the same over the years, I don't think there is a strict contract. Having said that, it may be different with latency sensitive stuff such as UIs though since it is very observable and can be very painful to users. If you do not document the UABI contract, then different drivers are likely to implement it differently, leading to differing behaviour. Also userspace will invent wild ways to abuse the UABI if there is no documentation guiding it on proper use. If userspace or end users observe different behaviour, that's bad even if it's not a regression. I don't like the situation either, but it is what it is. UABI stability trumps everything regardless of whether it was documented or not. I bet userspace is going to use this as a "make it faster, make it hotter" button. I would not be surprised if someone wrote a LD_PRELOAD library that stamps any and all fences with an expired deadline to just squeeze out a little more through some weird side-effect. Well, that's hopefully overboard in scaring, but in the end, I would like to see UABI documented so I can have a feeling of what it is for and how it was intended to be used. That's all. We share the same concern. If you read elsewhere in these threads you will notice I have been calling this an "arms race". If the ability to make yourself go faster does not required additional privilege I also worry everyone will do it at which point it becomes pointless. So yes, I do share this concern about exposing any of this as an unprivileged uapi. Is it possible to limit access to only compositors in some sane way? Sounds tricky when dma-fence should be disconnected from DRM.. Maybe it's not that bad in this particular case, because we are talking only about boosting GPU clocks which benefits everyone (except battery life) and it does not penalize other programs like e.g. job priorities do. Apart from efficiency that you mentioned, which does not always favor higher clocks, sometimes thermal budget is also shared between CPU and GPU. So more GPU clocks can mean fewer CPU clocks. It's really hard to make optimal choices without the full coordination between both schedulers. But that is even not the main point, which is that if everyone sets the immediate deadline then having the deadline API is a bit pointless. For instance there is a reason negative nice needs CAP_SYS_ADMIN. However Rob has also pointed out the existence of uclamp.min via sched_setattr which is unprivileged and can influence frequency selection in the CPU world, so I conceded on that point. If CPU world has accepted it so can we I guess. So IMO we are back
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Fri, 24 Feb 2023 10:50:51 + Tvrtko Ursulin wrote: > On 24/02/2023 10:24, Pekka Paalanen wrote: > > On Fri, 24 Feb 2023 09:41:46 + > > Tvrtko Ursulin wrote: > > > >> On 24/02/2023 09:26, Pekka Paalanen wrote: > >>> On Thu, 23 Feb 2023 10:51:48 -0800 > >>> Rob Clark wrote: > >>> > On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > wrote: > > > > On Wed, 22 Feb 2023 07:37:26 -0800 > > Rob Clark wrote: > > > >> On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > >> wrote: > >>> > >>> ... > >>> > >>> On another matter, if the application uses SET_DEADLINE with one > >>> timestamp, and the compositor uses SET_DEADLINE on the same thing with > >>> another timestamp, what should happen? > >> > >> The expectation is that many deadline hints can be set on a fence. > >> The fence signaller should track the soonest deadline. > > > > You need to document that as UAPI, since it is observable to userspace. > > It would be bad if drivers or subsystems would differ in behaviour. > > > > It is in the end a hint. It is about giving the driver more > information so that it can make better choices. But the driver is > even free to ignore it. So maybe "expectation" is too strong of a > word. Rather, any other behavior doesn't really make sense. But it > could end up being dictated by how the hw and/or fw works. > >>> > >>> It will stop being a hint once it has been implemented and used in the > >>> wild long enough. The kernel userspace regression rules make sure of > >>> that. > >> > >> Yeah, tricky and maybe a gray area in this case. I think we eluded > >> elsewhere in the thread that renaming the thing might be an option. > >> > >> So maybe instead of deadline, which is a very strong word, use something > >> along the lines of "present time hint", or "signalled time hint"? Maybe > >> reads clumsy. Just throwing some ideas for a start. > > > > You can try, but I fear that if it ever changes behaviour and > > someone notices that, it's labelled as a kernel regression. I don't > > think documentation has ever been the authoritative definition of UABI > > in Linux, it just guides drivers and userspace towards a common > > understanding and common usage patterns. > > > > So even if the UABI contract is not documented (ugh), you need to be > > prepared to set the UABI contract through kernel implementation. > > To be the devil's advocate it probably wouldn't be an ABI regression but > just an regression. Same way as what nice(2) priorities mean hasn't > always been the same over the years, I don't think there is a strict > contract. > > Having said that, it may be different with latency sensitive stuff such > as UIs though since it is very observable and can be very painful to users. > > > If you do not document the UABI contract, then different drivers are > > likely to implement it differently, leading to differing behaviour. > > Also userspace will invent wild ways to abuse the UABI if there is no > > documentation guiding it on proper use. If userspace or end users > > observe different behaviour, that's bad even if it's not a regression. > > > > I don't like the situation either, but it is what it is. UABI stability > > trumps everything regardless of whether it was documented or not. > > > > I bet userspace is going to use this as a "make it faster, make it > > hotter" button. I would not be surprised if someone wrote a LD_PRELOAD > > library that stamps any and all fences with an expired deadline to > > just squeeze out a little more through some weird side-effect. > > > > Well, that's hopefully overboard in scaring, but in the end, I would > > like to see UABI documented so I can have a feeling of what it is for > > and how it was intended to be used. That's all. > > We share the same concern. If you read elsewhere in these threads you > will notice I have been calling this an "arms race". If the ability to > make yourself go faster does not required additional privilege I also > worry everyone will do it at which point it becomes pointless. So yes, I > do share this concern about exposing any of this as an unprivileged uapi. > > Is it possible to limit access to only compositors in some sane way? > Sounds tricky when dma-fence should be disconnected from DRM.. Maybe it's not that bad in this particular case, because we are talking only about boosting GPU clocks which benefits everyone (except battery life) and it does not penalize other programs like e.g. job priorities do. Drivers are not going to use the deadline for scheduling priorities, right? I don't recall seeing any mention of that. ...right? Thanks, pq pgpVo2EngByFO.pgp Description: OpenPGP digital signature
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On 24/02/2023 10:24, Pekka Paalanen wrote: On Fri, 24 Feb 2023 09:41:46 + Tvrtko Ursulin wrote: On 24/02/2023 09:26, Pekka Paalanen wrote: On Thu, 23 Feb 2023 10:51:48 -0800 Rob Clark wrote: On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen wrote: On Wed, 22 Feb 2023 07:37:26 -0800 Rob Clark wrote: On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen wrote: ... On another matter, if the application uses SET_DEADLINE with one timestamp, and the compositor uses SET_DEADLINE on the same thing with another timestamp, what should happen? The expectation is that many deadline hints can be set on a fence. The fence signaller should track the soonest deadline. You need to document that as UAPI, since it is observable to userspace. It would be bad if drivers or subsystems would differ in behaviour. It is in the end a hint. It is about giving the driver more information so that it can make better choices. But the driver is even free to ignore it. So maybe "expectation" is too strong of a word. Rather, any other behavior doesn't really make sense. But it could end up being dictated by how the hw and/or fw works. It will stop being a hint once it has been implemented and used in the wild long enough. The kernel userspace regression rules make sure of that. Yeah, tricky and maybe a gray area in this case. I think we eluded elsewhere in the thread that renaming the thing might be an option. So maybe instead of deadline, which is a very strong word, use something along the lines of "present time hint", or "signalled time hint"? Maybe reads clumsy. Just throwing some ideas for a start. You can try, but I fear that if it ever changes behaviour and someone notices that, it's labelled as a kernel regression. I don't think documentation has ever been the authoritative definition of UABI in Linux, it just guides drivers and userspace towards a common understanding and common usage patterns. So even if the UABI contract is not documented (ugh), you need to be prepared to set the UABI contract through kernel implementation. To be the devil's advocate it probably wouldn't be an ABI regression but just an regression. Same way as what nice(2) priorities mean hasn't always been the same over the years, I don't think there is a strict contract. Having said that, it may be different with latency sensitive stuff such as UIs though since it is very observable and can be very painful to users. If you do not document the UABI contract, then different drivers are likely to implement it differently, leading to differing behaviour. Also userspace will invent wild ways to abuse the UABI if there is no documentation guiding it on proper use. If userspace or end users observe different behaviour, that's bad even if it's not a regression. I don't like the situation either, but it is what it is. UABI stability trumps everything regardless of whether it was documented or not. I bet userspace is going to use this as a "make it faster, make it hotter" button. I would not be surprised if someone wrote a LD_PRELOAD library that stamps any and all fences with an expired deadline to just squeeze out a little more through some weird side-effect. Well, that's hopefully overboard in scaring, but in the end, I would like to see UABI documented so I can have a feeling of what it is for and how it was intended to be used. That's all. We share the same concern. If you read elsewhere in these threads you will notice I have been calling this an "arms race". If the ability to make yourself go faster does not required additional privilege I also worry everyone will do it at which point it becomes pointless. So yes, I do share this concern about exposing any of this as an unprivileged uapi. Is it possible to limit access to only compositors in some sane way? Sounds tricky when dma-fence should be disconnected from DRM.. Regards, Tvrtko
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Fri, 24 Feb 2023 09:41:46 + Tvrtko Ursulin wrote: > On 24/02/2023 09:26, Pekka Paalanen wrote: > > On Thu, 23 Feb 2023 10:51:48 -0800 > > Rob Clark wrote: > > > >> On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen > >> wrote: > >>> > >>> On Wed, 22 Feb 2023 07:37:26 -0800 > >>> Rob Clark wrote: > >>> > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > wrote: > > > > ... > > > > On another matter, if the application uses SET_DEADLINE with one > > timestamp, and the compositor uses SET_DEADLINE on the same thing with > > another timestamp, what should happen? > > The expectation is that many deadline hints can be set on a fence. > The fence signaller should track the soonest deadline. > >>> > >>> You need to document that as UAPI, since it is observable to userspace. > >>> It would be bad if drivers or subsystems would differ in behaviour. > >>> > >> > >> It is in the end a hint. It is about giving the driver more > >> information so that it can make better choices. But the driver is > >> even free to ignore it. So maybe "expectation" is too strong of a > >> word. Rather, any other behavior doesn't really make sense. But it > >> could end up being dictated by how the hw and/or fw works. > > > > It will stop being a hint once it has been implemented and used in the > > wild long enough. The kernel userspace regression rules make sure of > > that. > > Yeah, tricky and maybe a gray area in this case. I think we eluded > elsewhere in the thread that renaming the thing might be an option. > > So maybe instead of deadline, which is a very strong word, use something > along the lines of "present time hint", or "signalled time hint"? Maybe > reads clumsy. Just throwing some ideas for a start. You can try, but I fear that if it ever changes behaviour and someone notices that, it's labelled as a kernel regression. I don't think documentation has ever been the authoritative definition of UABI in Linux, it just guides drivers and userspace towards a common understanding and common usage patterns. So even if the UABI contract is not documented (ugh), you need to be prepared to set the UABI contract through kernel implementation. If you do not document the UABI contract, then different drivers are likely to implement it differently, leading to differing behaviour. Also userspace will invent wild ways to abuse the UABI if there is no documentation guiding it on proper use. If userspace or end users observe different behaviour, that's bad even if it's not a regression. I don't like the situation either, but it is what it is. UABI stability trumps everything regardless of whether it was documented or not. I bet userspace is going to use this as a "make it faster, make it hotter" button. I would not be surprised if someone wrote a LD_PRELOAD library that stamps any and all fences with an expired deadline to just squeeze out a little more through some weird side-effect. Well, that's hopefully overboard in scaring, but in the end, I would like to see UABI documented so I can have a feeling of what it is for and how it was intended to be used. That's all. Thanks, pq pgptX5jK8Yo4f.pgp Description: OpenPGP digital signature
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On 24/02/2023 09:26, Pekka Paalanen wrote: On Thu, 23 Feb 2023 10:51:48 -0800 Rob Clark wrote: On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen wrote: On Wed, 22 Feb 2023 07:37:26 -0800 Rob Clark wrote: On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen wrote: ... On another matter, if the application uses SET_DEADLINE with one timestamp, and the compositor uses SET_DEADLINE on the same thing with another timestamp, what should happen? The expectation is that many deadline hints can be set on a fence. The fence signaller should track the soonest deadline. You need to document that as UAPI, since it is observable to userspace. It would be bad if drivers or subsystems would differ in behaviour. It is in the end a hint. It is about giving the driver more information so that it can make better choices. But the driver is even free to ignore it. So maybe "expectation" is too strong of a word. Rather, any other behavior doesn't really make sense. But it could end up being dictated by how the hw and/or fw works. It will stop being a hint once it has been implemented and used in the wild long enough. The kernel userspace regression rules make sure of that. Yeah, tricky and maybe a gray area in this case. I think we eluded elsewhere in the thread that renaming the thing might be an option. So maybe instead of deadline, which is a very strong word, use something along the lines of "present time hint", or "signalled time hint"? Maybe reads clumsy. Just throwing some ideas for a start. Regards, Tvrtko See the topic of implementing triple-buffering in Mutter in order to put more work to the GPU in order to have the GPU ramp up clocks in order to not miss rendering deadlines. I don't think that patch set has landed in Mutter upstream, but I hear distributions in downstream are already carrying it. https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1383 https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1441 Granted, GPU clocks are just one side of that story it seems, and triple-buffering may have other benefits. If SET_DEADLINE would fix that problem without triple-buffering, it is definitely userspace observable, expected and eventually required behaviour. Thanks, pq
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Thu, 23 Feb 2023 10:51:48 -0800 Rob Clark wrote: > On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen wrote: > > > > On Wed, 22 Feb 2023 07:37:26 -0800 > > Rob Clark wrote: > > > > > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen > > > wrote: ... > > > > On another matter, if the application uses SET_DEADLINE with one > > > > timestamp, and the compositor uses SET_DEADLINE on the same thing with > > > > another timestamp, what should happen? > > > > > > The expectation is that many deadline hints can be set on a fence. > > > The fence signaller should track the soonest deadline. > > > > You need to document that as UAPI, since it is observable to userspace. > > It would be bad if drivers or subsystems would differ in behaviour. > > > > It is in the end a hint. It is about giving the driver more > information so that it can make better choices. But the driver is > even free to ignore it. So maybe "expectation" is too strong of a > word. Rather, any other behavior doesn't really make sense. But it > could end up being dictated by how the hw and/or fw works. It will stop being a hint once it has been implemented and used in the wild long enough. The kernel userspace regression rules make sure of that. See the topic of implementing triple-buffering in Mutter in order to put more work to the GPU in order to have the GPU ramp up clocks in order to not miss rendering deadlines. I don't think that patch set has landed in Mutter upstream, but I hear distributions in downstream are already carrying it. https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1383 https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1441 Granted, GPU clocks are just one side of that story it seems, and triple-buffering may have other benefits. If SET_DEADLINE would fix that problem without triple-buffering, it is definitely userspace observable, expected and eventually required behaviour. Thanks, pq pgpvqVGbQ3KLU.pgp Description: OpenPGP digital signature
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Thu, Feb 23, 2023 at 1:38 AM Pekka Paalanen wrote: > > On Wed, 22 Feb 2023 07:37:26 -0800 > Rob Clark wrote: > > > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen wrote: > > > > > > On Tue, 21 Feb 2023 09:53:56 -0800 > > > Rob Clark wrote: > > > > > > > On Tue, Feb 21, 2023 at 8:48 AM Luben Tuikov > > > > wrote: > > > > > > > > > > On 2023-02-20 11:14, Rob Clark wrote: > > > > > > On Mon, Feb 20, 2023 at 12:53 AM Pekka Paalanen > > > > > > wrote: > > > > > >> > > > > > >> On Sat, 18 Feb 2023 13:15:49 -0800 > > > > > >> Rob Clark wrote: > > > > > >> > > > > > >>> From: Rob Clark > > > > > >>> > > > > > >>> Allow userspace to use the EPOLLPRI/POLLPRI flag to indicate an > > > > > >>> urgent > > > > > >>> wait (as opposed to a "housekeeping" wait to know when to cleanup > > > > > >>> after > > > > > >>> some work has completed). Usermode components of GPU driver > > > > > >>> stacks > > > > > >>> often poll() on fence fd's to know when it is safe to do things > > > > > >>> like > > > > > >>> free or reuse a buffer, but they can also poll() on a fence fd > > > > > >>> when > > > > > >>> waiting to read back results from the GPU. The EPOLLPRI/POLLPRI > > > > > >>> flag > > > > > >>> lets the kernel differentiate these two cases. > > > > > >>> > > > > > >>> Signed-off-by: Rob Clark > > > > > >> > > > > > >> Hi, > > > > > >> > > > > > >> where would the UAPI documentation of this go? > > > > > >> It seems to be missing. > > > > > > > > > > > > Good question, I am not sure. The poll() man page has a > > > > > > description, > > > > > > but my usage doesn't fit that _exactly_ (but OTOH the description > > > > > > is a > > > > > > bit vague). > > > > > > > > > > > >> If a Wayland compositor is polling application fences to know which > > > > > >> client buffer to use in its rendering, should the compositor poll > > > > > >> with > > > > > >> PRI or not? If a compositor polls with PRI, then all fences from > > > > > >> all > > > > > >> applications would always be PRI. Would that be harmful somehow or > > > > > >> would it be beneficial? > > > > > > > > > > > > I think a compositor would rather use the deadline ioctl and then > > > > > > poll > > > > > > without PRI. Otherwise you are giving an urgency signal to the > > > > > > fence > > > > > > signaller which might not necessarily be needed. > > > > > > > > > > > > The places where I expect PRI to be useful is more in mesa (things > > > > > > like glFinish(), readpix, and other similar sorts of blocking APIs) > > > > > Hi, > > > > > > > > > > Hmm, but then user-space could do the opposite, namely, submit work > > > > > as usual--never > > > > > using the SET_DEADLINE ioctl, and then at the end, poll using > > > > > (E)POLLPRI. That seems > > > > > like a possible usage pattern, unintended--maybe, but possible. Do we > > > > > want to discourage > > > > > this? Wouldn't SET_DEADLINE be enough? I mean, one can call > > > > > SET_DEADLINE with the current > > > > > time, and then wouldn't that be equivalent to (E)POLLPRI? > > > > > > > > Yeah, (E)POLLPRI isn't strictly needed if we have SET_DEADLINE. It is > > > > slightly more convenient if you want an immediate deadline (single > > > > syscall instead of two), but not strictly needed. OTOH it piggy-backs > > > > on existing UABI. > > > > > > In that case, I would be conservative, and not add the POLLPRI > > > semantics. An UAPI addition that is not strictly needed and somewhat > > > unclear if it violates any design principles is best not done, until it > > > is proven to be beneficial. > > > > > > Besides, a Wayland compositor does not necessary need to add the fd > > > to its main event loop for poll. It could just SET_DEADLINE, and then > > > when it renders simply check if the fence passed or not already. Not > > > polling means the compositor does not need to wake up at the moment the > > > fence signals to just record a flag. > > > > poll(POLLPRI) isn't intended for wayland.. but is a thing I want in > > mesa for fence waits. I _could_ use SET_DEADLINE but it is two > > syscalls and correspondingly more code ;-) > > But is it actually beneficial? "More code" seems quite irrelevant. > > Would there be a hundred or more of those per frame? Or would it be > always limited to one or two? Or totally depend on what the application > is doing? Is it a significant impact? In general, any time the CPU is waiting on the GPU, you have already lost. So I don't think the extra syscall is too much of a problem. Just less convenient. > > > On another matter, if the application uses SET_DEADLINE with one > > > timestamp, and the compositor uses SET_DEADLINE on the same thing with > > > another timestamp, what should happen? > > > > The expectation is that many deadline hints can be set on a fence. > > The fence signaller should track the soonest deadline. > > You need to document that as UAPI, since it is observable to userspace. > It would be bad if drivers or subsystems would differ
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Wed, 22 Feb 2023 07:37:26 -0800 Rob Clark wrote: > On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen wrote: > > > > On Tue, 21 Feb 2023 09:53:56 -0800 > > Rob Clark wrote: > > > > > On Tue, Feb 21, 2023 at 8:48 AM Luben Tuikov > > > wrote: > > > > > > > > On 2023-02-20 11:14, Rob Clark wrote: > > > > > On Mon, Feb 20, 2023 at 12:53 AM Pekka Paalanen > > > > > wrote: > > > > >> > > > > >> On Sat, 18 Feb 2023 13:15:49 -0800 > > > > >> Rob Clark wrote: > > > > >> > > > > >>> From: Rob Clark > > > > >>> > > > > >>> Allow userspace to use the EPOLLPRI/POLLPRI flag to indicate an > > > > >>> urgent > > > > >>> wait (as opposed to a "housekeeping" wait to know when to cleanup > > > > >>> after > > > > >>> some work has completed). Usermode components of GPU driver stacks > > > > >>> often poll() on fence fd's to know when it is safe to do things like > > > > >>> free or reuse a buffer, but they can also poll() on a fence fd when > > > > >>> waiting to read back results from the GPU. The EPOLLPRI/POLLPRI > > > > >>> flag > > > > >>> lets the kernel differentiate these two cases. > > > > >>> > > > > >>> Signed-off-by: Rob Clark > > > > >> > > > > >> Hi, > > > > >> > > > > >> where would the UAPI documentation of this go? > > > > >> It seems to be missing. > > > > > > > > > > Good question, I am not sure. The poll() man page has a description, > > > > > but my usage doesn't fit that _exactly_ (but OTOH the description is a > > > > > bit vague). > > > > > > > > > >> If a Wayland compositor is polling application fences to know which > > > > >> client buffer to use in its rendering, should the compositor poll > > > > >> with > > > > >> PRI or not? If a compositor polls with PRI, then all fences from all > > > > >> applications would always be PRI. Would that be harmful somehow or > > > > >> would it be beneficial? > > > > > > > > > > I think a compositor would rather use the deadline ioctl and then poll > > > > > without PRI. Otherwise you are giving an urgency signal to the fence > > > > > signaller which might not necessarily be needed. > > > > > > > > > > The places where I expect PRI to be useful is more in mesa (things > > > > > like glFinish(), readpix, and other similar sorts of blocking APIs) > > > > Hi, > > > > > > > > Hmm, but then user-space could do the opposite, namely, submit work as > > > > usual--never > > > > using the SET_DEADLINE ioctl, and then at the end, poll using > > > > (E)POLLPRI. That seems > > > > like a possible usage pattern, unintended--maybe, but possible. Do we > > > > want to discourage > > > > this? Wouldn't SET_DEADLINE be enough? I mean, one can call > > > > SET_DEADLINE with the current > > > > time, and then wouldn't that be equivalent to (E)POLLPRI? > > > > > > Yeah, (E)POLLPRI isn't strictly needed if we have SET_DEADLINE. It is > > > slightly more convenient if you want an immediate deadline (single > > > syscall instead of two), but not strictly needed. OTOH it piggy-backs > > > on existing UABI. > > > > In that case, I would be conservative, and not add the POLLPRI > > semantics. An UAPI addition that is not strictly needed and somewhat > > unclear if it violates any design principles is best not done, until it > > is proven to be beneficial. > > > > Besides, a Wayland compositor does not necessary need to add the fd > > to its main event loop for poll. It could just SET_DEADLINE, and then > > when it renders simply check if the fence passed or not already. Not > > polling means the compositor does not need to wake up at the moment the > > fence signals to just record a flag. > > poll(POLLPRI) isn't intended for wayland.. but is a thing I want in > mesa for fence waits. I _could_ use SET_DEADLINE but it is two > syscalls and correspondingly more code ;-) But is it actually beneficial? "More code" seems quite irrelevant. Would there be a hundred or more of those per frame? Or would it be always limited to one or two? Or totally depend on what the application is doing? Is it a significant impact? > > On another matter, if the application uses SET_DEADLINE with one > > timestamp, and the compositor uses SET_DEADLINE on the same thing with > > another timestamp, what should happen? > > The expectation is that many deadline hints can be set on a fence. > The fence signaller should track the soonest deadline. You need to document that as UAPI, since it is observable to userspace. It would be bad if drivers or subsystems would differ in behaviour. Thanks, pq pgp7aIX0EMl5C.pgp Description: OpenPGP digital signature
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Wed, Feb 22, 2023 at 1:49 AM Pekka Paalanen wrote: > > On Tue, 21 Feb 2023 09:53:56 -0800 > Rob Clark wrote: > > > On Tue, Feb 21, 2023 at 8:48 AM Luben Tuikov wrote: > > > > > > On 2023-02-20 11:14, Rob Clark wrote: > > > > On Mon, Feb 20, 2023 at 12:53 AM Pekka Paalanen > > > > wrote: > > > >> > > > >> On Sat, 18 Feb 2023 13:15:49 -0800 > > > >> Rob Clark wrote: > > > >> > > > >>> From: Rob Clark > > > >>> > > > >>> Allow userspace to use the EPOLLPRI/POLLPRI flag to indicate an urgent > > > >>> wait (as opposed to a "housekeeping" wait to know when to cleanup > > > >>> after > > > >>> some work has completed). Usermode components of GPU driver stacks > > > >>> often poll() on fence fd's to know when it is safe to do things like > > > >>> free or reuse a buffer, but they can also poll() on a fence fd when > > > >>> waiting to read back results from the GPU. The EPOLLPRI/POLLPRI flag > > > >>> lets the kernel differentiate these two cases. > > > >>> > > > >>> Signed-off-by: Rob Clark > > > >> > > > >> Hi, > > > >> > > > >> where would the UAPI documentation of this go? > > > >> It seems to be missing. > > > > > > > > Good question, I am not sure. The poll() man page has a description, > > > > but my usage doesn't fit that _exactly_ (but OTOH the description is a > > > > bit vague). > > > > > > > >> If a Wayland compositor is polling application fences to know which > > > >> client buffer to use in its rendering, should the compositor poll with > > > >> PRI or not? If a compositor polls with PRI, then all fences from all > > > >> applications would always be PRI. Would that be harmful somehow or > > > >> would it be beneficial? > > > > > > > > I think a compositor would rather use the deadline ioctl and then poll > > > > without PRI. Otherwise you are giving an urgency signal to the fence > > > > signaller which might not necessarily be needed. > > > > > > > > The places where I expect PRI to be useful is more in mesa (things > > > > like glFinish(), readpix, and other similar sorts of blocking APIs) > > > Hi, > > > > > > Hmm, but then user-space could do the opposite, namely, submit work as > > > usual--never > > > using the SET_DEADLINE ioctl, and then at the end, poll using (E)POLLPRI. > > > That seems > > > like a possible usage pattern, unintended--maybe, but possible. Do we > > > want to discourage > > > this? Wouldn't SET_DEADLINE be enough? I mean, one can call SET_DEADLINE > > > with the current > > > time, and then wouldn't that be equivalent to (E)POLLPRI? > > > > Yeah, (E)POLLPRI isn't strictly needed if we have SET_DEADLINE. It is > > slightly more convenient if you want an immediate deadline (single > > syscall instead of two), but not strictly needed. OTOH it piggy-backs > > on existing UABI. > > In that case, I would be conservative, and not add the POLLPRI > semantics. An UAPI addition that is not strictly needed and somewhat > unclear if it violates any design principles is best not done, until it > is proven to be beneficial. > > Besides, a Wayland compositor does not necessary need to add the fd > to its main event loop for poll. It could just SET_DEADLINE, and then > when it renders simply check if the fence passed or not already. Not > polling means the compositor does not need to wake up at the moment the > fence signals to just record a flag. poll(POLLPRI) isn't intended for wayland.. but is a thing I want in mesa for fence waits. I _could_ use SET_DEADLINE but it is two syscalls and correspondingly more code ;-) > On another matter, if the application uses SET_DEADLINE with one > timestamp, and the compositor uses SET_DEADLINE on the same thing with > another timestamp, what should happen? The expectation is that many deadline hints can be set on a fence. The fence signaller should track the soonest deadline. BR, -R > Maybe it's a soft-realtime app whose primary goal is not display, and > it needs the result faster than the window server? > > Maybe SET_DEADLINE should set the deadline only to an earlier timestamp > and never later? > > > Thanks, > pq
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On 2023-02-22 04:49, Pekka Paalanen wrote: > On Tue, 21 Feb 2023 09:53:56 -0800 > Rob Clark wrote: > >> On Tue, Feb 21, 2023 at 8:48 AM Luben Tuikov wrote: >>> >>> On 2023-02-20 11:14, Rob Clark wrote: On Mon, Feb 20, 2023 at 12:53 AM Pekka Paalanen wrote: > > On Sat, 18 Feb 2023 13:15:49 -0800 > Rob Clark wrote: > >> From: Rob Clark >> >> Allow userspace to use the EPOLLPRI/POLLPRI flag to indicate an urgent >> wait (as opposed to a "housekeeping" wait to know when to cleanup after >> some work has completed). Usermode components of GPU driver stacks >> often poll() on fence fd's to know when it is safe to do things like >> free or reuse a buffer, but they can also poll() on a fence fd when >> waiting to read back results from the GPU. The EPOLLPRI/POLLPRI flag >> lets the kernel differentiate these two cases. >> >> Signed-off-by: Rob Clark > > Hi, > > where would the UAPI documentation of this go? > It seems to be missing. Good question, I am not sure. The poll() man page has a description, but my usage doesn't fit that _exactly_ (but OTOH the description is a bit vague). > If a Wayland compositor is polling application fences to know which > client buffer to use in its rendering, should the compositor poll with > PRI or not? If a compositor polls with PRI, then all fences from all > applications would always be PRI. Would that be harmful somehow or > would it be beneficial? I think a compositor would rather use the deadline ioctl and then poll without PRI. Otherwise you are giving an urgency signal to the fence signaller which might not necessarily be needed. The places where I expect PRI to be useful is more in mesa (things like glFinish(), readpix, and other similar sorts of blocking APIs) >>> Hi, >>> >>> Hmm, but then user-space could do the opposite, namely, submit work as >>> usual--never >>> using the SET_DEADLINE ioctl, and then at the end, poll using (E)POLLPRI. >>> That seems >>> like a possible usage pattern, unintended--maybe, but possible. Do we want >>> to discourage >>> this? Wouldn't SET_DEADLINE be enough? I mean, one can call SET_DEADLINE >>> with the current >>> time, and then wouldn't that be equivalent to (E)POLLPRI? >> >> Yeah, (E)POLLPRI isn't strictly needed if we have SET_DEADLINE. It is >> slightly more convenient if you want an immediate deadline (single >> syscall instead of two), but not strictly needed. OTOH it piggy-backs >> on existing UABI. > > In that case, I would be conservative, and not add the POLLPRI > semantics. An UAPI addition that is not strictly needed and somewhat > unclear if it violates any design principles is best not done, until it > is proven to be beneficial. That is my sentiment as well. Moreover, on hard-realtime systems, one would want to set the deadline at the outset and not at poll time. -- Regards, Luben
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Tue, 21 Feb 2023 09:53:56 -0800 Rob Clark wrote: > On Tue, Feb 21, 2023 at 8:48 AM Luben Tuikov wrote: > > > > On 2023-02-20 11:14, Rob Clark wrote: > > > On Mon, Feb 20, 2023 at 12:53 AM Pekka Paalanen > > > wrote: > > >> > > >> On Sat, 18 Feb 2023 13:15:49 -0800 > > >> Rob Clark wrote: > > >> > > >>> From: Rob Clark > > >>> > > >>> Allow userspace to use the EPOLLPRI/POLLPRI flag to indicate an urgent > > >>> wait (as opposed to a "housekeeping" wait to know when to cleanup after > > >>> some work has completed). Usermode components of GPU driver stacks > > >>> often poll() on fence fd's to know when it is safe to do things like > > >>> free or reuse a buffer, but they can also poll() on a fence fd when > > >>> waiting to read back results from the GPU. The EPOLLPRI/POLLPRI flag > > >>> lets the kernel differentiate these two cases. > > >>> > > >>> Signed-off-by: Rob Clark > > >> > > >> Hi, > > >> > > >> where would the UAPI documentation of this go? > > >> It seems to be missing. > > > > > > Good question, I am not sure. The poll() man page has a description, > > > but my usage doesn't fit that _exactly_ (but OTOH the description is a > > > bit vague). > > > > > >> If a Wayland compositor is polling application fences to know which > > >> client buffer to use in its rendering, should the compositor poll with > > >> PRI or not? If a compositor polls with PRI, then all fences from all > > >> applications would always be PRI. Would that be harmful somehow or > > >> would it be beneficial? > > > > > > I think a compositor would rather use the deadline ioctl and then poll > > > without PRI. Otherwise you are giving an urgency signal to the fence > > > signaller which might not necessarily be needed. > > > > > > The places where I expect PRI to be useful is more in mesa (things > > > like glFinish(), readpix, and other similar sorts of blocking APIs) > > Hi, > > > > Hmm, but then user-space could do the opposite, namely, submit work as > > usual--never > > using the SET_DEADLINE ioctl, and then at the end, poll using (E)POLLPRI. > > That seems > > like a possible usage pattern, unintended--maybe, but possible. Do we want > > to discourage > > this? Wouldn't SET_DEADLINE be enough? I mean, one can call SET_DEADLINE > > with the current > > time, and then wouldn't that be equivalent to (E)POLLPRI? > > Yeah, (E)POLLPRI isn't strictly needed if we have SET_DEADLINE. It is > slightly more convenient if you want an immediate deadline (single > syscall instead of two), but not strictly needed. OTOH it piggy-backs > on existing UABI. In that case, I would be conservative, and not add the POLLPRI semantics. An UAPI addition that is not strictly needed and somewhat unclear if it violates any design principles is best not done, until it is proven to be beneficial. Besides, a Wayland compositor does not necessary need to add the fd to its main event loop for poll. It could just SET_DEADLINE, and then when it renders simply check if the fence passed or not already. Not polling means the compositor does not need to wake up at the moment the fence signals to just record a flag. On another matter, if the application uses SET_DEADLINE with one timestamp, and the compositor uses SET_DEADLINE on the same thing with another timestamp, what should happen? Maybe it's a soft-realtime app whose primary goal is not display, and it needs the result faster than the window server? Maybe SET_DEADLINE should set the deadline only to an earlier timestamp and never later? Thanks, pq pgpuFLuposEbE.pgp Description: OpenPGP digital signature
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Tue, Feb 21, 2023 at 8:01 AM Sebastian Wick
wrote:
>
> On Tue, Feb 21, 2023 at 9:38 AM Pekka Paalanen wrote:
> >
> > On Mon, 20 Feb 2023 08:14:47 -0800
> > Rob Clark wrote:
> >
> > > On Mon, Feb 20, 2023 at 12:53 AM Pekka Paalanen
> > > wrote:
> > > >
> > > > On Sat, 18 Feb 2023 13:15:49 -0800
> > > > Rob Clark wrote:
> > > >
> > > > > From: Rob Clark
> > > > >
> > > > > Allow userspace to use the EPOLLPRI/POLLPRI flag to indicate an urgent
> > > > > wait (as opposed to a "housekeeping" wait to know when to cleanup
> > > > > after
> > > > > some work has completed). Usermode components of GPU driver stacks
> > > > > often poll() on fence fd's to know when it is safe to do things like
> > > > > free or reuse a buffer, but they can also poll() on a fence fd when
> > > > > waiting to read back results from the GPU. The EPOLLPRI/POLLPRI flag
> > > > > lets the kernel differentiate these two cases.
> > > > >
> > > > > Signed-off-by: Rob Clark
> > > >
> > > > Hi,
> > > >
> > > > where would the UAPI documentation of this go?
> > > > It seems to be missing.
> > >
> > > Good question, I am not sure. The poll() man page has a description,
> > > but my usage doesn't fit that _exactly_ (but OTOH the description is a
> > > bit vague).
> > >
> > > > If a Wayland compositor is polling application fences to know which
> > > > client buffer to use in its rendering, should the compositor poll with
> > > > PRI or not? If a compositor polls with PRI, then all fences from all
> > > > applications would always be PRI. Would that be harmful somehow or
> > > > would it be beneficial?
> > >
> > > I think a compositor would rather use the deadline ioctl and then poll
> > > without PRI. Otherwise you are giving an urgency signal to the fence
> > > signaller which might not necessarily be needed.
> > >
> > > The places where I expect PRI to be useful is more in mesa (things
> > > like glFinish(), readpix, and other similar sorts of blocking APIs)
> >
> > Sounds good. Docs... ;-)
> >
> > Hmm, so a compositor should set the deadline when it processes the
> > wl_surface.commit, and not when it actually starts repainting, to give
> > time for the driver to react and the GPU to do some more work. The
> > deadline would be the time when the compositor starts its repaint, so
> > it knows if the buffer is ready or not.
>
> Technically we don't know when the commit is supposed to be shown.
> Just passing a deadline of the next possible deadline however is
> probably a good enough guess for this feature to be useful.
>
> One thing that neither API allows us to do is tell the kernel in
> advance when we're going to submit work and what the deadline for it
> is and unfortunately that work is the most timing sensitive.
Presumably you are talking about the final compositing step?
Elsewhere in this series that atomic wait-for-fences step sets the
deadline hint.
BR,
-R
> >
> >
> > Thanks,
> > pq
> >
> >
> > >
> > > BR,
> > > -R
> > >
> > > >
> > > >
> > > > Thanks,
> > > > pq
> > > >
> > > > > ---
> > > > > drivers/dma-buf/sync_file.c | 8
> > > > > 1 file changed, 8 insertions(+)
> > > > >
> > > > > diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> > > > > index fb6ca1032885..c30b2085ee0a 100644
> > > > > --- a/drivers/dma-buf/sync_file.c
> > > > > +++ b/drivers/dma-buf/sync_file.c
> > > > > @@ -192,6 +192,14 @@ static __poll_t sync_file_poll(struct file
> > > > > *file, poll_table *wait)
> > > > > {
> > > > > struct sync_file *sync_file = file->private_data;
> > > > >
> > > > > + /*
> > > > > + * The POLLPRI/EPOLLPRI flag can be used to signal that
> > > > > + * userspace wants the fence to signal ASAP, express this
> > > > > + * as an immediate deadline.
> > > > > + */
> > > > > + if (poll_requested_events(wait) & EPOLLPRI)
> > > > > + dma_fence_set_deadline(sync_file->fence, ktime_get());
> > > > > +
> > > > > poll_wait(file, _file->wq, wait);
> > > > >
> > > > > if (list_empty(_file->cb.node) &&
> > > >
> >
>
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Tue, Feb 21, 2023 at 8:48 AM Luben Tuikov wrote: > > On 2023-02-20 11:14, Rob Clark wrote: > > On Mon, Feb 20, 2023 at 12:53 AM Pekka Paalanen wrote: > >> > >> On Sat, 18 Feb 2023 13:15:49 -0800 > >> Rob Clark wrote: > >> > >>> From: Rob Clark > >>> > >>> Allow userspace to use the EPOLLPRI/POLLPRI flag to indicate an urgent > >>> wait (as opposed to a "housekeeping" wait to know when to cleanup after > >>> some work has completed). Usermode components of GPU driver stacks > >>> often poll() on fence fd's to know when it is safe to do things like > >>> free or reuse a buffer, but they can also poll() on a fence fd when > >>> waiting to read back results from the GPU. The EPOLLPRI/POLLPRI flag > >>> lets the kernel differentiate these two cases. > >>> > >>> Signed-off-by: Rob Clark > >> > >> Hi, > >> > >> where would the UAPI documentation of this go? > >> It seems to be missing. > > > > Good question, I am not sure. The poll() man page has a description, > > but my usage doesn't fit that _exactly_ (but OTOH the description is a > > bit vague). > > > >> If a Wayland compositor is polling application fences to know which > >> client buffer to use in its rendering, should the compositor poll with > >> PRI or not? If a compositor polls with PRI, then all fences from all > >> applications would always be PRI. Would that be harmful somehow or > >> would it be beneficial? > > > > I think a compositor would rather use the deadline ioctl and then poll > > without PRI. Otherwise you are giving an urgency signal to the fence > > signaller which might not necessarily be needed. > > > > The places where I expect PRI to be useful is more in mesa (things > > like glFinish(), readpix, and other similar sorts of blocking APIs) > Hi, > > Hmm, but then user-space could do the opposite, namely, submit work as > usual--never > using the SET_DEADLINE ioctl, and then at the end, poll using (E)POLLPRI. > That seems > like a possible usage pattern, unintended--maybe, but possible. Do we want to > discourage > this? Wouldn't SET_DEADLINE be enough? I mean, one can call SET_DEADLINE with > the current > time, and then wouldn't that be equivalent to (E)POLLPRI? Yeah, (E)POLLPRI isn't strictly needed if we have SET_DEADLINE. It is slightly more convenient if you want an immediate deadline (single syscall instead of two), but not strictly needed. OTOH it piggy-backs on existing UABI. BR, -R
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On 2023-02-20 11:14, Rob Clark wrote: > On Mon, Feb 20, 2023 at 12:53 AM Pekka Paalanen wrote: >> >> On Sat, 18 Feb 2023 13:15:49 -0800 >> Rob Clark wrote: >> >>> From: Rob Clark >>> >>> Allow userspace to use the EPOLLPRI/POLLPRI flag to indicate an urgent >>> wait (as opposed to a "housekeeping" wait to know when to cleanup after >>> some work has completed). Usermode components of GPU driver stacks >>> often poll() on fence fd's to know when it is safe to do things like >>> free or reuse a buffer, but they can also poll() on a fence fd when >>> waiting to read back results from the GPU. The EPOLLPRI/POLLPRI flag >>> lets the kernel differentiate these two cases. >>> >>> Signed-off-by: Rob Clark >> >> Hi, >> >> where would the UAPI documentation of this go? >> It seems to be missing. > > Good question, I am not sure. The poll() man page has a description, > but my usage doesn't fit that _exactly_ (but OTOH the description is a > bit vague). > >> If a Wayland compositor is polling application fences to know which >> client buffer to use in its rendering, should the compositor poll with >> PRI or not? If a compositor polls with PRI, then all fences from all >> applications would always be PRI. Would that be harmful somehow or >> would it be beneficial? > > I think a compositor would rather use the deadline ioctl and then poll > without PRI. Otherwise you are giving an urgency signal to the fence > signaller which might not necessarily be needed. > > The places where I expect PRI to be useful is more in mesa (things > like glFinish(), readpix, and other similar sorts of blocking APIs) Hi, Hmm, but then user-space could do the opposite, namely, submit work as usual--never using the SET_DEADLINE ioctl, and then at the end, poll using (E)POLLPRI. That seems like a possible usage pattern, unintended--maybe, but possible. Do we want to discourage this? Wouldn't SET_DEADLINE be enough? I mean, one can call SET_DEADLINE with the current time, and then wouldn't that be equivalent to (E)POLLPRI? -- Regards, Luben
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Tue, Feb 21, 2023 at 9:38 AM Pekka Paalanen wrote:
>
> On Mon, 20 Feb 2023 08:14:47 -0800
> Rob Clark wrote:
>
> > On Mon, Feb 20, 2023 at 12:53 AM Pekka Paalanen wrote:
> > >
> > > On Sat, 18 Feb 2023 13:15:49 -0800
> > > Rob Clark wrote:
> > >
> > > > From: Rob Clark
> > > >
> > > > Allow userspace to use the EPOLLPRI/POLLPRI flag to indicate an urgent
> > > > wait (as opposed to a "housekeeping" wait to know when to cleanup after
> > > > some work has completed). Usermode components of GPU driver stacks
> > > > often poll() on fence fd's to know when it is safe to do things like
> > > > free or reuse a buffer, but they can also poll() on a fence fd when
> > > > waiting to read back results from the GPU. The EPOLLPRI/POLLPRI flag
> > > > lets the kernel differentiate these two cases.
> > > >
> > > > Signed-off-by: Rob Clark
> > >
> > > Hi,
> > >
> > > where would the UAPI documentation of this go?
> > > It seems to be missing.
> >
> > Good question, I am not sure. The poll() man page has a description,
> > but my usage doesn't fit that _exactly_ (but OTOH the description is a
> > bit vague).
> >
> > > If a Wayland compositor is polling application fences to know which
> > > client buffer to use in its rendering, should the compositor poll with
> > > PRI or not? If a compositor polls with PRI, then all fences from all
> > > applications would always be PRI. Would that be harmful somehow or
> > > would it be beneficial?
> >
> > I think a compositor would rather use the deadline ioctl and then poll
> > without PRI. Otherwise you are giving an urgency signal to the fence
> > signaller which might not necessarily be needed.
> >
> > The places where I expect PRI to be useful is more in mesa (things
> > like glFinish(), readpix, and other similar sorts of blocking APIs)
>
> Sounds good. Docs... ;-)
>
> Hmm, so a compositor should set the deadline when it processes the
> wl_surface.commit, and not when it actually starts repainting, to give
> time for the driver to react and the GPU to do some more work. The
> deadline would be the time when the compositor starts its repaint, so
> it knows if the buffer is ready or not.
Technically we don't know when the commit is supposed to be shown.
Just passing a deadline of the next possible deadline however is
probably a good enough guess for this feature to be useful.
One thing that neither API allows us to do is tell the kernel in
advance when we're going to submit work and what the deadline for it
is and unfortunately that work is the most timing sensitive.
>
>
> Thanks,
> pq
>
>
> >
> > BR,
> > -R
> >
> > >
> > >
> > > Thanks,
> > > pq
> > >
> > > > ---
> > > > drivers/dma-buf/sync_file.c | 8
> > > > 1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> > > > index fb6ca1032885..c30b2085ee0a 100644
> > > > --- a/drivers/dma-buf/sync_file.c
> > > > +++ b/drivers/dma-buf/sync_file.c
> > > > @@ -192,6 +192,14 @@ static __poll_t sync_file_poll(struct file *file,
> > > > poll_table *wait)
> > > > {
> > > > struct sync_file *sync_file = file->private_data;
> > > >
> > > > + /*
> > > > + * The POLLPRI/EPOLLPRI flag can be used to signal that
> > > > + * userspace wants the fence to signal ASAP, express this
> > > > + * as an immediate deadline.
> > > > + */
> > > > + if (poll_requested_events(wait) & EPOLLPRI)
> > > > + dma_fence_set_deadline(sync_file->fence, ktime_get());
> > > > +
> > > > poll_wait(file, _file->wq, wait);
> > > >
> > > > if (list_empty(_file->cb.node) &&
> > >
>
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Mon, 20 Feb 2023 09:31:41 +0100
Christian König wrote:
> Am 18.02.23 um 22:15 schrieb Rob Clark:
> > From: Rob Clark
> >
> > Allow userspace to use the EPOLLPRI/POLLPRI flag to indicate an urgent
> > wait (as opposed to a "housekeeping" wait to know when to cleanup after
> > some work has completed). Usermode components of GPU driver stacks
> > often poll() on fence fd's to know when it is safe to do things like
> > free or reuse a buffer, but they can also poll() on a fence fd when
> > waiting to read back results from the GPU. The EPOLLPRI/POLLPRI flag
> > lets the kernel differentiate these two cases.
> >
> > Signed-off-by: Rob Clark
>
> The code looks clean, but the different poll flags and their meaning are
> certainly not my field of expertise.
A good question. epoll_ctl manual refers to poll(2) which says:
POLLPRI
There is some exceptional condition on the file descriptor.
Possibilities include:
• There is out-of-band data on a TCP socket (see tcp(7)).
• A pseudoterminal master in packet mode has seen a state change
on the slave (see ioctl_tty(2)).
• A cgroup.events file has been modified (see cgroups(7)).
It seems to be about selecting what events will trigger the poll,
more than how (fast) to poll. At least it is not documented to be
ignored in 'events', so I guess it should work.
Thanks,
pq
> Feel free to add Acked-by: Christian König ,
> somebody with more background in this should probably take a look as well.
>
> Regards,
> Christian.
>
> > ---
> > drivers/dma-buf/sync_file.c | 8
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> > index fb6ca1032885..c30b2085ee0a 100644
> > --- a/drivers/dma-buf/sync_file.c
> > +++ b/drivers/dma-buf/sync_file.c
> > @@ -192,6 +192,14 @@ static __poll_t sync_file_poll(struct file *file,
> > poll_table *wait)
> > {
> > struct sync_file *sync_file = file->private_data;
> >
> > + /*
> > +* The POLLPRI/EPOLLPRI flag can be used to signal that
> > +* userspace wants the fence to signal ASAP, express this
> > +* as an immediate deadline.
> > +*/
> > + if (poll_requested_events(wait) & EPOLLPRI)
> > + dma_fence_set_deadline(sync_file->fence, ktime_get());
> > +
> > poll_wait(file, _file->wq, wait);
> >
> > if (list_empty(_file->cb.node) &&
>
pgpPUXXpDEAPq.pgp
Description: OpenPGP digital signature
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Mon, 20 Feb 2023 08:14:47 -0800
Rob Clark wrote:
> On Mon, Feb 20, 2023 at 12:53 AM Pekka Paalanen wrote:
> >
> > On Sat, 18 Feb 2023 13:15:49 -0800
> > Rob Clark wrote:
> >
> > > From: Rob Clark
> > >
> > > Allow userspace to use the EPOLLPRI/POLLPRI flag to indicate an urgent
> > > wait (as opposed to a "housekeeping" wait to know when to cleanup after
> > > some work has completed). Usermode components of GPU driver stacks
> > > often poll() on fence fd's to know when it is safe to do things like
> > > free or reuse a buffer, but they can also poll() on a fence fd when
> > > waiting to read back results from the GPU. The EPOLLPRI/POLLPRI flag
> > > lets the kernel differentiate these two cases.
> > >
> > > Signed-off-by: Rob Clark
> >
> > Hi,
> >
> > where would the UAPI documentation of this go?
> > It seems to be missing.
>
> Good question, I am not sure. The poll() man page has a description,
> but my usage doesn't fit that _exactly_ (but OTOH the description is a
> bit vague).
>
> > If a Wayland compositor is polling application fences to know which
> > client buffer to use in its rendering, should the compositor poll with
> > PRI or not? If a compositor polls with PRI, then all fences from all
> > applications would always be PRI. Would that be harmful somehow or
> > would it be beneficial?
>
> I think a compositor would rather use the deadline ioctl and then poll
> without PRI. Otherwise you are giving an urgency signal to the fence
> signaller which might not necessarily be needed.
>
> The places where I expect PRI to be useful is more in mesa (things
> like glFinish(), readpix, and other similar sorts of blocking APIs)
Sounds good. Docs... ;-)
Hmm, so a compositor should set the deadline when it processes the
wl_surface.commit, and not when it actually starts repainting, to give
time for the driver to react and the GPU to do some more work. The
deadline would be the time when the compositor starts its repaint, so
it knows if the buffer is ready or not.
Thanks,
pq
>
> BR,
> -R
>
> >
> >
> > Thanks,
> > pq
> >
> > > ---
> > > drivers/dma-buf/sync_file.c | 8
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> > > index fb6ca1032885..c30b2085ee0a 100644
> > > --- a/drivers/dma-buf/sync_file.c
> > > +++ b/drivers/dma-buf/sync_file.c
> > > @@ -192,6 +192,14 @@ static __poll_t sync_file_poll(struct file *file,
> > > poll_table *wait)
> > > {
> > > struct sync_file *sync_file = file->private_data;
> > >
> > > + /*
> > > + * The POLLPRI/EPOLLPRI flag can be used to signal that
> > > + * userspace wants the fence to signal ASAP, express this
> > > + * as an immediate deadline.
> > > + */
> > > + if (poll_requested_events(wait) & EPOLLPRI)
> > > + dma_fence_set_deadline(sync_file->fence, ktime_get());
> > > +
> > > poll_wait(file, _file->wq, wait);
> > >
> > > if (list_empty(_file->cb.node) &&
> >
pgpnVJYkLApzI.pgp
Description: OpenPGP digital signature
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Mon, Feb 20, 2023 at 12:53 AM Pekka Paalanen wrote:
>
> On Sat, 18 Feb 2023 13:15:49 -0800
> Rob Clark wrote:
>
> > From: Rob Clark
> >
> > Allow userspace to use the EPOLLPRI/POLLPRI flag to indicate an urgent
> > wait (as opposed to a "housekeeping" wait to know when to cleanup after
> > some work has completed). Usermode components of GPU driver stacks
> > often poll() on fence fd's to know when it is safe to do things like
> > free or reuse a buffer, but they can also poll() on a fence fd when
> > waiting to read back results from the GPU. The EPOLLPRI/POLLPRI flag
> > lets the kernel differentiate these two cases.
> >
> > Signed-off-by: Rob Clark
>
> Hi,
>
> where would the UAPI documentation of this go?
> It seems to be missing.
Good question, I am not sure. The poll() man page has a description,
but my usage doesn't fit that _exactly_ (but OTOH the description is a
bit vague).
> If a Wayland compositor is polling application fences to know which
> client buffer to use in its rendering, should the compositor poll with
> PRI or not? If a compositor polls with PRI, then all fences from all
> applications would always be PRI. Would that be harmful somehow or
> would it be beneficial?
I think a compositor would rather use the deadline ioctl and then poll
without PRI. Otherwise you are giving an urgency signal to the fence
signaller which might not necessarily be needed.
The places where I expect PRI to be useful is more in mesa (things
like glFinish(), readpix, and other similar sorts of blocking APIs)
BR,
-R
>
>
> Thanks,
> pq
>
> > ---
> > drivers/dma-buf/sync_file.c | 8
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> > index fb6ca1032885..c30b2085ee0a 100644
> > --- a/drivers/dma-buf/sync_file.c
> > +++ b/drivers/dma-buf/sync_file.c
> > @@ -192,6 +192,14 @@ static __poll_t sync_file_poll(struct file *file,
> > poll_table *wait)
> > {
> > struct sync_file *sync_file = file->private_data;
> >
> > + /*
> > + * The POLLPRI/EPOLLPRI flag can be used to signal that
> > + * userspace wants the fence to signal ASAP, express this
> > + * as an immediate deadline.
> > + */
> > + if (poll_requested_events(wait) & EPOLLPRI)
> > + dma_fence_set_deadline(sync_file->fence, ktime_get());
> > +
> > poll_wait(file, _file->wq, wait);
> >
> > if (list_empty(_file->cb.node) &&
>
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
On Sat, 18 Feb 2023 13:15:49 -0800
Rob Clark wrote:
> From: Rob Clark
>
> Allow userspace to use the EPOLLPRI/POLLPRI flag to indicate an urgent
> wait (as opposed to a "housekeeping" wait to know when to cleanup after
> some work has completed). Usermode components of GPU driver stacks
> often poll() on fence fd's to know when it is safe to do things like
> free or reuse a buffer, but they can also poll() on a fence fd when
> waiting to read back results from the GPU. The EPOLLPRI/POLLPRI flag
> lets the kernel differentiate these two cases.
>
> Signed-off-by: Rob Clark
Hi,
where would the UAPI documentation of this go?
It seems to be missing.
If a Wayland compositor is polling application fences to know which
client buffer to use in its rendering, should the compositor poll with
PRI or not? If a compositor polls with PRI, then all fences from all
applications would always be PRI. Would that be harmful somehow or
would it be beneficial?
Thanks,
pq
> ---
> drivers/dma-buf/sync_file.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> index fb6ca1032885..c30b2085ee0a 100644
> --- a/drivers/dma-buf/sync_file.c
> +++ b/drivers/dma-buf/sync_file.c
> @@ -192,6 +192,14 @@ static __poll_t sync_file_poll(struct file *file,
> poll_table *wait)
> {
> struct sync_file *sync_file = file->private_data;
>
> + /*
> + * The POLLPRI/EPOLLPRI flag can be used to signal that
> + * userspace wants the fence to signal ASAP, express this
> + * as an immediate deadline.
> + */
> + if (poll_requested_events(wait) & EPOLLPRI)
> + dma_fence_set_deadline(sync_file->fence, ktime_get());
> +
> poll_wait(file, _file->wq, wait);
>
> if (list_empty(_file->cb.node) &&
pgpissQ7ejTcj.pgp
Description: OpenPGP digital signature
Re: [Freedreno] [PATCH v4 06/14] dma-buf/sync_file: Support (E)POLLPRI
Am 18.02.23 um 22:15 schrieb Rob Clark:
From: Rob Clark
Allow userspace to use the EPOLLPRI/POLLPRI flag to indicate an urgent
wait (as opposed to a "housekeeping" wait to know when to cleanup after
some work has completed). Usermode components of GPU driver stacks
often poll() on fence fd's to know when it is safe to do things like
free or reuse a buffer, but they can also poll() on a fence fd when
waiting to read back results from the GPU. The EPOLLPRI/POLLPRI flag
lets the kernel differentiate these two cases.
Signed-off-by: Rob Clark
The code looks clean, but the different poll flags and their meaning are
certainly not my field of expertise.
Feel free to add Acked-by: Christian König ,
somebody with more background in this should probably take a look as well.
Regards,
Christian.
---
drivers/dma-buf/sync_file.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index fb6ca1032885..c30b2085ee0a 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -192,6 +192,14 @@ static __poll_t sync_file_poll(struct file *file,
poll_table *wait)
{
struct sync_file *sync_file = file->private_data;
+ /*
+* The POLLPRI/EPOLLPRI flag can be used to signal that
+* userspace wants the fence to signal ASAP, express this
+* as an immediate deadline.
+*/
+ if (poll_requested_events(wait) & EPOLLPRI)
+ dma_fence_set_deadline(sync_file->fence, ktime_get());
+
poll_wait(file, _file->wq, wait);
if (list_empty(_file->cb.node) &&
