Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

drm/panthor: Extend IRQ helpers for mask modification/restoration

The current IRQ helpers do not guarantee mutual exclusion that covers
the entire transaction from accessing the mask member and modifying the
mask register.

This makes it hard, if not impossible, to implement mask modification
helpers that may change one of these outside the normal
suspend/resume/isr code paths.

Add a spinlock to struct panthor_irq that protects both the mask member
and register. Acquire it in all code paths that access these, but drop
it before processing the threaded handler function. Then, add the
aforementioned new helpers: enable_events, and disable_events. They work
by ORing and NANDing the mask bits.

resume is changed to no longer have a mask passed, as pirq->mask is
supposed to be the user-requested mask now, rather than a mirror of the
INT_MASK register contents. Users of the resume helper are adjusted
accordingly, including a rather painful refactor in panthor_mmu.c.

In panthor_mmu.c, the bespoke mask modification is excised, and replaced
with enable_events/disable_events in as_enable/as_disable.

Co-developed-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
Reviewed-by: Steven Price <steven.price@arm.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Link: https://patch.msgid.link/20260116-panthor-tracepoints-v10-2-d925986e3d1b@collabora.com

authored by

Nicolas Frattaroli and committed by
Boris Brezillon
c5bf1d4e 0b2d8667

+97 -41
+69 -15
drivers/gpu/drm/panthor/panthor_device.h
··· 84 84 /** @irq: IRQ number. */ 85 85 int irq; 86 86 87 - /** @mask: Current mask being applied to xxx_INT_MASK. */ 87 + /** @mask: Values to write to xxx_INT_MASK if active. */ 88 88 u32 mask; 89 + 90 + /** 91 + * @mask_lock: protects modifications to _INT_MASK and @mask. 92 + * 93 + * In paths where _INT_MASK is updated based on a state 94 + * transition/check, it's crucial for the state update/check to be 95 + * inside the locked section, otherwise it introduces a race window 96 + * leading to potential _INT_MASK inconsistencies. 97 + */ 98 + spinlock_t mask_lock; 89 99 90 100 /** @state: one of &enum panthor_irq_state reflecting the current state. */ 91 101 atomic_t state; ··· 435 425 if (!gpu_read(ptdev, __reg_prefix ## _INT_STAT)) \ 436 426 return IRQ_NONE; \ 437 427 \ 428 + guard(spinlock_irqsave)(&pirq->mask_lock); \ 438 429 old_state = atomic_cmpxchg(&pirq->state, \ 439 430 PANTHOR_IRQ_STATE_ACTIVE, \ 440 431 PANTHOR_IRQ_STATE_PROCESSING); \ ··· 450 439 { \ 451 440 struct panthor_irq *pirq = data; \ 452 441 struct panthor_device *ptdev = pirq->ptdev; \ 453 - enum panthor_irq_state old_state; \ 454 442 irqreturn_t ret = IRQ_NONE; \ 455 443 \ 456 444 while (true) { \ 445 + /* It's safe to access pirq->mask without the lock held here. If a new \ 446 + * event gets added to the mask and the corresponding IRQ is pending, \ 447 + * we'll process it right away instead of adding an extra raw -> threaded \ 448 + * round trip. If an event is removed and the status bit is set, it will \ 449 + * be ignored, just like it would have been if the mask had been adjusted \ 450 + * right before the HW event kicks in. TLDR; it's all expected races we're \ 451 + * covered for. \ 452 + */ \ 457 453 u32 status = gpu_read(ptdev, __reg_prefix ## _INT_RAWSTAT) & pirq->mask; \ 458 454 \ 459 455 if (!status) \ ··· 470 452 ret = IRQ_HANDLED; \ 471 453 } \ 472 454 \ 473 - old_state = atomic_cmpxchg(&pirq->state, \ 474 - PANTHOR_IRQ_STATE_PROCESSING, \ 475 - PANTHOR_IRQ_STATE_ACTIVE); \ 476 - if (old_state == PANTHOR_IRQ_STATE_PROCESSING) \ 477 - gpu_write(ptdev, __reg_prefix ## _INT_MASK, pirq->mask); \ 455 + scoped_guard(spinlock_irqsave, &pirq->mask_lock) { \ 456 + enum panthor_irq_state old_state; \ 457 + \ 458 + old_state = atomic_cmpxchg(&pirq->state, \ 459 + PANTHOR_IRQ_STATE_PROCESSING, \ 460 + PANTHOR_IRQ_STATE_ACTIVE); \ 461 + if (old_state == PANTHOR_IRQ_STATE_PROCESSING) \ 462 + gpu_write(ptdev, __reg_prefix ## _INT_MASK, pirq->mask); \ 463 + } \ 478 464 \ 479 465 return ret; \ 480 466 } \ 481 467 \ 482 468 static inline void panthor_ ## __name ## _irq_suspend(struct panthor_irq *pirq) \ 483 469 { \ 484 - pirq->mask = 0; \ 485 - gpu_write(pirq->ptdev, __reg_prefix ## _INT_MASK, 0); \ 486 - atomic_set(&pirq->state, PANTHOR_IRQ_STATE_SUSPENDING); \ 470 + scoped_guard(spinlock_irqsave, &pirq->mask_lock) { \ 471 + atomic_set(&pirq->state, PANTHOR_IRQ_STATE_SUSPENDING); \ 472 + gpu_write(pirq->ptdev, __reg_prefix ## _INT_MASK, 0); \ 473 + } \ 487 474 synchronize_irq(pirq->irq); \ 488 475 atomic_set(&pirq->state, PANTHOR_IRQ_STATE_SUSPENDED); \ 489 476 } \ 490 477 \ 491 - static inline void panthor_ ## __name ## _irq_resume(struct panthor_irq *pirq, u32 mask) \ 478 + static inline void panthor_ ## __name ## _irq_resume(struct panthor_irq *pirq) \ 492 479 { \ 493 - pirq->mask = mask; \ 480 + guard(spinlock_irqsave)(&pirq->mask_lock); \ 481 + \ 494 482 atomic_set(&pirq->state, PANTHOR_IRQ_STATE_ACTIVE); \ 495 - gpu_write(pirq->ptdev, __reg_prefix ## _INT_CLEAR, mask); \ 496 - gpu_write(pirq->ptdev, __reg_prefix ## _INT_MASK, mask); \ 483 + gpu_write(pirq->ptdev, __reg_prefix ## _INT_CLEAR, pirq->mask); \ 484 + gpu_write(pirq->ptdev, __reg_prefix ## _INT_MASK, pirq->mask); \ 497 485 } \ 498 486 \ 499 487 static int panthor_request_ ## __name ## _irq(struct panthor_device *ptdev, \ ··· 508 484 { \ 509 485 pirq->ptdev = ptdev; \ 510 486 pirq->irq = irq; \ 511 - panthor_ ## __name ## _irq_resume(pirq, mask); \ 487 + pirq->mask = mask; \ 488 + spin_lock_init(&pirq->mask_lock); \ 489 + panthor_ ## __name ## _irq_resume(pirq); \ 512 490 \ 513 491 return devm_request_threaded_irq(ptdev->base.dev, irq, \ 514 492 panthor_ ## __name ## _irq_raw_handler, \ 515 493 panthor_ ## __name ## _irq_threaded_handler, \ 516 494 IRQF_SHARED, KBUILD_MODNAME "-" # __name, \ 517 495 pirq); \ 496 + } \ 497 + \ 498 + static inline void panthor_ ## __name ## _irq_enable_events(struct panthor_irq *pirq, u32 mask) \ 499 + { \ 500 + guard(spinlock_irqsave)(&pirq->mask_lock); \ 501 + pirq->mask |= mask; \ 502 + \ 503 + /* The only situation where we need to write the new mask is if the IRQ is active. \ 504 + * If it's being processed, the mask will be restored for us in _irq_threaded_handler() \ 505 + * on the PROCESSING -> ACTIVE transition. \ 506 + * If the IRQ is suspended/suspending, the mask is restored at resume time. \ 507 + */ \ 508 + if (atomic_read(&pirq->state) == PANTHOR_IRQ_STATE_ACTIVE) \ 509 + gpu_write(pirq->ptdev, __reg_prefix ## _INT_MASK, pirq->mask); \ 510 + } \ 511 + \ 512 + static inline void panthor_ ## __name ## _irq_disable_events(struct panthor_irq *pirq, u32 mask)\ 513 + { \ 514 + guard(spinlock_irqsave)(&pirq->mask_lock); \ 515 + pirq->mask &= ~mask; \ 516 + \ 517 + /* The only situation where we need to write the new mask is if the IRQ is active. \ 518 + * If it's being processed, the mask will be restored for us in _irq_threaded_handler() \ 519 + * on the PROCESSING -> ACTIVE transition. \ 520 + * If the IRQ is suspended/suspending, the mask is restored at resume time. \ 521 + */ \ 522 + if (atomic_read(&pirq->state) == PANTHOR_IRQ_STATE_ACTIVE) \ 523 + gpu_write(pirq->ptdev, __reg_prefix ## _INT_MASK, pirq->mask); \ 518 524 } 519 525 520 526 extern struct workqueue_struct *panthor_cleanup_wq;
+2 -1
drivers/gpu/drm/panthor/panthor_fw.c
··· 1080 1080 bool timedout = false; 1081 1081 1082 1082 ptdev->fw->booted = false; 1083 - panthor_job_irq_resume(&ptdev->fw->irq, ~0); 1083 + panthor_job_irq_enable_events(&ptdev->fw->irq, ~0); 1084 + panthor_job_irq_resume(&ptdev->fw->irq); 1084 1085 gpu_write(ptdev, MCU_CONTROL, MCU_CONTROL_AUTO); 1085 1086 1086 1087 if (!wait_event_timeout(ptdev->fw->req_waitqueue,
+1 -1
drivers/gpu/drm/panthor/panthor_gpu.c
··· 395 395 */ 396 396 void panthor_gpu_resume(struct panthor_device *ptdev) 397 397 { 398 - panthor_gpu_irq_resume(&ptdev->gpu->irq, GPU_INTERRUPTS_MASK); 398 + panthor_gpu_irq_resume(&ptdev->gpu->irq); 399 399 panthor_hw_l2_power_on(ptdev); 400 400 } 401 401
+24 -23
drivers/gpu/drm/panthor/panthor_mmu.c
··· 562 562 return region_width | *region_start; 563 563 } 564 564 565 + static u32 panthor_mmu_as_fault_mask(struct panthor_device *ptdev, u32 as) 566 + { 567 + return BIT(as); 568 + } 569 + 570 + /* Forward declaration to call helpers within as_enable/disable */ 571 + static void panthor_mmu_irq_handler(struct panthor_device *ptdev, u32 status); 572 + PANTHOR_IRQ_HANDLER(mmu, MMU, panthor_mmu_irq_handler); 573 + 565 574 static int panthor_mmu_as_enable(struct panthor_device *ptdev, u32 as_nr, 566 575 u64 transtab, u64 transcfg, u64 memattr) 567 576 { 577 + panthor_mmu_irq_enable_events(&ptdev->mmu->irq, 578 + panthor_mmu_as_fault_mask(ptdev, as_nr)); 579 + 568 580 gpu_write64(ptdev, AS_TRANSTAB(as_nr), transtab); 569 581 gpu_write64(ptdev, AS_MEMATTR(as_nr), memattr); 570 582 gpu_write64(ptdev, AS_TRANSCFG(as_nr), transcfg); ··· 591 579 int ret; 592 580 593 581 lockdep_assert_held(&ptdev->mmu->as.slots_lock); 582 + 583 + panthor_mmu_irq_disable_events(&ptdev->mmu->irq, 584 + panthor_mmu_as_fault_mask(ptdev, as_nr)); 594 585 595 586 /* Flush+invalidate RW caches, invalidate RO ones. */ 596 587 ret = panthor_gpu_flush_caches(ptdev, CACHE_CLEAN | CACHE_INV, ··· 625 610 { 626 611 /* Bits 16 to 31 mean REQ_COMPLETE. */ 627 612 return value & GENMASK(15, 0); 628 - } 629 - 630 - static u32 panthor_mmu_as_fault_mask(struct panthor_device *ptdev, u32 as) 631 - { 632 - return BIT(as); 633 613 } 634 614 635 615 /** ··· 680 670 struct io_pgtable_cfg *cfg = &io_pgtable_ops_to_pgtable(vm->pgtbl_ops)->cfg; 681 671 int ret = 0, as, cookie; 682 672 u64 transtab, transcfg; 673 + u32 fault_mask; 683 674 684 675 if (!drm_dev_enter(&ptdev->base, &cookie)) 685 676 return -ENODEV; ··· 754 743 /* If the VM is re-activated, we clear the fault. */ 755 744 vm->unhandled_fault = false; 756 745 757 - /* Unhandled pagefault on this AS, clear the fault and re-enable interrupts 758 - * before enabling the AS. 746 + /* Unhandled pagefault on this AS, clear the fault and enable the AS, 747 + * which re-enables interrupts. 759 748 */ 760 - if (ptdev->mmu->as.faulty_mask & panthor_mmu_as_fault_mask(ptdev, as)) { 761 - gpu_write(ptdev, MMU_INT_CLEAR, panthor_mmu_as_fault_mask(ptdev, as)); 762 - ptdev->mmu->as.faulty_mask &= ~panthor_mmu_as_fault_mask(ptdev, as); 763 - ptdev->mmu->irq.mask |= panthor_mmu_as_fault_mask(ptdev, as); 764 - gpu_write(ptdev, MMU_INT_MASK, ~ptdev->mmu->as.faulty_mask); 749 + fault_mask = panthor_mmu_as_fault_mask(ptdev, as); 750 + if (ptdev->mmu->as.faulty_mask & fault_mask) { 751 + gpu_write(ptdev, MMU_INT_CLEAR, fault_mask); 752 + ptdev->mmu->as.faulty_mask &= ~fault_mask; 765 753 } 766 754 767 755 /* The VM update is guarded by ::op_lock, which we take at the beginning ··· 1718 1708 while (status) { 1719 1709 u32 as = ffs(status | (status >> 16)) - 1; 1720 1710 u32 mask = panthor_mmu_as_fault_mask(ptdev, as); 1721 - u32 new_int_mask; 1722 1711 u64 addr; 1723 1712 u32 fault_status; 1724 1713 u32 exception_type; ··· 1735 1726 mutex_lock(&ptdev->mmu->as.slots_lock); 1736 1727 1737 1728 ptdev->mmu->as.faulty_mask |= mask; 1738 - new_int_mask = 1739 - panthor_mmu_fault_mask(ptdev, ~ptdev->mmu->as.faulty_mask); 1740 1729 1741 1730 /* terminal fault, print info about the fault */ 1742 1731 drm_err(&ptdev->base, ··· 1758 1751 */ 1759 1752 gpu_write(ptdev, MMU_INT_CLEAR, mask); 1760 1753 1761 - /* Ignore MMU interrupts on this AS until it's been 1762 - * re-enabled. 1763 - */ 1764 - ptdev->mmu->irq.mask = new_int_mask; 1765 - 1766 1754 if (ptdev->mmu->as.slots[as].vm) 1767 1755 ptdev->mmu->as.slots[as].vm->unhandled_fault = true; 1768 1756 ··· 1772 1770 if (has_unhandled_faults) 1773 1771 panthor_sched_report_mmu_fault(ptdev); 1774 1772 } 1775 - PANTHOR_IRQ_HANDLER(mmu, MMU, panthor_mmu_irq_handler); 1776 1773 1777 1774 /** 1778 1775 * panthor_mmu_suspend() - Suspend the MMU logic ··· 1816 1815 ptdev->mmu->as.faulty_mask = 0; 1817 1816 mutex_unlock(&ptdev->mmu->as.slots_lock); 1818 1817 1819 - panthor_mmu_irq_resume(&ptdev->mmu->irq, panthor_mmu_fault_mask(ptdev, ~0)); 1818 + panthor_mmu_irq_resume(&ptdev->mmu->irq); 1820 1819 } 1821 1820 1822 1821 /** ··· 1870 1869 1871 1870 mutex_unlock(&ptdev->mmu->as.slots_lock); 1872 1871 1873 - panthor_mmu_irq_resume(&ptdev->mmu->irq, panthor_mmu_fault_mask(ptdev, ~0)); 1872 + panthor_mmu_irq_resume(&ptdev->mmu->irq); 1874 1873 1875 1874 /* Restart the VM_BIND queues. */ 1876 1875 mutex_lock(&ptdev->mmu->vm.lock);
+1 -1
drivers/gpu/drm/panthor/panthor_pwr.c
··· 545 545 if (!ptdev->pwr) 546 546 return; 547 547 548 - panthor_pwr_irq_resume(&ptdev->pwr->irq, PWR_INTERRUPTS_MASK); 548 + panthor_pwr_irq_resume(&ptdev->pwr->irq); 549 549 }