From: Thomas Zimmermann <tzimmermann@suse.de> To: Paul Cercueil <paul@crapouillou.net>, Maarten Lankhorst <maarten.lankhorst@linux.intel.com>, Maxime Ripard <mripard@kernel.org>, David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch> Cc: Christoph Hellwig <hch@infradead.org>, list@opendingux.net, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org Subject: Re: [PATCH v4 2/3] drm: Add and export function drm_gem_cma_sync_data Date: Sat, 15 May 2021 21:06:56 +0200 [thread overview] Message-ID: <93fce1ea-f18d-7941-e973-9748243882b6@suse.de> (raw) In-Reply-To: <20210515145359.64802-3-paul@crapouillou.net> [-- Attachment #1.1: Type: text/plain, Size: 5094 bytes --] Hi Am 15.05.21 um 16:53 schrieb Paul Cercueil: > This function can be used by drivers that use damage clips and have > CMA GEM objects backed by non-coherent memory. Calling this function > in a plane's .atomic_update ensures that all the data in the backing > memory have been written to RAM. > > v3: - Only sync data if using GEM objects backed by non-coherent memory. > - Use a drm_device pointer instead of device pointer in prototype > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> > --- > drivers/gpu/drm/drm_gem_cma_helper.c | 55 ++++++++++++++++++++++++++++ > include/drm/drm_gem_cma_helper.h | 5 +++ > 2 files changed, 60 insertions(+) > > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c > index 235c7a63da2b..41f309e0e049 100644 > --- a/drivers/gpu/drm/drm_gem_cma_helper.c > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c > @@ -17,9 +17,14 @@ > #include <linux/slab.h> > > #include <drm/drm.h> > +#include <drm/drm_damage_helper.h> > #include <drm/drm_device.h> > #include <drm/drm_drv.h> > +#include <drm/drm_fourcc.h> > +#include <drm/drm_fb_cma_helper.h> Alphabetical order: fb < fourcc > +#include <drm/drm_framebuffer.h> > #include <drm/drm_gem_cma_helper.h> > +#include <drm/drm_plane.h> > #include <drm/drm_vma_manager.h> > > /** > @@ -576,3 +581,53 @@ drm_gem_cma_prime_import_sg_table_vmap(struct drm_device *dev, > return obj; > } > EXPORT_SYMBOL(drm_gem_cma_prime_import_sg_table_vmap); > + > +/** > + * drm_gem_cma_sync_data - Sync GEM object to non-coherent backing memory > + * @drm: DRM device > + * @old_state: Old plane state > + * @state: New plane state > + * > + * This function can be used by drivers that use damage clips and have > + * CMA GEM objects backed by non-coherent memory. Calling this function > + * in a plane's .atomic_update ensures that all the data in the backing > + * memory have been written to RAM. > + */ > +void drm_gem_cma_sync_data(struct drm_device *drm, > + struct drm_plane_state *old_state, > + struct drm_plane_state *state) > +{ > + const struct drm_format_info *finfo = state->fb->format; > + struct drm_atomic_helper_damage_iter iter; > + const struct drm_gem_cma_object *cma_obj; > + unsigned int offset, i; > + struct drm_rect clip; > + dma_addr_t daddr; > + > + for (i = 0; i < finfo->num_planes; i++) { > + cma_obj = drm_fb_cma_get_gem_obj(state->fb, i); > + > + if (cma_obj->map_noncoherent) > + break; > + } > + > + /* No non-coherent buffers - no need to sync anything. */ > + if (i == finfo->num_planes) > + return; > + > + drm_atomic_helper_damage_iter_init(&iter, old_state, state); > + > + drm_atomic_for_each_plane_damage(&iter, &clip) { > + for (i = 0; i < finfo->num_planes; i++) { > + daddr = drm_fb_cma_get_gem_addr(state->fb, state, i); > + > + /* Ignore x1/x2 values, invalidate complete lines */ > + offset = clip.y1 * state->fb->pitches[i]; > + > + dma_sync_single_for_device(drm->dev, daddr + offset, > + (clip.y2 - clip.y1) * state->fb->pitches[i], > + DMA_TO_DEVICE); A framebuffer can have multiple BOs with different coherency. The current loop syncs every BO, but you only have to sync non-coherent memory. I suggest to merge the above test loop into this sync loop, such that only non-coherent BOs get synced damage_iter_init(iter) for_each_damage_plane(iter) { for (i < finfo->num_planes) { cma_obj = drm_fb_cma_get_gem_obj(i) if (!cma_obj->non_coherent) continue; dma_sync_single_for_device() } } For cache locality, it might be better to exchange the loops: for (i < finfo->num_planes) { damage_iter_init(iter) for_each_damage_plane(iter) { } } This way, you operate on the BOs one by one. > + } > + } > +} > +EXPORT_SYMBOL_GPL(drm_gem_cma_sync_data); > diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h > index cd13508acbc1..76af066ae3a7 100644 > --- a/include/drm/drm_gem_cma_helper.h > +++ b/include/drm/drm_gem_cma_helper.h > @@ -7,6 +7,7 @@ > #include <drm/drm_gem.h> > > struct drm_mode_create_dumb; > +struct drm_plane_state; > > /** > * struct drm_gem_cma_object - GEM object backed by CMA memory allocations > @@ -185,4 +186,8 @@ drm_gem_cma_prime_import_sg_table_vmap(struct drm_device *drm, > struct dma_buf_attachment *attach, > struct sg_table *sgt); > > +void drm_gem_cma_sync_data(struct drm_device *drm, > + struct drm_plane_state *old_state, > + struct drm_plane_state *state); > + Maybe call this function drm_gem_cma_sync_non_coherent() so that it's clear what the sync is about. Best regards Thomas > #endif /* __DRM_GEM_CMA_HELPER_H__ */ > -- 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 --]
WARNING: multiple messages have this Message-ID (diff)
From: Thomas Zimmermann <tzimmermann@suse.de> To: Paul Cercueil <paul@crapouillou.net>, Maarten Lankhorst <maarten.lankhorst@linux.intel.com>, Maxime Ripard <mripard@kernel.org>, David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch> Cc: Christoph Hellwig <hch@infradead.org>, list@opendingux.net, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-mips@vger.kernel.org Subject: Re: [PATCH v4 2/3] drm: Add and export function drm_gem_cma_sync_data Date: Sat, 15 May 2021 21:06:56 +0200 [thread overview] Message-ID: <93fce1ea-f18d-7941-e973-9748243882b6@suse.de> (raw) In-Reply-To: <20210515145359.64802-3-paul@crapouillou.net> [-- Attachment #1.1: Type: text/plain, Size: 5094 bytes --] Hi Am 15.05.21 um 16:53 schrieb Paul Cercueil: > This function can be used by drivers that use damage clips and have > CMA GEM objects backed by non-coherent memory. Calling this function > in a plane's .atomic_update ensures that all the data in the backing > memory have been written to RAM. > > v3: - Only sync data if using GEM objects backed by non-coherent memory. > - Use a drm_device pointer instead of device pointer in prototype > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> > --- > drivers/gpu/drm/drm_gem_cma_helper.c | 55 ++++++++++++++++++++++++++++ > include/drm/drm_gem_cma_helper.h | 5 +++ > 2 files changed, 60 insertions(+) > > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c > index 235c7a63da2b..41f309e0e049 100644 > --- a/drivers/gpu/drm/drm_gem_cma_helper.c > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c > @@ -17,9 +17,14 @@ > #include <linux/slab.h> > > #include <drm/drm.h> > +#include <drm/drm_damage_helper.h> > #include <drm/drm_device.h> > #include <drm/drm_drv.h> > +#include <drm/drm_fourcc.h> > +#include <drm/drm_fb_cma_helper.h> Alphabetical order: fb < fourcc > +#include <drm/drm_framebuffer.h> > #include <drm/drm_gem_cma_helper.h> > +#include <drm/drm_plane.h> > #include <drm/drm_vma_manager.h> > > /** > @@ -576,3 +581,53 @@ drm_gem_cma_prime_import_sg_table_vmap(struct drm_device *dev, > return obj; > } > EXPORT_SYMBOL(drm_gem_cma_prime_import_sg_table_vmap); > + > +/** > + * drm_gem_cma_sync_data - Sync GEM object to non-coherent backing memory > + * @drm: DRM device > + * @old_state: Old plane state > + * @state: New plane state > + * > + * This function can be used by drivers that use damage clips and have > + * CMA GEM objects backed by non-coherent memory. Calling this function > + * in a plane's .atomic_update ensures that all the data in the backing > + * memory have been written to RAM. > + */ > +void drm_gem_cma_sync_data(struct drm_device *drm, > + struct drm_plane_state *old_state, > + struct drm_plane_state *state) > +{ > + const struct drm_format_info *finfo = state->fb->format; > + struct drm_atomic_helper_damage_iter iter; > + const struct drm_gem_cma_object *cma_obj; > + unsigned int offset, i; > + struct drm_rect clip; > + dma_addr_t daddr; > + > + for (i = 0; i < finfo->num_planes; i++) { > + cma_obj = drm_fb_cma_get_gem_obj(state->fb, i); > + > + if (cma_obj->map_noncoherent) > + break; > + } > + > + /* No non-coherent buffers - no need to sync anything. */ > + if (i == finfo->num_planes) > + return; > + > + drm_atomic_helper_damage_iter_init(&iter, old_state, state); > + > + drm_atomic_for_each_plane_damage(&iter, &clip) { > + for (i = 0; i < finfo->num_planes; i++) { > + daddr = drm_fb_cma_get_gem_addr(state->fb, state, i); > + > + /* Ignore x1/x2 values, invalidate complete lines */ > + offset = clip.y1 * state->fb->pitches[i]; > + > + dma_sync_single_for_device(drm->dev, daddr + offset, > + (clip.y2 - clip.y1) * state->fb->pitches[i], > + DMA_TO_DEVICE); A framebuffer can have multiple BOs with different coherency. The current loop syncs every BO, but you only have to sync non-coherent memory. I suggest to merge the above test loop into this sync loop, such that only non-coherent BOs get synced damage_iter_init(iter) for_each_damage_plane(iter) { for (i < finfo->num_planes) { cma_obj = drm_fb_cma_get_gem_obj(i) if (!cma_obj->non_coherent) continue; dma_sync_single_for_device() } } For cache locality, it might be better to exchange the loops: for (i < finfo->num_planes) { damage_iter_init(iter) for_each_damage_plane(iter) { } } This way, you operate on the BOs one by one. > + } > + } > +} > +EXPORT_SYMBOL_GPL(drm_gem_cma_sync_data); > diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h > index cd13508acbc1..76af066ae3a7 100644 > --- a/include/drm/drm_gem_cma_helper.h > +++ b/include/drm/drm_gem_cma_helper.h > @@ -7,6 +7,7 @@ > #include <drm/drm_gem.h> > > struct drm_mode_create_dumb; > +struct drm_plane_state; > > /** > * struct drm_gem_cma_object - GEM object backed by CMA memory allocations > @@ -185,4 +186,8 @@ drm_gem_cma_prime_import_sg_table_vmap(struct drm_device *drm, > struct dma_buf_attachment *attach, > struct sg_table *sgt); > > +void drm_gem_cma_sync_data(struct drm_device *drm, > + struct drm_plane_state *old_state, > + struct drm_plane_state *state); > + Maybe call this function drm_gem_cma_sync_non_coherent() so that it's clear what the sync is about. Best regards Thomas > #endif /* __DRM_GEM_CMA_HELPER_H__ */ > -- 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 --]
next prev parent reply other threads:[~2021-05-15 19:07 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-15 14:53 [PATCH v4 0/3] Add option to mmap GEM buffers cached Paul Cercueil 2021-05-15 14:53 ` Paul Cercueil 2021-05-15 14:53 ` [PATCH v4 1/3] drm: Add support for GEM buffers backed by non-coherent memory Paul Cercueil 2021-05-15 14:53 ` Paul Cercueil 2021-05-15 18:49 ` Thomas Zimmermann 2021-05-15 18:49 ` Thomas Zimmermann 2021-05-15 14:53 ` [PATCH v4 2/3] drm: Add and export function drm_gem_cma_sync_data Paul Cercueil 2021-05-15 14:53 ` Paul Cercueil 2021-05-15 19:06 ` Thomas Zimmermann [this message] 2021-05-15 19:06 ` Thomas Zimmermann 2021-05-15 14:53 ` [PATCH v4 3/3] drm/ingenic: Add option to alloc cached GEM buffers Paul Cercueil 2021-05-15 14:53 ` Paul Cercueil 2021-05-15 19:42 ` Thomas Zimmermann 2021-05-15 19:42 ` Thomas Zimmermann 2021-05-15 20:08 ` Paul Cercueil 2021-05-15 20:08 ` Paul Cercueil
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=93fce1ea-f18d-7941-e973-9748243882b6@suse.de \ --to=tzimmermann@suse.de \ --cc=airlied@linux.ie \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=hch@infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mips@vger.kernel.org \ --cc=list@opendingux.net \ --cc=maarten.lankhorst@linux.intel.com \ --cc=mripard@kernel.org \ --cc=paul@crapouillou.net \ /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: linkBe 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.