On Tue, May 05, 2015 at 06:30:50PM -0300, Paulo Zanoni wrote: > 2015-04-07 10:44 GMT-03:00 Daniel Vetter <daniel@xxxxxxxx>: > > On Tue, Apr 07, 2015 at 11:12:09AM +0100, Chris Wilson wrote: > >> On Tue, Apr 07, 2015 at 11:07:07AM +0200, Daniel Vetter wrote: > >> > On Tue, Apr 07, 2015 at 09:36:37AM +0100, Chris Wilson wrote: > >> > > On Tue, Apr 07, 2015 at 10:10:25AM +0200, Daniel Vetter wrote: > >> > > > On Thu, Apr 02, 2015 at 12:15:13AM +0100, Chris Wilson wrote: > >> > > > > On Wed, Apr 01, 2015 at 07:40:59PM -0300, Paulo Zanoni wrote: > >> > > > > > +static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect *rect, > >> > > > > > + uint32_t color) > >> > > > > > +{ > >> > > > > > + uint32_t *ptr; > >> > > > > > + uint32_t tiling, swizzle; > >> > > > > > + > >> > > > > > + gem_get_tiling(fd, buf->handle, &tiling, &swizzle); > >> > > > > > + > >> > > > > > + /* We didn't implement suport for the older tiling methods yet. */ > >> > > > > > + if (tiling != I915_TILING_NONE) > >> > > > > > + igt_require(intel_gen(intel_get_drm_devid(fd)) >= 5); > >> > > > > > >> > > > > But you now do! You need something like: > >> > > > > >> > > > The problem is that the kernel hides bit17 swizzling. I chatted with Paulo > >> > > > on irc about this and we decided just ignore them all is the simplest > >> > > > approach. > >> > > > >> > > Urm, that was the whole point of GET_TILING v2. That small function is > >> > > all you need to determine when bit17 is in effect and then you get to > >> > > reuse all the direct CPU methods (as they are also used by userspace) > >> > > for earlier gen. > >> > > >> > Oh right completely forgot that we've added this. But imo can be added on > >> > top once we need it (it's not just gen2 but also some gen3 which need > >> > different tile dimensions). > >> > >> Tile size is 2048 for gen2 only right. Tile width is the same for all > >> gen3 for tiling X (and gen2), but tiling Y width depends on subgen. > >> Right? > >> > >> Just need to check because I have code that depends on this... > > > > Oh right thought about Y tiling which changes on gen3. X tiling matches > > your description afaik - I consider gem_tiled_pread the authoritative > > source for this stuff. > > Ok, so after all this discussion, what is the conclusion here? What is > required before we can merge the patch? > > Notice that with this patch we still won't have anybody using the > library (except for the test added by the patch), and all the users I > plan to add are gen5+. Also, I created VIZ-5495 to make sure we create > intel_bo_fill() at some point (and, when we do it, I suggest we just > use the implementation from igt_draw.c). > > I really think the improvements discussed here can be done after we > merge the patch because the it's not breaking anything, AFAICS. So, > can I push this? Also, after the code is on igt, it will probably be > easier to discuss the possible improvements since we'll be able to > send patches. I guess you could capture the discussion here in a TODO comment in igt_draw? But yeah I thought we've discussed this on irc and agreed that moving ahead is ok, and that extensions can be done later on. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx