On Tue, Apr 29, 2014 at 04:34:33PM -0700, Eric Anholt wrote:
> Once a buffer has been named, drm_intel_bo_flink() is just a getter.
Yep. Surprising but true. (Well, it surprised me).
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index 28118b9..e35cc7e 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -1267,7 +1267,16 @@ intel_process_dri2_buffer(struct brw_context *brw,
> else
> last_mt = rb->singlesample_mt;
>
> - if (last_mt && last_mt->region->name == buffer->name)
> + /* Get the name for our previous RB mt. We know it had a name already
> (and
> + * thus the DRM call is just a getter), because it could only have been
> + * allocated by a previous intel_process_dri2_buffer(), so
> + * drm_intel_bo_flink() is just a getter.
> + */
> + uint32_t old_name = 0;
> + if (last_mt)
> + drm_intel_bo_flink(last_mt->region->bo, &old_name);
> +
> + if (old_name == buffer->name)
> return;
Without the context provided the commit message, I would not understand
the above comment. It feels too circular and wordy. Also, it's making
a claim that's only true *inside* the if-block, but the commit is
outside. And it says "is just a getter" twice; another point of
confusion.
How about something like this?
uint32_t old_name = 0;
if (last_mt) {
/* The bo already has a name because the miptree was created by
* a previous call to intel_process_dri2_buffer(). If a bo already
* has a name, then drm_intel_bo_flink() is a lowcost getter. It
* does not create a new name.
*/
drm_intel_bo_flink(last_mt->region->bo, &old_name);
}
> -bool
> -intel_region_flink(struct intel_region *region, uint32_t *name)
> -{
> - if (region->name == 0) {
> - if (drm_intel_bo_flink(region->bo, ®ion->name))
> - return false;
> - }
> -
> - *name = region->name;
> -
> - return true;
> -}
> -
Functions like intel_region_flink() make me sad. Good to see it die.
Patches up to here are
Reviewed-by: Chad Versace <chad.vers...@linux.intel.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
