intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: "Dixit, Ashutosh" <ashutosh.dixit@intel.com>
To: Matthew Auld <matthew.auld@intel.com>
Cc: igt-dev@lists.freedesktop.org, intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/7] lib/i915/gem_mman: add FIXED mmap mode
Date: Tue, 27 Jul 2021 19:01:24 -0700	[thread overview]
Message-ID: <87fsvznqmj.wl-ashutosh.dixit@intel.com> (raw)
In-Reply-To: <20210726120310.1115522-1-matthew.auld@intel.com>

On Mon, 26 Jul 2021 05:03:04 -0700, Matthew Auld wrote:
>
> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
> index 4b4f2114..e2514f0c 100644
> --- a/lib/i915/gem_mman.c
> +++ b/lib/i915/gem_mman.c
> @@ -497,6 +497,43 @@ void *gem_mmap_offset__cpu(int fd, uint32_t handle, uint64_t offset,
>	return ptr;
>  }
>
> +#define LOCAL_I915_MMAP_OFFSET_FIXED 4

Cc: @Petri Latvala

This use of LOCAL declarations is more related to the methodology we follow
in IGT rather than this patch. We have seen in the past that such LOCAL's
linger on in the code for months and years till someone decides to clean
them so the question is can we prevent these LOCAL's from getting
introduced in the first place.

One reason for these is that we sync IGT headers with drm-next whereas IGT
is used to test drm-tip. So the delta between the two results in such
LOCAL's as in this case.

My proposal is that even if we don't start sync'ing IGT headers with
drm-tip (instead of drm-next) we allow direct modification of the headers
when needed to avoid introducing such LOCAL's. So in the above case we
would add:

#define I915_MMAP_OFFSET_FIXED 4

to i915_drm.h as part of this patch and then just use
I915_MMAP_OFFSET_FIXED. If another sync happens from drm-next before this
#define has appeared there, the compile will break and whoever syncs will
need to add this again to i915_drm.h.

What do people think about a scheme such as this? The other, perhaps
better, option of course is to sync the headers directly with drm-tip
(whenever and as often as needed). But the goal in both cases is to avoid
LOCAL's, or other things like #ifndef's distributed throughout multiple
source files which we also do in such cases. A centralized internal header
to contain such declarations might not be so bad. Thanks.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2021-07-28  2:08 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-26 12:03 [Intel-gfx] [PATCH i-g-t 1/7] lib/i915/gem_mman: add FIXED mmap mode Matthew Auld
2021-07-26 12:03 ` [Intel-gfx] [PATCH i-g-t 2/7] lib/i915/gem_mman: add fixed mode to mmap__device_coherent Matthew Auld
2021-07-26 12:03 ` [Intel-gfx] [PATCH i-g-t 3/7] lib/i915/gem_mman: add fixed mode to mmap__cpu_coherent Matthew Auld
2021-07-26 12:03 ` [Intel-gfx] [PATCH i-g-t 4/7] lib/i915/gem_mman: update mmap_offset_types with FIXED Matthew Auld
2021-07-26 12:03 ` [Intel-gfx] [PATCH i-g-t 5/7] lib/ioctl_wrappers: update mmap_{read, write} for discrete Matthew Auld
2021-07-27 21:51   ` Dixit, Ashutosh
2021-07-26 12:03 ` [Intel-gfx] [PATCH i-g-t 6/7] lib/ioctl_wrappers: update set_domain " Matthew Auld
2021-07-26 12:03 ` [Intel-gfx] [PATCH i-g-t 7/7] tests/i915/module_load: update " Matthew Auld
2021-07-28  2:01 ` Dixit, Ashutosh [this message]
2021-07-28  5:12   ` [Intel-gfx] [igt-dev] [PATCH i-g-t 1/7] lib/i915/gem_mman: add FIXED mmap mode Dixit, Ashutosh
2021-07-28  6:08   ` Petri Latvala
2021-07-29  1:53     ` Dixit, Ashutosh
2021-07-29  8:22       ` Petri Latvala

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=87fsvznqmj.wl-ashutosh.dixit@intel.com \
    --to=ashutosh.dixit@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=matthew.auld@intel.com \
    /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).