Re: [Mesa-dev] [PATCH v2 2/5] intel: Move tools/decoder.[ch] to common/gen_decoder.[ch].

2017-03-24 Thread Rob Herring 
On Thu, Mar 23, 2017 at 5:32 PM, Matt Turner  wrote:
> On Wed, Mar 22, 2017 at 12:53 PM, Rob Herring  wrote:
>> On Wed, Mar 22, 2017 at 6:50 AM, Emil Velikov  
>> wrote:
>>> n 21 March 2017 at 20:58, Kenneth Graunke  wrote:
 On Tuesday, March 21, 2017 4:40:26 AM PDT Emil Velikov wrote:
> On 21 March 2017 at 11:14, Emil Velikov  wrote:
> > On 20 March 2017 at 22:04, Kenneth Graunke  
> > wrote:
> >> This way they become part of libintel_common.la so I can use them in
> >> the i965 driver.
> >> ---
> >>  src/intel/Makefile.sources  | 2 ++
> >>  src/intel/Makefile.tools.am | 2 --
> >>  src/intel/{tools/decoder.c => common/gen_decoder.c} | 2 +-
> >>  src/intel/{tools/decoder.h => common/gen_decoder.h} | 6 +++---
> >>  src/intel/tools/aubinator.c | 2 +-
> >>  5 files changed, 7 insertions(+), 7 deletions(-)
> >>  rename src/intel/{tools/decoder.c => common/gen_decoder.c} (99%)
> >>  rename src/intel/{tools/decoder.h => common/gen_decoder.h} (98%)
> >>
> >> I'd forgotten than I still had my gross symlinking hack in the first
> >> version of this series.  Here's a new spin which does this "properly" 
> >> :)
> >>
> > You can do the symlink if you want to - I don't mind. This approach
> > will "bloat" every binary that links with libintel_common, since we
> > don't ask the linker to garbage collect.
> > Admittedly we only care about the ANV case, so I'll just follow-up and
> > add the GC toggle ;-)
> >
> Scratch that we do enabled it for ANV ;-)
>
> You really nicely reworked [in 3/5] making the code "debug only", so
> perhaps can we guard the code in gen_decoder.c the same way ? ... But
> that may interact badly with aubinator :-\
> Another option would be to guard the code in ifndef ANDROID - like toggle.
>
> Otherwise one will need to copy the _xml.h rules in Android, alongside
> a dummy libmesa_genxml-like library. The latter of which will need to
> be added as LOCAL_WHOLE_STATIC_LIBRARIES [for libmesa_intel_common] to
> pull/generate the headers. At the same time none of it is used since
> Android never defines DEBUG ... nor does it use the linker garbage
> collector (GC_SECTIONS)
> Tapani feel free to grep mesa for GC_SECTIONS and use in Android.

 Ugh, good point - I forgot about Android (and SCons, but thankfully we
 don't care about SCons here).  I don't have any clue how to test Android
 builds, so I'm going to go ahead and land it as is (sorry!).

>>> No need to apologise - I'm not expecting !Android people to have the
>>> setup and/or do Android builds.
>>
>> Could we at least expect the !Android people to not commit things that
>> are known to break Android until the issue is solved?
>>
>> I can't keep up with the breakage on Intel, so I guess I'll stop caring.
>
> Is it easier for you to fix things before they're in the tree, vs after?

No, either way I can't keep up with it. My annoyance here was I
rebased to pickup my latest batch of fixes and something else broke.

> We've got Jenkins doing a scons build to make sure that keeps working,
> but I'm not sure if it's reasonably possible to do the same for
> Android.

I've got a CI setup[1]. It's broken so much though, I haven't started
spamming the list with reports. I can't find new problems if the tree
stays in a state of brokenness.

> Is your suggestion for Mesa developers to gate their patches on
> updating the Android build system?

For trivial cases, yes. This one even had the change described in the
thread and the fix doesn't even touch Android build files. At least
have some coordination with various folks who do care and can fix
things. Maybe we can provide fixes before things are committed.
Minimally we can start fixing things when patches are under review
rather than after being committed and after CI build breaks.

Rob

[1] https://ci.linaro.org/job/robher-aosp/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 2/5] intel: Move tools/decoder.[ch] to common/gen_decoder.[ch].

2017-03-23 Thread Rob Clark 
On Thu, Mar 23, 2017 at 6:32 PM, Matt Turner  wrote:
> On Wed, Mar 22, 2017 at 12:53 PM, Rob Herring  wrote:
>> On Wed, Mar 22, 2017 at 6:50 AM, Emil Velikov  
>> wrote:
>>> n 21 March 2017 at 20:58, Kenneth Graunke  wrote:
 On Tuesday, March 21, 2017 4:40:26 AM PDT Emil Velikov wrote:
> On 21 March 2017 at 11:14, Emil Velikov  wrote:
> > On 20 March 2017 at 22:04, Kenneth Graunke  
> > wrote:
> >> This way they become part of libintel_common.la so I can use them in
> >> the i965 driver.
> >> ---
> >>  src/intel/Makefile.sources  | 2 ++
> >>  src/intel/Makefile.tools.am | 2 --
> >>  src/intel/{tools/decoder.c => common/gen_decoder.c} | 2 +-
> >>  src/intel/{tools/decoder.h => common/gen_decoder.h} | 6 +++---
> >>  src/intel/tools/aubinator.c | 2 +-
> >>  5 files changed, 7 insertions(+), 7 deletions(-)
> >>  rename src/intel/{tools/decoder.c => common/gen_decoder.c} (99%)
> >>  rename src/intel/{tools/decoder.h => common/gen_decoder.h} (98%)
> >>
> >> I'd forgotten than I still had my gross symlinking hack in the first
> >> version of this series.  Here's a new spin which does this "properly" 
> >> :)
> >>
> > You can do the symlink if you want to - I don't mind. This approach
> > will "bloat" every binary that links with libintel_common, since we
> > don't ask the linker to garbage collect.
> > Admittedly we only care about the ANV case, so I'll just follow-up and
> > add the GC toggle ;-)
> >
> Scratch that we do enabled it for ANV ;-)
>
> You really nicely reworked [in 3/5] making the code "debug only", so
> perhaps can we guard the code in gen_decoder.c the same way ? ... But
> that may interact badly with aubinator :-\
> Another option would be to guard the code in ifndef ANDROID - like toggle.
>
> Otherwise one will need to copy the _xml.h rules in Android, alongside
> a dummy libmesa_genxml-like library. The latter of which will need to
> be added as LOCAL_WHOLE_STATIC_LIBRARIES [for libmesa_intel_common] to
> pull/generate the headers. At the same time none of it is used since
> Android never defines DEBUG ... nor does it use the linker garbage
> collector (GC_SECTIONS)
> Tapani feel free to grep mesa for GC_SECTIONS and use in Android.

 Ugh, good point - I forgot about Android (and SCons, but thankfully we
 don't care about SCons here).  I don't have any clue how to test Android
 builds, so I'm going to go ahead and land it as is (sorry!).

>>> No need to apologise - I'm not expecting !Android people to have the
>>> setup and/or do Android builds.
>>
>> Could we at least expect the !Android people to not commit things that
>> are known to break Android until the issue is solved?
>>
>> I can't keep up with the breakage on Intel, so I guess I'll stop caring.
>
> Is it easier for you to fix things before they're in the tree, vs after?
>
> We've got Jenkins doing a scons build to make sure that keeps working,
> but I'm not sure if it's reasonably possible to do the same for
> Android.
>
> Is your suggestion for Mesa developers to gate their patches on
> updating the Android build system?

to be fair, I guess not *every* patch is touching the build system..

That said, I guess intel has folks actually focusing on doing android
(judging by drm-hwc gerrit) so I guess in the intel case, Rob ignoring
it and different intel folks fixing the android build on their own
schedule seems like a valid option..

(although if somehow meson provided some magic way that more could be
shared between android and linux build systems, that would certainly
be a good thing.. building android isn't something I'd want to inflict
on any mesa dev who had the audacity to move a bit of code around..
but in the current state I also don't envy the task of keeping AOSP
master + mesa master working together)

BR,
-R
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 2/5] intel: Move tools/decoder.[ch] to common/gen_decoder.[ch].

2017-03-23 Thread Matt Turner 
On Wed, Mar 22, 2017 at 12:53 PM, Rob Herring  wrote:
> On Wed, Mar 22, 2017 at 6:50 AM, Emil Velikov  
> wrote:
>> n 21 March 2017 at 20:58, Kenneth Graunke  wrote:
>>> On Tuesday, March 21, 2017 4:40:26 AM PDT Emil Velikov wrote:
 On 21 March 2017 at 11:14, Emil Velikov  wrote:
 > On 20 March 2017 at 22:04, Kenneth Graunke  wrote:
 >> This way they become part of libintel_common.la so I can use them in
 >> the i965 driver.
 >> ---
 >>  src/intel/Makefile.sources  | 2 ++
 >>  src/intel/Makefile.tools.am | 2 --
 >>  src/intel/{tools/decoder.c => common/gen_decoder.c} | 2 +-
 >>  src/intel/{tools/decoder.h => common/gen_decoder.h} | 6 +++---
 >>  src/intel/tools/aubinator.c | 2 +-
 >>  5 files changed, 7 insertions(+), 7 deletions(-)
 >>  rename src/intel/{tools/decoder.c => common/gen_decoder.c} (99%)
 >>  rename src/intel/{tools/decoder.h => common/gen_decoder.h} (98%)
 >>
 >> I'd forgotten than I still had my gross symlinking hack in the first
 >> version of this series.  Here's a new spin which does this "properly" :)
 >>
 > You can do the symlink if you want to - I don't mind. This approach
 > will "bloat" every binary that links with libintel_common, since we
 > don't ask the linker to garbage collect.
 > Admittedly we only care about the ANV case, so I'll just follow-up and
 > add the GC toggle ;-)
 >
 Scratch that we do enabled it for ANV ;-)

 You really nicely reworked [in 3/5] making the code "debug only", so
 perhaps can we guard the code in gen_decoder.c the same way ? ... But
 that may interact badly with aubinator :-\
 Another option would be to guard the code in ifndef ANDROID - like toggle.

 Otherwise one will need to copy the _xml.h rules in Android, alongside
 a dummy libmesa_genxml-like library. The latter of which will need to
 be added as LOCAL_WHOLE_STATIC_LIBRARIES [for libmesa_intel_common] to
 pull/generate the headers. At the same time none of it is used since
 Android never defines DEBUG ... nor does it use the linker garbage
 collector (GC_SECTIONS)
 Tapani feel free to grep mesa for GC_SECTIONS and use in Android.
>>>
>>> Ugh, good point - I forgot about Android (and SCons, but thankfully we
>>> don't care about SCons here).  I don't have any clue how to test Android
>>> builds, so I'm going to go ahead and land it as is (sorry!).
>>>
>> No need to apologise - I'm not expecting !Android people to have the
>> setup and/or do Android builds.
>
> Could we at least expect the !Android people to not commit things that
> are known to break Android until the issue is solved?
>
> I can't keep up with the breakage on Intel, so I guess I'll stop caring.

Is it easier for you to fix things before they're in the tree, vs after?

We've got Jenkins doing a scons build to make sure that keeps working,
but I'm not sure if it's reasonably possible to do the same for
Android.

Is your suggestion for Mesa developers to gate their patches on
updating the Android build system?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 2/5] intel: Move tools/decoder.[ch] to common/gen_decoder.[ch].

2017-03-22 Thread Rob Herring 
On Wed, Mar 22, 2017 at 6:50 AM, Emil Velikov  wrote:
> n 21 March 2017 at 20:58, Kenneth Graunke  wrote:
>> On Tuesday, March 21, 2017 4:40:26 AM PDT Emil Velikov wrote:
>>> On 21 March 2017 at 11:14, Emil Velikov  wrote:
>>> > On 20 March 2017 at 22:04, Kenneth Graunke  wrote:
>>> >> This way they become part of libintel_common.la so I can use them in
>>> >> the i965 driver.
>>> >> ---
>>> >>  src/intel/Makefile.sources  | 2 ++
>>> >>  src/intel/Makefile.tools.am | 2 --
>>> >>  src/intel/{tools/decoder.c => common/gen_decoder.c} | 2 +-
>>> >>  src/intel/{tools/decoder.h => common/gen_decoder.h} | 6 +++---
>>> >>  src/intel/tools/aubinator.c | 2 +-
>>> >>  5 files changed, 7 insertions(+), 7 deletions(-)
>>> >>  rename src/intel/{tools/decoder.c => common/gen_decoder.c} (99%)
>>> >>  rename src/intel/{tools/decoder.h => common/gen_decoder.h} (98%)
>>> >>
>>> >> I'd forgotten than I still had my gross symlinking hack in the first
>>> >> version of this series.  Here's a new spin which does this "properly" :)
>>> >>
>>> > You can do the symlink if you want to - I don't mind. This approach
>>> > will "bloat" every binary that links with libintel_common, since we
>>> > don't ask the linker to garbage collect.
>>> > Admittedly we only care about the ANV case, so I'll just follow-up and
>>> > add the GC toggle ;-)
>>> >
>>> Scratch that we do enabled it for ANV ;-)
>>>
>>> You really nicely reworked [in 3/5] making the code "debug only", so
>>> perhaps can we guard the code in gen_decoder.c the same way ? ... But
>>> that may interact badly with aubinator :-\
>>> Another option would be to guard the code in ifndef ANDROID - like toggle.
>>>
>>> Otherwise one will need to copy the _xml.h rules in Android, alongside
>>> a dummy libmesa_genxml-like library. The latter of which will need to
>>> be added as LOCAL_WHOLE_STATIC_LIBRARIES [for libmesa_intel_common] to
>>> pull/generate the headers. At the same time none of it is used since
>>> Android never defines DEBUG ... nor does it use the linker garbage
>>> collector (GC_SECTIONS)
>>> Tapani feel free to grep mesa for GC_SECTIONS and use in Android.
>>
>> Ugh, good point - I forgot about Android (and SCons, but thankfully we
>> don't care about SCons here).  I don't have any clue how to test Android
>> builds, so I'm going to go ahead and land it as is (sorry!).
>>
> No need to apologise - I'm not expecting !Android people to have the
> setup and/or do Android builds.

Could we at least expect the !Android people to not commit things that
are known to break Android until the issue is solved?

I can't keep up with the breakage on Intel, so I guess I'll stop caring.

Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 2/5] intel: Move tools/decoder.[ch] to common/gen_decoder.[ch].

2017-03-22 Thread Emil Velikov 
n 21 March 2017 at 20:58, Kenneth Graunke  wrote:
> On Tuesday, March 21, 2017 4:40:26 AM PDT Emil Velikov wrote:
>> On 21 March 2017 at 11:14, Emil Velikov  wrote:
>> > On 20 March 2017 at 22:04, Kenneth Graunke  wrote:
>> >> This way they become part of libintel_common.la so I can use them in
>> >> the i965 driver.
>> >> ---
>> >>  src/intel/Makefile.sources  | 2 ++
>> >>  src/intel/Makefile.tools.am | 2 --
>> >>  src/intel/{tools/decoder.c => common/gen_decoder.c} | 2 +-
>> >>  src/intel/{tools/decoder.h => common/gen_decoder.h} | 6 +++---
>> >>  src/intel/tools/aubinator.c | 2 +-
>> >>  5 files changed, 7 insertions(+), 7 deletions(-)
>> >>  rename src/intel/{tools/decoder.c => common/gen_decoder.c} (99%)
>> >>  rename src/intel/{tools/decoder.h => common/gen_decoder.h} (98%)
>> >>
>> >> I'd forgotten than I still had my gross symlinking hack in the first
>> >> version of this series.  Here's a new spin which does this "properly" :)
>> >>
>> > You can do the symlink if you want to - I don't mind. This approach
>> > will "bloat" every binary that links with libintel_common, since we
>> > don't ask the linker to garbage collect.
>> > Admittedly we only care about the ANV case, so I'll just follow-up and
>> > add the GC toggle ;-)
>> >
>> Scratch that we do enabled it for ANV ;-)
>>
>> You really nicely reworked [in 3/5] making the code "debug only", so
>> perhaps can we guard the code in gen_decoder.c the same way ? ... But
>> that may interact badly with aubinator :-\
>> Another option would be to guard the code in ifndef ANDROID - like toggle.
>>
>> Otherwise one will need to copy the _xml.h rules in Android, alongside
>> a dummy libmesa_genxml-like library. The latter of which will need to
>> be added as LOCAL_WHOLE_STATIC_LIBRARIES [for libmesa_intel_common] to
>> pull/generate the headers. At the same time none of it is used since
>> Android never defines DEBUG ... nor does it use the linker garbage
>> collector (GC_SECTIONS)
>> Tapani feel free to grep mesa for GC_SECTIONS and use in Android.
>
> Ugh, good point - I forgot about Android (and SCons, but thankfully we
> don't care about SCons here).  I don't have any clue how to test Android
> builds, so I'm going to go ahead and land it as is (sorry!).
>
No need to apologise - I'm not expecting !Android people to have the
setup and/or do Android builds.

> I think there's an easy fix: in Makefile.sources, rename COMMON_FILES
> to MOST_COMMON_FILES (or some better name).  In Makefile.common.am, set
> COMMON_FILES to MOST_COMMON_FILES and gen_decoder.c.  Then, the code
> that needs the _xml.h rules won't get built.  It also won't be used
> if Android doesn't define DEBUG.
>
This sounds like a lot better solution. thanks for bringing it up !

Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 2/5] intel: Move tools/decoder.[ch] to common/gen_decoder.[ch].

2017-03-21 Thread Kenneth Graunke 
On Tuesday, March 21, 2017 4:40:26 AM PDT Emil Velikov wrote:
> On 21 March 2017 at 11:14, Emil Velikov  wrote:
> > On 20 March 2017 at 22:04, Kenneth Graunke  wrote:
> >> This way they become part of libintel_common.la so I can use them in
> >> the i965 driver.
> >> ---
> >>  src/intel/Makefile.sources  | 2 ++
> >>  src/intel/Makefile.tools.am | 2 --
> >>  src/intel/{tools/decoder.c => common/gen_decoder.c} | 2 +-
> >>  src/intel/{tools/decoder.h => common/gen_decoder.h} | 6 +++---
> >>  src/intel/tools/aubinator.c | 2 +-
> >>  5 files changed, 7 insertions(+), 7 deletions(-)
> >>  rename src/intel/{tools/decoder.c => common/gen_decoder.c} (99%)
> >>  rename src/intel/{tools/decoder.h => common/gen_decoder.h} (98%)
> >>
> >> I'd forgotten than I still had my gross symlinking hack in the first
> >> version of this series.  Here's a new spin which does this "properly" :)
> >>
> > You can do the symlink if you want to - I don't mind. This approach
> > will "bloat" every binary that links with libintel_common, since we
> > don't ask the linker to garbage collect.
> > Admittedly we only care about the ANV case, so I'll just follow-up and
> > add the GC toggle ;-)
> >
> Scratch that we do enabled it for ANV ;-)
> 
> You really nicely reworked [in 3/5] making the code "debug only", so
> perhaps can we guard the code in gen_decoder.c the same way ? ... But
> that may interact badly with aubinator :-\
> Another option would be to guard the code in ifndef ANDROID - like toggle.
> 
> Otherwise one will need to copy the _xml.h rules in Android, alongside
> a dummy libmesa_genxml-like library. The latter of which will need to
> be added as LOCAL_WHOLE_STATIC_LIBRARIES [for libmesa_intel_common] to
> pull/generate the headers. At the same time none of it is used since
> Android never defines DEBUG ... nor does it use the linker garbage
> collector (GC_SECTIONS)
> Tapani feel free to grep mesa for GC_SECTIONS and use in Android.

Ugh, good point - I forgot about Android (and SCons, but thankfully we
don't care about SCons here).  I don't have any clue how to test Android
builds, so I'm going to go ahead and land it as is (sorry!).

I think there's an easy fix: in Makefile.sources, rename COMMON_FILES
to MOST_COMMON_FILES (or some better name).  In Makefile.common.am, set
COMMON_FILES to MOST_COMMON_FILES and gen_decoder.c.  Then, the code
that needs the _xml.h rules won't get built.  It also won't be used
if Android doesn't define DEBUG.

> Either way, not my call - perhaps Tapani can weight in ?
> 
> IMHO the series looks great as-is and is
> Reviewed-by: Emil Velikov 
> 
> Emil

Thanks!


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 2/5] intel: Move tools/decoder.[ch] to common/gen_decoder.[ch].

2017-03-21 Thread Tapani Pälli 



On 03/21/2017 01:40 PM, Emil Velikov wrote:

On 21 March 2017 at 11:14, Emil Velikov  wrote:

On 20 March 2017 at 22:04, Kenneth Graunke  wrote:

This way they become part of libintel_common.la so I can use them in
the i965 driver.
---
 src/intel/Makefile.sources  | 2 ++
 src/intel/Makefile.tools.am | 2 --
 src/intel/{tools/decoder.c => common/gen_decoder.c} | 2 +-
 src/intel/{tools/decoder.h => common/gen_decoder.h} | 6 +++---
 src/intel/tools/aubinator.c | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)
 rename src/intel/{tools/decoder.c => common/gen_decoder.c} (99%)
 rename src/intel/{tools/decoder.h => common/gen_decoder.h} (98%)

I'd forgotten than I still had my gross symlinking hack in the first
version of this series.  Here's a new spin which does this "properly" :)


You can do the symlink if you want to - I don't mind. This approach
will "bloat" every binary that links with libintel_common, since we
don't ask the linker to garbage collect.
Admittedly we only care about the ANV case, so I'll just follow-up and
add the GC toggle ;-)


Scratch that we do enabled it for ANV ;-)

You really nicely reworked [in 3/5] making the code "debug only", so
perhaps can we guard the code in gen_decoder.c the same way ? ... But
that may interact badly with aubinator :-\
Another option would be to guard the code in ifndef ANDROID - like toggle.

Otherwise one will need to copy the _xml.h rules in Android, alongside
a dummy libmesa_genxml-like library. The latter of which will need to
be added as LOCAL_WHOLE_STATIC_LIBRARIES [for libmesa_intel_common] to
pull/generate the headers. At the same time none of it is used since
Android never defines DEBUG ... nor does it use the linker garbage
collector (GC_SECTIONS)
Tapani feel free to grep mesa for GC_SECTIONS and use in Android.

Either way, not my call - perhaps Tapani can weight in ?


#ifndef ANDROID would feel better instead of rather complicated build 
rules but not sure how dirty it will look and feel in the end. I guess I 
will get taste of it when these patches land :)



IMHO the series looks great as-is and is
Reviewed-by: Emil Velikov 

Emil


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 2/5] intel: Move tools/decoder.[ch] to common/gen_decoder.[ch].

2017-03-21 Thread Emil Velikov 
On 21 March 2017 at 11:14, Emil Velikov  wrote:
> On 20 March 2017 at 22:04, Kenneth Graunke  wrote:
>> This way they become part of libintel_common.la so I can use them in
>> the i965 driver.
>> ---
>>  src/intel/Makefile.sources  | 2 ++
>>  src/intel/Makefile.tools.am | 2 --
>>  src/intel/{tools/decoder.c => common/gen_decoder.c} | 2 +-
>>  src/intel/{tools/decoder.h => common/gen_decoder.h} | 6 +++---
>>  src/intel/tools/aubinator.c | 2 +-
>>  5 files changed, 7 insertions(+), 7 deletions(-)
>>  rename src/intel/{tools/decoder.c => common/gen_decoder.c} (99%)
>>  rename src/intel/{tools/decoder.h => common/gen_decoder.h} (98%)
>>
>> I'd forgotten than I still had my gross symlinking hack in the first
>> version of this series.  Here's a new spin which does this "properly" :)
>>
> You can do the symlink if you want to - I don't mind. This approach
> will "bloat" every binary that links with libintel_common, since we
> don't ask the linker to garbage collect.
> Admittedly we only care about the ANV case, so I'll just follow-up and
> add the GC toggle ;-)
>
Scratch that we do enabled it for ANV ;-)

You really nicely reworked [in 3/5] making the code "debug only", so
perhaps can we guard the code in gen_decoder.c the same way ? ... But
that may interact badly with aubinator :-\
Another option would be to guard the code in ifndef ANDROID - like toggle.

Otherwise one will need to copy the _xml.h rules in Android, alongside
a dummy libmesa_genxml-like library. The latter of which will need to
be added as LOCAL_WHOLE_STATIC_LIBRARIES [for libmesa_intel_common] to
pull/generate the headers. At the same time none of it is used since
Android never defines DEBUG ... nor does it use the linker garbage
collector (GC_SECTIONS)
Tapani feel free to grep mesa for GC_SECTIONS and use in Android.

Either way, not my call - perhaps Tapani can weight in ?

IMHO the series looks great as-is and is
Reviewed-by: Emil Velikov 

Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 2/5] intel: Move tools/decoder.[ch] to common/gen_decoder.[ch].

2017-03-21 Thread Emil Velikov 
On 20 March 2017 at 22:04, Kenneth Graunke  wrote:
> This way they become part of libintel_common.la so I can use them in
> the i965 driver.
> ---
>  src/intel/Makefile.sources  | 2 ++
>  src/intel/Makefile.tools.am | 2 --
>  src/intel/{tools/decoder.c => common/gen_decoder.c} | 2 +-
>  src/intel/{tools/decoder.h => common/gen_decoder.h} | 6 +++---
>  src/intel/tools/aubinator.c | 2 +-
>  5 files changed, 7 insertions(+), 7 deletions(-)
>  rename src/intel/{tools/decoder.c => common/gen_decoder.c} (99%)
>  rename src/intel/{tools/decoder.h => common/gen_decoder.h} (98%)
>
> I'd forgotten than I still had my gross symlinking hack in the first
> version of this series.  Here's a new spin which does this "properly" :)
>
You can do the symlink if you want to - I don't mind. This approach
will "bloat" every binary that links with libintel_common, since we
don't ask the linker to garbage collect.
Admittedly we only care about the ANV case, so I'll just follow-up and
add the GC toggle ;-)

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2 2/5] intel: Move tools/decoder.[ch] to common/gen_decoder.[ch].

2017-03-20 Thread Kenneth Graunke 
This way they become part of libintel_common.la so I can use them in
the i965 driver.
---
 src/intel/Makefile.sources  | 2 ++
 src/intel/Makefile.tools.am | 2 --
 src/intel/{tools/decoder.c => common/gen_decoder.c} | 2 +-
 src/intel/{tools/decoder.h => common/gen_decoder.h} | 6 +++---
 src/intel/tools/aubinator.c | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)
 rename src/intel/{tools/decoder.c => common/gen_decoder.c} (99%)
 rename src/intel/{tools/decoder.h => common/gen_decoder.h} (98%)

I'd forgotten than I still had my gross symlinking hack in the first
version of this series.  Here's a new spin which does this "properly" :)

diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
index 4eaf380492f..839ea47d752 100644
--- a/src/intel/Makefile.sources
+++ b/src/intel/Makefile.sources
@@ -9,6 +9,8 @@ BLORP_FILES = \
 COMMON_FILES = \
common/gen_debug.c \
common/gen_debug.h \
+   common/gen_decoder.c \
+   common/gen_decoder.h \
common/gen_device_info.c \
common/gen_device_info.h \
common/gen_l3_config.c \
diff --git a/src/intel/Makefile.tools.am b/src/intel/Makefile.tools.am
index 3c98c81c876..245bd03eef5 100644
--- a/src/intel/Makefile.tools.am
+++ b/src/intel/Makefile.tools.am
@@ -23,8 +23,6 @@ noinst_PROGRAMS += tools/aubinator
 
 tools_aubinator_SOURCES = \
tools/aubinator.c \
-   tools/decoder.c \
-   tools/decoder.h \
tools/disasm.c \
tools/gen_disasm.h
 
diff --git a/src/intel/tools/decoder.c b/src/intel/common/gen_decoder.c
similarity index 99%
rename from src/intel/tools/decoder.c
rename to src/intel/common/gen_decoder.c
index 42eed4af693..1c3246f265e 100644
--- a/src/intel/tools/decoder.c
+++ b/src/intel/common/gen_decoder.c
@@ -32,7 +32,7 @@
 
 #include 
 
-#include "decoder.h"
+#include "gen_decoder.h"
 
 #include "genxml/gen6_xml.h"
 #include "genxml/gen7_xml.h"
diff --git a/src/intel/tools/decoder.h b/src/intel/common/gen_decoder.h
similarity index 98%
rename from src/intel/tools/decoder.h
rename to src/intel/common/gen_decoder.h
index 4352dea9679..1c41de80a4a 100644
--- a/src/intel/tools/decoder.h
+++ b/src/intel/common/gen_decoder.h
@@ -21,8 +21,8 @@
  * IN THE SOFTWARE.
  */
 
-#ifndef DECODER_H
-#define DECODER_H
+#ifndef GEN_DECODER_H
+#define GEN_DECODER_H
 
 #include 
 #include 
@@ -135,4 +135,4 @@ void gen_print_group(FILE *out,
  uint64_t offset, const uint32_t *p,
  int starting_dword, bool color);
 
-#endif /* DECODER_H */
+#endif /* GEN_DECODER_H */
diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c
index 42cff8c4dc5..68fd18cd684 100644
--- a/src/intel/tools/aubinator.c
+++ b/src/intel/tools/aubinator.c
@@ -39,7 +39,7 @@
 
 #include "util/macros.h"
 
-#include "decoder.h"
+#include "common/gen_decoder.h"
 #include "intel_aub.h"
 #include "gen_disasm.h"
 
-- 
2.12.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev