Re: [Intel-gfx] [PATCH i-g-t v3] tests/core_getparams: Create new test core_getparams

[Jeff McGee <jeff.mcgee@intel.com>]

On Fri, Mar 13, 2015 at 05:59:13PM +0100, Daniel Vetter wrote:
> On Fri, Mar 13, 2015 at 09:51:57AM -0700, Jeff McGee wrote:
> > On Fri, Mar 13, 2015 at 05:32:41PM +0100, Daniel Vetter wrote:
> > > On Fri, Mar 13, 2015 at 05:09:46PM +0800, Zhigang Gong wrote:
> > > > My only concern is about the following macros:
> > > > 
> > > > > +#define LOCAL_I915_PARAM_SUBSLICE_TOTAL      33
> > > > > +#define LOCAL_I915_PARAM_EU_TOTAL    34
> > > > 
> > > > How about to just use the definitons in the kernel header file?
> > > > For an example:
> > > > 
> > > >   #include <drm/i915_drm.h>
> > > > 
> > > >   #ifdef LOCAL_I915_PARAM_SUBSLICE_TOTAL
> > > >   //Put all the code into this block.
> > > >   #endif
> > > > 
> > > > Then we can avoid put the same definitions in different files,
> > > > and we can avoid unecessary testing on an old kernel which doesn't
> > > > have this kernel interface.
> > > > 
> > > > For all the other part, it LGTM.
> > > > 
> > > > Reviewed-by: Zhigang Gong <zhigang.gong@linux.intel.com>
> > > 
> > > Once we update the libdrm requirements in igt we tend to go around and
> > > replace all the now obsolete LOCAL_ defines. Imo not worth doing extra
> > > work until then.
> > > 
> > > Patch applied, thanks.
> > > -Daniel
> > > 
> > 
> > Patch applied? Do you want me to make the name change first? Should the
> > kernel part be reviewed and merged first?
> 
> Forgot my own review comment already ;-) Fixed up with a follow-up patch.
> And I'll pull the kernel part in now too.
> -Daniel

Thanks. That leaves just the libdrm patch and version bump before Beignet
can merge their patches.
-Jeff
_______________________________________________
Beignet mailing list
Beignet@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/beignet