All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@intel.com>
To: dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
	jani.nikula@intel.com, ville.syrjala@linux.intel.com,
	Thomas Zimmermann <tzimmermann@suse.de>
Subject: [PATCH v2 6/7] drm/crtc-helper: switch to drm device based logging and warns
Date: Mon,  8 Apr 2024 12:24:01 +0300	[thread overview]
Message-ID: <b8557c4b2db0e5c931a6d82b5cc8ac5f3a3e1a77.1712568037.git.jani.nikula@intel.com> (raw)
In-Reply-To: <cover.1712568037.git.jani.nikula@intel.com>

Prefer drm device based drm_dbg_kms(), drm_err(), drm_WARN_ON() over
DRM_DEBUG_KMS(), DRM_ERROR(), and WARN_ON(). Also update encoder,
connector, and crtc logging to include the object id and name, where
possible.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_crtc_helper.c | 95 +++++++++++++++++--------------
 1 file changed, 52 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 2dafc39a27cb..af7ac9d9192a 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -110,15 +110,15 @@ bool drm_helper_encoder_in_use(struct drm_encoder *encoder)
 	struct drm_connector_list_iter conn_iter;
 	struct drm_device *dev = encoder->dev;
 
-	WARN_ON(drm_drv_uses_atomic_modeset(dev));
+	drm_WARN_ON(dev, drm_drv_uses_atomic_modeset(dev));
 
 	/*
 	 * We can expect this mutex to be locked if we are not panicking.
 	 * Locking is currently fubar in the panic handler.
 	 */
 	if (!oops_in_progress) {
-		WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
-		WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
+		drm_WARN_ON(dev, !mutex_is_locked(&dev->mode_config.mutex));
+		drm_WARN_ON(dev, !drm_modeset_is_locked(&dev->mode_config.connection_mutex));
 	}
 
 
@@ -150,14 +150,14 @@ bool drm_helper_crtc_in_use(struct drm_crtc *crtc)
 	struct drm_encoder *encoder;
 	struct drm_device *dev = crtc->dev;
 
-	WARN_ON(drm_drv_uses_atomic_modeset(dev));
+	drm_WARN_ON(dev, drm_drv_uses_atomic_modeset(dev));
 
 	/*
 	 * We can expect this mutex to be locked if we are not panicking.
 	 * Locking is currently fubar in the panic handler.
 	 */
 	if (!oops_in_progress)
-		WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
+		drm_WARN_ON(dev, !mutex_is_locked(&dev->mode_config.mutex));
 
 	drm_for_each_encoder(encoder, dev)
 		if (encoder->crtc == crtc && drm_helper_encoder_in_use(encoder))
@@ -230,7 +230,7 @@ static void __drm_helper_disable_unused_functions(struct drm_device *dev)
  */
 void drm_helper_disable_unused_functions(struct drm_device *dev)
 {
-	WARN_ON(drm_drv_uses_atomic_modeset(dev));
+	drm_WARN_ON(dev, drm_drv_uses_atomic_modeset(dev));
 
 	drm_modeset_lock_all(dev);
 	__drm_helper_disable_unused_functions(dev);
@@ -294,7 +294,7 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
 	struct drm_encoder *encoder;
 	bool ret = true;
 
-	WARN_ON(drm_drv_uses_atomic_modeset(dev));
+	drm_WARN_ON(dev, drm_drv_uses_atomic_modeset(dev));
 
 	drm_warn_on_modeset_not_all_locked(dev);
 
@@ -338,7 +338,8 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
 		if (encoder_funcs->mode_fixup) {
 			if (!(ret = encoder_funcs->mode_fixup(encoder, mode,
 							      adjusted_mode))) {
-				DRM_DEBUG_KMS("Encoder fixup failed\n");
+				drm_dbg_kms(dev, "[ENCODER:%d:%s] mode fixup failed\n",
+					    encoder->base.id, encoder->name);
 				goto done;
 			}
 		}
@@ -347,11 +348,12 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
 	if (crtc_funcs->mode_fixup) {
 		if (!(ret = crtc_funcs->mode_fixup(crtc, mode,
 						adjusted_mode))) {
-			DRM_DEBUG_KMS("CRTC fixup failed\n");
+			drm_dbg_kms(dev, "[CRTC:%d:%s] mode fixup failed\n",
+				    crtc->base.id, crtc->name);
 			goto done;
 		}
 	}
-	DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name);
+	drm_dbg_kms(dev, "[CRTC:%d:%s]\n", crtc->base.id, crtc->name);
 
 	drm_mode_copy(&crtc->hwmode, adjusted_mode);
 
@@ -390,8 +392,8 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
 		if (!encoder_funcs)
 			continue;
 
-		DRM_DEBUG_KMS("[ENCODER:%d:%s] set [MODE:%s]\n",
-			encoder->base.id, encoder->name, mode->name);
+		drm_dbg_kms(dev, "[ENCODER:%d:%s] set [MODE:%s]\n",
+			    encoder->base.id, encoder->name, mode->name);
 		if (encoder_funcs->mode_set)
 			encoder_funcs->mode_set(encoder, mode, adjusted_mode);
 	}
@@ -503,7 +505,7 @@ drm_connector_get_single_encoder(struct drm_connector *connector)
 {
 	struct drm_encoder *encoder;
 
-	WARN_ON(hweight32(connector->possible_encoders) > 1);
+	drm_WARN_ON(connector->dev, hweight32(connector->possible_encoders) > 1);
 	drm_connector_for_each_possible_encoder(connector, encoder)
 		return encoder;
 
@@ -564,8 +566,6 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
 	int ret;
 	int i;
 
-	DRM_DEBUG_KMS("\n");
-
 	BUG_ON(!set);
 	BUG_ON(!set->crtc);
 	BUG_ON(!set->crtc->helper_private);
@@ -577,19 +577,22 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
 	crtc_funcs = set->crtc->helper_private;
 
 	dev = set->crtc->dev;
-	WARN_ON(drm_drv_uses_atomic_modeset(dev));
+
+	drm_dbg_kms(dev, "\n");
+
+	drm_WARN_ON(dev, drm_drv_uses_atomic_modeset(dev));
 
 	if (!set->mode)
 		set->fb = NULL;
 
 	if (set->fb) {
-		DRM_DEBUG_KMS("[CRTC:%d:%s] [FB:%d] #connectors=%d (x y) (%i %i)\n",
-			      set->crtc->base.id, set->crtc->name,
-			      set->fb->base.id,
-			      (int)set->num_connectors, set->x, set->y);
+		drm_dbg_kms(dev, "[CRTC:%d:%s] [FB:%d] #connectors=%d (x y) (%i %i)\n",
+			    set->crtc->base.id, set->crtc->name,
+			    set->fb->base.id,
+			    (int)set->num_connectors, set->x, set->y);
 	} else {
-		DRM_DEBUG_KMS("[CRTC:%d:%s] [NOFB]\n",
-			      set->crtc->base.id, set->crtc->name);
+		drm_dbg_kms(dev, "[CRTC:%d:%s] [NOFB]\n",
+			    set->crtc->base.id, set->crtc->name);
 		drm_crtc_helper_disable(set->crtc);
 		return 0;
 	}
@@ -639,7 +642,8 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
 	if (set->crtc->primary->fb != set->fb) {
 		/* If we have no fb then treat it as a full mode set */
 		if (set->crtc->primary->fb == NULL) {
-			DRM_DEBUG_KMS("crtc has no fb, full mode set\n");
+			drm_dbg_kms(dev, "[CRTC:%d:%s] no fb, full mode set\n",
+				    set->crtc->base.id, set->crtc->name);
 			mode_changed = true;
 		} else if (set->fb->format != set->crtc->primary->fb->format) {
 			mode_changed = true;
@@ -651,7 +655,8 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
 		fb_changed = true;
 
 	if (!drm_mode_equal(set->mode, &set->crtc->mode)) {
-		DRM_DEBUG_KMS("modes are different, full mode set\n");
+		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);
 		mode_changed = true;
@@ -687,7 +692,8 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
 					fail = 1;
 
 				if (connector->dpms != DRM_MODE_DPMS_ON) {
-					DRM_DEBUG_KMS("connector dpms not on, full mode switch\n");
+					drm_dbg_kms(dev, "[CONNECTOR:%d:%s] DPMS not on, full mode switch\n",
+						    connector->base.id, connector->name);
 					mode_changed = true;
 				}
 
@@ -696,7 +702,8 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
 		}
 
 		if (new_encoder != connector->encoder) {
-			DRM_DEBUG_KMS("encoder changed, full mode switch\n");
+			drm_dbg_kms(dev, "[CONNECTOR:%d:%s] encoder changed, full mode switch\n",
+				    connector->base.id, connector->name);
 			mode_changed = true;
 			/* If the encoder is reused for another connector, then
 			 * the appropriate crtc will be set later.
@@ -737,17 +744,18 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
 			goto fail;
 		}
 		if (new_crtc != connector->encoder->crtc) {
-			DRM_DEBUG_KMS("crtc changed, full mode switch\n");
+			drm_dbg_kms(dev, "[CONNECTOR:%d:%s] CRTC changed, full mode switch\n",
+				    connector->base.id, connector->name);
 			mode_changed = true;
 			connector->encoder->crtc = new_crtc;
 		}
 		if (new_crtc) {
-			DRM_DEBUG_KMS("[CONNECTOR:%d:%s] to [CRTC:%d:%s]\n",
-				      connector->base.id, connector->name,
-				      new_crtc->base.id, new_crtc->name);
+			drm_dbg_kms(dev, "[CONNECTOR:%d:%s] to [CRTC:%d:%s]\n",
+				    connector->base.id, connector->name,
+				    new_crtc->base.id, new_crtc->name);
 		} else {
-			DRM_DEBUG_KMS("[CONNECTOR:%d:%s] to [NOCRTC]\n",
-				      connector->base.id, connector->name);
+			drm_dbg_kms(dev, "[CONNECTOR:%d:%s] to [NOCRTC]\n",
+				    connector->base.id, connector->name);
 		}
 	}
 	drm_connector_list_iter_end(&conn_iter);
@@ -758,23 +766,24 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
 
 	if (mode_changed) {
 		if (drm_helper_crtc_in_use(set->crtc)) {
-			DRM_DEBUG_KMS("attempting to set mode from"
-					" userspace\n");
+			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);
 			set->crtc->primary->fb = set->fb;
 			if (!drm_crtc_helper_set_mode(set->crtc, set->mode,
 						      set->x, set->y,
 						      save_set.fb)) {
-				DRM_ERROR("failed to set mode on [CRTC:%d:%s]\n",
-					  set->crtc->base.id, set->crtc->name);
+				drm_err(dev, "[CRTC:%d:%s] failed to set mode\n",
+					set->crtc->base.id, set->crtc->name);
 				set->crtc->primary->fb = save_set.fb;
 				ret = -EINVAL;
 				goto fail;
 			}
-			DRM_DEBUG_KMS("Setting connector DPMS state to on\n");
+			drm_dbg_kms(dev, "[CRTC:%d:%s] Setting connector DPMS state to on\n",
+				    set->crtc->base.id, set->crtc->name);
 			for (i = 0; i < set->num_connectors; i++) {
-				DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS on\n", set->connectors[i]->base.id,
-					      set->connectors[i]->name);
+				drm_dbg_kms(dev, "\t[CONNECTOR:%d:%s] set DPMS on\n", set->connectors[i]->base.id,
+					    set->connectors[i]->name);
 				set->connectors[i]->funcs->dpms(set->connectors[i], DRM_MODE_DPMS_ON);
 			}
 		}
@@ -823,7 +832,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
 	if (mode_changed &&
 	    !drm_crtc_helper_set_mode(save_set.crtc, save_set.mode, save_set.x,
 				      save_set.y, save_set.fb))
-		DRM_ERROR("failed to restore config after modeset failure\n");
+		drm_err(dev, "failed to restore config after modeset failure\n");
 
 	kfree(save_connector_encoders);
 	kfree(save_encoder_crtcs);
@@ -905,7 +914,7 @@ int drm_helper_connector_dpms(struct drm_connector *connector, int mode)
 	struct drm_crtc *crtc = encoder ? encoder->crtc : NULL;
 	int old_dpms, encoder_dpms = DRM_MODE_DPMS_OFF;
 
-	WARN_ON(drm_drv_uses_atomic_modeset(connector->dev));
+	drm_WARN_ON(connector->dev, drm_drv_uses_atomic_modeset(connector->dev));
 
 	if (mode == connector->dpms)
 		return 0;
@@ -980,7 +989,7 @@ void drm_helper_resume_force_mode(struct drm_device *dev)
 	int encoder_dpms;
 	bool ret;
 
-	WARN_ON(drm_drv_uses_atomic_modeset(dev));
+	drm_WARN_ON(dev, drm_drv_uses_atomic_modeset(dev));
 
 	drm_modeset_lock_all(dev);
 	drm_for_each_crtc(crtc, dev) {
@@ -993,7 +1002,7 @@ void drm_helper_resume_force_mode(struct drm_device *dev)
 
 		/* Restoring the old config should never fail! */
 		if (ret == false)
-			DRM_ERROR("failed to set mode on crtc %p\n", crtc);
+			drm_err(dev, "failed to set mode on crtc %p\n", crtc);
 
 		/* Turn off outputs that were already powered off */
 		if (drm_helper_choose_crtc_dpms(crtc)) {
-- 
2.39.2


  parent reply	other threads:[~2024-04-08  9:24 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-08  9:23 [PATCH v2 0/7] drm: debug logging improvements Jani Nikula
2024-04-08  9:23 ` [PATCH v2 1/7] drm/probe-helper: switch to drm device based logging Jani Nikula
2024-04-08  9:23 ` [PATCH v2 2/7] drm/modes: switch to drm device based error logging Jani Nikula
2024-04-08  9:23 ` [PATCH v2 3/7] drm/sysfs: switch to drm device based logging Jani Nikula
2024-04-08  9:23 ` [PATCH v2 4/7] drm/client: switch to drm device based logging, and more Jani Nikula
2024-04-08  9:24 ` [PATCH v2 5/7] drm/crtc: switch to drm device based logging Jani Nikula
2024-04-08  9:24 ` Jani Nikula [this message]
2024-04-08 12:38   ` [PATCH v2 6/7] drm/crtc-helper: switch to drm device based logging and warns Ville Syrjälä
2024-04-08  9:24 ` [PATCH v2 7/7] drm: prefer DRM_MODE_FMT/ARG over drm_mode_debug_printmodeline() Jani Nikula
2024-04-08 12:51   ` Ville Syrjälä
2024-04-08 11:15 ` [PATCH v2 0/7] drm: debug logging improvements Thomas Zimmermann
2024-04-08 14:13 ` ✓ CI.Patch_applied: success for drm: debug logging improvements (rev2) Patchwork
2024-04-08 14:14 ` ✗ CI.checkpatch: warning " Patchwork
2024-04-08 14:15 ` ✓ CI.KUnit: success " Patchwork
2024-04-08 14:32 ` ✓ CI.Build: " Patchwork
2024-04-08 14:37 ` ✓ CI.Hooks: " Patchwork
2024-04-08 14:38 ` ✗ CI.checksparse: warning " Patchwork
2024-04-08 15:01 ` ✓ CI.BAT: success " Patchwork
2024-04-08 16:11 ` ✗ CI.FULL: failure " Patchwork
2024-04-08 18:39 ` ✗ Fi.CI.CHECKPATCH: warning " Patchwork
2024-04-08 18:59 ` ✗ Fi.CI.BAT: failure " Patchwork
2024-04-08 23:33 ` ✗ Fi.CI.CHECKPATCH: warning for drm: debug logging improvements (rev3) Patchwork
2024-04-08 23:33 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-04-08 23:44 ` ✗ Fi.CI.BAT: failure " Patchwork
2024-04-09 14:44 ` ✗ Fi.CI.CHECKPATCH: warning for drm: debug logging improvements (rev4) Patchwork
2024-04-09 14:44 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-04-09 14:51 ` ✓ Fi.CI.BAT: success " Patchwork
2024-04-10 17:14 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-04-15 13:44 ` [PATCH v2 0/7] drm: debug logging improvements Jani Nikula

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=b8557c4b2db0e5c931a6d82b5cc8ac5f3a3e1a77.1712568037.git.jani.nikula@intel.com \
    --to=jani.nikula@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=tzimmermann@suse.de \
    --cc=ville.syrjala@linux.intel.com \
    /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.