* [PATCH] drm/amd/display: Convert some legacy DRM debug macros into appropriate categories
@ 2024-05-28 12:57 Tvrtko Ursulin
2024-05-30 19:57 ` Hamza Mahfooz
0 siblings, 1 reply; 3+ messages in thread
From: Tvrtko Ursulin @ 2024-05-28 12:57 UTC (permalink / raw)
To: amd-gfx; +Cc: kernel-dev, Tvrtko Ursulin
From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Currently when one enables driver debugging dmesg gets spammed, at I
suspect vblank rate, with messages like:
[drm:amdgpu_dm_atomic_check [amdgpu]] MPO enablement requested on crtc:[00000000f073c3bb]
Fix if by converting some logging from deprecated and incorrect
DRM_DEBUG_DRIVER to drm_dbg_atomic. Plus some localized drive-by changes
to drm_dbg_kms.
By no means an exhaustive conversion but at least it allows turning on
driver debug selectively.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 73 ++++++++++---------
1 file changed, 39 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 975feb1c69b8..bf9db488eee6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2363,13 +2363,13 @@ static int detect_mst_link_for_all_connectors(struct drm_device *dev)
aconnector = to_amdgpu_dm_connector(connector);
if (aconnector->dc_link->type == dc_connection_mst_branch &&
aconnector->mst_mgr.aux) {
- DRM_DEBUG_DRIVER("DM_MST: starting TM on aconnector: %p [id: %d]\n",
+ drm_dbg_kms(dev, "DM_MST: starting TM on aconnector: %p [id: %d]\n",
aconnector,
aconnector->base.base.id);
ret = drm_dp_mst_topology_mgr_set_mst(&aconnector->mst_mgr, true);
if (ret < 0) {
- DRM_ERROR("DM_MST: Failed to start MST\n");
+ drm_err(dev, "DM_MST: Failed to start MST\n");
aconnector->dc_link->type =
dc_connection_single;
ret = dm_helpers_dp_mst_stop_top_mgr(aconnector->dc_link->ctx,
@@ -3298,15 +3298,15 @@ void amdgpu_dm_update_connector_after_detect(
* We got a DP short pulse (Link Loss, DP CTS, etc...).
* Do nothing!!
*/
- DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: dc_sink didn't change.\n",
- aconnector->connector_id);
+ drm_dbg_kms(dev, "DCHPD: connector_id=%d: dc_sink didn't change.\n",
+ aconnector->connector_id);
if (sink)
dc_sink_release(sink);
return;
}
- DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: Old sink=%p New sink=%p\n",
- aconnector->connector_id, aconnector->dc_sink, sink);
+ drm_dbg_kms(dev, "DCHPD: connector_id=%d: Old sink=%p New sink=%p\n",
+ aconnector->connector_id, aconnector->dc_sink, sink);
mutex_lock(&dev->mode_config.mutex);
@@ -9191,7 +9191,9 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state,
if (amdgpu_dm_crtc_modeset_required(new_crtc_state, dm_new_crtc_state->stream, dm_old_crtc_state->stream)) {
- DRM_DEBUG_ATOMIC("Atomic commit: SET crtc id %d: [%p]\n", acrtc->crtc_id, acrtc);
+ drm_dbg_atomic(dev,
+ "Atomic commit: SET crtc id %d: [%p]\n",
+ acrtc->crtc_id, acrtc);
if (!dm_new_crtc_state->stream) {
/*
@@ -9209,8 +9211,9 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state,
* have a sink to keep the pipe running so that
* hw state is consistent with the sw state
*/
- DRM_DEBUG_DRIVER("%s: Failed to create new stream for crtc %d\n",
- __func__, acrtc->base.base.id);
+ drm_dbg_atomic(dev,
+ "Failed to create new stream for crtc %d\n",
+ acrtc->base.base.id);
continue;
}
@@ -9224,7 +9227,9 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state,
crtc->hwmode = new_crtc_state->mode;
mode_set_reset_required = true;
} else if (modereset_required(new_crtc_state)) {
- DRM_DEBUG_ATOMIC("Atomic commit: RESET. crtc id %d:[%p]\n", acrtc->crtc_id, acrtc);
+ drm_dbg_atomic(dev,
+ "Atomic commit: RESET. crtc id %d:[%p]\n",
+ acrtc->crtc_id, acrtc);
/* i.e. reset mode */
if (dm_old_crtc_state->stream)
remove_stream(adev, acrtc, dm_old_crtc_state->stream);
@@ -9679,7 +9684,7 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
#endif
if (amdgpu_dm_crtc_configure_crc_source(
crtc, dm_new_crtc_state, cur_crc_src))
- DRM_DEBUG_DRIVER("Failed to configure crc source");
+ drm_dbg_atomic(dev, "Failed to configure crc source");
}
}
#endif
@@ -10851,7 +10856,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
ret = drm_atomic_helper_check_modeset(dev, state);
if (ret) {
- DRM_DEBUG_DRIVER("drm_atomic_helper_check_modeset() failed\n");
+ drm_dbg_atomic(dev, "drm_atomic_helper_check_modeset() failed\n");
goto fail;
}
@@ -10866,7 +10871,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
new_crtc_state = drm_atomic_get_crtc_state(state, new_con_state->crtc);
if (IS_ERR(new_crtc_state)) {
- DRM_DEBUG_DRIVER("drm_atomic_get_crtc_state() failed\n");
+ drm_dbg_atomic(dev, "drm_atomic_get_crtc_state() failed\n");
ret = PTR_ERR(new_crtc_state);
goto fail;
}
@@ -10881,7 +10886,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
if (drm_atomic_crtc_needs_modeset(new_crtc_state)) {
ret = add_affected_mst_dsc_crtcs(state, crtc);
if (ret) {
- DRM_DEBUG_DRIVER("add_affected_mst_dsc_crtcs() failed\n");
+ drm_dbg_atomic(dev, "add_affected_mst_dsc_crtcs() failed\n");
goto fail;
}
}
@@ -10898,7 +10903,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
ret = amdgpu_dm_verify_lut_sizes(new_crtc_state);
if (ret) {
- DRM_DEBUG_DRIVER("amdgpu_dm_verify_lut_sizes() failed\n");
+ drm_dbg_atomic(dev, "amdgpu_dm_verify_lut_sizes() failed\n");
goto fail;
}
@@ -10907,13 +10912,13 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
ret = drm_atomic_add_affected_connectors(state, crtc);
if (ret) {
- DRM_DEBUG_DRIVER("drm_atomic_add_affected_connectors() failed\n");
+ drm_dbg_atomic(dev, "drm_atomic_add_affected_connectors() failed\n");
goto fail;
}
ret = drm_atomic_add_affected_planes(state, crtc);
if (ret) {
- DRM_DEBUG_DRIVER("drm_atomic_add_affected_planes() failed\n");
+ drm_dbg_atomic(dev, "drm_atomic_add_affected_planes() failed\n");
goto fail;
}
@@ -10952,7 +10957,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
if (IS_ERR(new_plane_state)) {
ret = PTR_ERR(new_plane_state);
- DRM_DEBUG_DRIVER("new_plane_state is BAD\n");
+ drm_dbg_atomic(dev, "new_plane_state is BAD\n");
goto fail;
}
}
@@ -10984,7 +10989,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
&lock_and_validation_needed,
&is_top_most_overlay);
if (ret) {
- DRM_DEBUG_DRIVER("dm_update_plane_state() failed\n");
+ drm_dbg_atomic(dev, "dm_update_plane_state() failed\n");
goto fail;
}
}
@@ -10997,7 +11002,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
false,
&lock_and_validation_needed);
if (ret) {
- DRM_DEBUG_DRIVER("DISABLE: dm_update_crtc_state() failed\n");
+ drm_dbg_atomic(dev, "DISABLE: dm_update_crtc_state() failed\n");
goto fail;
}
}
@@ -11010,7 +11015,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
true,
&lock_and_validation_needed);
if (ret) {
- DRM_DEBUG_DRIVER("ENABLE: dm_update_crtc_state() failed\n");
+ drm_dbg_atomic(dev, "ENABLE: dm_update_crtc_state() failed\n");
goto fail;
}
}
@@ -11024,7 +11029,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
&lock_and_validation_needed,
&is_top_most_overlay);
if (ret) {
- DRM_DEBUG_DRIVER("dm_update_plane_state() failed\n");
+ drm_dbg_atomic(dev, "dm_update_plane_state() failed\n");
goto fail;
}
}
@@ -11040,21 +11045,21 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
/* Run this here since we want to validate the streams we created */
ret = drm_atomic_helper_check_planes(dev, state);
if (ret) {
- DRM_DEBUG_DRIVER("drm_atomic_helper_check_planes() failed\n");
+ drm_dbg_atomic(dev, "drm_atomic_helper_check_planes() failed\n");
goto fail;
}
for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
if (dm_new_crtc_state->mpo_requested)
- DRM_DEBUG_DRIVER("MPO enablement requested on crtc:[%p]\n", crtc);
+ drm_dbg_atomic(dev, "MPO enablement requested on crtc:[%p]\n", crtc);
}
/* Check cursor planes scaling */
for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
ret = dm_check_crtc_cursor(state, crtc, new_crtc_state);
if (ret) {
- DRM_DEBUG_DRIVER("dm_check_crtc_cursor() failed\n");
+ drm_dbg_atomic(dev, "dm_check_crtc_cursor() failed\n");
goto fail;
}
}
@@ -11137,13 +11142,13 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
if (lock_and_validation_needed) {
ret = dm_atomic_get_state(state, &dm_state);
if (ret) {
- DRM_DEBUG_DRIVER("dm_atomic_get_state() failed\n");
+ drm_dbg_atomic(dev, "dm_atomic_get_state() failed\n");
goto fail;
}
ret = do_aquire_global_lock(dev, state);
if (ret) {
- DRM_DEBUG_DRIVER("do_aquire_global_lock() failed\n");
+ drm_dbg_atomic(dev, "do_aquire_global_lock() failed\n");
goto fail;
}
@@ -11151,7 +11156,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
if (dc_resource_is_dsc_encoding_supported(dc)) {
ret = compute_mst_dsc_configs_for_state(state, dm_state->context, vars);
if (ret) {
- DRM_DEBUG_DRIVER("compute_mst_dsc_configs_for_state() failed\n");
+ drm_dbg_atomic(dev, "compute_mst_dsc_configs_for_state() failed\n");
ret = -EINVAL;
goto fail;
}
@@ -11160,7 +11165,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
ret = dm_update_mst_vcpi_slots_for_dsc(state, dm_state->context, vars);
if (ret) {
- DRM_DEBUG_DRIVER("dm_update_mst_vcpi_slots_for_dsc() failed\n");
+ drm_dbg_atomic(dev, "dm_update_mst_vcpi_slots_for_dsc() failed\n");
goto fail;
}
@@ -11172,12 +11177,12 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
*/
ret = drm_dp_mst_atomic_check(state);
if (ret) {
- DRM_DEBUG_DRIVER("drm_dp_mst_atomic_check() failed\n");
+ drm_dbg_atomic(dev, "drm_dp_mst_atomic_check() failed\n");
goto fail;
}
status = dc_validate_global_state(dc, dm_state->context, true);
if (status != DC_OK) {
- DRM_DEBUG_DRIVER("DC global validation failure: %s (%d)",
+ drm_dbg_atomic(dev, "DC global validation failure: %s (%d)",
dc_status_to_str(status), status);
ret = -EINVAL;
goto fail;
@@ -11255,11 +11260,11 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
fail:
if (ret == -EDEADLK)
- DRM_DEBUG_DRIVER("Atomic check stopped to avoid deadlock.\n");
+ drm_dbg_atomic(dev, "Atomic check stopped to avoid deadlock.\n");
else if (ret == -EINTR || ret == -EAGAIN || ret == -ERESTARTSYS)
- DRM_DEBUG_DRIVER("Atomic check stopped due to signal.\n");
+ drm_dbg_atomic(dev, "Atomic check stopped due to signal.\n");
else
- DRM_DEBUG_DRIVER("Atomic check failed with err: %d\n", ret);
+ drm_dbg_atomic(dev, "Atomic check failed with err: %d\n", ret);
trace_amdgpu_dm_atomic_check_finish(state, ret);
--
2.44.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/amd/display: Convert some legacy DRM debug macros into appropriate categories
2024-05-28 12:57 [PATCH] drm/amd/display: Convert some legacy DRM debug macros into appropriate categories Tvrtko Ursulin
@ 2024-05-30 19:57 ` Hamza Mahfooz
2024-05-31 17:10 ` Tvrtko Ursulin
0 siblings, 1 reply; 3+ messages in thread
From: Hamza Mahfooz @ 2024-05-30 19:57 UTC (permalink / raw)
To: Tvrtko Ursulin, amd-gfx; +Cc: kernel-dev, Tvrtko Ursulin
On 5/28/24 08:57, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>
> Currently when one enables driver debugging dmesg gets spammed, at I
> suspect vblank rate, with messages like:
>
> [drm:amdgpu_dm_atomic_check [amdgpu]] MPO enablement requested on crtc:[00000000f073c3bb]
>
> Fix if by converting some logging from deprecated and incorrect
> DRM_DEBUG_DRIVER to drm_dbg_atomic. Plus some localized drive-by changes
> to drm_dbg_kms.
>
> By no means an exhaustive conversion but at least it allows turning on
> driver debug selectively.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Applied, thanks!
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 73 ++++++++++---------
> 1 file changed, 39 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 975feb1c69b8..bf9db488eee6 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2363,13 +2363,13 @@ static int detect_mst_link_for_all_connectors(struct drm_device *dev)
> aconnector = to_amdgpu_dm_connector(connector);
> if (aconnector->dc_link->type == dc_connection_mst_branch &&
> aconnector->mst_mgr.aux) {
> - DRM_DEBUG_DRIVER("DM_MST: starting TM on aconnector: %p [id: %d]\n",
> + drm_dbg_kms(dev, "DM_MST: starting TM on aconnector: %p [id: %d]\n",
> aconnector,
> aconnector->base.base.id);
>
> ret = drm_dp_mst_topology_mgr_set_mst(&aconnector->mst_mgr, true);
> if (ret < 0) {
> - DRM_ERROR("DM_MST: Failed to start MST\n");
> + drm_err(dev, "DM_MST: Failed to start MST\n");
> aconnector->dc_link->type =
> dc_connection_single;
> ret = dm_helpers_dp_mst_stop_top_mgr(aconnector->dc_link->ctx,
> @@ -3298,15 +3298,15 @@ void amdgpu_dm_update_connector_after_detect(
> * We got a DP short pulse (Link Loss, DP CTS, etc...).
> * Do nothing!!
> */
> - DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: dc_sink didn't change.\n",
> - aconnector->connector_id);
> + drm_dbg_kms(dev, "DCHPD: connector_id=%d: dc_sink didn't change.\n",
> + aconnector->connector_id);
> if (sink)
> dc_sink_release(sink);
> return;
> }
>
> - DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: Old sink=%p New sink=%p\n",
> - aconnector->connector_id, aconnector->dc_sink, sink);
> + drm_dbg_kms(dev, "DCHPD: connector_id=%d: Old sink=%p New sink=%p\n",
> + aconnector->connector_id, aconnector->dc_sink, sink);
>
> mutex_lock(&dev->mode_config.mutex);
>
> @@ -9191,7 +9191,9 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state,
>
> if (amdgpu_dm_crtc_modeset_required(new_crtc_state, dm_new_crtc_state->stream, dm_old_crtc_state->stream)) {
>
> - DRM_DEBUG_ATOMIC("Atomic commit: SET crtc id %d: [%p]\n", acrtc->crtc_id, acrtc);
> + drm_dbg_atomic(dev,
> + "Atomic commit: SET crtc id %d: [%p]\n",
> + acrtc->crtc_id, acrtc);
>
> if (!dm_new_crtc_state->stream) {
> /*
> @@ -9209,8 +9211,9 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state,
> * have a sink to keep the pipe running so that
> * hw state is consistent with the sw state
> */
> - DRM_DEBUG_DRIVER("%s: Failed to create new stream for crtc %d\n",
> - __func__, acrtc->base.base.id);
> + drm_dbg_atomic(dev,
> + "Failed to create new stream for crtc %d\n",
> + acrtc->base.base.id);
> continue;
> }
>
> @@ -9224,7 +9227,9 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state,
> crtc->hwmode = new_crtc_state->mode;
> mode_set_reset_required = true;
> } else if (modereset_required(new_crtc_state)) {
> - DRM_DEBUG_ATOMIC("Atomic commit: RESET. crtc id %d:[%p]\n", acrtc->crtc_id, acrtc);
> + drm_dbg_atomic(dev,
> + "Atomic commit: RESET. crtc id %d:[%p]\n",
> + acrtc->crtc_id, acrtc);
> /* i.e. reset mode */
> if (dm_old_crtc_state->stream)
> remove_stream(adev, acrtc, dm_old_crtc_state->stream);
> @@ -9679,7 +9684,7 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
> #endif
> if (amdgpu_dm_crtc_configure_crc_source(
> crtc, dm_new_crtc_state, cur_crc_src))
> - DRM_DEBUG_DRIVER("Failed to configure crc source");
> + drm_dbg_atomic(dev, "Failed to configure crc source");
> }
> }
> #endif
> @@ -10851,7 +10856,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
>
> ret = drm_atomic_helper_check_modeset(dev, state);
> if (ret) {
> - DRM_DEBUG_DRIVER("drm_atomic_helper_check_modeset() failed\n");
> + drm_dbg_atomic(dev, "drm_atomic_helper_check_modeset() failed\n");
> goto fail;
> }
>
> @@ -10866,7 +10871,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
>
> new_crtc_state = drm_atomic_get_crtc_state(state, new_con_state->crtc);
> if (IS_ERR(new_crtc_state)) {
> - DRM_DEBUG_DRIVER("drm_atomic_get_crtc_state() failed\n");
> + drm_dbg_atomic(dev, "drm_atomic_get_crtc_state() failed\n");
> ret = PTR_ERR(new_crtc_state);
> goto fail;
> }
> @@ -10881,7 +10886,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
> if (drm_atomic_crtc_needs_modeset(new_crtc_state)) {
> ret = add_affected_mst_dsc_crtcs(state, crtc);
> if (ret) {
> - DRM_DEBUG_DRIVER("add_affected_mst_dsc_crtcs() failed\n");
> + drm_dbg_atomic(dev, "add_affected_mst_dsc_crtcs() failed\n");
> goto fail;
> }
> }
> @@ -10898,7 +10903,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
>
> ret = amdgpu_dm_verify_lut_sizes(new_crtc_state);
> if (ret) {
> - DRM_DEBUG_DRIVER("amdgpu_dm_verify_lut_sizes() failed\n");
> + drm_dbg_atomic(dev, "amdgpu_dm_verify_lut_sizes() failed\n");
> goto fail;
> }
>
> @@ -10907,13 +10912,13 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
>
> ret = drm_atomic_add_affected_connectors(state, crtc);
> if (ret) {
> - DRM_DEBUG_DRIVER("drm_atomic_add_affected_connectors() failed\n");
> + drm_dbg_atomic(dev, "drm_atomic_add_affected_connectors() failed\n");
> goto fail;
> }
>
> ret = drm_atomic_add_affected_planes(state, crtc);
> if (ret) {
> - DRM_DEBUG_DRIVER("drm_atomic_add_affected_planes() failed\n");
> + drm_dbg_atomic(dev, "drm_atomic_add_affected_planes() failed\n");
> goto fail;
> }
>
> @@ -10952,7 +10957,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
>
> if (IS_ERR(new_plane_state)) {
> ret = PTR_ERR(new_plane_state);
> - DRM_DEBUG_DRIVER("new_plane_state is BAD\n");
> + drm_dbg_atomic(dev, "new_plane_state is BAD\n");
> goto fail;
> }
> }
> @@ -10984,7 +10989,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
> &lock_and_validation_needed,
> &is_top_most_overlay);
> if (ret) {
> - DRM_DEBUG_DRIVER("dm_update_plane_state() failed\n");
> + drm_dbg_atomic(dev, "dm_update_plane_state() failed\n");
> goto fail;
> }
> }
> @@ -10997,7 +11002,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
> false,
> &lock_and_validation_needed);
> if (ret) {
> - DRM_DEBUG_DRIVER("DISABLE: dm_update_crtc_state() failed\n");
> + drm_dbg_atomic(dev, "DISABLE: dm_update_crtc_state() failed\n");
> goto fail;
> }
> }
> @@ -11010,7 +11015,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
> true,
> &lock_and_validation_needed);
> if (ret) {
> - DRM_DEBUG_DRIVER("ENABLE: dm_update_crtc_state() failed\n");
> + drm_dbg_atomic(dev, "ENABLE: dm_update_crtc_state() failed\n");
> goto fail;
> }
> }
> @@ -11024,7 +11029,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
> &lock_and_validation_needed,
> &is_top_most_overlay);
> if (ret) {
> - DRM_DEBUG_DRIVER("dm_update_plane_state() failed\n");
> + drm_dbg_atomic(dev, "dm_update_plane_state() failed\n");
> goto fail;
> }
> }
> @@ -11040,21 +11045,21 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
> /* Run this here since we want to validate the streams we created */
> ret = drm_atomic_helper_check_planes(dev, state);
> if (ret) {
> - DRM_DEBUG_DRIVER("drm_atomic_helper_check_planes() failed\n");
> + drm_dbg_atomic(dev, "drm_atomic_helper_check_planes() failed\n");
> goto fail;
> }
>
> for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
> if (dm_new_crtc_state->mpo_requested)
> - DRM_DEBUG_DRIVER("MPO enablement requested on crtc:[%p]\n", crtc);
> + drm_dbg_atomic(dev, "MPO enablement requested on crtc:[%p]\n", crtc);
> }
>
> /* Check cursor planes scaling */
> for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> ret = dm_check_crtc_cursor(state, crtc, new_crtc_state);
> if (ret) {
> - DRM_DEBUG_DRIVER("dm_check_crtc_cursor() failed\n");
> + drm_dbg_atomic(dev, "dm_check_crtc_cursor() failed\n");
> goto fail;
> }
> }
> @@ -11137,13 +11142,13 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
> if (lock_and_validation_needed) {
> ret = dm_atomic_get_state(state, &dm_state);
> if (ret) {
> - DRM_DEBUG_DRIVER("dm_atomic_get_state() failed\n");
> + drm_dbg_atomic(dev, "dm_atomic_get_state() failed\n");
> goto fail;
> }
>
> ret = do_aquire_global_lock(dev, state);
> if (ret) {
> - DRM_DEBUG_DRIVER("do_aquire_global_lock() failed\n");
> + drm_dbg_atomic(dev, "do_aquire_global_lock() failed\n");
> goto fail;
> }
>
> @@ -11151,7 +11156,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
> if (dc_resource_is_dsc_encoding_supported(dc)) {
> ret = compute_mst_dsc_configs_for_state(state, dm_state->context, vars);
> if (ret) {
> - DRM_DEBUG_DRIVER("compute_mst_dsc_configs_for_state() failed\n");
> + drm_dbg_atomic(dev, "compute_mst_dsc_configs_for_state() failed\n");
> ret = -EINVAL;
> goto fail;
> }
> @@ -11160,7 +11165,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
>
> ret = dm_update_mst_vcpi_slots_for_dsc(state, dm_state->context, vars);
> if (ret) {
> - DRM_DEBUG_DRIVER("dm_update_mst_vcpi_slots_for_dsc() failed\n");
> + drm_dbg_atomic(dev, "dm_update_mst_vcpi_slots_for_dsc() failed\n");
> goto fail;
> }
>
> @@ -11172,12 +11177,12 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
> */
> ret = drm_dp_mst_atomic_check(state);
> if (ret) {
> - DRM_DEBUG_DRIVER("drm_dp_mst_atomic_check() failed\n");
> + drm_dbg_atomic(dev, "drm_dp_mst_atomic_check() failed\n");
> goto fail;
> }
> status = dc_validate_global_state(dc, dm_state->context, true);
> if (status != DC_OK) {
> - DRM_DEBUG_DRIVER("DC global validation failure: %s (%d)",
> + drm_dbg_atomic(dev, "DC global validation failure: %s (%d)",
> dc_status_to_str(status), status);
> ret = -EINVAL;
> goto fail;
> @@ -11255,11 +11260,11 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
>
> fail:
> if (ret == -EDEADLK)
> - DRM_DEBUG_DRIVER("Atomic check stopped to avoid deadlock.\n");
> + drm_dbg_atomic(dev, "Atomic check stopped to avoid deadlock.\n");
> else if (ret == -EINTR || ret == -EAGAIN || ret == -ERESTARTSYS)
> - DRM_DEBUG_DRIVER("Atomic check stopped due to signal.\n");
> + drm_dbg_atomic(dev, "Atomic check stopped due to signal.\n");
> else
> - DRM_DEBUG_DRIVER("Atomic check failed with err: %d\n", ret);
> + drm_dbg_atomic(dev, "Atomic check failed with err: %d\n", ret);
>
> trace_amdgpu_dm_atomic_check_finish(state, ret);
>
--
Hamza
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/amd/display: Convert some legacy DRM debug macros into appropriate categories
2024-05-30 19:57 ` Hamza Mahfooz
@ 2024-05-31 17:10 ` Tvrtko Ursulin
0 siblings, 0 replies; 3+ messages in thread
From: Tvrtko Ursulin @ 2024-05-31 17:10 UTC (permalink / raw)
To: Hamza Mahfooz, Tvrtko Ursulin, amd-gfx; +Cc: kernel-dev
On 30/05/2024 20:57, Hamza Mahfooz wrote:
> On 5/28/24 08:57, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>>
>> Currently when one enables driver debugging dmesg gets spammed, at I
>> suspect vblank rate, with messages like:
>>
>> [drm:amdgpu_dm_atomic_check [amdgpu]] MPO enablement requested on
>> crtc:[00000000f073c3bb]
>>
>> Fix if by converting some logging from deprecated and incorrect
>> DRM_DEBUG_DRIVER to drm_dbg_atomic. Plus some localized drive-by changes
>> to drm_dbg_kms.
>>
>> By no means an exhaustive conversion but at least it allows turning on
>> driver debug selectively.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>
> Applied, thanks!
Oh so fast for this one, thank you!
Regards,
Tvrtko
>
>> ---
>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 73 ++++++++++---------
>> 1 file changed, 39 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index 975feb1c69b8..bf9db488eee6 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -2363,13 +2363,13 @@ static int
>> detect_mst_link_for_all_connectors(struct drm_device *dev)
>> aconnector = to_amdgpu_dm_connector(connector);
>> if (aconnector->dc_link->type == dc_connection_mst_branch &&
>> aconnector->mst_mgr.aux) {
>> - DRM_DEBUG_DRIVER("DM_MST: starting TM on aconnector: %p
>> [id: %d]\n",
>> + drm_dbg_kms(dev, "DM_MST: starting TM on aconnector: %p
>> [id: %d]\n",
>> aconnector,
>> aconnector->base.base.id);
>> ret =
>> drm_dp_mst_topology_mgr_set_mst(&aconnector->mst_mgr, true);
>> if (ret < 0) {
>> - DRM_ERROR("DM_MST: Failed to start MST\n");
>> + drm_err(dev, "DM_MST: Failed to start MST\n");
>> aconnector->dc_link->type =
>> dc_connection_single;
>> ret =
>> dm_helpers_dp_mst_stop_top_mgr(aconnector->dc_link->ctx,
>> @@ -3298,15 +3298,15 @@ void amdgpu_dm_update_connector_after_detect(
>> * We got a DP short pulse (Link Loss, DP CTS, etc...).
>> * Do nothing!!
>> */
>> - DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: dc_sink didn't
>> change.\n",
>> - aconnector->connector_id);
>> + drm_dbg_kms(dev, "DCHPD: connector_id=%d: dc_sink didn't
>> change.\n",
>> + aconnector->connector_id);
>> if (sink)
>> dc_sink_release(sink);
>> return;
>> }
>> - DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: Old sink=%p New
>> sink=%p\n",
>> - aconnector->connector_id, aconnector->dc_sink, sink);
>> + drm_dbg_kms(dev, "DCHPD: connector_id=%d: Old sink=%p New
>> sink=%p\n",
>> + aconnector->connector_id, aconnector->dc_sink, sink);
>> mutex_lock(&dev->mode_config.mutex);
>> @@ -9191,7 +9191,9 @@ static void amdgpu_dm_commit_streams(struct
>> drm_atomic_state *state,
>> if (amdgpu_dm_crtc_modeset_required(new_crtc_state,
>> dm_new_crtc_state->stream, dm_old_crtc_state->stream)) {
>> - DRM_DEBUG_ATOMIC("Atomic commit: SET crtc id %d: [%p]\n",
>> acrtc->crtc_id, acrtc);
>> + drm_dbg_atomic(dev,
>> + "Atomic commit: SET crtc id %d: [%p]\n",
>> + acrtc->crtc_id, acrtc);
>> if (!dm_new_crtc_state->stream) {
>> /*
>> @@ -9209,8 +9211,9 @@ static void amdgpu_dm_commit_streams(struct
>> drm_atomic_state *state,
>> * have a sink to keep the pipe running so that
>> * hw state is consistent with the sw state
>> */
>> - DRM_DEBUG_DRIVER("%s: Failed to create new stream for
>> crtc %d\n",
>> - __func__, acrtc->base.base.id);
>> + drm_dbg_atomic(dev,
>> + "Failed to create new stream for crtc %d\n",
>> + acrtc->base.base.id);
>> continue;
>> }
>> @@ -9224,7 +9227,9 @@ static void amdgpu_dm_commit_streams(struct
>> drm_atomic_state *state,
>> crtc->hwmode = new_crtc_state->mode;
>> mode_set_reset_required = true;
>> } else if (modereset_required(new_crtc_state)) {
>> - DRM_DEBUG_ATOMIC("Atomic commit: RESET. crtc id
>> %d:[%p]\n", acrtc->crtc_id, acrtc);
>> + drm_dbg_atomic(dev,
>> + "Atomic commit: RESET. crtc id %d:[%p]\n",
>> + acrtc->crtc_id, acrtc);
>> /* i.e. reset mode */
>> if (dm_old_crtc_state->stream)
>> remove_stream(adev, acrtc, dm_old_crtc_state->stream);
>> @@ -9679,7 +9684,7 @@ static void amdgpu_dm_atomic_commit_tail(struct
>> drm_atomic_state *state)
>> #endif
>> if (amdgpu_dm_crtc_configure_crc_source(
>> crtc, dm_new_crtc_state, cur_crc_src))
>> - DRM_DEBUG_DRIVER("Failed to configure crc source");
>> + drm_dbg_atomic(dev, "Failed to configure crc
>> source");
>> }
>> }
>> #endif
>> @@ -10851,7 +10856,7 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> ret = drm_atomic_helper_check_modeset(dev, state);
>> if (ret) {
>> - DRM_DEBUG_DRIVER("drm_atomic_helper_check_modeset() failed\n");
>> + drm_dbg_atomic(dev, "drm_atomic_helper_check_modeset()
>> failed\n");
>> goto fail;
>> }
>> @@ -10866,7 +10871,7 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> new_crtc_state = drm_atomic_get_crtc_state(state,
>> new_con_state->crtc);
>> if (IS_ERR(new_crtc_state)) {
>> - DRM_DEBUG_DRIVER("drm_atomic_get_crtc_state() failed\n");
>> + drm_dbg_atomic(dev, "drm_atomic_get_crtc_state() failed\n");
>> ret = PTR_ERR(new_crtc_state);
>> goto fail;
>> }
>> @@ -10881,7 +10886,7 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> if (drm_atomic_crtc_needs_modeset(new_crtc_state)) {
>> ret = add_affected_mst_dsc_crtcs(state, crtc);
>> if (ret) {
>> - DRM_DEBUG_DRIVER("add_affected_mst_dsc_crtcs()
>> failed\n");
>> + drm_dbg_atomic(dev, "add_affected_mst_dsc_crtcs()
>> failed\n");
>> goto fail;
>> }
>> }
>> @@ -10898,7 +10903,7 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> ret = amdgpu_dm_verify_lut_sizes(new_crtc_state);
>> if (ret) {
>> - DRM_DEBUG_DRIVER("amdgpu_dm_verify_lut_sizes() failed\n");
>> + drm_dbg_atomic(dev, "amdgpu_dm_verify_lut_sizes()
>> failed\n");
>> goto fail;
>> }
>> @@ -10907,13 +10912,13 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> ret = drm_atomic_add_affected_connectors(state, crtc);
>> if (ret) {
>> - DRM_DEBUG_DRIVER("drm_atomic_add_affected_connectors()
>> failed\n");
>> + drm_dbg_atomic(dev, "drm_atomic_add_affected_connectors()
>> failed\n");
>> goto fail;
>> }
>> ret = drm_atomic_add_affected_planes(state, crtc);
>> if (ret) {
>> - DRM_DEBUG_DRIVER("drm_atomic_add_affected_planes()
>> failed\n");
>> + drm_dbg_atomic(dev, "drm_atomic_add_affected_planes()
>> failed\n");
>> goto fail;
>> }
>> @@ -10952,7 +10957,7 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> if (IS_ERR(new_plane_state)) {
>> ret = PTR_ERR(new_plane_state);
>> - DRM_DEBUG_DRIVER("new_plane_state is BAD\n");
>> + drm_dbg_atomic(dev, "new_plane_state is BAD\n");
>> goto fail;
>> }
>> }
>> @@ -10984,7 +10989,7 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> &lock_and_validation_needed,
>> &is_top_most_overlay);
>> if (ret) {
>> - DRM_DEBUG_DRIVER("dm_update_plane_state() failed\n");
>> + drm_dbg_atomic(dev, "dm_update_plane_state() failed\n");
>> goto fail;
>> }
>> }
>> @@ -10997,7 +11002,7 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> false,
>> &lock_and_validation_needed);
>> if (ret) {
>> - DRM_DEBUG_DRIVER("DISABLE: dm_update_crtc_state()
>> failed\n");
>> + drm_dbg_atomic(dev, "DISABLE: dm_update_crtc_state()
>> failed\n");
>> goto fail;
>> }
>> }
>> @@ -11010,7 +11015,7 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> true,
>> &lock_and_validation_needed);
>> if (ret) {
>> - DRM_DEBUG_DRIVER("ENABLE: dm_update_crtc_state() failed\n");
>> + drm_dbg_atomic(dev, "ENABLE: dm_update_crtc_state()
>> failed\n");
>> goto fail;
>> }
>> }
>> @@ -11024,7 +11029,7 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> &lock_and_validation_needed,
>> &is_top_most_overlay);
>> if (ret) {
>> - DRM_DEBUG_DRIVER("dm_update_plane_state() failed\n");
>> + drm_dbg_atomic(dev, "dm_update_plane_state() failed\n");
>> goto fail;
>> }
>> }
>> @@ -11040,21 +11045,21 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> /* Run this here since we want to validate the streams we
>> created */
>> ret = drm_atomic_helper_check_planes(dev, state);
>> if (ret) {
>> - DRM_DEBUG_DRIVER("drm_atomic_helper_check_planes() failed\n");
>> + drm_dbg_atomic(dev, "drm_atomic_helper_check_planes()
>> failed\n");
>> goto fail;
>> }
>> for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
>> dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
>> if (dm_new_crtc_state->mpo_requested)
>> - DRM_DEBUG_DRIVER("MPO enablement requested on
>> crtc:[%p]\n", crtc);
>> + drm_dbg_atomic(dev, "MPO enablement requested on
>> crtc:[%p]\n", crtc);
>> }
>> /* Check cursor planes scaling */
>> for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
>> ret = dm_check_crtc_cursor(state, crtc, new_crtc_state);
>> if (ret) {
>> - DRM_DEBUG_DRIVER("dm_check_crtc_cursor() failed\n");
>> + drm_dbg_atomic(dev, "dm_check_crtc_cursor() failed\n");
>> goto fail;
>> }
>> }
>> @@ -11137,13 +11142,13 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> if (lock_and_validation_needed) {
>> ret = dm_atomic_get_state(state, &dm_state);
>> if (ret) {
>> - DRM_DEBUG_DRIVER("dm_atomic_get_state() failed\n");
>> + drm_dbg_atomic(dev, "dm_atomic_get_state() failed\n");
>> goto fail;
>> }
>> ret = do_aquire_global_lock(dev, state);
>> if (ret) {
>> - DRM_DEBUG_DRIVER("do_aquire_global_lock() failed\n");
>> + drm_dbg_atomic(dev, "do_aquire_global_lock() failed\n");
>> goto fail;
>> }
>> @@ -11151,7 +11156,7 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> if (dc_resource_is_dsc_encoding_supported(dc)) {
>> ret = compute_mst_dsc_configs_for_state(state,
>> dm_state->context, vars);
>> if (ret) {
>> - DRM_DEBUG_DRIVER("compute_mst_dsc_configs_for_state()
>> failed\n");
>> + drm_dbg_atomic(dev,
>> "compute_mst_dsc_configs_for_state() failed\n");
>> ret = -EINVAL;
>> goto fail;
>> }
>> @@ -11160,7 +11165,7 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> ret = dm_update_mst_vcpi_slots_for_dsc(state,
>> dm_state->context, vars);
>> if (ret) {
>> - DRM_DEBUG_DRIVER("dm_update_mst_vcpi_slots_for_dsc()
>> failed\n");
>> + drm_dbg_atomic(dev, "dm_update_mst_vcpi_slots_for_dsc()
>> failed\n");
>> goto fail;
>> }
>> @@ -11172,12 +11177,12 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> */
>> ret = drm_dp_mst_atomic_check(state);
>> if (ret) {
>> - DRM_DEBUG_DRIVER("drm_dp_mst_atomic_check() failed\n");
>> + drm_dbg_atomic(dev, "drm_dp_mst_atomic_check() failed\n");
>> goto fail;
>> }
>> status = dc_validate_global_state(dc, dm_state->context, true);
>> if (status != DC_OK) {
>> - DRM_DEBUG_DRIVER("DC global validation failure: %s (%d)",
>> + drm_dbg_atomic(dev, "DC global validation failure: %s (%d)",
>> dc_status_to_str(status), status);
>> ret = -EINVAL;
>> goto fail;
>> @@ -11255,11 +11260,11 @@ static int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>> fail:
>> if (ret == -EDEADLK)
>> - DRM_DEBUG_DRIVER("Atomic check stopped to avoid deadlock.\n");
>> + drm_dbg_atomic(dev, "Atomic check stopped to avoid
>> deadlock.\n");
>> else if (ret == -EINTR || ret == -EAGAIN || ret == -ERESTARTSYS)
>> - DRM_DEBUG_DRIVER("Atomic check stopped due to signal.\n");
>> + drm_dbg_atomic(dev, "Atomic check stopped due to signal.\n");
>> else
>> - DRM_DEBUG_DRIVER("Atomic check failed with err: %d\n", ret);
>> + drm_dbg_atomic(dev, "Atomic check failed with err: %d\n", ret);
>> trace_amdgpu_dm_atomic_check_finish(state, ret);
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-06-03 12:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-28 12:57 [PATCH] drm/amd/display: Convert some legacy DRM debug macros into appropriate categories Tvrtko Ursulin
2024-05-30 19:57 ` Hamza Mahfooz
2024-05-31 17:10 ` Tvrtko Ursulin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox