All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sam Ravnborg <sam@ravnborg.org>
To: Simon Ser <contact@emersion.fr>
Cc: dri-devel@lists.freedesktop.org, Thomas Zimmermann <tzimmermann@suse.de>
Subject: Re: [PATCH v2 1/2] drm: convert drm_atomic_uapi.c to new debug helpers
Date: Wed, 11 Nov 2020 19:31:31 +0100	[thread overview]
Message-ID: <20201111183131.GA2999095@ravnborg.org> (raw)
In-Reply-To: <FpVdsmqIh7IkH7YIwBjmp5ict1qi4NZlwHrIps@cp4-web-034.plabs.ch>

Hi Simon,
On Wed, Nov 11, 2020 at 09:07:36AM +0000, Simon Ser wrote:
> Migrate from DRM_DEBUG_ATOMIC to drm_dbg_atomic.
> 
> 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>

Thanks for doing this. I had not been shy about using longer lines as
the hard limit these days are 100 - but thats just an observation.

Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

Note: I expect you will commit the patch

	Sam

> ---
>  drivers/gpu/drm/drm_atomic_uapi.c | 113 +++++++++++++++++-------------
>  1 file changed, 66 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index ef82009035e6..efab3d518891 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -85,13 +85,15 @@ int drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state,
>  
>  		drm_mode_copy(&state->mode, mode);
>  		state->enable = true;
> -		DRM_DEBUG_ATOMIC("Set [MODE:%s] for [CRTC:%d:%s] state %p\n",
> -				 mode->name, crtc->base.id, crtc->name, state);
> +		drm_dbg_atomic(crtc->dev,
> +			       "Set [MODE:%s] for [CRTC:%d:%s] state %p\n",
> +			       mode->name, crtc->base.id, crtc->name, state);
>  	} else {
>  		memset(&state->mode, 0, sizeof(state->mode));
>  		state->enable = false;
> -		DRM_DEBUG_ATOMIC("Set [NOMODE] for [CRTC:%d:%s] state %p\n",
> -				 crtc->base.id, crtc->name, state);
> +		drm_dbg_atomic(crtc->dev,
> +			       "Set [NOMODE] for [CRTC:%d:%s] state %p\n",
> +			       crtc->base.id, crtc->name, state);
>  	}
>  
>  	return 0;
> @@ -128,31 +130,35 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
>  		int ret;
>  
>  		if (blob->length != sizeof(struct drm_mode_modeinfo)) {
> -			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] bad mode blob length: %zu\n",
> -					 crtc->base.id, crtc->name,
> -					 blob->length);
> +			drm_dbg_atomic(crtc->dev,
> +				       "[CRTC:%d:%s] bad mode blob length: %zu\n",
> +				       crtc->base.id, crtc->name,
> +				       blob->length);
>  			return -EINVAL;
>  		}
>  
>  		ret = drm_mode_convert_umode(crtc->dev,
>  					     &state->mode, blob->data);
>  		if (ret) {
> -			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] invalid mode (ret=%d, status=%s):\n",
> -					 crtc->base.id, crtc->name,
> -					 ret, drm_get_mode_status_name(state->mode.status));
> +			drm_dbg_atomic(crtc->dev,
> +				       "[CRTC:%d:%s] invalid mode (ret=%d, status=%s):\n",
> +				       crtc->base.id, crtc->name,
> +				       ret, drm_get_mode_status_name(state->mode.status));
>  			drm_mode_debug_printmodeline(&state->mode);
>  			return -EINVAL;
>  		}
>  
>  		state->mode_blob = drm_property_blob_get(blob);
>  		state->enable = true;
> -		DRM_DEBUG_ATOMIC("Set [MODE:%s] for [CRTC:%d:%s] state %p\n",
> -				 state->mode.name, crtc->base.id, crtc->name,
> -				 state);
> +		drm_dbg_atomic(crtc->dev,
> +			       "Set [MODE:%s] for [CRTC:%d:%s] state %p\n",
> +			       state->mode.name, crtc->base.id, crtc->name,
> +			       state);
>  	} else {
>  		state->enable = false;
> -		DRM_DEBUG_ATOMIC("Set [NOMODE] for [CRTC:%d:%s] state %p\n",
> -				 crtc->base.id, crtc->name, state);
> +		drm_dbg_atomic(crtc->dev,
> +			       "Set [NOMODE] for [CRTC:%d:%s] state %p\n",
> +			       crtc->base.id, crtc->name, state);
>  	}
>  
>  	return 0;
> @@ -202,12 +208,14 @@ drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state,
>  	}
>  
>  	if (crtc)
> -		DRM_DEBUG_ATOMIC("Link [PLANE:%d:%s] state %p to [CRTC:%d:%s]\n",
> -				 plane->base.id, plane->name, plane_state,
> -				 crtc->base.id, crtc->name);
> +		drm_dbg_atomic(plane->dev,
> +			       "Link [PLANE:%d:%s] state %p to [CRTC:%d:%s]\n",
> +			       plane->base.id, plane->name, plane_state,
> +			       crtc->base.id, crtc->name);
>  	else
> -		DRM_DEBUG_ATOMIC("Link [PLANE:%d:%s] state %p to [NOCRTC]\n",
> -				 plane->base.id, plane->name, plane_state);
> +		drm_dbg_atomic(plane->dev,
> +			       "Link [PLANE:%d:%s] state %p to [NOCRTC]\n",
> +			       plane->base.id, plane->name, plane_state);
>  
>  	return 0;
>  }
> @@ -230,12 +238,14 @@ drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state,
>  	struct drm_plane *plane = plane_state->plane;
>  
>  	if (fb)
> -		DRM_DEBUG_ATOMIC("Set [FB:%d] for [PLANE:%d:%s] state %p\n",
> -				 fb->base.id, plane->base.id, plane->name,
> -				 plane_state);
> +		drm_dbg_atomic(plane->dev,
> +			       "Set [FB:%d] for [PLANE:%d:%s] state %p\n",
> +			       fb->base.id, plane->base.id, plane->name,
> +			       plane_state);
>  	else
> -		DRM_DEBUG_ATOMIC("Set [NOFB] for [PLANE:%d:%s] state %p\n",
> -				 plane->base.id, plane->name, plane_state);
> +		drm_dbg_atomic(plane->dev,
> +			       "Set [NOFB] for [PLANE:%d:%s] state %p\n",
> +			       plane->base.id, plane->name, plane_state);
>  
>  	drm_framebuffer_assign(&plane_state->fb, fb);
>  }
> @@ -324,13 +334,15 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
>  		drm_connector_get(conn_state->connector);
>  		conn_state->crtc = crtc;
>  
> -		DRM_DEBUG_ATOMIC("Link [CONNECTOR:%d:%s] state %p to [CRTC:%d:%s]\n",
> -				 connector->base.id, connector->name,
> -				 conn_state, crtc->base.id, crtc->name);
> +		drm_dbg_atomic(crtc->dev,
> +			       "Link [CONNECTOR:%d:%s] state %p to [CRTC:%d:%s]\n",
> +			       connector->base.id, connector->name,
> +			       conn_state, crtc->base.id, crtc->name);
>  	} else {
> -		DRM_DEBUG_ATOMIC("Link [CONNECTOR:%d:%s] state %p to [NOCRTC]\n",
> -				 connector->base.id, connector->name,
> -				 conn_state);
> +		drm_dbg_atomic(crtc->dev,
> +			       "Link [CONNECTOR:%d:%s] state %p to [NOCRTC]\n",
> +			       connector->base.id, connector->name,
> +			       conn_state);
>  	}
>  
>  	return 0;
> @@ -474,9 +486,10 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>  	} else if (crtc->funcs->atomic_set_property) {
>  		return crtc->funcs->atomic_set_property(crtc, state, property, val);
>  	} else {
> -		DRM_DEBUG_ATOMIC("[CRTC:%d:%s] unknown property [PROP:%d:%s]]\n",
> -				 crtc->base.id, crtc->name,
> -				 property->base.id, property->name);
> +		drm_dbg_atomic(crtc->dev,
> +			       "[CRTC:%d:%s] unknown property [PROP:%d:%s]]\n",
> +			       crtc->base.id, crtc->name,
> +			       property->base.id, property->name);
>  		return -EINVAL;
>  	}
>  
> @@ -570,8 +583,9 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		state->pixel_blend_mode = val;
>  	} else if (property == plane->rotation_property) {
>  		if (!is_power_of_2(val & DRM_MODE_ROTATE_MASK)) {
> -			DRM_DEBUG_ATOMIC("[PLANE:%d:%s] bad rotation bitmask: 0x%llx\n",
> -					 plane->base.id, plane->name, val);
> +			drm_dbg_atomic(plane->dev,
> +				       "[PLANE:%d:%s] bad rotation bitmask: 0x%llx\n",
> +				       plane->base.id, plane->name, val);
>  			return -EINVAL;
>  		}
>  		state->rotation = val;
> @@ -595,9 +609,10 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
>  	} else {
> -		DRM_DEBUG_ATOMIC("[PLANE:%d:%s] unknown property [PROP:%d:%s]]\n",
> -				 plane->base.id, plane->name,
> -				 property->base.id, property->name);
> +		drm_dbg_atomic(plane->dev,
> +			       "[PLANE:%d:%s] unknown property [PROP:%d:%s]]\n",
> +			       plane->base.id, plane->name,
> +			       property->base.id, property->name);
>  		return -EINVAL;
>  	}
>  
> @@ -665,17 +680,20 @@ static int drm_atomic_set_writeback_fb_for_connector(
>  		struct drm_framebuffer *fb)
>  {
>  	int ret;
> +	struct drm_connector *conn = conn_state->connector;
>  
>  	ret = drm_writeback_set_fb(conn_state, fb);
>  	if (ret < 0)
>  		return ret;
>  
>  	if (fb)
> -		DRM_DEBUG_ATOMIC("Set [FB:%d] for connector state %p\n",
> -				 fb->base.id, conn_state);
> +		drm_dbg_atomic(conn->dev,
> +			       "Set [FB:%d] for connector state %p\n",
> +			       fb->base.id, conn_state);
>  	else
> -		DRM_DEBUG_ATOMIC("Set [NOFB] for connector state %p\n",
> -				 conn_state);
> +		drm_dbg_atomic(conn->dev,
> +			       "Set [NOFB] for connector state %p\n",
> +			       conn_state);
>  
>  	return 0;
>  }
> @@ -782,9 +800,10 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>  		return connector->funcs->atomic_set_property(connector,
>  				state, property, val);
>  	} else {
> -		DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] unknown property [PROP:%d:%s]]\n",
> -				 connector->base.id, connector->name,
> -				 property->base.id, property->name);
> +		drm_dbg_atomic(connector->dev,
> +			       "[CONNECTOR:%d:%s] unknown property [PROP:%d:%s]]\n",
> +			       connector->base.id, connector->name,
> +			       property->base.id, property->name);
>  		return -EINVAL;
>  	}
>  
> @@ -1282,7 +1301,7 @@ static void complete_signaling(struct drm_device *dev,
>  		/* If this fails log error to the user */
>  		if (fence_state[i].out_fence_ptr &&
>  		    put_user(-1, fence_state[i].out_fence_ptr))
> -			DRM_DEBUG_ATOMIC("Couldn't clear out_fence_ptr\n");
> +			drm_dbg_atomic(dev, "Couldn't clear out_fence_ptr\n");
>  	}
>  
>  	kfree(fence_state);
> -- 
> 2.29.2
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2020-11-11 18:31 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20201116090608eucas1p128a4fa2e03380fe0cffea2f9bc2af816@eucas1p1.samsung.com>
2020-11-11  9:07 ` [PATCH v2 1/2] drm: convert drm_atomic_uapi.c to new debug helpers Simon Ser
2020-11-11 18:31   ` Sam Ravnborg [this message]
2020-11-16  9:06   ` [v2,1/2] " Marek Szyprowski
2020-11-16  9:06     ` Marek Szyprowski
2020-11-16  9:08     ` Simon Ser
2020-11-16  9:08       ` Simon Ser

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201111183131.GA2999095@ravnborg.org \
    --to=sam@ravnborg.org \
    --cc=contact@emersion.fr \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.