All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dixit, Ashutosh" <ashutosh.dixit@intel.com>
To: "Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH i-g-t] lib/i915/gem_mman.c: add cpu coherency mapping wrapper
Date: Mon, 13 Jan 2020 21:59:54 -0800	[thread overview]
Message-ID: <87eew2zig5.wl-ashutosh.dixit@intel.com> (raw)
In-Reply-To: <20200114052728.13313-1-zbigniew.kempczynski@intel.com>

On Mon, 13 Jan 2020 21:27:28 -0800, Zbigniew Kempczyński wrote:
>
> For reduce code redundancy adding a wrapper for cpu memory mapping.
>
> +void *__gem_mmap__cpu_coherent(int fd, uint32_t handle, uint64_t offset,
> +			       uint64_t size, unsigned prot)
> +{
> +	void *ptr = __gem_mmap_offset__cpu(fd, handle, offset, size, prot);
> +
> +	if (!ptr)
> +		ptr = __gem_mmap__cpu(fd, handle, offset, size, prot);
> +
> +	return ptr;
> +}

We need similar wrappers for WC and GTT too. So why don't we put this code
in __gem_mmap__cpu() itself and we can do the same for __gem_mmap__wc() and
__gem_mmap__gtt() too? Otherwise what are we going to call those functions?
Something like:

void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
{
	if (gem_has_mmap_offset(fd))
		return __gem_mmap_offset(fd, handle, offset, size, prot, I915_MMAP_OFFSET_WB);
	else
		return __gem_mmap(fd, handle, offset, size, prot, 0);
}

So I am not sure of the point of introducing new wrappers, this code could
just be put in the old existing wrappers.

> +
> +/**
> + * gem_mmap__cpu_coherent:
> + * @fd: open i915 drm file descriptor
> + * @handle: gem buffer object handle
> + * @offset: offset in the gem buffer of the mmap arena
> + * @size: size of the mmap arena
> + * @prot: memory protection bits as used by mmap()
> + *
> + * Call __gem_mmap__cpu__coherent(), asserts on fail.
> + * Offset argument passed in function call must be 0. In the future
> + * when driver will allow slice mapping of buffer object this restriction
> + * will be removed.
> + *
> + * Returns: A pointer to the created memory mapping.
> + */
> +void *gem_mmap__cpu_coherent(int fd, uint32_t handle, uint64_t offset,

This can just be gem_mmap__cpu()?

> +			     uint64_t size, unsigned prot)
> +{
> +	void *ptr;
> +
> +	igt_assert(offset == 0);

Not needed?
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

  reply	other threads:[~2020-01-14  5:59 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-14  5:27 [igt-dev] [PATCH i-g-t] lib/i915/gem_mman.c: add cpu coherency mapping wrapper Zbigniew Kempczyński
2020-01-14  5:59 ` Dixit, Ashutosh [this message]
2020-01-14 11:14   ` Chris Wilson
2020-01-16 21:47     ` Dixit, Ashutosh
2020-01-14  6:12 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2020-01-16  9:18 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork

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=87eew2zig5.wl-ashutosh.dixit@intel.com \
    --to=ashutosh.dixit@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=zbigniew.kempczynski@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.