[PATCH v4 05/13] drm/mgag200: Use drm_gem_shmem_vmap_local() in damage handling

[Daniel Vetter]

On Mon, Jan 11, 2021 at 05:53:41PM +0100, Daniel Vetter wrote:
> On Fri, Jan 08, 2021 at 10:43:32AM +0100, Thomas Zimmermann wrote:
> > Damage handling in mgag200 requires a short-term mapping of the source
> > BO. Use drm_gem_shmem_vmap_local().
> > 
> > Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> 
> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

On second thought, strike that r-b, I have a confused question.
> 
> > ---
> >  drivers/gpu/drm/mgag200/mgag200_mode.c | 16 +++++++++++++---
> >  1 file changed, 13 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> > index 1dfc42170059..a33e28d4c5e9 100644
> > --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> > +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> > @@ -1552,22 +1552,32 @@ mgag200_handle_damage(struct mga_device *mdev, struct drm_framebuffer *fb,
> >  		      struct drm_rect *clip)
> >  {
> >  	struct drm_device *dev = &mdev->base;
> > +	struct drm_gem_object *obj = fb->obj[0];
> >  	struct dma_buf_map map;
> >  	void *vmap;
> >  	int ret;
> >  
> > -	ret = drm_gem_shmem_vmap(fb->obj[0], &map);
> > +	ret = dma_resv_lock(obj->resv, NULL);
> >  	if (drm_WARN_ON(dev, ret))
> > -		return; /* BUG: SHMEM BO should always be vmapped */
> > +		return;
> > +	ret = drm_gem_shmem_vmap_local(obj, &map);
> > +	if (drm_WARN_ON(dev, ret))
> > +		goto err_dma_resv_unlock; /* BUG: SHMEM BO should always be vmapped */

Why is this guaranteed? I tried to hunt for a vmap in mga200g code, and
dind't find any. I'd ahve expected something in prepare/finish_fb.

Also since this is not a vram-helper using driver, why convert it over to
vmap_local? I guess that should also be explained in the commit message a
bit better.
-Daniel

> >  	vmap = map.vaddr; /* TODO: Use mapping abstraction properly */
> >  
> >  	drm_fb_memcpy_dstclip(mdev->vram, vmap, fb, clip);
> >  
> > -	drm_gem_shmem_vunmap(fb->obj[0], &map);
> > +	drm_gem_shmem_vunmap_local(obj, &map);
> > +	dma_resv_unlock(obj->resv);
> >  
> >  	/* Always scanout image at VRAM offset 0 */
> >  	mgag200_set_startadd(mdev, (u32)0);
> >  	mgag200_set_offset(mdev, fb);
> > +
> > +	return;
> > +
> > +err_dma_resv_unlock:
> > +	dma_resv_unlock(obj->resv);
> >  }
> >  
> >  static void
> > -- 
> > 2.29.2
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch