diff mbox series

[v2,7/7] drm: prefer DRM_MODE_FMT/ARG over drm_mode_debug_printmodeline()

Message ID 6df18588dfa17c5d0a1501f5af9ff21f25a1981b.1712568037.git.jani.nikula@intel.com (mailing list archive)
State New
Headers show
Series drm: debug logging improvements | expand

Commit Message

Jani Nikula April 8, 2024, 9:24 a.m. UTC 
We have DRM_MODE_FMT and DRM_MODE_ARG() macros to allow unified debug
printing of modes in any printk-formatted logging. Prefer them over
drm_mode_debug_printmodeline().

This allows drm device specific logging of modes, in the right drm debug
category, and inline with the rest of the logging instead of split to
multiple lines.

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c  |  6 +++---
 drivers/gpu/drm/drm_crtc.c         |  6 +++---
 drivers/gpu/drm/drm_crtc_helper.c  |  9 ++++-----
 drivers/gpu/drm/drm_modes.c        | 13 +++++--------
 drivers/gpu/drm/drm_probe_helper.c |  3 ++-
 5 files changed, 17 insertions(+), 20 deletions(-)

Comments

Ville Syrjala April 8, 2024, 12:51 p.m. UTC | #1 
On Mon, Apr 08, 2024 at 12:24:02PM +0300, Jani Nikula wrote:
> We have DRM_MODE_FMT and DRM_MODE_ARG() macros to allow unified debug
> printing of modes in any printk-formatted logging. Prefer them over
> drm_mode_debug_printmodeline().
> 
> This allows drm device specific logging of modes, in the right drm debug
> category, and inline with the rest of the logging instead of split to
> multiple lines.
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/drm_atomic_uapi.c  |  6 +++---
>  drivers/gpu/drm/drm_crtc.c         |  6 +++---
>  drivers/gpu/drm/drm_crtc_helper.c  |  9 ++++-----
>  drivers/gpu/drm/drm_modes.c        | 13 +++++--------
>  drivers/gpu/drm/drm_probe_helper.c |  3 ++-
>  5 files changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 29d4940188d4..fc16fddee5c5 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -145,10 +145,10 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
>  					     &state->mode, blob->data);
>  		if (ret) {
>  			drm_dbg_atomic(crtc->dev,
> -				       "[CRTC:%d:%s] invalid mode (ret=%d, status=%s):\n",
> +				       "[CRTC:%d:%s] invalid mode (%s, %pe): " DRM_MODE_FMT "\n",
>  				       crtc->base.id, crtc->name,
> -				       ret, drm_get_mode_status_name(state->mode.status));
> -			drm_mode_debug_printmodeline(&state->mode);
> +				       drm_get_mode_status_name(state->mode.status),
> +				       ERR_PTR(ret), DRM_MODE_ARG(&state->mode));
>  			return -EINVAL;
>  		}
>  
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index b0a0e27e83eb..483969b84a30 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -775,9 +775,9 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>  
>  		ret = drm_mode_convert_umode(dev, mode, &crtc_req->mode);
>  		if (ret) {
> -			drm_dbg_kms(dev, "Invalid mode (ret=%d, status=%s)\n",
> -				    ret, drm_get_mode_status_name(mode->status));
> -			drm_mode_debug_printmodeline(mode);
> +			drm_dbg_kms(dev, "Invalid mode (%s, %pe): " DRM_MODE_FMT "\n",
> +				    drm_get_mode_status_name(mode->status),
> +				    ERR_PTR(ret), DRM_MODE_ARG(mode));
>  			goto out;
>  		}
>  
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index af7ac9d9192a..0955f1c385dd 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -657,8 +657,8 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
>  	if (!drm_mode_equal(set->mode, &set->crtc->mode)) {
>  		drm_dbg_kms(dev, "[CRTC:%d:%s] modes are different, full mode set:\n",
>  			    set->crtc->base.id, set->crtc->name);
> -		drm_mode_debug_printmodeline(&set->crtc->mode);
> -		drm_mode_debug_printmodeline(set->mode);
> +		drm_dbg_kms(dev, DRM_MODE_FMT "\n", DRM_MODE_ARG(&set->crtc->mode));
> +		drm_dbg_kms(dev, DRM_MODE_FMT "\n", DRM_MODE_ARG(set->mode));
>  		mode_changed = true;
>  	}
>  
> @@ -766,9 +766,8 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
>  
>  	if (mode_changed) {
>  		if (drm_helper_crtc_in_use(set->crtc)) {
> -			drm_dbg_kms(dev, "[CRTC:%d:%s] attempting to set mode from userspace\n",
> -				    set->crtc->base.id, set->crtc->name);
> -			drm_mode_debug_printmodeline(set->mode);
> +			drm_dbg_kms(dev, "[CRTC:%d:%s] attempting to set mode from userspace: " DRM_MODE_FMT "\n",
> +				    set->crtc->base.id, set->crtc->name, DRM_MODE_ARG(set->mode));
>  			set->crtc->primary->fb = set->fb;
>  			if (!drm_crtc_helper_set_mode(set->crtc, set->mode,
>  						      set->x, set->y,
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 408ee1b5e44d..2d8b0371619d 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1201,9 +1201,8 @@ int of_get_drm_display_mode(struct device_node *np,
>  	if (bus_flags)
>  		drm_bus_flags_from_videomode(&vm, bus_flags);
>  
> -	pr_debug("%pOF: got %dx%d display mode\n",
> -		np, vm.hactive, vm.vactive);
> -	drm_mode_debug_printmodeline(dmode);
> +	pr_debug("%pOF: got %dx%d display mode: " DRM_MODE_FMT "\n",
> +		 np, vm.hactive, vm.vactive, DRM_MODE_ARG(dmode));
>  
>  	return 0;
>  }
> @@ -1251,7 +1250,7 @@ int of_get_drm_panel_display_mode(struct device_node *np,
>  	dmode->width_mm = width_mm;
>  	dmode->height_mm = height_mm;
>  
> -	drm_mode_debug_printmodeline(dmode);
> +	pr_debug(DRM_MODE_FMT "\n", DRM_MODE_ARG(dmode));
>  
>  	return 0;
>  }
> @@ -1813,10 +1812,8 @@ void drm_mode_prune_invalid(struct drm_device *dev,
>  					 DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
>  			}
>  			if (verbose) {
> -				drm_mode_debug_printmodeline(mode);
> -				drm_dbg_kms(dev, "Not using %s mode: %s\n",
> -					    mode->name,
> -					    drm_get_mode_status_name(mode->status));
> +				drm_dbg_kms(dev, "Rejected mode: " DRM_MODE_FMT " (%s)\n",
> +					    DRM_MODE_ARG(mode), drm_get_mode_status_name(mode->status));
>  			}
>  			drm_mode_destroy(dev, mode);
>  		}
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 19bf5d298257..4f75a1cfd820 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -701,7 +701,8 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>  
>  	list_for_each_entry(mode, &connector->modes, head) {
>  		drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V);
> -		drm_mode_debug_printmodeline(mode);
> +		drm_dbg_kms(dev, "Probed mode: " DRM_MODE_FMT "\n",
> +			    DRM_MODE_ARG(mode));
>  	}
>  
>  	return count;
> -- 
> 2.39.2
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 29d4940188d4..fc16fddee5c5 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -145,10 +145,10 @@  int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
 					     &state->mode, blob->data);
 		if (ret) {
 			drm_dbg_atomic(crtc->dev,
-				       "[CRTC:%d:%s] invalid mode (ret=%d, status=%s):\n",
+				       "[CRTC:%d:%s] invalid mode (%s, %pe): " DRM_MODE_FMT "\n",
 				       crtc->base.id, crtc->name,
-				       ret, drm_get_mode_status_name(state->mode.status));
-			drm_mode_debug_printmodeline(&state->mode);
+				       drm_get_mode_status_name(state->mode.status),
+				       ERR_PTR(ret), DRM_MODE_ARG(&state->mode));
 			return -EINVAL;
 		}
 
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index b0a0e27e83eb..483969b84a30 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -775,9 +775,9 @@  int drm_mode_setcrtc(struct drm_device *dev, void *data,
 
 		ret = drm_mode_convert_umode(dev, mode, &crtc_req->mode);
 		if (ret) {
-			drm_dbg_kms(dev, "Invalid mode (ret=%d, status=%s)\n",
-				    ret, drm_get_mode_status_name(mode->status));
-			drm_mode_debug_printmodeline(mode);
+			drm_dbg_kms(dev, "Invalid mode (%s, %pe): " DRM_MODE_FMT "\n",
+				    drm_get_mode_status_name(mode->status),
+				    ERR_PTR(ret), DRM_MODE_ARG(mode));
 			goto out;
 		}
 
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index af7ac9d9192a..0955f1c385dd 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -657,8 +657,8 @@  int drm_crtc_helper_set_config(struct drm_mode_set *set,
 	if (!drm_mode_equal(set->mode, &set->crtc->mode)) {
 		drm_dbg_kms(dev, "[CRTC:%d:%s] modes are different, full mode set:\n",
 			    set->crtc->base.id, set->crtc->name);
-		drm_mode_debug_printmodeline(&set->crtc->mode);
-		drm_mode_debug_printmodeline(set->mode);
+		drm_dbg_kms(dev, DRM_MODE_FMT "\n", DRM_MODE_ARG(&set->crtc->mode));
+		drm_dbg_kms(dev, DRM_MODE_FMT "\n", DRM_MODE_ARG(set->mode));
 		mode_changed = true;
 	}
 
@@ -766,9 +766,8 @@  int drm_crtc_helper_set_config(struct drm_mode_set *set,
 
 	if (mode_changed) {
 		if (drm_helper_crtc_in_use(set->crtc)) {
-			drm_dbg_kms(dev, "[CRTC:%d:%s] attempting to set mode from userspace\n",
-				    set->crtc->base.id, set->crtc->name);
-			drm_mode_debug_printmodeline(set->mode);
+			drm_dbg_kms(dev, "[CRTC:%d:%s] attempting to set mode from userspace: " DRM_MODE_FMT "\n",
+				    set->crtc->base.id, set->crtc->name, DRM_MODE_ARG(set->mode));
 			set->crtc->primary->fb = set->fb;
 			if (!drm_crtc_helper_set_mode(set->crtc, set->mode,
 						      set->x, set->y,
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 408ee1b5e44d..2d8b0371619d 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1201,9 +1201,8 @@  int of_get_drm_display_mode(struct device_node *np,
 	if (bus_flags)
 		drm_bus_flags_from_videomode(&vm, bus_flags);
 
-	pr_debug("%pOF: got %dx%d display mode\n",
-		np, vm.hactive, vm.vactive);
-	drm_mode_debug_printmodeline(dmode);
+	pr_debug("%pOF: got %dx%d display mode: " DRM_MODE_FMT "\n",
+		 np, vm.hactive, vm.vactive, DRM_MODE_ARG(dmode));
 
 	return 0;
 }
@@ -1251,7 +1250,7 @@  int of_get_drm_panel_display_mode(struct device_node *np,
 	dmode->width_mm = width_mm;
 	dmode->height_mm = height_mm;
 
-	drm_mode_debug_printmodeline(dmode);
+	pr_debug(DRM_MODE_FMT "\n", DRM_MODE_ARG(dmode));
 
 	return 0;
 }
@@ -1813,10 +1812,8 @@  void drm_mode_prune_invalid(struct drm_device *dev,
 					 DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
 			}
 			if (verbose) {
-				drm_mode_debug_printmodeline(mode);
-				drm_dbg_kms(dev, "Not using %s mode: %s\n",
-					    mode->name,
-					    drm_get_mode_status_name(mode->status));
+				drm_dbg_kms(dev, "Rejected mode: " DRM_MODE_FMT " (%s)\n",
+					    DRM_MODE_ARG(mode), drm_get_mode_status_name(mode->status));
 			}
 			drm_mode_destroy(dev, mode);
 		}
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 19bf5d298257..4f75a1cfd820 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -701,7 +701,8 @@  int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
 
 	list_for_each_entry(mode, &connector->modes, head) {
 		drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V);
-		drm_mode_debug_printmodeline(mode);
+		drm_dbg_kms(dev, "Probed mode: " DRM_MODE_FMT "\n",
+			    DRM_MODE_ARG(mode));
 	}
 
 	return count;