mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Thomas Zimmermann <tzimmermann@suse.de>
To: Paul Cercueil <paul@crapouillou.net>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Sam Ravnborg <sam@ravnborg.org>,
	od@zcrc.me, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org
Subject: Re: [PATCH v2 0/5] Add option to mmap GEM buffers cached
Date: Thu, 11 Mar 2021 08:52:27 +0100	[thread overview]
Message-ID: <5246f93f-b383-03be-4d5c-53cd530df079@suse.de> (raw)
In-Reply-To: <YVORPQ.MCVK409VD57J2@crapouillou.net>


[-- Attachment #1.1: Type: text/plain, Size: 4211 bytes --]

Hi

Am 10.03.21 um 20:02 schrieb Paul Cercueil:
> Hi Thomas,
> 
> Le lun. 8 mars 2021 à 9:41, Thomas Zimmermann <tzimmermann@suse.de> a 
> écrit :
>> Hi Paul,
>>
>> having individual functions for each mode only makes sense if the 
>> decision is at compile time. But in patch 5, you're working around 
>> your earlier design by introducing in-driver helpers that select the 
>> correct CMA function.
>>
>> In SHMEM helpers we have the flag map_wc in the GEM structure that 
>> selects the pages caching mode (wc vs uncached). I think CMA should 

Re-reading this, it should rather be WC and cached.

>> use this design as well. Have a map_noncoherent flag in the CMA GEM 
>> object and set it from the driver's implementation of gem_create_object.
> 
> Is that a new addition? That severely reduces the patchset size, which 
> is perfect.

It was added a few months ago, around the time you send the first 
version of the patches at hand.

Originally, SHMEM uses write combining by default. And several drivers 
used default mapping flags instead (so usually cached). IIRC I 
streamlined everything and flipped the default. Most drivers can use 
cached mappings and only some require WC.

Recently there was also a patchset that added support for cached video 
RAM (or something like that). So at some point we could store these 
flags in drm_gem_object. For now, I'd just put a flag into 
drm_gem_cma_object.

> 
> I'll send a V3 then.

Great!

Best regards
Thomas

> 
> Cheers,
> -Paul
> 
>> And in the long run, we could try to consolidate all drivers/helpers 
>> mapping flags in struct drm_gem_object.
>>
>> Best regards
>> Thomas
>>
>> Am 07.03.21 um 21:28 schrieb Paul Cercueil:
>>> Rework of my previous patchset which added support for GEM buffers
>>> backed by non-coherent memory to the ingenic-drm driver.
>>>
>>> Having GEM buffers backed by non-coherent memory is interesting in
>>> the particular case where it is faster to render to a non-coherent
>>> buffer then sync the data cache, than to render to a write-combine
>>> buffer, and (by extension) much faster than using a shadow buffer.
>>> This is true for instance on some Ingenic SoCs, where even simple
>>> blits (e.g. memcpy) are about three times faster using this method.
>>>
>>> For the record, the previous patchset was accepted for 5.10 then had
>>> to be reverted, as it conflicted with some changes made to the DMA API.
>>>
>>> This new patchset is pretty different as it adds the functionality to
>>> the DRM core. The first three patches add variants to existing functions
>>> but with the "non-coherent memory" twist, exported as GPL symbols. The
>>> fourth patch adds a function to be used with the damage helpers.
>>> Finally, the last patch adds support for non-coherent GEM buffers to the
>>> ingenic-drm driver. The functionality is enabled through a module
>>> parameter, and is disabled by default.
>>>
>>> Cheers,
>>> -Paul
>>>
>>> Paul Cercueil (5):
>>>    drm: Add and export function drm_gem_cma_create_noncoherent
>>>    drm: Add and export function drm_gem_cma_dumb_create_noncoherent
>>>    drm: Add and export function drm_gem_cma_mmap_noncoherent
>>>    drm: Add and export function drm_gem_cma_sync_data
>>>    drm/ingenic: Add option to alloc cached GEM buffers
>>>
>>>   drivers/gpu/drm/drm_gem_cma_helper.c      | 223 +++++++++++++++++++---
>>>   drivers/gpu/drm/ingenic/ingenic-drm-drv.c |  49 ++++-
>>>   drivers/gpu/drm/ingenic/ingenic-drm.h     |   4 +
>>>   drivers/gpu/drm/ingenic/ingenic-ipu.c     |  14 +-
>>>   include/drm/drm_gem_cma_helper.h          |  13 ++
>>>   5 files changed, 273 insertions(+), 30 deletions(-)
>>>
>>
>> -- 
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Felix Imendörffer
>>
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

      reply	other threads:[~2021-03-11  7:53 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-07 20:28 Paul Cercueil
2021-03-07 20:28 ` [PATCH v2 1/5] drm: Add and export function drm_gem_cma_create_noncoherent Paul Cercueil
2021-03-11 12:23   ` Christoph Hellwig
2021-03-07 20:28 ` [PATCH v2 2/5] drm: Add and export function drm_gem_cma_dumb_create_noncoherent Paul Cercueil
2021-03-07 20:28 ` [PATCH v2 3/5] drm: Add and export function drm_gem_cma_mmap_noncoherent Paul Cercueil
2021-03-11 12:26   ` Christoph Hellwig
2021-03-11 12:32     ` Paul Cercueil
2021-03-11 12:36       ` Christoph Hellwig
2021-03-11 16:12         ` Paul Cercueil
2021-03-12 16:36           ` Christoph Hellwig
2021-03-07 20:28 ` [PATCH v2 4/5] drm: Add and export function drm_gem_cma_sync_data Paul Cercueil
2021-03-11 12:28   ` Christoph Hellwig
2021-03-11 12:33     ` Paul Cercueil
2021-03-15  7:43       ` Thomas Zimmermann
2021-03-15 11:59         ` Paul Cercueil
2021-03-07 20:28 ` [PATCH v2 5/5] drm/ingenic: Add option to alloc cached GEM buffers Paul Cercueil
     [not found]   ` <20210308034727.1951-1-hdanton@sina.com>
2021-03-10 19:01     ` Paul Cercueil
     [not found]       ` <20210311022743.2542-1-hdanton@sina.com>
2021-03-11 12:10         ` Paul Cercueil
2021-03-11 12:30   ` Christoph Hellwig
2021-03-11 13:40     ` Paul Cercueil
2021-03-08  8:41 ` [PATCH v2 0/5] Add option to mmap GEM buffers cached Thomas Zimmermann
2021-03-10 19:02   ` Paul Cercueil
2021-03-11  7:52     ` Thomas Zimmermann [this message]

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=5246f93f-b383-03be-4d5c-53cd530df079@suse.de \
    --to=tzimmermann@suse.de \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=od@zcrc.me \
    --cc=paul@crapouillou.net \
    --cc=sam@ravnborg.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

all inboxes | Powered by JetHome®