[igt-dev] [PATCH i-g-t 1/2] lib/i915/intel_memory_region: Provide system memory region stub

[Zbigniew Kempczyński]

On Mon, Dec 13, 2021 at 08:55:15AM -0800, Dixit, Ashutosh wrote:
> On Thu, 09 Dec 2021 12:04:52 -0800, Zbigniew Kempczyński wrote:
> >
> > diff --git a/lib/i915/intel_memory_region.c b/lib/i915/intel_memory_region.c
> > index 058b273dc..fd29eec90 100644
> > --- a/lib/i915/intel_memory_region.c
> > +++ b/lib/i915/intel_memory_region.c
> > @@ -123,9 +123,17 @@ struct drm_i915_query_memory_regions *gem_get_query_memory_regions(int fd)
> >	 * Any DRM_I915_QUERY_MEMORY_REGIONS specific errors are encoded in the
> >	 * item.length, even though the ioctl might still return success.
> >	 */
> > +
> >	if (item.length < 0) {
> > -		igt_critical("DRM_I915_QUERY_MEMORY_REGIONS failed with %d\n",
> > -			     item.length);
> > +		/*
> > +		 * If kernel supports query but not memory regions query
> > +		 * just return predefined system memory region only.
> > +		 */
> > +		size_t sys_qi_size = offsetof(typeof(*query_info), regions[1]);
> > +
> > +		query_info = calloc(1, sys_qi_size);
> > +		query_info->num_regions = 1;
> > +		query_info->regions[0].region.memory_class = I915_MEMORY_CLASS_SYSTEM;
> 
> Should there be another check here that DRM_I915_QUERY_MEMORY_REGIONS query
> was not recognized? Otherwise the query could have failed for some other
> reason?

I may differentiate for item.length == -ENODEV and return system memory
region or assert for the other errors. Currently -ENODEV path is executed
on stable kernels where we fail on integrated what's not what we want.

So you've rightly noticed I return CLASS_SYSTEM for each error we encounter
from the kernel. This is wrong for discrete because we should fail where 
invalid arguments would be passed (and not return system memory region).
As the code is already merged (rb-d by Petri on irc), I'll prepare fix for it.

Thanks for review.
--
Zbigniew