| Message ID | 20210108094340.15290-7-tzimmermann@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | drm: Support short-term vmap via vmap_local | expand |
On Fri, Jan 08, 2021 at 10:43:33AM +0100, Thomas Zimmermann wrote: > Damage handling in cirrus requires a short-term mapping of the source > BO. Use drm_gem_shmem_vmap_local(). > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Hm more possible errors that we don't report to userspace ... Why don't we vmap/vunmap these in prepare/cleanup_fb? Generally we'd want a long-term vmap here to make sure this all works nicely. Since it's nothing new, on this patch: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/tiny/cirrus.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c > index a043e602199e..21cd7056d45f 100644 > --- a/drivers/gpu/drm/tiny/cirrus.c > +++ b/drivers/gpu/drm/tiny/cirrus.c > @@ -315,6 +315,7 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, > struct drm_rect *rect) > { > struct cirrus_device *cirrus = to_cirrus(fb->dev); > + struct drm_gem_object *obj = fb->obj[0]; > struct dma_buf_map map; > void *vmap; > int idx, ret; > @@ -323,9 +324,12 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, > if (!drm_dev_enter(&cirrus->dev, &idx)) > goto out; > > - ret = drm_gem_shmem_vmap(fb->obj[0], &map); > + ret = dma_resv_lock(obj->resv, NULL); > if (ret) > goto out_dev_exit; > + ret = drm_gem_shmem_vmap_local(fb->obj[0], &map); > + if (ret) > + goto out_dma_resv_unlock; > vmap = map.vaddr; /* TODO: Use mapping abstraction properly */ > > if (cirrus->cpp == fb->format->cpp[0]) > @@ -345,9 +349,11 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, > else > WARN_ON_ONCE("cpp mismatch"); > > - drm_gem_shmem_vunmap(fb->obj[0], &map); > ret = 0; > > + drm_gem_shmem_vunmap_local(obj, &map); > +out_dma_resv_unlock: > + dma_resv_unlock(obj->resv); > out_dev_exit: > drm_dev_exit(idx); > out: > -- > 2.29.2 >
On Mon, Jan 11, 2021 at 06:00:42PM +0100, Daniel Vetter wrote: > On Fri, Jan 08, 2021 at 10:43:33AM +0100, Thomas Zimmermann wrote: > > Damage handling in cirrus requires a short-term mapping of the source > > BO. Use drm_gem_shmem_vmap_local(). > > > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > > Hm more possible errors that we don't report to userspace ... Why don't we > vmap/vunmap these in prepare/cleanup_fb? Generally we'd want a long-term > vmap here to make sure this all works nicely. > > Since it's nothing new, on this patch: > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Ok, also strike this r-b here. This is called from that atomic commit paths, and we cannot call dma_resv_lock here. This should splat with lockdep enabled against the dma-fence annotations I've merged, I'm kinda surprised it doesn't? -Daniel > > > --- > > drivers/gpu/drm/tiny/cirrus.c | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c > > index a043e602199e..21cd7056d45f 100644 > > --- a/drivers/gpu/drm/tiny/cirrus.c > > +++ b/drivers/gpu/drm/tiny/cirrus.c > > @@ -315,6 +315,7 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, > > struct drm_rect *rect) > > { > > struct cirrus_device *cirrus = to_cirrus(fb->dev); > > + struct drm_gem_object *obj = fb->obj[0]; > > struct dma_buf_map map; > > void *vmap; > > int idx, ret; > > @@ -323,9 +324,12 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, > > if (!drm_dev_enter(&cirrus->dev, &idx)) > > goto out; > > > > - ret = drm_gem_shmem_vmap(fb->obj[0], &map); > > + ret = dma_resv_lock(obj->resv, NULL); > > if (ret) > > goto out_dev_exit; > > + ret = drm_gem_shmem_vmap_local(fb->obj[0], &map); > > + if (ret) > > + goto out_dma_resv_unlock; > > vmap = map.vaddr; /* TODO: Use mapping abstraction properly */ > > > > if (cirrus->cpp == fb->format->cpp[0]) > > @@ -345,9 +349,11 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, > > else > > WARN_ON_ONCE("cpp mismatch"); > > > > - drm_gem_shmem_vunmap(fb->obj[0], &map); > > ret = 0; > > > > + drm_gem_shmem_vunmap_local(obj, &map); > > +out_dma_resv_unlock: > > + dma_resv_unlock(obj->resv); > > out_dev_exit: > > drm_dev_exit(idx); > > out: > > -- > > 2.29.2 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c index a043e602199e..21cd7056d45f 100644 --- a/drivers/gpu/drm/tiny/cirrus.c +++ b/drivers/gpu/drm/tiny/cirrus.c @@ -315,6 +315,7 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, struct drm_rect *rect) { struct cirrus_device *cirrus = to_cirrus(fb->dev); + struct drm_gem_object *obj = fb->obj[0]; struct dma_buf_map map; void *vmap; int idx, ret; @@ -323,9 +324,12 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, if (!drm_dev_enter(&cirrus->dev, &idx)) goto out; - ret = drm_gem_shmem_vmap(fb->obj[0], &map); + ret = dma_resv_lock(obj->resv, NULL); if (ret) goto out_dev_exit; + ret = drm_gem_shmem_vmap_local(fb->obj[0], &map); + if (ret) + goto out_dma_resv_unlock; vmap = map.vaddr; /* TODO: Use mapping abstraction properly */ if (cirrus->cpp == fb->format->cpp[0]) @@ -345,9 +349,11 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, else WARN_ON_ONCE("cpp mismatch"); - drm_gem_shmem_vunmap(fb->obj[0], &map); ret = 0; + drm_gem_shmem_vunmap_local(obj, &map); +out_dma_resv_unlock: + dma_resv_unlock(obj->resv); out_dev_exit: drm_dev_exit(idx); out:
Damage handling in cirrus requires a short-term mapping of the source BO. Use drm_gem_shmem_vmap_local(). Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/tiny/cirrus.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)