[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <ZE1PHQ.WGCBAFO9R38I3@crapouillou.net>
Date: Sun, 04 Oct 2020 22:11:23 +0200
From: Paul Cercueil <paul@...pouillou.net>
To: Sam Ravnborg <sam@...nborg.org>,
Stephen Rothwell <sfr@...b.auug.org.au>
Cc: Christoph Hellwig <hch@....de>, Dave Airlie <airlied@...ux.ie>,
od@...c.me, DRI <dri-devel@...ts.freedesktop.org>,
Linux Next Mailing List <linux-next@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Revert "gpu/drm: ingenic: Add option to mmap GEM
buffers
cached"
Hi,
Le dim. 4 oct. 2020 à 21:59, Sam Ravnborg <sam@...nborg.org> a écrit :
> Hi Paul.
>
> On Sun, Oct 04, 2020 at 04:17:58PM +0200, Paul Cercueil wrote:
>> This reverts commit 37054fc81443cc6a8c3a38395f384412b8373d82.
>
> In the changelog please refer to commits like this:
> 37054fc81443 ("gpu/drm: ingenic: Add option to mmap GEM buffers
> cached")
>
> Use "dim cite 37054fc81443cc6a8c3a38395f384412b8373d82" to get the
> right format.
>
>>
>> At the very moment this commit was created, the DMA API it relied
>> on was
>> modified in the DMA tree, which caused the driver to break in
>> linux-next.
>>
>> Revert it for now, and it will be resubmitted later to work with
>> the new
>> DMA API.
>>
>> Signed-off-by: Paul Cercueil <paul@...pouillou.net>
>
> With the changelog updated:
> Acked-by: Sam Ravnborg <sam@...nborg.org>
Pushed to drm-misc-next with the changelog fix, thanks.
Stephen:
Now it should build fine again. Could you remove the BROKEN flag?
Cheers,
-Paul
>> ---
>> drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 114
>> +---------------------
>> drivers/gpu/drm/ingenic/ingenic-drm.h | 4 -
>> drivers/gpu/drm/ingenic/ingenic-ipu.c | 12 +--
>> 3 files changed, 4 insertions(+), 126 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>> index 0225dc1f5eb8..7d8b0ad52979 100644
>> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>> @@ -9,8 +9,6 @@
>> #include <linux/component.h>
>> #include <linux/clk.h>
>> #include <linux/dma-mapping.h>
>> -#include <linux/dma-noncoherent.h>
>> -#include <linux/io.h>
>> #include <linux/module.h>
>> #include <linux/mutex.h>
>> #include <linux/of_device.h>
>> @@ -24,7 +22,6 @@
>> #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>
>> @@ -100,11 +97,6 @@ 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) {
>> @@ -402,8 +394,6 @@ 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;
>> }
>>
>> @@ -521,38 +511,6 @@ void ingenic_drm_plane_config(struct device
>> *dev,
>> }
>> }
>>
>> -void ingenic_drm_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 ingenic_drm *priv = dev_get_drvdata(dev);
>> - struct drm_atomic_helper_damage_iter iter;
>> - unsigned int offset, i;
>> - struct drm_rect clip;
>> - dma_addr_t paddr;
>> - void *addr;
>> -
>> - if (!ingenic_drm_cached_gem_buf)
>> - 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++) {
>> - paddr = drm_fb_cma_get_gem_addr(state->fb, state, i);
>> - addr = phys_to_virt(paddr);
>> -
>> - /* Ignore x1/x2 values, invalidate complete lines */
>> - offset = clip.y1 * state->fb->pitches[i];
>> -
>> - dma_cache_sync(priv->dev, addr + offset,
>> - (clip.y2 - clip.y1) * state->fb->pitches[i],
>> - DMA_TO_DEVICE);
>> - }
>> - }
>> -}
>> -
>> static void ingenic_drm_update_palette(struct ingenic_drm *priv,
>> const struct drm_color_lut *lut)
>> {
>> @@ -581,8 +539,6 @@ static void
>> ingenic_drm_plane_atomic_update(struct drm_plane *plane,
>> if (state && state->fb) {
>> crtc_state = state->crtc->state;
>>
>> - ingenic_drm_sync_data(priv->dev, oldstate, state);
>> -
>> addr = drm_fb_cma_get_gem_addr(state->fb, state, 0);
>> width = state->src_w >> 16;
>> height = state->src_h >> 16;
>> @@ -752,69 +708,7 @@ 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);
>> }
>>
>> -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_mmap(struct drm_gem_object *obj,
>> - struct vm_area_struct *vma)
>> -{
>> - struct drm_gem_cma_object *cma_obj = to_drm_gem_cma_obj(obj);
>> - struct device *dev = cma_obj->base.dev->dev;
>> - unsigned long attrs;
>> - int ret;
>> -
>> - if (ingenic_drm_cached_gem_buf)
>> - attrs = DMA_ATTR_NON_CONSISTENT;
>> - else
>> - attrs = DMA_ATTR_WRITE_COMBINE;
>> -
>> - /*
>> - * 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);
>> -
>> - ret = dma_mmap_attrs(dev, vma, cma_obj->vaddr, cma_obj->paddr,
>> - vma->vm_end - vma->vm_start, attrs);
>> - if (ret)
>> - drm_gem_vm_close(vma);
>> -
>> - return ret;
>> -}
>> -
>> -static int ingenic_drm_gem_cma_mmap(struct file *filp,
>> - struct vm_area_struct *vma)
>> -{
>> - int ret;
>> -
>> - ret = drm_gem_mmap(filp, vma);
>> - if (ret)
>> - return ret;
>> -
>> - return ingenic_drm_gem_mmap(vma->vm_private_data, 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,
>> -};
>> +DEFINE_DRM_GEM_CMA_FOPS(ingenic_drm_fops);
>>
>> static struct drm_driver ingenic_drm_driver_data = {
>> .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
>> @@ -878,7 +772,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 = ingenic_drm_gem_fb_create,
>> + .fb_create = 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,
>> @@ -1032,8 +926,6 @@ 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,
>> @@ -1062,8 +954,6 @@ 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 ee3a892c0383..9b48ce02803d 100644
>> --- a/drivers/gpu/drm/ingenic/ingenic-drm.h
>> +++ b/drivers/gpu/drm/ingenic/ingenic-drm.h
>> @@ -171,10 +171,6 @@ 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 38c83e8cc6a5..fc8c6e970ee3 100644
>> --- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
>> +++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
>> @@ -20,7 +20,6 @@
>>
>> #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>
>> @@ -317,8 +316,6 @@ 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)
>> @@ -537,7 +534,7 @@ static int
>> ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
>>
>> if (!state->crtc ||
>> !crtc_state->mode.hdisplay || !crtc_state->mode.vdisplay)
>> - goto out_check_damage;
>> + return 0;
>>
>> /* Plane must be fully visible */
>> if (state->crtc_x < 0 || state->crtc_y < 0 ||
>> @@ -554,7 +551,7 @@ static int
>> ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
>> return -EINVAL;
>>
>> if (!osd_changed(state, plane->state))
>> - goto out_check_damage;
>> + return 0;
>>
>> crtc_state->mode_changed = true;
>>
>> @@ -581,9 +578,6 @@ 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;
>> }
>>
>> @@ -765,8 +759,6 @@ 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
Powered by blists - more mailing lists