From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTPS id EE4B66E91C for ; Mon, 26 Jul 2021 21:08:51 +0000 (UTC) Date: Mon, 26 Jul 2021 13:54:04 -0700 Message-ID: <87r1fkokyb.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" In-Reply-To: <20210726122636.159703-1-bhanuprakash.modem@intel.com> References: <20210726094938.155619-1-bhanuprakash.modem@intel.com> <20210726122636.159703-1-bhanuprakash.modem@intel.com> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Subject: Re: [igt-dev] [i-g-t V2] lib/i915: Handle QUERY_MEMORY_REGIONS on older kernels List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Bhanuprakash Modem Cc: igt-dev@lists.freedesktop.org List-ID: On Mon, 26 Jul 2021 05:26:36 -0700, Bhanuprakash Modem wrote: > > While checking for LMEM support on older kernels with the > DRM_I915_QUERY_MEMORY_REGIONS, it may fail with EINVAL. > > Instead of abort/asserting the test, we must handle this > case properly. The patch is fine now. But I would still state clearly in the commit message what the objective for the patch is. So something like: "Instead of abort/asserting the test make sure gem_has_lmem remains functional". With a suitable commit message this is: Reviewed-by: Ashutosh Dixit > V2: > * Rebase > > Cc: Ashutosh Dixit > Cc: Vidya Srinivas > Signed-off-by: Bhanuprakash Modem > --- > lib/i915/intel_memory_region.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/lib/i915/intel_memory_region.c b/lib/i915/intel_memory_region.c > index e1e210f2c..72e337af3 100644 > --- a/lib/i915/intel_memory_region.c > +++ b/lib/i915/intel_memory_region.c > @@ -114,7 +114,7 @@ uint32_t gem_get_batch_size(int fd, uint8_t mem_region_type) > struct drm_i915_query_memory_regions *gem_get_query_memory_regions(int fd) > { > struct drm_i915_query_item item; > - struct drm_i915_query_memory_regions *query_info; > + struct drm_i915_query_memory_regions *query_info = NULL; > > memset(&item, 0, sizeof(item)); > item.query_id = DRM_I915_QUERY_MEMORY_REGIONS; > @@ -123,15 +123,18 @@ 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. > */ > - igt_assert_f(item.length > 0, > - "DRM_I915_QUERY_MEMORY_REGIONS failed with %d\n", > - item.length); > + if (item.length < 0) { > + igt_critical("DRM_I915_QUERY_MEMORY_REGIONS failed with %d\n", > + item.length); > + goto out; > + } > > query_info = calloc(1, item.length); > > item.data_ptr = to_user_pointer(query_info); > i915_query_items(fd, &item, 1); > > +out: > return query_info; > } > > @@ -150,6 +153,9 @@ uint8_t gem_get_lmem_region_count(int fd) > uint8_t lmem_regions = 0; > > query_info = gem_get_query_memory_regions(fd); > + if (!query_info) > + goto out; > + > num_regions = query_info->num_regions; > > for (int i = 0; i < num_regions; i++) { > @@ -158,6 +164,7 @@ uint8_t gem_get_lmem_region_count(int fd) > } > free(query_info); > > +out: > return lmem_regions; > } > > -- > 2.32.0 > _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev