Rework of my previous patchset which added support for GEM buffers backed by non-coherent memory to the ingenic-drm driver.
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 | 190 +++++++++++++++++++--- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 58 ++++++- drivers/gpu/drm/ingenic/ingenic-drm.h | 4 + drivers/gpu/drm/ingenic/ingenic-ipu.c | 12 +- include/drm/drm_gem_cma_helper.h | 13 ++ 5 files changed, 251 insertions(+), 26 deletions(-)
This function can be used by drivers that need to create a GEM object with non-coherent backing memory.
Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/gpu/drm/drm_gem_cma_helper.c | 71 +++++++++++++++++++++------- include/drm/drm_gem_cma_helper.h | 2 + 2 files changed, 56 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index 2165633c9b9e..717871d741fb 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -77,21 +77,10 @@ __drm_gem_cma_create(struct drm_device *drm, size_t size) return ERR_PTR(ret); }
-/** - * drm_gem_cma_create - allocate an object with the given size - * @drm: DRM device - * @size: size of the object to allocate - * - * This function creates a CMA GEM object and allocates a contiguous chunk of - * memory as backing store. The backing memory has the writecombine attribute - * set. - * - * Returns: - * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded negative - * error code on failure. - */ -struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, - size_t size) +static struct drm_gem_cma_object * +drm_gem_cma_create_with_cache_param(struct drm_device *drm, + size_t size, + bool noncoherent) { struct drm_gem_cma_object *cma_obj; int ret; @@ -102,8 +91,16 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, if (IS_ERR(cma_obj)) return cma_obj;
- cma_obj->vaddr = dma_alloc_wc(drm->dev, size, &cma_obj->paddr, - GFP_KERNEL | __GFP_NOWARN); + if (noncoherent) { + cma_obj->vaddr = dma_alloc_noncoherent(drm->dev, size, + &cma_obj->paddr, + DMA_TO_DEVICE, + GFP_KERNEL | __GFP_NOWARN); + + } else { + cma_obj->vaddr = dma_alloc_wc(drm->dev, size, &cma_obj->paddr, + GFP_KERNEL | __GFP_NOWARN); + } if (!cma_obj->vaddr) { drm_dbg(drm, "failed to allocate buffer with size %zu\n", size); @@ -117,8 +114,48 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, drm_gem_object_put(&cma_obj->base); return ERR_PTR(ret); } + +/** + * drm_gem_cma_create - allocate an object with the given size + * @drm: DRM device + * @size: size of the object to allocate + * + * This function creates a CMA GEM object and allocates a contiguous chunk of + * memory as backing store. The backing memory has the writecombine attribute + * set. + * + * Returns: + * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded negative + * error code on failure. + */ +struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, + size_t size) +{ + return drm_gem_cma_create_with_cache_param(drm, size, false); +} EXPORT_SYMBOL_GPL(drm_gem_cma_create);
+/** + * drm_gem_cma_create_noncoherent - allocate an object with the given size + * and non-coherent cache attribute + * @drm: DRM device + * @size: size of the object to allocate + * + * This function creates a CMA GEM object and allocates a contiguous chunk of + * memory as backing store. The backing memory has the noncoherent attribute + * set. + * + * Returns: + * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded negative + * error code on failure. + */ +struct drm_gem_cma_object * +drm_gem_cma_create_noncoherent(struct drm_device *drm, size_t size) +{ + return drm_gem_cma_create_with_cache_param(drm, size, true); +} +EXPORT_SYMBOL_GPL(drm_gem_cma_create_noncoherent); + /** * drm_gem_cma_create_with_handle - allocate an object with the given size and * return a GEM handle to it diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h index 2bfa2502607a..071c73ad7177 100644 --- a/include/drm/drm_gem_cma_helper.h +++ b/include/drm/drm_gem_cma_helper.h @@ -82,6 +82,8 @@ int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma); /* allocate physical memory */ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, size_t size); +struct drm_gem_cma_object * +drm_gem_cma_create_noncoherent(struct drm_device *drm, size_t size);
extern const struct vm_operations_struct drm_gem_cma_vm_ops;
This function can be used by drivers to create dumb buffers with non-coherent backing memory.
Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/gpu/drm/drm_gem_cma_helper.c | 37 +++++++++++++++++++++++++--- include/drm/drm_gem_cma_helper.h | 4 +++ 2 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index 717871d741fb..3bdd67795e20 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -163,6 +163,7 @@ EXPORT_SYMBOL_GPL(drm_gem_cma_create_noncoherent); * @drm: DRM device * @size: size of the object to allocate * @handle: return location for the GEM handle + * @noncoherent: allocate object with non-coherent cache attribute * * This function creates a CMA GEM object, allocating a physically contiguous * chunk of memory as backing store. The GEM object is then added to the list @@ -175,13 +176,13 @@ EXPORT_SYMBOL_GPL(drm_gem_cma_create_noncoherent); static struct drm_gem_cma_object * drm_gem_cma_create_with_handle(struct drm_file *file_priv, struct drm_device *drm, size_t size, - uint32_t *handle) + uint32_t *handle, bool noncoherent) { struct drm_gem_cma_object *cma_obj; struct drm_gem_object *gem_obj; int ret;
- cma_obj = drm_gem_cma_create(drm, size); + cma_obj = drm_gem_cma_create_with_cache_param(drm, size, noncoherent); if (IS_ERR(cma_obj)) return cma_obj;
@@ -258,7 +259,7 @@ int drm_gem_cma_dumb_create_internal(struct drm_file *file_priv, args->size = args->pitch * args->height;
cma_obj = drm_gem_cma_create_with_handle(file_priv, drm, args->size, - &args->handle); + &args->handle, false); return PTR_ERR_OR_ZERO(cma_obj); } EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create_internal); @@ -291,11 +292,39 @@ int drm_gem_cma_dumb_create(struct drm_file *file_priv, args->size = args->pitch * args->height;
cma_obj = drm_gem_cma_create_with_handle(file_priv, drm, args->size, - &args->handle); + &args->handle, false); return PTR_ERR_OR_ZERO(cma_obj); } EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create);
+/** + * drm_gem_cma_dumb_create_noncoherent - create a dumb buffer object with + * non-coherent cache attribute + * @file_priv: DRM file-private structure to create the dumb buffer for + * @drm: DRM device + * @args: IOCTL data + * + * Same as drm_gem_cma_dumb_create, but the dumb buffer object created has + * the non-coherent cache attribute set. + * + * Returns: + * 0 on success or a negative error code on failure. + */ +int drm_gem_cma_dumb_create_noncoherent(struct drm_file *file_priv, + struct drm_device *drm, + struct drm_mode_create_dumb *args) +{ + struct drm_gem_cma_object *cma_obj; + + args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8); + args->size = args->pitch * args->height; + + cma_obj = drm_gem_cma_create_with_handle(file_priv, drm, args->size, + &args->handle, true); + return PTR_ERR_OR_ZERO(cma_obj); +} +EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create_noncoherent); + const struct vm_operations_struct drm_gem_cma_vm_ops = { .open = drm_gem_vm_open, .close = drm_gem_vm_close, diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h index 071c73ad7177..d0e6a1cd0950 100644 --- a/include/drm/drm_gem_cma_helper.h +++ b/include/drm/drm_gem_cma_helper.h @@ -76,6 +76,10 @@ int drm_gem_cma_dumb_create(struct drm_file *file_priv, struct drm_device *drm, struct drm_mode_create_dumb *args);
+int drm_gem_cma_dumb_create_noncoherent(struct drm_file *file_priv, + struct drm_device *drm, + struct drm_mode_create_dumb *args); + /* set vm_flags and we can change the VM attribute to other one at here */ int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma);
This function can be used by drivers that need to mmap dumb buffers created with non-coherent backing memory.
Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/gpu/drm/drm_gem_cma_helper.c | 39 ++++++++++++++++++++++++++++ include/drm/drm_gem_cma_helper.h | 2 ++ 2 files changed, 41 insertions(+)
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index 3bdd67795e20..4ed63f4896bd 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -387,6 +387,45 @@ int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma) } EXPORT_SYMBOL_GPL(drm_gem_cma_mmap);
+/** + * drm_gem_cma_mmap_noncoherent - memory-map a CMA GEM object with + * non-coherent cache attribute + * @filp: file object + * @vma: VMA for the area to be mapped + * + * Just like drm_gem_cma_mmap, but for a GEM object backed by non-coherent + * memory. + * + * Returns: + * 0 on success or a negative error code on failure. + */ +int drm_gem_cma_mmap_noncoherent(struct file *filp, struct vm_area_struct *vma) +{ + struct drm_gem_cma_object *cma_obj; + int ret; + + ret = drm_gem_mmap(filp, vma); + if (ret) + return ret; + + cma_obj = to_drm_gem_cma_obj(vma->vm_private_data); + + /* + * Clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the + * vm_pgoff (used as a fake buffer offset by DRM) to 0 as we want to map + * the whole buffer. + */ + vma->vm_flags &= ~VM_PFNMAP; + vma->vm_pgoff = 0; + vma->vm_page_prot = vm_get_page_prot(vma->vm_flags); + + return remap_pfn_range(vma, vma->vm_start, + cma_obj->paddr >> PAGE_SHIFT, + vma->vm_end - vma->vm_start, + vma->vm_page_prot); +} +EXPORT_SYMBOL_GPL(drm_gem_cma_mmap_noncoherent); + #ifndef CONFIG_MMU /** * drm_gem_cma_get_unmapped_area - propose address for mapping in noMMU cases diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h index d0e6a1cd0950..6b01ad5581c3 100644 --- a/include/drm/drm_gem_cma_helper.h +++ b/include/drm/drm_gem_cma_helper.h @@ -83,6 +83,8 @@ int drm_gem_cma_dumb_create_noncoherent(struct drm_file *file_priv, /* set vm_flags and we can change the VM attribute to other one at here */ int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma);
+int drm_gem_cma_mmap_noncoherent(struct file *filep, struct vm_area_struct *vma); + /* allocate physical memory */ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, size_t size);
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.
Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/gpu/drm/drm_gem_cma_helper.c | 43 ++++++++++++++++++++++++++++ include/drm/drm_gem_cma_helper.h | 5 ++++ 2 files changed, 48 insertions(+)
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index 4ed63f4896bd..74bff6d45d4e 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> +#include <drm/drm_framebuffer.h> #include <drm/drm_gem_cma_helper.h> +#include <drm/drm_plane.h> #include <drm/drm_vma_manager.h>
/** @@ -742,3 +747,41 @@ 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 + * @dev: 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 device *dev, + 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; + unsigned int offset, i; + struct drm_rect clip; + dma_addr_t daddr; + + 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(dev, daddr + offset, + (clip.y2 - clip.y1) * state->fb->pitches[i], + DMA_TO_DEVICE); + } + } +} +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 6b01ad5581c3..9bdba4ce65ba 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 @@ -200,4 +201,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 device *dev, + struct drm_plane_state *old_state, + struct drm_plane_state *state); + #endif /* __DRM_GEM_CMA_HELPER_H__ */
With the module parameter ingenic-drm.cached_gem_buffers, it is possible to specify that we want GEM buffers backed by non-coherent memory.
This dramatically speeds up software rendering on Ingenic SoCs, even for tasks where write-combine memory should in theory be faster (e.g. simple blits).
Leave it disabled by default, since it is specific to one use-case (software rendering).
Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 58 +++++++++++++++++++++-- drivers/gpu/drm/ingenic/ingenic-drm.h | 4 ++ drivers/gpu/drm/ingenic/ingenic-ipu.c | 12 ++++- 3 files changed, 69 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index b9c156e13156..1ec2ec2faa04 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -9,6 +9,7 @@ #include <linux/component.h> #include <linux/clk.h> #include <linux/dma-mapping.h> +#include <linux/io.h> #include <linux/module.h> #include <linux/mutex.h> #include <linux/of_device.h> @@ -22,6 +23,7 @@ #include <drm/drm_color_mgmt.h> #include <drm/drm_crtc.h> #include <drm/drm_crtc_helper.h> +#include <drm/drm_damage_helper.h> #include <drm/drm_drv.h> #include <drm/drm_gem_cma_helper.h> #include <drm/drm_fb_cma_helper.h> @@ -97,6 +99,11 @@ struct ingenic_drm { struct notifier_block clock_nb; };
+static bool ingenic_drm_cached_gem_buf; +module_param_named(cached_gem_buffers, ingenic_drm_cached_gem_buf, bool, 0400); +MODULE_PARM_DESC(cached_gem_buffers, + "Enable fully cached GEM buffers [default=false]"); + static bool ingenic_drm_writeable_reg(struct device *dev, unsigned int reg) { switch (reg) { @@ -400,6 +407,8 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane, plane->state->fb->format->format != state->fb->format->format)) crtc_state->mode_changed = true;
+ drm_atomic_helper_check_plane_damage(state->state, state); + return 0; }
@@ -531,6 +540,14 @@ static void ingenic_drm_update_palette(struct ingenic_drm *priv, } }
+void ingenic_drm_sync_data(struct device *dev, + struct drm_plane_state *old_state, + struct drm_plane_state *state) +{ + if (ingenic_drm_cached_gem_buf) + drm_gem_cma_sync_data(dev, old_state, state); +} + static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, struct drm_plane_state *oldstate) { @@ -543,6 +560,8 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, u32 fourcc;
if (state && state->fb) { + ingenic_drm_sync_data(priv->dev, oldstate, state); + crtc_state = state->crtc->state;
addr = drm_fb_cma_get_gem_addr(state->fb, state, 0); @@ -714,7 +733,36 @@ static void ingenic_drm_disable_vblank(struct drm_crtc *crtc) regmap_update_bits(priv->map, JZ_REG_LCD_CTRL, JZ_LCD_CTRL_EOF_IRQ, 0); }
-DEFINE_DRM_GEM_CMA_FOPS(ingenic_drm_fops); +static struct drm_framebuffer * +ingenic_drm_gem_fb_create(struct drm_device *dev, struct drm_file *file, + const struct drm_mode_fb_cmd2 *mode_cmd) +{ + if (ingenic_drm_cached_gem_buf) + return drm_gem_fb_create_with_dirty(dev, file, mode_cmd); + + return drm_gem_fb_create(dev, file, mode_cmd); +} + +static int ingenic_drm_gem_cma_mmap(struct file *filp, + struct vm_area_struct *vma) +{ + if (ingenic_drm_cached_gem_buf) + return drm_gem_cma_mmap_noncoherent(filp, vma); + + return drm_gem_cma_mmap(filp, vma); +} + +static const struct file_operations ingenic_drm_fops = { + .owner = THIS_MODULE, + .open = drm_open, + .release = drm_release, + .unlocked_ioctl = drm_ioctl, + .compat_ioctl = drm_compat_ioctl, + .poll = drm_poll, + .read = drm_read, + .llseek = noop_llseek, + .mmap = ingenic_drm_gem_cma_mmap, +};
static struct drm_driver ingenic_drm_driver_data = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC, @@ -726,7 +774,7 @@ static struct drm_driver ingenic_drm_driver_data = { .patchlevel = 0,
.fops = &ingenic_drm_fops, - DRM_GEM_CMA_DRIVER_OPS, + DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE(drm_gem_cma_dumb_create_noncoherent),
.irq_handler = ingenic_drm_irq_handler, }; @@ -778,7 +826,7 @@ static const struct drm_encoder_helper_funcs ingenic_drm_encoder_helper_funcs = };
static const struct drm_mode_config_funcs ingenic_drm_mode_config_funcs = { - .fb_create = drm_gem_fb_create, + .fb_create = ingenic_drm_gem_fb_create, .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = drm_atomic_helper_check, .atomic_commit = drm_atomic_helper_commit, @@ -932,6 +980,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) return ret; }
+ drm_plane_enable_fb_damage_clips(&priv->f1); + drm_crtc_helper_add(&priv->crtc, &ingenic_drm_crtc_helper_funcs);
ret = drm_crtc_init_with_planes(drm, &priv->crtc, &priv->f1, @@ -960,6 +1010,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) return ret; }
+ drm_plane_enable_fb_damage_clips(&priv->f0); + if (IS_ENABLED(CONFIG_DRM_INGENIC_IPU) && has_components) { ret = component_bind_all(dev, drm); if (ret) { diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h b/drivers/gpu/drm/ingenic/ingenic-drm.h index 9b48ce02803d..ee3a892c0383 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm.h +++ b/drivers/gpu/drm/ingenic/ingenic-drm.h @@ -171,6 +171,10 @@ void ingenic_drm_plane_config(struct device *dev, struct drm_plane *plane, u32 fourcc); void ingenic_drm_plane_disable(struct device *dev, struct drm_plane *plane);
+void ingenic_drm_sync_data(struct device *dev, + struct drm_plane_state *old_state, + struct drm_plane_state *state); + extern struct platform_driver *ingenic_ipu_driver_ptr;
#endif /* DRIVERS_GPU_DRM_INGENIC_INGENIC_DRM_H */ diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c index fc8c6e970ee3..38c83e8cc6a5 100644 --- a/drivers/gpu/drm/ingenic/ingenic-ipu.c +++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c @@ -20,6 +20,7 @@
#include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> +#include <drm/drm_damage_helper.h> #include <drm/drm_drv.h> #include <drm/drm_fb_cma_helper.h> #include <drm/drm_fourcc.h> @@ -316,6 +317,8 @@ static void ingenic_ipu_plane_atomic_update(struct drm_plane *plane, JZ_IPU_CTRL_CHIP_EN | JZ_IPU_CTRL_LCDC_SEL); }
+ ingenic_drm_sync_data(ipu->master, oldstate, state); + /* New addresses will be committed in vblank handler... */ ipu->addr_y = drm_fb_cma_get_gem_addr(state->fb, state, 0); if (finfo->num_planes > 1) @@ -534,7 +537,7 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
if (!state->crtc || !crtc_state->mode.hdisplay || !crtc_state->mode.vdisplay) - return 0; + goto out_check_damage;
/* Plane must be fully visible */ if (state->crtc_x < 0 || state->crtc_y < 0 || @@ -551,7 +554,7 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane, return -EINVAL;
if (!osd_changed(state, plane->state)) - return 0; + goto out_check_damage;
crtc_state->mode_changed = true;
@@ -578,6 +581,9 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane, ipu->denom_w = denom_w; ipu->denom_h = denom_h;
+out_check_damage: + drm_atomic_helper_check_plane_damage(state->state, state); + return 0; }
@@ -759,6 +765,8 @@ static int ingenic_ipu_bind(struct device *dev, struct device *master, void *d) return err; }
+ drm_plane_enable_fb_damage_clips(plane); + /* * Sharpness settings range is [0,32] * 0 : nearest-neighbor
On Mon, Nov 02, 2020 at 10:06:51PM +0000, Paul Cercueil wrote:
With the module parameter ingenic-drm.cached_gem_buffers, it is possible to specify that we want GEM buffers backed by non-coherent memory.
This dramatically speeds up software rendering on Ingenic SoCs, even for tasks where write-combine memory should in theory be faster (e.g. simple blits).
Leave it disabled by default, since it is specific to one use-case (software rendering).
Signed-off-by: Paul Cercueil paul@crapouillou.net
Hm so maybe I'm making myself supremely unpopular here again with being very late, but we have dev->mode_config.prefer_shadow for this. Drivers should set this if software rendering is slow, and userspace should follow this.
Now unfortunately it looks like most kms drivers don't bother setting this, and we're a lot worse.
So if "slow sw rendering" is the only reason, setting that flag is the right option.
Now the other thing is fbdev, since fbdev doesn't have this hint at all. But we already have full generic fbdev shadow support (for defio), so for that I think the best option is adding a force_shadow option to fbdev.
If we otoh do this here, and some drivers get a in-kernel shadow support for kms, while others don't, then we have a pretty supreme mess. I'd like to avoid that.
So maybe the right solution here is that we make sure that when you have cma helpers set up that mode_config.prefer_shadow is set. Ideally only when coherent dma memory is uncached/write-combine and hence slow for sw rendering. This might need a slight dma-api layering violation. -Daniel
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 58 +++++++++++++++++++++-- drivers/gpu/drm/ingenic/ingenic-drm.h | 4 ++ drivers/gpu/drm/ingenic/ingenic-ipu.c | 12 ++++- 3 files changed, 69 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index b9c156e13156..1ec2ec2faa04 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -9,6 +9,7 @@ #include <linux/component.h> #include <linux/clk.h> #include <linux/dma-mapping.h> +#include <linux/io.h> #include <linux/module.h> #include <linux/mutex.h> #include <linux/of_device.h> @@ -22,6 +23,7 @@ #include <drm/drm_color_mgmt.h> #include <drm/drm_crtc.h> #include <drm/drm_crtc_helper.h> +#include <drm/drm_damage_helper.h> #include <drm/drm_drv.h> #include <drm/drm_gem_cma_helper.h> #include <drm/drm_fb_cma_helper.h> @@ -97,6 +99,11 @@ struct ingenic_drm { struct notifier_block clock_nb; };
+static bool ingenic_drm_cached_gem_buf; +module_param_named(cached_gem_buffers, ingenic_drm_cached_gem_buf, bool, 0400); +MODULE_PARM_DESC(cached_gem_buffers,
"Enable fully cached GEM buffers [default=false]");static bool ingenic_drm_writeable_reg(struct device *dev, unsigned int reg) { switch (reg) { @@ -400,6 +407,8 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane, plane->state->fb->format->format != state->fb->format->format)) crtc_state->mode_changed = true;
- drm_atomic_helper_check_plane_damage(state->state, state);
- return 0;
}
@@ -531,6 +540,14 @@ static void ingenic_drm_update_palette(struct ingenic_drm *priv, } }
+void ingenic_drm_sync_data(struct device *dev,
struct drm_plane_state *old_state,struct drm_plane_state *state)+{
- if (ingenic_drm_cached_gem_buf)
drm_gem_cma_sync_data(dev, old_state, state);+}
static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, struct drm_plane_state *oldstate) { @@ -543,6 +560,8 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, u32 fourcc;
if (state && state->fb) {
ingenic_drm_sync_data(priv->dev, oldstate, state);crtc_state = state->crtc->state;
addr = drm_fb_cma_get_gem_addr(state->fb, state, 0);
@@ -714,7 +733,36 @@ static void ingenic_drm_disable_vblank(struct drm_crtc *crtc) regmap_update_bits(priv->map, JZ_REG_LCD_CTRL, JZ_LCD_CTRL_EOF_IRQ, 0); }
-DEFINE_DRM_GEM_CMA_FOPS(ingenic_drm_fops); +static struct drm_framebuffer * +ingenic_drm_gem_fb_create(struct drm_device *dev, struct drm_file *file,
const struct drm_mode_fb_cmd2 *mode_cmd)+{
- if (ingenic_drm_cached_gem_buf)
return drm_gem_fb_create_with_dirty(dev, file, mode_cmd);- return drm_gem_fb_create(dev, file, mode_cmd);
+}
+static int ingenic_drm_gem_cma_mmap(struct file *filp,
struct vm_area_struct *vma)+{
- if (ingenic_drm_cached_gem_buf)
return drm_gem_cma_mmap_noncoherent(filp, vma);- return drm_gem_cma_mmap(filp, vma);
+}
+static const struct file_operations ingenic_drm_fops = {
- .owner = THIS_MODULE,
- .open = drm_open,
- .release = drm_release,
- .unlocked_ioctl = drm_ioctl,
- .compat_ioctl = drm_compat_ioctl,
- .poll = drm_poll,
- .read = drm_read,
- .llseek = noop_llseek,
- .mmap = ingenic_drm_gem_cma_mmap,
+};
static struct drm_driver ingenic_drm_driver_data = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC, @@ -726,7 +774,7 @@ static struct drm_driver ingenic_drm_driver_data = { .patchlevel = 0,
.fops = &ingenic_drm_fops,
- DRM_GEM_CMA_DRIVER_OPS,
DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE(drm_gem_cma_dumb_create_noncoherent),
.irq_handler = ingenic_drm_irq_handler,
}; @@ -778,7 +826,7 @@ static const struct drm_encoder_helper_funcs ingenic_drm_encoder_helper_funcs = };
static const struct drm_mode_config_funcs ingenic_drm_mode_config_funcs = {
- .fb_create = drm_gem_fb_create,
- .fb_create = ingenic_drm_gem_fb_create, .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = drm_atomic_helper_check, .atomic_commit = drm_atomic_helper_commit,
@@ -932,6 +980,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) return ret; }
drm_plane_enable_fb_damage_clips(&priv->f1);
drm_crtc_helper_add(&priv->crtc, &ingenic_drm_crtc_helper_funcs);
ret = drm_crtc_init_with_planes(drm, &priv->crtc, &priv->f1,
@@ -960,6 +1010,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) return ret; }
drm_plane_enable_fb_damage_clips(&priv->f0);- if (IS_ENABLED(CONFIG_DRM_INGENIC_IPU) && has_components) { ret = component_bind_all(dev, drm); if (ret) {
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h b/drivers/gpu/drm/ingenic/ingenic-drm.h index 9b48ce02803d..ee3a892c0383 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm.h +++ b/drivers/gpu/drm/ingenic/ingenic-drm.h @@ -171,6 +171,10 @@ void ingenic_drm_plane_config(struct device *dev, struct drm_plane *plane, u32 fourcc); void ingenic_drm_plane_disable(struct device *dev, struct drm_plane *plane);
+void ingenic_drm_sync_data(struct device *dev,
struct drm_plane_state *old_state,struct drm_plane_state *state);extern struct platform_driver *ingenic_ipu_driver_ptr;
#endif /* DRIVERS_GPU_DRM_INGENIC_INGENIC_DRM_H */ diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c index fc8c6e970ee3..38c83e8cc6a5 100644 --- a/drivers/gpu/drm/ingenic/ingenic-ipu.c +++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c @@ -20,6 +20,7 @@
#include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> +#include <drm/drm_damage_helper.h> #include <drm/drm_drv.h> #include <drm/drm_fb_cma_helper.h> #include <drm/drm_fourcc.h> @@ -316,6 +317,8 @@ static void ingenic_ipu_plane_atomic_update(struct drm_plane *plane, JZ_IPU_CTRL_CHIP_EN | JZ_IPU_CTRL_LCDC_SEL); }
- ingenic_drm_sync_data(ipu->master, oldstate, state);
- /* New addresses will be committed in vblank handler... */ ipu->addr_y = drm_fb_cma_get_gem_addr(state->fb, state, 0); if (finfo->num_planes > 1)
@@ -534,7 +537,7 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
if (!state->crtc || !crtc_state->mode.hdisplay || !crtc_state->mode.vdisplay)
return 0;
goto out_check_damage;/* Plane must be fully visible */ if (state->crtc_x < 0 || state->crtc_y < 0 ||
@@ -551,7 +554,7 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane, return -EINVAL;
if (!osd_changed(state, plane->state))
return 0;
goto out_check_damage;crtc_state->mode_changed = true;
@@ -578,6 +581,9 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane, ipu->denom_w = denom_w; ipu->denom_h = denom_h;
+out_check_damage:
- drm_atomic_helper_check_plane_damage(state->state, state);
- return 0;
}
@@ -759,6 +765,8 @@ static int ingenic_ipu_bind(struct device *dev, struct device *master, void *d) return err; }
- drm_plane_enable_fb_damage_clips(plane);
- /*
- Sharpness settings range is [0,32]
- 0 : nearest-neighbor
-- 2.28.0
Hi Daniel,
Le mar. 3 nov. 2020 à 11:17, Daniel Vetter daniel@ffwll.ch a écrit :
On Mon, Nov 02, 2020 at 10:06:51PM +0000, Paul Cercueil wrote:
With the module parameter ingenic-drm.cached_gem_buffers, it is possible to specify that we want GEM buffers backed by non-coherent memory.
This dramatically speeds up software rendering on Ingenic SoCs, even for tasks where write-combine memory should in theory be faster (e.g. simple blits).
Leave it disabled by default, since it is specific to one use-case (software rendering).
Signed-off-by: Paul Cercueil paul@crapouillou.net
Hm so maybe I'm making myself supremely unpopular here again with being very late, but we have dev->mode_config.prefer_shadow for this. Drivers should set this if software rendering is slow, and userspace should follow this.
Now unfortunately it looks like most kms drivers don't bother setting this, and we're a lot worse.
So if "slow sw rendering" is the only reason, setting that flag is the right option.
The "prefer_shadow" is based on the assumption that rendering to a shadow buffer, then memcpy that buffer to the write-combine destination buffer, is faster than rendering to a non-coherent destination buffer and sync the data cache. This might be true on some hardware, but is not the case on Ingenic SoCs, where even simple blits (e.g. memcpy) are about three times faster using the second method.
-Paul
Now the other thing is fbdev, since fbdev doesn't have this hint at all. But we already have full generic fbdev shadow support (for defio), so for that I think the best option is adding a force_shadow option to fbdev.
If we otoh do this here, and some drivers get a in-kernel shadow support for kms, while others don't, then we have a pretty supreme mess. I'd like to avoid that.
So maybe the right solution here is that we make sure that when you have cma helpers set up that mode_config.prefer_shadow is set. Ideally only when coherent dma memory is uncached/write-combine and hence slow for sw rendering. This might need a slight dma-api layering violation.
-Daniel
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 58 +++++++++++++++++++++-- drivers/gpu/drm/ingenic/ingenic-drm.h | 4 ++ drivers/gpu/drm/ingenic/ingenic-ipu.c | 12 ++++- 3 files changed, 69 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index b9c156e13156..1ec2ec2faa04 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -9,6 +9,7 @@ #include <linux/component.h> #include <linux/clk.h> #include <linux/dma-mapping.h> +#include <linux/io.h> #include <linux/module.h> #include <linux/mutex.h> #include <linux/of_device.h> @@ -22,6 +23,7 @@ #include <drm/drm_color_mgmt.h> #include <drm/drm_crtc.h> #include <drm/drm_crtc_helper.h> +#include <drm/drm_damage_helper.h> #include <drm/drm_drv.h> #include <drm/drm_gem_cma_helper.h> #include <drm/drm_fb_cma_helper.h> @@ -97,6 +99,11 @@ struct ingenic_drm { struct notifier_block clock_nb; };
+static bool ingenic_drm_cached_gem_buf; +module_param_named(cached_gem_buffers, ingenic_drm_cached_gem_buf, bool, 0400); +MODULE_PARM_DESC(cached_gem_buffers,
"Enable fully cached GEM buffers [default=false]");static bool ingenic_drm_writeable_reg(struct device *dev, unsigned int reg) { switch (reg) { @@ -400,6 +407,8 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane, plane->state->fb->format->format != state->fb->format->format)) crtc_state->mode_changed = true;
- drm_atomic_helper_check_plane_damage(state->state, state);
- return 0;
}
@@ -531,6 +540,14 @@ static void ingenic_drm_update_palette(struct ingenic_drm *priv, } }
+void ingenic_drm_sync_data(struct device *dev,
struct drm_plane_state *old_state,struct drm_plane_state *state)+{
- if (ingenic_drm_cached_gem_buf)
drm_gem_cma_sync_data(dev, old_state, state);+}
static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, struct drm_plane_state *oldstate) { @@ -543,6 +560,8 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, u32 fourcc;
if (state && state->fb) {
ingenic_drm_sync_data(priv->dev, oldstate, state);crtc_state = state->crtc->state;
addr = drm_fb_cma_get_gem_addr(state->fb, state, 0);
@@ -714,7 +733,36 @@ static void ingenic_drm_disable_vblank(struct drm_crtc *crtc) regmap_update_bits(priv->map, JZ_REG_LCD_CTRL, JZ_LCD_CTRL_EOF_IRQ, 0); }
-DEFINE_DRM_GEM_CMA_FOPS(ingenic_drm_fops); +static struct drm_framebuffer * +ingenic_drm_gem_fb_create(struct drm_device *dev, struct drm_file *file,
const struct drm_mode_fb_cmd2 *mode_cmd)+{
- if (ingenic_drm_cached_gem_buf)
return drm_gem_fb_create_with_dirty(dev, file, mode_cmd);- return drm_gem_fb_create(dev, file, mode_cmd);
+}
+static int ingenic_drm_gem_cma_mmap(struct file *filp,
struct vm_area_struct *vma)+{
- if (ingenic_drm_cached_gem_buf)
return drm_gem_cma_mmap_noncoherent(filp, vma);- return drm_gem_cma_mmap(filp, vma);
+}
+static const struct file_operations ingenic_drm_fops = {
- .owner = THIS_MODULE,
- .open = drm_open,
- .release = drm_release,
- .unlocked_ioctl = drm_ioctl,
- .compat_ioctl = drm_compat_ioctl,
- .poll = drm_poll,
- .read = drm_read,
- .llseek = noop_llseek,
- .mmap = ingenic_drm_gem_cma_mmap,
+};
static struct drm_driver ingenic_drm_driver_data = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC, @@ -726,7 +774,7 @@ static struct drm_driver ingenic_drm_driver_data = { .patchlevel = 0,
.fops = &ingenic_drm_fops,
- DRM_GEM_CMA_DRIVER_OPS,
DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE(drm_gem_cma_dumb_create_noncoherent),
.irq_handler = ingenic_drm_irq_handler, };
@@ -778,7 +826,7 @@ static const struct drm_encoder_helper_funcs ingenic_drm_encoder_helper_funcs = };
static const struct drm_mode_config_funcs ingenic_drm_mode_config_funcs = {
- .fb_create = drm_gem_fb_create,
- .fb_create = ingenic_drm_gem_fb_create, .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = drm_atomic_helper_check, .atomic_commit = drm_atomic_helper_commit,
@@ -932,6 +980,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) return ret; }
drm_plane_enable_fb_damage_clips(&priv->f1);
drm_crtc_helper_add(&priv->crtc, &ingenic_drm_crtc_helper_funcs);
ret = drm_crtc_init_with_planes(drm, &priv->crtc, &priv->f1,
@@ -960,6 +1010,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) return ret; }
drm_plane_enable_fb_damage_clips(&priv->f0);- if (IS_ENABLED(CONFIG_DRM_INGENIC_IPU) && has_components) { ret = component_bind_all(dev, drm); if (ret) {
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h b/drivers/gpu/drm/ingenic/ingenic-drm.h index 9b48ce02803d..ee3a892c0383 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm.h +++ b/drivers/gpu/drm/ingenic/ingenic-drm.h @@ -171,6 +171,10 @@ void ingenic_drm_plane_config(struct device *dev, struct drm_plane *plane, u32 fourcc); void ingenic_drm_plane_disable(struct device *dev, struct drm_plane *plane);
+void ingenic_drm_sync_data(struct device *dev,
struct drm_plane_state *old_state,struct drm_plane_state *state);extern struct platform_driver *ingenic_ipu_driver_ptr;
#endif /* DRIVERS_GPU_DRM_INGENIC_INGENIC_DRM_H */ diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c index fc8c6e970ee3..38c83e8cc6a5 100644 --- a/drivers/gpu/drm/ingenic/ingenic-ipu.c +++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c @@ -20,6 +20,7 @@
#include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> +#include <drm/drm_damage_helper.h> #include <drm/drm_drv.h> #include <drm/drm_fb_cma_helper.h> #include <drm/drm_fourcc.h> @@ -316,6 +317,8 @@ static void ingenic_ipu_plane_atomic_update(struct drm_plane *plane, JZ_IPU_CTRL_CHIP_EN | JZ_IPU_CTRL_LCDC_SEL); }
- ingenic_drm_sync_data(ipu->master, oldstate, state);
- /* New addresses will be committed in vblank handler... */ ipu->addr_y = drm_fb_cma_get_gem_addr(state->fb, state, 0); if (finfo->num_planes > 1)
@@ -534,7 +537,7 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
if (!state->crtc || !crtc_state->mode.hdisplay || !crtc_state->mode.vdisplay)
return 0;
goto out_check_damage;/* Plane must be fully visible */ if (state->crtc_x < 0 || state->crtc_y < 0 ||
@@ -551,7 +554,7 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane, return -EINVAL;
if (!osd_changed(state, plane->state))
return 0;
goto out_check_damage;crtc_state->mode_changed = true;
@@ -578,6 +581,9 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane, ipu->denom_w = denom_w; ipu->denom_h = denom_h;
+out_check_damage:
- drm_atomic_helper_check_plane_damage(state->state, state);
- return 0;
}
@@ -759,6 +765,8 @@ static int ingenic_ipu_bind(struct device *dev, struct device *master, void *d) return err; }
- drm_plane_enable_fb_damage_clips(plane);
- /*
- Sharpness settings range is [0,32]
- 0 : nearest-neighbor
-- 2.28.0
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
On Tue, Nov 3, 2020 at 12:57 PM Paul Cercueil paul@crapouillou.net wrote:
Hi Daniel,
Le mar. 3 nov. 2020 à 11:17, Daniel Vetter daniel@ffwll.ch a écrit :
On Mon, Nov 02, 2020 at 10:06:51PM +0000, Paul Cercueil wrote:
With the module parameter ingenic-drm.cached_gem_buffers, it is possible to specify that we want GEM buffers backed by non-coherent memory.
This dramatically speeds up software rendering on Ingenic SoCs, even for tasks where write-combine memory should in theory be faster (e.g. simple blits).
Leave it disabled by default, since it is specific to one use-case (software rendering).
Signed-off-by: Paul Cercueil paul@crapouillou.net
Hm so maybe I'm making myself supremely unpopular here again with being very late, but we have dev->mode_config.prefer_shadow for this. Drivers should set this if software rendering is slow, and userspace should follow this.
Now unfortunately it looks like most kms drivers don't bother setting this, and we're a lot worse.
So if "slow sw rendering" is the only reason, setting that flag is the right option.
The "prefer_shadow" is based on the assumption that rendering to a shadow buffer, then memcpy that buffer to the write-combine destination buffer, is faster than rendering to a non-coherent destination buffer and sync the data cache. This might be true on some hardware, but is not the case on Ingenic SoCs, where even simple blits (e.g. memcpy) are about three times faster using the second method.
Ah right, vague memories coming back, pretty sure I asked this before. Please include this in your commit message (and ideally also in some code comment somewhere), since otherwise I'll keep asking the same question every single time you submit this or I stumble over it again :-)
Cheers, Daniel
-Paul
Now the other thing is fbdev, since fbdev doesn't have this hint at all. But we already have full generic fbdev shadow support (for defio), so for that I think the best option is adding a force_shadow option to fbdev.
If we otoh do this here, and some drivers get a in-kernel shadow support for kms, while others don't, then we have a pretty supreme mess. I'd like to avoid that.
So maybe the right solution here is that we make sure that when you have cma helpers set up that mode_config.prefer_shadow is set. Ideally only when coherent dma memory is uncached/write-combine and hence slow for sw rendering. This might need a slight dma-api layering violation.
-Daniel
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 58 +++++++++++++++++++++-- drivers/gpu/drm/ingenic/ingenic-drm.h | 4 ++ drivers/gpu/drm/ingenic/ingenic-ipu.c | 12 ++++- 3 files changed, 69 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index b9c156e13156..1ec2ec2faa04 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -9,6 +9,7 @@ #include <linux/component.h> #include <linux/clk.h> #include <linux/dma-mapping.h> +#include <linux/io.h> #include <linux/module.h> #include <linux/mutex.h> #include <linux/of_device.h> @@ -22,6 +23,7 @@ #include <drm/drm_color_mgmt.h> #include <drm/drm_crtc.h> #include <drm/drm_crtc_helper.h> +#include <drm/drm_damage_helper.h> #include <drm/drm_drv.h> #include <drm/drm_gem_cma_helper.h> #include <drm/drm_fb_cma_helper.h> @@ -97,6 +99,11 @@ struct ingenic_drm { struct notifier_block clock_nb; };
+static bool ingenic_drm_cached_gem_buf; +module_param_named(cached_gem_buffers, ingenic_drm_cached_gem_buf, bool, 0400); +MODULE_PARM_DESC(cached_gem_buffers,
"Enable fully cached GEM buffers [default=false]");static bool ingenic_drm_writeable_reg(struct device *dev, unsigned int reg) { switch (reg) { @@ -400,6 +407,8 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane, plane->state->fb->format->format != state->fb->format->format)) crtc_state->mode_changed = true;
- drm_atomic_helper_check_plane_damage(state->state, state);
- return 0;
}
@@ -531,6 +540,14 @@ static void ingenic_drm_update_palette(struct ingenic_drm *priv, } }
+void ingenic_drm_sync_data(struct device *dev,
struct drm_plane_state *old_state,struct drm_plane_state *state)+{
- if (ingenic_drm_cached_gem_buf)
drm_gem_cma_sync_data(dev, old_state, state);+}
static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, struct drm_plane_state *oldstate) { @@ -543,6 +560,8 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, u32 fourcc;
if (state && state->fb) {
ingenic_drm_sync_data(priv->dev, oldstate, state);crtc_state = state->crtc->state; addr = drm_fb_cma_get_gem_addr(state->fb, state, 0);@@ -714,7 +733,36 @@ static void ingenic_drm_disable_vblank(struct drm_crtc *crtc) regmap_update_bits(priv->map, JZ_REG_LCD_CTRL, JZ_LCD_CTRL_EOF_IRQ, 0); }
-DEFINE_DRM_GEM_CMA_FOPS(ingenic_drm_fops); +static struct drm_framebuffer * +ingenic_drm_gem_fb_create(struct drm_device *dev, struct drm_file *file,
const struct drm_mode_fb_cmd2 *mode_cmd)+{
- if (ingenic_drm_cached_gem_buf)
return drm_gem_fb_create_with_dirty(dev, file, mode_cmd);- return drm_gem_fb_create(dev, file, mode_cmd);
+}
+static int ingenic_drm_gem_cma_mmap(struct file *filp,
struct vm_area_struct *vma)+{
- if (ingenic_drm_cached_gem_buf)
return drm_gem_cma_mmap_noncoherent(filp, vma);- return drm_gem_cma_mmap(filp, vma);
+}
+static const struct file_operations ingenic_drm_fops = {
- .owner = THIS_MODULE,
- .open = drm_open,
- .release = drm_release,
- .unlocked_ioctl = drm_ioctl,
- .compat_ioctl = drm_compat_ioctl,
- .poll = drm_poll,
- .read = drm_read,
- .llseek = noop_llseek,
- .mmap = ingenic_drm_gem_cma_mmap,
+};
static struct drm_driver ingenic_drm_driver_data = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC, @@ -726,7 +774,7 @@ static struct drm_driver ingenic_drm_driver_data = { .patchlevel = 0,
.fops = &ingenic_drm_fops,
- DRM_GEM_CMA_DRIVER_OPS,
DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE(drm_gem_cma_dumb_create_noncoherent),
.irq_handler = ingenic_drm_irq_handler, };
@@ -778,7 +826,7 @@ static const struct drm_encoder_helper_funcs ingenic_drm_encoder_helper_funcs = };
static const struct drm_mode_config_funcs ingenic_drm_mode_config_funcs = {
- .fb_create = drm_gem_fb_create,
- .fb_create = ingenic_drm_gem_fb_create, .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = drm_atomic_helper_check, .atomic_commit = drm_atomic_helper_commit,
@@ -932,6 +980,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) return ret; }
drm_plane_enable_fb_damage_clips(&priv->f1);
drm_crtc_helper_add(&priv->crtc, &ingenic_drm_crtc_helper_funcs);
ret = drm_crtc_init_with_planes(drm, &priv->crtc, &priv->f1,
@@ -960,6 +1010,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) return ret; }
drm_plane_enable_fb_damage_clips(&priv->f0);if (IS_ENABLED(CONFIG_DRM_INGENIC_IPU) && has_components) { ret = component_bind_all(dev, drm); if (ret) {diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h b/drivers/gpu/drm/ingenic/ingenic-drm.h index 9b48ce02803d..ee3a892c0383 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm.h +++ b/drivers/gpu/drm/ingenic/ingenic-drm.h @@ -171,6 +171,10 @@ void ingenic_drm_plane_config(struct device *dev, struct drm_plane *plane, u32 fourcc); void ingenic_drm_plane_disable(struct device *dev, struct drm_plane *plane);
+void ingenic_drm_sync_data(struct device *dev,
struct drm_plane_state *old_state,struct drm_plane_state *state);extern struct platform_driver *ingenic_ipu_driver_ptr;
#endif /* DRIVERS_GPU_DRM_INGENIC_INGENIC_DRM_H */ diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c index fc8c6e970ee3..38c83e8cc6a5 100644 --- a/drivers/gpu/drm/ingenic/ingenic-ipu.c +++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c @@ -20,6 +20,7 @@
#include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> +#include <drm/drm_damage_helper.h> #include <drm/drm_drv.h> #include <drm/drm_fb_cma_helper.h> #include <drm/drm_fourcc.h> @@ -316,6 +317,8 @@ static void ingenic_ipu_plane_atomic_update(struct drm_plane *plane, JZ_IPU_CTRL_CHIP_EN | JZ_IPU_CTRL_LCDC_SEL); }
- ingenic_drm_sync_data(ipu->master, oldstate, state);
- /* New addresses will be committed in vblank handler... */ ipu->addr_y = drm_fb_cma_get_gem_addr(state->fb, state, 0); if (finfo->num_planes > 1)
@@ -534,7 +537,7 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
if (!state->crtc || !crtc_state->mode.hdisplay || !crtc_state->mode.vdisplay)
return 0;
goto out_check_damage;/* Plane must be fully visible */ if (state->crtc_x < 0 || state->crtc_y < 0 ||
@@ -551,7 +554,7 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane, return -EINVAL;
if (!osd_changed(state, plane->state))
return 0;
goto out_check_damage;crtc_state->mode_changed = true;
@@ -578,6 +581,9 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane, ipu->denom_w = denom_w; ipu->denom_h = denom_h;
+out_check_damage:
- drm_atomic_helper_check_plane_damage(state->state, state);
- return 0;
}
@@ -759,6 +765,8 @@ static int ingenic_ipu_bind(struct device *dev, struct device *master, void *d) return err; }
- drm_plane_enable_fb_damage_clips(plane);
- /*
- Sharpness settings range is [0,32]
- 0 : nearest-neighbor
-- 2.28.0
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Paul Cercueil