Re: [igt-dev] [PATCH i-g-t v3] tests/kms_prime: Create the exporting BO with smem

["Ruhl, Michael J" <michael.j.ruhl@intel.com>]

>-----Original Message-----
>From: Ruhl, Michael J
>Sent: Friday, September 17, 2021 9:04 AM
>To: C, Ramalingam <ramalingam.c@intel.com>; igt-dev <igt-
>dev@lists.freedesktop.org>
>Cc: Dixit, Ashutosh <ashutosh.dixit@intel.com>
>Subject: RE: [PATCH i-g-t v3] tests/kms_prime: Create the exporting BO with
>smem
>
>
>>-----Original Message-----
>>From: C, Ramalingam <ramalingam.c@intel.com>
>>Sent: Thursday, September 16, 2021 9:33 AM
>>To: igt-dev <igt-dev@lists.freedesktop.org>
>>Cc: Ruhl, Michael J <michael.j.ruhl@intel.com>; Dixit, Ashutosh
>><ashutosh.dixit@intel.com>
>>Subject: [PATCH i-g-t v3] tests/kms_prime: Create the exporting BO with
>>smem
>>
>>On i915, to avail the dmabuf, the sharing object needs to migratable
>>into smem. So if the shared object is lmem, then it needs to have the
>>smem as second placement option.
>
>Some minor word smithing:
>
>"the shared object needs to be migratable"
>
>"needs to have smem as"
>
>>Currently kms_prime sharing the dumb buffer between the devices.
>
>s/sharing/shares/
>
>>But dumb buffer can't have the placements and resides at lmem for the
>>dgfx. Hence to meet the i915 expectation for dgfx, we create the BO
>>using the gem_create with smem as second placement option.
>
>Would it make more sense to just have i915_gem_dumb_create() create
>the object with both placements?
>
>So then for the use case, it will "just work"?
>
>This patch looks reasonable, but the change in dumb buffer usage is a
>concern because we will need to change all usages in order to cover this
>new feature/ability.

This has apparently already been discussed, and this patch is the result.

So, with the above fixes:

Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>

m

>Mike
>
>>v2: Used gem_mmap__device_coherent for mmaped ptr (Ashutosh)
>>v3: dumb buffer ioctls are used for non i915 drivers
>>
>>Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>>Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
>>Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
>>---
>> tests/kms_prime.c | 33 +++++++++++++++++++++------------
>> 1 file changed, 21 insertions(+), 12 deletions(-)
>>
>>diff --git a/tests/kms_prime.c b/tests/kms_prime.c
>>index 2e20c58bc16e..5cdb559778b3 100644
>>--- a/tests/kms_prime.c
>>+++ b/tests/kms_prime.c
>>@@ -100,18 +100,27 @@ static void prepare_scratch(int exporter_fd, struct
>>dumb_bo *scratch,
>> 	scratch->height = mode->vdisplay;
>> 	scratch->bpp = 32;
>>
>>-	scratch->handle = kmstest_dumb_create(exporter_fd,
>>-			ALIGN(scratch->width, 256),
>>-			scratch->height,
>>-			scratch->bpp,
>>-			&scratch->pitch,
>>-			&scratch->size);
>>-
>>-
>>-	ptr = kmstest_dumb_map_buffer(exporter_fd,
>>-				      scratch->handle,
>>-				      scratch->size,
>>-				      PROT_WRITE);
>>+	if (!is_i915_device(exporter_fd)) {
>>+		scratch->handle = kmstest_dumb_create(exporter_fd,
>>+						      ALIGN(scratch->width,
>>256),
>>+						      scratch->height, scratch-
>>>bpp,
>>+						      &scratch->pitch, &scratch-
>>>size);
>>+
>>+		ptr = kmstest_dumb_map_buffer(exporter_fd, scratch-
>>>handle,
>>+					      scratch->size, PROT_WRITE);
>>+	} else {
>>+		igt_calc_fb_size(exporter_fd, mode->hdisplay, mode-
>>>vdisplay, DRM_FORMAT_XRGB8888,
>>+				 DRM_FORMAT_MOD_NONE, &scratch->size,
>>&scratch->pitch);
>>+		if (gem_has_lmem(exporter_fd))
>>+			scratch->handle =
>>gem_create_in_memory_regions(exporter_fd, scratch->size,
>>+
>>REGION_LMEM(0), REGION_SMEM);
>>+		else
>>+			scratch->handle =
>>gem_create_in_memory_regions(exporter_fd, scratch->size,
>>+
>>REGION_SMEM);
>>+
>>+		ptr = gem_mmap__device_coherent(exporter_fd, scratch-
>>>handle, 0, scratch->size,
>>+						PROT_WRITE | PROT_READ);
>>+	}
>>
>> 	for (size_t idx = 0; idx < scratch->size / sizeof(*ptr); ++idx)
>> 		ptr[idx] = color;
>>--
>>2.20.1