diff mbox series

[v2,2/2] drm: add debug logs for drm_mode_atomic_ioctl errors

Message ID AVFi5ZzKwRVLtY8AQyxvqgIbcUfVgNebOtGVwSNMKvs@cp4-web-038.plabs.ch (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Simon Ser Nov. 11, 2020, 9:07 a.m. UTC
Be nice to user-space and log what happened when returning EINVAL in
drm_mode_atomic_ioctl.

v2:

- Migrate to drm_dbg_atomic (Sam)
- Add debug log for arg->reserved (Ville)
- Drop redundant "atomic" word in log messages (Ville)

Signed-off-by: Simon Ser <contact@emersion.fr>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

Comments

Daniel Vetter Nov. 11, 2020, 4:48 p.m. UTC | #1
On Wed, Nov 11, 2020 at 09:07:39AM +0000, Simon Ser wrote:
> Be nice to user-space and log what happened when returning EINVAL in
> drm_mode_atomic_ioctl.
> 
> v2:
> 
> - Migrate to drm_dbg_atomic (Sam)
> - Add debug log for arg->reserved (Ville)
> - Drop redundant "atomic" word in log messages (Ville)
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>

Submission bikeshed: Patch 1&2 aren't threaded. I thought git send-email
gets that right by default nowadays ...

Anyway, on both patches: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_atomic_uapi.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index efab3d518891..9df7f2a170e3 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -1330,22 +1330,35 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>  	 * though this may be a bit overkill, since legacy userspace
>  	 * wouldn't know how to call this ioctl)
>  	 */
> -	if (!file_priv->atomic)
> +	if (!file_priv->atomic) {
> +		drm_dbg_atomic(dev,
> +			       "commit failed: atomic cap not enabled\n");
>  		return -EINVAL;
> +	}
>  
> -	if (arg->flags & ~DRM_MODE_ATOMIC_FLAGS)
> +	if (arg->flags & ~DRM_MODE_ATOMIC_FLAGS) {
> +		drm_dbg_atomic(dev, "commit failed: invalid flag\n");
>  		return -EINVAL;
> +	}
>  
> -	if (arg->reserved)
> +	if (arg->reserved) {
> +		drm_dbg_atomic(dev, "commit failed: reserved field set\n");
>  		return -EINVAL;
> +	}
>  
> -	if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC)
> +	if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC) {
> +		drm_dbg_atomic(dev,
> +			       "commit failed: invalid flag DRM_MODE_PAGE_FLIP_ASYNC\n");
>  		return -EINVAL;
> +	}
>  
>  	/* can't test and expect an event at the same time. */
>  	if ((arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) &&
> -			(arg->flags & DRM_MODE_PAGE_FLIP_EVENT))
> +			(arg->flags & DRM_MODE_PAGE_FLIP_EVENT)) {
> +		drm_dbg_atomic(dev,
> +			       "commit failed: page-flip event requested with test-only commit\n");
>  		return -EINVAL;
> +	}
>  
>  	state = drm_atomic_state_alloc(dev);
>  	if (!state)
> -- 
> 2.29.2
> 
>
Simon Ser Nov. 11, 2020, 4:51 p.m. UTC | #2
On Wednesday, November 11, 2020 5:48 PM, Daniel Vetter <daniel@ffwll.ch> wrote:

> Submission bikeshed: Patch 1&2 aren't threaded. I thought git send-email
> gets that right by default nowadays ...

Yeah, it's a bug in my SMTP server I haven't bothered to write a patch
for yet. Sorry about that.

> Anyway, on both patches: Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch

Thanks for the review!
Sam Ravnborg Nov. 11, 2020, 6:31 p.m. UTC | #3
On Wed, Nov 11, 2020 at 09:07:39AM +0000, Simon Ser wrote:
> Be nice to user-space and log what happened when returning EINVAL in
> drm_mode_atomic_ioctl.
> 
> v2:
> 
> - Migrate to drm_dbg_atomic (Sam)
> - Add debug log for arg->reserved (Ville)
> - Drop redundant "atomic" word in log messages (Ville)
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index efab3d518891..9df7f2a170e3 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1330,22 +1330,35 @@  int drm_mode_atomic_ioctl(struct drm_device *dev,
 	 * though this may be a bit overkill, since legacy userspace
 	 * wouldn't know how to call this ioctl)
 	 */
-	if (!file_priv->atomic)
+	if (!file_priv->atomic) {
+		drm_dbg_atomic(dev,
+			       "commit failed: atomic cap not enabled\n");
 		return -EINVAL;
+	}
 
-	if (arg->flags & ~DRM_MODE_ATOMIC_FLAGS)
+	if (arg->flags & ~DRM_MODE_ATOMIC_FLAGS) {
+		drm_dbg_atomic(dev, "commit failed: invalid flag\n");
 		return -EINVAL;
+	}
 
-	if (arg->reserved)
+	if (arg->reserved) {
+		drm_dbg_atomic(dev, "commit failed: reserved field set\n");
 		return -EINVAL;
+	}
 
-	if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC)
+	if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC) {
+		drm_dbg_atomic(dev,
+			       "commit failed: invalid flag DRM_MODE_PAGE_FLIP_ASYNC\n");
 		return -EINVAL;
+	}
 
 	/* can't test and expect an event at the same time. */
 	if ((arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) &&
-			(arg->flags & DRM_MODE_PAGE_FLIP_EVENT))
+			(arg->flags & DRM_MODE_PAGE_FLIP_EVENT)) {
+		drm_dbg_atomic(dev,
+			       "commit failed: page-flip event requested with test-only commit\n");
 		return -EINVAL;
+	}
 
 	state = drm_atomic_state_alloc(dev);
 	if (!state)