Skip to content

Commit

Permalink
media: bcm2835-unicam: Fix bug in buffer swapping logic
Browse files Browse the repository at this point in the history
If multiple sets of interrupts occur simultaneously, it may be unsafe
to swap buffers, as the hardware may already be re-using the current
buffers. In such cases, avoid swapping buffers, and wait for the next
opportunity at the Frame End interrupt to signal completion.

Additionally, check the packet compare status when watching for frame
end for buffers swaps, as this could also signify a frame end event.

Signed-off-by: Naushir Patuck <[email protected]>
  • Loading branch information
naushir authored and pelwell committed Mar 5, 2021
1 parent 95b18fa commit c1cfa73
Showing 1 changed file with 18 additions and 3 deletions.
21 changes: 18 additions & 3 deletions drivers/media/platform/bcm2835/bcm2835-unicam.c
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,7 @@ static irqreturn_t unicam_isr(int irq, void *dev)
unsigned int sequence = unicam->sequence;
unsigned int i;
u32 ista, sta;
bool fe;
u64 ts;

sta = reg_read(unicam, UNICAM_STA);
Expand All @@ -814,13 +815,19 @@ static irqreturn_t unicam_isr(int irq, void *dev)
if (!(sta & (UNICAM_IS | UNICAM_PI0)))
return IRQ_HANDLED;

/*
* Look for either the Frame End interrupt or the Packet Capture status
* to signal a frame end.
*/
fe = (ista & UNICAM_FEI || sta & UNICAM_PI0);

/*
* We must run the frame end handler first. If we have a valid next_frm
* and we get a simultaneout FE + FS interrupt, running the FS handler
* first would null out the next_frm ptr and we would have lost the
* buffer forever.
*/
if (ista & UNICAM_FEI || sta & UNICAM_PI0) {
if (fe) {
/*
* Ensure we have swapped buffers already as we can't
* stop the peripheral. If no buffer is available, use a
Expand All @@ -831,7 +838,15 @@ static irqreturn_t unicam_isr(int irq, void *dev)
if (!unicam->node[i].streaming)
continue;

if (unicam->node[i].cur_frm)
/*
* If cur_frm == next_frm, it means we have not had
* a chance to swap buffers, likely due to having
* multiple interrupts occurring simultaneously (like FE
* + FS + LS). In this case, we cannot signal the buffer
* as complete, as the HW will reuse that buffer.
*/
if (unicam->node[i].cur_frm &&
unicam->node[i].cur_frm != unicam->node[i].next_frm)
unicam_process_buffer_complete(&unicam->node[i],
sequence);
unicam->node[i].cur_frm = unicam->node[i].next_frm;
Expand Down Expand Up @@ -868,7 +883,7 @@ static irqreturn_t unicam_isr(int irq, void *dev)
* where the HW does not actually swap it if the new frame has
* already started.
*/
if (ista & (UNICAM_FSI | UNICAM_LCI) && !(ista & UNICAM_FEI)) {
if (ista & (UNICAM_FSI | UNICAM_LCI) && !fe) {
for (i = 0; i < ARRAY_SIZE(unicam->node); i++) {
if (!unicam->node[i].streaming)
continue;
Expand Down

0 comments on commit c1cfa73

Please sign in to comment.