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@xxxxxxxxxxxxxxx> > > > > > > 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
