| Message ID | 1427904935-14387-3-git-send-email-emil.l.velikov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 01, 2015 at 05:15:13PM +0100, Emil Velikov wrote: > Missing definition and unused since their introduction. > > Cc: Jerome Glisse <jglisse@redhat.com> > Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> NAK I use all this in tools to debug lockup. Best course of action is to exclude bof.h from being distributed. My tools static link and i just point them to libdrm git tree. Cheers, Jérôme > --- > radeon/bof.h | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/radeon/bof.h b/radeon/bof.h > index 014affb..cb829a1 100644 > --- a/radeon/bof.h > +++ b/radeon/bof.h > @@ -51,10 +51,6 @@ typedef struct bof { > long offset; > } bof_t; > > -extern int bof_file_flush(bof_t *root); > -extern bof_t *bof_file_new(const char *filename); > -extern int bof_object_dump(bof_t *object, const char *filename); > - > /* object */ > extern bof_t *bof_object(void); > extern bof_t *bof_object_get(bof_t *object, const char *keyname); > -- > 2.3.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On 1 April 2015 at 18:30, Jerome Glisse <j.glisse@gmail.com> wrote: > On Wed, Apr 01, 2015 at 05:15:13PM +0100, Emil Velikov wrote: >> Missing definition and unused since their introduction. >> >> Cc: Jerome Glisse <jglisse@redhat.com> >> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> > > NAK > > I use all this in tools to debug lockup. Best course of action is to > exclude bof.h from being distributed. My tools static link and i just > point them to libdrm git tree. > Did not notice any mention of such out-of-tree tools in the commit that introduced these functions, so I've naively assumed that they are unused. Sorry about that. Do you mind if I add a note about it, or alternatively will you be ok with pushing your tool to libdrm ? The Intel team already have a test_decode tool in, which is similar in nature. I'm not sure that your suggestion will work - one cannot exclude bof.h (and bof.c) from the distribution as it's used by radeon_cs_gem.c. Annotating the symbols as hidden/private should work for everyone. How does that sound ? ... >> -extern int bof_file_flush(bof_t *root); >> -extern bof_t *bof_file_new(const char *filename); >> -extern int bof_object_dump(bof_t *object, const char *filename); >> - Can you please elaborate how you are using these three, do you have them implemented outside of libdrm as well ? Cheers, Emil
On 1 April 2015 at 21:34, Emil Velikov <emil.l.velikov@gmail.com> wrote: > On 1 April 2015 at 18:30, Jerome Glisse <j.glisse@gmail.com> wrote: >> On Wed, Apr 01, 2015 at 05:15:13PM +0100, Emil Velikov wrote: >>> Missing definition and unused since their introduction. >>> >>> Cc: Jerome Glisse <jglisse@redhat.com> >>> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> >> >> NAK >> >> I use all this in tools to debug lockup. Best course of action is to >> exclude bof.h from being distributed. My tools static link and i just >> point them to libdrm git tree. >> > Did not notice any mention of such out-of-tree tools in the commit > that introduced these functions, so I've naively assumed that they are > unused. Scratch that - I'm blind. Upon closer look at your radeondb repo, I cannot see any static linking in there. Also it seems that some of the functionality is duplicated between the two. With the radeondb version being out of date :'( -Emil
On Wed, Apr 01, 2015 at 09:34:05PM +0100, Emil Velikov wrote: > On 1 April 2015 at 18:30, Jerome Glisse <j.glisse@gmail.com> wrote: > > On Wed, Apr 01, 2015 at 05:15:13PM +0100, Emil Velikov wrote: > >> Missing definition and unused since their introduction. > >> > >> Cc: Jerome Glisse <jglisse@redhat.com> > >> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> > > > > NAK > > > > I use all this in tools to debug lockup. Best course of action is to > > exclude bof.h from being distributed. My tools static link and i just > > point them to libdrm git tree. > > > Did not notice any mention of such out-of-tree tools in the commit > that introduced these functions, so I've naively assumed that they are > unused. Sorry about that. Do you mind if I add a note about it, or > alternatively will you be ok with pushing your tool to libdrm ? The > Intel team already have a test_decode tool in, which is similar in > nature. It would need cleanup before this can happen, saddly my schedule is kind of full for foreseeable future. So i do not want to commit to do such thing. But i definitly use the bof feature, last time was a month or so ago to debug something. It have been very usefull to me in the past and i expect for as long as the radeon ddx stays releavant it will be in the future. But with the advance of the modesetting ddx and glamor, the tracing that does exist in mesa will be as easy as the bof tracing. If not easier. So i am not sure of the value there is into putting effort into this. This kind of feature is really usefull when debugging lockup, at least this allow me to bisect offend command stream to pin point the last dword before lockup and thus get a clue about what kind of cmd is the root cause. Dunno how others dev do such thing. > I'm not sure that your suggestion will work - one cannot exclude bof.h > (and bof.c) from the distribution as it's used by radeon_cs_gem.c. > Annotating the symbols as hidden/private should work for everyone. How > does that sound ? I do not see how this is an issue, symbol needed by radeon_cs_gem can be hidden from other and thus there is no point into shipping bof.h Really no symbol need to be exported, iirc i tend to ln -s the bof files in my tools or simply cp the lastest version from libdrm into a local copy but i still need bof.h to have all symbol listed. Cheers, Jérôme > > ... > >> -extern int bof_file_flush(bof_t *root); > >> -extern bof_t *bof_file_new(const char *filename); > >> -extern int bof_object_dump(bof_t *object, const char *filename); > >> - > Can you please elaborate how you are using these three, do you have > them implemented outside of libdrm as well ? > > Cheers, > Emil
On Wed, Apr 01, 2015 at 09:57:40PM +0100, Emil Velikov wrote: > On 1 April 2015 at 21:34, Emil Velikov <emil.l.velikov@gmail.com> wrote: > > On 1 April 2015 at 18:30, Jerome Glisse <j.glisse@gmail.com> wrote: > >> On Wed, Apr 01, 2015 at 05:15:13PM +0100, Emil Velikov wrote: > >>> Missing definition and unused since their introduction. > >>> > >>> Cc: Jerome Glisse <jglisse@redhat.com> > >>> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> > >> > >> NAK > >> > >> I use all this in tools to debug lockup. Best course of action is to > >> exclude bof.h from being distributed. My tools static link and i just > >> point them to libdrm git tree. > >> > > Did not notice any mention of such out-of-tree tools in the commit > > that introduced these functions, so I've naively assumed that they are > > unused. > Scratch that - I'm blind. > > Upon closer look at your radeondb repo, I cannot see any static > linking in there. Also it seems that some of the functionality is > duplicated between the two. With the radeondb version being out of > date :'( Yeah i guess i never pushed anywhere patches that did that, divergence btw my memory and what is out there. All this symbol can just be hidden and never exported. It would cleaner, but i still need the bof.h intact as i tend to just cp it afaict into my local radeondb copy so that i am in sync with libdrm code. Cheers, Jérôme > > -Emil
On 1 April 2015 at 22:26, Jerome Glisse <j.glisse@gmail.com> wrote: > On Wed, Apr 01, 2015 at 09:57:40PM +0100, Emil Velikov wrote: >> On 1 April 2015 at 21:34, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> > On 1 April 2015 at 18:30, Jerome Glisse <j.glisse@gmail.com> wrote: >> >> On Wed, Apr 01, 2015 at 05:15:13PM +0100, Emil Velikov wrote: >> >>> Missing definition and unused since their introduction. >> >>> >> >>> Cc: Jerome Glisse <jglisse@redhat.com> >> >>> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> >> >> >> >> NAK >> >> >> >> I use all this in tools to debug lockup. Best course of action is to >> >> exclude bof.h from being distributed. My tools static link and i just >> >> point them to libdrm git tree. >> >> >> > Did not notice any mention of such out-of-tree tools in the commit >> > that introduced these functions, so I've naively assumed that they are >> > unused. >> Scratch that - I'm blind. >> >> Upon closer look at your radeondb repo, I cannot see any static >> linking in there. Also it seems that some of the functionality is >> duplicated between the two. With the radeondb version being out of >> date :'( > > Yeah i guess i never pushed anywhere patches that did that, divergence btw > my memory and what is out there. All this symbol can just be hidden and > never exported. It would cleaner, but i still need the bof.h intact as i > tend to just cp it afaict into my local radeondb copy so that i am in > sync with libdrm code. > I can volunteer with the cleanup/integration of radeondb next to libdrm_radeon. If you update your repo (or push your work elsewhere), I could double-check, integrate and nuke the duplication. It will avoid the next person from coming over and trying to nuke things, the divergence mentioned, plus the copy/pasting of bof.[ch] every time you use the tool. How does that sound ? Cheers, Emil
On Wed, Apr 01, 2015 at 11:04:45PM +0100, Emil Velikov wrote: > On 1 April 2015 at 22:26, Jerome Glisse <j.glisse@gmail.com> wrote: > > On Wed, Apr 01, 2015 at 09:57:40PM +0100, Emil Velikov wrote: > >> On 1 April 2015 at 21:34, Emil Velikov <emil.l.velikov@gmail.com> wrote: > >> > On 1 April 2015 at 18:30, Jerome Glisse <j.glisse@gmail.com> wrote: > >> >> On Wed, Apr 01, 2015 at 05:15:13PM +0100, Emil Velikov wrote: > >> >>> Missing definition and unused since their introduction. > >> >>> > >> >>> Cc: Jerome Glisse <jglisse@redhat.com> > >> >>> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> > >> >> > >> >> NAK > >> >> > >> >> I use all this in tools to debug lockup. Best course of action is to > >> >> exclude bof.h from being distributed. My tools static link and i just > >> >> point them to libdrm git tree. > >> >> > >> > Did not notice any mention of such out-of-tree tools in the commit > >> > that introduced these functions, so I've naively assumed that they are > >> > unused. > >> Scratch that - I'm blind. > >> > >> Upon closer look at your radeondb repo, I cannot see any static > >> linking in there. Also it seems that some of the functionality is > >> duplicated between the two. With the radeondb version being out of > >> date :'( > > > > Yeah i guess i never pushed anywhere patches that did that, divergence btw > > my memory and what is out there. All this symbol can just be hidden and > > never exported. It would cleaner, but i still need the bof.h intact as i > > tend to just cp it afaict into my local radeondb copy so that i am in > > sync with libdrm code. > > > I can volunteer with the cleanup/integration of radeondb next to > libdrm_radeon. If you update your repo (or push your work elsewhere), > I could double-check, integrate and nuke the duplication. It will > avoid the next person from coming over and trying to nuke things, the > divergence mentioned, plus the copy/pasting of bof.[ch] every time you > use the tool. > > How does that sound ? If you feel like it yes, but as i said i fear bof will stay relevant only for the duration xf86-video-ati is and i fear with the advance of the generic modesetting and glamor acceleration this might not last long. As mesa is using a different scheme to allow capture and replay of cs. Anyway, the only tool that matter regarding bof is bofreplay from my joujou repository git://people.freedesktop.org/~glisse/joujou I used to have a tool to allow bisecting bof cs but it might have been lost in translation somewhere. Thought all is needed is a parameter to bofreplay to limit the number of dwords replayed. Happy coding if you decide to go down that road :) Cheers, Jérôme > > Cheers, > Emil
diff --git a/radeon/bof.h b/radeon/bof.h index 014affb..cb829a1 100644 --- a/radeon/bof.h +++ b/radeon/bof.h @@ -51,10 +51,6 @@ typedef struct bof { long offset; } bof_t; -extern int bof_file_flush(bof_t *root); -extern bof_t *bof_file_new(const char *filename); -extern int bof_object_dump(bof_t *object, const char *filename); - /* object */ extern bof_t *bof_object(void); extern bof_t *bof_object_get(bof_t *object, const char *keyname);
Missing definition and unused since their introduction. Cc: Jerome Glisse <jglisse@redhat.com> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> --- radeon/bof.h | 4 ---- 1 file changed, 4 deletions(-)