On Wed, Jun 18, 2025 at 06:11:23PM +0200, Jacopo Mondi wrote:
> On Wed, Jun 18, 2025 at 05:01:49AM +0300, Laurent Pinchart wrote:
> > A few functions have documented locking conventions. Documentation is
> > nice, but runtime checks are better. Enforce the conventions with
> > lockdep assertions.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > ---
> > drivers/media/platform/renesas/vsp1/vsp1_dl.c | 5 +++++
> > drivers/media/platform/renesas/vsp1/vsp1_pipe.c | 3 +++
> > 2 files changed, 8 insertions(+)
> >
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_dl.c b/drivers/media/platform/renesas/vsp1/vsp1_dl.c
> > index c660f8539ff5..d732b4ed1180 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_dl.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_dl.c
> > @@ -10,6 +10,7 @@
> > #include <linux/device.h>
> > #include <linux/dma-mapping.h>
> > #include <linux/gfp.h>
> > +#include <linux/lockdep.h>
> > #include <linux/refcount.h>
> > #include <linux/slab.h>
> > #include <linux/workqueue.h>
> > @@ -612,6 +613,8 @@ struct vsp1_dl_list *vsp1_dl_list_get(struct vsp1_dl_manager *dlm)
> > struct vsp1_dl_list *dl = NULL;
> > unsigned long flags;
> >
> > + lockdep_assert_not_held(&dlm->lock);
> > +
> > spin_lock_irqsave(&dlm->lock, flags);
> >
> > if (!list_empty(&dlm->free)) {
> > @@ -639,6 +642,8 @@ static void __vsp1_dl_list_put(struct vsp1_dl_list *dl)
> > if (!dl)
> > return;
> >
> > + lockdep_assert_held(&dl->dlm->lock);
> > +
>
> Is it intentional to place this after the !dl check ? Is this to avoid
> a lockdep warning in case !dl ?
The lockdep call derefereces dl.
> Anyway
> Reviewed-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
>
> > /*
> > * Release any linked display-lists which were chained for a single
> > * hardware operation.
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
> > index 3cbb768cf6ad..5d769cc42fe1 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
> > @@ -9,6 +9,7 @@
> >
> > #include <linux/delay.h>
> > #include <linux/list.h>
> > +#include <linux/lockdep.h>
> > #include <linux/sched.h>
> > #include <linux/wait.h>
> >
> > @@ -473,6 +474,8 @@ void vsp1_pipeline_run(struct vsp1_pipeline *pipe)
> > {
> > struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
> >
> > + lockdep_assert_held(&pipe->irqlock);
> > +
> > if (pipe->state == VSP1_PIPELINE_STOPPED) {
> > vsp1_write(vsp1, VI6_CMD(pipe->output->entity.index),
> > VI6_CMD_STRCMD);
> >
> > base-commit: d20469375306163719ee458dd83b7d6c1c93d4d1
--
Regards,
Laurent Pinchart
