linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rob Clark <robdclark@chromium.org>
To: Christoph Hellwig <hch@lst.de>
Cc: Rob Clark <robdclark@gmail.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	Sean Paul <sean@poorly.run>, David Airlie <airlied@linux.ie>,
	Daniel Vetter <daniel@ffwll.ch>,
	Allison Randal <allison@lohutok.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-arm-kernel@lists.infradead.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] drm: add cache support for arm64
Date: Tue, 6 Aug 2019 07:11:41 -0700	[thread overview]
Message-ID: <CAJs_Fx6eh1w7c=crMoD5XyEOMzP6orLhqUewErE51cPGYmObBQ@mail.gmail.com> (raw)
In-Reply-To: <20190806084821.GA17129@lst.de>

On Tue, Aug 6, 2019 at 1:48 AM Christoph Hellwig <hch@lst.de> wrote:
>
> This goes in the wrong direction.  drm_cflush_* are a bad API we need to
> get rid of, not add use of it.  The reason for that is two-fold:
>
>  a) it doesn't address how cache maintaince actually works in most
>     platforms.  When talking about a cache we three fundamental operations:
>
>         1) write back - this writes the content of the cache back to the
>            backing memory
>         2) invalidate - this remove the content of the cache
>         3) write back + invalidate - do both of the above

Agreed that drm_cflush_* isn't a great API.  In this particular case
(IIUC), I need wb+inv so that there aren't dirty cache lines that drop
out to memory later, and so that I don't get a cache hit on
uncached/wc mmap'ing.

>  b) which of the above operation you use when depends on a couple of
>     factors of what you want to do with the range you do the cache
>     maintainance operations
>
> Take a look at the comment in arch/arc/mm/dma.c around line 30 that
> explains how this applies to buffer ownership management.  Note that
> "for device" applies to "for userspace" in the same way, just that
> userspace then also needs to follow this protocol.  So the whole idea
> that random driver code calls random low-level cache maintainance
> operations (and use the non-specific term flush to make it all more
> confusing) is a bad idea.  Fortunately enough we have really good
> arch helpers for all non-coherent architectures (this excludes the
> magic i915 won't be covered by that, but that is a separate issue
> to be addressed later, and the fact that while arm32 did grew them
> very recently and doesn't expose them for all configs, which is easily
> fixable if needed) with arch_sync_dma_for_device and
> arch_sync_dma_for_cpu.  So what we need is to figure out where we
> have valid cases for buffer ownership transfer outside the DMA
> API, and build proper wrappers around the above function for that.
> My guess is it should probably be build to go with the iommu API
> as that is the only other way to map memory for DMA access, but
> if you have a better idea I'd be open to discussion.

Tying it in w/ iommu seems a bit weird to me.. but maybe that is just
me, I'm certainly willing to consider proposals or to try things and
see how they work out.

Exposing the arch_sync_* API and using that directly (bypassing
drm_cflush_*) actually seems pretty reasonable and pragmatic.  I did
have one doubt, as phys_to_virt() is only valid for kernel direct
mapped memory (AFAIU), what happens for pages that are not in kernel
linear map?  Maybe it is ok to ignore those pages, since they won't
have an aliased mapping?

BR,
-R

  parent reply	other threads:[~2019-08-06 14:11 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-05 21:14 [PATCH 1/2] drm: add cache support for arm64 Rob Clark
2019-08-05 21:14 ` [PATCH 2/2] drm/msm: use drm_cache when available Rob Clark
2019-08-06  8:48 ` [PATCH 1/2] drm: add cache support for arm64 Christoph Hellwig
2019-08-06  9:38   ` Daniel Vetter
2019-08-06 11:38     ` Christoph Hellwig
2019-08-06 14:11   ` Rob Clark [this message]
2019-08-06 14:34     ` Mark Rutland
2019-08-06 16:31       ` Rob Clark
2019-08-07 12:38         ` Mark Rutland
2019-08-07 16:15           ` Rob Clark
2019-08-07 16:49             ` Mark Rutland
2019-08-07 17:30               ` Rob Clark
2019-08-08  7:59                 ` Christoph Hellwig
2019-08-08 16:44                   ` Rob Clark
2019-08-09  8:18                     ` Christoph Hellwig
2019-08-08  7:58               ` Christoph Hellwig
2019-08-08 10:20                 ` Mark Rutland
2019-08-08 10:24                   ` Mark Rutland
2019-08-08 10:32                   ` Will Deacon
2019-08-08  7:53           ` Christoph Hellwig
2019-08-06 15:50     ` Christoph Hellwig
2019-08-06 16:23       ` Rob Clark
2019-08-06 16:26         ` Rob Clark
2019-08-07  6:25         ` Christoph Hellwig
2019-08-07  8:48           ` Daniel Vetter
2019-08-08  9:55             ` Christoph Hellwig
2019-08-08 11:58               ` Daniel Vetter
2019-08-09  8:14                 ` Christoph Hellwig
2019-08-07 16:09           ` Rob Clark
2019-08-08 10:00             ` Christoph Hellwig
2019-08-08 16:32               ` Rob Clark

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='CAJs_Fx6eh1w7c=crMoD5XyEOMzP6orLhqUewErE51cPGYmObBQ@mail.gmail.com' \
    --to=robdclark@chromium.org \
    --cc=airlied@linux.ie \
    --cc=allison@lohutok.net \
    --cc=catalin.marinas@arm.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@lst.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=maxime.ripard@bootlin.com \
    --cc=robdclark@gmail.com \
    --cc=sean@poorly.run \
    --cc=tglx@linutronix.de \
    --cc=will@kernel.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).