> > I notice this had a print to stderr before with an assertion out, but > > now it fails silently. Is this change of behaviour intentional? > > It is.
Alright! :-) > > BO > > creation would previously return a valid BO gauranteed. This is no > > longer so obviously true -- although I see we later assert that the > > return is non-NULL in the caller. > > > > Could you help me understand the new logic a bit? Thank you! > > The rationale behind this change being that panfrost_bo_alloc() will > not be our last option (see patch 9). I can add the fprintf() back in > this patch, and move it to the caller in patch 9 if you prefer. Ah, that makes sense; thank you for clarifying! > > > + if (!(flags & (PAN_ALLOCATE_INVISIBLE | > > > PAN_ALLOCATE_DELAY_MMAP))) > > > + panfrost_bo_mmap(bo); > > > + else if ((flags & PAN_ALLOCATE_INVISIBLE) && (pan_debug & > > > PAN_DBG_TRACE)) > > > > I think the spacing got wacky here (on the beginning of the last line) > > > > Will fix that. +1 > > I see we now have the distinction between panfrost_bo_release (cached) > > and panfrost_bo_free (uncached). I'm worried the distinction might not > > be obvious to future Panfrost hackers. > > > > Could you add a comment above each function clarifying the cache > > behaviour? > > Looks like the _release() function can be inlined in > panfrost_bo_unreference(). I'm still not happy with the > panfrost_bo_create() name though. Maybe we should rename this one into > panfrost_get_bo(). I think splitting free/release to separate functions is good; I don't know that inlining _release() is inherently needed. I'm just wondering if we want a comment to make the distinction clear for future denizens trying to figure out which routine to use --- although inlining one would certainly solve that part... > Yes, I guess I got tired splitting things up and decided to group > changes that were kind of related in a single patch (also don't like > having 30+ patch series). I'll split that up in v4. No need to split it for v4; just a general note for future series :) _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
