| Message ID | 20180219225356.24996-5-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 19, 2018 at 11:53:56PM +0100, Daniel Vetter wrote: > Note: This is untested because the new stuff hasn't landed in upstream > yet. But it should work. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > include/drm/drm_vblank.h | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) I couldn't find a reference to the new stuff, so maybe this was already discussed, but wouldn't it be possible for the parser to figure out the substruct by itself? That way the event. prefix could be avoided. Thierry
On Tue, Feb 20, 2018 at 10:49:52AM +0100, Thierry Reding wrote: > On Mon, Feb 19, 2018 at 11:53:56PM +0100, Daniel Vetter wrote: > > Note: This is untested because the new stuff hasn't landed in upstream > > yet. But it should work. Ok Jani put the patch into topic/core-for-CI temporarily, so I could test. It works. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > --- > > include/drm/drm_vblank.h | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > I couldn't find a reference to the new stuff, so maybe this was already > discussed, but wouldn't it be possible for the parser to figure out the > substruct by itself? That way the event. prefix could be avoided. https://lkml.org/lkml/2018/2/16/339 Jon said he pulled it all in already, I guess this way was simplest, the actual fix is a one-liner: https://lkml.org/lkml/2018/2/16/344 The reason for this is likely that you'll end up with name clashes everywhere if you don't insist on the fully qualified name. I merged the other patches meanwhile, thanks a lot for your review. -Daniel
On Tue, Feb 20, 2018 at 12:10:35PM +0100, Daniel Vetter wrote: > On Tue, Feb 20, 2018 at 10:49:52AM +0100, Thierry Reding wrote: > > On Mon, Feb 19, 2018 at 11:53:56PM +0100, Daniel Vetter wrote: > > > Note: This is untested because the new stuff hasn't landed in upstream > > > yet. But it should work. > > Ok Jani put the patch into topic/core-for-CI temporarily, so I could test. > It works. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > > --- > > > include/drm/drm_vblank.h | 16 ++++++++++++++++ > > > 1 file changed, 16 insertions(+) > > > > I couldn't find a reference to the new stuff, so maybe this was already > > discussed, but wouldn't it be possible for the parser to figure out the > > substruct by itself? That way the event. prefix could be avoided. > > https://lkml.org/lkml/2018/2/16/339 > > Jon said he pulled it all in already, I guess this way was simplest, the > actual fix is a one-liner: > > https://lkml.org/lkml/2018/2/16/344 > > The reason for this is likely that you'll end up with name clashes > everywhere if you don't insist on the fully qualified name. Thanks for the pointers. I would've thought that the parser could simply stick the prefix onto the name itself to make it fully qualified and thereby avoid any name clashes. But looking at the patches and the parser a little, this will have to do for now: Reviewed-by: Thierry Reding <treding@nvidia.com>
On Tue, 20 Feb 2018, Thierry Reding <thierry.reding@gmail.com> wrote: > On Mon, Feb 19, 2018 at 11:53:56PM +0100, Daniel Vetter wrote: >> Note: This is untested because the new stuff hasn't landed in upstream >> yet. But it should work. >> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >> --- >> include/drm/drm_vblank.h | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) > > I couldn't find a reference to the new stuff, so maybe this was already > discussed, but wouldn't it be possible for the parser to figure out the > substruct by itself? That way the event. prefix could be avoided. The short answer, supporting this format is a one-line change, while I'm told anything else is harder. Also, the format then is the same at all nesting levels, and you can do any one of the three options below: /** * struct foo - foo * @bar.baz: baz description, option 1 (support upstream) */ struct foo { /** * @bar.baz: baz description, option 2 (support in docs-next) */ struct bar { /** * @bar.baz: baz description, option 3 (support in docs-next) */ int baz; }; } I don't deny it would be nice to be able to leave out the prefix when it can be figured out from the context, but I'm not prepared to do the work, and Mauro helpfully provided the patch for this approach, so it seemed like the rational choice. See also [1] and [2]. BR, Jani. [1] http://mid.mail-archive.com/874lmjlfmg.fsf@intel.com [2] http://mid.mail-archive.com/cover.1518788761.git.mchehab@s-opensource.com
On Tue, Feb 20, 2018 at 12:22:18PM +0100, Thierry Reding wrote: > On Tue, Feb 20, 2018 at 12:10:35PM +0100, Daniel Vetter wrote: > > On Tue, Feb 20, 2018 at 10:49:52AM +0100, Thierry Reding wrote: > > > On Mon, Feb 19, 2018 at 11:53:56PM +0100, Daniel Vetter wrote: > > > > Note: This is untested because the new stuff hasn't landed in upstream > > > > yet. But it should work. > > > > Ok Jani put the patch into topic/core-for-CI temporarily, so I could test. > > It works. > > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > > > --- > > > > include/drm/drm_vblank.h | 16 ++++++++++++++++ > > > > 1 file changed, 16 insertions(+) > > > > > > I couldn't find a reference to the new stuff, so maybe this was already > > > discussed, but wouldn't it be possible for the parser to figure out the > > > substruct by itself? That way the event. prefix could be avoided. > > > > https://lkml.org/lkml/2018/2/16/339 > > > > Jon said he pulled it all in already, I guess this way was simplest, the > > actual fix is a one-liner: > > > > https://lkml.org/lkml/2018/2/16/344 > > > > The reason for this is likely that you'll end up with name clashes > > everywhere if you don't insist on the fully qualified name. > > Thanks for the pointers. I would've thought that the parser could simply > stick the prefix onto the name itself to make it fully qualified and > thereby avoid any name clashes. > > But looking at the patches and the parser a little, this will have to do > for now: > > Reviewed-by: Thierry Reding <treding@nvidia.com> Thanks for your review, patch applied. -Daniel
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h index 16d46e2a6854..d25a9603ab57 100644 --- a/include/drm/drm_vblank.h +++ b/include/drm/drm_vblank.h @@ -55,8 +55,24 @@ struct drm_pending_vblank_event { * @event: Actual event which will be sent to userspace. */ union { + /** + * @event.base: DRM event base class. + */ struct drm_event base; + + /** + * @event.vbl: + * + * Event payload for vblank events, requested through + * either the MODE_PAGE_FLIP or MODE_ATOMIC IOCTL. Also + * generated by the legacy WAIT_VBLANK IOCTL, but new userspace + * should use MODE_QUEUE_SEQUENCE and &event.seq instead. + */ struct drm_event_vblank vbl; + + /** + * @event.seq: Event payload for the MODE_QUEUEU_SEQUENCE IOCTL. + */ struct drm_event_crtc_sequence seq; } event; };
Note: This is untested because the new stuff hasn't landed in upstream yet. But it should work. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- include/drm/drm_vblank.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)