Re: [igt-dev] [PATCH i-g-t v2 2/4] lib/i915/intel_memory_region: Add lib to manage memory regions

["Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>]

On Tue, Nov 19, 2019 at 10:03:29AM -0800, Vanshidhar Konda wrote:

<cut>
> > + * gem_query_has_memory_region:
> > + * @query_info: query result of memory regions
> > + * @region: region existance to check inside @query_info regions
> > + *
> > + * This function check existence of region in @query_info
> > + *
> > + * Returns: true if memory region was found. Otherwise false.
> > + */
> > +bool gem_query_has_memory_region(struct local_i915_query_memory_region_info *query_info,
> > +			   uint32_t region)
> > +{
> > +	for (int i = 0; i < query_info->num_regions; i++)
> > +		if (query_info->regions[i].id == region)
> > +			return true;
> > +
> > +	return false;
> > +}
> > +
> > +/**
> > + * gem_query_require_region:
> > + * @query_info: query result of memory regions
> > + * @region: region to check inside query
> > + *
> > + * Function lead to skipping test if @region doesn't exists in @query_info.
> > + */
> > +void gem_query_require_region(struct local_i915_query_memory_region_info *query_info,
> > +			      uint32_t region)
> 
> Fix formatting for second line

Ok, be in v3 in an hour.
 
> > +/* gem_create_in_memory_region_list:
> > + * @fd: opened i915 drm file descriptor
> > + * @size: requested size of the buffer
> > + * @mem_regions: memory regions array (priority list)
> > + * @num_regions: @mem_regions length
> > + */
> > +uint32_t gem_create_in_memory_region_list(int fd, uint64_t size,
> > +					  uint32_t *mem_regions,
> > +					  int num_regions)
> > +{
> > +	uint32_t handle = gem_create(fd, size);
> > +
> > +	if (gem_has_lmem(fd)) {
> 
> Why have a check for LMEM only? Is it not possible to have multiple
> SMEM regions? Or that the object is always created in SMEM?
> 
> Other than these two comments, looks good to me.

Good point. Until now I thought we will have single SMEM region,
but who knows :)

> 
> Acked-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>

Thanks for comments and A-B.

Zbigniew
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev