diff mbox series

media: renesas: vsp1: Print debug message to diagnose validation failure

Message ID 20240618194140.26788-1-laurent.pinchart+renesas@ideasonboard.com (mailing list archive)
State New
Headers
Series media: renesas: vsp1: Print debug message to diagnose validation failure |

Commit Message

Laurent Pinchart June 18, 2024, 7:41 p.m. UTC 
  When formats don't match between a subdev and a connected video device,
starting streaming returns an error without giving the user any
indication as to what went wrong. To help debugging pipeline
misconfigurations, add a debug message that indicates the cause of the
failure.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/vsp1/vsp1_video.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)


base-commit: 91798162245991e26949ef62851719bb2177a9c2

Comments

Jacopo Mondi June 19, 2024, 7:24 a.m. UTC | #1 
Hi Laurent

On Tue, Jun 18, 2024 at 10:41:40PM GMT, Laurent Pinchart wrote:
> When formats don't match between a subdev and a connected video device,
> starting streaming returns an error without giving the user any
> indication as to what went wrong. To help debugging pipeline
> misconfigurations, add a debug message that indicates the cause of the
> failure.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Doesn't v4l2-subdev.c give you the same information if you enable
debug there ?

> ---
>  drivers/media/platform/renesas/vsp1/vsp1_video.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> index 5a9cb0e5640e..d6f2739456bf 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> @@ -78,8 +78,14 @@ static int vsp1_video_verify_format(struct vsp1_video *video)
>
>  	if (video->rwpf->fmtinfo->mbus != fmt.format.code ||
>  	    video->rwpf->format.height != fmt.format.height ||
> -	    video->rwpf->format.width != fmt.format.width)
> +	    video->rwpf->format.width != fmt.format.width) {
> +		dev_dbg(video->vsp1->dev,
> +			"Format mismatch: 0x%04x/%ux%u != 0x%04x/%ux%u\n",
> +			video->rwpf->fmtinfo->mbus, video->rwpf->format.width,
> +			video->rwpf->format.height, fmt.format.code,
> +			fmt.format.width, fmt.format.height);
>  		return -EINVAL;
> +	}
>
>  	return 0;
>  }
>
> base-commit: 91798162245991e26949ef62851719bb2177a9c2
> --
> Regards,
>
> Laurent Pinchart
>
Kieran Bingham June 19, 2024, 9:19 a.m. UTC | #2 
Quoting Jacopo Mondi (2024-06-19 08:24:09)
> Hi Laurent
> 
> On Tue, Jun 18, 2024 at 10:41:40PM GMT, Laurent Pinchart wrote:
> > When formats don't match between a subdev and a connected video device,
> > starting streaming returns an error without giving the user any
> > indication as to what went wrong. To help debugging pipeline
> > misconfigurations, add a debug message that indicates the cause of the
> > failure.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> Doesn't v4l2-subdev.c give you the same information if you enable
> debug there ?

I think it would, except that this is being called from streamon - not the
link-validation.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> 
> > ---
> >  drivers/media/platform/renesas/vsp1/vsp1_video.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> > index 5a9cb0e5640e..d6f2739456bf 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> > @@ -78,8 +78,14 @@ static int vsp1_video_verify_format(struct vsp1_video *video)
> >
> >       if (video->rwpf->fmtinfo->mbus != fmt.format.code ||
> >           video->rwpf->format.height != fmt.format.height ||
> > -         video->rwpf->format.width != fmt.format.width)
> > +         video->rwpf->format.width != fmt.format.width) {
> > +             dev_dbg(video->vsp1->dev,
> > +                     "Format mismatch: 0x%04x/%ux%u != 0x%04x/%ux%u\n",
> > +                     video->rwpf->fmtinfo->mbus, video->rwpf->format.width,
> > +                     video->rwpf->format.height, fmt.format.code,
> > +                     fmt.format.width, fmt.format.height);
> >               return -EINVAL;
> > +     }
> >
> >       return 0;
> >  }
> >
> > base-commit: 91798162245991e26949ef62851719bb2177a9c2
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
Laurent Pinchart June 19, 2024, 10:49 a.m. UTC | #3 
On Wed, Jun 19, 2024 at 10:19:29AM +0100, Kieran Bingham wrote:
> Quoting Jacopo Mondi (2024-06-19 08:24:09)
> > On Tue, Jun 18, 2024 at 10:41:40PM GMT, Laurent Pinchart wrote:
> > > When formats don't match between a subdev and a connected video device,
> > > starting streaming returns an error without giving the user any
> > > indication as to what went wrong. To help debugging pipeline
> > > misconfigurations, add a debug message that indicates the cause of the
> > > failure.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > 
> > Doesn't v4l2-subdev.c give you the same information if you enable
> > debug there ?
> 
> I think it would, except that this is being called from streamon - not the
> link-validation.

It does for subdev-to-subdev links validated by
v4l2_subdev_link_validate(). This is for the subdev-to-video link,
validated manually by the driver.

> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> > > ---
> > >  drivers/media/platform/renesas/vsp1/vsp1_video.c | 8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> > > index 5a9cb0e5640e..d6f2739456bf 100644
> > > --- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
> > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> > > @@ -78,8 +78,14 @@ static int vsp1_video_verify_format(struct vsp1_video *video)
> > >
> > >       if (video->rwpf->fmtinfo->mbus != fmt.format.code ||
> > >           video->rwpf->format.height != fmt.format.height ||
> > > -         video->rwpf->format.width != fmt.format.width)
> > > +         video->rwpf->format.width != fmt.format.width) {
> > > +             dev_dbg(video->vsp1->dev,
> > > +                     "Format mismatch: 0x%04x/%ux%u != 0x%04x/%ux%u\n",
> > > +                     video->rwpf->fmtinfo->mbus, video->rwpf->format.width,
> > > +                     video->rwpf->format.height, fmt.format.code,
> > > +                     fmt.format.width, fmt.format.height);
> > >               return -EINVAL;
> > > +     }
> > >
> > >       return 0;
> > >  }
> > >
> > > base-commit: 91798162245991e26949ef62851719bb2177a9c2
diff mbox series

Patch

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
index 5a9cb0e5640e..d6f2739456bf 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
@@ -78,8 +78,14 @@  static int vsp1_video_verify_format(struct vsp1_video *video)
 
 	if (video->rwpf->fmtinfo->mbus != fmt.format.code ||
 	    video->rwpf->format.height != fmt.format.height ||
-	    video->rwpf->format.width != fmt.format.width)
+	    video->rwpf->format.width != fmt.format.width) {
+		dev_dbg(video->vsp1->dev,
+			"Format mismatch: 0x%04x/%ux%u != 0x%04x/%ux%u\n",
+			video->rwpf->fmtinfo->mbus, video->rwpf->format.width,
+			video->rwpf->format.height, fmt.format.code,
+			fmt.format.width, fmt.format.height);
 		return -EINVAL;
+	}
 
 	return 0;
 }