tjh.dev / kernel
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.

media: pisp_be: Split jobs creation and scheduling

Currently the 'pispbe_schedule()' function does two things:

1) Tries to assemble a job by inspecting all the video node queues
to make sure all the required buffers are available
2) Submit the job to the hardware

The pispbe_schedule() function is called at:

- video device start_streaming() time
- video device qbuf() time
- irq handler

As assembling a job requires inspecting all queues, it is a rather
time consuming operation which is better not run in IRQ context.

To avoid executing the time consuming job creation in interrupt
context split the job creation and job scheduling in two distinct
operations. When a well-formed job is created, append it to the
newly introduced 'pispbe->job_queue' where it will be dequeued from
by the scheduling routine.

As the per-node 'ready_queue' buffer list is only accessed in vb2 ops
callbacks, protected by the node->queue_lock mutex, it is not necessary
to guard it with a dedicated spinlock so drop it. Also use the
spin_lock_irq() variant in all functions not called from an IRQ context
where the spin_lock_irqsave() version was used.

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>

authored by

Jacopo Mondi and committed by
Hans Verkuil 972eed08 a773b614

+87 -74
+87 -74
drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
··· 12 12 #include <linux/module.h> 13 13 #include <linux/platform_device.h> 14 14 #include <linux/pm_runtime.h> 15 + #include <linux/slab.h> 15 16 #include <media/v4l2-device.h> 16 17 #include <media/v4l2-ioctl.h> 17 18 #include <media/videobuf2-dma-contig.h> ··· 162 161 struct mutex node_lock; 163 162 /* vb2_queue lock */ 164 163 struct mutex queue_lock; 165 - /* Protect pispbe_node->ready_queue and pispbe_buffer->ready_list */ 166 - spinlock_t ready_lock; 167 164 struct list_head ready_queue; 168 165 struct vb2_queue queue; 169 166 struct v4l2_format format; ··· 189 190 190 191 /* Records a job configuration and memory addresses. */ 191 192 struct pispbe_job_descriptor { 193 + struct list_head queue; 194 + struct pispbe_buffer *buffers[PISPBE_NUM_NODES]; 192 195 dma_addr_t hw_dma_addrs[N_HW_ADDRESSES]; 193 196 struct pisp_be_tiles_config *config; 194 197 struct pispbe_hw_enables hw_enables; ··· 216 215 unsigned int sequence; 217 216 u32 streaming_map; 218 217 struct pispbe_job queued_job, running_job; 219 - spinlock_t hw_lock; /* protects "hw_busy" flag and streaming_map */ 218 + /* protects "hw_busy" flag, streaming_map and job_queue */ 219 + spinlock_t hw_lock; 220 220 bool hw_busy; /* non-zero if a job is queued or is being started */ 221 + struct list_head job_queue; 221 222 int irq; 222 223 u32 hw_version; 223 224 u8 done, started; ··· 443 440 * For Output0, Output1, Tdn and Stitch, a buffer only needs to be 444 441 * available if the blocks are enabled in the config. 445 442 * 446 - * Needs to be called with hw_lock held. 443 + * If all the buffers required to form a job are available, append the 444 + * job descriptor to the job queue to be later queued to the HW. 447 445 * 448 446 * Returns 0 if a job has been successfully prepared, < 0 otherwise. 449 447 */ 450 - static int pispbe_prepare_job(struct pispbe_dev *pispbe, 451 - struct pispbe_job_descriptor *job) 448 + static int pispbe_prepare_job(struct pispbe_dev *pispbe) 452 449 { 450 + struct pispbe_job_descriptor __free(kfree) *job = NULL; 453 451 struct pispbe_buffer *buf[PISPBE_NUM_NODES] = {}; 452 + unsigned int streaming_map; 454 453 unsigned int config_index; 455 454 struct pispbe_node *node; 456 - unsigned long flags; 457 455 458 - lockdep_assert_held(&pispbe->hw_lock); 456 + lockdep_assert_irqs_enabled(); 459 457 460 - memset(job, 0, sizeof(struct pispbe_job_descriptor)); 458 + scoped_guard(spinlock_irq, &pispbe->hw_lock) { 459 + static const u32 mask = BIT(CONFIG_NODE) | BIT(MAIN_INPUT_NODE); 461 460 462 - if (((BIT(CONFIG_NODE) | BIT(MAIN_INPUT_NODE)) & 463 - pispbe->streaming_map) != 464 - (BIT(CONFIG_NODE) | BIT(MAIN_INPUT_NODE))) 465 - return -ENODEV; 461 + if ((pispbe->streaming_map & mask) != mask) 462 + return -ENODEV; 463 + 464 + /* 465 + * Take a copy of streaming_map: nodes activated after this 466 + * point are ignored when preparing this job. 467 + */ 468 + streaming_map = pispbe->streaming_map; 469 + } 470 + 471 + job = kzalloc(sizeof(*job), GFP_KERNEL); 472 + if (!job) 473 + return -ENOMEM; 466 474 467 475 node = &pispbe->node[CONFIG_NODE]; 468 - spin_lock_irqsave(&node->ready_lock, flags); 469 476 buf[CONFIG_NODE] = list_first_entry_or_null(&node->ready_queue, 470 477 struct pispbe_buffer, 471 478 ready_list); 472 - if (buf[CONFIG_NODE]) { 473 - list_del(&buf[CONFIG_NODE]->ready_list); 474 - pispbe->queued_job.buf[CONFIG_NODE] = buf[CONFIG_NODE]; 475 - } 476 - spin_unlock_irqrestore(&node->ready_lock, flags); 477 - 478 - /* Exit early if no config buffer has been queued. */ 479 479 if (!buf[CONFIG_NODE]) 480 480 return -ENODEV; 481 + 482 + list_del(&buf[CONFIG_NODE]->ready_list); 483 + job->buffers[CONFIG_NODE] = buf[CONFIG_NODE]; 481 484 482 485 config_index = buf[CONFIG_NODE]->vb.vb2_buf.index; 483 486 job->config = &pispbe->config[config_index]; ··· 504 495 continue; 505 496 506 497 buf[i] = NULL; 507 - if (!(pispbe->streaming_map & BIT(i))) 498 + if (!(streaming_map & BIT(i))) 508 499 continue; 509 500 510 501 if ((!(rgb_en & PISP_BE_RGB_ENABLE_OUTPUT0) && ··· 531 522 node = &pispbe->node[i]; 532 523 533 524 /* Pull a buffer from each V4L2 queue to form the queued job */ 534 - spin_lock_irqsave(&node->ready_lock, flags); 535 525 buf[i] = list_first_entry_or_null(&node->ready_queue, 536 526 struct pispbe_buffer, 537 527 ready_list); 538 528 if (buf[i]) { 539 529 list_del(&buf[i]->ready_list); 540 - pispbe->queued_job.buf[i] = buf[i]; 530 + job->buffers[i] = buf[i]; 541 531 } 542 - spin_unlock_irqrestore(&node->ready_lock, flags); 543 532 544 533 if (!buf[i] && !ignore_buffers) 545 534 goto err_return_buffers; 546 535 } 547 536 548 - pispbe->queued_job.valid = true; 549 - 550 537 /* Convert buffers to DMA addresses for the hardware */ 551 538 pispbe_xlate_addrs(pispbe, job, buf); 539 + 540 + scoped_guard(spinlock_irq, &pispbe->hw_lock) { 541 + list_add_tail(&job->queue, &pispbe->job_queue); 542 + } 543 + 544 + /* Set job to NULL to avoid automatic release due to __free(). */ 545 + job = NULL; 552 546 553 547 return 0; 554 548 ··· 563 551 continue; 564 552 565 553 /* Return the buffer to the ready_list queue */ 566 - spin_lock_irqsave(&n->ready_lock, flags); 567 554 list_add(&buf[i]->ready_list, &n->ready_queue); 568 - spin_unlock_irqrestore(&n->ready_lock, flags); 569 555 } 570 - 571 - memset(&pispbe->queued_job, 0, sizeof(pispbe->queued_job)); 572 556 573 557 return -ENODEV; 574 558 } 575 559 576 560 static void pispbe_schedule(struct pispbe_dev *pispbe, bool clear_hw_busy) 577 561 { 578 - struct pispbe_job_descriptor job; 579 - unsigned long flags; 580 - int ret; 562 + struct pispbe_job_descriptor *job; 581 563 582 - spin_lock_irqsave(&pispbe->hw_lock, flags); 564 + scoped_guard(spinlock_irqsave, &pispbe->hw_lock) { 565 + if (clear_hw_busy) 566 + pispbe->hw_busy = false; 583 567 584 - if (clear_hw_busy) 585 - pispbe->hw_busy = false; 568 + if (pispbe->hw_busy) 569 + return; 586 570 587 - if (pispbe->hw_busy) 588 - goto unlock_and_return; 571 + job = list_first_entry_or_null(&pispbe->job_queue, 572 + struct pispbe_job_descriptor, 573 + queue); 574 + if (!job) 575 + return; 589 576 590 - ret = pispbe_prepare_job(pispbe, &job); 591 - if (ret) 592 - goto unlock_and_return; 577 + list_del(&job->queue); 578 + 579 + for (unsigned int i = 0; i < PISPBE_NUM_NODES; i++) 580 + pispbe->queued_job.buf[i] = job->buffers[i]; 581 + pispbe->queued_job.valid = true; 582 + 583 + pispbe->hw_busy = true; 584 + } 593 585 594 586 /* 595 587 * We can kick the job off without the hw_lock, as this can ··· 601 585 * only when the following job has been queued and an interrupt 602 586 * is rised. 603 587 */ 604 - pispbe->hw_busy = true; 605 - spin_unlock_irqrestore(&pispbe->hw_lock, flags); 606 - 607 - pispbe_queue_job(pispbe, &job); 608 - 609 - return; 610 - 611 - unlock_and_return: 612 - /* No job has been queued, just release the lock and return. */ 613 - spin_unlock_irqrestore(&pispbe->hw_lock, flags); 588 + pispbe_queue_job(pispbe, job); 589 + kfree(job); 614 590 } 615 591 616 592 static void pispbe_isr_jobdone(struct pispbe_dev *pispbe, ··· 854 846 container_of(vbuf, struct pispbe_buffer, vb); 855 847 struct pispbe_node *node = vb2_get_drv_priv(buf->vb2_queue); 856 848 struct pispbe_dev *pispbe = node->pispbe; 857 - unsigned long flags; 858 849 859 850 dev_dbg(pispbe->dev, "%s: for node %s\n", __func__, NODE_NAME(node)); 860 - spin_lock_irqsave(&node->ready_lock, flags); 861 851 list_add_tail(&buffer->ready_list, &node->ready_queue); 862 - spin_unlock_irqrestore(&node->ready_lock, flags); 863 852 864 853 /* 865 854 * Every time we add a buffer, check if there's now some work for the hw 866 855 * to do. 867 856 */ 868 - pispbe_schedule(pispbe, false); 857 + if (!pispbe_prepare_job(pispbe)) 858 + pispbe_schedule(pispbe, false); 869 859 } 870 860 871 861 static int pispbe_node_start_streaming(struct vb2_queue *q, unsigned int count) ··· 871 865 struct pispbe_node *node = vb2_get_drv_priv(q); 872 866 struct pispbe_dev *pispbe = node->pispbe; 873 867 struct pispbe_buffer *buf, *tmp; 874 - unsigned long flags; 875 868 int ret; 876 869 877 870 ret = pm_runtime_resume_and_get(pispbe->dev); 878 871 if (ret < 0) 879 872 goto err_return_buffers; 880 873 881 - spin_lock_irqsave(&pispbe->hw_lock, flags); 882 - node->pispbe->streaming_map |= BIT(node->id); 883 - node->pispbe->sequence = 0; 884 - spin_unlock_irqrestore(&pispbe->hw_lock, flags); 874 + scoped_guard(spinlock_irq, &pispbe->hw_lock) { 875 + node->pispbe->streaming_map |= BIT(node->id); 876 + node->pispbe->sequence = 0; 877 + } 885 878 886 879 dev_dbg(pispbe->dev, "%s: for node %s (count %u)\n", 887 880 __func__, NODE_NAME(node), count); ··· 888 883 node->pispbe->streaming_map); 889 884 890 885 /* Maybe we're ready to run. */ 891 - pispbe_schedule(pispbe, false); 886 + if (!pispbe_prepare_job(pispbe)) 887 + pispbe_schedule(pispbe, false); 892 888 893 889 return 0; 894 890 895 891 err_return_buffers: 896 - spin_lock_irqsave(&pispbe->hw_lock, flags); 897 892 list_for_each_entry_safe(buf, tmp, &node->ready_queue, ready_list) { 898 893 list_del(&buf->ready_list); 899 894 vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED); 900 895 } 901 - spin_unlock_irqrestore(&pispbe->hw_lock, flags); 902 896 903 897 return ret; 904 898 } ··· 906 902 { 907 903 struct pispbe_node *node = vb2_get_drv_priv(q); 908 904 struct pispbe_dev *pispbe = node->pispbe; 905 + struct pispbe_job_descriptor *job, *temp; 909 906 struct pispbe_buffer *buf; 910 - unsigned long flags; 907 + LIST_HEAD(tmp_list); 911 908 912 909 /* 913 910 * Now this is a bit awkward. In a simple M2M device we could just wait ··· 920 915 * This may return buffers out of order. 921 916 */ 922 917 dev_dbg(pispbe->dev, "%s: for node %s\n", __func__, NODE_NAME(node)); 923 - spin_lock_irqsave(&pispbe->hw_lock, flags); 924 918 do { 925 - unsigned long flags1; 926 - 927 - spin_lock_irqsave(&node->ready_lock, flags1); 928 919 buf = list_first_entry_or_null(&node->ready_queue, 929 920 struct pispbe_buffer, 930 921 ready_list); ··· 928 927 list_del(&buf->ready_list); 929 928 vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR); 930 929 } 931 - spin_unlock_irqrestore(&node->ready_lock, flags1); 932 930 } while (buf); 933 - spin_unlock_irqrestore(&pispbe->hw_lock, flags); 934 931 935 932 vb2_wait_for_all_buffers(&node->queue); 936 933 937 - spin_lock_irqsave(&pispbe->hw_lock, flags); 934 + spin_lock_irq(&pispbe->hw_lock); 938 935 pispbe->streaming_map &= ~BIT(node->id); 939 - spin_unlock_irqrestore(&pispbe->hw_lock, flags); 936 + 937 + if (pispbe->streaming_map == 0) { 938 + /* 939 + * If all nodes have stopped streaming release all jobs 940 + * without holding the lock. 941 + */ 942 + list_splice_init(&pispbe->job_queue, &tmp_list); 943 + } 944 + spin_unlock_irq(&pispbe->hw_lock); 945 + 946 + list_for_each_entry_safe(job, temp, &tmp_list, queue) { 947 + list_del(&job->queue); 948 + kfree(job); 949 + } 940 950 941 951 pm_runtime_mark_last_busy(pispbe->dev); 942 952 pm_runtime_put_autosuspend(pispbe->dev); ··· 1405 1393 mutex_init(&node->node_lock); 1406 1394 mutex_init(&node->queue_lock); 1407 1395 INIT_LIST_HEAD(&node->ready_queue); 1408 - spin_lock_init(&node->ready_lock); 1409 1396 1410 1397 node->format.type = node->buf_type; 1411 1398 pispbe_node_def_fmt(node); ··· 1687 1676 pispbe = devm_kzalloc(&pdev->dev, sizeof(*pispbe), GFP_KERNEL); 1688 1677 if (!pispbe) 1689 1678 return -ENOMEM; 1679 + 1680 + INIT_LIST_HEAD(&pispbe->job_queue); 1690 1681 1691 1682 dev_set_drvdata(&pdev->dev, pispbe); 1692 1683 pispbe->dev = &pdev->dev;