Re: [PATCH v3] media: renesas: vsp1: Add underrun debug print

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Tomi,

On Wed, Feb 15, 2023 at 08:52:02AM +0200, Tomi Valkeinen wrote:
> On 15/02/2023 00:15, Laurent Pinchart wrote:
> > On Tue, Feb 14, 2023 at 06:42:23PM +0200, Tomi Valkeinen wrote:
> >> Print underrun interrupts with ratelimited print.
> >>
> >> Note that we don't enable the underrun interrupt. If we have underruns,
> >> we don't want to get flooded with interrupts about them. It's enough to
> >> see that an underrun happened at the end of a frame.
> >>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@xxxxxxxxxxxxxxxx>
> >> ---
> >>
> >> Changes in v3:
> >> - Reset underrun counter when enabling VSP
> >>
> >> I have to say I'm not familiar enough with the VSP driver to say if
> >> these are the correct places where to reset the counters.
> > 
> > It's fine. We could factor it out to a clear function, but it's not
> > worth it if there's nothing else to factor out. It could be done later.
> > 
> >> There's also a
> >> possibility of a race, but my assumption is that we cannot get underrun
> >> interrupts for the WPF we are currently enabling.
> > 
> > It should be fine.
> > 
> >> Also, I realized the underrun counter could be moved to struct
> >> vsp1_rwpf, but as that's used also for RPF, I didn't do that change.
> > 
> > Another option would be to store it in the pipeline structure, as a
> > pipeline has one and only one WPF. What do you think ?
> 
> Hmm, the pipe is allocated and assigned as needed, isn't it? So in the 
> irq handler we might get an underflow with !pipe. We could skip the 
> print in that case, of course.

For display pipelines, the pipeline is embedded in the vsp1_drm_pipeline
structure, itself embedded in the vsp1_drm structure, so it won't come
and go. The WPF's pipe pointer (in the vsp1_entity structure) is set in
vsp1_drm_init() and never reset.

For memory-to-memory pipelines, yes, the pipeline is allocated
dynamically.

Interrupts are not supposed to occur when the pipeline is stopped, but
of course there's real life and race conditions. Underrun notifications
are likely useless in that case so we could indeed skip them.

> Is a pipe allocated every time VSP is started? Or does the allocation 
> normally happen only once? If the former, then if the counter was stored 
> in the pipe, that would handle clearing the counter automatically.

For memory-to-memory pipelines, the allocation happens when the user
calls VIDIOC_STREAMON the first time on any video node in the pipeline
(a pipeline has one or more RPFs and exactly one WPF), and the pipeline
is freed when the last video node is stopped with VIDIOC_STREAMOFF. It
is thus possible to stop and restart the pipeline without the
vsp1_pipeline structure being freed, but in that case, I'd say we don't
have to actually reset the counter. If the user keeps some video nodes
streaming, I think it's acceptable to keep the underrun counter going.

For display pipelines, you'll have to reset the counter manually in
vsp1_du_setup_lif(). It could be done at stop time instead of start time
if desired.

-- 
Regards,

Laurent Pinchart



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux