| Message ID | 20201209142527.26415-9-tzimmermann@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm: Support short-term vmap via vmap_local | expand |
Hi Am 11.12.20 um 11:01 schrieb Daniel Vetter: > On Wed, Dec 09, 2020 at 03:25:27PM +0100, Thomas Zimmermann wrote: >> Fbdev emulation has to lock the BO into place while flushing the shadow >> buffer into the BO's memory. Remove any interference with pinning by >> using vmap_local functionality (instead of full vmap). This requires >> BO reservation locking in fbdev's damage worker. >> >> The new DRM client functions for locking and vmap_local functionality >> are added for consistency with the existing style. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > > The old vmap/vunmap in the client library aren't used anymore, please > delete. That will also make it clearer in the diff what's going on and > that it makes sense to have the client and fb-helper part in one patch. They are still around for perma-mapped BOs where HW supports it (really only CMA-based drivers). See drm_fb_helper_generic_probe() and drm_fbdev_cleanup(). Best regards Thomas > > With that: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > >> --- >> drivers/gpu/drm/drm_client.c | 91 +++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/drm_fb_helper.c | 41 +++++++-------- >> include/drm/drm_client.h | 4 ++ >> 3 files changed, 116 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c >> index ce45e380f4a2..795f5cb052ba 100644 >> --- a/drivers/gpu/drm/drm_client.c >> +++ b/drivers/gpu/drm/drm_client.c >> @@ -288,6 +288,37 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u >> return ERR_PTR(ret); >> } >> >> +/** >> + * drm_client_buffer_lock - Locks the DRM client buffer >> + * @buffer: DRM client buffer >> + * >> + * This function locks the client buffer by acquiring the buffer >> + * object's reservation lock. >> + * >> + * Unlock the buffer with drm_client_buffer_unlock(). >> + * >> + * Returns: >> + * 0 on success, or a negative errno code otherwise. >> + */ >> +int >> +drm_client_buffer_lock(struct drm_client_buffer *buffer) >> +{ >> + return dma_resv_lock(buffer->gem->resv, NULL); >> +} >> +EXPORT_SYMBOL(drm_client_buffer_lock); >> + >> +/** >> + * drm_client_buffer_unlock - Unlock DRM client buffer >> + * @buffer: DRM client buffer >> + * >> + * Unlocks a client buffer. See drm_client_buffer_lock(). >> + */ >> +void drm_client_buffer_unlock(struct drm_client_buffer *buffer) >> +{ >> + dma_resv_unlock(buffer->gem->resv); >> +} >> +EXPORT_SYMBOL(drm_client_buffer_unlock); >> + >> /** >> * drm_client_buffer_vmap - Map DRM client buffer into address space >> * @buffer: DRM client buffer >> @@ -348,6 +379,66 @@ void drm_client_buffer_vunmap(struct drm_client_buffer *buffer) >> } >> EXPORT_SYMBOL(drm_client_buffer_vunmap); >> >> +/** >> + * drm_client_buffer_vmap_local - Map DRM client buffer into address space >> + * @buffer: DRM client buffer >> + * @map_copy: Returns the mapped memory's address >> + * >> + * This function maps a client buffer into kernel address space. If the >> + * buffer is already mapped, it returns the existing mapping's address. >> + * >> + * Client buffer mappings are not ref'counted. Each call to >> + * drm_client_buffer_vmap_local() should be followed by a call to >> + * drm_client_buffer_vunmap_local(); or the client buffer should be mapped >> + * throughout its lifetime. >> + * >> + * The returned address is a copy of the internal value. In contrast to >> + * other vmap interfaces, you don't need it for the client's vunmap >> + * function. So you can modify it at will during blit and draw operations. >> + * >> + * Returns: >> + * 0 on success, or a negative errno code otherwise. >> + */ >> +int >> +drm_client_buffer_vmap_local(struct drm_client_buffer *buffer, struct dma_buf_map *map_copy) >> +{ >> + struct dma_buf_map *map = &buffer->map; >> + int ret; >> + >> + /* >> + * FIXME: The dependency on GEM here isn't required, we could >> + * convert the driver handle to a dma-buf instead and use the >> + * backend-agnostic dma-buf vmap_local support instead. This would >> + * require that the handle2fd prime ioctl is reworked to pull the >> + * fd_install step out of the driver backend hooks, to make that >> + * final step optional for internal users. >> + */ >> + ret = drm_gem_vmap_local(buffer->gem, map); >> + if (ret) >> + return ret; >> + >> + *map_copy = *map; >> + >> + return 0; >> +} >> +EXPORT_SYMBOL(drm_client_buffer_vmap_local); >> + >> +/** >> + * drm_client_buffer_vunmap_local - Unmap DRM client buffer >> + * @buffer: DRM client buffer >> + * >> + * This function removes a client buffer's memory mapping. Calling this >> + * function is only required by clients that manage their buffer mappings >> + * by themselves. >> + */ >> +void drm_client_buffer_vunmap_local(struct drm_client_buffer *buffer) >> +{ >> + struct dma_buf_map *map = &buffer->map; >> + >> + drm_gem_vunmap_local(buffer->gem, map); >> +} >> +EXPORT_SYMBOL(drm_client_buffer_vunmap_local); >> + >> static void drm_client_buffer_rmfb(struct drm_client_buffer *buffer) >> { >> int ret; >> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c >> index e82db0f4e771..a56a7d9f7e35 100644 >> --- a/drivers/gpu/drm/drm_fb_helper.c >> +++ b/drivers/gpu/drm/drm_fb_helper.c >> @@ -399,28 +399,34 @@ static int drm_fb_helper_damage_blit(struct drm_fb_helper *fb_helper, >> int ret; >> >> /* >> - * We have to pin the client buffer to its current location while >> - * flushing the shadow buffer. In the general case, concurrent >> - * modesetting operations could try to move the buffer and would >> - * fail. The modeset has to be serialized by acquiring the reservation >> - * object of the underlying BO here. >> - * >> * For fbdev emulation, we only have to protect against fbdev modeset >> * operations. Nothing else will involve the client buffer's BO. So it >> * is sufficient to acquire struct drm_fb_helper.lock here. >> */ >> mutex_lock(&fb_helper->lock); >> >> - ret = drm_client_buffer_vmap(buffer, &map); >> + /* >> + * We have to keep the client buffer at its current location while >> + * flushing the shadow buffer. Concurrent operations could otherwise >> + * try to move the buffer. Therefore acquiring the reservation >> + * object of the underlying BO here. >> + */ >> + ret = drm_client_buffer_lock(buffer); >> + if (ret) >> + goto out_mutex_unlock; >> + >> + ret = drm_client_buffer_vmap_local(buffer, &map); >> if (ret) >> - goto out; >> + goto out_drm_client_buffer_unlock; >> >> dst = map; >> drm_fb_helper_damage_blit_real(fb_helper, clip, &dst); >> >> - drm_client_buffer_vunmap(buffer); >> + drm_client_buffer_vunmap_local(buffer); >> >> -out: >> +out_drm_client_buffer_unlock: >> + drm_client_buffer_unlock(buffer); >> +out_mutex_unlock: >> mutex_unlock(&fb_helper->lock); >> >> return ret; >> @@ -946,15 +952,11 @@ static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info) >> drm_modeset_lock_all(fb_helper->dev); >> drm_client_for_each_modeset(modeset, &fb_helper->client) { >> crtc = modeset->crtc; >> - if (!crtc->funcs->gamma_set || !crtc->gamma_size) { >> - ret = -EINVAL; >> - goto out; >> - } >> + if (!crtc->funcs->gamma_set || !crtc->gamma_size) >> + return -EINVAL; >> >> - if (cmap->start + cmap->len > crtc->gamma_size) { >> - ret = -EINVAL; >> - goto out; >> - } >> + if (cmap->start + cmap->len > crtc->gamma_size) >> + return -EINVAL; >> >> r = crtc->gamma_store; >> g = r + crtc->gamma_size; >> @@ -967,9 +969,8 @@ static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info) >> ret = crtc->funcs->gamma_set(crtc, r, g, b, >> crtc->gamma_size, NULL); >> if (ret) >> - goto out; >> + return ret; >> } >> -out: >> drm_modeset_unlock_all(fb_helper->dev); >> >> return ret; >> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h >> index f07f2fb02e75..df61e339a11c 100644 >> --- a/include/drm/drm_client.h >> +++ b/include/drm/drm_client.h >> @@ -156,8 +156,12 @@ struct drm_client_buffer * >> drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format); >> void drm_client_framebuffer_delete(struct drm_client_buffer *buffer); >> int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct drm_rect *rect); >> +int drm_client_buffer_lock(struct drm_client_buffer *buffer); >> +void drm_client_buffer_unlock(struct drm_client_buffer *buffer); >> int drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct dma_buf_map *map); >> void drm_client_buffer_vunmap(struct drm_client_buffer *buffer); >> +int drm_client_buffer_vmap_local(struct drm_client_buffer *buffer, struct dma_buf_map *map); >> +void drm_client_buffer_vunmap_local(struct drm_client_buffer *buffer); >> >> int drm_client_modeset_create(struct drm_client_dev *client); >> void drm_client_modeset_free(struct drm_client_dev *client); >> -- >> 2.29.2 >> >
On Fri, Dec 11, 2020 at 11:16:25AM +0100, Thomas Zimmermann wrote: > Hi > > Am 11.12.20 um 11:01 schrieb Daniel Vetter: > > On Wed, Dec 09, 2020 at 03:25:27PM +0100, Thomas Zimmermann wrote: > > > Fbdev emulation has to lock the BO into place while flushing the shadow > > > buffer into the BO's memory. Remove any interference with pinning by > > > using vmap_local functionality (instead of full vmap). This requires > > > BO reservation locking in fbdev's damage worker. > > > > > > The new DRM client functions for locking and vmap_local functionality > > > are added for consistency with the existing style. > > > > > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > > > > The old vmap/vunmap in the client library aren't used anymore, please > > delete. That will also make it clearer in the diff what's going on and > > that it makes sense to have the client and fb-helper part in one patch. > > They are still around for perma-mapped BOs where HW supports it (really only > CMA-based drivers). See drm_fb_helper_generic_probe() and > drm_fbdev_cleanup(). Ah right I didn't grep this carefully enough. I guess in that case splitting this into the drm_client patch and fb-helper patch would be good I think. Also some kerneldoc commment addition for normal vmap that vmap_local is preferred for short-term mappings, and for vmap_local that _vmap() gives you the long term mapping. Just for more links and stuff in docs. With that: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> -Daniel > > Best regards > Thomas > > > > > With that: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > > --- > > > drivers/gpu/drm/drm_client.c | 91 +++++++++++++++++++++++++++++++++ > > > drivers/gpu/drm/drm_fb_helper.c | 41 +++++++-------- > > > include/drm/drm_client.h | 4 ++ > > > 3 files changed, 116 insertions(+), 20 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c > > > index ce45e380f4a2..795f5cb052ba 100644 > > > --- a/drivers/gpu/drm/drm_client.c > > > +++ b/drivers/gpu/drm/drm_client.c > > > @@ -288,6 +288,37 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u > > > return ERR_PTR(ret); > > > } > > > +/** > > > + * drm_client_buffer_lock - Locks the DRM client buffer > > > + * @buffer: DRM client buffer > > > + * > > > + * This function locks the client buffer by acquiring the buffer > > > + * object's reservation lock. > > > + * > > > + * Unlock the buffer with drm_client_buffer_unlock(). > > > + * > > > + * Returns: > > > + * 0 on success, or a negative errno code otherwise. > > > + */ > > > +int > > > +drm_client_buffer_lock(struct drm_client_buffer *buffer) > > > +{ > > > + return dma_resv_lock(buffer->gem->resv, NULL); > > > +} > > > +EXPORT_SYMBOL(drm_client_buffer_lock); > > > + > > > +/** > > > + * drm_client_buffer_unlock - Unlock DRM client buffer > > > + * @buffer: DRM client buffer > > > + * > > > + * Unlocks a client buffer. See drm_client_buffer_lock(). > > > + */ > > > +void drm_client_buffer_unlock(struct drm_client_buffer *buffer) > > > +{ > > > + dma_resv_unlock(buffer->gem->resv); > > > +} > > > +EXPORT_SYMBOL(drm_client_buffer_unlock); > > > + > > > /** > > > * drm_client_buffer_vmap - Map DRM client buffer into address space > > > * @buffer: DRM client buffer > > > @@ -348,6 +379,66 @@ void drm_client_buffer_vunmap(struct drm_client_buffer *buffer) > > > } > > > EXPORT_SYMBOL(drm_client_buffer_vunmap); > > > +/** > > > + * drm_client_buffer_vmap_local - Map DRM client buffer into address space > > > + * @buffer: DRM client buffer > > > + * @map_copy: Returns the mapped memory's address > > > + * > > > + * This function maps a client buffer into kernel address space. If the > > > + * buffer is already mapped, it returns the existing mapping's address. > > > + * > > > + * Client buffer mappings are not ref'counted. Each call to > > > + * drm_client_buffer_vmap_local() should be followed by a call to > > > + * drm_client_buffer_vunmap_local(); or the client buffer should be mapped > > > + * throughout its lifetime. > > > + * > > > + * The returned address is a copy of the internal value. In contrast to > > > + * other vmap interfaces, you don't need it for the client's vunmap > > > + * function. So you can modify it at will during blit and draw operations. > > > + * > > > + * Returns: > > > + * 0 on success, or a negative errno code otherwise. > > > + */ > > > +int > > > +drm_client_buffer_vmap_local(struct drm_client_buffer *buffer, struct dma_buf_map *map_copy) > > > +{ > > > + struct dma_buf_map *map = &buffer->map; > > > + int ret; > > > + > > > + /* > > > + * FIXME: The dependency on GEM here isn't required, we could > > > + * convert the driver handle to a dma-buf instead and use the > > > + * backend-agnostic dma-buf vmap_local support instead. This would > > > + * require that the handle2fd prime ioctl is reworked to pull the > > > + * fd_install step out of the driver backend hooks, to make that > > > + * final step optional for internal users. > > > + */ > > > + ret = drm_gem_vmap_local(buffer->gem, map); > > > + if (ret) > > > + return ret; > > > + > > > + *map_copy = *map; > > > + > > > + return 0; > > > +} > > > +EXPORT_SYMBOL(drm_client_buffer_vmap_local); > > > + > > > +/** > > > + * drm_client_buffer_vunmap_local - Unmap DRM client buffer > > > + * @buffer: DRM client buffer > > > + * > > > + * This function removes a client buffer's memory mapping. Calling this > > > + * function is only required by clients that manage their buffer mappings > > > + * by themselves. > > > + */ > > > +void drm_client_buffer_vunmap_local(struct drm_client_buffer *buffer) > > > +{ > > > + struct dma_buf_map *map = &buffer->map; > > > + > > > + drm_gem_vunmap_local(buffer->gem, map); > > > +} > > > +EXPORT_SYMBOL(drm_client_buffer_vunmap_local); > > > + > > > static void drm_client_buffer_rmfb(struct drm_client_buffer *buffer) > > > { > > > int ret; > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > > > index e82db0f4e771..a56a7d9f7e35 100644 > > > --- a/drivers/gpu/drm/drm_fb_helper.c > > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > > @@ -399,28 +399,34 @@ static int drm_fb_helper_damage_blit(struct drm_fb_helper *fb_helper, > > > int ret; > > > /* > > > - * We have to pin the client buffer to its current location while > > > - * flushing the shadow buffer. In the general case, concurrent > > > - * modesetting operations could try to move the buffer and would > > > - * fail. The modeset has to be serialized by acquiring the reservation > > > - * object of the underlying BO here. > > > - * > > > * For fbdev emulation, we only have to protect against fbdev modeset > > > * operations. Nothing else will involve the client buffer's BO. So it > > > * is sufficient to acquire struct drm_fb_helper.lock here. > > > */ > > > mutex_lock(&fb_helper->lock); > > > - ret = drm_client_buffer_vmap(buffer, &map); > > > + /* > > > + * We have to keep the client buffer at its current location while > > > + * flushing the shadow buffer. Concurrent operations could otherwise > > > + * try to move the buffer. Therefore acquiring the reservation > > > + * object of the underlying BO here. > > > + */ > > > + ret = drm_client_buffer_lock(buffer); > > > + if (ret) > > > + goto out_mutex_unlock; > > > + > > > + ret = drm_client_buffer_vmap_local(buffer, &map); > > > if (ret) > > > - goto out; > > > + goto out_drm_client_buffer_unlock; > > > dst = map; > > > drm_fb_helper_damage_blit_real(fb_helper, clip, &dst); > > > - drm_client_buffer_vunmap(buffer); > > > + drm_client_buffer_vunmap_local(buffer); > > > -out: > > > +out_drm_client_buffer_unlock: > > > + drm_client_buffer_unlock(buffer); > > > +out_mutex_unlock: > > > mutex_unlock(&fb_helper->lock); > > > return ret; > > > @@ -946,15 +952,11 @@ static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info) > > > drm_modeset_lock_all(fb_helper->dev); > > > drm_client_for_each_modeset(modeset, &fb_helper->client) { > > > crtc = modeset->crtc; > > > - if (!crtc->funcs->gamma_set || !crtc->gamma_size) { > > > - ret = -EINVAL; > > > - goto out; > > > - } > > > + if (!crtc->funcs->gamma_set || !crtc->gamma_size) > > > + return -EINVAL; > > > - if (cmap->start + cmap->len > crtc->gamma_size) { > > > - ret = -EINVAL; > > > - goto out; > > > - } > > > + if (cmap->start + cmap->len > crtc->gamma_size) > > > + return -EINVAL; > > > r = crtc->gamma_store; > > > g = r + crtc->gamma_size; > > > @@ -967,9 +969,8 @@ static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info) > > > ret = crtc->funcs->gamma_set(crtc, r, g, b, > > > crtc->gamma_size, NULL); > > > if (ret) > > > - goto out; > > > + return ret; > > > } > > > -out: > > > drm_modeset_unlock_all(fb_helper->dev); > > > return ret; > > > diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h > > > index f07f2fb02e75..df61e339a11c 100644 > > > --- a/include/drm/drm_client.h > > > +++ b/include/drm/drm_client.h > > > @@ -156,8 +156,12 @@ struct drm_client_buffer * > > > drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format); > > > void drm_client_framebuffer_delete(struct drm_client_buffer *buffer); > > > int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct drm_rect *rect); > > > +int drm_client_buffer_lock(struct drm_client_buffer *buffer); > > > +void drm_client_buffer_unlock(struct drm_client_buffer *buffer); > > > int drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct dma_buf_map *map); > > > void drm_client_buffer_vunmap(struct drm_client_buffer *buffer); > > > +int drm_client_buffer_vmap_local(struct drm_client_buffer *buffer, struct dma_buf_map *map); > > > +void drm_client_buffer_vunmap_local(struct drm_client_buffer *buffer); > > > int drm_client_modeset_create(struct drm_client_dev *client); > > > void drm_client_modeset_free(struct drm_client_dev *client); > > > -- > > > 2.29.2 > > > > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer >
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c index ce45e380f4a2..795f5cb052ba 100644 --- a/drivers/gpu/drm/drm_client.c +++ b/drivers/gpu/drm/drm_client.c @@ -288,6 +288,37 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u return ERR_PTR(ret); } +/** + * drm_client_buffer_lock - Locks the DRM client buffer + * @buffer: DRM client buffer + * + * This function locks the client buffer by acquiring the buffer + * object's reservation lock. + * + * Unlock the buffer with drm_client_buffer_unlock(). + * + * Returns: + * 0 on success, or a negative errno code otherwise. + */ +int +drm_client_buffer_lock(struct drm_client_buffer *buffer) +{ + return dma_resv_lock(buffer->gem->resv, NULL); +} +EXPORT_SYMBOL(drm_client_buffer_lock); + +/** + * drm_client_buffer_unlock - Unlock DRM client buffer + * @buffer: DRM client buffer + * + * Unlocks a client buffer. See drm_client_buffer_lock(). + */ +void drm_client_buffer_unlock(struct drm_client_buffer *buffer) +{ + dma_resv_unlock(buffer->gem->resv); +} +EXPORT_SYMBOL(drm_client_buffer_unlock); + /** * drm_client_buffer_vmap - Map DRM client buffer into address space * @buffer: DRM client buffer @@ -348,6 +379,66 @@ void drm_client_buffer_vunmap(struct drm_client_buffer *buffer) } EXPORT_SYMBOL(drm_client_buffer_vunmap); +/** + * drm_client_buffer_vmap_local - Map DRM client buffer into address space + * @buffer: DRM client buffer + * @map_copy: Returns the mapped memory's address + * + * This function maps a client buffer into kernel address space. If the + * buffer is already mapped, it returns the existing mapping's address. + * + * Client buffer mappings are not ref'counted. Each call to + * drm_client_buffer_vmap_local() should be followed by a call to + * drm_client_buffer_vunmap_local(); or the client buffer should be mapped + * throughout its lifetime. + * + * The returned address is a copy of the internal value. In contrast to + * other vmap interfaces, you don't need it for the client's vunmap + * function. So you can modify it at will during blit and draw operations. + * + * Returns: + * 0 on success, or a negative errno code otherwise. + */ +int +drm_client_buffer_vmap_local(struct drm_client_buffer *buffer, struct dma_buf_map *map_copy) +{ + struct dma_buf_map *map = &buffer->map; + int ret; + + /* + * FIXME: The dependency on GEM here isn't required, we could + * convert the driver handle to a dma-buf instead and use the + * backend-agnostic dma-buf vmap_local support instead. This would + * require that the handle2fd prime ioctl is reworked to pull the + * fd_install step out of the driver backend hooks, to make that + * final step optional for internal users. + */ + ret = drm_gem_vmap_local(buffer->gem, map); + if (ret) + return ret; + + *map_copy = *map; + + return 0; +} +EXPORT_SYMBOL(drm_client_buffer_vmap_local); + +/** + * drm_client_buffer_vunmap_local - Unmap DRM client buffer + * @buffer: DRM client buffer + * + * This function removes a client buffer's memory mapping. Calling this + * function is only required by clients that manage their buffer mappings + * by themselves. + */ +void drm_client_buffer_vunmap_local(struct drm_client_buffer *buffer) +{ + struct dma_buf_map *map = &buffer->map; + + drm_gem_vunmap_local(buffer->gem, map); +} +EXPORT_SYMBOL(drm_client_buffer_vunmap_local); + static void drm_client_buffer_rmfb(struct drm_client_buffer *buffer) { int ret; diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index e82db0f4e771..a56a7d9f7e35 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -399,28 +399,34 @@ static int drm_fb_helper_damage_blit(struct drm_fb_helper *fb_helper, int ret; /* - * We have to pin the client buffer to its current location while - * flushing the shadow buffer. In the general case, concurrent - * modesetting operations could try to move the buffer and would - * fail. The modeset has to be serialized by acquiring the reservation - * object of the underlying BO here. - * * For fbdev emulation, we only have to protect against fbdev modeset * operations. Nothing else will involve the client buffer's BO. So it * is sufficient to acquire struct drm_fb_helper.lock here. */ mutex_lock(&fb_helper->lock); - ret = drm_client_buffer_vmap(buffer, &map); + /* + * We have to keep the client buffer at its current location while + * flushing the shadow buffer. Concurrent operations could otherwise + * try to move the buffer. Therefore acquiring the reservation + * object of the underlying BO here. + */ + ret = drm_client_buffer_lock(buffer); + if (ret) + goto out_mutex_unlock; + + ret = drm_client_buffer_vmap_local(buffer, &map); if (ret) - goto out; + goto out_drm_client_buffer_unlock; dst = map; drm_fb_helper_damage_blit_real(fb_helper, clip, &dst); - drm_client_buffer_vunmap(buffer); + drm_client_buffer_vunmap_local(buffer); -out: +out_drm_client_buffer_unlock: + drm_client_buffer_unlock(buffer); +out_mutex_unlock: mutex_unlock(&fb_helper->lock); return ret; @@ -946,15 +952,11 @@ static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info) drm_modeset_lock_all(fb_helper->dev); drm_client_for_each_modeset(modeset, &fb_helper->client) { crtc = modeset->crtc; - if (!crtc->funcs->gamma_set || !crtc->gamma_size) { - ret = -EINVAL; - goto out; - } + if (!crtc->funcs->gamma_set || !crtc->gamma_size) + return -EINVAL; - if (cmap->start + cmap->len > crtc->gamma_size) { - ret = -EINVAL; - goto out; - } + if (cmap->start + cmap->len > crtc->gamma_size) + return -EINVAL; r = crtc->gamma_store; g = r + crtc->gamma_size; @@ -967,9 +969,8 @@ static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info) ret = crtc->funcs->gamma_set(crtc, r, g, b, crtc->gamma_size, NULL); if (ret) - goto out; + return ret; } -out: drm_modeset_unlock_all(fb_helper->dev); return ret; diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h index f07f2fb02e75..df61e339a11c 100644 --- a/include/drm/drm_client.h +++ b/include/drm/drm_client.h @@ -156,8 +156,12 @@ struct drm_client_buffer * drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format); void drm_client_framebuffer_delete(struct drm_client_buffer *buffer); int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct drm_rect *rect); +int drm_client_buffer_lock(struct drm_client_buffer *buffer); +void drm_client_buffer_unlock(struct drm_client_buffer *buffer); int drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct dma_buf_map *map); void drm_client_buffer_vunmap(struct drm_client_buffer *buffer); +int drm_client_buffer_vmap_local(struct drm_client_buffer *buffer, struct dma_buf_map *map); +void drm_client_buffer_vunmap_local(struct drm_client_buffer *buffer); int drm_client_modeset_create(struct drm_client_dev *client); void drm_client_modeset_free(struct drm_client_dev *client);
Fbdev emulation has to lock the BO into place while flushing the shadow buffer into the BO's memory. Remove any interference with pinning by using vmap_local functionality (instead of full vmap). This requires BO reservation locking in fbdev's damage worker. The new DRM client functions for locking and vmap_local functionality are added for consistency with the existing style. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/drm_client.c | 91 +++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_fb_helper.c | 41 +++++++-------- include/drm/drm_client.h | 4 ++ 3 files changed, 116 insertions(+), 20 deletions(-)