Re: [PATCH 00/14] Add SOF/EOF sequence logic

["Gjorgji Rosikopulos (Consultant)" <gjorgji.rosikopulos@oss.qualcomm.com>]

On 10/29/2026 4:44 PM, Bryan O'Donoghue wrote:
> On 29/10/2026 13:28, Gjorgji Rosikopulos (Consultant) wrote:
>> Hi Bryan,
>>
>> On 10/29/2026 11:55 AM,bryan.odonoghue@linaro.org wrote:
>>> This series enables SOF AND EOF event tracking making use of both events.
>>> Whilst the code is actually very simple the functionality we want to build
>>> off of that simple code is not.
>>>
>>> - SOF
>>>    Serves two functions. Firstly. Informing user-space of CSID SOF events.
>>>    Standard Linux V4L2_EVENT_FRAME_SYNC is trigger from this event.
>>>    User-space wants to capture this trigger because it allows user-space to
>>>    better align the time a frame begins to events elsewhere such as IMU
>>>    event timestamps.
>>>
>>>    That functionality is directly applicable in this series.
>>>
>>>    However we will also build on the SOF event in later patches to allow us
>>>    to begin programming of shadow-registers for the next camera frame.
>>>
>>>    When the EOF event happens, we switch from one shadow bank to the next.
>>>    This is only possible if you have the SOF event.
>>>
>>>    So SOF serves the immediate purpose of timestamp alignment in user-space
>>>    as well as the 'evil plan' to trigger off of it programming camera
>>>    parameters for the next frame.
>>>
>>> - EOF
>>>    EOF even on its own is useful because it allows us to increment
>>>    vb.sequence for each frame as per the spec not just when we have
>>>    buffer-data.
>>>
>> It is great to finally have V4L2_EVENT_FRAME_SYNC support in CAMSS. 🙂
>>
>> I would like to share my opinion on tagging vb.sequence at EOF.
>>
>> The EOF event is received from the CSID decoder when the EOF short packet
>> is decoded. In contrast, buffer completion is reported when the write
>> master has finished writing the corresponding buffer.
>>
>> Because the processing pipeline includes buffering and line buffers,
>> buf_done on the pixel path may be received after the EOF.
>> Conversely, when cropping is enabled on an output, buf_done may be
>> received before EOF.
>>
>> That is why using EOF to assign the buffer sequence may result in inconsistent
>> sequence numbers across buffers.
>>
>> I would propose:
>>
>> SOF
>> Increment the frame sequence and generate the V4L2_EVENT_FRAME_SYNC event
>> for userspace.
>>
>> REG_UPDATE
>> Once REG_UPDATE occurs, we have a guarantee that the configuration for the
>> new frame has been latched. At this point, we can iterate over all active
>> buffers being processed and assign them the current sequence number.
>> The bus-write active-status register indicates which address is currently being
>> processed by the bus-write engine and could also be used to detect frame skips,
>> if necessary.
>>
>> BUSWR_DONE
>> Complete the corresponding buffer as usual. Optionally, we can verify whether its
>> sequence has already been updated. If it has not, we can determine how the buffer
>> should be handled.
>>
>> This approach ties the buffer sequence to the point at which the frame configuration
>> is latched, rather than to the arrival of EOF, which may not be ordered consistently
>> with buffer completion.
> 
> Come to think of it, I'm not entirely sure an EOF or reg-update 
> additional variable is required.
> 
> SOF.seq should do.
> 
> - Reading SOF.seq @ crop is fine because the SOF.seq always preceds
> - Reading SOF.seq @ buf_done is fine ditto
> - Reading SOF.seq for params return stamping is correct
> - Reading SOF.seq for stats frame correlation is also correct
> 
> We want the EOF signal but we don't need a second seqno to accompany. 
> And with one source of seq truth we don't need to change the point at 
> which we latch since pre-frame-drop SOF < everythign else.

I agree. REG_UPDATE is only necessary if we decide to use the bus-write active-status
register to determine which buffer is currently being processed. This can give real guarantee
that we are not missing something on heavily loaded systems when irq-s were disabled for some time,
in that case sequence may not be in sync anyway...

~Gjorgji