[Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events (v3) -> v4
On 12.11.2015 14:48, Thierry Reding wrote: > On Wed, Nov 11, 2015 at 09:12:33PM +0100, poma wrote: >> On 10.11.2015 17:25, Mario Kleiner wrote: >>> On 11/10/2015 05:00 PM, Thierry Reding wrote: On Tue, Nov 10, 2015 at 03:54:52PM +0100, Mario Kleiner wrote: > From: Daniel Vetter > > Apparently pre-nv50 pageflip events happen before the actual vblank > period. Therefore that functionality got semi-disabled in > > commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28 > Author: Mario Kleiner > Date: Tue May 13 00:42:08 2014 +0200 > > drm/nouveau/kms/nv04-nv40: fix pageflip events via special case. > > Unfortunately that hack got uprooted in > > commit cc1ef118fc099295ae6aabbacc8af94d8d8885eb > Author: Thierry Reding > Date: Wed Aug 12 17:00:31 2015 +0200 > > drm/irq: Make pipe unsigned and name consistent > > Trigering a warning when trying to sample the vblank timestamp for a > non-existing pipe. There's a few ways to fix this: > > - Open-code the old behaviour, which just enshrines this slight >breakage of the userspace ABI. > > - Revert Mario's commit and again inflict broken timestamps, again not >pretty. > > - Fix this for real by delaying the pageflip TS until the next vblank >interrupt, thereby making it accurate. > > This patch implements the third option. Since having a page flip > interrupt that happens when the pageflip gets armed and not when it > completes in the next vblank seems to be fairly common (older i915 hw > works very similarly) create a new helper to arm vblank events for > such drivers. > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431 > Cc: Thierry Reding > Cc: Mario Kleiner > Cc: Ben Skeggs > Cc: Ilia Mirkin > > v2 (mario): Integrate my own review comments into Daniels patch. > - Fix function prototypes in drmP.h > - Add missing vblank_put() for pageflip completion without > pageflip event. > - Initialize sequence number for queued pageflip event to avoidng > trouble in drm_handle_vblank_events(). > - Remove dead code and spelling fix. > > v3 (mario): Add a signed-off-by and cc stable tag per Ilja's advice. > > Signed-off-by: Daniel Vetter > (v1) Reviewed-by: Mario Kleiner > (v2/v3) Signed-off-by: Mario Kleiner > > Cc: stable at vger.kernel.org # v4.3 > --- > drivers/gpu/drm/drm_irq.c | 54 > ++- > drivers/gpu/drm/nouveau/nouveau_display.c | 19 ++- > include/drm/drmP.h| 4 +++ > 3 files changed, 68 insertions(+), 9 deletions(-) This looks good to me. Let me clean this up a little and submit it to Dave. Thierry >>> >>> Btw., if somebody has a functional old card for testing this, it should >>> be easy to verify if it works on pre-nv50. If it would not work it would >>> deliver the pageflip event 1 frame delayed, so at least on standard >>> nouveau + default DRI2 + default double-buffering the rate for a tight >>> loop of page-flipped swaps should go down to 30 fps on a 60 Hz display, >>> quite noticeable. Afaik we also have Piglit tests for OML_sync_control >>> which would likely fail if this would be broken. >>> >>> Oh and if someone has tips on how to resurrect an old nv-40 PC (booted >>> with BIOS only) graphics card in a MacPro (EFI boot), i wouldn't mind >>> hearing them. It would be nice to still be able to use that card for >>> testing. >>> >>> thanks, >>> -mario >> >> >> [ cut here ] >> WARNING: CPU: 0 PID: 313 at lib/dma-debug.c:1205 check_sync+0x169/0x6e0() >> nouveau :01:00.0: DMA-API: device driver tries to sync DMA memory it has >> not allocated [device address=0xc0bf6468] [size=4096 bytes] >> Modules linked in: nouveau(+) ... >> CPU: 0 PID: 313 Comm: systemd-udevd Not tainted 4.3.0-3.fc22.i686+debug #1 >> ... >> Call Trace: >> [] dump_stack+0x48/0x69 >> [] warn_slowpath_common+0x87/0xc0 >> [] ? check_sync+0x169/0x6e0 >> [] ? check_sync+0x169/0x6e0 >> [] warn_slowpath_fmt+0x3e/0x60 >> [] check_sync+0x169/0x6e0 >> [] debug_dma_sync_single_for_device+0x7d/0x90 >> [] ? ttm_bo_del_sub_from_lru+0x18/0x50 [ttm] >> [] ? text_poke_bp+0xd0/0xd0 >> [] nouveau_bo_sync_for_device+0x8b/0xa0 [nouveau] >> [] nouveau_bo_validate+0x34/0x40 [nouveau] >> [] nouveau_bo_pin+0x188/0x290 [nouveau] >> [] ? nv10_bo_put_tile_region+0x80/0x80 [nouveau] >> [] nouveau_channel_prep+0xfd/0x2c0 [nouveau] >> [] nouveau_channel_new+0x57/0x7a0 [nouveau] >> [] ? kfree+0xdc/0x280 >> [] ? nvif_object_sclass_put+0x12/0x20 [nouveau] >> [] nouveau_drm_load+0x596/0x8d0 [nouveau] >> [] ? trace_hardirqs_on_caller+0x12c/0x1d0 >> [] ? drm_minor_register+0x89/0x120 [drm] >> [] drm_dev_register+0x96/0xa0 [drm] >>
[Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events (v3) -> v4
On Wed, Nov 11, 2015 at 09:12:33PM +0100, poma wrote: > On 10.11.2015 17:25, Mario Kleiner wrote: > > On 11/10/2015 05:00 PM, Thierry Reding wrote: > >> On Tue, Nov 10, 2015 at 03:54:52PM +0100, Mario Kleiner wrote: > >>> From: Daniel Vetter > >>> > >>> Apparently pre-nv50 pageflip events happen before the actual vblank > >>> period. Therefore that functionality got semi-disabled in > >>> > >>> commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28 > >>> Author: Mario Kleiner > >>> Date: Tue May 13 00:42:08 2014 +0200 > >>> > >>> drm/nouveau/kms/nv04-nv40: fix pageflip events via special case. > >>> > >>> Unfortunately that hack got uprooted in > >>> > >>> commit cc1ef118fc099295ae6aabbacc8af94d8d8885eb > >>> Author: Thierry Reding > >>> Date: Wed Aug 12 17:00:31 2015 +0200 > >>> > >>> drm/irq: Make pipe unsigned and name consistent > >>> > >>> Trigering a warning when trying to sample the vblank timestamp for a > >>> non-existing pipe. There's a few ways to fix this: > >>> > >>> - Open-code the old behaviour, which just enshrines this slight > >>>breakage of the userspace ABI. > >>> > >>> - Revert Mario's commit and again inflict broken timestamps, again not > >>>pretty. > >>> > >>> - Fix this for real by delaying the pageflip TS until the next vblank > >>>interrupt, thereby making it accurate. > >>> > >>> This patch implements the third option. Since having a page flip > >>> interrupt that happens when the pageflip gets armed and not when it > >>> completes in the next vblank seems to be fairly common (older i915 hw > >>> works very similarly) create a new helper to arm vblank events for > >>> such drivers. > >>> > >>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431 > >>> Cc: Thierry Reding > >>> Cc: Mario Kleiner > >>> Cc: Ben Skeggs > >>> Cc: Ilia Mirkin > >>> > >>> v2 (mario): Integrate my own review comments into Daniels patch. > >>> - Fix function prototypes in drmP.h > >>> - Add missing vblank_put() for pageflip completion without > >>> pageflip event. > >>> - Initialize sequence number for queued pageflip event to avoidng > >>> trouble in drm_handle_vblank_events(). > >>> - Remove dead code and spelling fix. > >>> > >>> v3 (mario): Add a signed-off-by and cc stable tag per Ilja's advice. > >>> > >>> Signed-off-by: Daniel Vetter > >>> (v1) Reviewed-by: Mario Kleiner > >>> (v2/v3) Signed-off-by: Mario Kleiner > >>> > >>> Cc: stable at vger.kernel.org # v4.3 > >>> --- > >>> drivers/gpu/drm/drm_irq.c | 54 > >>> ++- > >>> drivers/gpu/drm/nouveau/nouveau_display.c | 19 ++- > >>> include/drm/drmP.h| 4 +++ > >>> 3 files changed, 68 insertions(+), 9 deletions(-) > >> > >> This looks good to me. Let me clean this up a little and submit it to > >> Dave. > >> > >> Thierry > >> > > > > Btw., if somebody has a functional old card for testing this, it should > > be easy to verify if it works on pre-nv50. If it would not work it would > > deliver the pageflip event 1 frame delayed, so at least on standard > > nouveau + default DRI2 + default double-buffering the rate for a tight > > loop of page-flipped swaps should go down to 30 fps on a 60 Hz display, > > quite noticeable. Afaik we also have Piglit tests for OML_sync_control > > which would likely fail if this would be broken. > > > > Oh and if someone has tips on how to resurrect an old nv-40 PC (booted > > with BIOS only) graphics card in a MacPro (EFI boot), i wouldn't mind > > hearing them. It would be nice to still be able to use that card for > > testing. > > > > thanks, > > -mario > > > [ cut here ] > WARNING: CPU: 0 PID: 313 at lib/dma-debug.c:1205 check_sync+0x169/0x6e0() > nouveau :01:00.0: DMA-API: device driver tries to sync DMA memory it has > not allocated [device address=0xc0bf6468] [size=4096 bytes] > Modules linked in: nouveau(+) ... > CPU: 0 PID: 313 Comm: systemd-udevd Not tainted 4.3.0-3.fc22.i686+debug #1 > ... > Call Trace: > [] dump_stack+0x48/0x69 > [] warn_slowpath_common+0x87/0xc0 > [] ? check_sync+0x169/0x6e0 > [] ? check_sync+0x169/0x6e0 > [] warn_slowpath_fmt+0x3e/0x60 > [] check_sync+0x169/0x6e0 > [] debug_dma_sync_single_for_device+0x7d/0x90 > [] ? ttm_bo_del_sub_from_lru+0x18/0x50 [ttm] > [] ? text_poke_bp+0xd0/0xd0 > [] nouveau_bo_sync_for_device+0x8b/0xa0 [nouveau] > [] nouveau_bo_validate+0x34/0x40 [nouveau] > [] nouveau_bo_pin+0x188/0x290 [nouveau] > [] ? nv10_bo_put_tile_region+0x80/0x80 [nouveau] > [] nouveau_channel_prep+0xfd/0x2c0 [nouveau] > [] nouveau_channel_new+0x57/0x7a0 [nouveau] > [] ? kfree+0xdc/0x280 > [] ? nvif_object_sclass_put+0x12/0x20 [nouveau] > [] nouveau_drm_load+0x596/0x8d0 [nouveau] > [] ? trace_hardirqs_on_caller+0x12c/0x1d0 > [] ? drm_minor_register+0x89/0x120 [drm] > [] drm_dev_register+0x96/0xa0 [drm] > [] drm_get_pci_dev+0x79/0x1c0 [drm] > [] ? pcibios_set_master+0x4e/0xa0
[Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events (v3) -> v4
On 10.11.2015 17:25, Mario Kleiner wrote: > On 11/10/2015 05:00 PM, Thierry Reding wrote: >> On Tue, Nov 10, 2015 at 03:54:52PM +0100, Mario Kleiner wrote: >>> From: Daniel Vetter >>> >>> Apparently pre-nv50 pageflip events happen before the actual vblank >>> period. Therefore that functionality got semi-disabled in >>> >>> commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28 >>> Author: Mario Kleiner >>> Date: Tue May 13 00:42:08 2014 +0200 >>> >>> drm/nouveau/kms/nv04-nv40: fix pageflip events via special case. >>> >>> Unfortunately that hack got uprooted in >>> >>> commit cc1ef118fc099295ae6aabbacc8af94d8d8885eb >>> Author: Thierry Reding >>> Date: Wed Aug 12 17:00:31 2015 +0200 >>> >>> drm/irq: Make pipe unsigned and name consistent >>> >>> Trigering a warning when trying to sample the vblank timestamp for a >>> non-existing pipe. There's a few ways to fix this: >>> >>> - Open-code the old behaviour, which just enshrines this slight >>>breakage of the userspace ABI. >>> >>> - Revert Mario's commit and again inflict broken timestamps, again not >>>pretty. >>> >>> - Fix this for real by delaying the pageflip TS until the next vblank >>>interrupt, thereby making it accurate. >>> >>> This patch implements the third option. Since having a page flip >>> interrupt that happens when the pageflip gets armed and not when it >>> completes in the next vblank seems to be fairly common (older i915 hw >>> works very similarly) create a new helper to arm vblank events for >>> such drivers. >>> >>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431 >>> Cc: Thierry Reding >>> Cc: Mario Kleiner >>> Cc: Ben Skeggs >>> Cc: Ilia Mirkin >>> >>> v2 (mario): Integrate my own review comments into Daniels patch. >>> - Fix function prototypes in drmP.h >>> - Add missing vblank_put() for pageflip completion without >>> pageflip event. >>> - Initialize sequence number for queued pageflip event to avoidng >>> trouble in drm_handle_vblank_events(). >>> - Remove dead code and spelling fix. >>> >>> v3 (mario): Add a signed-off-by and cc stable tag per Ilja's advice. >>> >>> Signed-off-by: Daniel Vetter >>> (v1) Reviewed-by: Mario Kleiner >>> (v2/v3) Signed-off-by: Mario Kleiner >>> >>> Cc: stable at vger.kernel.org # v4.3 >>> --- >>> drivers/gpu/drm/drm_irq.c | 54 >>> ++- >>> drivers/gpu/drm/nouveau/nouveau_display.c | 19 ++- >>> include/drm/drmP.h| 4 +++ >>> 3 files changed, 68 insertions(+), 9 deletions(-) >> >> This looks good to me. Let me clean this up a little and submit it to >> Dave. >> >> Thierry >> > > Btw., if somebody has a functional old card for testing this, it should > be easy to verify if it works on pre-nv50. If it would not work it would > deliver the pageflip event 1 frame delayed, so at least on standard > nouveau + default DRI2 + default double-buffering the rate for a tight > loop of page-flipped swaps should go down to 30 fps on a 60 Hz display, > quite noticeable. Afaik we also have Piglit tests for OML_sync_control > which would likely fail if this would be broken. > > Oh and if someone has tips on how to resurrect an old nv-40 PC (booted > with BIOS only) graphics card in a MacPro (EFI boot), i wouldn't mind > hearing them. It would be nice to still be able to use that card for > testing. > > thanks, > -mario [ cut here ] WARNING: CPU: 0 PID: 313 at lib/dma-debug.c:1205 check_sync+0x169/0x6e0() nouveau :01:00.0: DMA-API: device driver tries to sync DMA memory it has not allocated [device address=0xc0bf6468] [size=4096 bytes] Modules linked in: nouveau(+) ... CPU: 0 PID: 313 Comm: systemd-udevd Not tainted 4.3.0-3.fc22.i686+debug #1 ... Call Trace: [] dump_stack+0x48/0x69 [] warn_slowpath_common+0x87/0xc0 [] ? check_sync+0x169/0x6e0 [] ? check_sync+0x169/0x6e0 [] warn_slowpath_fmt+0x3e/0x60 [] check_sync+0x169/0x6e0 [] debug_dma_sync_single_for_device+0x7d/0x90 [] ? ttm_bo_del_sub_from_lru+0x18/0x50 [ttm] [] ? text_poke_bp+0xd0/0xd0 [] nouveau_bo_sync_for_device+0x8b/0xa0 [nouveau] [] nouveau_bo_validate+0x34/0x40 [nouveau] [] nouveau_bo_pin+0x188/0x290 [nouveau] [] ? nv10_bo_put_tile_region+0x80/0x80 [nouveau] [] nouveau_channel_prep+0xfd/0x2c0 [nouveau] [] nouveau_channel_new+0x57/0x7a0 [nouveau] [] ? kfree+0xdc/0x280 [] ? nvif_object_sclass_put+0x12/0x20 [nouveau] [] nouveau_drm_load+0x596/0x8d0 [nouveau] [] ? trace_hardirqs_on_caller+0x12c/0x1d0 [] ? drm_minor_register+0x89/0x120 [drm] [] drm_dev_register+0x96/0xa0 [drm] [] drm_get_pci_dev+0x79/0x1c0 [drm] [] ? pcibios_set_master+0x4e/0xa0 [] nouveau_drm_probe+0x1ee/0x220 [nouveau] [] pci_device_probe+0x7b/0xf0 [] ? devices_kset_move_last+0x56/0xa0 [] driver_probe_device+0x204/0x490 [] ? __driver_attach+0x4c/0x90 [] ? pci_match_device+0xd2/0x100 [] __driver_attach+0x81/0x90 [] ? driver_probe_device+0x490/0x490 [] bus_for_e
