[PATCH v2 0/2] Fixes for omapdrm console

[Thomas Zimmermann]

Hi

Am 27.02.24 um 09:01 schrieb Tony Lindgren:
> * Thomas Zimmermann <tzimmermann at suse.de> [240227 07:56]:
>> Am 27.02.24 um 08:06 schrieb Tony Lindgren:
>>> * Tony Lindgren <tony at atomide.com> [240226 13:26]:
>>>> * Thomas Zimmermann <tzimmermann at suse.de> [240226 09:10]:
>>>>> Am 26.02.24 um 10:01 schrieb Tomi Valkeinen:
>>>>>> On 26/02/2024 10:26, Tomi Valkeinen wrote:
>>>>>>> How is it broken? I don't usually use the console (or fbdev) but
>>>>>>> enabling it now, it seems to work fine for me, on DRA76 EVM with
>>>>>>> HDMI output.
>>>>> Omapdrm implements drm_framebuffer_funcs.dirty withomap_framebuffer_dirty().
>>>>> AFAIK DRM semantics requires to run the dirty helper after writing to the
>>>>> framebuffer's memory. Userspace does this via the DIRTYFB ioctl. [1] But (at
>>>>> least) for correctness the console needs to do the same.
>>>>>
>>>>> [1] https://elixir.bootlin.com/linux/v6.7.6/source/drivers/gpu/drm/drm_ioctl.c#L679
>>>> Yes I noticed console not updating and bisected it down to the two
>>>> commits listed. I did the bisect on a droid4 though with command mode
>>>> LCD. I did not test with HDMI, will give that a try too.
>>> I can reproduce the cache issue with Tomi's omapfb-tests [2] below:
>>>
>>> while true;
>>>         do dd if=/dev/urandom of=/dev/fb0
>>>         ~/src/omapfb-tests/test
>>>         sleep 1
>>> done
>>>
>>> That produces short random data stripes on the test image.
>>>
>>>>>> After applying your patches, I see a lot of cache-related artifacts on
>>>>>> the screen when updating the fb.
>>>>> I guess we might need a dma-specific mmap helper to make this work
>>>>> correctly.
>>> Comparing the difference between drm_gem_mmap_obj() and
>>> fb_deferred_io_mmap(), the following test patch makes the cache issue
>>> go away for me. Not sure if this can be set based on some flag, or if
>>> we need a separate fb_deferred_io_wc_mmap() or something like that?
>>>
>>> [2] https://github.com/tomba/omapfb-tests
>>>
>>> 8< --------------------
>>> diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
>>> --- a/drivers/video/fbdev/core/fb_defio.c
>>> +++ b/drivers/video/fbdev/core/fb_defio.c
>>> @@ -224,6 +224,7 @@ static const struct address_space_operations fb_deferred_io_aops = {
>>>    int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
>>>    {
>>>    	vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
>>> +	vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
>> Great, that's exactly what I had in mind!
> OK :)
>
>> My proposal is to add this mmap function directly to omapdrm. I'll later
>> take care of integrating this into the overall framework. I have a few other
>> ideas in mind that are related to this issue. Ok?
> OK that sounds good to me, I'll post v3 set of patches.

I just realized the fb_deferred_io_mmap() is already exported. So please 
use it instead of duplicating the code in omapdrm.

[1] 
https://elixir.bootlin.com/linux/v6.7/source/drivers/video/fbdev/core/fb_defio.c#L237

I also noticed that omapdrm does not yet select the correct Kconfig 
symbols. That can be fixed by

  1) creating Kconfig FB_DMAMEM_HELPERS_DEFERRED that are similar to 
their SYSMEM equivalent at [2]. The tokens should look like this

configFB_DMAMEM_HELPERS_DEFERRED  <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_SYSMEM_HELPERS_DEFERRED>
bool
depends onFB_CORE  <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_CORE>
selectFB_DEFERRED_IO  <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_DEFERRED_IO>
selectFB_DMAMEM_HELPERS  <https://elixir.bootlin.com/linux/latest/K/ident/CONFIG_FB_SYSMEM_HELPERS>


   2) and selecting it instead of FB_DMAMEM_HELPERS under omapdrm's 
Kconfig symbol.

[2] 
https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/core/Kconfig#L147

Best regards
Thomas

>
> Regards,
>
> Tony
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)