intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Matthew Auld <matthew.auld@intel.com>
To: "Dixit, Ashutosh" <ashutosh.dixit@intel.com>
Cc: igt-dev@lists.freedesktop.org,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH i-g-t v2 04/11] lib/i915/gem_mman: add fixed mode to gem_mmap__cpu
Date: Thu, 29 Jul 2021 09:50:45 +0100	[thread overview]
Message-ID: <ba2ce167-59eb-95d5-61be-d9b2828ff13b@intel.com> (raw)
In-Reply-To: <87eebi3ume.wl-ashutosh.dixit@intel.com>

On 29/07/2021 00:07, Dixit, Ashutosh wrote:
> On Wed, 28 Jul 2021 03:30:34 -0700, Matthew Auld wrote:
>>
>> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
>> index 337d28fb..6f5e6d72 100644
>> --- a/lib/i915/gem_mman.c
>> +++ b/lib/i915/gem_mman.c
>> @@ -434,7 +434,13 @@ void *gem_mmap__device_coherent(int fd, uint32_t handle, uint64_t offset,
>>    */
>>   void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
>>   {
>> -	return __gem_mmap(fd, handle, offset, size, prot, 0);
>> +	void *ptr;
>> +
>> +	ptr = __gem_mmap(fd, handle, offset, size, prot, 0);
>> +	if (!ptr)
>> +		ptr = __gem_mmap_offset__fixed(fd, handle, offset, size, prot);
>> +
>> +	return ptr;
> 
> What about __gem_mmap__wc? Also shouldn't we just fix the __gem_mmap_offset
> fallback in __gem_mmap and that will take care of both __gem_mmap__cpu and
> __gem_mmap__wc?

For gem_mmap__wc it felt like slightly too much lying, since on discrete 
smem-only buffers are always wb, and so the __wc here is not what the 
user gets with the new FIXED mode. gem_mmap__device_coherent() I think 
matches this new behaviour well, where we don't explicitly state what 
the mapping type is, but instead just guarantee that the returned 
mapping is device coherent. My rough thinking was to convert most users 
of __wc over to __device_coherent(), at least in the tests that we care 
about for discrete?

On the other hand if we are happy with the lie, I don't think anything 
will break, and pretty much all testscases using mmap I think should 
just magically work on discrete, and it does mean a less less work vs 
converting to __device_coherent?

> 
> (I think it will actually also fix __gem_mmap__device_coherent and
> __gem_mmap__cpu_coherent but maybe we can still have those patches in this
> series especially if they save a couple of system calls).
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2021-07-29  8:50 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-28 10:30 [Intel-gfx] [PATCH i-g-t v2 01/11] lib/i915/gem_mman: add FIXED mmap mode Matthew Auld
2021-07-28 10:30 ` [Intel-gfx] [PATCH i-g-t v2 02/11] lib/i915/gem_mman: add fixed mode to mmap__device_coherent Matthew Auld
2021-07-28 22:23   ` Dixit, Ashutosh
2021-07-28 10:30 ` [Intel-gfx] [PATCH i-g-t v2 03/11] lib/i915/gem_mman: add fixed mode to mmap__cpu_coherent Matthew Auld
2021-07-28 22:24   ` Dixit, Ashutosh
2021-07-28 10:30 ` [Intel-gfx] [PATCH i-g-t v2 04/11] lib/i915/gem_mman: add fixed mode to gem_mmap__cpu Matthew Auld
2021-07-28 23:07   ` Dixit, Ashutosh
2021-07-29  8:50     ` Matthew Auld [this message]
2021-08-02  6:29       ` Dixit, Ashutosh
2021-07-28 10:30 ` [Intel-gfx] [PATCH i-g-t v2 05/11] lib/i915/gem_mman: update mmap_offset_types with FIXED Matthew Auld
2021-07-28 10:30 ` [Intel-gfx] [PATCH i-g-t v2 06/11] lib/ioctl_wrappers: update mmap_{read, write} for discrete Matthew Auld
2021-07-28 10:30 ` [Intel-gfx] [PATCH i-g-t v2 07/11] lib/intel_bufops: " Matthew Auld
2021-07-28 10:30 ` [Intel-gfx] [PATCH i-g-t v2 08/11] lib/ioctl_wrappers: update set_domain " Matthew Auld
2021-07-28 10:30 ` [Intel-gfx] [PATCH i-g-t v2 09/11] tests/i915/module_load: update " Matthew Auld
2021-07-28 10:30 ` [Intel-gfx] [PATCH i-g-t v2 10/11] lib/i915/gem_mman: add helper query for has_device_coherent Matthew Auld
2021-07-28 10:30 ` [Intel-gfx] [PATCH i-g-t v2 11/11] tests/i915/gem_exec_fence: use device_coherent mmap Matthew Auld
2021-07-28 22:20 ` [Intel-gfx] [PATCH i-g-t v2 01/11] lib/i915/gem_mman: add FIXED mmap mode Dixit, Ashutosh
2021-07-30  2:03   ` Dixit, Ashutosh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ba2ce167-59eb-95d5-61be-d9b2828ff13b@intel.com \
    --to=matthew.auld@intel.com \
    --cc=ashutosh.dixit@intel.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).