Hi Sam

Am 03.08.21 um 18:21 schrieb Sam Ravnborg:
> Hi Thomas,
> 
> On Tue, Aug 03, 2021 at 02:59:17PM +0200, Thomas Zimmermann wrote:
>> A framebuffer's offsets field might be non-zero to make the BO data
>> start at the specified offset within the BO's memory. Handle this
>> case in drm_gem_fb_vmap() and update all callers. So far, many drivers
>> ignore the offsets, which can lead to visual artifacts.
>>
>> Patch 1 adds an optional argument to drm_gem_fb_vmap() to return the
>> offset-adjusted data address for use with shadow-buffered planes.
>>
>> Patches 3 and 11 convert gud and vkms, which are the other callers of
>> drm_gem_fb_vmap(). For gud, it's just a cleanup. Vkms will handle the
>> framebuffer offsets correctly for its input and output framebuffers.
>>
>> The other patches convert users of shadow-buffered planes to use the
>> data address. After conversion, each driver will use the correct data
>> for non-zero offsets.
>>
> 
>>    drm/ast: Use offset-adjusted shadow-plane mappings
>>    drm/gud: Get offset-adjusted mapping from drm_gem_fb_vmap()
>>    drm/hyperv: Use offset-adjusted shadow-plane mappings
>>    drm/mgag200: Use offset-adjusted shadow-plane mappings
>>    drm/cirrus: Use offset-adjusted shadow-plane mappings
>>    drm/gm12u320: Use offset-adjusted shadow-plane mappings
>>    drm/simpledrm: Use offset-adjusted shadow-plane mapping
>>    drm/udl: Use offset-adjusted shadow-plane mapping
>>    drm/vbox: Use offset-adjusted shadow-plane mappings
>>    drm/vkms: Use offset-adjusted shadow-plane mappings and output
> Everything looked good while reading through the patches.
> I cannot say if everything was properly converted but the patches looked
> good.
> 
> So they are all:
> Acked-by: Sam Ravnborg <sam@ravnborg.org>

Thanks!

> 
> There was a few TODO comments visible aboput using the mapping api
> properly. I assume this is coming in a later patch set..

There are indeed quite a few such comments. When we introduced the 
dma_buf_map type to solve the fbdev issue on sparc64, in many places I 
simply put the existing vaddr pointers into the map structure, and vice 
versa.

The code works as expected, but in the future we may want to use 
dma_buf_map for all framebuffer pointers. This would, for example, 
require format conversion helpers that operate on these structures. 
Adding that will require a number of changes throughout these helpers.

Best regards
Thomas

> 
> 	Sam
> 

-- 
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