linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] v4l: subdev: Improve link format validation debug messages
@ 2020-10-09 14:02 Sakari Ailus
  2020-10-10 21:22 ` Laurent Pinchart
  0 siblings, 1 reply; 4+ messages in thread
From: Sakari Ailus @ 2020-10-09 14:02 UTC (permalink / raw)
  To: linux-media

The existing link format validation failure debug message in media-entity.c
helped to poinpoint the point of failure but provided no additional
information what's wrong. Tell the user exactly why the validation failed.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
I pulled this old patch from the VC patchset. It can be merged now without
the rest.

 drivers/media/v4l2-core/v4l2-subdev.c | 40 ++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index a7d508e74d6b..e1dfbcc96a4a 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -792,21 +792,47 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd,
 				      struct v4l2_subdev_format *source_fmt,
 				      struct v4l2_subdev_format *sink_fmt)
 {
+	bool pass = true;
+
 	/* The width, height and code must match. */
-	if (source_fmt->format.width != sink_fmt->format.width
-	    || source_fmt->format.height != sink_fmt->format.height
-	    || source_fmt->format.code != sink_fmt->format.code)
-		return -EPIPE;
+	if (source_fmt->format.width != sink_fmt->format.width) {
+		dev_dbg(sd->entity.graph_obj.mdev->dev,
+			"%s: width does not match (source %u, sink %u)\n",
+			__func__,
+			source_fmt->format.width, sink_fmt->format.width);
+		pass = false;
+	}
+
+	if (source_fmt->format.height != sink_fmt->format.height) {
+		dev_dbg(sd->entity.graph_obj.mdev->dev,
+			"%s: height does not match (source %u, sink %u)\n",
+			__func__,
+			source_fmt->format.height, sink_fmt->format.height);
+		pass = false;
+	}
+
+	if (source_fmt->format.code != sink_fmt->format.code) {
+		dev_dbg(sd->entity.graph_obj.mdev->dev,
+			"%s: media bus code does not match (source 0x%8.8x, sink 0x%8.8x)\n",
+			__func__,
+			source_fmt->format.code, sink_fmt->format.code);
+		pass = false;
+	}
 
 	/* The field order must match, or the sink field order must be NONE
 	 * to support interlaced hardware connected to bridges that support
 	 * progressive formats only.
 	 */
 	if (source_fmt->format.field != sink_fmt->format.field &&
-	    sink_fmt->format.field != V4L2_FIELD_NONE)
-		return -EPIPE;
+	    sink_fmt->format.field != V4L2_FIELD_NONE) {
+		dev_dbg(sd->entity.graph_obj.mdev->dev,
+			"%s: field does not match (source %u, sink %u)\n",
+			__func__,
+			source_fmt->format.field, sink_fmt->format.field);
+		pass = false;
+	}
 
-	return 0;
+	return pass ? 0 : -EPIPE;
 }
 EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate_default);
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] v4l: subdev: Improve link format validation debug messages
  2020-10-09 14:02 [PATCH 1/1] v4l: subdev: Improve link format validation debug messages Sakari Ailus
@ 2020-10-10 21:22 ` Laurent Pinchart
  0 siblings, 0 replies; 4+ messages in thread
From: Laurent Pinchart @ 2020-10-10 21:22 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media

Hi Sakari,

Thank you for the patch.

On Fri, Oct 09, 2020 at 05:02:23PM +0300, Sakari Ailus wrote:
> The existing link format validation failure debug message in media-entity.c
> helped to poinpoint the point of failure but provided no additional
> information what's wrong. Tell the user exactly why the validation failed.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> I pulled this old patch from the VC patchset. It can be merged now without
> the rest.
> 
>  drivers/media/v4l2-core/v4l2-subdev.c | 40 ++++++++++++++++++++++-----
>  1 file changed, 33 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index a7d508e74d6b..e1dfbcc96a4a 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -792,21 +792,47 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd,
>  				      struct v4l2_subdev_format *source_fmt,
>  				      struct v4l2_subdev_format *sink_fmt)
>  {
> +	bool pass = true;
> +
>  	/* The width, height and code must match. */
> -	if (source_fmt->format.width != sink_fmt->format.width
> -	    || source_fmt->format.height != sink_fmt->format.height
> -	    || source_fmt->format.code != sink_fmt->format.code)
> -		return -EPIPE;
> +	if (source_fmt->format.width != sink_fmt->format.width) {
> +		dev_dbg(sd->entity.graph_obj.mdev->dev,
> +			"%s: width does not match (source %u, sink %u)\n",
> +			__func__,
> +			source_fmt->format.width, sink_fmt->format.width);

It would be nice to print the sink and source names and pads, but we
don't have that information. Do you think we should print sd->name to
help identifying which link is faulty ?

> +		pass = false;
> +	}
> +
> +	if (source_fmt->format.height != sink_fmt->format.height) {
> +		dev_dbg(sd->entity.graph_obj.mdev->dev,
> +			"%s: height does not match (source %u, sink %u)\n",
> +			__func__,
> +			source_fmt->format.height, sink_fmt->format.height);
> +		pass = false;
> +	}
> +
> +	if (source_fmt->format.code != sink_fmt->format.code) {
> +		dev_dbg(sd->entity.graph_obj.mdev->dev,
> +			"%s: media bus code does not match (source 0x%8.8x, sink 0x%8.8x)\n",
> +			__func__,
> +			source_fmt->format.code, sink_fmt->format.code);
> +		pass = false;
> +	}
>  
>  	/* The field order must match, or the sink field order must be NONE
>  	 * to support interlaced hardware connected to bridges that support
>  	 * progressive formats only.
>  	 */
>  	if (source_fmt->format.field != sink_fmt->format.field &&
> -	    sink_fmt->format.field != V4L2_FIELD_NONE)
> -		return -EPIPE;
> +	    sink_fmt->format.field != V4L2_FIELD_NONE) {
> +		dev_dbg(sd->entity.graph_obj.mdev->dev,
> +			"%s: field does not match (source %u, sink %u)\n",
> +			__func__,
> +			source_fmt->format.field, sink_fmt->format.field);
> +		pass = false;
> +	}
>  
> -	return 0;
> +	return pass ? 0 : -EPIPE;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate_default);
>  

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] v4l: subdev: Improve link format validation debug messages
  2020-10-12  8:37 Sakari Ailus
@ 2020-10-12  8:57 ` Laurent Pinchart
  0 siblings, 0 replies; 4+ messages in thread
From: Laurent Pinchart @ 2020-10-12  8:57 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media

Hi Sakari,

Thank you for the patch.

On Mon, Oct 12, 2020 at 11:37:19AM +0300, Sakari Ailus wrote:
> The existing link format validation failure debug message in media-entity.c
> helped to poinpoint the point of failure but provided no additional
> information what's wrong. Tell the user exactly why the validation failed.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> since v1:
> 
> - Print the link if there were mismatches
> 
>  drivers/media/v4l2-core/v4l2-subdev.c | 48 +++++++++++++++++++++++----
>  1 file changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index a7d508e74d6b..956dafab43d4 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -792,21 +792,55 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd,
>  				      struct v4l2_subdev_format *source_fmt,
>  				      struct v4l2_subdev_format *sink_fmt)
>  {
> +	bool pass = true;
> +
>  	/* The width, height and code must match. */
> -	if (source_fmt->format.width != sink_fmt->format.width
> -	    || source_fmt->format.height != sink_fmt->format.height
> -	    || source_fmt->format.code != sink_fmt->format.code)
> -		return -EPIPE;
> +	if (source_fmt->format.width != sink_fmt->format.width) {
> +		dev_dbg(sd->entity.graph_obj.mdev->dev,
> +			"%s: width does not match (source %u, sink %u)\n",
> +			__func__,
> +			source_fmt->format.width, sink_fmt->format.width);
> +		pass = false;
> +	}
> +
> +	if (source_fmt->format.height != sink_fmt->format.height) {
> +		dev_dbg(sd->entity.graph_obj.mdev->dev,
> +			"%s: height does not match (source %u, sink %u)\n",
> +			__func__,
> +			source_fmt->format.height, sink_fmt->format.height);
> +		pass = false;
> +	}
> +
> +	if (source_fmt->format.code != sink_fmt->format.code) {
> +		dev_dbg(sd->entity.graph_obj.mdev->dev,
> +			"%s: media bus code does not match (source 0x%8.8x, sink 0x%8.8x)\n",
> +			__func__,
> +			source_fmt->format.code, sink_fmt->format.code);
> +		pass = false;
> +	}
>  
>  	/* The field order must match, or the sink field order must be NONE
>  	 * to support interlaced hardware connected to bridges that support
>  	 * progressive formats only.
>  	 */
>  	if (source_fmt->format.field != sink_fmt->format.field &&
> -	    sink_fmt->format.field != V4L2_FIELD_NONE)
> -		return -EPIPE;
> +	    sink_fmt->format.field != V4L2_FIELD_NONE) {
> +		dev_dbg(sd->entity.graph_obj.mdev->dev,
> +			"%s: field does not match (source %u, sink %u)\n",
> +			__func__,
> +			source_fmt->format.field, sink_fmt->format.field);
> +		pass = false;
> +	}
>  
> -	return 0;
> +	if (pass)
> +		return 0;
> +
> +	dev_dbg(sd->entity.graph_obj.mdev->dev,
> +		"%s: link was \"%s\":%u -> \"%s\":%u\n", __func__,
> +		link->source->entity->name, link->source->index,
> +		link->sink->entity->name, link->sink->index);
> +
> +	return -EPIPE;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate_default);
>  

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/1] v4l: subdev: Improve link format validation debug messages
@ 2020-10-12  8:37 Sakari Ailus
  2020-10-12  8:57 ` Laurent Pinchart
  0 siblings, 1 reply; 4+ messages in thread
From: Sakari Ailus @ 2020-10-12  8:37 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart

The existing link format validation failure debug message in media-entity.c
helped to poinpoint the point of failure but provided no additional
information what's wrong. Tell the user exactly why the validation failed.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
since v1:

- Print the link if there were mismatches

 drivers/media/v4l2-core/v4l2-subdev.c | 48 +++++++++++++++++++++++----
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index a7d508e74d6b..956dafab43d4 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -792,21 +792,55 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd,
 				      struct v4l2_subdev_format *source_fmt,
 				      struct v4l2_subdev_format *sink_fmt)
 {
+	bool pass = true;
+
 	/* The width, height and code must match. */
-	if (source_fmt->format.width != sink_fmt->format.width
-	    || source_fmt->format.height != sink_fmt->format.height
-	    || source_fmt->format.code != sink_fmt->format.code)
-		return -EPIPE;
+	if (source_fmt->format.width != sink_fmt->format.width) {
+		dev_dbg(sd->entity.graph_obj.mdev->dev,
+			"%s: width does not match (source %u, sink %u)\n",
+			__func__,
+			source_fmt->format.width, sink_fmt->format.width);
+		pass = false;
+	}
+
+	if (source_fmt->format.height != sink_fmt->format.height) {
+		dev_dbg(sd->entity.graph_obj.mdev->dev,
+			"%s: height does not match (source %u, sink %u)\n",
+			__func__,
+			source_fmt->format.height, sink_fmt->format.height);
+		pass = false;
+	}
+
+	if (source_fmt->format.code != sink_fmt->format.code) {
+		dev_dbg(sd->entity.graph_obj.mdev->dev,
+			"%s: media bus code does not match (source 0x%8.8x, sink 0x%8.8x)\n",
+			__func__,
+			source_fmt->format.code, sink_fmt->format.code);
+		pass = false;
+	}
 
 	/* The field order must match, or the sink field order must be NONE
 	 * to support interlaced hardware connected to bridges that support
 	 * progressive formats only.
 	 */
 	if (source_fmt->format.field != sink_fmt->format.field &&
-	    sink_fmt->format.field != V4L2_FIELD_NONE)
-		return -EPIPE;
+	    sink_fmt->format.field != V4L2_FIELD_NONE) {
+		dev_dbg(sd->entity.graph_obj.mdev->dev,
+			"%s: field does not match (source %u, sink %u)\n",
+			__func__,
+			source_fmt->format.field, sink_fmt->format.field);
+		pass = false;
+	}
 
-	return 0;
+	if (pass)
+		return 0;
+
+	dev_dbg(sd->entity.graph_obj.mdev->dev,
+		"%s: link was \"%s\":%u -> \"%s\":%u\n", __func__,
+		link->source->entity->name, link->source->index,
+		link->sink->entity->name, link->sink->index);
+
+	return -EPIPE;
 }
 EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate_default);
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-10-12  8:58 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-09 14:02 [PATCH 1/1] v4l: subdev: Improve link format validation debug messages Sakari Ailus
2020-10-10 21:22 ` Laurent Pinchart
2020-10-12  8:37 Sakari Ailus
2020-10-12  8:57 ` Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).