[PATCH 5/8] drm/mipi-dbi: Prepare framebuffer copy operation in pipe-update helpers

[Noralf Trønnes]

Den 02.12.2022 12.27, skrev Thomas Zimmermann:
> Hi
> 
> Am 25.11.22 um 18:39 schrieb Noralf Trønnes:
>>
>>
>> Den 21.11.2022 11.45, skrev Thomas Zimmermann:
>>> Move the vmap/vunmap blocks from the inner fb_dirty helpers into the
>>> MIPI DBI update helpers. The function calls can result in waiting and/or
>>> processing overhead. Reduce the penalties by executing the functions
>>> once
>>> in the outer-most function of the pipe update.
>>>
>>> This change also prepares for MIPI DBI for shadow-plane helpers. With
>>> shadow-plane helpers, transfer source buffers are mapped into kernel
>>> address space automatically.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>>> ---

>>> @@ -303,9 +291,6 @@ static void mipi_dbi_fb_dirty(struct
>>> drm_framebuffer *fb, struct drm_rect *rect)
>>>       if (ret)
>>>           drm_err_once(fb->dev, "Failed to update display %d\n", ret);
>>>   -    drm_gem_fb_vunmap(fb, map);
>>> -
>>> -err_drm_dev_exit:
>>>       drm_dev_exit(idx);
>>>   }
>>>   @@ -338,14 +323,27 @@ EXPORT_SYMBOL(mipi_dbi_pipe_mode_valid);
>>>   void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
>>>                 struct drm_plane_state *old_state)
>>>   {
>>> +    struct iosys_map map[DRM_FORMAT_MAX_PLANES];
>>> +    struct iosys_map data[DRM_FORMAT_MAX_PLANES];
>>
>> These should have been zeroed to avoid UBSAN complaint, but you'll
>> remove these later so not so important.
> 
> Will be fixed.
> 
> But do you know how these warnings happen? I went through the helpers
> before and changed them to only access the format-info's relevant
> planes. No unintialized memory access should be reported.
> 

It happens with imported buffers, iosys_map_clear() looks at the
uninitialized boolean variable ->is_iomem:

drm_gem_fb_vmap -> ... -> dma_buf_vmap -> iosys_map_clear

static inline void iosys_map_clear(struct iosys_map *map)
{
	if (map->is_iomem) {
		map->vaddr_iomem = NULL;
		map->is_iomem = false;
	} else {
		map->vaddr = NULL;
	}
}

Noralf.