Re: [v2,5/6] drm/fb-helper: Schedule deferred-I/O worker after writing to framebuffer

2022-12-05 Thread Marek Szyprowski
On 17.11.2022 18:21, Marek Szyprowski wrote:
> On 17.11.2022 17:07, Thomas Zimmermann wrote:
>> Am 17.11.22 um 13:57 schrieb Marek Szyprowski:
>>> On 15.11.2022 12:58, Thomas Zimmermann wrote:
 Schedule the deferred-I/O worker instead of the damage worker after
 writing to the fbdev framebuffer. The deferred-I/O worker then 
 performs
 the dirty-fb update. The fbdev emulation will initialize deferred I/O
 for all drivers that require damage updates. It is therefore a valid
 assumption that the deferred-I/O worker is present.

 It would be possible to perform the damage handling directly from 
 within
 the write operation. But doing this could increase the overhead of the
 write or interfere with a concurrently scheduled deferred-I/O worker.
 Instead, scheduling the deferred-I/O worker with its regular delay of
 50 ms removes load off the write operation and allows the deferred-I/O
 worker to handle multiple write operations that arrived during the 
 delay
 time window.

 v2:
 * keep drm_fb_helper_damage() (Daniel)
 * use fb_deferred_io_schedule_flush() (Daniel)
 * clarify comments (Daniel)

 Signed-off-by: Thomas Zimmermann 
 Reviewed-by: Daniel Vetter 
>>>
>>> This patch, merged into today's linux-next as commit 7f5cc4a3e5e4
>>> ("drm/fb-helper: Schedule deferred-I/O worker after writing to
>>> framebuffer"), triggers a following warning on Raspberry Pi 3 & 4 as
>>> well as all Amlogic Meson G12A/B based boards:
>>>
>>> [ cut here ]
>>> WARNING: CPU: 0 PID: 220 at drivers/video/fbdev/core/fb_defio.c:340
>>
>> Thank you so much for reporting. That line should never be executed 
>> with vc4 et al.
>>
>> If you have the time, could you please try the attached patch and 
>> report the results. Thanks a lot.
>
> This fixes the issue observed on my Raspberry Pi 3/4 and Amlogic Meson 
> based boards. Feel free to add:
>
> Tested-by: Marek Szyprowski 

Gentle ping. This issue is not fixed in linux-next.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R Institute Poland



Re: [v2,5/6] drm/fb-helper: Schedule deferred-I/O worker after writing to framebuffer

2022-11-17 Thread Marek Szyprowski
On 17.11.2022 17:07, Thomas Zimmermann wrote:
> Am 17.11.22 um 13:57 schrieb Marek Szyprowski:
>> On 15.11.2022 12:58, Thomas Zimmermann wrote:
>>> Schedule the deferred-I/O worker instead of the damage worker after
>>> writing to the fbdev framebuffer. The deferred-I/O worker then performs
>>> the dirty-fb update. The fbdev emulation will initialize deferred I/O
>>> for all drivers that require damage updates. It is therefore a valid
>>> assumption that the deferred-I/O worker is present.
>>>
>>> It would be possible to perform the damage handling directly from 
>>> within
>>> the write operation. But doing this could increase the overhead of the
>>> write or interfere with a concurrently scheduled deferred-I/O worker.
>>> Instead, scheduling the deferred-I/O worker with its regular delay of
>>> 50 ms removes load off the write operation and allows the deferred-I/O
>>> worker to handle multiple write operations that arrived during the 
>>> delay
>>> time window.
>>>
>>> v2:
>>> * keep drm_fb_helper_damage() (Daniel)
>>> * use fb_deferred_io_schedule_flush() (Daniel)
>>> * clarify comments (Daniel)
>>>
>>> Signed-off-by: Thomas Zimmermann 
>>> Reviewed-by: Daniel Vetter 
>>
>> This patch, merged into today's linux-next as commit 7f5cc4a3e5e4
>> ("drm/fb-helper: Schedule deferred-I/O worker after writing to
>> framebuffer"), triggers a following warning on Raspberry Pi 3 & 4 as
>> well as all Amlogic Meson G12A/B based boards:
>>
>> [ cut here ]
>> WARNING: CPU: 0 PID: 220 at drivers/video/fbdev/core/fb_defio.c:340
>
> Thank you so much for reporting. That line should never be executed 
> with vc4 et al.
>
> If you have the time, could you please try the attached patch and 
> report the results. Thanks a lot.

This fixes the issue observed on my Raspberry Pi 3/4 and Amlogic Meson 
based boards. Feel free to add:

Tested-by: Marek Szyprowski 

Best regards

-- 
Marek Szyprowski, PhD
Samsung R Institute Poland



Re: [v2,5/6] drm/fb-helper: Schedule deferred-I/O worker after writing to framebuffer

2022-11-17 Thread Thomas Zimmermann

Hi

Am 17.11.22 um 13:57 schrieb Marek Szyprowski:

Hi Thomas,

On 15.11.2022 12:58, Thomas Zimmermann wrote:

Schedule the deferred-I/O worker instead of the damage worker after
writing to the fbdev framebuffer. The deferred-I/O worker then performs
the dirty-fb update. The fbdev emulation will initialize deferred I/O
for all drivers that require damage updates. It is therefore a valid
assumption that the deferred-I/O worker is present.

It would be possible to perform the damage handling directly from within
the write operation. But doing this could increase the overhead of the
write or interfere with a concurrently scheduled deferred-I/O worker.
Instead, scheduling the deferred-I/O worker with its regular delay of
50 ms removes load off the write operation and allows the deferred-I/O
worker to handle multiple write operations that arrived during the delay
time window.

v2:
* keep drm_fb_helper_damage() (Daniel)
* use fb_deferred_io_schedule_flush() (Daniel)
* clarify comments (Daniel)

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Daniel Vetter 


This patch, merged into today's linux-next as commit 7f5cc4a3e5e4
("drm/fb-helper: Schedule deferred-I/O worker after writing to
framebuffer"), triggers a following warning on Raspberry Pi 3 & 4 as
well as all Amlogic Meson G12A/B based boards:

[ cut here ]
WARNING: CPU: 0 PID: 220 at drivers/video/fbdev/core/fb_defio.c:340


Thank you so much for reporting. That line should never be executed with 
vc4 et al.


If you have the time, could you please try the attached patch and report 
the results. Thanks a lot.


Best regards
Thomas


soft_cursor+0x180/0x1f0
Modules linked in: brcmfmac brcmutil vc4(+) sha256_generic libsha256
snd_soc_hdmi_codec sha256_arm cfg80211 snd_soc_core ac97_bus
snd_pcm_dmaengine hci_uart btbcm snd_pcm snd_timer snd crc32_arm_ce
soundcore raspberrypi_hwmon drm_dma_helper bluetooth bcm2835_thermal
ecdh_generic ecc libaes
CPU: 0 PID: 220 Comm: systemd-udevd Not tainted
6.1.0-rc5-next-20221117-00041-g13334c897c2b #5953
Hardware name: BCM2835
   unwind_backtrace from show_stack+0x10/0x14
   show_stack from dump_stack_lvl+0x40/0x4c
   dump_stack_lvl from __warn+0xc8/0x13c
   __warn from warn_slowpath_fmt+0x5c/0xb8
   warn_slowpath_fmt from soft_cursor+0x180/0x1f0
   soft_cursor from bit_cursor+0x320/0x4d0
   bit_cursor from fbcon_cursor+0xf4/0x124
   fbcon_cursor from hide_cursor+0x30/0x98
   hide_cursor from redraw_screen+0x1e8/0x230
   redraw_screen from fbcon_prepare_logo+0x390/0x44c
   fbcon_prepare_logo from fbcon_init+0x494/0x5ac
   fbcon_init from visual_init+0xc0/0x108
   visual_init from do_bind_con_driver+0x1b8/0x3a8
   do_bind_con_driver from do_take_over_console+0x13c/0x1e8
   do_take_over_console from do_fbcon_takeover+0x70/0xd0
   do_fbcon_takeover from fbcon_fb_registered+0x19c/0x1ac
   fbcon_fb_registered from register_framebuffer+0x1ec/0x2ec
   register_framebuffer from
__drm_fb_helper_initial_config_and_unlock+0x3f0/0x5b8
   __drm_fb_helper_initial_config_and_unlock from
drm_fbdev_client_hotplug+0xbc/0x120
   drm_fbdev_client_hotplug from drm_fbdev_generic_setup+0x88/0x174
   drm_fbdev_generic_setup from vc4_drm_bind+0x1fc/0x294 [vc4]
   vc4_drm_bind [vc4] from try_to_bring_up_aggregate_device+0x160/0x1bc
   try_to_bring_up_aggregate_device from
component_master_add_with_match+0xc4/0xf8
   component_master_add_with_match from vc4_platform_drm_probe+0xa0/0xc0
[vc4]
   vc4_platform_drm_probe [vc4] from platform_probe+0x5c/0xb8
   platform_probe from really_probe+0xc8/0x2f0
   really_probe from __driver_probe_device+0x84/0xe4
   __driver_probe_device from driver_probe_device+0x30/0x104
   driver_probe_device from __driver_attach+0x90/0x174
   __driver_attach from bus_for_each_dev+0x70/0xb0
   bus_for_each_dev from bus_add_driver+0x164/0x1f0
   bus_add_driver from driver_register+0x88/0x11c
   driver_register from vc4_drm_register+0x44/0x1000 [vc4]
   vc4_drm_register [vc4] from do_one_initcall+0x40/0x1e0
   do_one_initcall from do_init_module+0x44/0x1d4
   do_init_module from sys_finit_module+0xbc/0xf8
   sys_finit_module from ret_fast_syscall+0x0/0x54
Exception stack(0xf0d85fa8 to 0xf0d85ff0)
...
---[ end trace  ]---
Console: switching to colour frame buffer device 90x30

It looks that at least the VC4 DRM and Meson DRM drivers needs some
adjustments to avoid this warning. Am I right?



---
   drivers/gpu/drm/drm_fb_helper.c | 10 +-
   drivers/video/fbdev/core/fb_defio.c | 16 
   include/linux/fb.h  |  1 +
   3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index cdbf03e941b2b..fbb9088f7defc 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -599,9 +599,17 @@ static void drm_fb_helper_add_damage_clip(struct 
drm_fb_helper *helper, u32 x, u
   static void drm_fb_helper_damage(struct drm_fb_helper 

Re: [v2,5/6] drm/fb-helper: Schedule deferred-I/O worker after writing to framebuffer

2022-11-17 Thread Marek Szyprowski
Hi Thomas,

On 15.11.2022 12:58, Thomas Zimmermann wrote:
> Schedule the deferred-I/O worker instead of the damage worker after
> writing to the fbdev framebuffer. The deferred-I/O worker then performs
> the dirty-fb update. The fbdev emulation will initialize deferred I/O
> for all drivers that require damage updates. It is therefore a valid
> assumption that the deferred-I/O worker is present.
>
> It would be possible to perform the damage handling directly from within
> the write operation. But doing this could increase the overhead of the
> write or interfere with a concurrently scheduled deferred-I/O worker.
> Instead, scheduling the deferred-I/O worker with its regular delay of
> 50 ms removes load off the write operation and allows the deferred-I/O
> worker to handle multiple write operations that arrived during the delay
> time window.
>
> v2:
>   * keep drm_fb_helper_damage() (Daniel)
>   * use fb_deferred_io_schedule_flush() (Daniel)
>   * clarify comments (Daniel)
>
> Signed-off-by: Thomas Zimmermann 
> Reviewed-by: Daniel Vetter 

This patch, merged into today's linux-next as commit 7f5cc4a3e5e4 
("drm/fb-helper: Schedule deferred-I/O worker after writing to 
framebuffer"), triggers a following warning on Raspberry Pi 3 & 4 as 
well as all Amlogic Meson G12A/B based boards:

[ cut here ]
WARNING: CPU: 0 PID: 220 at drivers/video/fbdev/core/fb_defio.c:340 
soft_cursor+0x180/0x1f0
Modules linked in: brcmfmac brcmutil vc4(+) sha256_generic libsha256 
snd_soc_hdmi_codec sha256_arm cfg80211 snd_soc_core ac97_bus 
snd_pcm_dmaengine hci_uart btbcm snd_pcm snd_timer snd crc32_arm_ce 
soundcore raspberrypi_hwmon drm_dma_helper bluetooth bcm2835_thermal 
ecdh_generic ecc libaes
CPU: 0 PID: 220 Comm: systemd-udevd Not tainted 
6.1.0-rc5-next-20221117-00041-g13334c897c2b #5953
Hardware name: BCM2835
  unwind_backtrace from show_stack+0x10/0x14
  show_stack from dump_stack_lvl+0x40/0x4c
  dump_stack_lvl from __warn+0xc8/0x13c
  __warn from warn_slowpath_fmt+0x5c/0xb8
  warn_slowpath_fmt from soft_cursor+0x180/0x1f0
  soft_cursor from bit_cursor+0x320/0x4d0
  bit_cursor from fbcon_cursor+0xf4/0x124
  fbcon_cursor from hide_cursor+0x30/0x98
  hide_cursor from redraw_screen+0x1e8/0x230
  redraw_screen from fbcon_prepare_logo+0x390/0x44c
  fbcon_prepare_logo from fbcon_init+0x494/0x5ac
  fbcon_init from visual_init+0xc0/0x108
  visual_init from do_bind_con_driver+0x1b8/0x3a8
  do_bind_con_driver from do_take_over_console+0x13c/0x1e8
  do_take_over_console from do_fbcon_takeover+0x70/0xd0
  do_fbcon_takeover from fbcon_fb_registered+0x19c/0x1ac
  fbcon_fb_registered from register_framebuffer+0x1ec/0x2ec
  register_framebuffer from 
__drm_fb_helper_initial_config_and_unlock+0x3f0/0x5b8
  __drm_fb_helper_initial_config_and_unlock from 
drm_fbdev_client_hotplug+0xbc/0x120
  drm_fbdev_client_hotplug from drm_fbdev_generic_setup+0x88/0x174
  drm_fbdev_generic_setup from vc4_drm_bind+0x1fc/0x294 [vc4]
  vc4_drm_bind [vc4] from try_to_bring_up_aggregate_device+0x160/0x1bc
  try_to_bring_up_aggregate_device from 
component_master_add_with_match+0xc4/0xf8
  component_master_add_with_match from vc4_platform_drm_probe+0xa0/0xc0 
[vc4]
  vc4_platform_drm_probe [vc4] from platform_probe+0x5c/0xb8
  platform_probe from really_probe+0xc8/0x2f0
  really_probe from __driver_probe_device+0x84/0xe4
  __driver_probe_device from driver_probe_device+0x30/0x104
  driver_probe_device from __driver_attach+0x90/0x174
  __driver_attach from bus_for_each_dev+0x70/0xb0
  bus_for_each_dev from bus_add_driver+0x164/0x1f0
  bus_add_driver from driver_register+0x88/0x11c
  driver_register from vc4_drm_register+0x44/0x1000 [vc4]
  vc4_drm_register [vc4] from do_one_initcall+0x40/0x1e0
  do_one_initcall from do_init_module+0x44/0x1d4
  do_init_module from sys_finit_module+0xbc/0xf8
  sys_finit_module from ret_fast_syscall+0x0/0x54
Exception stack(0xf0d85fa8 to 0xf0d85ff0)
...
---[ end trace  ]---
Console: switching to colour frame buffer device 90x30

It looks that at least the VC4 DRM and Meson DRM drivers needs some 
adjustments to avoid this warning. Am I right?


> ---
>   drivers/gpu/drm/drm_fb_helper.c | 10 +-
>   drivers/video/fbdev/core/fb_defio.c | 16 
>   include/linux/fb.h  |  1 +
>   3 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index cdbf03e941b2b..fbb9088f7defc 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -599,9 +599,17 @@ static void drm_fb_helper_add_damage_clip(struct 
> drm_fb_helper *helper, u32 x, u
>   static void drm_fb_helper_damage(struct drm_fb_helper *helper, u32 x, u32 y,
>u32 width, u32 height)
>   {
> + struct drm_device *dev = helper->dev;
> + struct fb_info *info = helper->info;
> +
>   drm_fb_helper_add_damage_clip(helper, x, y, width, height);