Re: [PATCH] Revert "gpu/drm: ingenic: Add option to mmap GEM buffers cached"
Hi all, On Mon, 5 Oct 2020 23:01:50 +1100 Stephen Rothwell wrote: > > On Sun, 04 Oct 2020 22:11:23 +0200 Paul Cercueil wrote: > > > > Pushed to drm-misc-next with the changelog fix, thanks. > > > > Stephen: > > Now it should build fine again. Could you remove the BROKEN flag? > > Thanks for letting me know, but the fix has not appeared in any drm > tree included in linux-next yet ... > > If it doesn't show up by the time I will merge the drm tree tomorrow, I > will apply this revert patch myself (instead of the patch marking the > driver BROKEN). Unfortunately, the revert patch does not apply to the drm tree merge, so I have just marked the driver BROKEN again today. -- Cheers, Stephen Rothwell pgpm2BhugBYw4.pgp Description: OpenPGP digital signature
Re: [PATCH] Revert "gpu/drm: ingenic: Add option to mmap GEM buffers cached"
On Mon, Oct 5, 2020 at 4:47 PM Paul Cercueil wrote: > > Hi, > > Le lun. 5 oct. 2020 à 16:05, Daniel Vetter a écrit : > > On Mon, Oct 05, 2020 at 11:01:50PM +1100, Stephen Rothwell wrote: > >> Hi Paul, > >> > >> On Sun, 04 Oct 2020 22:11:23 +0200 Paul Cercueil > >> wrote: > >> > > >> > Pushed to drm-misc-next with the changelog fix, thanks. > >> > > >> > Stephen: > >> > Now it should build fine again. Could you remove the BROKEN flag? > >> > >> Thanks for letting me know, but the fix has not appeared in any drm > >> tree included in linux-next yet ... > >> > >> If it doesn't show up by the time I will merge the drm tree > >> tomorrow, I > >> will apply this revert patch myself (instead of the patch marking > >> the > >> driver BROKEN). > > > > Yeah it should have been pushed to drm-misc-next-fixes per > > > > https://drm.pages.freedesktop.org/maintainer-tools/committer-drm-misc.html#where-do-i-apply-my-patch > > > > Paul, can you pls git cherry-pick -x this over to drm-misc-next-fixes? > > I had a few commits on top of it in drm-misc-next, so the revert > doesn't apply cleanly in drm-misc-next-fixes... I can revert it there, > but then we'd have a different revert commit in drm-misc-next and > drm-misc-next-next. > > Sorry for the mess. What should I do? Hm not sure why, but the reply I thought I've typed didn't seem to have gone out. Cherry pick up, fix up conflict and then fix up the conflict when rebuilding drm-tip. Please tell drm-misc maintainers, they probably want to do a backmerge once the drm-next merge window pull is merged in Linus tree. If we don't fix this up then the drm-next pull goes nowhere. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH] Revert "gpu/drm: ingenic: Add option to mmap GEM buffers cached"
On Mon, Oct 5, 2020 at 4:47 PM Paul Cercueil wrote: > > Hi, > > Le lun. 5 oct. 2020 à 16:05, Daniel Vetter a écrit : > > On Mon, Oct 05, 2020 at 11:01:50PM +1100, Stephen Rothwell wrote: > >> Hi Paul, > >> > >> On Sun, 04 Oct 2020 22:11:23 +0200 Paul Cercueil > >> wrote: > >> > > >> > Pushed to drm-misc-next with the changelog fix, thanks. > >> > > >> > Stephen: > >> > Now it should build fine again. Could you remove the BROKEN flag? > >> > >> Thanks for letting me know, but the fix has not appeared in any drm > >> tree included in linux-next yet ... > >> > >> If it doesn't show up by the time I will merge the drm tree > >> tomorrow, I > >> will apply this revert patch myself (instead of the patch marking > >> the > >> driver BROKEN). > > > > Yeah it should have been pushed to drm-misc-next-fixes per > > > > https://drm.pages.freedesktop.org/maintainer-tools/committer-drm-misc.html#where-do-i-apply-my-patch > > > > Paul, can you pls git cherry-pick -x this over to drm-misc-next-fixes? > > I had a few commits on top of it in drm-misc-next, so the revert > doesn't apply cleanly in drm-misc-next-fixes... I can revert it there, > but then we'd have a different revert commit in drm-misc-next and > drm-misc-next-next. > > Sorry for the mess. What should I do? We need the revert in drm-misc-next-fixes or the drm-next pull request doesn't work out. So cherry-pick over, fix up conflicts, push the tree, and don't forget to fix up the conflicts when dim rebuilds drm-tip. Also tell drm-misc maintainers what you've done, they probably want to do a backmerge to clean this up a bit once the drm-next pull request has landed. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH] Revert "gpu/drm: ingenic: Add option to mmap GEM buffers cached"
Hi, Le lun. 5 oct. 2020 à 16:05, Daniel Vetter a écrit : On Mon, Oct 05, 2020 at 11:01:50PM +1100, Stephen Rothwell wrote: Hi Paul, On Sun, 04 Oct 2020 22:11:23 +0200 Paul Cercueil wrote: > > Pushed to drm-misc-next with the changelog fix, thanks. > > Stephen: > Now it should build fine again. Could you remove the BROKEN flag? Thanks for letting me know, but the fix has not appeared in any drm tree included in linux-next yet ... If it doesn't show up by the time I will merge the drm tree tomorrow, I will apply this revert patch myself (instead of the patch marking the driver BROKEN). Yeah it should have been pushed to drm-misc-next-fixes per https://drm.pages.freedesktop.org/maintainer-tools/committer-drm-misc.html#where-do-i-apply-my-patch Paul, can you pls git cherry-pick -x this over to drm-misc-next-fixes? I had a few commits on top of it in drm-misc-next, so the revert doesn't apply cleanly in drm-misc-next-fixes... I can revert it there, but then we'd have a different revert commit in drm-misc-next and drm-misc-next-next. Sorry for the mess. What should I do? -Paul
Re: [PATCH] Revert "gpu/drm: ingenic: Add option to mmap GEM buffers cached"
On Mon, Oct 05, 2020 at 11:01:50PM +1100, Stephen Rothwell wrote: > Hi Paul, > > On Sun, 04 Oct 2020 22:11:23 +0200 Paul Cercueil wrote: > > > > Pushed to drm-misc-next with the changelog fix, thanks. > > > > Stephen: > > Now it should build fine again. Could you remove the BROKEN flag? > > Thanks for letting me know, but the fix has not appeared in any drm > tree included in linux-next yet ... > > If it doesn't show up by the time I will merge the drm tree tomorrow, I > will apply this revert patch myself (instead of the patch marking the > driver BROKEN). Yeah it should have been pushed to drm-misc-next-fixes per https://drm.pages.freedesktop.org/maintainer-tools/committer-drm-misc.html#where-do-i-apply-my-patch Paul, can you pls git cherry-pick -x this over to drm-misc-next-fixes? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH] Revert "gpu/drm: ingenic: Add option to mmap GEM buffers cached"
Hi Paul, On Sun, 04 Oct 2020 22:11:23 +0200 Paul Cercueil wrote: > > Pushed to drm-misc-next with the changelog fix, thanks. > > Stephen: > Now it should build fine again. Could you remove the BROKEN flag? Thanks for letting me know, but the fix has not appeared in any drm tree included in linux-next yet ... If it doesn't show up by the time I will merge the drm tree tomorrow, I will apply this revert patch myself (instead of the patch marking the driver BROKEN). -- Cheers, Stephen Rothwell pgpPmFVaY98cA.pgp Description: OpenPGP digital signature
Re: [PATCH] Revert "gpu/drm: ingenic: Add option to mmap GEM buffers cached"
Hi,
Le dim. 4 oct. 2020 à 21:59, Sam Ravnborg 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
With the changelog updated:
Acked-by: Sam Ravnborg
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
#include
#include
-#include
-#include
#include
#include
#include
@@ -24,7 +22,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -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(, old_state, state);
-
- drm_atomic_for_each_plane_damage(, ) {
- 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;
Re: [PATCH] Revert "gpu/drm: ingenic: Add option to mmap GEM buffers cached"
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
With the changelog updated:
Acked-by: Sam Ravnborg
> ---
> 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
> #include
> #include
> -#include
> -#include
> #include
> #include
> #include
> @@ -24,7 +22,6 @@
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> @@ -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(, old_state, state);
> -
> - drm_atomic_for_each_plane_damage(, ) {
> - 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
> -
[PATCH] Revert "gpu/drm: ingenic: Add option to mmap GEM buffers cached"
This reverts commit 37054fc81443cc6a8c3a38395f384412b8373d82.
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
---
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
#include
#include
-#include
-#include
#include
#include
#include
@@ -24,7 +22,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -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(, old_state, state);
-
- drm_atomic_for_each_plane_damage(, ) {
- 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,
