* [PATCH 0/3] Make DC logging more configurable
@ 2017-09-15 14:32 Harry Wentland
[not found] ` <20170915143226.3963-1-harry.wentland-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 16+ messages in thread
From: Harry Wentland @ 2017-09-15 14:32 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: alexander.deucher-5C7GfCeVMHo, andrey.grodzovsky-5C7GfCeVMHo,
tony.cheng-5C7GfCeVMHo, Harry Wentland
We shouldn't be as chatty as we are, sitting in the kernel. This
patchset reduces the default log levels greatly but still allows
bug reporters and internal teams run with previous log levels. To
do that we use a module parameter and also make it a compile
time option.
One possibility would've been to expose all DC log levels through
the module parameter but I feel that would be too much for little
benefit. Going forward we can see if there's a better way of
dealing with logging.
Harry Wentland (3):
drm/amdgpu: Add dc_log module parameter
drm/amd/display: Pass log_mask from DM
drm/amd/display: Reduce DC chattiness
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++
drivers/gpu/drm/amd/display/Kconfig | 10 +++
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 80 ++++++++++++----------
drivers/gpu/drm/amd/display/dc/basics/logger.c | 41 ++---------
drivers/gpu/drm/amd/display/dc/core/dc.c | 2 +-
drivers/gpu/drm/amd/display/dc/dc.h | 1 +
.../gpu/drm/amd/display/include/logger_interface.h | 2 +-
drivers/gpu/drm/amd/display/include/logger_types.h | 35 ++++++++++
9 files changed, 103 insertions(+), 73 deletions(-)
--
2.11.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] drm/amdgpu: Add dc_log module parameter
[not found] ` <20170915143226.3963-1-harry.wentland-5C7GfCeVMHo@public.gmane.org>
@ 2017-09-15 14:32 ` Harry Wentland
2017-09-15 14:32 ` [PATCH 2/3] drm/amd/display: Pass log_mask from DM Harry Wentland
2017-09-15 14:32 ` [PATCH 3/3] drm/amd/display: Reduce DC chattiness Harry Wentland
2 siblings, 0 replies; 16+ messages in thread
From: Harry Wentland @ 2017-09-15 14:32 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: alexander.deucher-5C7GfCeVMHo, andrey.grodzovsky-5C7GfCeVMHo,
tony.cheng-5C7GfCeVMHo, Harry Wentland
We want to make DC less chatty but still allow bug reporters to
provide more detailed logs.
Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++++
2 files changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a34c4cb0178d..fdfc95ad24bc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -103,6 +103,7 @@ extern int amdgpu_vm_fault_stop;
extern int amdgpu_vm_debug;
extern int amdgpu_vm_update_mode;
extern int amdgpu_dc;
+extern int amdgpu_dc_log;
extern int amdgpu_sched_jobs;
extern int amdgpu_sched_hw_submission;
extern int amdgpu_no_evict;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 5035305cf32c..916201660db9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -104,6 +104,7 @@ int amdgpu_vram_page_split = 512;
int amdgpu_vm_update_mode = -1;
int amdgpu_exp_hw_support = 0;
int amdgpu_dc = -1;
+int amdgpu_dc_log = 0;
int amdgpu_sched_jobs = 32;
int amdgpu_sched_hw_submission = 2;
int amdgpu_no_evict = 0;
@@ -211,6 +212,9 @@ module_param_named(exp_hw_support, amdgpu_exp_hw_support, int, 0444);
MODULE_PARM_DESC(dc, "Display Core driver (1 = enable, 0 = disable, -1 = auto (default))");
module_param_named(dc, amdgpu_dc, int, 0444);
+MODULE_PARM_DESC(dc, "Display Core Log Level (0 = minimal (default), 1 = chatty");
+module_param_named(dc_log, amdgpu_dc_log, int, 0444);
+
MODULE_PARM_DESC(sched_jobs, "the max number of jobs supported in the sw queue (default 32)");
module_param_named(sched_jobs, amdgpu_sched_jobs, int, 0444);
--
2.11.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] drm/amd/display: Pass log_mask from DM
[not found] ` <20170915143226.3963-1-harry.wentland-5C7GfCeVMHo@public.gmane.org>
2017-09-15 14:32 ` [PATCH 1/3] drm/amdgpu: Add dc_log module parameter Harry Wentland
@ 2017-09-15 14:32 ` Harry Wentland
2017-09-15 14:32 ` [PATCH 3/3] drm/amd/display: Reduce DC chattiness Harry Wentland
2 siblings, 0 replies; 16+ messages in thread
From: Harry Wentland @ 2017-09-15 14:32 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: alexander.deucher-5C7GfCeVMHo, andrey.grodzovsky-5C7GfCeVMHo,
tony.cheng-5C7GfCeVMHo, Harry Wentland
Linux and Windows often desire different log levels.
Change-Id: Idd3c672e8b36f41de240e94d0c2d4b56ce60376e
Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +-
drivers/gpu/drm/amd/display/dc/basics/logger.c | 41 +++-------------------
drivers/gpu/drm/amd/display/dc/core/dc.c | 2 +-
drivers/gpu/drm/amd/display/dc/dc.h | 1 +
.../gpu/drm/amd/display/include/logger_interface.h | 2 +-
drivers/gpu/drm/amd/display/include/logger_types.h | 32 +++++++++++++++++
6 files changed, 42 insertions(+), 39 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 c9432c4c2646..abe89e3fed5b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -323,7 +323,6 @@ void amdgpu_dm_initialize_fbc(struct amdgpu_device *adev)
}
#endif
-
/* Init display KMS
*
* Returns 0 on success
@@ -374,6 +373,8 @@ int amdgpu_dm_init(struct amdgpu_device *adev)
init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
+ init_data.log_mask = DC_DEFAULT_LOG_MASK;
+
#ifdef ENABLE_FBC
if (adev->family == FAMILY_CZ)
amdgpu_dm_initialize_fbc(adev);
diff --git a/drivers/gpu/drm/amd/display/dc/basics/logger.c b/drivers/gpu/drm/amd/display/dc/basics/logger.c
index 0b17374b1535..5895dd3903a3 100644
--- a/drivers/gpu/drm/amd/display/dc/basics/logger.c
+++ b/drivers/gpu/drm/amd/display/dc/basics/logger.c
@@ -64,40 +64,9 @@ static const struct dc_log_type_info log_type_info_tbl[] = {
};
-#define DC_DEFAULT_LOG_MASK ((1 << LOG_ERROR) | \
- (1 << LOG_WARNING) | \
- (1 << LOG_EVENT_MODE_SET) | \
- (1 << LOG_EVENT_DETECTION) | \
- (1 << LOG_EVENT_LINK_TRAINING) | \
- (1 << LOG_EVENT_LINK_LOSS) | \
- (1 << LOG_EVENT_UNDERFLOW) | \
- (1 << LOG_RESOURCE) | \
- (1 << LOG_FEATURE_OVERRIDE) | \
- (1 << LOG_DETECTION_EDID_PARSER) | \
- (1 << LOG_DC) | \
- (1 << LOG_HW_HOTPLUG) | \
- (1 << LOG_HW_SET_MODE) | \
- (1 << LOG_HW_RESUME_S3) | \
- (1 << LOG_HW_HPD_IRQ) | \
- (1 << LOG_SYNC) | \
- (1 << LOG_BANDWIDTH_VALIDATION) | \
- (1 << LOG_MST) | \
- (1 << LOG_DETECTION_DP_CAPS) | \
- (1 << LOG_BACKLIGHT)) | \
- (1 << LOG_I2C_AUX) | \
- (1 << LOG_IF_TRACE) | \
- (1 << LOG_DTN) /* | \
- (1 << LOG_DEBUG) | \
- (1 << LOG_BIOS) | \
- (1 << LOG_SURFACE) | \
- (1 << LOG_SCALER) | \
- (1 << LOG_DML) | \
- (1 << LOG_HW_LINK_TRAINING) | \
- (1 << LOG_HW_AUDIO)| \
- (1 << LOG_BANDWIDTH_CALCS)*/
-
/* ----------- Object init and destruction ----------- */
-static bool construct(struct dc_context *ctx, struct dal_logger *logger)
+static bool construct(struct dc_context *ctx, struct dal_logger *logger,
+ uint32_t log_mask)
{
/* malloc buffer and init offsets */
logger->log_buffer_size = DAL_LOGGER_BUFFER_MAX_SIZE;
@@ -120,7 +89,7 @@ static bool construct(struct dc_context *ctx, struct dal_logger *logger)
logger->ctx = ctx;
- logger->mask = DC_DEFAULT_LOG_MASK;
+ logger->mask = log_mask;
return true;
}
@@ -133,14 +102,14 @@ static void destruct(struct dal_logger *logger)
}
}
-struct dal_logger *dal_logger_create(struct dc_context *ctx)
+struct dal_logger *dal_logger_create(struct dc_context *ctx, uint32_t log_mask)
{
/* malloc struct */
struct dal_logger *logger = dm_alloc(sizeof(struct dal_logger));
if (!logger)
return NULL;
- if (!construct(ctx, logger)) {
+ if (!construct(ctx, logger, log_mask)) {
dm_free(logger);
return NULL;
}
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index dd86b864efe6..7454a5a6a8fe 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -495,7 +495,7 @@ static bool construct(struct dc *dc,
dc_ctx->asic_id = init_params->asic_id;
/* Create logger */
- logger = dal_logger_create(dc_ctx);
+ logger = dal_logger_create(dc_ctx, init_params->log_mask);
if (!logger) {
/* can *not* call logger. call base driver 'print error' */
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index bf2d42561362..b5971f98dd23 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -267,6 +267,7 @@ struct dc_init_data {
enum dce_environment dce_environment;
struct dc_config flags;
+ uint32_t log_mask;
#ifdef ENABLE_FBC
uint64_t fbc_gpu_addr;
#endif
diff --git a/drivers/gpu/drm/amd/display/include/logger_interface.h b/drivers/gpu/drm/amd/display/include/logger_interface.h
index 6641e8001e97..5aaf2dacfe38 100644
--- a/drivers/gpu/drm/amd/display/include/logger_interface.h
+++ b/drivers/gpu/drm/amd/display/include/logger_interface.h
@@ -40,7 +40,7 @@ struct dc_state;
*
*/
-struct dal_logger *dal_logger_create(struct dc_context *ctx);
+struct dal_logger *dal_logger_create(struct dc_context *ctx, uint32_t log_mask);
uint32_t dal_logger_destroy(struct dal_logger **logger);
diff --git a/drivers/gpu/drm/amd/display/include/logger_types.h b/drivers/gpu/drm/amd/display/include/logger_types.h
index 42ffb93e3172..044805ccac25 100644
--- a/drivers/gpu/drm/amd/display/include/logger_types.h
+++ b/drivers/gpu/drm/amd/display/include/logger_types.h
@@ -70,6 +70,38 @@ enum dc_log_type {
LOG_SECTION_TOTAL_COUNT
};
+#define DC_DEFAULT_LOG_MASK ((1 << LOG_ERROR) | \
+ (1 << LOG_WARNING) | \
+ (1 << LOG_EVENT_MODE_SET) | \
+ (1 << LOG_EVENT_DETECTION) | \
+ (1 << LOG_EVENT_LINK_TRAINING) | \
+ (1 << LOG_EVENT_LINK_LOSS) | \
+ (1 << LOG_EVENT_UNDERFLOW) | \
+ (1 << LOG_RESOURCE) | \
+ (1 << LOG_FEATURE_OVERRIDE) | \
+ (1 << LOG_DETECTION_EDID_PARSER) | \
+ (1 << LOG_DC) | \
+ (1 << LOG_HW_HOTPLUG) | \
+ (1 << LOG_HW_SET_MODE) | \
+ (1 << LOG_HW_RESUME_S3) | \
+ (1 << LOG_HW_HPD_IRQ) | \
+ (1 << LOG_SYNC) | \
+ (1 << LOG_BANDWIDTH_VALIDATION) | \
+ (1 << LOG_MST) | \
+ (1 << LOG_DETECTION_DP_CAPS) | \
+ (1 << LOG_BACKLIGHT)) | \
+ (1 << LOG_I2C_AUX) | \
+ (1 << LOG_IF_TRACE) | \
+ (1 << LOG_DTN) /* | \
+ (1 << LOG_DEBUG) | \
+ (1 << LOG_BIOS) | \
+ (1 << LOG_SURFACE) | \
+ (1 << LOG_SCALER) | \
+ (1 << LOG_DML) | \
+ (1 << LOG_HW_LINK_TRAINING) | \
+ (1 << LOG_HW_AUDIO)| \
+ (1 << LOG_BANDWIDTH_CALCS)*/
+
union logger_flags {
struct {
uint32_t ENABLE_CONSOLE:1; /* Print to console */
--
2.11.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] drm/amd/display: Reduce DC chattiness
[not found] ` <20170915143226.3963-1-harry.wentland-5C7GfCeVMHo@public.gmane.org>
2017-09-15 14:32 ` [PATCH 1/3] drm/amdgpu: Add dc_log module parameter Harry Wentland
2017-09-15 14:32 ` [PATCH 2/3] drm/amd/display: Pass log_mask from DM Harry Wentland
@ 2017-09-15 14:32 ` Harry Wentland
[not found] ` <20170915143226.3963-4-harry.wentland-5C7GfCeVMHo@public.gmane.org>
2 siblings, 1 reply; 16+ messages in thread
From: Harry Wentland @ 2017-09-15 14:32 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: alexander.deucher-5C7GfCeVMHo, andrey.grodzovsky-5C7GfCeVMHo,
tony.cheng-5C7GfCeVMHo, Harry Wentland
Log DC init but default log level to 0 (default for
amdgpu_dc_log) otherwise. Bug reporters can still make
DC more chatty like so
amdgpu.dc_log = 1
Change-Id: Icdfb849fa678225e2460519fbd8066540feb451a
Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
drivers/gpu/drm/amd/display/Kconfig | 10 +++
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 77 ++++++++++++----------
drivers/gpu/drm/amd/display/include/logger_types.h | 3 +
3 files changed, 56 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig
index 1d1a5f807030..baab055dd362 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -31,4 +31,14 @@ config DEBUG_KERNEL_DC
if you want to hit
kdgb_break in assert.
+config DRM_AMD_DC_CHATTY
+ bool "Make DC chatty again"
+ default n
+ depends on DRM_AMD_DC
+ help
+ Sometimes it's useful to have a chatty DC
+ without a ton of spam from DRM. This allows
+ for that and is recommended for anyone
+ reporting bugs to DC.
+
endmenu
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 abe89e3fed5b..6ecb420b2a63 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -333,7 +333,6 @@ int amdgpu_dm_init(struct amdgpu_device *adev)
adev->dm.ddev = adev->ddev;
adev->dm.adev = adev;
- DRM_INFO("DAL is enabled\n");
/* Zero all the fields */
memset(&init_data, 0, sizeof(init_data));
@@ -373,7 +372,15 @@ int amdgpu_dm_init(struct amdgpu_device *adev)
init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
+#ifdef CONFIG_DRM_AMD_DC_CHATTY
+ /* always be chatty */
init_data.log_mask = DC_DEFAULT_LOG_MASK;
+#else
+ if (amdgpu_dc_log)
+ init_data.log_mask = DC_DEFAULT_LOG_MASK;
+ else
+ init_data.log_mask = DC_MIN_LOG_MASK;
+#endif
#ifdef ENABLE_FBC
if (adev->family == FAMILY_CZ)
@@ -383,7 +390,9 @@ int amdgpu_dm_init(struct amdgpu_device *adev)
/* Display Core create. */
adev->dm.dc = dc_create(&init_data);
- if (!adev->dm.dc)
+ if (adev->dm.dc)
+ DRM_INFO("Display Core initialized!\n");
+ else
DRM_INFO("Display Core failed to initialize!\n");
INIT_WORK(&adev->dm.mst_hotplug_work, hotplug_notify_work_func);
@@ -393,7 +402,7 @@ int amdgpu_dm_init(struct amdgpu_device *adev)
DRM_ERROR(
"amdgpu: failed to initialize freesync_module.\n");
} else
- DRM_INFO("amdgpu: freesync_module init done %p.\n",
+ DRM_DEBUG_DRIVER("amdgpu: freesync_module init done %p.\n",
adev->dm.freesync_module);
if (amdgpu_dm_initialize_drm_device(adev)) {
@@ -417,7 +426,7 @@ int amdgpu_dm_init(struct amdgpu_device *adev)
goto error;
}
- DRM_INFO("KMS initialized.\n");
+ DRM_DEBUG_DRIVER("KMS initialized.\n");
return 0;
error:
@@ -475,7 +484,7 @@ static int detect_mst_link_for_all_connectors(struct drm_device *dev)
list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
aconnector = to_amdgpu_dm_connector(connector);
if (aconnector->dc_link->type == dc_connection_mst_branch) {
- DRM_INFO("DM_MST: starting TM on aconnector: %p [id: %d]\n",
+ DRM_DEBUG_DRIVER("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);
@@ -819,12 +828,12 @@ void amdgpu_dm_update_connector_after_detect(
if (aconnector->dc_sink == sink) {
/* We got a DP short pulse (Link Loss, DP CTS, etc...).
* Do nothing!! */
- DRM_INFO("DCHPD: connector_id=%d: dc_sink didn't change.\n",
+ DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: dc_sink didn't change.\n",
aconnector->connector_id);
return;
}
- DRM_INFO("DCHPD: connector_id=%d: Old sink=%p New sink=%p\n",
+ DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: Old sink=%p New sink=%p\n",
aconnector->connector_id, aconnector->dc_sink, sink);
mutex_lock(&dev->mode_config.mutex);
@@ -926,7 +935,7 @@ static void dm_handle_hpd_rx_irq(struct amdgpu_dm_connector *aconnector)
process_count++;
- DRM_DEBUG_KMS("ESI %02x %02x %02x\n", esi[0], esi[1], esi[2]);
+ DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], esi[1], esi[2]);
/* handle HPD short pulse irq */
if (aconnector->mst_mgr.mst_state)
drm_dp_mst_hpd_irq(
@@ -964,7 +973,7 @@ static void dm_handle_hpd_rx_irq(struct amdgpu_dm_connector *aconnector)
}
if (process_count == max_process_count)
- DRM_DEBUG_KMS("Loop exceeded max iterations\n");
+ DRM_DEBUG_DRIVER("Loop exceeded max iterations\n");
}
static void handle_hpd_rx_irq(void *param)
@@ -1283,7 +1292,7 @@ void amdgpu_dm_register_backlight_device(struct amdgpu_display_manager *dm)
if (NULL == dm->backlight_dev)
DRM_ERROR("DM: Backlight registration failed!\n");
else
- DRM_INFO("DM: Registered Backlight device: %s\n", bl_name);
+ DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n", bl_name);
}
#endif
@@ -2064,7 +2073,7 @@ static void update_stream_scaling_settings(
stream->src = src;
stream->dst = dst;
- DRM_DEBUG_KMS("Destination Rectangle x:%d y:%d width:%d height:%d\n",
+ DRM_DEBUG_DRIVER("Destination Rectangle x:%d y:%d width:%d height:%d\n",
dst.x, dst.y, dst.width, dst.height);
}
@@ -2374,7 +2383,7 @@ static struct dc_stream_state *create_stream_for_sink(
* case, we call set mode ourselves to restore the previous mode
* and the modelist may not be filled in in time.
*/
- DRM_INFO("No preferred mode found\n");
+ DRM_DEBUG_DRIVER("No preferred mode found\n");
} else {
decide_crtc_timing_for_drm_display_mode(
&mode, preferred_mode,
@@ -2749,7 +2758,7 @@ static struct drm_encoder *best_encoder(struct drm_connector *connector)
struct drm_mode_object *obj;
struct drm_encoder *encoder;
- DRM_DEBUG_KMS("Finding the best encoder\n");
+ DRM_DEBUG_DRIVER("Finding the best encoder\n");
/* pick the encoder ids */
if (enc_id) {
@@ -3019,7 +3028,7 @@ static int dm_plane_helper_prepare_fb(
dm_plane_state_new = to_dm_plane_state(new_state);
if (!new_state->fb) {
- DRM_DEBUG_KMS("No FB bound\n");
+ DRM_DEBUG_DRIVER("No FB bound\n");
return 0;
}
@@ -3594,7 +3603,7 @@ int amdgpu_dm_connector_init(
struct amdgpu_i2c_adapter *i2c;
((struct dc_link *)link)->priv = aconnector;
- DRM_DEBUG_KMS("%s()\n", __func__);
+ DRM_DEBUG_DRIVER("%s()\n", __func__);
i2c = create_i2c(link->ddc, link->link_index, &res);
aconnector->i2c = i2c;
@@ -3835,11 +3844,11 @@ static void handle_cursor_update(
if (!plane->state->fb && !old_plane_state->fb)
return;
- DRM_DEBUG_KMS("%s: crtc_id=%d with size %d to %d\n",
- __func__,
- amdgpu_crtc->crtc_id,
- plane->state->crtc_w,
- plane->state->crtc_h);
+ DRM_DEBUG_DRIVER("%s: crtc_id=%d with size %d to %d\n",
+ __func__,
+ amdgpu_crtc->crtc_id,
+ plane->state->crtc_w,
+ plane->state->crtc_h);
ret = get_cursor_position(plane, crtc, &position);
if (ret)
@@ -4142,7 +4151,7 @@ void amdgpu_dm_atomic_commit_tail(
new_acrtc_state = to_dm_crtc_state(new_state);
old_acrtc_state = to_dm_crtc_state(old_crtc_state);
- DRM_DEBUG_KMS(
+ DRM_DEBUG_DRIVER(
"amdgpu_crtc id:%d crtc_state_flags: enable:%d, active:%d, "
"planes_changed:%d, mode_changed:%d,active_changed:%d,"
"connectors_changed:%d\n",
@@ -4160,7 +4169,7 @@ void amdgpu_dm_atomic_commit_tail(
if (modeset_required(new_state, new_acrtc_state->stream, old_acrtc_state->stream)) {
- DRM_INFO("Atomic commit: SET crtc id %d: [%p]\n", acrtc->crtc_id, acrtc);
+ DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d: [%p]\n", acrtc->crtc_id, acrtc);
if (!new_acrtc_state->stream) {
/*
@@ -4178,7 +4187,7 @@ void amdgpu_dm_atomic_commit_tail(
* have a sink to keep the pipe running so that
* hw state is consistent with the sw state
*/
- DRM_DEBUG_KMS("%s: Failed to create new stream for crtc %d\n",
+ DRM_DEBUG_DRIVER("%s: Failed to create new stream for crtc %d\n",
__func__, acrtc->base.base.id);
continue;
}
@@ -4205,7 +4214,7 @@ void amdgpu_dm_atomic_commit_tail(
acrtc->hw_mode = crtc->state->mode;
crtc->hwmode = crtc->state->mode;
} else if (modereset_required(new_state)) {
- DRM_INFO("Atomic commit: RESET. crtc id %d:[%p]\n", acrtc->crtc_id, acrtc);
+ DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id %d:[%p]\n", acrtc->crtc_id, acrtc);
/* i.e. reset mode */
if (old_acrtc_state->stream)
@@ -4230,7 +4239,7 @@ void amdgpu_dm_atomic_commit_tail(
&new_crtcs[i]->base,
false);
if (!aconnector) {
- DRM_INFO("Atomic commit: Failed to find connector for acrtc id:%d "
+ DRM_DEBUG_DRIVER("Atomic commit: Failed to find connector for acrtc id:%d "
"skipping freesync init\n",
new_crtcs[i]->crtc_id);
continue;
@@ -4539,7 +4548,7 @@ static int dm_update_crtcs_state(
*/
if (!new_stream) {
- DRM_DEBUG_KMS("%s: Failed to create new stream for crtc %d\n",
+ DRM_DEBUG_DRIVER("%s: Failed to create new stream for crtc %d\n",
__func__, acrtc->base.base.id);
break;
}
@@ -4550,7 +4559,7 @@ static int dm_update_crtcs_state(
crtc_state->mode_changed = false;
- DRM_DEBUG_KMS("Mode change not required, setting mode_changed to %d",
+ DRM_DEBUG_DRIVER("Mode change not required, setting mode_changed to %d",
crtc_state->mode_changed);
}
@@ -4558,7 +4567,7 @@ static int dm_update_crtcs_state(
if (!drm_atomic_crtc_needs_modeset(crtc_state))
goto next_crtc;
- DRM_DEBUG_KMS(
+ DRM_DEBUG_DRIVER(
"amdgpu_crtc id:%d crtc_state_flags: enable:%d, active:%d, "
"planes_changed:%d, mode_changed:%d,active_changed:%d,"
"connectors_changed:%d\n",
@@ -4576,7 +4585,7 @@ static int dm_update_crtcs_state(
if (!old_acrtc_state->stream)
goto next_crtc;
- DRM_DEBUG_KMS("Disabling DRM crtc: %d\n",
+ DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n",
crtc->base.id);
/* i.e. reset mode */
@@ -4606,7 +4615,7 @@ static int dm_update_crtcs_state(
new_acrtc_state->stream = new_stream;
dc_stream_retain(new_stream);
- DRM_DEBUG_KMS("Enabling DRM crtc: %d\n",
+ DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n",
crtc->base.id);
if (!dc_add_stream_to_ctx(
@@ -4681,7 +4690,7 @@ static int dm_update_planes_state(
if (!old_acrtc_state->stream)
continue;
- DRM_DEBUG_KMS("Disabling DRM plane: %d on DRM crtc %d\n",
+ DRM_DEBUG_DRIVER("Disabling DRM plane: %d on DRM crtc %d\n",
plane->base.id, old_plane_crtc->base.id);
if (!dc_remove_plane_from_context(
@@ -4719,7 +4728,7 @@ static int dm_update_planes_state(
new_dm_plane_state->dc_state = dc_create_plane_state(dc);
- DRM_DEBUG_KMS("Enabling DRM plane: %d on DRM crtc %d\n",
+ DRM_DEBUG_DRIVER("Enabling DRM plane: %d on DRM crtc %d\n",
plane->base.id, new_plane_crtc->base.id);
if (!new_dm_plane_state->dc_state) {
@@ -4874,9 +4883,9 @@ int amdgpu_dm_atomic_check(struct drm_device *dev,
fail:
if (ret == -EDEADLK)
- DRM_DEBUG_KMS("Atomic check stopped due to to deadlock.\n");
+ DRM_DEBUG_DRIVER("Atomic check stopped due to to deadlock.\n");
else if (ret == -EINTR || ret == -EAGAIN || ret == -ERESTARTSYS)
- DRM_DEBUG_KMS("Atomic check stopped due to to signal.\n");
+ DRM_DEBUG_DRIVER("Atomic check stopped due to to signal.\n");
else
DRM_ERROR("Atomic check failed with err: %d \n", ret);
diff --git a/drivers/gpu/drm/amd/display/include/logger_types.h b/drivers/gpu/drm/amd/display/include/logger_types.h
index 044805ccac25..1f22e84cedb9 100644
--- a/drivers/gpu/drm/amd/display/include/logger_types.h
+++ b/drivers/gpu/drm/amd/display/include/logger_types.h
@@ -70,6 +70,9 @@ enum dc_log_type {
LOG_SECTION_TOTAL_COUNT
};
+#define DC_MIN_LOG_MASK ((1 << LOG_ERROR) | \
+ (1 << LOG_DETECTION_EDID_PARSER))
+
#define DC_DEFAULT_LOG_MASK ((1 << LOG_ERROR) | \
(1 << LOG_WARNING) | \
(1 << LOG_EVENT_MODE_SET) | \
--
2.11.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
[not found] ` <20170915143226.3963-4-harry.wentland-5C7GfCeVMHo@public.gmane.org>
@ 2017-09-15 15:08 ` Christian König
[not found] ` <c28eee04-9c45-39b9-7bb1-682726e7d6b2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 16+ messages in thread
From: Christian König @ 2017-09-15 15:08 UTC (permalink / raw)
To: Harry Wentland, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: alexander.deucher-5C7GfCeVMHo, andrey.grodzovsky-5C7GfCeVMHo,
tony.cheng-5C7GfCeVMHo
Am 15.09.2017 um 16:32 schrieb Harry Wentland:
> Log DC init but default log level to 0 (default for
> amdgpu_dc_log) otherwise. Bug reporters can still make
> DC more chatty like so
> amdgpu.dc_log = 1
Which is exactly the reason why I think this patch is superfluous.
Either have a compile time option or a run time option, but please not
both that's just confusing.
Christian.
>
> Change-Id: Icdfb849fa678225e2460519fbd8066540feb451a
> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> ---
> drivers/gpu/drm/amd/display/Kconfig | 10 +++
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 77 ++++++++++++----------
> drivers/gpu/drm/amd/display/include/logger_types.h | 3 +
> 3 files changed, 56 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig
> index 1d1a5f807030..baab055dd362 100644
> --- a/drivers/gpu/drm/amd/display/Kconfig
> +++ b/drivers/gpu/drm/amd/display/Kconfig
> @@ -31,4 +31,14 @@ config DEBUG_KERNEL_DC
> if you want to hit
> kdgb_break in assert.
>
> +config DRM_AMD_DC_CHATTY
> + bool "Make DC chatty again"
> + default n
> + depends on DRM_AMD_DC
> + help
> + Sometimes it's useful to have a chatty DC
> + without a ton of spam from DRM. This allows
> + for that and is recommended for anyone
> + reporting bugs to DC.
> +
> endmenu
> 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 abe89e3fed5b..6ecb420b2a63 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -333,7 +333,6 @@ int amdgpu_dm_init(struct amdgpu_device *adev)
> adev->dm.ddev = adev->ddev;
> adev->dm.adev = adev;
>
> - DRM_INFO("DAL is enabled\n");
> /* Zero all the fields */
> memset(&init_data, 0, sizeof(init_data));
>
> @@ -373,7 +372,15 @@ int amdgpu_dm_init(struct amdgpu_device *adev)
>
> init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
>
> +#ifdef CONFIG_DRM_AMD_DC_CHATTY
> + /* always be chatty */
> init_data.log_mask = DC_DEFAULT_LOG_MASK;
> +#else
> + if (amdgpu_dc_log)
> + init_data.log_mask = DC_DEFAULT_LOG_MASK;
> + else
> + init_data.log_mask = DC_MIN_LOG_MASK;
> +#endif
>
> #ifdef ENABLE_FBC
> if (adev->family == FAMILY_CZ)
> @@ -383,7 +390,9 @@ int amdgpu_dm_init(struct amdgpu_device *adev)
> /* Display Core create. */
> adev->dm.dc = dc_create(&init_data);
>
> - if (!adev->dm.dc)
> + if (adev->dm.dc)
> + DRM_INFO("Display Core initialized!\n");
> + else
> DRM_INFO("Display Core failed to initialize!\n");
>
> INIT_WORK(&adev->dm.mst_hotplug_work, hotplug_notify_work_func);
> @@ -393,7 +402,7 @@ int amdgpu_dm_init(struct amdgpu_device *adev)
> DRM_ERROR(
> "amdgpu: failed to initialize freesync_module.\n");
> } else
> - DRM_INFO("amdgpu: freesync_module init done %p.\n",
> + DRM_DEBUG_DRIVER("amdgpu: freesync_module init done %p.\n",
> adev->dm.freesync_module);
>
> if (amdgpu_dm_initialize_drm_device(adev)) {
> @@ -417,7 +426,7 @@ int amdgpu_dm_init(struct amdgpu_device *adev)
> goto error;
> }
>
> - DRM_INFO("KMS initialized.\n");
> + DRM_DEBUG_DRIVER("KMS initialized.\n");
>
> return 0;
> error:
> @@ -475,7 +484,7 @@ static int detect_mst_link_for_all_connectors(struct drm_device *dev)
> list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> aconnector = to_amdgpu_dm_connector(connector);
> if (aconnector->dc_link->type == dc_connection_mst_branch) {
> - DRM_INFO("DM_MST: starting TM on aconnector: %p [id: %d]\n",
> + DRM_DEBUG_DRIVER("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);
> @@ -819,12 +828,12 @@ void amdgpu_dm_update_connector_after_detect(
> if (aconnector->dc_sink == sink) {
> /* We got a DP short pulse (Link Loss, DP CTS, etc...).
> * Do nothing!! */
> - DRM_INFO("DCHPD: connector_id=%d: dc_sink didn't change.\n",
> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: dc_sink didn't change.\n",
> aconnector->connector_id);
> return;
> }
>
> - DRM_INFO("DCHPD: connector_id=%d: Old sink=%p New sink=%p\n",
> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: Old sink=%p New sink=%p\n",
> aconnector->connector_id, aconnector->dc_sink, sink);
>
> mutex_lock(&dev->mode_config.mutex);
> @@ -926,7 +935,7 @@ static void dm_handle_hpd_rx_irq(struct amdgpu_dm_connector *aconnector)
>
> process_count++;
>
> - DRM_DEBUG_KMS("ESI %02x %02x %02x\n", esi[0], esi[1], esi[2]);
> + DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], esi[1], esi[2]);
> /* handle HPD short pulse irq */
> if (aconnector->mst_mgr.mst_state)
> drm_dp_mst_hpd_irq(
> @@ -964,7 +973,7 @@ static void dm_handle_hpd_rx_irq(struct amdgpu_dm_connector *aconnector)
> }
>
> if (process_count == max_process_count)
> - DRM_DEBUG_KMS("Loop exceeded max iterations\n");
> + DRM_DEBUG_DRIVER("Loop exceeded max iterations\n");
> }
>
> static void handle_hpd_rx_irq(void *param)
> @@ -1283,7 +1292,7 @@ void amdgpu_dm_register_backlight_device(struct amdgpu_display_manager *dm)
> if (NULL == dm->backlight_dev)
> DRM_ERROR("DM: Backlight registration failed!\n");
> else
> - DRM_INFO("DM: Registered Backlight device: %s\n", bl_name);
> + DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n", bl_name);
> }
>
> #endif
> @@ -2064,7 +2073,7 @@ static void update_stream_scaling_settings(
> stream->src = src;
> stream->dst = dst;
>
> - DRM_DEBUG_KMS("Destination Rectangle x:%d y:%d width:%d height:%d\n",
> + DRM_DEBUG_DRIVER("Destination Rectangle x:%d y:%d width:%d height:%d\n",
> dst.x, dst.y, dst.width, dst.height);
>
> }
> @@ -2374,7 +2383,7 @@ static struct dc_stream_state *create_stream_for_sink(
> * case, we call set mode ourselves to restore the previous mode
> * and the modelist may not be filled in in time.
> */
> - DRM_INFO("No preferred mode found\n");
> + DRM_DEBUG_DRIVER("No preferred mode found\n");
> } else {
> decide_crtc_timing_for_drm_display_mode(
> &mode, preferred_mode,
> @@ -2749,7 +2758,7 @@ static struct drm_encoder *best_encoder(struct drm_connector *connector)
> struct drm_mode_object *obj;
> struct drm_encoder *encoder;
>
> - DRM_DEBUG_KMS("Finding the best encoder\n");
> + DRM_DEBUG_DRIVER("Finding the best encoder\n");
>
> /* pick the encoder ids */
> if (enc_id) {
> @@ -3019,7 +3028,7 @@ static int dm_plane_helper_prepare_fb(
> dm_plane_state_new = to_dm_plane_state(new_state);
>
> if (!new_state->fb) {
> - DRM_DEBUG_KMS("No FB bound\n");
> + DRM_DEBUG_DRIVER("No FB bound\n");
> return 0;
> }
>
> @@ -3594,7 +3603,7 @@ int amdgpu_dm_connector_init(
> struct amdgpu_i2c_adapter *i2c;
> ((struct dc_link *)link)->priv = aconnector;
>
> - DRM_DEBUG_KMS("%s()\n", __func__);
> + DRM_DEBUG_DRIVER("%s()\n", __func__);
>
> i2c = create_i2c(link->ddc, link->link_index, &res);
> aconnector->i2c = i2c;
> @@ -3835,11 +3844,11 @@ static void handle_cursor_update(
> if (!plane->state->fb && !old_plane_state->fb)
> return;
>
> - DRM_DEBUG_KMS("%s: crtc_id=%d with size %d to %d\n",
> - __func__,
> - amdgpu_crtc->crtc_id,
> - plane->state->crtc_w,
> - plane->state->crtc_h);
> + DRM_DEBUG_DRIVER("%s: crtc_id=%d with size %d to %d\n",
> + __func__,
> + amdgpu_crtc->crtc_id,
> + plane->state->crtc_w,
> + plane->state->crtc_h);
>
> ret = get_cursor_position(plane, crtc, &position);
> if (ret)
> @@ -4142,7 +4151,7 @@ void amdgpu_dm_atomic_commit_tail(
> new_acrtc_state = to_dm_crtc_state(new_state);
> old_acrtc_state = to_dm_crtc_state(old_crtc_state);
>
> - DRM_DEBUG_KMS(
> + DRM_DEBUG_DRIVER(
> "amdgpu_crtc id:%d crtc_state_flags: enable:%d, active:%d, "
> "planes_changed:%d, mode_changed:%d,active_changed:%d,"
> "connectors_changed:%d\n",
> @@ -4160,7 +4169,7 @@ void amdgpu_dm_atomic_commit_tail(
>
> if (modeset_required(new_state, new_acrtc_state->stream, old_acrtc_state->stream)) {
>
> - DRM_INFO("Atomic commit: SET crtc id %d: [%p]\n", acrtc->crtc_id, acrtc);
> + DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d: [%p]\n", acrtc->crtc_id, acrtc);
>
> if (!new_acrtc_state->stream) {
> /*
> @@ -4178,7 +4187,7 @@ void amdgpu_dm_atomic_commit_tail(
> * have a sink to keep the pipe running so that
> * hw state is consistent with the sw state
> */
> - DRM_DEBUG_KMS("%s: Failed to create new stream for crtc %d\n",
> + DRM_DEBUG_DRIVER("%s: Failed to create new stream for crtc %d\n",
> __func__, acrtc->base.base.id);
> continue;
> }
> @@ -4205,7 +4214,7 @@ void amdgpu_dm_atomic_commit_tail(
> acrtc->hw_mode = crtc->state->mode;
> crtc->hwmode = crtc->state->mode;
> } else if (modereset_required(new_state)) {
> - DRM_INFO("Atomic commit: RESET. crtc id %d:[%p]\n", acrtc->crtc_id, acrtc);
> + DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id %d:[%p]\n", acrtc->crtc_id, acrtc);
>
> /* i.e. reset mode */
> if (old_acrtc_state->stream)
> @@ -4230,7 +4239,7 @@ void amdgpu_dm_atomic_commit_tail(
> &new_crtcs[i]->base,
> false);
> if (!aconnector) {
> - DRM_INFO("Atomic commit: Failed to find connector for acrtc id:%d "
> + DRM_DEBUG_DRIVER("Atomic commit: Failed to find connector for acrtc id:%d "
> "skipping freesync init\n",
> new_crtcs[i]->crtc_id);
> continue;
> @@ -4539,7 +4548,7 @@ static int dm_update_crtcs_state(
> */
>
> if (!new_stream) {
> - DRM_DEBUG_KMS("%s: Failed to create new stream for crtc %d\n",
> + DRM_DEBUG_DRIVER("%s: Failed to create new stream for crtc %d\n",
> __func__, acrtc->base.base.id);
> break;
> }
> @@ -4550,7 +4559,7 @@ static int dm_update_crtcs_state(
>
> crtc_state->mode_changed = false;
>
> - DRM_DEBUG_KMS("Mode change not required, setting mode_changed to %d",
> + DRM_DEBUG_DRIVER("Mode change not required, setting mode_changed to %d",
> crtc_state->mode_changed);
> }
>
> @@ -4558,7 +4567,7 @@ static int dm_update_crtcs_state(
> if (!drm_atomic_crtc_needs_modeset(crtc_state))
> goto next_crtc;
>
> - DRM_DEBUG_KMS(
> + DRM_DEBUG_DRIVER(
> "amdgpu_crtc id:%d crtc_state_flags: enable:%d, active:%d, "
> "planes_changed:%d, mode_changed:%d,active_changed:%d,"
> "connectors_changed:%d\n",
> @@ -4576,7 +4585,7 @@ static int dm_update_crtcs_state(
> if (!old_acrtc_state->stream)
> goto next_crtc;
>
> - DRM_DEBUG_KMS("Disabling DRM crtc: %d\n",
> + DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n",
> crtc->base.id);
>
> /* i.e. reset mode */
> @@ -4606,7 +4615,7 @@ static int dm_update_crtcs_state(
> new_acrtc_state->stream = new_stream;
> dc_stream_retain(new_stream);
>
> - DRM_DEBUG_KMS("Enabling DRM crtc: %d\n",
> + DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n",
> crtc->base.id);
>
> if (!dc_add_stream_to_ctx(
> @@ -4681,7 +4690,7 @@ static int dm_update_planes_state(
> if (!old_acrtc_state->stream)
> continue;
>
> - DRM_DEBUG_KMS("Disabling DRM plane: %d on DRM crtc %d\n",
> + DRM_DEBUG_DRIVER("Disabling DRM plane: %d on DRM crtc %d\n",
> plane->base.id, old_plane_crtc->base.id);
>
> if (!dc_remove_plane_from_context(
> @@ -4719,7 +4728,7 @@ static int dm_update_planes_state(
>
> new_dm_plane_state->dc_state = dc_create_plane_state(dc);
>
> - DRM_DEBUG_KMS("Enabling DRM plane: %d on DRM crtc %d\n",
> + DRM_DEBUG_DRIVER("Enabling DRM plane: %d on DRM crtc %d\n",
> plane->base.id, new_plane_crtc->base.id);
>
> if (!new_dm_plane_state->dc_state) {
> @@ -4874,9 +4883,9 @@ int amdgpu_dm_atomic_check(struct drm_device *dev,
>
> fail:
> if (ret == -EDEADLK)
> - DRM_DEBUG_KMS("Atomic check stopped due to to deadlock.\n");
> + DRM_DEBUG_DRIVER("Atomic check stopped due to to deadlock.\n");
> else if (ret == -EINTR || ret == -EAGAIN || ret == -ERESTARTSYS)
> - DRM_DEBUG_KMS("Atomic check stopped due to to signal.\n");
> + DRM_DEBUG_DRIVER("Atomic check stopped due to to signal.\n");
> else
> DRM_ERROR("Atomic check failed with err: %d \n", ret);
>
> diff --git a/drivers/gpu/drm/amd/display/include/logger_types.h b/drivers/gpu/drm/amd/display/include/logger_types.h
> index 044805ccac25..1f22e84cedb9 100644
> --- a/drivers/gpu/drm/amd/display/include/logger_types.h
> +++ b/drivers/gpu/drm/amd/display/include/logger_types.h
> @@ -70,6 +70,9 @@ enum dc_log_type {
> LOG_SECTION_TOTAL_COUNT
> };
>
> +#define DC_MIN_LOG_MASK ((1 << LOG_ERROR) | \
> + (1 << LOG_DETECTION_EDID_PARSER))
> +
> #define DC_DEFAULT_LOG_MASK ((1 << LOG_ERROR) | \
> (1 << LOG_WARNING) | \
> (1 << LOG_EVENT_MODE_SET) | \
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
[not found] ` <c28eee04-9c45-39b9-7bb1-682726e7d6b2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-09-15 15:20 ` Harry Wentland
[not found] ` <e7a119fe-9c62-d03b-d115-e69fde3866b5-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 16+ messages in thread
From: Harry Wentland @ 2017-09-15 15:20 UTC (permalink / raw)
To: christian.koenig-5C7GfCeVMHo, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: alexander.deucher-5C7GfCeVMHo, andrey.grodzovsky-5C7GfCeVMHo,
tony.cheng-5C7GfCeVMHo
On 2017-09-15 11:08 AM, Christian König wrote:
> Am 15.09.2017 um 16:32 schrieb Harry Wentland:
>> Log DC init but default log level to 0 (default for
>> amdgpu_dc_log) otherwise. Bug reporters can still make
>> DC more chatty like so
>> amdgpu.dc_log = 1
>
> Which is exactly the reason why I think this patch is superfluous.
>
> Either have a compile time option or a run time option, but please not
> both that's just confusing.
>
True. Thanks for the input.
Gonna leave out the run time option for now.
Harry
> Christian.
>
>>
>> Change-Id: Icdfb849fa678225e2460519fbd8066540feb451a
>> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
>> ---
>> drivers/gpu/drm/amd/display/Kconfig | 10 +++
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 77
>> ++++++++++++----------
>> drivers/gpu/drm/amd/display/include/logger_types.h | 3 +
>> 3 files changed, 56 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/Kconfig
>> b/drivers/gpu/drm/amd/display/Kconfig
>> index 1d1a5f807030..baab055dd362 100644
>> --- a/drivers/gpu/drm/amd/display/Kconfig
>> +++ b/drivers/gpu/drm/amd/display/Kconfig
>> @@ -31,4 +31,14 @@ config DEBUG_KERNEL_DC
>> if you want to hit
>> kdgb_break in assert.
>> +config DRM_AMD_DC_CHATTY
>> + bool "Make DC chatty again"
>> + default n
>> + depends on DRM_AMD_DC
>> + help
>> + Sometimes it's useful to have a chatty DC
>> + without a ton of spam from DRM. This allows
>> + for that and is recommended for anyone
>> + reporting bugs to DC.
>> +
>> endmenu
>> 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 abe89e3fed5b..6ecb420b2a63 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -333,7 +333,6 @@ int amdgpu_dm_init(struct amdgpu_device *adev)
>> adev->dm.ddev = adev->ddev;
>> adev->dm.adev = adev;
>> - DRM_INFO("DAL is enabled\n");
>> /* Zero all the fields */
>> memset(&init_data, 0, sizeof(init_data));
>> @@ -373,7 +372,15 @@ int amdgpu_dm_init(struct amdgpu_device *adev)
>> init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
>> +#ifdef CONFIG_DRM_AMD_DC_CHATTY
>> + /* always be chatty */
>> init_data.log_mask = DC_DEFAULT_LOG_MASK;
>> +#else
>> + if (amdgpu_dc_log)
>> + init_data.log_mask = DC_DEFAULT_LOG_MASK;
>> + else
>> + init_data.log_mask = DC_MIN_LOG_MASK;
>> +#endif
>> #ifdef ENABLE_FBC
>> if (adev->family == FAMILY_CZ)
>> @@ -383,7 +390,9 @@ int amdgpu_dm_init(struct amdgpu_device *adev)
>> /* Display Core create. */
>> adev->dm.dc = dc_create(&init_data);
>> - if (!adev->dm.dc)
>> + if (adev->dm.dc)
>> + DRM_INFO("Display Core initialized!\n");
>> + else
>> DRM_INFO("Display Core failed to initialize!\n");
>> INIT_WORK(&adev->dm.mst_hotplug_work, hotplug_notify_work_func);
>> @@ -393,7 +402,7 @@ int amdgpu_dm_init(struct amdgpu_device *adev)
>> DRM_ERROR(
>> "amdgpu: failed to initialize freesync_module.\n");
>> } else
>> - DRM_INFO("amdgpu: freesync_module init done %p.\n",
>> + DRM_DEBUG_DRIVER("amdgpu: freesync_module init done %p.\n",
>> adev->dm.freesync_module);
>> if (amdgpu_dm_initialize_drm_device(adev)) {
>> @@ -417,7 +426,7 @@ int amdgpu_dm_init(struct amdgpu_device *adev)
>> goto error;
>> }
>> - DRM_INFO("KMS initialized.\n");
>> + DRM_DEBUG_DRIVER("KMS initialized.\n");
>> return 0;
>> error:
>> @@ -475,7 +484,7 @@ static int
>> detect_mst_link_for_all_connectors(struct drm_device *dev)
>> list_for_each_entry(connector, &dev->mode_config.connector_list,
>> head) {
>> aconnector = to_amdgpu_dm_connector(connector);
>> if (aconnector->dc_link->type == dc_connection_mst_branch) {
>> - DRM_INFO("DM_MST: starting TM on aconnector: %p [id: %d]\n",
>> + DRM_DEBUG_DRIVER("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);
>> @@ -819,12 +828,12 @@ void amdgpu_dm_update_connector_after_detect(
>> if (aconnector->dc_sink == sink) {
>> /* We got a DP short pulse (Link Loss, DP CTS, etc...).
>> * Do nothing!! */
>> - DRM_INFO("DCHPD: connector_id=%d: dc_sink didn't change.\n",
>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: dc_sink didn't
>> change.\n",
>> aconnector->connector_id);
>> return;
>> }
>> - DRM_INFO("DCHPD: connector_id=%d: Old sink=%p New sink=%p\n",
>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: Old sink=%p New
>> sink=%p\n",
>> aconnector->connector_id, aconnector->dc_sink, sink);
>> mutex_lock(&dev->mode_config.mutex);
>> @@ -926,7 +935,7 @@ static void dm_handle_hpd_rx_irq(struct
>> amdgpu_dm_connector *aconnector)
>> process_count++;
>> - DRM_DEBUG_KMS("ESI %02x %02x %02x\n", esi[0], esi[1], esi[2]);
>> + DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], esi[1],
>> esi[2]);
>> /* handle HPD short pulse irq */
>> if (aconnector->mst_mgr.mst_state)
>> drm_dp_mst_hpd_irq(
>> @@ -964,7 +973,7 @@ static void dm_handle_hpd_rx_irq(struct
>> amdgpu_dm_connector *aconnector)
>> }
>> if (process_count == max_process_count)
>> - DRM_DEBUG_KMS("Loop exceeded max iterations\n");
>> + DRM_DEBUG_DRIVER("Loop exceeded max iterations\n");
>> }
>> static void handle_hpd_rx_irq(void *param)
>> @@ -1283,7 +1292,7 @@ void amdgpu_dm_register_backlight_device(struct
>> amdgpu_display_manager *dm)
>> if (NULL == dm->backlight_dev)
>> DRM_ERROR("DM: Backlight registration failed!\n");
>> else
>> - DRM_INFO("DM: Registered Backlight device: %s\n", bl_name);
>> + DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n",
>> bl_name);
>> }
>> #endif
>> @@ -2064,7 +2073,7 @@ static void update_stream_scaling_settings(
>> stream->src = src;
>> stream->dst = dst;
>> - DRM_DEBUG_KMS("Destination Rectangle x:%d y:%d width:%d
>> height:%d\n",
>> + DRM_DEBUG_DRIVER("Destination Rectangle x:%d y:%d width:%d
>> height:%d\n",
>> dst.x, dst.y, dst.width, dst.height);
>> }
>> @@ -2374,7 +2383,7 @@ static struct dc_stream_state
>> *create_stream_for_sink(
>> * case, we call set mode ourselves to restore the previous
>> mode
>> * and the modelist may not be filled in in time.
>> */
>> - DRM_INFO("No preferred mode found\n");
>> + DRM_DEBUG_DRIVER("No preferred mode found\n");
>> } else {
>> decide_crtc_timing_for_drm_display_mode(
>> &mode, preferred_mode,
>> @@ -2749,7 +2758,7 @@ static struct drm_encoder *best_encoder(struct
>> drm_connector *connector)
>> struct drm_mode_object *obj;
>> struct drm_encoder *encoder;
>> - DRM_DEBUG_KMS("Finding the best encoder\n");
>> + DRM_DEBUG_DRIVER("Finding the best encoder\n");
>> /* pick the encoder ids */
>> if (enc_id) {
>> @@ -3019,7 +3028,7 @@ static int dm_plane_helper_prepare_fb(
>> dm_plane_state_new = to_dm_plane_state(new_state);
>> if (!new_state->fb) {
>> - DRM_DEBUG_KMS("No FB bound\n");
>> + DRM_DEBUG_DRIVER("No FB bound\n");
>> return 0;
>> }
>> @@ -3594,7 +3603,7 @@ int amdgpu_dm_connector_init(
>> struct amdgpu_i2c_adapter *i2c;
>> ((struct dc_link *)link)->priv = aconnector;
>> - DRM_DEBUG_KMS("%s()\n", __func__);
>> + DRM_DEBUG_DRIVER("%s()\n", __func__);
>> i2c = create_i2c(link->ddc, link->link_index, &res);
>> aconnector->i2c = i2c;
>> @@ -3835,11 +3844,11 @@ static void handle_cursor_update(
>> if (!plane->state->fb && !old_plane_state->fb)
>> return;
>> - DRM_DEBUG_KMS("%s: crtc_id=%d with size %d to %d\n",
>> - __func__,
>> - amdgpu_crtc->crtc_id,
>> - plane->state->crtc_w,
>> - plane->state->crtc_h);
>> + DRM_DEBUG_DRIVER("%s: crtc_id=%d with size %d to %d\n",
>> + __func__,
>> + amdgpu_crtc->crtc_id,
>> + plane->state->crtc_w,
>> + plane->state->crtc_h);
>> ret = get_cursor_position(plane, crtc, &position);
>> if (ret)
>> @@ -4142,7 +4151,7 @@ void amdgpu_dm_atomic_commit_tail(
>> new_acrtc_state = to_dm_crtc_state(new_state);
>> old_acrtc_state = to_dm_crtc_state(old_crtc_state);
>> - DRM_DEBUG_KMS(
>> + DRM_DEBUG_DRIVER(
>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>> active:%d, "
>> "planes_changed:%d, mode_changed:%d,active_changed:%d,"
>> "connectors_changed:%d\n",
>> @@ -4160,7 +4169,7 @@ void amdgpu_dm_atomic_commit_tail(
>> if (modeset_required(new_state, new_acrtc_state->stream,
>> old_acrtc_state->stream)) {
>> - DRM_INFO("Atomic commit: SET crtc id %d: [%p]\n",
>> acrtc->crtc_id, acrtc);
>> + DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d: [%p]\n",
>> acrtc->crtc_id, acrtc);
>> if (!new_acrtc_state->stream) {
>> /*
>> @@ -4178,7 +4187,7 @@ void amdgpu_dm_atomic_commit_tail(
>> * have a sink to keep the pipe running so that
>> * hw state is consistent with the sw state
>> */
>> - DRM_DEBUG_KMS("%s: Failed to create new stream for
>> crtc %d\n",
>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream for
>> crtc %d\n",
>> __func__, acrtc->base.base.id);
>> continue;
>> }
>> @@ -4205,7 +4214,7 @@ void amdgpu_dm_atomic_commit_tail(
>> acrtc->hw_mode = crtc->state->mode;
>> crtc->hwmode = crtc->state->mode;
>> } else if (modereset_required(new_state)) {
>> - DRM_INFO("Atomic commit: RESET. crtc id %d:[%p]\n",
>> acrtc->crtc_id, acrtc);
>> + DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id
>> %d:[%p]\n", acrtc->crtc_id, acrtc);
>> /* i.e. reset mode */
>> if (old_acrtc_state->stream)
>> @@ -4230,7 +4239,7 @@ void amdgpu_dm_atomic_commit_tail(
>> &new_crtcs[i]->base,
>> false);
>> if (!aconnector) {
>> - DRM_INFO("Atomic commit: Failed to find connector for
>> acrtc id:%d "
>> + DRM_DEBUG_DRIVER("Atomic commit: Failed to find
>> connector for acrtc id:%d "
>> "skipping freesync init\n",
>> new_crtcs[i]->crtc_id);
>> continue;
>> @@ -4539,7 +4548,7 @@ static int dm_update_crtcs_state(
>> */
>> if (!new_stream) {
>> - DRM_DEBUG_KMS("%s: Failed to create new stream for
>> crtc %d\n",
>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream for
>> crtc %d\n",
>> __func__, acrtc->base.base.id);
>> break;
>> }
>> @@ -4550,7 +4559,7 @@ static int dm_update_crtcs_state(
>> crtc_state->mode_changed = false;
>> - DRM_DEBUG_KMS("Mode change not required, setting
>> mode_changed to %d",
>> + DRM_DEBUG_DRIVER("Mode change not required, setting
>> mode_changed to %d",
>> crtc_state->mode_changed);
>> }
>> @@ -4558,7 +4567,7 @@ static int dm_update_crtcs_state(
>> if (!drm_atomic_crtc_needs_modeset(crtc_state))
>> goto next_crtc;
>> - DRM_DEBUG_KMS(
>> + DRM_DEBUG_DRIVER(
>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>> active:%d, "
>> "planes_changed:%d, mode_changed:%d,active_changed:%d,"
>> "connectors_changed:%d\n",
>> @@ -4576,7 +4585,7 @@ static int dm_update_crtcs_state(
>> if (!old_acrtc_state->stream)
>> goto next_crtc;
>> - DRM_DEBUG_KMS("Disabling DRM crtc: %d\n",
>> + DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n",
>> crtc->base.id);
>> /* i.e. reset mode */
>> @@ -4606,7 +4615,7 @@ static int dm_update_crtcs_state(
>> new_acrtc_state->stream = new_stream;
>> dc_stream_retain(new_stream);
>> - DRM_DEBUG_KMS("Enabling DRM crtc: %d\n",
>> + DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n",
>> crtc->base.id);
>> if (!dc_add_stream_to_ctx(
>> @@ -4681,7 +4690,7 @@ static int dm_update_planes_state(
>> if (!old_acrtc_state->stream)
>> continue;
>> - DRM_DEBUG_KMS("Disabling DRM plane: %d on DRM crtc %d\n",
>> + DRM_DEBUG_DRIVER("Disabling DRM plane: %d on DRM crtc %d\n",
>> plane->base.id, old_plane_crtc->base.id);
>> if (!dc_remove_plane_from_context(
>> @@ -4719,7 +4728,7 @@ static int dm_update_planes_state(
>> new_dm_plane_state->dc_state = dc_create_plane_state(dc);
>> - DRM_DEBUG_KMS("Enabling DRM plane: %d on DRM crtc %d\n",
>> + DRM_DEBUG_DRIVER("Enabling DRM plane: %d on DRM crtc %d\n",
>> plane->base.id, new_plane_crtc->base.id);
>> if (!new_dm_plane_state->dc_state) {
>> @@ -4874,9 +4883,9 @@ int amdgpu_dm_atomic_check(struct drm_device *dev,
>> fail:
>> if (ret == -EDEADLK)
>> - DRM_DEBUG_KMS("Atomic check stopped due to to deadlock.\n");
>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to deadlock.\n");
>> else if (ret == -EINTR || ret == -EAGAIN || ret == -ERESTARTSYS)
>> - DRM_DEBUG_KMS("Atomic check stopped due to to signal.\n");
>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to signal.\n");
>> else
>> DRM_ERROR("Atomic check failed with err: %d \n", ret);
>> diff --git a/drivers/gpu/drm/amd/display/include/logger_types.h
>> b/drivers/gpu/drm/amd/display/include/logger_types.h
>> index 044805ccac25..1f22e84cedb9 100644
>> --- a/drivers/gpu/drm/amd/display/include/logger_types.h
>> +++ b/drivers/gpu/drm/amd/display/include/logger_types.h
>> @@ -70,6 +70,9 @@ enum dc_log_type {
>> LOG_SECTION_TOTAL_COUNT
>> };
>> +#define DC_MIN_LOG_MASK ((1 << LOG_ERROR) | \
>> + (1 << LOG_DETECTION_EDID_PARSER))
>> +
>> #define DC_DEFAULT_LOG_MASK ((1 << LOG_ERROR) | \
>> (1 << LOG_WARNING) | \
>> (1 << LOG_EVENT_MODE_SET) | \
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
[not found] ` <e7a119fe-9c62-d03b-d115-e69fde3866b5-5C7GfCeVMHo@public.gmane.org>
@ 2017-09-15 15:26 ` Deucher, Alexander
[not found] ` <BN6PR12MB1652A614351C9BF5F1807ED0F76C0-/b2+HYfkarQqUD6E6FAiowdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-09-15 15:27 ` Deucher, Alexander
1 sibling, 1 reply; 16+ messages in thread
From: Deucher, Alexander @ 2017-09-15 15:26 UTC (permalink / raw)
To: Wentland, Harry, Koenig, Christian,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Grodzovsky, Andrey, Cheng, Tony
> -----Original Message-----
> From: Wentland, Harry
> Sent: Friday, September 15, 2017 11:21 AM
> To: Koenig, Christian; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander; Grodzovsky, Andrey; Cheng, Tony
> Subject: Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
>
>
>
> On 2017-09-15 11:08 AM, Christian König wrote:
> > Am 15.09.2017 um 16:32 schrieb Harry Wentland:
> >> Log DC init but default log level to 0 (default for
> >> amdgpu_dc_log) otherwise. Bug reporters can still make
> >> DC more chatty like so
> >> amdgpu.dc_log = 1
> >
> > Which is exactly the reason why I think this patch is superfluous.
> >
> > Either have a compile time option or a run time option, but please not
> > both that's just confusing.
> >
>
> True. Thanks for the input.
>
> Gonna leave out the run time option for now.
Another option would be to tie the dc debug levels to drm debug levels.
Alex
>
> Harry
>
> > Christian.
> >
> >>
> >> Change-Id: Icdfb849fa678225e2460519fbd8066540feb451a
> >> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> >> ---
> >> drivers/gpu/drm/amd/display/Kconfig | 10 +++
> >> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 77
> >> ++++++++++++----------
> >> drivers/gpu/drm/amd/display/include/logger_types.h | 3 +
> >> 3 files changed, 56 insertions(+), 34 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/display/Kconfig
> >> b/drivers/gpu/drm/amd/display/Kconfig
> >> index 1d1a5f807030..baab055dd362 100644
> >> --- a/drivers/gpu/drm/amd/display/Kconfig
> >> +++ b/drivers/gpu/drm/amd/display/Kconfig
> >> @@ -31,4 +31,14 @@ config DEBUG_KERNEL_DC
> >> if you want to hit
> >> kdgb_break in assert.
> >> +config DRM_AMD_DC_CHATTY
> >> + bool "Make DC chatty again"
> >> + default n
> >> + depends on DRM_AMD_DC
> >> + help
> >> + Sometimes it's useful to have a chatty DC
> >> + without a ton of spam from DRM. This allows
> >> + for that and is recommended for anyone
> >> + reporting bugs to DC.
> >> +
> >> endmenu
> >> 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 abe89e3fed5b..6ecb420b2a63 100644
> >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> @@ -333,7 +333,6 @@ int amdgpu_dm_init(struct amdgpu_device
> *adev)
> >> adev->dm.ddev = adev->ddev;
> >> adev->dm.adev = adev;
> >> - DRM_INFO("DAL is enabled\n");
> >> /* Zero all the fields */
> >> memset(&init_data, 0, sizeof(init_data));
> >> @@ -373,7 +372,15 @@ int amdgpu_dm_init(struct amdgpu_device
> *adev)
> >> init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
> >> +#ifdef CONFIG_DRM_AMD_DC_CHATTY
> >> + /* always be chatty */
> >> init_data.log_mask = DC_DEFAULT_LOG_MASK;
> >> +#else
> >> + if (amdgpu_dc_log)
> >> + init_data.log_mask = DC_DEFAULT_LOG_MASK;
> >> + else
> >> + init_data.log_mask = DC_MIN_LOG_MASK;
> >> +#endif
> >> #ifdef ENABLE_FBC
> >> if (adev->family == FAMILY_CZ)
> >> @@ -383,7 +390,9 @@ int amdgpu_dm_init(struct amdgpu_device
> *adev)
> >> /* Display Core create. */
> >> adev->dm.dc = dc_create(&init_data);
> >> - if (!adev->dm.dc)
> >> + if (adev->dm.dc)
> >> + DRM_INFO("Display Core initialized!\n");
> >> + else
> >> DRM_INFO("Display Core failed to initialize!\n");
> >> INIT_WORK(&adev->dm.mst_hotplug_work,
> hotplug_notify_work_func);
> >> @@ -393,7 +402,7 @@ int amdgpu_dm_init(struct amdgpu_device
> *adev)
> >> DRM_ERROR(
> >> "amdgpu: failed to initialize freesync_module.\n");
> >> } else
> >> - DRM_INFO("amdgpu: freesync_module init done %p.\n",
> >> + DRM_DEBUG_DRIVER("amdgpu: freesync_module init done %p.\n",
> >> adev->dm.freesync_module);
> >> if (amdgpu_dm_initialize_drm_device(adev)) {
> >> @@ -417,7 +426,7 @@ int amdgpu_dm_init(struct amdgpu_device
> *adev)
> >> goto error;
> >> }
> >> - DRM_INFO("KMS initialized.\n");
> >> + DRM_DEBUG_DRIVER("KMS initialized.\n");
> >> return 0;
> >> error:
> >> @@ -475,7 +484,7 @@ static int
> >> detect_mst_link_for_all_connectors(struct drm_device *dev)
> >> list_for_each_entry(connector, &dev->mode_config.connector_list,
> >> head) {
> >> aconnector = to_amdgpu_dm_connector(connector);
> >> if (aconnector->dc_link->type == dc_connection_mst_branch) {
> >> - DRM_INFO("DM_MST: starting TM on aconnector: %p [id: %d]\n",
> >> + DRM_DEBUG_DRIVER("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);
> >> @@ -819,12 +828,12 @@ void
> amdgpu_dm_update_connector_after_detect(
> >> if (aconnector->dc_sink == sink) {
> >> /* We got a DP short pulse (Link Loss, DP CTS, etc...).
> >> * Do nothing!! */
> >> - DRM_INFO("DCHPD: connector_id=%d: dc_sink didn't change.\n",
> >> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: dc_sink didn't
> >> change.\n",
> >> aconnector->connector_id);
> >> return;
> >> }
> >> - DRM_INFO("DCHPD: connector_id=%d: Old sink=%p New
> sink=%p\n",
> >> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: Old sink=%p New
> >> sink=%p\n",
> >> aconnector->connector_id, aconnector->dc_sink, sink);
> >> mutex_lock(&dev->mode_config.mutex);
> >> @@ -926,7 +935,7 @@ static void dm_handle_hpd_rx_irq(struct
> >> amdgpu_dm_connector *aconnector)
> >> process_count++;
> >> - DRM_DEBUG_KMS("ESI %02x %02x %02x\n", esi[0], esi[1], esi[2]);
> >> + DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], esi[1],
> >> esi[2]);
> >> /* handle HPD short pulse irq */
> >> if (aconnector->mst_mgr.mst_state)
> >> drm_dp_mst_hpd_irq(
> >> @@ -964,7 +973,7 @@ static void dm_handle_hpd_rx_irq(struct
> >> amdgpu_dm_connector *aconnector)
> >> }
> >> if (process_count == max_process_count)
> >> - DRM_DEBUG_KMS("Loop exceeded max iterations\n");
> >> + DRM_DEBUG_DRIVER("Loop exceeded max iterations\n");
> >> }
> >> static void handle_hpd_rx_irq(void *param)
> >> @@ -1283,7 +1292,7 @@ void
> amdgpu_dm_register_backlight_device(struct
> >> amdgpu_display_manager *dm)
> >> if (NULL == dm->backlight_dev)
> >> DRM_ERROR("DM: Backlight registration failed!\n");
> >> else
> >> - DRM_INFO("DM: Registered Backlight device: %s\n", bl_name);
> >> + DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n",
> >> bl_name);
> >> }
> >> #endif
> >> @@ -2064,7 +2073,7 @@ static void update_stream_scaling_settings(
> >> stream->src = src;
> >> stream->dst = dst;
> >> - DRM_DEBUG_KMS("Destination Rectangle x:%d y:%d width:%d
> >> height:%d\n",
> >> + DRM_DEBUG_DRIVER("Destination Rectangle x:%d y:%d width:%d
> >> height:%d\n",
> >> dst.x, dst.y, dst.width, dst.height);
> >> }
> >> @@ -2374,7 +2383,7 @@ static struct dc_stream_state
> >> *create_stream_for_sink(
> >> * case, we call set mode ourselves to restore the previous
> >> mode
> >> * and the modelist may not be filled in in time.
> >> */
> >> - DRM_INFO("No preferred mode found\n");
> >> + DRM_DEBUG_DRIVER("No preferred mode found\n");
> >> } else {
> >> decide_crtc_timing_for_drm_display_mode(
> >> &mode, preferred_mode,
> >> @@ -2749,7 +2758,7 @@ static struct drm_encoder *best_encoder(struct
> >> drm_connector *connector)
> >> struct drm_mode_object *obj;
> >> struct drm_encoder *encoder;
> >> - DRM_DEBUG_KMS("Finding the best encoder\n");
> >> + DRM_DEBUG_DRIVER("Finding the best encoder\n");
> >> /* pick the encoder ids */
> >> if (enc_id) {
> >> @@ -3019,7 +3028,7 @@ static int dm_plane_helper_prepare_fb(
> >> dm_plane_state_new = to_dm_plane_state(new_state);
> >> if (!new_state->fb) {
> >> - DRM_DEBUG_KMS("No FB bound\n");
> >> + DRM_DEBUG_DRIVER("No FB bound\n");
> >> return 0;
> >> }
> >> @@ -3594,7 +3603,7 @@ int amdgpu_dm_connector_init(
> >> struct amdgpu_i2c_adapter *i2c;
> >> ((struct dc_link *)link)->priv = aconnector;
> >> - DRM_DEBUG_KMS("%s()\n", __func__);
> >> + DRM_DEBUG_DRIVER("%s()\n", __func__);
> >> i2c = create_i2c(link->ddc, link->link_index, &res);
> >> aconnector->i2c = i2c;
> >> @@ -3835,11 +3844,11 @@ static void handle_cursor_update(
> >> if (!plane->state->fb && !old_plane_state->fb)
> >> return;
> >> - DRM_DEBUG_KMS("%s: crtc_id=%d with size %d to %d\n",
> >> - __func__,
> >> - amdgpu_crtc->crtc_id,
> >> - plane->state->crtc_w,
> >> - plane->state->crtc_h);
> >> + DRM_DEBUG_DRIVER("%s: crtc_id=%d with size %d to %d\n",
> >> + __func__,
> >> + amdgpu_crtc->crtc_id,
> >> + plane->state->crtc_w,
> >> + plane->state->crtc_h);
> >> ret = get_cursor_position(plane, crtc, &position);
> >> if (ret)
> >> @@ -4142,7 +4151,7 @@ void amdgpu_dm_atomic_commit_tail(
> >> new_acrtc_state = to_dm_crtc_state(new_state);
> >> old_acrtc_state = to_dm_crtc_state(old_crtc_state);
> >> - DRM_DEBUG_KMS(
> >> + DRM_DEBUG_DRIVER(
> >> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
> >> active:%d, "
> >> "planes_changed:%d, mode_changed:%d,active_changed:%d,"
> >> "connectors_changed:%d\n",
> >> @@ -4160,7 +4169,7 @@ void amdgpu_dm_atomic_commit_tail(
> >> if (modeset_required(new_state, new_acrtc_state->stream,
> >> old_acrtc_state->stream)) {
> >> - DRM_INFO("Atomic commit: SET crtc id %d: [%p]\n",
> >> acrtc->crtc_id, acrtc);
> >> + DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d: [%p]\n",
> >> acrtc->crtc_id, acrtc);
> >> if (!new_acrtc_state->stream) {
> >> /*
> >> @@ -4178,7 +4187,7 @@ void amdgpu_dm_atomic_commit_tail(
> >> * have a sink to keep the pipe running so that
> >> * hw state is consistent with the sw state
> >> */
> >> - DRM_DEBUG_KMS("%s: Failed to create new stream for
> >> crtc %d\n",
> >> + DRM_DEBUG_DRIVER("%s: Failed to create new stream for
> >> crtc %d\n",
> >> __func__, acrtc->base.base.id);
> >> continue;
> >> }
> >> @@ -4205,7 +4214,7 @@ void amdgpu_dm_atomic_commit_tail(
> >> acrtc->hw_mode = crtc->state->mode;
> >> crtc->hwmode = crtc->state->mode;
> >> } else if (modereset_required(new_state)) {
> >> - DRM_INFO("Atomic commit: RESET. crtc id %d:[%p]\n",
> >> acrtc->crtc_id, acrtc);
> >> + DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id
> >> %d:[%p]\n", acrtc->crtc_id, acrtc);
> >> /* i.e. reset mode */
> >> if (old_acrtc_state->stream)
> >> @@ -4230,7 +4239,7 @@ void amdgpu_dm_atomic_commit_tail(
> >> &new_crtcs[i]->base,
> >> false);
> >> if (!aconnector) {
> >> - DRM_INFO("Atomic commit: Failed to find connector for
> >> acrtc id:%d "
> >> + DRM_DEBUG_DRIVER("Atomic commit: Failed to find
> >> connector for acrtc id:%d "
> >> "skipping freesync init\n",
> >> new_crtcs[i]->crtc_id);
> >> continue;
> >> @@ -4539,7 +4548,7 @@ static int dm_update_crtcs_state(
> >> */
> >> if (!new_stream) {
> >> - DRM_DEBUG_KMS("%s: Failed to create new stream for
> >> crtc %d\n",
> >> + DRM_DEBUG_DRIVER("%s: Failed to create new stream for
> >> crtc %d\n",
> >> __func__, acrtc->base.base.id);
> >> break;
> >> }
> >> @@ -4550,7 +4559,7 @@ static int dm_update_crtcs_state(
> >> crtc_state->mode_changed = false;
> >> - DRM_DEBUG_KMS("Mode change not required, setting
> >> mode_changed to %d",
> >> + DRM_DEBUG_DRIVER("Mode change not required, setting
> >> mode_changed to %d",
> >> crtc_state->mode_changed);
> >> }
> >> @@ -4558,7 +4567,7 @@ static int dm_update_crtcs_state(
> >> if (!drm_atomic_crtc_needs_modeset(crtc_state))
> >> goto next_crtc;
> >> - DRM_DEBUG_KMS(
> >> + DRM_DEBUG_DRIVER(
> >> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
> >> active:%d, "
> >> "planes_changed:%d, mode_changed:%d,active_changed:%d,"
> >> "connectors_changed:%d\n",
> >> @@ -4576,7 +4585,7 @@ static int dm_update_crtcs_state(
> >> if (!old_acrtc_state->stream)
> >> goto next_crtc;
> >> - DRM_DEBUG_KMS("Disabling DRM crtc: %d\n",
> >> + DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n",
> >> crtc->base.id);
> >> /* i.e. reset mode */
> >> @@ -4606,7 +4615,7 @@ static int dm_update_crtcs_state(
> >> new_acrtc_state->stream = new_stream;
> >> dc_stream_retain(new_stream);
> >> - DRM_DEBUG_KMS("Enabling DRM crtc: %d\n",
> >> + DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n",
> >> crtc->base.id);
> >> if (!dc_add_stream_to_ctx(
> >> @@ -4681,7 +4690,7 @@ static int dm_update_planes_state(
> >> if (!old_acrtc_state->stream)
> >> continue;
> >> - DRM_DEBUG_KMS("Disabling DRM plane: %d on DRM crtc %d\n",
> >> + DRM_DEBUG_DRIVER("Disabling DRM plane: %d on DRM crtc
> %d\n",
> >> plane->base.id, old_plane_crtc->base.id);
> >> if (!dc_remove_plane_from_context(
> >> @@ -4719,7 +4728,7 @@ static int dm_update_planes_state(
> >> new_dm_plane_state->dc_state = dc_create_plane_state(dc);
> >> - DRM_DEBUG_KMS("Enabling DRM plane: %d on DRM crtc %d\n",
> >> + DRM_DEBUG_DRIVER("Enabling DRM plane: %d on DRM crtc
> %d\n",
> >> plane->base.id, new_plane_crtc->base.id);
> >> if (!new_dm_plane_state->dc_state) {
> >> @@ -4874,9 +4883,9 @@ int amdgpu_dm_atomic_check(struct
> drm_device *dev,
> >> fail:
> >> if (ret == -EDEADLK)
> >> - DRM_DEBUG_KMS("Atomic check stopped due to to deadlock.\n");
> >> + DRM_DEBUG_DRIVER("Atomic check stopped due to to
> deadlock.\n");
> >> else if (ret == -EINTR || ret == -EAGAIN || ret == -ERESTARTSYS)
> >> - DRM_DEBUG_KMS("Atomic check stopped due to to signal.\n");
> >> + DRM_DEBUG_DRIVER("Atomic check stopped due to to signal.\n");
> >> else
> >> DRM_ERROR("Atomic check failed with err: %d \n", ret);
> >> diff --git a/drivers/gpu/drm/amd/display/include/logger_types.h
> >> b/drivers/gpu/drm/amd/display/include/logger_types.h
> >> index 044805ccac25..1f22e84cedb9 100644
> >> --- a/drivers/gpu/drm/amd/display/include/logger_types.h
> >> +++ b/drivers/gpu/drm/amd/display/include/logger_types.h
> >> @@ -70,6 +70,9 @@ enum dc_log_type {
> >> LOG_SECTION_TOTAL_COUNT
> >> };
> >> +#define DC_MIN_LOG_MASK ((1 << LOG_ERROR) | \
> >> + (1 << LOG_DETECTION_EDID_PARSER))
> >> +
> >> #define DC_DEFAULT_LOG_MASK ((1 << LOG_ERROR) | \
> >> (1 << LOG_WARNING) | \
> >> (1 << LOG_EVENT_MODE_SET) | \
> >
> >
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
[not found] ` <e7a119fe-9c62-d03b-d115-e69fde3866b5-5C7GfCeVMHo@public.gmane.org>
2017-09-15 15:26 ` Deucher, Alexander
@ 2017-09-15 15:27 ` Deucher, Alexander
[not found] ` <BN6PR12MB1652D287EE41EA30B10CCF0CF76C0-/b2+HYfkarQqUD6E6FAiowdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
1 sibling, 1 reply; 16+ messages in thread
From: Deucher, Alexander @ 2017-09-15 15:27 UTC (permalink / raw)
To: Wentland, Harry, Koenig, Christian,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Grodzovsky, Andrey, Cheng, Tony
> -----Original Message-----
> From: Wentland, Harry
> Sent: Friday, September 15, 2017 11:21 AM
> To: Koenig, Christian; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander; Grodzovsky, Andrey; Cheng, Tony
> Subject: Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
>
>
>
> On 2017-09-15 11:08 AM, Christian König wrote:
> > Am 15.09.2017 um 16:32 schrieb Harry Wentland:
> >> Log DC init but default log level to 0 (default for
> >> amdgpu_dc_log) otherwise. Bug reporters can still make
> >> DC more chatty like so
> >> amdgpu.dc_log = 1
> >
> > Which is exactly the reason why I think this patch is superfluous.
> >
> > Either have a compile time option or a run time option, but please not
> > both that's just confusing.
> >
>
> True. Thanks for the input.
>
> Gonna leave out the run time option for now.
I'd rather leave the runtime option and remove the compile option. It's easier to get logs by changing a runtime option than to ask a user to recompile their kernel.
Alex
>
> Harry
>
> > Christian.
> >
> >>
> >> Change-Id: Icdfb849fa678225e2460519fbd8066540feb451a
> >> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> >> ---
> >> drivers/gpu/drm/amd/display/Kconfig | 10 +++
> >> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 77
> >> ++++++++++++----------
> >> drivers/gpu/drm/amd/display/include/logger_types.h | 3 +
> >> 3 files changed, 56 insertions(+), 34 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/display/Kconfig
> >> b/drivers/gpu/drm/amd/display/Kconfig
> >> index 1d1a5f807030..baab055dd362 100644
> >> --- a/drivers/gpu/drm/amd/display/Kconfig
> >> +++ b/drivers/gpu/drm/amd/display/Kconfig
> >> @@ -31,4 +31,14 @@ config DEBUG_KERNEL_DC
> >> if you want to hit
> >> kdgb_break in assert.
> >> +config DRM_AMD_DC_CHATTY
> >> + bool "Make DC chatty again"
> >> + default n
> >> + depends on DRM_AMD_DC
> >> + help
> >> + Sometimes it's useful to have a chatty DC
> >> + without a ton of spam from DRM. This allows
> >> + for that and is recommended for anyone
> >> + reporting bugs to DC.
> >> +
> >> endmenu
> >> 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 abe89e3fed5b..6ecb420b2a63 100644
> >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> @@ -333,7 +333,6 @@ int amdgpu_dm_init(struct amdgpu_device
> *adev)
> >> adev->dm.ddev = adev->ddev;
> >> adev->dm.adev = adev;
> >> - DRM_INFO("DAL is enabled\n");
> >> /* Zero all the fields */
> >> memset(&init_data, 0, sizeof(init_data));
> >> @@ -373,7 +372,15 @@ int amdgpu_dm_init(struct amdgpu_device
> *adev)
> >> init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
> >> +#ifdef CONFIG_DRM_AMD_DC_CHATTY
> >> + /* always be chatty */
> >> init_data.log_mask = DC_DEFAULT_LOG_MASK;
> >> +#else
> >> + if (amdgpu_dc_log)
> >> + init_data.log_mask = DC_DEFAULT_LOG_MASK;
> >> + else
> >> + init_data.log_mask = DC_MIN_LOG_MASK;
> >> +#endif
> >> #ifdef ENABLE_FBC
> >> if (adev->family == FAMILY_CZ)
> >> @@ -383,7 +390,9 @@ int amdgpu_dm_init(struct amdgpu_device
> *adev)
> >> /* Display Core create. */
> >> adev->dm.dc = dc_create(&init_data);
> >> - if (!adev->dm.dc)
> >> + if (adev->dm.dc)
> >> + DRM_INFO("Display Core initialized!\n");
> >> + else
> >> DRM_INFO("Display Core failed to initialize!\n");
> >> INIT_WORK(&adev->dm.mst_hotplug_work,
> hotplug_notify_work_func);
> >> @@ -393,7 +402,7 @@ int amdgpu_dm_init(struct amdgpu_device
> *adev)
> >> DRM_ERROR(
> >> "amdgpu: failed to initialize freesync_module.\n");
> >> } else
> >> - DRM_INFO("amdgpu: freesync_module init done %p.\n",
> >> + DRM_DEBUG_DRIVER("amdgpu: freesync_module init done %p.\n",
> >> adev->dm.freesync_module);
> >> if (amdgpu_dm_initialize_drm_device(adev)) {
> >> @@ -417,7 +426,7 @@ int amdgpu_dm_init(struct amdgpu_device
> *adev)
> >> goto error;
> >> }
> >> - DRM_INFO("KMS initialized.\n");
> >> + DRM_DEBUG_DRIVER("KMS initialized.\n");
> >> return 0;
> >> error:
> >> @@ -475,7 +484,7 @@ static int
> >> detect_mst_link_for_all_connectors(struct drm_device *dev)
> >> list_for_each_entry(connector, &dev->mode_config.connector_list,
> >> head) {
> >> aconnector = to_amdgpu_dm_connector(connector);
> >> if (aconnector->dc_link->type == dc_connection_mst_branch) {
> >> - DRM_INFO("DM_MST: starting TM on aconnector: %p [id: %d]\n",
> >> + DRM_DEBUG_DRIVER("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);
> >> @@ -819,12 +828,12 @@ void
> amdgpu_dm_update_connector_after_detect(
> >> if (aconnector->dc_sink == sink) {
> >> /* We got a DP short pulse (Link Loss, DP CTS, etc...).
> >> * Do nothing!! */
> >> - DRM_INFO("DCHPD: connector_id=%d: dc_sink didn't change.\n",
> >> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: dc_sink didn't
> >> change.\n",
> >> aconnector->connector_id);
> >> return;
> >> }
> >> - DRM_INFO("DCHPD: connector_id=%d: Old sink=%p New
> sink=%p\n",
> >> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: Old sink=%p New
> >> sink=%p\n",
> >> aconnector->connector_id, aconnector->dc_sink, sink);
> >> mutex_lock(&dev->mode_config.mutex);
> >> @@ -926,7 +935,7 @@ static void dm_handle_hpd_rx_irq(struct
> >> amdgpu_dm_connector *aconnector)
> >> process_count++;
> >> - DRM_DEBUG_KMS("ESI %02x %02x %02x\n", esi[0], esi[1], esi[2]);
> >> + DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], esi[1],
> >> esi[2]);
> >> /* handle HPD short pulse irq */
> >> if (aconnector->mst_mgr.mst_state)
> >> drm_dp_mst_hpd_irq(
> >> @@ -964,7 +973,7 @@ static void dm_handle_hpd_rx_irq(struct
> >> amdgpu_dm_connector *aconnector)
> >> }
> >> if (process_count == max_process_count)
> >> - DRM_DEBUG_KMS("Loop exceeded max iterations\n");
> >> + DRM_DEBUG_DRIVER("Loop exceeded max iterations\n");
> >> }
> >> static void handle_hpd_rx_irq(void *param)
> >> @@ -1283,7 +1292,7 @@ void
> amdgpu_dm_register_backlight_device(struct
> >> amdgpu_display_manager *dm)
> >> if (NULL == dm->backlight_dev)
> >> DRM_ERROR("DM: Backlight registration failed!\n");
> >> else
> >> - DRM_INFO("DM: Registered Backlight device: %s\n", bl_name);
> >> + DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n",
> >> bl_name);
> >> }
> >> #endif
> >> @@ -2064,7 +2073,7 @@ static void update_stream_scaling_settings(
> >> stream->src = src;
> >> stream->dst = dst;
> >> - DRM_DEBUG_KMS("Destination Rectangle x:%d y:%d width:%d
> >> height:%d\n",
> >> + DRM_DEBUG_DRIVER("Destination Rectangle x:%d y:%d width:%d
> >> height:%d\n",
> >> dst.x, dst.y, dst.width, dst.height);
> >> }
> >> @@ -2374,7 +2383,7 @@ static struct dc_stream_state
> >> *create_stream_for_sink(
> >> * case, we call set mode ourselves to restore the previous
> >> mode
> >> * and the modelist may not be filled in in time.
> >> */
> >> - DRM_INFO("No preferred mode found\n");
> >> + DRM_DEBUG_DRIVER("No preferred mode found\n");
> >> } else {
> >> decide_crtc_timing_for_drm_display_mode(
> >> &mode, preferred_mode,
> >> @@ -2749,7 +2758,7 @@ static struct drm_encoder *best_encoder(struct
> >> drm_connector *connector)
> >> struct drm_mode_object *obj;
> >> struct drm_encoder *encoder;
> >> - DRM_DEBUG_KMS("Finding the best encoder\n");
> >> + DRM_DEBUG_DRIVER("Finding the best encoder\n");
> >> /* pick the encoder ids */
> >> if (enc_id) {
> >> @@ -3019,7 +3028,7 @@ static int dm_plane_helper_prepare_fb(
> >> dm_plane_state_new = to_dm_plane_state(new_state);
> >> if (!new_state->fb) {
> >> - DRM_DEBUG_KMS("No FB bound\n");
> >> + DRM_DEBUG_DRIVER("No FB bound\n");
> >> return 0;
> >> }
> >> @@ -3594,7 +3603,7 @@ int amdgpu_dm_connector_init(
> >> struct amdgpu_i2c_adapter *i2c;
> >> ((struct dc_link *)link)->priv = aconnector;
> >> - DRM_DEBUG_KMS("%s()\n", __func__);
> >> + DRM_DEBUG_DRIVER("%s()\n", __func__);
> >> i2c = create_i2c(link->ddc, link->link_index, &res);
> >> aconnector->i2c = i2c;
> >> @@ -3835,11 +3844,11 @@ static void handle_cursor_update(
> >> if (!plane->state->fb && !old_plane_state->fb)
> >> return;
> >> - DRM_DEBUG_KMS("%s: crtc_id=%d with size %d to %d\n",
> >> - __func__,
> >> - amdgpu_crtc->crtc_id,
> >> - plane->state->crtc_w,
> >> - plane->state->crtc_h);
> >> + DRM_DEBUG_DRIVER("%s: crtc_id=%d with size %d to %d\n",
> >> + __func__,
> >> + amdgpu_crtc->crtc_id,
> >> + plane->state->crtc_w,
> >> + plane->state->crtc_h);
> >> ret = get_cursor_position(plane, crtc, &position);
> >> if (ret)
> >> @@ -4142,7 +4151,7 @@ void amdgpu_dm_atomic_commit_tail(
> >> new_acrtc_state = to_dm_crtc_state(new_state);
> >> old_acrtc_state = to_dm_crtc_state(old_crtc_state);
> >> - DRM_DEBUG_KMS(
> >> + DRM_DEBUG_DRIVER(
> >> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
> >> active:%d, "
> >> "planes_changed:%d, mode_changed:%d,active_changed:%d,"
> >> "connectors_changed:%d\n",
> >> @@ -4160,7 +4169,7 @@ void amdgpu_dm_atomic_commit_tail(
> >> if (modeset_required(new_state, new_acrtc_state->stream,
> >> old_acrtc_state->stream)) {
> >> - DRM_INFO("Atomic commit: SET crtc id %d: [%p]\n",
> >> acrtc->crtc_id, acrtc);
> >> + DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d: [%p]\n",
> >> acrtc->crtc_id, acrtc);
> >> if (!new_acrtc_state->stream) {
> >> /*
> >> @@ -4178,7 +4187,7 @@ void amdgpu_dm_atomic_commit_tail(
> >> * have a sink to keep the pipe running so that
> >> * hw state is consistent with the sw state
> >> */
> >> - DRM_DEBUG_KMS("%s: Failed to create new stream for
> >> crtc %d\n",
> >> + DRM_DEBUG_DRIVER("%s: Failed to create new stream for
> >> crtc %d\n",
> >> __func__, acrtc->base.base.id);
> >> continue;
> >> }
> >> @@ -4205,7 +4214,7 @@ void amdgpu_dm_atomic_commit_tail(
> >> acrtc->hw_mode = crtc->state->mode;
> >> crtc->hwmode = crtc->state->mode;
> >> } else if (modereset_required(new_state)) {
> >> - DRM_INFO("Atomic commit: RESET. crtc id %d:[%p]\n",
> >> acrtc->crtc_id, acrtc);
> >> + DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id
> >> %d:[%p]\n", acrtc->crtc_id, acrtc);
> >> /* i.e. reset mode */
> >> if (old_acrtc_state->stream)
> >> @@ -4230,7 +4239,7 @@ void amdgpu_dm_atomic_commit_tail(
> >> &new_crtcs[i]->base,
> >> false);
> >> if (!aconnector) {
> >> - DRM_INFO("Atomic commit: Failed to find connector for
> >> acrtc id:%d "
> >> + DRM_DEBUG_DRIVER("Atomic commit: Failed to find
> >> connector for acrtc id:%d "
> >> "skipping freesync init\n",
> >> new_crtcs[i]->crtc_id);
> >> continue;
> >> @@ -4539,7 +4548,7 @@ static int dm_update_crtcs_state(
> >> */
> >> if (!new_stream) {
> >> - DRM_DEBUG_KMS("%s: Failed to create new stream for
> >> crtc %d\n",
> >> + DRM_DEBUG_DRIVER("%s: Failed to create new stream for
> >> crtc %d\n",
> >> __func__, acrtc->base.base.id);
> >> break;
> >> }
> >> @@ -4550,7 +4559,7 @@ static int dm_update_crtcs_state(
> >> crtc_state->mode_changed = false;
> >> - DRM_DEBUG_KMS("Mode change not required, setting
> >> mode_changed to %d",
> >> + DRM_DEBUG_DRIVER("Mode change not required, setting
> >> mode_changed to %d",
> >> crtc_state->mode_changed);
> >> }
> >> @@ -4558,7 +4567,7 @@ static int dm_update_crtcs_state(
> >> if (!drm_atomic_crtc_needs_modeset(crtc_state))
> >> goto next_crtc;
> >> - DRM_DEBUG_KMS(
> >> + DRM_DEBUG_DRIVER(
> >> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
> >> active:%d, "
> >> "planes_changed:%d, mode_changed:%d,active_changed:%d,"
> >> "connectors_changed:%d\n",
> >> @@ -4576,7 +4585,7 @@ static int dm_update_crtcs_state(
> >> if (!old_acrtc_state->stream)
> >> goto next_crtc;
> >> - DRM_DEBUG_KMS("Disabling DRM crtc: %d\n",
> >> + DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n",
> >> crtc->base.id);
> >> /* i.e. reset mode */
> >> @@ -4606,7 +4615,7 @@ static int dm_update_crtcs_state(
> >> new_acrtc_state->stream = new_stream;
> >> dc_stream_retain(new_stream);
> >> - DRM_DEBUG_KMS("Enabling DRM crtc: %d\n",
> >> + DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n",
> >> crtc->base.id);
> >> if (!dc_add_stream_to_ctx(
> >> @@ -4681,7 +4690,7 @@ static int dm_update_planes_state(
> >> if (!old_acrtc_state->stream)
> >> continue;
> >> - DRM_DEBUG_KMS("Disabling DRM plane: %d on DRM crtc %d\n",
> >> + DRM_DEBUG_DRIVER("Disabling DRM plane: %d on DRM crtc
> %d\n",
> >> plane->base.id, old_plane_crtc->base.id);
> >> if (!dc_remove_plane_from_context(
> >> @@ -4719,7 +4728,7 @@ static int dm_update_planes_state(
> >> new_dm_plane_state->dc_state = dc_create_plane_state(dc);
> >> - DRM_DEBUG_KMS("Enabling DRM plane: %d on DRM crtc %d\n",
> >> + DRM_DEBUG_DRIVER("Enabling DRM plane: %d on DRM crtc
> %d\n",
> >> plane->base.id, new_plane_crtc->base.id);
> >> if (!new_dm_plane_state->dc_state) {
> >> @@ -4874,9 +4883,9 @@ int amdgpu_dm_atomic_check(struct
> drm_device *dev,
> >> fail:
> >> if (ret == -EDEADLK)
> >> - DRM_DEBUG_KMS("Atomic check stopped due to to deadlock.\n");
> >> + DRM_DEBUG_DRIVER("Atomic check stopped due to to
> deadlock.\n");
> >> else if (ret == -EINTR || ret == -EAGAIN || ret == -ERESTARTSYS)
> >> - DRM_DEBUG_KMS("Atomic check stopped due to to signal.\n");
> >> + DRM_DEBUG_DRIVER("Atomic check stopped due to to signal.\n");
> >> else
> >> DRM_ERROR("Atomic check failed with err: %d \n", ret);
> >> diff --git a/drivers/gpu/drm/amd/display/include/logger_types.h
> >> b/drivers/gpu/drm/amd/display/include/logger_types.h
> >> index 044805ccac25..1f22e84cedb9 100644
> >> --- a/drivers/gpu/drm/amd/display/include/logger_types.h
> >> +++ b/drivers/gpu/drm/amd/display/include/logger_types.h
> >> @@ -70,6 +70,9 @@ enum dc_log_type {
> >> LOG_SECTION_TOTAL_COUNT
> >> };
> >> +#define DC_MIN_LOG_MASK ((1 << LOG_ERROR) | \
> >> + (1 << LOG_DETECTION_EDID_PARSER))
> >> +
> >> #define DC_DEFAULT_LOG_MASK ((1 << LOG_ERROR) | \
> >> (1 << LOG_WARNING) | \
> >> (1 << LOG_EVENT_MODE_SET) | \
> >
> >
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
[not found] ` <BN6PR12MB1652A614351C9BF5F1807ED0F76C0-/b2+HYfkarQqUD6E6FAiowdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2017-09-15 15:28 ` Christian König
[not found] ` <15920289-e141-b80e-4399-8f62b355883c-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 16+ messages in thread
From: Christian König @ 2017-09-15 15:28 UTC (permalink / raw)
To: Deucher, Alexander, Wentland, Harry,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Grodzovsky, Andrey, Cheng, Tony
Am 15.09.2017 um 17:26 schrieb Deucher, Alexander:
>> -----Original Message-----
>> From: Wentland, Harry
>> Sent: Friday, September 15, 2017 11:21 AM
>> To: Koenig, Christian; amd-gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander; Grodzovsky, Andrey; Cheng, Tony
>> Subject: Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
>>
>>
>>
>> On 2017-09-15 11:08 AM, Christian König wrote:
>>> Am 15.09.2017 um 16:32 schrieb Harry Wentland:
>>>> Log DC init but default log level to 0 (default for
>>>> amdgpu_dc_log) otherwise. Bug reporters can still make
>>>> DC more chatty like so
>>>> amdgpu.dc_log = 1
>>> Which is exactly the reason why I think this patch is superfluous.
>>>
>>> Either have a compile time option or a run time option, but please not
>>> both that's just confusing.
>>>
>> True. Thanks for the input.
>>
>> Gonna leave out the run time option for now.
> Another option would be to tie the dc debug levels to drm debug levels.
Which actually sounds like the correct solution to me.
I mean we have drm debug levels for display debugging stuff for years,
why do we need an extra logging for DC now?
Christian.
>
> Alex
>
>> Harry
>>
>>> Christian.
>>>
>>>> Change-Id: Icdfb849fa678225e2460519fbd8066540feb451a
>>>> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/display/Kconfig | 10 +++
>>>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 77
>>>> ++++++++++++----------
>>>> drivers/gpu/drm/amd/display/include/logger_types.h | 3 +
>>>> 3 files changed, 56 insertions(+), 34 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/display/Kconfig
>>>> b/drivers/gpu/drm/amd/display/Kconfig
>>>> index 1d1a5f807030..baab055dd362 100644
>>>> --- a/drivers/gpu/drm/amd/display/Kconfig
>>>> +++ b/drivers/gpu/drm/amd/display/Kconfig
>>>> @@ -31,4 +31,14 @@ config DEBUG_KERNEL_DC
>>>> if you want to hit
>>>> kdgb_break in assert.
>>>> +config DRM_AMD_DC_CHATTY
>>>> + bool "Make DC chatty again"
>>>> + default n
>>>> + depends on DRM_AMD_DC
>>>> + help
>>>> + Sometimes it's useful to have a chatty DC
>>>> + without a ton of spam from DRM. This allows
>>>> + for that and is recommended for anyone
>>>> + reporting bugs to DC.
>>>> +
>>>> endmenu
>>>> 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 abe89e3fed5b..6ecb420b2a63 100644
>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> @@ -333,7 +333,6 @@ int amdgpu_dm_init(struct amdgpu_device
>> *adev)
>>>> adev->dm.ddev = adev->ddev;
>>>> adev->dm.adev = adev;
>>>> - DRM_INFO("DAL is enabled\n");
>>>> /* Zero all the fields */
>>>> memset(&init_data, 0, sizeof(init_data));
>>>> @@ -373,7 +372,15 @@ int amdgpu_dm_init(struct amdgpu_device
>> *adev)
>>>> init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
>>>> +#ifdef CONFIG_DRM_AMD_DC_CHATTY
>>>> + /* always be chatty */
>>>> init_data.log_mask = DC_DEFAULT_LOG_MASK;
>>>> +#else
>>>> + if (amdgpu_dc_log)
>>>> + init_data.log_mask = DC_DEFAULT_LOG_MASK;
>>>> + else
>>>> + init_data.log_mask = DC_MIN_LOG_MASK;
>>>> +#endif
>>>> #ifdef ENABLE_FBC
>>>> if (adev->family == FAMILY_CZ)
>>>> @@ -383,7 +390,9 @@ int amdgpu_dm_init(struct amdgpu_device
>> *adev)
>>>> /* Display Core create. */
>>>> adev->dm.dc = dc_create(&init_data);
>>>> - if (!adev->dm.dc)
>>>> + if (adev->dm.dc)
>>>> + DRM_INFO("Display Core initialized!\n");
>>>> + else
>>>> DRM_INFO("Display Core failed to initialize!\n");
>>>> INIT_WORK(&adev->dm.mst_hotplug_work,
>> hotplug_notify_work_func);
>>>> @@ -393,7 +402,7 @@ int amdgpu_dm_init(struct amdgpu_device
>> *adev)
>>>> DRM_ERROR(
>>>> "amdgpu: failed to initialize freesync_module.\n");
>>>> } else
>>>> - DRM_INFO("amdgpu: freesync_module init done %p.\n",
>>>> + DRM_DEBUG_DRIVER("amdgpu: freesync_module init done %p.\n",
>>>> adev->dm.freesync_module);
>>>> if (amdgpu_dm_initialize_drm_device(adev)) {
>>>> @@ -417,7 +426,7 @@ int amdgpu_dm_init(struct amdgpu_device
>> *adev)
>>>> goto error;
>>>> }
>>>> - DRM_INFO("KMS initialized.\n");
>>>> + DRM_DEBUG_DRIVER("KMS initialized.\n");
>>>> return 0;
>>>> error:
>>>> @@ -475,7 +484,7 @@ static int
>>>> detect_mst_link_for_all_connectors(struct drm_device *dev)
>>>> list_for_each_entry(connector, &dev->mode_config.connector_list,
>>>> head) {
>>>> aconnector = to_amdgpu_dm_connector(connector);
>>>> if (aconnector->dc_link->type == dc_connection_mst_branch) {
>>>> - DRM_INFO("DM_MST: starting TM on aconnector: %p [id: %d]\n",
>>>> + DRM_DEBUG_DRIVER("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);
>>>> @@ -819,12 +828,12 @@ void
>> amdgpu_dm_update_connector_after_detect(
>>>> if (aconnector->dc_sink == sink) {
>>>> /* We got a DP short pulse (Link Loss, DP CTS, etc...).
>>>> * Do nothing!! */
>>>> - DRM_INFO("DCHPD: connector_id=%d: dc_sink didn't change.\n",
>>>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: dc_sink didn't
>>>> change.\n",
>>>> aconnector->connector_id);
>>>> return;
>>>> }
>>>> - DRM_INFO("DCHPD: connector_id=%d: Old sink=%p New
>> sink=%p\n",
>>>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: Old sink=%p New
>>>> sink=%p\n",
>>>> aconnector->connector_id, aconnector->dc_sink, sink);
>>>> mutex_lock(&dev->mode_config.mutex);
>>>> @@ -926,7 +935,7 @@ static void dm_handle_hpd_rx_irq(struct
>>>> amdgpu_dm_connector *aconnector)
>>>> process_count++;
>>>> - DRM_DEBUG_KMS("ESI %02x %02x %02x\n", esi[0], esi[1], esi[2]);
>>>> + DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], esi[1],
>>>> esi[2]);
>>>> /* handle HPD short pulse irq */
>>>> if (aconnector->mst_mgr.mst_state)
>>>> drm_dp_mst_hpd_irq(
>>>> @@ -964,7 +973,7 @@ static void dm_handle_hpd_rx_irq(struct
>>>> amdgpu_dm_connector *aconnector)
>>>> }
>>>> if (process_count == max_process_count)
>>>> - DRM_DEBUG_KMS("Loop exceeded max iterations\n");
>>>> + DRM_DEBUG_DRIVER("Loop exceeded max iterations\n");
>>>> }
>>>> static void handle_hpd_rx_irq(void *param)
>>>> @@ -1283,7 +1292,7 @@ void
>> amdgpu_dm_register_backlight_device(struct
>>>> amdgpu_display_manager *dm)
>>>> if (NULL == dm->backlight_dev)
>>>> DRM_ERROR("DM: Backlight registration failed!\n");
>>>> else
>>>> - DRM_INFO("DM: Registered Backlight device: %s\n", bl_name);
>>>> + DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n",
>>>> bl_name);
>>>> }
>>>> #endif
>>>> @@ -2064,7 +2073,7 @@ static void update_stream_scaling_settings(
>>>> stream->src = src;
>>>> stream->dst = dst;
>>>> - DRM_DEBUG_KMS("Destination Rectangle x:%d y:%d width:%d
>>>> height:%d\n",
>>>> + DRM_DEBUG_DRIVER("Destination Rectangle x:%d y:%d width:%d
>>>> height:%d\n",
>>>> dst.x, dst.y, dst.width, dst.height);
>>>> }
>>>> @@ -2374,7 +2383,7 @@ static struct dc_stream_state
>>>> *create_stream_for_sink(
>>>> * case, we call set mode ourselves to restore the previous
>>>> mode
>>>> * and the modelist may not be filled in in time.
>>>> */
>>>> - DRM_INFO("No preferred mode found\n");
>>>> + DRM_DEBUG_DRIVER("No preferred mode found\n");
>>>> } else {
>>>> decide_crtc_timing_for_drm_display_mode(
>>>> &mode, preferred_mode,
>>>> @@ -2749,7 +2758,7 @@ static struct drm_encoder *best_encoder(struct
>>>> drm_connector *connector)
>>>> struct drm_mode_object *obj;
>>>> struct drm_encoder *encoder;
>>>> - DRM_DEBUG_KMS("Finding the best encoder\n");
>>>> + DRM_DEBUG_DRIVER("Finding the best encoder\n");
>>>> /* pick the encoder ids */
>>>> if (enc_id) {
>>>> @@ -3019,7 +3028,7 @@ static int dm_plane_helper_prepare_fb(
>>>> dm_plane_state_new = to_dm_plane_state(new_state);
>>>> if (!new_state->fb) {
>>>> - DRM_DEBUG_KMS("No FB bound\n");
>>>> + DRM_DEBUG_DRIVER("No FB bound\n");
>>>> return 0;
>>>> }
>>>> @@ -3594,7 +3603,7 @@ int amdgpu_dm_connector_init(
>>>> struct amdgpu_i2c_adapter *i2c;
>>>> ((struct dc_link *)link)->priv = aconnector;
>>>> - DRM_DEBUG_KMS("%s()\n", __func__);
>>>> + DRM_DEBUG_DRIVER("%s()\n", __func__);
>>>> i2c = create_i2c(link->ddc, link->link_index, &res);
>>>> aconnector->i2c = i2c;
>>>> @@ -3835,11 +3844,11 @@ static void handle_cursor_update(
>>>> if (!plane->state->fb && !old_plane_state->fb)
>>>> return;
>>>> - DRM_DEBUG_KMS("%s: crtc_id=%d with size %d to %d\n",
>>>> - __func__,
>>>> - amdgpu_crtc->crtc_id,
>>>> - plane->state->crtc_w,
>>>> - plane->state->crtc_h);
>>>> + DRM_DEBUG_DRIVER("%s: crtc_id=%d with size %d to %d\n",
>>>> + __func__,
>>>> + amdgpu_crtc->crtc_id,
>>>> + plane->state->crtc_w,
>>>> + plane->state->crtc_h);
>>>> ret = get_cursor_position(plane, crtc, &position);
>>>> if (ret)
>>>> @@ -4142,7 +4151,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>> new_acrtc_state = to_dm_crtc_state(new_state);
>>>> old_acrtc_state = to_dm_crtc_state(old_crtc_state);
>>>> - DRM_DEBUG_KMS(
>>>> + DRM_DEBUG_DRIVER(
>>>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>>>> active:%d, "
>>>> "planes_changed:%d, mode_changed:%d,active_changed:%d,"
>>>> "connectors_changed:%d\n",
>>>> @@ -4160,7 +4169,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>> if (modeset_required(new_state, new_acrtc_state->stream,
>>>> old_acrtc_state->stream)) {
>>>> - DRM_INFO("Atomic commit: SET crtc id %d: [%p]\n",
>>>> acrtc->crtc_id, acrtc);
>>>> + DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d: [%p]\n",
>>>> acrtc->crtc_id, acrtc);
>>>> if (!new_acrtc_state->stream) {
>>>> /*
>>>> @@ -4178,7 +4187,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>> * have a sink to keep the pipe running so that
>>>> * hw state is consistent with the sw state
>>>> */
>>>> - DRM_DEBUG_KMS("%s: Failed to create new stream for
>>>> crtc %d\n",
>>>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream for
>>>> crtc %d\n",
>>>> __func__, acrtc->base.base.id);
>>>> continue;
>>>> }
>>>> @@ -4205,7 +4214,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>> acrtc->hw_mode = crtc->state->mode;
>>>> crtc->hwmode = crtc->state->mode;
>>>> } else if (modereset_required(new_state)) {
>>>> - DRM_INFO("Atomic commit: RESET. crtc id %d:[%p]\n",
>>>> acrtc->crtc_id, acrtc);
>>>> + DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id
>>>> %d:[%p]\n", acrtc->crtc_id, acrtc);
>>>> /* i.e. reset mode */
>>>> if (old_acrtc_state->stream)
>>>> @@ -4230,7 +4239,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>> &new_crtcs[i]->base,
>>>> false);
>>>> if (!aconnector) {
>>>> - DRM_INFO("Atomic commit: Failed to find connector for
>>>> acrtc id:%d "
>>>> + DRM_DEBUG_DRIVER("Atomic commit: Failed to find
>>>> connector for acrtc id:%d "
>>>> "skipping freesync init\n",
>>>> new_crtcs[i]->crtc_id);
>>>> continue;
>>>> @@ -4539,7 +4548,7 @@ static int dm_update_crtcs_state(
>>>> */
>>>> if (!new_stream) {
>>>> - DRM_DEBUG_KMS("%s: Failed to create new stream for
>>>> crtc %d\n",
>>>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream for
>>>> crtc %d\n",
>>>> __func__, acrtc->base.base.id);
>>>> break;
>>>> }
>>>> @@ -4550,7 +4559,7 @@ static int dm_update_crtcs_state(
>>>> crtc_state->mode_changed = false;
>>>> - DRM_DEBUG_KMS("Mode change not required, setting
>>>> mode_changed to %d",
>>>> + DRM_DEBUG_DRIVER("Mode change not required, setting
>>>> mode_changed to %d",
>>>> crtc_state->mode_changed);
>>>> }
>>>> @@ -4558,7 +4567,7 @@ static int dm_update_crtcs_state(
>>>> if (!drm_atomic_crtc_needs_modeset(crtc_state))
>>>> goto next_crtc;
>>>> - DRM_DEBUG_KMS(
>>>> + DRM_DEBUG_DRIVER(
>>>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>>>> active:%d, "
>>>> "planes_changed:%d, mode_changed:%d,active_changed:%d,"
>>>> "connectors_changed:%d\n",
>>>> @@ -4576,7 +4585,7 @@ static int dm_update_crtcs_state(
>>>> if (!old_acrtc_state->stream)
>>>> goto next_crtc;
>>>> - DRM_DEBUG_KMS("Disabling DRM crtc: %d\n",
>>>> + DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n",
>>>> crtc->base.id);
>>>> /* i.e. reset mode */
>>>> @@ -4606,7 +4615,7 @@ static int dm_update_crtcs_state(
>>>> new_acrtc_state->stream = new_stream;
>>>> dc_stream_retain(new_stream);
>>>> - DRM_DEBUG_KMS("Enabling DRM crtc: %d\n",
>>>> + DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n",
>>>> crtc->base.id);
>>>> if (!dc_add_stream_to_ctx(
>>>> @@ -4681,7 +4690,7 @@ static int dm_update_planes_state(
>>>> if (!old_acrtc_state->stream)
>>>> continue;
>>>> - DRM_DEBUG_KMS("Disabling DRM plane: %d on DRM crtc %d\n",
>>>> + DRM_DEBUG_DRIVER("Disabling DRM plane: %d on DRM crtc
>> %d\n",
>>>> plane->base.id, old_plane_crtc->base.id);
>>>> if (!dc_remove_plane_from_context(
>>>> @@ -4719,7 +4728,7 @@ static int dm_update_planes_state(
>>>> new_dm_plane_state->dc_state = dc_create_plane_state(dc);
>>>> - DRM_DEBUG_KMS("Enabling DRM plane: %d on DRM crtc %d\n",
>>>> + DRM_DEBUG_DRIVER("Enabling DRM plane: %d on DRM crtc
>> %d\n",
>>>> plane->base.id, new_plane_crtc->base.id);
>>>> if (!new_dm_plane_state->dc_state) {
>>>> @@ -4874,9 +4883,9 @@ int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>>>> fail:
>>>> if (ret == -EDEADLK)
>>>> - DRM_DEBUG_KMS("Atomic check stopped due to to deadlock.\n");
>>>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to
>> deadlock.\n");
>>>> else if (ret == -EINTR || ret == -EAGAIN || ret == -ERESTARTSYS)
>>>> - DRM_DEBUG_KMS("Atomic check stopped due to to signal.\n");
>>>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to signal.\n");
>>>> else
>>>> DRM_ERROR("Atomic check failed with err: %d \n", ret);
>>>> diff --git a/drivers/gpu/drm/amd/display/include/logger_types.h
>>>> b/drivers/gpu/drm/amd/display/include/logger_types.h
>>>> index 044805ccac25..1f22e84cedb9 100644
>>>> --- a/drivers/gpu/drm/amd/display/include/logger_types.h
>>>> +++ b/drivers/gpu/drm/amd/display/include/logger_types.h
>>>> @@ -70,6 +70,9 @@ enum dc_log_type {
>>>> LOG_SECTION_TOTAL_COUNT
>>>> };
>>>> +#define DC_MIN_LOG_MASK ((1 << LOG_ERROR) | \
>>>> + (1 << LOG_DETECTION_EDID_PARSER))
>>>> +
>>>> #define DC_DEFAULT_LOG_MASK ((1 << LOG_ERROR) | \
>>>> (1 << LOG_WARNING) | \
>>>> (1 << LOG_EVENT_MODE_SET) | \
>>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
[not found] ` <15920289-e141-b80e-4399-8f62b355883c-5C7GfCeVMHo@public.gmane.org>
@ 2017-09-15 15:31 ` Harry Wentland
2017-09-15 15:35 ` Felix Kuehling
1 sibling, 0 replies; 16+ messages in thread
From: Harry Wentland @ 2017-09-15 15:31 UTC (permalink / raw)
To: Christian König, Deucher, Alexander,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Grodzovsky, Andrey, Cheng, Tony
On 2017-09-15 11:28 AM, Christian König wrote:
> Am 15.09.2017 um 17:26 schrieb Deucher, Alexander:
>>> -----Original Message-----
>>> From: Wentland, Harry
>>> Sent: Friday, September 15, 2017 11:21 AM
>>> To: Koenig, Christian; amd-gfx@lists.freedesktop.org
>>> Cc: Deucher, Alexander; Grodzovsky, Andrey; Cheng, Tony
>>> Subject: Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
>>>
>>>
>>>
>>> On 2017-09-15 11:08 AM, Christian König wrote:
>>>> Am 15.09.2017 um 16:32 schrieb Harry Wentland:
>>>>> Log DC init but default log level to 0 (default for
>>>>> amdgpu_dc_log) otherwise. Bug reporters can still make
>>>>> DC more chatty like so
>>>>> amdgpu.dc_log = 1
>>>> Which is exactly the reason why I think this patch is superfluous.
>>>>
>>>> Either have a compile time option or a run time option, but please not
>>>> both that's just confusing.
>>>>
>>> True. Thanks for the input.
>>>
>>> Gonna leave out the run time option for now.
>> Another option would be to tie the dc debug levels to drm debug levels.
>
> Which actually sounds like the correct solution to me.
>
> I mean we have drm debug levels for display debugging stuff for years,
> why do we need an extra logging for DC now?
>
I agree, but one of the issues we've faced is that there's a whole lot
of log-spam that's unrelated to DC which often prevents people seeing DC
problems. Does DRM have log levels that can be used more fine-grained
than DEBUG_KMS/DRIVER/etc?
Harry
> Christian.
>
>>
>> Alex
>>
>>> Harry
>>>
>>>> Christian.
>>>>
>>>>> Change-Id: Icdfb849fa678225e2460519fbd8066540feb451a
>>>>> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
>>>>> ---
>>>>> drivers/gpu/drm/amd/display/Kconfig | 10 +++
>>>>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 77
>>>>> ++++++++++++----------
>>>>> drivers/gpu/drm/amd/display/include/logger_types.h | 3 +
>>>>> 3 files changed, 56 insertions(+), 34 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/display/Kconfig
>>>>> b/drivers/gpu/drm/amd/display/Kconfig
>>>>> index 1d1a5f807030..baab055dd362 100644
>>>>> --- a/drivers/gpu/drm/amd/display/Kconfig
>>>>> +++ b/drivers/gpu/drm/amd/display/Kconfig
>>>>> @@ -31,4 +31,14 @@ config DEBUG_KERNEL_DC
>>>>> if you want to hit
>>>>> kdgb_break in assert.
>>>>> +config DRM_AMD_DC_CHATTY
>>>>> + bool "Make DC chatty again"
>>>>> + default n
>>>>> + depends on DRM_AMD_DC
>>>>> + help
>>>>> + Sometimes it's useful to have a chatty DC
>>>>> + without a ton of spam from DRM. This allows
>>>>> + for that and is recommended for anyone
>>>>> + reporting bugs to DC.
>>>>> +
>>>>> endmenu
>>>>> 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 abe89e3fed5b..6ecb420b2a63 100644
>>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>> @@ -333,7 +333,6 @@ int amdgpu_dm_init(struct amdgpu_device
>>> *adev)
>>>>> adev->dm.ddev = adev->ddev;
>>>>> adev->dm.adev = adev;
>>>>> - DRM_INFO("DAL is enabled\n");
>>>>> /* Zero all the fields */
>>>>> memset(&init_data, 0, sizeof(init_data));
>>>>> @@ -373,7 +372,15 @@ int amdgpu_dm_init(struct amdgpu_device
>>> *adev)
>>>>> init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
>>>>> +#ifdef CONFIG_DRM_AMD_DC_CHATTY
>>>>> + /* always be chatty */
>>>>> init_data.log_mask = DC_DEFAULT_LOG_MASK;
>>>>> +#else
>>>>> + if (amdgpu_dc_log)
>>>>> + init_data.log_mask = DC_DEFAULT_LOG_MASK;
>>>>> + else
>>>>> + init_data.log_mask = DC_MIN_LOG_MASK;
>>>>> +#endif
>>>>> #ifdef ENABLE_FBC
>>>>> if (adev->family == FAMILY_CZ)
>>>>> @@ -383,7 +390,9 @@ int amdgpu_dm_init(struct amdgpu_device
>>> *adev)
>>>>> /* Display Core create. */
>>>>> adev->dm.dc = dc_create(&init_data);
>>>>> - if (!adev->dm.dc)
>>>>> + if (adev->dm.dc)
>>>>> + DRM_INFO("Display Core initialized!\n");
>>>>> + else
>>>>> DRM_INFO("Display Core failed to initialize!\n");
>>>>> INIT_WORK(&adev->dm.mst_hotplug_work,
>>> hotplug_notify_work_func);
>>>>> @@ -393,7 +402,7 @@ int amdgpu_dm_init(struct amdgpu_device
>>> *adev)
>>>>> DRM_ERROR(
>>>>> "amdgpu: failed to initialize freesync_module.\n");
>>>>> } else
>>>>> - DRM_INFO("amdgpu: freesync_module init done %p.\n",
>>>>> + DRM_DEBUG_DRIVER("amdgpu: freesync_module init done %p.\n",
>>>>> adev->dm.freesync_module);
>>>>> if (amdgpu_dm_initialize_drm_device(adev)) {
>>>>> @@ -417,7 +426,7 @@ int amdgpu_dm_init(struct amdgpu_device
>>> *adev)
>>>>> goto error;
>>>>> }
>>>>> - DRM_INFO("KMS initialized.\n");
>>>>> + DRM_DEBUG_DRIVER("KMS initialized.\n");
>>>>> return 0;
>>>>> error:
>>>>> @@ -475,7 +484,7 @@ static int
>>>>> detect_mst_link_for_all_connectors(struct drm_device *dev)
>>>>> list_for_each_entry(connector,
>>>>> &dev->mode_config.connector_list,
>>>>> head) {
>>>>> aconnector = to_amdgpu_dm_connector(connector);
>>>>> if (aconnector->dc_link->type ==
>>>>> dc_connection_mst_branch) {
>>>>> - DRM_INFO("DM_MST: starting TM on aconnector: %p [id:
>>>>> %d]\n",
>>>>> + DRM_DEBUG_DRIVER("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);
>>>>> @@ -819,12 +828,12 @@ void
>>> amdgpu_dm_update_connector_after_detect(
>>>>> if (aconnector->dc_sink == sink) {
>>>>> /* We got a DP short pulse (Link Loss, DP CTS, etc...).
>>>>> * Do nothing!! */
>>>>> - DRM_INFO("DCHPD: connector_id=%d: dc_sink didn't change.\n",
>>>>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: dc_sink didn't
>>>>> change.\n",
>>>>> aconnector->connector_id);
>>>>> return;
>>>>> }
>>>>> - DRM_INFO("DCHPD: connector_id=%d: Old sink=%p New
>>> sink=%p\n",
>>>>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: Old sink=%p New
>>>>> sink=%p\n",
>>>>> aconnector->connector_id, aconnector->dc_sink, sink);
>>>>> mutex_lock(&dev->mode_config.mutex);
>>>>> @@ -926,7 +935,7 @@ static void dm_handle_hpd_rx_irq(struct
>>>>> amdgpu_dm_connector *aconnector)
>>>>> process_count++;
>>>>> - DRM_DEBUG_KMS("ESI %02x %02x %02x\n", esi[0], esi[1],
>>>>> esi[2]);
>>>>> + DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], esi[1],
>>>>> esi[2]);
>>>>> /* handle HPD short pulse irq */
>>>>> if (aconnector->mst_mgr.mst_state)
>>>>> drm_dp_mst_hpd_irq(
>>>>> @@ -964,7 +973,7 @@ static void dm_handle_hpd_rx_irq(struct
>>>>> amdgpu_dm_connector *aconnector)
>>>>> }
>>>>> if (process_count == max_process_count)
>>>>> - DRM_DEBUG_KMS("Loop exceeded max iterations\n");
>>>>> + DRM_DEBUG_DRIVER("Loop exceeded max iterations\n");
>>>>> }
>>>>> static void handle_hpd_rx_irq(void *param)
>>>>> @@ -1283,7 +1292,7 @@ void
>>> amdgpu_dm_register_backlight_device(struct
>>>>> amdgpu_display_manager *dm)
>>>>> if (NULL == dm->backlight_dev)
>>>>> DRM_ERROR("DM: Backlight registration failed!\n");
>>>>> else
>>>>> - DRM_INFO("DM: Registered Backlight device: %s\n", bl_name);
>>>>> + DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n",
>>>>> bl_name);
>>>>> }
>>>>> #endif
>>>>> @@ -2064,7 +2073,7 @@ static void update_stream_scaling_settings(
>>>>> stream->src = src;
>>>>> stream->dst = dst;
>>>>> - DRM_DEBUG_KMS("Destination Rectangle x:%d y:%d width:%d
>>>>> height:%d\n",
>>>>> + DRM_DEBUG_DRIVER("Destination Rectangle x:%d y:%d width:%d
>>>>> height:%d\n",
>>>>> dst.x, dst.y, dst.width, dst.height);
>>>>> }
>>>>> @@ -2374,7 +2383,7 @@ static struct dc_stream_state
>>>>> *create_stream_for_sink(
>>>>> * case, we call set mode ourselves to restore the previous
>>>>> mode
>>>>> * and the modelist may not be filled in in time.
>>>>> */
>>>>> - DRM_INFO("No preferred mode found\n");
>>>>> + DRM_DEBUG_DRIVER("No preferred mode found\n");
>>>>> } else {
>>>>> decide_crtc_timing_for_drm_display_mode(
>>>>> &mode, preferred_mode,
>>>>> @@ -2749,7 +2758,7 @@ static struct drm_encoder *best_encoder(struct
>>>>> drm_connector *connector)
>>>>> struct drm_mode_object *obj;
>>>>> struct drm_encoder *encoder;
>>>>> - DRM_DEBUG_KMS("Finding the best encoder\n");
>>>>> + DRM_DEBUG_DRIVER("Finding the best encoder\n");
>>>>> /* pick the encoder ids */
>>>>> if (enc_id) {
>>>>> @@ -3019,7 +3028,7 @@ static int dm_plane_helper_prepare_fb(
>>>>> dm_plane_state_new = to_dm_plane_state(new_state);
>>>>> if (!new_state->fb) {
>>>>> - DRM_DEBUG_KMS("No FB bound\n");
>>>>> + DRM_DEBUG_DRIVER("No FB bound\n");
>>>>> return 0;
>>>>> }
>>>>> @@ -3594,7 +3603,7 @@ int amdgpu_dm_connector_init(
>>>>> struct amdgpu_i2c_adapter *i2c;
>>>>> ((struct dc_link *)link)->priv = aconnector;
>>>>> - DRM_DEBUG_KMS("%s()\n", __func__);
>>>>> + DRM_DEBUG_DRIVER("%s()\n", __func__);
>>>>> i2c = create_i2c(link->ddc, link->link_index, &res);
>>>>> aconnector->i2c = i2c;
>>>>> @@ -3835,11 +3844,11 @@ static void handle_cursor_update(
>>>>> if (!plane->state->fb && !old_plane_state->fb)
>>>>> return;
>>>>> - DRM_DEBUG_KMS("%s: crtc_id=%d with size %d to %d\n",
>>>>> - __func__,
>>>>> - amdgpu_crtc->crtc_id,
>>>>> - plane->state->crtc_w,
>>>>> - plane->state->crtc_h);
>>>>> + DRM_DEBUG_DRIVER("%s: crtc_id=%d with size %d to %d\n",
>>>>> + __func__,
>>>>> + amdgpu_crtc->crtc_id,
>>>>> + plane->state->crtc_w,
>>>>> + plane->state->crtc_h);
>>>>> ret = get_cursor_position(plane, crtc, &position);
>>>>> if (ret)
>>>>> @@ -4142,7 +4151,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>> new_acrtc_state = to_dm_crtc_state(new_state);
>>>>> old_acrtc_state = to_dm_crtc_state(old_crtc_state);
>>>>> - DRM_DEBUG_KMS(
>>>>> + DRM_DEBUG_DRIVER(
>>>>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>>>>> active:%d, "
>>>>> "planes_changed:%d, mode_changed:%d,active_changed:%d,"
>>>>> "connectors_changed:%d\n",
>>>>> @@ -4160,7 +4169,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>> if (modeset_required(new_state, new_acrtc_state->stream,
>>>>> old_acrtc_state->stream)) {
>>>>> - DRM_INFO("Atomic commit: SET crtc id %d: [%p]\n",
>>>>> acrtc->crtc_id, acrtc);
>>>>> + DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d: [%p]\n",
>>>>> acrtc->crtc_id, acrtc);
>>>>> if (!new_acrtc_state->stream) {
>>>>> /*
>>>>> @@ -4178,7 +4187,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>> * have a sink to keep the pipe running so that
>>>>> * hw state is consistent with the sw state
>>>>> */
>>>>> - DRM_DEBUG_KMS("%s: Failed to create new stream for
>>>>> crtc %d\n",
>>>>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream for
>>>>> crtc %d\n",
>>>>> __func__, acrtc->base.base.id);
>>>>> continue;
>>>>> }
>>>>> @@ -4205,7 +4214,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>> acrtc->hw_mode = crtc->state->mode;
>>>>> crtc->hwmode = crtc->state->mode;
>>>>> } else if (modereset_required(new_state)) {
>>>>> - DRM_INFO("Atomic commit: RESET. crtc id %d:[%p]\n",
>>>>> acrtc->crtc_id, acrtc);
>>>>> + DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id
>>>>> %d:[%p]\n", acrtc->crtc_id, acrtc);
>>>>> /* i.e. reset mode */
>>>>> if (old_acrtc_state->stream)
>>>>> @@ -4230,7 +4239,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>> &new_crtcs[i]->base,
>>>>> false);
>>>>> if (!aconnector) {
>>>>> - DRM_INFO("Atomic commit: Failed to find connector for
>>>>> acrtc id:%d "
>>>>> + DRM_DEBUG_DRIVER("Atomic commit: Failed to find
>>>>> connector for acrtc id:%d "
>>>>> "skipping freesync init\n",
>>>>> new_crtcs[i]->crtc_id);
>>>>> continue;
>>>>> @@ -4539,7 +4548,7 @@ static int dm_update_crtcs_state(
>>>>> */
>>>>> if (!new_stream) {
>>>>> - DRM_DEBUG_KMS("%s: Failed to create new stream for
>>>>> crtc %d\n",
>>>>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream for
>>>>> crtc %d\n",
>>>>> __func__, acrtc->base.base.id);
>>>>> break;
>>>>> }
>>>>> @@ -4550,7 +4559,7 @@ static int dm_update_crtcs_state(
>>>>> crtc_state->mode_changed = false;
>>>>> - DRM_DEBUG_KMS("Mode change not required, setting
>>>>> mode_changed to %d",
>>>>> + DRM_DEBUG_DRIVER("Mode change not required, setting
>>>>> mode_changed to %d",
>>>>> crtc_state->mode_changed);
>>>>> }
>>>>> @@ -4558,7 +4567,7 @@ static int dm_update_crtcs_state(
>>>>> if (!drm_atomic_crtc_needs_modeset(crtc_state))
>>>>> goto next_crtc;
>>>>> - DRM_DEBUG_KMS(
>>>>> + DRM_DEBUG_DRIVER(
>>>>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>>>>> active:%d, "
>>>>> "planes_changed:%d, mode_changed:%d,active_changed:%d,"
>>>>> "connectors_changed:%d\n",
>>>>> @@ -4576,7 +4585,7 @@ static int dm_update_crtcs_state(
>>>>> if (!old_acrtc_state->stream)
>>>>> goto next_crtc;
>>>>> - DRM_DEBUG_KMS("Disabling DRM crtc: %d\n",
>>>>> + DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n",
>>>>> crtc->base.id);
>>>>> /* i.e. reset mode */
>>>>> @@ -4606,7 +4615,7 @@ static int dm_update_crtcs_state(
>>>>> new_acrtc_state->stream = new_stream;
>>>>> dc_stream_retain(new_stream);
>>>>> - DRM_DEBUG_KMS("Enabling DRM crtc: %d\n",
>>>>> + DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n",
>>>>> crtc->base.id);
>>>>> if (!dc_add_stream_to_ctx(
>>>>> @@ -4681,7 +4690,7 @@ static int dm_update_planes_state(
>>>>> if (!old_acrtc_state->stream)
>>>>> continue;
>>>>> - DRM_DEBUG_KMS("Disabling DRM plane: %d on DRM crtc
>>>>> %d\n",
>>>>> + DRM_DEBUG_DRIVER("Disabling DRM plane: %d on DRM crtc
>>> %d\n",
>>>>> plane->base.id, old_plane_crtc->base.id);
>>>>> if (!dc_remove_plane_from_context(
>>>>> @@ -4719,7 +4728,7 @@ static int dm_update_planes_state(
>>>>> new_dm_plane_state->dc_state =
>>>>> dc_create_plane_state(dc);
>>>>> - DRM_DEBUG_KMS("Enabling DRM plane: %d on DRM crtc
>>>>> %d\n",
>>>>> + DRM_DEBUG_DRIVER("Enabling DRM plane: %d on DRM crtc
>>> %d\n",
>>>>> plane->base.id, new_plane_crtc->base.id);
>>>>> if (!new_dm_plane_state->dc_state) {
>>>>> @@ -4874,9 +4883,9 @@ int amdgpu_dm_atomic_check(struct
>>> drm_device *dev,
>>>>> fail:
>>>>> if (ret == -EDEADLK)
>>>>> - DRM_DEBUG_KMS("Atomic check stopped due to to deadlock.\n");
>>>>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to
>>> deadlock.\n");
>>>>> else if (ret == -EINTR || ret == -EAGAIN || ret ==
>>>>> -ERESTARTSYS)
>>>>> - DRM_DEBUG_KMS("Atomic check stopped due to to signal.\n");
>>>>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to signal.\n");
>>>>> else
>>>>> DRM_ERROR("Atomic check failed with err: %d \n", ret);
>>>>> diff --git a/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>> b/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>> index 044805ccac25..1f22e84cedb9 100644
>>>>> --- a/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>> +++ b/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>> @@ -70,6 +70,9 @@ enum dc_log_type {
>>>>> LOG_SECTION_TOTAL_COUNT
>>>>> };
>>>>> +#define DC_MIN_LOG_MASK ((1 << LOG_ERROR) | \
>>>>> + (1 << LOG_DETECTION_EDID_PARSER))
>>>>> +
>>>>> #define DC_DEFAULT_LOG_MASK ((1 << LOG_ERROR) | \
>>>>> (1 << LOG_WARNING) | \
>>>>> (1 << LOG_EVENT_MODE_SET) | \
>>>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
[not found] ` <15920289-e141-b80e-4399-8f62b355883c-5C7GfCeVMHo@public.gmane.org>
2017-09-15 15:31 ` Harry Wentland
@ 2017-09-15 15:35 ` Felix Kuehling
[not found] ` <3395270c-9ae1-b8ec-72c9-e24b227988f6-5C7GfCeVMHo@public.gmane.org>
1 sibling, 1 reply; 16+ messages in thread
From: Felix Kuehling @ 2017-09-15 15:35 UTC (permalink / raw)
To: Christian König, Deucher, Alexander, Wentland, Harry,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Grodzovsky, Andrey, Cheng, Tony
On 2017-09-15 11:28 AM, Christian König wrote:
> Am 15.09.2017 um 17:26 schrieb Deucher, Alexander:
>>> -----Original Message-----
>>> From: Wentland, Harry
>>> Sent: Friday, September 15, 2017 11:21 AM
>>> To: Koenig, Christian; amd-gfx@lists.freedesktop.org
>>> Cc: Deucher, Alexander; Grodzovsky, Andrey; Cheng, Tony
>>> Subject: Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
>>>
>>>
>>>
>>> On 2017-09-15 11:08 AM, Christian König wrote:
>>>> Am 15.09.2017 um 16:32 schrieb Harry Wentland:
>>>>> Log DC init but default log level to 0 (default for
>>>>> amdgpu_dc_log) otherwise. Bug reporters can still make
>>>>> DC more chatty like so
>>>>> amdgpu.dc_log = 1
>>>> Which is exactly the reason why I think this patch is superfluous.
>>>>
>>>> Either have a compile time option or a run time option, but please not
>>>> both that's just confusing.
>>>>
>>> True. Thanks for the input.
>>>
>>> Gonna leave out the run time option for now.
>> Another option would be to tie the dc debug levels to drm debug levels.
>
> Which actually sounds like the correct solution to me.
>
> I mean we have drm debug levels for display debugging stuff for years,
> why do we need an extra logging for DC now?
FWIW, in KFD we rely on dynamic debug messages (CONFIG_DYNAMIC_DEBUG)
with pr_debug. This pretty much obsoletes any driver-specific debug
messages options. And it's quite versatile because it allows us to turn
on and off messages, by module, source file, function, or even by line
number through debugfs. So you can be more or less specific about which
messages you want to see, and they're all quiet by default. See also
Documentation/admin-guide/dynamic-debug-howto.rst.
Regards,
Felix
>
> Christian.
>
>>
>> Alex
>>
>>> Harry
>>>
>>>> Christian.
>>>>
>>>>> Change-Id: Icdfb849fa678225e2460519fbd8066540feb451a
>>>>> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
>>>>> ---
>>>>> drivers/gpu/drm/amd/display/Kconfig | 10 +++
>>>>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 77
>>>>> ++++++++++++----------
>>>>> drivers/gpu/drm/amd/display/include/logger_types.h | 3 +
>>>>> 3 files changed, 56 insertions(+), 34 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/display/Kconfig
>>>>> b/drivers/gpu/drm/amd/display/Kconfig
>>>>> index 1d1a5f807030..baab055dd362 100644
>>>>> --- a/drivers/gpu/drm/amd/display/Kconfig
>>>>> +++ b/drivers/gpu/drm/amd/display/Kconfig
>>>>> @@ -31,4 +31,14 @@ config DEBUG_KERNEL_DC
>>>>> if you want to hit
>>>>> kdgb_break in assert.
>>>>> +config DRM_AMD_DC_CHATTY
>>>>> + bool "Make DC chatty again"
>>>>> + default n
>>>>> + depends on DRM_AMD_DC
>>>>> + help
>>>>> + Sometimes it's useful to have a chatty DC
>>>>> + without a ton of spam from DRM. This allows
>>>>> + for that and is recommended for anyone
>>>>> + reporting bugs to DC.
>>>>> +
>>>>> endmenu
>>>>> 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 abe89e3fed5b..6ecb420b2a63 100644
>>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>> @@ -333,7 +333,6 @@ int amdgpu_dm_init(struct amdgpu_device
>>> *adev)
>>>>> adev->dm.ddev = adev->ddev;
>>>>> adev->dm.adev = adev;
>>>>> - DRM_INFO("DAL is enabled\n");
>>>>> /* Zero all the fields */
>>>>> memset(&init_data, 0, sizeof(init_data));
>>>>> @@ -373,7 +372,15 @@ int amdgpu_dm_init(struct amdgpu_device
>>> *adev)
>>>>> init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
>>>>> +#ifdef CONFIG_DRM_AMD_DC_CHATTY
>>>>> + /* always be chatty */
>>>>> init_data.log_mask = DC_DEFAULT_LOG_MASK;
>>>>> +#else
>>>>> + if (amdgpu_dc_log)
>>>>> + init_data.log_mask = DC_DEFAULT_LOG_MASK;
>>>>> + else
>>>>> + init_data.log_mask = DC_MIN_LOG_MASK;
>>>>> +#endif
>>>>> #ifdef ENABLE_FBC
>>>>> if (adev->family == FAMILY_CZ)
>>>>> @@ -383,7 +390,9 @@ int amdgpu_dm_init(struct amdgpu_device
>>> *adev)
>>>>> /* Display Core create. */
>>>>> adev->dm.dc = dc_create(&init_data);
>>>>> - if (!adev->dm.dc)
>>>>> + if (adev->dm.dc)
>>>>> + DRM_INFO("Display Core initialized!\n");
>>>>> + else
>>>>> DRM_INFO("Display Core failed to initialize!\n");
>>>>> INIT_WORK(&adev->dm.mst_hotplug_work,
>>> hotplug_notify_work_func);
>>>>> @@ -393,7 +402,7 @@ int amdgpu_dm_init(struct amdgpu_device
>>> *adev)
>>>>> DRM_ERROR(
>>>>> "amdgpu: failed to initialize freesync_module.\n");
>>>>> } else
>>>>> - DRM_INFO("amdgpu: freesync_module init done %p.\n",
>>>>> + DRM_DEBUG_DRIVER("amdgpu: freesync_module init done %p.\n",
>>>>> adev->dm.freesync_module);
>>>>> if (amdgpu_dm_initialize_drm_device(adev)) {
>>>>> @@ -417,7 +426,7 @@ int amdgpu_dm_init(struct amdgpu_device
>>> *adev)
>>>>> goto error;
>>>>> }
>>>>> - DRM_INFO("KMS initialized.\n");
>>>>> + DRM_DEBUG_DRIVER("KMS initialized.\n");
>>>>> return 0;
>>>>> error:
>>>>> @@ -475,7 +484,7 @@ static int
>>>>> detect_mst_link_for_all_connectors(struct drm_device *dev)
>>>>> list_for_each_entry(connector,
>>>>> &dev->mode_config.connector_list,
>>>>> head) {
>>>>> aconnector = to_amdgpu_dm_connector(connector);
>>>>> if (aconnector->dc_link->type ==
>>>>> dc_connection_mst_branch) {
>>>>> - DRM_INFO("DM_MST: starting TM on aconnector: %p [id:
>>>>> %d]\n",
>>>>> + DRM_DEBUG_DRIVER("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);
>>>>> @@ -819,12 +828,12 @@ void
>>> amdgpu_dm_update_connector_after_detect(
>>>>> if (aconnector->dc_sink == sink) {
>>>>> /* We got a DP short pulse (Link Loss, DP CTS, etc...).
>>>>> * Do nothing!! */
>>>>> - DRM_INFO("DCHPD: connector_id=%d: dc_sink didn't change.\n",
>>>>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: dc_sink didn't
>>>>> change.\n",
>>>>> aconnector->connector_id);
>>>>> return;
>>>>> }
>>>>> - DRM_INFO("DCHPD: connector_id=%d: Old sink=%p New
>>> sink=%p\n",
>>>>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: Old sink=%p New
>>>>> sink=%p\n",
>>>>> aconnector->connector_id, aconnector->dc_sink, sink);
>>>>> mutex_lock(&dev->mode_config.mutex);
>>>>> @@ -926,7 +935,7 @@ static void dm_handle_hpd_rx_irq(struct
>>>>> amdgpu_dm_connector *aconnector)
>>>>> process_count++;
>>>>> - DRM_DEBUG_KMS("ESI %02x %02x %02x\n", esi[0], esi[1],
>>>>> esi[2]);
>>>>> + DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], esi[1],
>>>>> esi[2]);
>>>>> /* handle HPD short pulse irq */
>>>>> if (aconnector->mst_mgr.mst_state)
>>>>> drm_dp_mst_hpd_irq(
>>>>> @@ -964,7 +973,7 @@ static void dm_handle_hpd_rx_irq(struct
>>>>> amdgpu_dm_connector *aconnector)
>>>>> }
>>>>> if (process_count == max_process_count)
>>>>> - DRM_DEBUG_KMS("Loop exceeded max iterations\n");
>>>>> + DRM_DEBUG_DRIVER("Loop exceeded max iterations\n");
>>>>> }
>>>>> static void handle_hpd_rx_irq(void *param)
>>>>> @@ -1283,7 +1292,7 @@ void
>>> amdgpu_dm_register_backlight_device(struct
>>>>> amdgpu_display_manager *dm)
>>>>> if (NULL == dm->backlight_dev)
>>>>> DRM_ERROR("DM: Backlight registration failed!\n");
>>>>> else
>>>>> - DRM_INFO("DM: Registered Backlight device: %s\n", bl_name);
>>>>> + DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n",
>>>>> bl_name);
>>>>> }
>>>>> #endif
>>>>> @@ -2064,7 +2073,7 @@ static void update_stream_scaling_settings(
>>>>> stream->src = src;
>>>>> stream->dst = dst;
>>>>> - DRM_DEBUG_KMS("Destination Rectangle x:%d y:%d width:%d
>>>>> height:%d\n",
>>>>> + DRM_DEBUG_DRIVER("Destination Rectangle x:%d y:%d width:%d
>>>>> height:%d\n",
>>>>> dst.x, dst.y, dst.width, dst.height);
>>>>> }
>>>>> @@ -2374,7 +2383,7 @@ static struct dc_stream_state
>>>>> *create_stream_for_sink(
>>>>> * case, we call set mode ourselves to restore the
>>>>> previous
>>>>> mode
>>>>> * and the modelist may not be filled in in time.
>>>>> */
>>>>> - DRM_INFO("No preferred mode found\n");
>>>>> + DRM_DEBUG_DRIVER("No preferred mode found\n");
>>>>> } else {
>>>>> decide_crtc_timing_for_drm_display_mode(
>>>>> &mode, preferred_mode,
>>>>> @@ -2749,7 +2758,7 @@ static struct drm_encoder *best_encoder(struct
>>>>> drm_connector *connector)
>>>>> struct drm_mode_object *obj;
>>>>> struct drm_encoder *encoder;
>>>>> - DRM_DEBUG_KMS("Finding the best encoder\n");
>>>>> + DRM_DEBUG_DRIVER("Finding the best encoder\n");
>>>>> /* pick the encoder ids */
>>>>> if (enc_id) {
>>>>> @@ -3019,7 +3028,7 @@ static int dm_plane_helper_prepare_fb(
>>>>> dm_plane_state_new = to_dm_plane_state(new_state);
>>>>> if (!new_state->fb) {
>>>>> - DRM_DEBUG_KMS("No FB bound\n");
>>>>> + DRM_DEBUG_DRIVER("No FB bound\n");
>>>>> return 0;
>>>>> }
>>>>> @@ -3594,7 +3603,7 @@ int amdgpu_dm_connector_init(
>>>>> struct amdgpu_i2c_adapter *i2c;
>>>>> ((struct dc_link *)link)->priv = aconnector;
>>>>> - DRM_DEBUG_KMS("%s()\n", __func__);
>>>>> + DRM_DEBUG_DRIVER("%s()\n", __func__);
>>>>> i2c = create_i2c(link->ddc, link->link_index, &res);
>>>>> aconnector->i2c = i2c;
>>>>> @@ -3835,11 +3844,11 @@ static void handle_cursor_update(
>>>>> if (!plane->state->fb && !old_plane_state->fb)
>>>>> return;
>>>>> - DRM_DEBUG_KMS("%s: crtc_id=%d with size %d to %d\n",
>>>>> - __func__,
>>>>> - amdgpu_crtc->crtc_id,
>>>>> - plane->state->crtc_w,
>>>>> - plane->state->crtc_h);
>>>>> + DRM_DEBUG_DRIVER("%s: crtc_id=%d with size %d to %d\n",
>>>>> + __func__,
>>>>> + amdgpu_crtc->crtc_id,
>>>>> + plane->state->crtc_w,
>>>>> + plane->state->crtc_h);
>>>>> ret = get_cursor_position(plane, crtc, &position);
>>>>> if (ret)
>>>>> @@ -4142,7 +4151,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>> new_acrtc_state = to_dm_crtc_state(new_state);
>>>>> old_acrtc_state = to_dm_crtc_state(old_crtc_state);
>>>>> - DRM_DEBUG_KMS(
>>>>> + DRM_DEBUG_DRIVER(
>>>>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>>>>> active:%d, "
>>>>> "planes_changed:%d,
>>>>> mode_changed:%d,active_changed:%d,"
>>>>> "connectors_changed:%d\n",
>>>>> @@ -4160,7 +4169,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>> if (modeset_required(new_state, new_acrtc_state->stream,
>>>>> old_acrtc_state->stream)) {
>>>>> - DRM_INFO("Atomic commit: SET crtc id %d: [%p]\n",
>>>>> acrtc->crtc_id, acrtc);
>>>>> + DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d:
>>>>> [%p]\n",
>>>>> acrtc->crtc_id, acrtc);
>>>>> if (!new_acrtc_state->stream) {
>>>>> /*
>>>>> @@ -4178,7 +4187,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>> * have a sink to keep the pipe running so that
>>>>> * hw state is consistent with the sw state
>>>>> */
>>>>> - DRM_DEBUG_KMS("%s: Failed to create new stream for
>>>>> crtc %d\n",
>>>>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream
>>>>> for
>>>>> crtc %d\n",
>>>>> __func__, acrtc->base.base.id);
>>>>> continue;
>>>>> }
>>>>> @@ -4205,7 +4214,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>> acrtc->hw_mode = crtc->state->mode;
>>>>> crtc->hwmode = crtc->state->mode;
>>>>> } else if (modereset_required(new_state)) {
>>>>> - DRM_INFO("Atomic commit: RESET. crtc id %d:[%p]\n",
>>>>> acrtc->crtc_id, acrtc);
>>>>> + DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id
>>>>> %d:[%p]\n", acrtc->crtc_id, acrtc);
>>>>> /* i.e. reset mode */
>>>>> if (old_acrtc_state->stream)
>>>>> @@ -4230,7 +4239,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>> &new_crtcs[i]->base,
>>>>> false);
>>>>> if (!aconnector) {
>>>>> - DRM_INFO("Atomic commit: Failed to find connector
>>>>> for
>>>>> acrtc id:%d "
>>>>> + DRM_DEBUG_DRIVER("Atomic commit: Failed to find
>>>>> connector for acrtc id:%d "
>>>>> "skipping freesync init\n",
>>>>> new_crtcs[i]->crtc_id);
>>>>> continue;
>>>>> @@ -4539,7 +4548,7 @@ static int dm_update_crtcs_state(
>>>>> */
>>>>> if (!new_stream) {
>>>>> - DRM_DEBUG_KMS("%s: Failed to create new stream for
>>>>> crtc %d\n",
>>>>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream
>>>>> for
>>>>> crtc %d\n",
>>>>> __func__, acrtc->base.base.id);
>>>>> break;
>>>>> }
>>>>> @@ -4550,7 +4559,7 @@ static int dm_update_crtcs_state(
>>>>> crtc_state->mode_changed = false;
>>>>> - DRM_DEBUG_KMS("Mode change not required, setting
>>>>> mode_changed to %d",
>>>>> + DRM_DEBUG_DRIVER("Mode change not required, setting
>>>>> mode_changed to %d",
>>>>> crtc_state->mode_changed);
>>>>> }
>>>>> @@ -4558,7 +4567,7 @@ static int dm_update_crtcs_state(
>>>>> if (!drm_atomic_crtc_needs_modeset(crtc_state))
>>>>> goto next_crtc;
>>>>> - DRM_DEBUG_KMS(
>>>>> + DRM_DEBUG_DRIVER(
>>>>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>>>>> active:%d, "
>>>>> "planes_changed:%d,
>>>>> mode_changed:%d,active_changed:%d,"
>>>>> "connectors_changed:%d\n",
>>>>> @@ -4576,7 +4585,7 @@ static int dm_update_crtcs_state(
>>>>> if (!old_acrtc_state->stream)
>>>>> goto next_crtc;
>>>>> - DRM_DEBUG_KMS("Disabling DRM crtc: %d\n",
>>>>> + DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n",
>>>>> crtc->base.id);
>>>>> /* i.e. reset mode */
>>>>> @@ -4606,7 +4615,7 @@ static int dm_update_crtcs_state(
>>>>> new_acrtc_state->stream = new_stream;
>>>>> dc_stream_retain(new_stream);
>>>>> - DRM_DEBUG_KMS("Enabling DRM crtc: %d\n",
>>>>> + DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n",
>>>>> crtc->base.id);
>>>>> if (!dc_add_stream_to_ctx(
>>>>> @@ -4681,7 +4690,7 @@ static int dm_update_planes_state(
>>>>> if (!old_acrtc_state->stream)
>>>>> continue;
>>>>> - DRM_DEBUG_KMS("Disabling DRM plane: %d on DRM crtc
>>>>> %d\n",
>>>>> + DRM_DEBUG_DRIVER("Disabling DRM plane: %d on DRM crtc
>>> %d\n",
>>>>> plane->base.id, old_plane_crtc->base.id);
>>>>> if (!dc_remove_plane_from_context(
>>>>> @@ -4719,7 +4728,7 @@ static int dm_update_planes_state(
>>>>> new_dm_plane_state->dc_state =
>>>>> dc_create_plane_state(dc);
>>>>> - DRM_DEBUG_KMS("Enabling DRM plane: %d on DRM crtc
>>>>> %d\n",
>>>>> + DRM_DEBUG_DRIVER("Enabling DRM plane: %d on DRM crtc
>>> %d\n",
>>>>> plane->base.id, new_plane_crtc->base.id);
>>>>> if (!new_dm_plane_state->dc_state) {
>>>>> @@ -4874,9 +4883,9 @@ int amdgpu_dm_atomic_check(struct
>>> drm_device *dev,
>>>>> fail:
>>>>> if (ret == -EDEADLK)
>>>>> - DRM_DEBUG_KMS("Atomic check stopped due to to deadlock.\n");
>>>>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to
>>> deadlock.\n");
>>>>> else if (ret == -EINTR || ret == -EAGAIN || ret ==
>>>>> -ERESTARTSYS)
>>>>> - DRM_DEBUG_KMS("Atomic check stopped due to to signal.\n");
>>>>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to
>>>>> signal.\n");
>>>>> else
>>>>> DRM_ERROR("Atomic check failed with err: %d \n", ret);
>>>>> diff --git a/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>> b/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>> index 044805ccac25..1f22e84cedb9 100644
>>>>> --- a/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>> +++ b/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>> @@ -70,6 +70,9 @@ enum dc_log_type {
>>>>> LOG_SECTION_TOTAL_COUNT
>>>>> };
>>>>> +#define DC_MIN_LOG_MASK ((1 << LOG_ERROR) | \
>>>>> + (1 << LOG_DETECTION_EDID_PARSER))
>>>>> +
>>>>> #define DC_DEFAULT_LOG_MASK ((1 << LOG_ERROR) | \
>>>>> (1 << LOG_WARNING) | \
>>>>> (1 << LOG_EVENT_MODE_SET) | \
>>>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
[not found] ` <BN6PR12MB1652D287EE41EA30B10CCF0CF76C0-/b2+HYfkarQqUD6E6FAiowdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2017-09-15 15:42 ` Harry Wentland
0 siblings, 0 replies; 16+ messages in thread
From: Harry Wentland @ 2017-09-15 15:42 UTC (permalink / raw)
To: Deucher, Alexander, Koenig, Christian,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Grodzovsky, Andrey, Cheng, Tony
On 2017-09-15 11:27 AM, Deucher, Alexander wrote:
>> -----Original Message-----
>> From: Wentland, Harry
>> Sent: Friday, September 15, 2017 11:21 AM
>> To: Koenig, Christian; amd-gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander; Grodzovsky, Andrey; Cheng, Tony
>> Subject: Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
>>
>>
>>
>> On 2017-09-15 11:08 AM, Christian König wrote:
>>> Am 15.09.2017 um 16:32 schrieb Harry Wentland:
>>>> Log DC init but default log level to 0 (default for
>>>> amdgpu_dc_log) otherwise. Bug reporters can still make
>>>> DC more chatty like so
>>>> amdgpu.dc_log = 1
>>>
>>> Which is exactly the reason why I think this patch is superfluous.
>>>
>>> Either have a compile time option or a run time option, but please not
>>> both that's just confusing.
>>>
>>
>> True. Thanks for the input.
>>
>> Gonna leave out the run time option for now.
>
> I'd rather leave the runtime option and remove the compile option. It's easier to get logs by changing a runtime option than to ask a user to recompile their kernel.
>
True. Right now I was shooting for simplicity for internal teams and
know what a struggle it can be to teach some people how to pass a module
parameter. Enabling this in internal builds (to avoid this) plus giving
people a runtime option would be the ideal case in the short term.
I'll also look at Felix's suggestions but won't be able to fit this in
in a short period of time. I definitely realize that config option and
module param aren't quite the right way to go about this.
Harry
> Alex
>
>>
>> Harry
>>
>>> Christian.
>>>
>>>>
>>>> Change-Id: Icdfb849fa678225e2460519fbd8066540feb451a
>>>> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/display/Kconfig | 10 +++
>>>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 77
>>>> ++++++++++++----------
>>>> drivers/gpu/drm/amd/display/include/logger_types.h | 3 +
>>>> 3 files changed, 56 insertions(+), 34 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/display/Kconfig
>>>> b/drivers/gpu/drm/amd/display/Kconfig
>>>> index 1d1a5f807030..baab055dd362 100644
>>>> --- a/drivers/gpu/drm/amd/display/Kconfig
>>>> +++ b/drivers/gpu/drm/amd/display/Kconfig
>>>> @@ -31,4 +31,14 @@ config DEBUG_KERNEL_DC
>>>> if you want to hit
>>>> kdgb_break in assert.
>>>> +config DRM_AMD_DC_CHATTY
>>>> + bool "Make DC chatty again"
>>>> + default n
>>>> + depends on DRM_AMD_DC
>>>> + help
>>>> + Sometimes it's useful to have a chatty DC
>>>> + without a ton of spam from DRM. This allows
>>>> + for that and is recommended for anyone
>>>> + reporting bugs to DC.
>>>> +
>>>> endmenu
>>>> 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 abe89e3fed5b..6ecb420b2a63 100644
>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> @@ -333,7 +333,6 @@ int amdgpu_dm_init(struct amdgpu_device
>> *adev)
>>>> adev->dm.ddev = adev->ddev;
>>>> adev->dm.adev = adev;
>>>> - DRM_INFO("DAL is enabled\n");
>>>> /* Zero all the fields */
>>>> memset(&init_data, 0, sizeof(init_data));
>>>> @@ -373,7 +372,15 @@ int amdgpu_dm_init(struct amdgpu_device
>> *adev)
>>>> init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
>>>> +#ifdef CONFIG_DRM_AMD_DC_CHATTY
>>>> + /* always be chatty */
>>>> init_data.log_mask = DC_DEFAULT_LOG_MASK;
>>>> +#else
>>>> + if (amdgpu_dc_log)
>>>> + init_data.log_mask = DC_DEFAULT_LOG_MASK;
>>>> + else
>>>> + init_data.log_mask = DC_MIN_LOG_MASK;
>>>> +#endif
>>>> #ifdef ENABLE_FBC
>>>> if (adev->family == FAMILY_CZ)
>>>> @@ -383,7 +390,9 @@ int amdgpu_dm_init(struct amdgpu_device
>> *adev)
>>>> /* Display Core create. */
>>>> adev->dm.dc = dc_create(&init_data);
>>>> - if (!adev->dm.dc)
>>>> + if (adev->dm.dc)
>>>> + DRM_INFO("Display Core initialized!\n");
>>>> + else
>>>> DRM_INFO("Display Core failed to initialize!\n");
>>>> INIT_WORK(&adev->dm.mst_hotplug_work,
>> hotplug_notify_work_func);
>>>> @@ -393,7 +402,7 @@ int amdgpu_dm_init(struct amdgpu_device
>> *adev)
>>>> DRM_ERROR(
>>>> "amdgpu: failed to initialize freesync_module.\n");
>>>> } else
>>>> - DRM_INFO("amdgpu: freesync_module init done %p.\n",
>>>> + DRM_DEBUG_DRIVER("amdgpu: freesync_module init done %p.\n",
>>>> adev->dm.freesync_module);
>>>> if (amdgpu_dm_initialize_drm_device(adev)) {
>>>> @@ -417,7 +426,7 @@ int amdgpu_dm_init(struct amdgpu_device
>> *adev)
>>>> goto error;
>>>> }
>>>> - DRM_INFO("KMS initialized.\n");
>>>> + DRM_DEBUG_DRIVER("KMS initialized.\n");
>>>> return 0;
>>>> error:
>>>> @@ -475,7 +484,7 @@ static int
>>>> detect_mst_link_for_all_connectors(struct drm_device *dev)
>>>> list_for_each_entry(connector, &dev->mode_config.connector_list,
>>>> head) {
>>>> aconnector = to_amdgpu_dm_connector(connector);
>>>> if (aconnector->dc_link->type == dc_connection_mst_branch) {
>>>> - DRM_INFO("DM_MST: starting TM on aconnector: %p [id: %d]\n",
>>>> + DRM_DEBUG_DRIVER("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);
>>>> @@ -819,12 +828,12 @@ void
>> amdgpu_dm_update_connector_after_detect(
>>>> if (aconnector->dc_sink == sink) {
>>>> /* We got a DP short pulse (Link Loss, DP CTS, etc...).
>>>> * Do nothing!! */
>>>> - DRM_INFO("DCHPD: connector_id=%d: dc_sink didn't change.\n",
>>>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: dc_sink didn't
>>>> change.\n",
>>>> aconnector->connector_id);
>>>> return;
>>>> }
>>>> - DRM_INFO("DCHPD: connector_id=%d: Old sink=%p New
>> sink=%p\n",
>>>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: Old sink=%p New
>>>> sink=%p\n",
>>>> aconnector->connector_id, aconnector->dc_sink, sink);
>>>> mutex_lock(&dev->mode_config.mutex);
>>>> @@ -926,7 +935,7 @@ static void dm_handle_hpd_rx_irq(struct
>>>> amdgpu_dm_connector *aconnector)
>>>> process_count++;
>>>> - DRM_DEBUG_KMS("ESI %02x %02x %02x\n", esi[0], esi[1], esi[2]);
>>>> + DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], esi[1],
>>>> esi[2]);
>>>> /* handle HPD short pulse irq */
>>>> if (aconnector->mst_mgr.mst_state)
>>>> drm_dp_mst_hpd_irq(
>>>> @@ -964,7 +973,7 @@ static void dm_handle_hpd_rx_irq(struct
>>>> amdgpu_dm_connector *aconnector)
>>>> }
>>>> if (process_count == max_process_count)
>>>> - DRM_DEBUG_KMS("Loop exceeded max iterations\n");
>>>> + DRM_DEBUG_DRIVER("Loop exceeded max iterations\n");
>>>> }
>>>> static void handle_hpd_rx_irq(void *param)
>>>> @@ -1283,7 +1292,7 @@ void
>> amdgpu_dm_register_backlight_device(struct
>>>> amdgpu_display_manager *dm)
>>>> if (NULL == dm->backlight_dev)
>>>> DRM_ERROR("DM: Backlight registration failed!\n");
>>>> else
>>>> - DRM_INFO("DM: Registered Backlight device: %s\n", bl_name);
>>>> + DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n",
>>>> bl_name);
>>>> }
>>>> #endif
>>>> @@ -2064,7 +2073,7 @@ static void update_stream_scaling_settings(
>>>> stream->src = src;
>>>> stream->dst = dst;
>>>> - DRM_DEBUG_KMS("Destination Rectangle x:%d y:%d width:%d
>>>> height:%d\n",
>>>> + DRM_DEBUG_DRIVER("Destination Rectangle x:%d y:%d width:%d
>>>> height:%d\n",
>>>> dst.x, dst.y, dst.width, dst.height);
>>>> }
>>>> @@ -2374,7 +2383,7 @@ static struct dc_stream_state
>>>> *create_stream_for_sink(
>>>> * case, we call set mode ourselves to restore the previous
>>>> mode
>>>> * and the modelist may not be filled in in time.
>>>> */
>>>> - DRM_INFO("No preferred mode found\n");
>>>> + DRM_DEBUG_DRIVER("No preferred mode found\n");
>>>> } else {
>>>> decide_crtc_timing_for_drm_display_mode(
>>>> &mode, preferred_mode,
>>>> @@ -2749,7 +2758,7 @@ static struct drm_encoder *best_encoder(struct
>>>> drm_connector *connector)
>>>> struct drm_mode_object *obj;
>>>> struct drm_encoder *encoder;
>>>> - DRM_DEBUG_KMS("Finding the best encoder\n");
>>>> + DRM_DEBUG_DRIVER("Finding the best encoder\n");
>>>> /* pick the encoder ids */
>>>> if (enc_id) {
>>>> @@ -3019,7 +3028,7 @@ static int dm_plane_helper_prepare_fb(
>>>> dm_plane_state_new = to_dm_plane_state(new_state);
>>>> if (!new_state->fb) {
>>>> - DRM_DEBUG_KMS("No FB bound\n");
>>>> + DRM_DEBUG_DRIVER("No FB bound\n");
>>>> return 0;
>>>> }
>>>> @@ -3594,7 +3603,7 @@ int amdgpu_dm_connector_init(
>>>> struct amdgpu_i2c_adapter *i2c;
>>>> ((struct dc_link *)link)->priv = aconnector;
>>>> - DRM_DEBUG_KMS("%s()\n", __func__);
>>>> + DRM_DEBUG_DRIVER("%s()\n", __func__);
>>>> i2c = create_i2c(link->ddc, link->link_index, &res);
>>>> aconnector->i2c = i2c;
>>>> @@ -3835,11 +3844,11 @@ static void handle_cursor_update(
>>>> if (!plane->state->fb && !old_plane_state->fb)
>>>> return;
>>>> - DRM_DEBUG_KMS("%s: crtc_id=%d with size %d to %d\n",
>>>> - __func__,
>>>> - amdgpu_crtc->crtc_id,
>>>> - plane->state->crtc_w,
>>>> - plane->state->crtc_h);
>>>> + DRM_DEBUG_DRIVER("%s: crtc_id=%d with size %d to %d\n",
>>>> + __func__,
>>>> + amdgpu_crtc->crtc_id,
>>>> + plane->state->crtc_w,
>>>> + plane->state->crtc_h);
>>>> ret = get_cursor_position(plane, crtc, &position);
>>>> if (ret)
>>>> @@ -4142,7 +4151,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>> new_acrtc_state = to_dm_crtc_state(new_state);
>>>> old_acrtc_state = to_dm_crtc_state(old_crtc_state);
>>>> - DRM_DEBUG_KMS(
>>>> + DRM_DEBUG_DRIVER(
>>>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>>>> active:%d, "
>>>> "planes_changed:%d, mode_changed:%d,active_changed:%d,"
>>>> "connectors_changed:%d\n",
>>>> @@ -4160,7 +4169,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>> if (modeset_required(new_state, new_acrtc_state->stream,
>>>> old_acrtc_state->stream)) {
>>>> - DRM_INFO("Atomic commit: SET crtc id %d: [%p]\n",
>>>> acrtc->crtc_id, acrtc);
>>>> + DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d: [%p]\n",
>>>> acrtc->crtc_id, acrtc);
>>>> if (!new_acrtc_state->stream) {
>>>> /*
>>>> @@ -4178,7 +4187,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>> * have a sink to keep the pipe running so that
>>>> * hw state is consistent with the sw state
>>>> */
>>>> - DRM_DEBUG_KMS("%s: Failed to create new stream for
>>>> crtc %d\n",
>>>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream for
>>>> crtc %d\n",
>>>> __func__, acrtc->base.base.id);
>>>> continue;
>>>> }
>>>> @@ -4205,7 +4214,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>> acrtc->hw_mode = crtc->state->mode;
>>>> crtc->hwmode = crtc->state->mode;
>>>> } else if (modereset_required(new_state)) {
>>>> - DRM_INFO("Atomic commit: RESET. crtc id %d:[%p]\n",
>>>> acrtc->crtc_id, acrtc);
>>>> + DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id
>>>> %d:[%p]\n", acrtc->crtc_id, acrtc);
>>>> /* i.e. reset mode */
>>>> if (old_acrtc_state->stream)
>>>> @@ -4230,7 +4239,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>> &new_crtcs[i]->base,
>>>> false);
>>>> if (!aconnector) {
>>>> - DRM_INFO("Atomic commit: Failed to find connector for
>>>> acrtc id:%d "
>>>> + DRM_DEBUG_DRIVER("Atomic commit: Failed to find
>>>> connector for acrtc id:%d "
>>>> "skipping freesync init\n",
>>>> new_crtcs[i]->crtc_id);
>>>> continue;
>>>> @@ -4539,7 +4548,7 @@ static int dm_update_crtcs_state(
>>>> */
>>>> if (!new_stream) {
>>>> - DRM_DEBUG_KMS("%s: Failed to create new stream for
>>>> crtc %d\n",
>>>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream for
>>>> crtc %d\n",
>>>> __func__, acrtc->base.base.id);
>>>> break;
>>>> }
>>>> @@ -4550,7 +4559,7 @@ static int dm_update_crtcs_state(
>>>> crtc_state->mode_changed = false;
>>>> - DRM_DEBUG_KMS("Mode change not required, setting
>>>> mode_changed to %d",
>>>> + DRM_DEBUG_DRIVER("Mode change not required, setting
>>>> mode_changed to %d",
>>>> crtc_state->mode_changed);
>>>> }
>>>> @@ -4558,7 +4567,7 @@ static int dm_update_crtcs_state(
>>>> if (!drm_atomic_crtc_needs_modeset(crtc_state))
>>>> goto next_crtc;
>>>> - DRM_DEBUG_KMS(
>>>> + DRM_DEBUG_DRIVER(
>>>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>>>> active:%d, "
>>>> "planes_changed:%d, mode_changed:%d,active_changed:%d,"
>>>> "connectors_changed:%d\n",
>>>> @@ -4576,7 +4585,7 @@ static int dm_update_crtcs_state(
>>>> if (!old_acrtc_state->stream)
>>>> goto next_crtc;
>>>> - DRM_DEBUG_KMS("Disabling DRM crtc: %d\n",
>>>> + DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n",
>>>> crtc->base.id);
>>>> /* i.e. reset mode */
>>>> @@ -4606,7 +4615,7 @@ static int dm_update_crtcs_state(
>>>> new_acrtc_state->stream = new_stream;
>>>> dc_stream_retain(new_stream);
>>>> - DRM_DEBUG_KMS("Enabling DRM crtc: %d\n",
>>>> + DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n",
>>>> crtc->base.id);
>>>> if (!dc_add_stream_to_ctx(
>>>> @@ -4681,7 +4690,7 @@ static int dm_update_planes_state(
>>>> if (!old_acrtc_state->stream)
>>>> continue;
>>>> - DRM_DEBUG_KMS("Disabling DRM plane: %d on DRM crtc %d\n",
>>>> + DRM_DEBUG_DRIVER("Disabling DRM plane: %d on DRM crtc
>> %d\n",
>>>> plane->base.id, old_plane_crtc->base.id);
>>>> if (!dc_remove_plane_from_context(
>>>> @@ -4719,7 +4728,7 @@ static int dm_update_planes_state(
>>>> new_dm_plane_state->dc_state = dc_create_plane_state(dc);
>>>> - DRM_DEBUG_KMS("Enabling DRM plane: %d on DRM crtc %d\n",
>>>> + DRM_DEBUG_DRIVER("Enabling DRM plane: %d on DRM crtc
>> %d\n",
>>>> plane->base.id, new_plane_crtc->base.id);
>>>> if (!new_dm_plane_state->dc_state) {
>>>> @@ -4874,9 +4883,9 @@ int amdgpu_dm_atomic_check(struct
>> drm_device *dev,
>>>> fail:
>>>> if (ret == -EDEADLK)
>>>> - DRM_DEBUG_KMS("Atomic check stopped due to to deadlock.\n");
>>>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to
>> deadlock.\n");
>>>> else if (ret == -EINTR || ret == -EAGAIN || ret == -ERESTARTSYS)
>>>> - DRM_DEBUG_KMS("Atomic check stopped due to to signal.\n");
>>>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to signal.\n");
>>>> else
>>>> DRM_ERROR("Atomic check failed with err: %d \n", ret);
>>>> diff --git a/drivers/gpu/drm/amd/display/include/logger_types.h
>>>> b/drivers/gpu/drm/amd/display/include/logger_types.h
>>>> index 044805ccac25..1f22e84cedb9 100644
>>>> --- a/drivers/gpu/drm/amd/display/include/logger_types.h
>>>> +++ b/drivers/gpu/drm/amd/display/include/logger_types.h
>>>> @@ -70,6 +70,9 @@ enum dc_log_type {
>>>> LOG_SECTION_TOTAL_COUNT
>>>> };
>>>> +#define DC_MIN_LOG_MASK ((1 << LOG_ERROR) | \
>>>> + (1 << LOG_DETECTION_EDID_PARSER))
>>>> +
>>>> #define DC_DEFAULT_LOG_MASK ((1 << LOG_ERROR) | \
>>>> (1 << LOG_WARNING) | \
>>>> (1 << LOG_EVENT_MODE_SET) | \
>>>
>>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
[not found] ` <3395270c-9ae1-b8ec-72c9-e24b227988f6-5C7GfCeVMHo@public.gmane.org>
@ 2017-09-15 17:26 ` Christian König
[not found] ` <f7fc3ab1-7e2d-0db5-edca-c8cee228f46d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 16+ messages in thread
From: Christian König @ 2017-09-15 17:26 UTC (permalink / raw)
To: Felix Kuehling, Christian König, Deucher, Alexander,
Wentland, Harry, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Grodzovsky, Andrey, Cheng, Tony
Am 15.09.2017 um 17:35 schrieb Felix Kuehling:
> On 2017-09-15 11:28 AM, Christian König wrote:
>> Am 15.09.2017 um 17:26 schrieb Deucher, Alexander:
>>>> -----Original Message-----
>>>> From: Wentland, Harry
>>>> Sent: Friday, September 15, 2017 11:21 AM
>>>> To: Koenig, Christian; amd-gfx@lists.freedesktop.org
>>>> Cc: Deucher, Alexander; Grodzovsky, Andrey; Cheng, Tony
>>>> Subject: Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
>>>>
>>>>
>>>>
>>>> On 2017-09-15 11:08 AM, Christian König wrote:
>>>>> Am 15.09.2017 um 16:32 schrieb Harry Wentland:
>>>>>> Log DC init but default log level to 0 (default for
>>>>>> amdgpu_dc_log) otherwise. Bug reporters can still make
>>>>>> DC more chatty like so
>>>>>> amdgpu.dc_log = 1
>>>>> Which is exactly the reason why I think this patch is superfluous.
>>>>>
>>>>> Either have a compile time option or a run time option, but please not
>>>>> both that's just confusing.
>>>>>
>>>> True. Thanks for the input.
>>>>
>>>> Gonna leave out the run time option for now.
>>> Another option would be to tie the dc debug levels to drm debug levels.
>> Which actually sounds like the correct solution to me.
>>
>> I mean we have drm debug levels for display debugging stuff for years,
>> why do we need an extra logging for DC now?
> FWIW, in KFD we rely on dynamic debug messages (CONFIG_DYNAMIC_DEBUG)
> with pr_debug. This pretty much obsoletes any driver-specific debug
> messages options. And it's quite versatile because it allows us to turn
> on and off messages, by module, source file, function, or even by line
> number through debugfs. So you can be more or less specific about which
> messages you want to see, and they're all quiet by default. See also
> Documentation/admin-guide/dynamic-debug-howto.rst.
Yeah, that is certainly something valueable as well.
But for everything mode setting related we have this standardized
DRM_DEBUG_KMS flag.
We should really use this one and only add something else after
thoughtful consideration.
I also only realized that after Alex mentioned it.
Regards,
Christian.
>
> Regards,
> Felix
>
>> Christian.
>>
>>> Alex
>>>
>>>> Harry
>>>>
>>>>> Christian.
>>>>>
>>>>>> Change-Id: Icdfb849fa678225e2460519fbd8066540feb451a
>>>>>> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
>>>>>> ---
>>>>>> drivers/gpu/drm/amd/display/Kconfig | 10 +++
>>>>>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 77
>>>>>> ++++++++++++----------
>>>>>> drivers/gpu/drm/amd/display/include/logger_types.h | 3 +
>>>>>> 3 files changed, 56 insertions(+), 34 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/display/Kconfig
>>>>>> b/drivers/gpu/drm/amd/display/Kconfig
>>>>>> index 1d1a5f807030..baab055dd362 100644
>>>>>> --- a/drivers/gpu/drm/amd/display/Kconfig
>>>>>> +++ b/drivers/gpu/drm/amd/display/Kconfig
>>>>>> @@ -31,4 +31,14 @@ config DEBUG_KERNEL_DC
>>>>>> if you want to hit
>>>>>> kdgb_break in assert.
>>>>>> +config DRM_AMD_DC_CHATTY
>>>>>> + bool "Make DC chatty again"
>>>>>> + default n
>>>>>> + depends on DRM_AMD_DC
>>>>>> + help
>>>>>> + Sometimes it's useful to have a chatty DC
>>>>>> + without a ton of spam from DRM. This allows
>>>>>> + for that and is recommended for anyone
>>>>>> + reporting bugs to DC.
>>>>>> +
>>>>>> endmenu
>>>>>> 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 abe89e3fed5b..6ecb420b2a63 100644
>>>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>>> @@ -333,7 +333,6 @@ int amdgpu_dm_init(struct amdgpu_device
>>>> *adev)
>>>>>> adev->dm.ddev = adev->ddev;
>>>>>> adev->dm.adev = adev;
>>>>>> - DRM_INFO("DAL is enabled\n");
>>>>>> /* Zero all the fields */
>>>>>> memset(&init_data, 0, sizeof(init_data));
>>>>>> @@ -373,7 +372,15 @@ int amdgpu_dm_init(struct amdgpu_device
>>>> *adev)
>>>>>> init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
>>>>>> +#ifdef CONFIG_DRM_AMD_DC_CHATTY
>>>>>> + /* always be chatty */
>>>>>> init_data.log_mask = DC_DEFAULT_LOG_MASK;
>>>>>> +#else
>>>>>> + if (amdgpu_dc_log)
>>>>>> + init_data.log_mask = DC_DEFAULT_LOG_MASK;
>>>>>> + else
>>>>>> + init_data.log_mask = DC_MIN_LOG_MASK;
>>>>>> +#endif
>>>>>> #ifdef ENABLE_FBC
>>>>>> if (adev->family == FAMILY_CZ)
>>>>>> @@ -383,7 +390,9 @@ int amdgpu_dm_init(struct amdgpu_device
>>>> *adev)
>>>>>> /* Display Core create. */
>>>>>> adev->dm.dc = dc_create(&init_data);
>>>>>> - if (!adev->dm.dc)
>>>>>> + if (adev->dm.dc)
>>>>>> + DRM_INFO("Display Core initialized!\n");
>>>>>> + else
>>>>>> DRM_INFO("Display Core failed to initialize!\n");
>>>>>> INIT_WORK(&adev->dm.mst_hotplug_work,
>>>> hotplug_notify_work_func);
>>>>>> @@ -393,7 +402,7 @@ int amdgpu_dm_init(struct amdgpu_device
>>>> *adev)
>>>>>> DRM_ERROR(
>>>>>> "amdgpu: failed to initialize freesync_module.\n");
>>>>>> } else
>>>>>> - DRM_INFO("amdgpu: freesync_module init done %p.\n",
>>>>>> + DRM_DEBUG_DRIVER("amdgpu: freesync_module init done %p.\n",
>>>>>> adev->dm.freesync_module);
>>>>>> if (amdgpu_dm_initialize_drm_device(adev)) {
>>>>>> @@ -417,7 +426,7 @@ int amdgpu_dm_init(struct amdgpu_device
>>>> *adev)
>>>>>> goto error;
>>>>>> }
>>>>>> - DRM_INFO("KMS initialized.\n");
>>>>>> + DRM_DEBUG_DRIVER("KMS initialized.\n");
>>>>>> return 0;
>>>>>> error:
>>>>>> @@ -475,7 +484,7 @@ static int
>>>>>> detect_mst_link_for_all_connectors(struct drm_device *dev)
>>>>>> list_for_each_entry(connector,
>>>>>> &dev->mode_config.connector_list,
>>>>>> head) {
>>>>>> aconnector = to_amdgpu_dm_connector(connector);
>>>>>> if (aconnector->dc_link->type ==
>>>>>> dc_connection_mst_branch) {
>>>>>> - DRM_INFO("DM_MST: starting TM on aconnector: %p [id:
>>>>>> %d]\n",
>>>>>> + DRM_DEBUG_DRIVER("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);
>>>>>> @@ -819,12 +828,12 @@ void
>>>> amdgpu_dm_update_connector_after_detect(
>>>>>> if (aconnector->dc_sink == sink) {
>>>>>> /* We got a DP short pulse (Link Loss, DP CTS, etc...).
>>>>>> * Do nothing!! */
>>>>>> - DRM_INFO("DCHPD: connector_id=%d: dc_sink didn't change.\n",
>>>>>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: dc_sink didn't
>>>>>> change.\n",
>>>>>> aconnector->connector_id);
>>>>>> return;
>>>>>> }
>>>>>> - DRM_INFO("DCHPD: connector_id=%d: Old sink=%p New
>>>> sink=%p\n",
>>>>>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: Old sink=%p New
>>>>>> sink=%p\n",
>>>>>> aconnector->connector_id, aconnector->dc_sink, sink);
>>>>>> mutex_lock(&dev->mode_config.mutex);
>>>>>> @@ -926,7 +935,7 @@ static void dm_handle_hpd_rx_irq(struct
>>>>>> amdgpu_dm_connector *aconnector)
>>>>>> process_count++;
>>>>>> - DRM_DEBUG_KMS("ESI %02x %02x %02x\n", esi[0], esi[1],
>>>>>> esi[2]);
>>>>>> + DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], esi[1],
>>>>>> esi[2]);
>>>>>> /* handle HPD short pulse irq */
>>>>>> if (aconnector->mst_mgr.mst_state)
>>>>>> drm_dp_mst_hpd_irq(
>>>>>> @@ -964,7 +973,7 @@ static void dm_handle_hpd_rx_irq(struct
>>>>>> amdgpu_dm_connector *aconnector)
>>>>>> }
>>>>>> if (process_count == max_process_count)
>>>>>> - DRM_DEBUG_KMS("Loop exceeded max iterations\n");
>>>>>> + DRM_DEBUG_DRIVER("Loop exceeded max iterations\n");
>>>>>> }
>>>>>> static void handle_hpd_rx_irq(void *param)
>>>>>> @@ -1283,7 +1292,7 @@ void
>>>> amdgpu_dm_register_backlight_device(struct
>>>>>> amdgpu_display_manager *dm)
>>>>>> if (NULL == dm->backlight_dev)
>>>>>> DRM_ERROR("DM: Backlight registration failed!\n");
>>>>>> else
>>>>>> - DRM_INFO("DM: Registered Backlight device: %s\n", bl_name);
>>>>>> + DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n",
>>>>>> bl_name);
>>>>>> }
>>>>>> #endif
>>>>>> @@ -2064,7 +2073,7 @@ static void update_stream_scaling_settings(
>>>>>> stream->src = src;
>>>>>> stream->dst = dst;
>>>>>> - DRM_DEBUG_KMS("Destination Rectangle x:%d y:%d width:%d
>>>>>> height:%d\n",
>>>>>> + DRM_DEBUG_DRIVER("Destination Rectangle x:%d y:%d width:%d
>>>>>> height:%d\n",
>>>>>> dst.x, dst.y, dst.width, dst.height);
>>>>>> }
>>>>>> @@ -2374,7 +2383,7 @@ static struct dc_stream_state
>>>>>> *create_stream_for_sink(
>>>>>> * case, we call set mode ourselves to restore the
>>>>>> previous
>>>>>> mode
>>>>>> * and the modelist may not be filled in in time.
>>>>>> */
>>>>>> - DRM_INFO("No preferred mode found\n");
>>>>>> + DRM_DEBUG_DRIVER("No preferred mode found\n");
>>>>>> } else {
>>>>>> decide_crtc_timing_for_drm_display_mode(
>>>>>> &mode, preferred_mode,
>>>>>> @@ -2749,7 +2758,7 @@ static struct drm_encoder *best_encoder(struct
>>>>>> drm_connector *connector)
>>>>>> struct drm_mode_object *obj;
>>>>>> struct drm_encoder *encoder;
>>>>>> - DRM_DEBUG_KMS("Finding the best encoder\n");
>>>>>> + DRM_DEBUG_DRIVER("Finding the best encoder\n");
>>>>>> /* pick the encoder ids */
>>>>>> if (enc_id) {
>>>>>> @@ -3019,7 +3028,7 @@ static int dm_plane_helper_prepare_fb(
>>>>>> dm_plane_state_new = to_dm_plane_state(new_state);
>>>>>> if (!new_state->fb) {
>>>>>> - DRM_DEBUG_KMS("No FB bound\n");
>>>>>> + DRM_DEBUG_DRIVER("No FB bound\n");
>>>>>> return 0;
>>>>>> }
>>>>>> @@ -3594,7 +3603,7 @@ int amdgpu_dm_connector_init(
>>>>>> struct amdgpu_i2c_adapter *i2c;
>>>>>> ((struct dc_link *)link)->priv = aconnector;
>>>>>> - DRM_DEBUG_KMS("%s()\n", __func__);
>>>>>> + DRM_DEBUG_DRIVER("%s()\n", __func__);
>>>>>> i2c = create_i2c(link->ddc, link->link_index, &res);
>>>>>> aconnector->i2c = i2c;
>>>>>> @@ -3835,11 +3844,11 @@ static void handle_cursor_update(
>>>>>> if (!plane->state->fb && !old_plane_state->fb)
>>>>>> return;
>>>>>> - DRM_DEBUG_KMS("%s: crtc_id=%d with size %d to %d\n",
>>>>>> - __func__,
>>>>>> - amdgpu_crtc->crtc_id,
>>>>>> - plane->state->crtc_w,
>>>>>> - plane->state->crtc_h);
>>>>>> + DRM_DEBUG_DRIVER("%s: crtc_id=%d with size %d to %d\n",
>>>>>> + __func__,
>>>>>> + amdgpu_crtc->crtc_id,
>>>>>> + plane->state->crtc_w,
>>>>>> + plane->state->crtc_h);
>>>>>> ret = get_cursor_position(plane, crtc, &position);
>>>>>> if (ret)
>>>>>> @@ -4142,7 +4151,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>> new_acrtc_state = to_dm_crtc_state(new_state);
>>>>>> old_acrtc_state = to_dm_crtc_state(old_crtc_state);
>>>>>> - DRM_DEBUG_KMS(
>>>>>> + DRM_DEBUG_DRIVER(
>>>>>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>>>>>> active:%d, "
>>>>>> "planes_changed:%d,
>>>>>> mode_changed:%d,active_changed:%d,"
>>>>>> "connectors_changed:%d\n",
>>>>>> @@ -4160,7 +4169,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>> if (modeset_required(new_state, new_acrtc_state->stream,
>>>>>> old_acrtc_state->stream)) {
>>>>>> - DRM_INFO("Atomic commit: SET crtc id %d: [%p]\n",
>>>>>> acrtc->crtc_id, acrtc);
>>>>>> + DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d:
>>>>>> [%p]\n",
>>>>>> acrtc->crtc_id, acrtc);
>>>>>> if (!new_acrtc_state->stream) {
>>>>>> /*
>>>>>> @@ -4178,7 +4187,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>> * have a sink to keep the pipe running so that
>>>>>> * hw state is consistent with the sw state
>>>>>> */
>>>>>> - DRM_DEBUG_KMS("%s: Failed to create new stream for
>>>>>> crtc %d\n",
>>>>>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream
>>>>>> for
>>>>>> crtc %d\n",
>>>>>> __func__, acrtc->base.base.id);
>>>>>> continue;
>>>>>> }
>>>>>> @@ -4205,7 +4214,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>> acrtc->hw_mode = crtc->state->mode;
>>>>>> crtc->hwmode = crtc->state->mode;
>>>>>> } else if (modereset_required(new_state)) {
>>>>>> - DRM_INFO("Atomic commit: RESET. crtc id %d:[%p]\n",
>>>>>> acrtc->crtc_id, acrtc);
>>>>>> + DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id
>>>>>> %d:[%p]\n", acrtc->crtc_id, acrtc);
>>>>>> /* i.e. reset mode */
>>>>>> if (old_acrtc_state->stream)
>>>>>> @@ -4230,7 +4239,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>> &new_crtcs[i]->base,
>>>>>> false);
>>>>>> if (!aconnector) {
>>>>>> - DRM_INFO("Atomic commit: Failed to find connector
>>>>>> for
>>>>>> acrtc id:%d "
>>>>>> + DRM_DEBUG_DRIVER("Atomic commit: Failed to find
>>>>>> connector for acrtc id:%d "
>>>>>> "skipping freesync init\n",
>>>>>> new_crtcs[i]->crtc_id);
>>>>>> continue;
>>>>>> @@ -4539,7 +4548,7 @@ static int dm_update_crtcs_state(
>>>>>> */
>>>>>> if (!new_stream) {
>>>>>> - DRM_DEBUG_KMS("%s: Failed to create new stream for
>>>>>> crtc %d\n",
>>>>>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream
>>>>>> for
>>>>>> crtc %d\n",
>>>>>> __func__, acrtc->base.base.id);
>>>>>> break;
>>>>>> }
>>>>>> @@ -4550,7 +4559,7 @@ static int dm_update_crtcs_state(
>>>>>> crtc_state->mode_changed = false;
>>>>>> - DRM_DEBUG_KMS("Mode change not required, setting
>>>>>> mode_changed to %d",
>>>>>> + DRM_DEBUG_DRIVER("Mode change not required, setting
>>>>>> mode_changed to %d",
>>>>>> crtc_state->mode_changed);
>>>>>> }
>>>>>> @@ -4558,7 +4567,7 @@ static int dm_update_crtcs_state(
>>>>>> if (!drm_atomic_crtc_needs_modeset(crtc_state))
>>>>>> goto next_crtc;
>>>>>> - DRM_DEBUG_KMS(
>>>>>> + DRM_DEBUG_DRIVER(
>>>>>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>>>>>> active:%d, "
>>>>>> "planes_changed:%d,
>>>>>> mode_changed:%d,active_changed:%d,"
>>>>>> "connectors_changed:%d\n",
>>>>>> @@ -4576,7 +4585,7 @@ static int dm_update_crtcs_state(
>>>>>> if (!old_acrtc_state->stream)
>>>>>> goto next_crtc;
>>>>>> - DRM_DEBUG_KMS("Disabling DRM crtc: %d\n",
>>>>>> + DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n",
>>>>>> crtc->base.id);
>>>>>> /* i.e. reset mode */
>>>>>> @@ -4606,7 +4615,7 @@ static int dm_update_crtcs_state(
>>>>>> new_acrtc_state->stream = new_stream;
>>>>>> dc_stream_retain(new_stream);
>>>>>> - DRM_DEBUG_KMS("Enabling DRM crtc: %d\n",
>>>>>> + DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n",
>>>>>> crtc->base.id);
>>>>>> if (!dc_add_stream_to_ctx(
>>>>>> @@ -4681,7 +4690,7 @@ static int dm_update_planes_state(
>>>>>> if (!old_acrtc_state->stream)
>>>>>> continue;
>>>>>> - DRM_DEBUG_KMS("Disabling DRM plane: %d on DRM crtc
>>>>>> %d\n",
>>>>>> + DRM_DEBUG_DRIVER("Disabling DRM plane: %d on DRM crtc
>>>> %d\n",
>>>>>> plane->base.id, old_plane_crtc->base.id);
>>>>>> if (!dc_remove_plane_from_context(
>>>>>> @@ -4719,7 +4728,7 @@ static int dm_update_planes_state(
>>>>>> new_dm_plane_state->dc_state =
>>>>>> dc_create_plane_state(dc);
>>>>>> - DRM_DEBUG_KMS("Enabling DRM plane: %d on DRM crtc
>>>>>> %d\n",
>>>>>> + DRM_DEBUG_DRIVER("Enabling DRM plane: %d on DRM crtc
>>>> %d\n",
>>>>>> plane->base.id, new_plane_crtc->base.id);
>>>>>> if (!new_dm_plane_state->dc_state) {
>>>>>> @@ -4874,9 +4883,9 @@ int amdgpu_dm_atomic_check(struct
>>>> drm_device *dev,
>>>>>> fail:
>>>>>> if (ret == -EDEADLK)
>>>>>> - DRM_DEBUG_KMS("Atomic check stopped due to to deadlock.\n");
>>>>>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to
>>>> deadlock.\n");
>>>>>> else if (ret == -EINTR || ret == -EAGAIN || ret ==
>>>>>> -ERESTARTSYS)
>>>>>> - DRM_DEBUG_KMS("Atomic check stopped due to to signal.\n");
>>>>>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to
>>>>>> signal.\n");
>>>>>> else
>>>>>> DRM_ERROR("Atomic check failed with err: %d \n", ret);
>>>>>> diff --git a/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>>> b/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>>> index 044805ccac25..1f22e84cedb9 100644
>>>>>> --- a/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>>> +++ b/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>>> @@ -70,6 +70,9 @@ enum dc_log_type {
>>>>>> LOG_SECTION_TOTAL_COUNT
>>>>>> };
>>>>>> +#define DC_MIN_LOG_MASK ((1 << LOG_ERROR) | \
>>>>>> + (1 << LOG_DETECTION_EDID_PARSER))
>>>>>> +
>>>>>> #define DC_DEFAULT_LOG_MASK ((1 << LOG_ERROR) | \
>>>>>> (1 << LOG_WARNING) | \
>>>>>> (1 << LOG_EVENT_MODE_SET) | \
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
[not found] ` <f7fc3ab1-7e2d-0db5-edca-c8cee228f46d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-09-15 17:33 ` Harry Wentland
[not found] ` <3a3ede59-7628-59e1-dbcf-a2cfe4904800-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 16+ messages in thread
From: Harry Wentland @ 2017-09-15 17:33 UTC (permalink / raw)
To: christian.koenig-5C7GfCeVMHo, Felix Kuehling, Deucher, Alexander,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Grodzovsky, Andrey, Cheng, Tony
On 2017-09-15 01:26 PM, Christian König wrote:
> Am 15.09.2017 um 17:35 schrieb Felix Kuehling:
>> On 2017-09-15 11:28 AM, Christian König wrote:
>>> Am 15.09.2017 um 17:26 schrieb Deucher, Alexander:
>>>>> -----Original Message-----
>>>>> From: Wentland, Harry
>>>>> Sent: Friday, September 15, 2017 11:21 AM
>>>>> To: Koenig, Christian; amd-gfx@lists.freedesktop.org
>>>>> Cc: Deucher, Alexander; Grodzovsky, Andrey; Cheng, Tony
>>>>> Subject: Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
>>>>>
>>>>>
>>>>>
>>>>> On 2017-09-15 11:08 AM, Christian König wrote:
>>>>>> Am 15.09.2017 um 16:32 schrieb Harry Wentland:
>>>>>>> Log DC init but default log level to 0 (default for
>>>>>>> amdgpu_dc_log) otherwise. Bug reporters can still make
>>>>>>> DC more chatty like so
>>>>>>> amdgpu.dc_log = 1
>>>>>> Which is exactly the reason why I think this patch is superfluous.
>>>>>>
>>>>>> Either have a compile time option or a run time option, but please
>>>>>> not
>>>>>> both that's just confusing.
>>>>>>
>>>>> True. Thanks for the input.
>>>>>
>>>>> Gonna leave out the run time option for now.
>>>> Another option would be to tie the dc debug levels to drm debug levels.
>>> Which actually sounds like the correct solution to me.
>>>
>>> I mean we have drm debug levels for display debugging stuff for years,
>>> why do we need an extra logging for DC now?
>> FWIW, in KFD we rely on dynamic debug messages (CONFIG_DYNAMIC_DEBUG)
>> with pr_debug. This pretty much obsoletes any driver-specific debug
>> messages options. And it's quite versatile because it allows us to turn
>> on and off messages, by module, source file, function, or even by line
>> number through debugfs. So you can be more or less specific about which
>> messages you want to see, and they're all quiet by default. See also
>> Documentation/admin-guide/dynamic-debug-howto.rst.
>
> Yeah, that is certainly something valueable as well.
>
> But for everything mode setting related we have this standardized
> DRM_DEBUG_KMS flag.
>
> We should really use this one and only add something else after
> thoughtful consideration.
>
> I also only realized that after Alex mentioned it.
>
Any major objection for going with existing patches for now as an
intermediate solution? I don't mind keeping only one of the
runtime/compile time options.
This already improves the log-spam. Doing things properly will take some
time to review all of our log statements, pick the right log levels, and
make sure we have something workable for internal teams that everyone
can sign off. That would take a least a week.
Harry
> Regards,
> Christian.
>
>>
>> Regards,
>> Felix
>>
>>> Christian.
>>>
>>>> Alex
>>>>
>>>>> Harry
>>>>>
>>>>>> Christian.
>>>>>>
>>>>>>> Change-Id: Icdfb849fa678225e2460519fbd8066540feb451a
>>>>>>> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
>>>>>>> ---
>>>>>>> drivers/gpu/drm/amd/display/Kconfig | 10 +++
>>>>>>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 77
>>>>>>> ++++++++++++----------
>>>>>>> drivers/gpu/drm/amd/display/include/logger_types.h | 3 +
>>>>>>> 3 files changed, 56 insertions(+), 34 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/amd/display/Kconfig
>>>>>>> b/drivers/gpu/drm/amd/display/Kconfig
>>>>>>> index 1d1a5f807030..baab055dd362 100644
>>>>>>> --- a/drivers/gpu/drm/amd/display/Kconfig
>>>>>>> +++ b/drivers/gpu/drm/amd/display/Kconfig
>>>>>>> @@ -31,4 +31,14 @@ config DEBUG_KERNEL_DC
>>>>>>> if you want to hit
>>>>>>> kdgb_break in assert.
>>>>>>> +config DRM_AMD_DC_CHATTY
>>>>>>> + bool "Make DC chatty again"
>>>>>>> + default n
>>>>>>> + depends on DRM_AMD_DC
>>>>>>> + help
>>>>>>> + Sometimes it's useful to have a chatty DC
>>>>>>> + without a ton of spam from DRM. This allows
>>>>>>> + for that and is recommended for anyone
>>>>>>> + reporting bugs to DC.
>>>>>>> +
>>>>>>> endmenu
>>>>>>> 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 abe89e3fed5b..6ecb420b2a63 100644
>>>>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>>>> @@ -333,7 +333,6 @@ int amdgpu_dm_init(struct amdgpu_device
>>>>> *adev)
>>>>>>> adev->dm.ddev = adev->ddev;
>>>>>>> adev->dm.adev = adev;
>>>>>>> - DRM_INFO("DAL is enabled\n");
>>>>>>> /* Zero all the fields */
>>>>>>> memset(&init_data, 0, sizeof(init_data));
>>>>>>> @@ -373,7 +372,15 @@ int amdgpu_dm_init(struct amdgpu_device
>>>>> *adev)
>>>>>>> init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
>>>>>>> +#ifdef CONFIG_DRM_AMD_DC_CHATTY
>>>>>>> + /* always be chatty */
>>>>>>> init_data.log_mask = DC_DEFAULT_LOG_MASK;
>>>>>>> +#else
>>>>>>> + if (amdgpu_dc_log)
>>>>>>> + init_data.log_mask = DC_DEFAULT_LOG_MASK;
>>>>>>> + else
>>>>>>> + init_data.log_mask = DC_MIN_LOG_MASK;
>>>>>>> +#endif
>>>>>>> #ifdef ENABLE_FBC
>>>>>>> if (adev->family == FAMILY_CZ)
>>>>>>> @@ -383,7 +390,9 @@ int amdgpu_dm_init(struct amdgpu_device
>>>>> *adev)
>>>>>>> /* Display Core create. */
>>>>>>> adev->dm.dc = dc_create(&init_data);
>>>>>>> - if (!adev->dm.dc)
>>>>>>> + if (adev->dm.dc)
>>>>>>> + DRM_INFO("Display Core initialized!\n");
>>>>>>> + else
>>>>>>> DRM_INFO("Display Core failed to initialize!\n");
>>>>>>> INIT_WORK(&adev->dm.mst_hotplug_work,
>>>>> hotplug_notify_work_func);
>>>>>>> @@ -393,7 +402,7 @@ int amdgpu_dm_init(struct amdgpu_device
>>>>> *adev)
>>>>>>> DRM_ERROR(
>>>>>>> "amdgpu: failed to initialize freesync_module.\n");
>>>>>>> } else
>>>>>>> - DRM_INFO("amdgpu: freesync_module init done %p.\n",
>>>>>>> + DRM_DEBUG_DRIVER("amdgpu: freesync_module init done %p.\n",
>>>>>>> adev->dm.freesync_module);
>>>>>>> if (amdgpu_dm_initialize_drm_device(adev)) {
>>>>>>> @@ -417,7 +426,7 @@ int amdgpu_dm_init(struct amdgpu_device
>>>>> *adev)
>>>>>>> goto error;
>>>>>>> }
>>>>>>> - DRM_INFO("KMS initialized.\n");
>>>>>>> + DRM_DEBUG_DRIVER("KMS initialized.\n");
>>>>>>> return 0;
>>>>>>> error:
>>>>>>> @@ -475,7 +484,7 @@ static int
>>>>>>> detect_mst_link_for_all_connectors(struct drm_device *dev)
>>>>>>> list_for_each_entry(connector,
>>>>>>> &dev->mode_config.connector_list,
>>>>>>> head) {
>>>>>>> aconnector = to_amdgpu_dm_connector(connector);
>>>>>>> if (aconnector->dc_link->type ==
>>>>>>> dc_connection_mst_branch) {
>>>>>>> - DRM_INFO("DM_MST: starting TM on aconnector: %p [id:
>>>>>>> %d]\n",
>>>>>>> + DRM_DEBUG_DRIVER("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);
>>>>>>> @@ -819,12 +828,12 @@ void
>>>>> amdgpu_dm_update_connector_after_detect(
>>>>>>> if (aconnector->dc_sink == sink) {
>>>>>>> /* We got a DP short pulse (Link Loss, DP CTS, etc...).
>>>>>>> * Do nothing!! */
>>>>>>> - DRM_INFO("DCHPD: connector_id=%d: dc_sink didn't
>>>>>>> change.\n",
>>>>>>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: dc_sink didn't
>>>>>>> change.\n",
>>>>>>> aconnector->connector_id);
>>>>>>> return;
>>>>>>> }
>>>>>>> - DRM_INFO("DCHPD: connector_id=%d: Old sink=%p New
>>>>> sink=%p\n",
>>>>>>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: Old sink=%p New
>>>>>>> sink=%p\n",
>>>>>>> aconnector->connector_id, aconnector->dc_sink, sink);
>>>>>>> mutex_lock(&dev->mode_config.mutex);
>>>>>>> @@ -926,7 +935,7 @@ static void dm_handle_hpd_rx_irq(struct
>>>>>>> amdgpu_dm_connector *aconnector)
>>>>>>> process_count++;
>>>>>>> - DRM_DEBUG_KMS("ESI %02x %02x %02x\n", esi[0], esi[1],
>>>>>>> esi[2]);
>>>>>>> + DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], esi[1],
>>>>>>> esi[2]);
>>>>>>> /* handle HPD short pulse irq */
>>>>>>> if (aconnector->mst_mgr.mst_state)
>>>>>>> drm_dp_mst_hpd_irq(
>>>>>>> @@ -964,7 +973,7 @@ static void dm_handle_hpd_rx_irq(struct
>>>>>>> amdgpu_dm_connector *aconnector)
>>>>>>> }
>>>>>>> if (process_count == max_process_count)
>>>>>>> - DRM_DEBUG_KMS("Loop exceeded max iterations\n");
>>>>>>> + DRM_DEBUG_DRIVER("Loop exceeded max iterations\n");
>>>>>>> }
>>>>>>> static void handle_hpd_rx_irq(void *param)
>>>>>>> @@ -1283,7 +1292,7 @@ void
>>>>> amdgpu_dm_register_backlight_device(struct
>>>>>>> amdgpu_display_manager *dm)
>>>>>>> if (NULL == dm->backlight_dev)
>>>>>>> DRM_ERROR("DM: Backlight registration failed!\n");
>>>>>>> else
>>>>>>> - DRM_INFO("DM: Registered Backlight device: %s\n", bl_name);
>>>>>>> + DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n",
>>>>>>> bl_name);
>>>>>>> }
>>>>>>> #endif
>>>>>>> @@ -2064,7 +2073,7 @@ static void update_stream_scaling_settings(
>>>>>>> stream->src = src;
>>>>>>> stream->dst = dst;
>>>>>>> - DRM_DEBUG_KMS("Destination Rectangle x:%d y:%d width:%d
>>>>>>> height:%d\n",
>>>>>>> + DRM_DEBUG_DRIVER("Destination Rectangle x:%d y:%d width:%d
>>>>>>> height:%d\n",
>>>>>>> dst.x, dst.y, dst.width, dst.height);
>>>>>>> }
>>>>>>> @@ -2374,7 +2383,7 @@ static struct dc_stream_state
>>>>>>> *create_stream_for_sink(
>>>>>>> * case, we call set mode ourselves to restore the
>>>>>>> previous
>>>>>>> mode
>>>>>>> * and the modelist may not be filled in in time.
>>>>>>> */
>>>>>>> - DRM_INFO("No preferred mode found\n");
>>>>>>> + DRM_DEBUG_DRIVER("No preferred mode found\n");
>>>>>>> } else {
>>>>>>> decide_crtc_timing_for_drm_display_mode(
>>>>>>> &mode, preferred_mode,
>>>>>>> @@ -2749,7 +2758,7 @@ static struct drm_encoder *best_encoder(struct
>>>>>>> drm_connector *connector)
>>>>>>> struct drm_mode_object *obj;
>>>>>>> struct drm_encoder *encoder;
>>>>>>> - DRM_DEBUG_KMS("Finding the best encoder\n");
>>>>>>> + DRM_DEBUG_DRIVER("Finding the best encoder\n");
>>>>>>> /* pick the encoder ids */
>>>>>>> if (enc_id) {
>>>>>>> @@ -3019,7 +3028,7 @@ static int dm_plane_helper_prepare_fb(
>>>>>>> dm_plane_state_new = to_dm_plane_state(new_state);
>>>>>>> if (!new_state->fb) {
>>>>>>> - DRM_DEBUG_KMS("No FB bound\n");
>>>>>>> + DRM_DEBUG_DRIVER("No FB bound\n");
>>>>>>> return 0;
>>>>>>> }
>>>>>>> @@ -3594,7 +3603,7 @@ int amdgpu_dm_connector_init(
>>>>>>> struct amdgpu_i2c_adapter *i2c;
>>>>>>> ((struct dc_link *)link)->priv = aconnector;
>>>>>>> - DRM_DEBUG_KMS("%s()\n", __func__);
>>>>>>> + DRM_DEBUG_DRIVER("%s()\n", __func__);
>>>>>>> i2c = create_i2c(link->ddc, link->link_index, &res);
>>>>>>> aconnector->i2c = i2c;
>>>>>>> @@ -3835,11 +3844,11 @@ static void handle_cursor_update(
>>>>>>> if (!plane->state->fb && !old_plane_state->fb)
>>>>>>> return;
>>>>>>> - DRM_DEBUG_KMS("%s: crtc_id=%d with size %d to %d\n",
>>>>>>> - __func__,
>>>>>>> - amdgpu_crtc->crtc_id,
>>>>>>> - plane->state->crtc_w,
>>>>>>> - plane->state->crtc_h);
>>>>>>> + DRM_DEBUG_DRIVER("%s: crtc_id=%d with size %d to %d\n",
>>>>>>> + __func__,
>>>>>>> + amdgpu_crtc->crtc_id,
>>>>>>> + plane->state->crtc_w,
>>>>>>> + plane->state->crtc_h);
>>>>>>> ret = get_cursor_position(plane, crtc, &position);
>>>>>>> if (ret)
>>>>>>> @@ -4142,7 +4151,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>>> new_acrtc_state = to_dm_crtc_state(new_state);
>>>>>>> old_acrtc_state = to_dm_crtc_state(old_crtc_state);
>>>>>>> - DRM_DEBUG_KMS(
>>>>>>> + DRM_DEBUG_DRIVER(
>>>>>>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>>>>>>> active:%d, "
>>>>>>> "planes_changed:%d,
>>>>>>> mode_changed:%d,active_changed:%d,"
>>>>>>> "connectors_changed:%d\n",
>>>>>>> @@ -4160,7 +4169,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>>> if (modeset_required(new_state,
>>>>>>> new_acrtc_state->stream,
>>>>>>> old_acrtc_state->stream)) {
>>>>>>> - DRM_INFO("Atomic commit: SET crtc id %d: [%p]\n",
>>>>>>> acrtc->crtc_id, acrtc);
>>>>>>> + DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d:
>>>>>>> [%p]\n",
>>>>>>> acrtc->crtc_id, acrtc);
>>>>>>> if (!new_acrtc_state->stream) {
>>>>>>> /*
>>>>>>> @@ -4178,7 +4187,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>>> * have a sink to keep the pipe running so that
>>>>>>> * hw state is consistent with the sw state
>>>>>>> */
>>>>>>> - DRM_DEBUG_KMS("%s: Failed to create new stream for
>>>>>>> crtc %d\n",
>>>>>>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream
>>>>>>> for
>>>>>>> crtc %d\n",
>>>>>>> __func__, acrtc->base.base.id);
>>>>>>> continue;
>>>>>>> }
>>>>>>> @@ -4205,7 +4214,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>>> acrtc->hw_mode = crtc->state->mode;
>>>>>>> crtc->hwmode = crtc->state->mode;
>>>>>>> } else if (modereset_required(new_state)) {
>>>>>>> - DRM_INFO("Atomic commit: RESET. crtc id %d:[%p]\n",
>>>>>>> acrtc->crtc_id, acrtc);
>>>>>>> + DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id
>>>>>>> %d:[%p]\n", acrtc->crtc_id, acrtc);
>>>>>>> /* i.e. reset mode */
>>>>>>> if (old_acrtc_state->stream)
>>>>>>> @@ -4230,7 +4239,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>>> &new_crtcs[i]->base,
>>>>>>> false);
>>>>>>> if (!aconnector) {
>>>>>>> - DRM_INFO("Atomic commit: Failed to find connector
>>>>>>> for
>>>>>>> acrtc id:%d "
>>>>>>> + DRM_DEBUG_DRIVER("Atomic commit: Failed to find
>>>>>>> connector for acrtc id:%d "
>>>>>>> "skipping freesync init\n",
>>>>>>> new_crtcs[i]->crtc_id);
>>>>>>> continue;
>>>>>>> @@ -4539,7 +4548,7 @@ static int dm_update_crtcs_state(
>>>>>>> */
>>>>>>> if (!new_stream) {
>>>>>>> - DRM_DEBUG_KMS("%s: Failed to create new stream for
>>>>>>> crtc %d\n",
>>>>>>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream
>>>>>>> for
>>>>>>> crtc %d\n",
>>>>>>> __func__, acrtc->base.base.id);
>>>>>>> break;
>>>>>>> }
>>>>>>> @@ -4550,7 +4559,7 @@ static int dm_update_crtcs_state(
>>>>>>> crtc_state->mode_changed = false;
>>>>>>> - DRM_DEBUG_KMS("Mode change not required,
>>>>>>> setting
>>>>>>> mode_changed to %d",
>>>>>>> + DRM_DEBUG_DRIVER("Mode change not required, setting
>>>>>>> mode_changed to %d",
>>>>>>> crtc_state->mode_changed);
>>>>>>> }
>>>>>>> @@ -4558,7 +4567,7 @@ static int dm_update_crtcs_state(
>>>>>>> if (!drm_atomic_crtc_needs_modeset(crtc_state))
>>>>>>> goto next_crtc;
>>>>>>> - DRM_DEBUG_KMS(
>>>>>>> + DRM_DEBUG_DRIVER(
>>>>>>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>>>>>>> active:%d, "
>>>>>>> "planes_changed:%d,
>>>>>>> mode_changed:%d,active_changed:%d,"
>>>>>>> "connectors_changed:%d\n",
>>>>>>> @@ -4576,7 +4585,7 @@ static int dm_update_crtcs_state(
>>>>>>> if (!old_acrtc_state->stream)
>>>>>>> goto next_crtc;
>>>>>>> - DRM_DEBUG_KMS("Disabling DRM crtc: %d\n",
>>>>>>> + DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n",
>>>>>>> crtc->base.id);
>>>>>>> /* i.e. reset mode */
>>>>>>> @@ -4606,7 +4615,7 @@ static int dm_update_crtcs_state(
>>>>>>> new_acrtc_state->stream = new_stream;
>>>>>>> dc_stream_retain(new_stream);
>>>>>>> - DRM_DEBUG_KMS("Enabling DRM crtc: %d\n",
>>>>>>> + DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n",
>>>>>>> crtc->base.id);
>>>>>>> if (!dc_add_stream_to_ctx(
>>>>>>> @@ -4681,7 +4690,7 @@ static int dm_update_planes_state(
>>>>>>> if (!old_acrtc_state->stream)
>>>>>>> continue;
>>>>>>> - DRM_DEBUG_KMS("Disabling DRM plane: %d on DRM crtc
>>>>>>> %d\n",
>>>>>>> + DRM_DEBUG_DRIVER("Disabling DRM plane: %d on DRM crtc
>>>>> %d\n",
>>>>>>> plane->base.id, old_plane_crtc->base.id);
>>>>>>> if (!dc_remove_plane_from_context(
>>>>>>> @@ -4719,7 +4728,7 @@ static int dm_update_planes_state(
>>>>>>> new_dm_plane_state->dc_state =
>>>>>>> dc_create_plane_state(dc);
>>>>>>> - DRM_DEBUG_KMS("Enabling DRM plane: %d on DRM crtc
>>>>>>> %d\n",
>>>>>>> + DRM_DEBUG_DRIVER("Enabling DRM plane: %d on DRM crtc
>>>>> %d\n",
>>>>>>> plane->base.id, new_plane_crtc->base.id);
>>>>>>> if (!new_dm_plane_state->dc_state) {
>>>>>>> @@ -4874,9 +4883,9 @@ int amdgpu_dm_atomic_check(struct
>>>>> drm_device *dev,
>>>>>>> fail:
>>>>>>> if (ret == -EDEADLK)
>>>>>>> - DRM_DEBUG_KMS("Atomic check stopped due to to
>>>>>>> deadlock.\n");
>>>>>>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to
>>>>> deadlock.\n");
>>>>>>> else if (ret == -EINTR || ret == -EAGAIN || ret ==
>>>>>>> -ERESTARTSYS)
>>>>>>> - DRM_DEBUG_KMS("Atomic check stopped due to to signal.\n");
>>>>>>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to
>>>>>>> signal.\n");
>>>>>>> else
>>>>>>> DRM_ERROR("Atomic check failed with err: %d \n", ret);
>>>>>>> diff --git a/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>>>> b/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>>>> index 044805ccac25..1f22e84cedb9 100644
>>>>>>> --- a/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>>>> +++ b/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>>>> @@ -70,6 +70,9 @@ enum dc_log_type {
>>>>>>> LOG_SECTION_TOTAL_COUNT
>>>>>>> };
>>>>>>> +#define DC_MIN_LOG_MASK ((1 << LOG_ERROR) | \
>>>>>>> + (1 << LOG_DETECTION_EDID_PARSER))
>>>>>>> +
>>>>>>> #define DC_DEFAULT_LOG_MASK ((1 << LOG_ERROR) | \
>>>>>>> (1 << LOG_WARNING) | \
>>>>>>> (1 << LOG_EVENT_MODE_SET) | \
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
[not found] ` <3a3ede59-7628-59e1-dbcf-a2cfe4904800-5C7GfCeVMHo@public.gmane.org>
@ 2017-09-15 17:36 ` Christian König
[not found] ` <82e0f415-32ea-daed-499e-3f3757206305-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 16+ messages in thread
From: Christian König @ 2017-09-15 17:36 UTC (permalink / raw)
To: Harry Wentland, Felix Kuehling, Deucher, Alexander,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Grodzovsky, Andrey, Cheng, Tony
Am 15.09.2017 um 19:33 schrieb Harry Wentland:
> On 2017-09-15 01:26 PM, Christian König wrote:
>> Am 15.09.2017 um 17:35 schrieb Felix Kuehling:
>>> On 2017-09-15 11:28 AM, Christian König wrote:
>>>> Am 15.09.2017 um 17:26 schrieb Deucher, Alexander:
>>>>>> -----Original Message-----
>>>>>> From: Wentland, Harry
>>>>>> Sent: Friday, September 15, 2017 11:21 AM
>>>>>> To: Koenig, Christian; amd-gfx@lists.freedesktop.org
>>>>>> Cc: Deucher, Alexander; Grodzovsky, Andrey; Cheng, Tony
>>>>>> Subject: Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 2017-09-15 11:08 AM, Christian König wrote:
>>>>>>> Am 15.09.2017 um 16:32 schrieb Harry Wentland:
>>>>>>>> Log DC init but default log level to 0 (default for
>>>>>>>> amdgpu_dc_log) otherwise. Bug reporters can still make
>>>>>>>> DC more chatty like so
>>>>>>>> amdgpu.dc_log = 1
>>>>>>> Which is exactly the reason why I think this patch is superfluous.
>>>>>>>
>>>>>>> Either have a compile time option or a run time option, but please
>>>>>>> not
>>>>>>> both that's just confusing.
>>>>>>>
>>>>>> True. Thanks for the input.
>>>>>>
>>>>>> Gonna leave out the run time option for now.
>>>>> Another option would be to tie the dc debug levels to drm debug levels.
>>>> Which actually sounds like the correct solution to me.
>>>>
>>>> I mean we have drm debug levels for display debugging stuff for years,
>>>> why do we need an extra logging for DC now?
>>> FWIW, in KFD we rely on dynamic debug messages (CONFIG_DYNAMIC_DEBUG)
>>> with pr_debug. This pretty much obsoletes any driver-specific debug
>>> messages options. And it's quite versatile because it allows us to turn
>>> on and off messages, by module, source file, function, or even by line
>>> number through debugfs. So you can be more or less specific about which
>>> messages you want to see, and they're all quiet by default. See also
>>> Documentation/admin-guide/dynamic-debug-howto.rst.
>> Yeah, that is certainly something valueable as well.
>>
>> But for everything mode setting related we have this standardized
>> DRM_DEBUG_KMS flag.
>>
>> We should really use this one and only add something else after
>> thoughtful consideration.
>>
>> I also only realized that after Alex mentioned it.
>>
> Any major objection for going with existing patches for now as an
> intermediate solution? I don't mind keeping only one of the
> runtime/compile time options.
For the short term that should work.
But I agree with Alex that a module parameter is better suited than a
compile time option.
And if it's just an exercise for people how to set a module option :)
Christian.
>
> This already improves the log-spam. Doing things properly will take some
> time to review all of our log statements, pick the right log levels, and
> make sure we have something workable for internal teams that everyone
> can sign off. That would take a least a week.
>
> Harry
>
>> Regards,
>> Christian.
>>
>>> Regards,
>>> Felix
>>>
>>>> Christian.
>>>>
>>>>> Alex
>>>>>
>>>>>> Harry
>>>>>>
>>>>>>> Christian.
>>>>>>>
>>>>>>>> Change-Id: Icdfb849fa678225e2460519fbd8066540feb451a
>>>>>>>> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
>>>>>>>> ---
>>>>>>>> drivers/gpu/drm/amd/display/Kconfig | 10 +++
>>>>>>>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 77
>>>>>>>> ++++++++++++----------
>>>>>>>> drivers/gpu/drm/amd/display/include/logger_types.h | 3 +
>>>>>>>> 3 files changed, 56 insertions(+), 34 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/amd/display/Kconfig
>>>>>>>> b/drivers/gpu/drm/amd/display/Kconfig
>>>>>>>> index 1d1a5f807030..baab055dd362 100644
>>>>>>>> --- a/drivers/gpu/drm/amd/display/Kconfig
>>>>>>>> +++ b/drivers/gpu/drm/amd/display/Kconfig
>>>>>>>> @@ -31,4 +31,14 @@ config DEBUG_KERNEL_DC
>>>>>>>> if you want to hit
>>>>>>>> kdgb_break in assert.
>>>>>>>> +config DRM_AMD_DC_CHATTY
>>>>>>>> + bool "Make DC chatty again"
>>>>>>>> + default n
>>>>>>>> + depends on DRM_AMD_DC
>>>>>>>> + help
>>>>>>>> + Sometimes it's useful to have a chatty DC
>>>>>>>> + without a ton of spam from DRM. This allows
>>>>>>>> + for that and is recommended for anyone
>>>>>>>> + reporting bugs to DC.
>>>>>>>> +
>>>>>>>> endmenu
>>>>>>>> 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 abe89e3fed5b..6ecb420b2a63 100644
>>>>>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>>>>> @@ -333,7 +333,6 @@ int amdgpu_dm_init(struct amdgpu_device
>>>>>> *adev)
>>>>>>>> adev->dm.ddev = adev->ddev;
>>>>>>>> adev->dm.adev = adev;
>>>>>>>> - DRM_INFO("DAL is enabled\n");
>>>>>>>> /* Zero all the fields */
>>>>>>>> memset(&init_data, 0, sizeof(init_data));
>>>>>>>> @@ -373,7 +372,15 @@ int amdgpu_dm_init(struct amdgpu_device
>>>>>> *adev)
>>>>>>>> init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
>>>>>>>> +#ifdef CONFIG_DRM_AMD_DC_CHATTY
>>>>>>>> + /* always be chatty */
>>>>>>>> init_data.log_mask = DC_DEFAULT_LOG_MASK;
>>>>>>>> +#else
>>>>>>>> + if (amdgpu_dc_log)
>>>>>>>> + init_data.log_mask = DC_DEFAULT_LOG_MASK;
>>>>>>>> + else
>>>>>>>> + init_data.log_mask = DC_MIN_LOG_MASK;
>>>>>>>> +#endif
>>>>>>>> #ifdef ENABLE_FBC
>>>>>>>> if (adev->family == FAMILY_CZ)
>>>>>>>> @@ -383,7 +390,9 @@ int amdgpu_dm_init(struct amdgpu_device
>>>>>> *adev)
>>>>>>>> /* Display Core create. */
>>>>>>>> adev->dm.dc = dc_create(&init_data);
>>>>>>>> - if (!adev->dm.dc)
>>>>>>>> + if (adev->dm.dc)
>>>>>>>> + DRM_INFO("Display Core initialized!\n");
>>>>>>>> + else
>>>>>>>> DRM_INFO("Display Core failed to initialize!\n");
>>>>>>>> INIT_WORK(&adev->dm.mst_hotplug_work,
>>>>>> hotplug_notify_work_func);
>>>>>>>> @@ -393,7 +402,7 @@ int amdgpu_dm_init(struct amdgpu_device
>>>>>> *adev)
>>>>>>>> DRM_ERROR(
>>>>>>>> "amdgpu: failed to initialize freesync_module.\n");
>>>>>>>> } else
>>>>>>>> - DRM_INFO("amdgpu: freesync_module init done %p.\n",
>>>>>>>> + DRM_DEBUG_DRIVER("amdgpu: freesync_module init done %p.\n",
>>>>>>>> adev->dm.freesync_module);
>>>>>>>> if (amdgpu_dm_initialize_drm_device(adev)) {
>>>>>>>> @@ -417,7 +426,7 @@ int amdgpu_dm_init(struct amdgpu_device
>>>>>> *adev)
>>>>>>>> goto error;
>>>>>>>> }
>>>>>>>> - DRM_INFO("KMS initialized.\n");
>>>>>>>> + DRM_DEBUG_DRIVER("KMS initialized.\n");
>>>>>>>> return 0;
>>>>>>>> error:
>>>>>>>> @@ -475,7 +484,7 @@ static int
>>>>>>>> detect_mst_link_for_all_connectors(struct drm_device *dev)
>>>>>>>> list_for_each_entry(connector,
>>>>>>>> &dev->mode_config.connector_list,
>>>>>>>> head) {
>>>>>>>> aconnector = to_amdgpu_dm_connector(connector);
>>>>>>>> if (aconnector->dc_link->type ==
>>>>>>>> dc_connection_mst_branch) {
>>>>>>>> - DRM_INFO("DM_MST: starting TM on aconnector: %p [id:
>>>>>>>> %d]\n",
>>>>>>>> + DRM_DEBUG_DRIVER("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);
>>>>>>>> @@ -819,12 +828,12 @@ void
>>>>>> amdgpu_dm_update_connector_after_detect(
>>>>>>>> if (aconnector->dc_sink == sink) {
>>>>>>>> /* We got a DP short pulse (Link Loss, DP CTS, etc...).
>>>>>>>> * Do nothing!! */
>>>>>>>> - DRM_INFO("DCHPD: connector_id=%d: dc_sink didn't
>>>>>>>> change.\n",
>>>>>>>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: dc_sink didn't
>>>>>>>> change.\n",
>>>>>>>> aconnector->connector_id);
>>>>>>>> return;
>>>>>>>> }
>>>>>>>> - DRM_INFO("DCHPD: connector_id=%d: Old sink=%p New
>>>>>> sink=%p\n",
>>>>>>>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: Old sink=%p New
>>>>>>>> sink=%p\n",
>>>>>>>> aconnector->connector_id, aconnector->dc_sink, sink);
>>>>>>>> mutex_lock(&dev->mode_config.mutex);
>>>>>>>> @@ -926,7 +935,7 @@ static void dm_handle_hpd_rx_irq(struct
>>>>>>>> amdgpu_dm_connector *aconnector)
>>>>>>>> process_count++;
>>>>>>>> - DRM_DEBUG_KMS("ESI %02x %02x %02x\n", esi[0], esi[1],
>>>>>>>> esi[2]);
>>>>>>>> + DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], esi[1],
>>>>>>>> esi[2]);
>>>>>>>> /* handle HPD short pulse irq */
>>>>>>>> if (aconnector->mst_mgr.mst_state)
>>>>>>>> drm_dp_mst_hpd_irq(
>>>>>>>> @@ -964,7 +973,7 @@ static void dm_handle_hpd_rx_irq(struct
>>>>>>>> amdgpu_dm_connector *aconnector)
>>>>>>>> }
>>>>>>>> if (process_count == max_process_count)
>>>>>>>> - DRM_DEBUG_KMS("Loop exceeded max iterations\n");
>>>>>>>> + DRM_DEBUG_DRIVER("Loop exceeded max iterations\n");
>>>>>>>> }
>>>>>>>> static void handle_hpd_rx_irq(void *param)
>>>>>>>> @@ -1283,7 +1292,7 @@ void
>>>>>> amdgpu_dm_register_backlight_device(struct
>>>>>>>> amdgpu_display_manager *dm)
>>>>>>>> if (NULL == dm->backlight_dev)
>>>>>>>> DRM_ERROR("DM: Backlight registration failed!\n");
>>>>>>>> else
>>>>>>>> - DRM_INFO("DM: Registered Backlight device: %s\n", bl_name);
>>>>>>>> + DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n",
>>>>>>>> bl_name);
>>>>>>>> }
>>>>>>>> #endif
>>>>>>>> @@ -2064,7 +2073,7 @@ static void update_stream_scaling_settings(
>>>>>>>> stream->src = src;
>>>>>>>> stream->dst = dst;
>>>>>>>> - DRM_DEBUG_KMS("Destination Rectangle x:%d y:%d width:%d
>>>>>>>> height:%d\n",
>>>>>>>> + DRM_DEBUG_DRIVER("Destination Rectangle x:%d y:%d width:%d
>>>>>>>> height:%d\n",
>>>>>>>> dst.x, dst.y, dst.width, dst.height);
>>>>>>>> }
>>>>>>>> @@ -2374,7 +2383,7 @@ static struct dc_stream_state
>>>>>>>> *create_stream_for_sink(
>>>>>>>> * case, we call set mode ourselves to restore the
>>>>>>>> previous
>>>>>>>> mode
>>>>>>>> * and the modelist may not be filled in in time.
>>>>>>>> */
>>>>>>>> - DRM_INFO("No preferred mode found\n");
>>>>>>>> + DRM_DEBUG_DRIVER("No preferred mode found\n");
>>>>>>>> } else {
>>>>>>>> decide_crtc_timing_for_drm_display_mode(
>>>>>>>> &mode, preferred_mode,
>>>>>>>> @@ -2749,7 +2758,7 @@ static struct drm_encoder *best_encoder(struct
>>>>>>>> drm_connector *connector)
>>>>>>>> struct drm_mode_object *obj;
>>>>>>>> struct drm_encoder *encoder;
>>>>>>>> - DRM_DEBUG_KMS("Finding the best encoder\n");
>>>>>>>> + DRM_DEBUG_DRIVER("Finding the best encoder\n");
>>>>>>>> /* pick the encoder ids */
>>>>>>>> if (enc_id) {
>>>>>>>> @@ -3019,7 +3028,7 @@ static int dm_plane_helper_prepare_fb(
>>>>>>>> dm_plane_state_new = to_dm_plane_state(new_state);
>>>>>>>> if (!new_state->fb) {
>>>>>>>> - DRM_DEBUG_KMS("No FB bound\n");
>>>>>>>> + DRM_DEBUG_DRIVER("No FB bound\n");
>>>>>>>> return 0;
>>>>>>>> }
>>>>>>>> @@ -3594,7 +3603,7 @@ int amdgpu_dm_connector_init(
>>>>>>>> struct amdgpu_i2c_adapter *i2c;
>>>>>>>> ((struct dc_link *)link)->priv = aconnector;
>>>>>>>> - DRM_DEBUG_KMS("%s()\n", __func__);
>>>>>>>> + DRM_DEBUG_DRIVER("%s()\n", __func__);
>>>>>>>> i2c = create_i2c(link->ddc, link->link_index, &res);
>>>>>>>> aconnector->i2c = i2c;
>>>>>>>> @@ -3835,11 +3844,11 @@ static void handle_cursor_update(
>>>>>>>> if (!plane->state->fb && !old_plane_state->fb)
>>>>>>>> return;
>>>>>>>> - DRM_DEBUG_KMS("%s: crtc_id=%d with size %d to %d\n",
>>>>>>>> - __func__,
>>>>>>>> - amdgpu_crtc->crtc_id,
>>>>>>>> - plane->state->crtc_w,
>>>>>>>> - plane->state->crtc_h);
>>>>>>>> + DRM_DEBUG_DRIVER("%s: crtc_id=%d with size %d to %d\n",
>>>>>>>> + __func__,
>>>>>>>> + amdgpu_crtc->crtc_id,
>>>>>>>> + plane->state->crtc_w,
>>>>>>>> + plane->state->crtc_h);
>>>>>>>> ret = get_cursor_position(plane, crtc, &position);
>>>>>>>> if (ret)
>>>>>>>> @@ -4142,7 +4151,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>>>> new_acrtc_state = to_dm_crtc_state(new_state);
>>>>>>>> old_acrtc_state = to_dm_crtc_state(old_crtc_state);
>>>>>>>> - DRM_DEBUG_KMS(
>>>>>>>> + DRM_DEBUG_DRIVER(
>>>>>>>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>>>>>>>> active:%d, "
>>>>>>>> "planes_changed:%d,
>>>>>>>> mode_changed:%d,active_changed:%d,"
>>>>>>>> "connectors_changed:%d\n",
>>>>>>>> @@ -4160,7 +4169,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>>>> if (modeset_required(new_state,
>>>>>>>> new_acrtc_state->stream,
>>>>>>>> old_acrtc_state->stream)) {
>>>>>>>> - DRM_INFO("Atomic commit: SET crtc id %d: [%p]\n",
>>>>>>>> acrtc->crtc_id, acrtc);
>>>>>>>> + DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d:
>>>>>>>> [%p]\n",
>>>>>>>> acrtc->crtc_id, acrtc);
>>>>>>>> if (!new_acrtc_state->stream) {
>>>>>>>> /*
>>>>>>>> @@ -4178,7 +4187,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>>>> * have a sink to keep the pipe running so that
>>>>>>>> * hw state is consistent with the sw state
>>>>>>>> */
>>>>>>>> - DRM_DEBUG_KMS("%s: Failed to create new stream for
>>>>>>>> crtc %d\n",
>>>>>>>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream
>>>>>>>> for
>>>>>>>> crtc %d\n",
>>>>>>>> __func__, acrtc->base.base.id);
>>>>>>>> continue;
>>>>>>>> }
>>>>>>>> @@ -4205,7 +4214,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>>>> acrtc->hw_mode = crtc->state->mode;
>>>>>>>> crtc->hwmode = crtc->state->mode;
>>>>>>>> } else if (modereset_required(new_state)) {
>>>>>>>> - DRM_INFO("Atomic commit: RESET. crtc id %d:[%p]\n",
>>>>>>>> acrtc->crtc_id, acrtc);
>>>>>>>> + DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id
>>>>>>>> %d:[%p]\n", acrtc->crtc_id, acrtc);
>>>>>>>> /* i.e. reset mode */
>>>>>>>> if (old_acrtc_state->stream)
>>>>>>>> @@ -4230,7 +4239,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>>>> &new_crtcs[i]->base,
>>>>>>>> false);
>>>>>>>> if (!aconnector) {
>>>>>>>> - DRM_INFO("Atomic commit: Failed to find connector
>>>>>>>> for
>>>>>>>> acrtc id:%d "
>>>>>>>> + DRM_DEBUG_DRIVER("Atomic commit: Failed to find
>>>>>>>> connector for acrtc id:%d "
>>>>>>>> "skipping freesync init\n",
>>>>>>>> new_crtcs[i]->crtc_id);
>>>>>>>> continue;
>>>>>>>> @@ -4539,7 +4548,7 @@ static int dm_update_crtcs_state(
>>>>>>>> */
>>>>>>>> if (!new_stream) {
>>>>>>>> - DRM_DEBUG_KMS("%s: Failed to create new stream for
>>>>>>>> crtc %d\n",
>>>>>>>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream
>>>>>>>> for
>>>>>>>> crtc %d\n",
>>>>>>>> __func__, acrtc->base.base.id);
>>>>>>>> break;
>>>>>>>> }
>>>>>>>> @@ -4550,7 +4559,7 @@ static int dm_update_crtcs_state(
>>>>>>>> crtc_state->mode_changed = false;
>>>>>>>> - DRM_DEBUG_KMS("Mode change not required,
>>>>>>>> setting
>>>>>>>> mode_changed to %d",
>>>>>>>> + DRM_DEBUG_DRIVER("Mode change not required, setting
>>>>>>>> mode_changed to %d",
>>>>>>>> crtc_state->mode_changed);
>>>>>>>> }
>>>>>>>> @@ -4558,7 +4567,7 @@ static int dm_update_crtcs_state(
>>>>>>>> if (!drm_atomic_crtc_needs_modeset(crtc_state))
>>>>>>>> goto next_crtc;
>>>>>>>> - DRM_DEBUG_KMS(
>>>>>>>> + DRM_DEBUG_DRIVER(
>>>>>>>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>>>>>>>> active:%d, "
>>>>>>>> "planes_changed:%d,
>>>>>>>> mode_changed:%d,active_changed:%d,"
>>>>>>>> "connectors_changed:%d\n",
>>>>>>>> @@ -4576,7 +4585,7 @@ static int dm_update_crtcs_state(
>>>>>>>> if (!old_acrtc_state->stream)
>>>>>>>> goto next_crtc;
>>>>>>>> - DRM_DEBUG_KMS("Disabling DRM crtc: %d\n",
>>>>>>>> + DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n",
>>>>>>>> crtc->base.id);
>>>>>>>> /* i.e. reset mode */
>>>>>>>> @@ -4606,7 +4615,7 @@ static int dm_update_crtcs_state(
>>>>>>>> new_acrtc_state->stream = new_stream;
>>>>>>>> dc_stream_retain(new_stream);
>>>>>>>> - DRM_DEBUG_KMS("Enabling DRM crtc: %d\n",
>>>>>>>> + DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n",
>>>>>>>> crtc->base.id);
>>>>>>>> if (!dc_add_stream_to_ctx(
>>>>>>>> @@ -4681,7 +4690,7 @@ static int dm_update_planes_state(
>>>>>>>> if (!old_acrtc_state->stream)
>>>>>>>> continue;
>>>>>>>> - DRM_DEBUG_KMS("Disabling DRM plane: %d on DRM crtc
>>>>>>>> %d\n",
>>>>>>>> + DRM_DEBUG_DRIVER("Disabling DRM plane: %d on DRM crtc
>>>>>> %d\n",
>>>>>>>> plane->base.id, old_plane_crtc->base.id);
>>>>>>>> if (!dc_remove_plane_from_context(
>>>>>>>> @@ -4719,7 +4728,7 @@ static int dm_update_planes_state(
>>>>>>>> new_dm_plane_state->dc_state =
>>>>>>>> dc_create_plane_state(dc);
>>>>>>>> - DRM_DEBUG_KMS("Enabling DRM plane: %d on DRM crtc
>>>>>>>> %d\n",
>>>>>>>> + DRM_DEBUG_DRIVER("Enabling DRM plane: %d on DRM crtc
>>>>>> %d\n",
>>>>>>>> plane->base.id, new_plane_crtc->base.id);
>>>>>>>> if (!new_dm_plane_state->dc_state) {
>>>>>>>> @@ -4874,9 +4883,9 @@ int amdgpu_dm_atomic_check(struct
>>>>>> drm_device *dev,
>>>>>>>> fail:
>>>>>>>> if (ret == -EDEADLK)
>>>>>>>> - DRM_DEBUG_KMS("Atomic check stopped due to to
>>>>>>>> deadlock.\n");
>>>>>>>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to
>>>>>> deadlock.\n");
>>>>>>>> else if (ret == -EINTR || ret == -EAGAIN || ret ==
>>>>>>>> -ERESTARTSYS)
>>>>>>>> - DRM_DEBUG_KMS("Atomic check stopped due to to signal.\n");
>>>>>>>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to
>>>>>>>> signal.\n");
>>>>>>>> else
>>>>>>>> DRM_ERROR("Atomic check failed with err: %d \n", ret);
>>>>>>>> diff --git a/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>>>>> b/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>>>>> index 044805ccac25..1f22e84cedb9 100644
>>>>>>>> --- a/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>>>>> +++ b/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>>>>> @@ -70,6 +70,9 @@ enum dc_log_type {
>>>>>>>> LOG_SECTION_TOTAL_COUNT
>>>>>>>> };
>>>>>>>> +#define DC_MIN_LOG_MASK ((1 << LOG_ERROR) | \
>>>>>>>> + (1 << LOG_DETECTION_EDID_PARSER))
>>>>>>>> +
>>>>>>>> #define DC_DEFAULT_LOG_MASK ((1 << LOG_ERROR) | \
>>>>>>>> (1 << LOG_WARNING) | \
>>>>>>>> (1 << LOG_EVENT_MODE_SET) | \
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
[not found] ` <82e0f415-32ea-daed-499e-3f3757206305-5C7GfCeVMHo@public.gmane.org>
@ 2017-09-15 17:40 ` Harry Wentland
0 siblings, 0 replies; 16+ messages in thread
From: Harry Wentland @ 2017-09-15 17:40 UTC (permalink / raw)
To: Christian König, Felix Kuehling, Deucher, Alexander,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Grodzovsky, Andrey, Cheng, Tony
On 2017-09-15 01:36 PM, Christian König wrote:
> Am 15.09.2017 um 19:33 schrieb Harry Wentland:
>> On 2017-09-15 01:26 PM, Christian König wrote:
>>> Am 15.09.2017 um 17:35 schrieb Felix Kuehling:
>>>> On 2017-09-15 11:28 AM, Christian König wrote:
>>>>> Am 15.09.2017 um 17:26 schrieb Deucher, Alexander:
>>>>>>> -----Original Message-----
>>>>>>> From: Wentland, Harry
>>>>>>> Sent: Friday, September 15, 2017 11:21 AM
>>>>>>> To: Koenig, Christian; amd-gfx@lists.freedesktop.org
>>>>>>> Cc: Deucher, Alexander; Grodzovsky, Andrey; Cheng, Tony
>>>>>>> Subject: Re: [PATCH 3/3] drm/amd/display: Reduce DC chattiness
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 2017-09-15 11:08 AM, Christian König wrote:
>>>>>>>> Am 15.09.2017 um 16:32 schrieb Harry Wentland:
>>>>>>>>> Log DC init but default log level to 0 (default for
>>>>>>>>> amdgpu_dc_log) otherwise. Bug reporters can still make
>>>>>>>>> DC more chatty like so
>>>>>>>>> amdgpu.dc_log = 1
>>>>>>>> Which is exactly the reason why I think this patch is superfluous.
>>>>>>>>
>>>>>>>> Either have a compile time option or a run time option, but please
>>>>>>>> not
>>>>>>>> both that's just confusing.
>>>>>>>>
>>>>>>> True. Thanks for the input.
>>>>>>>
>>>>>>> Gonna leave out the run time option for now.
>>>>>> Another option would be to tie the dc debug levels to drm debug
>>>>>> levels.
>>>>> Which actually sounds like the correct solution to me.
>>>>>
>>>>> I mean we have drm debug levels for display debugging stuff for years,
>>>>> why do we need an extra logging for DC now?
>>>> FWIW, in KFD we rely on dynamic debug messages (CONFIG_DYNAMIC_DEBUG)
>>>> with pr_debug. This pretty much obsoletes any driver-specific debug
>>>> messages options. And it's quite versatile because it allows us to turn
>>>> on and off messages, by module, source file, function, or even by line
>>>> number through debugfs. So you can be more or less specific about which
>>>> messages you want to see, and they're all quiet by default. See also
>>>> Documentation/admin-guide/dynamic-debug-howto.rst.
>>> Yeah, that is certainly something valueable as well.
>>>
>>> But for everything mode setting related we have this standardized
>>> DRM_DEBUG_KMS flag.
>>>
>>> We should really use this one and only add something else after
>>> thoughtful consideration.
>>>
>>> I also only realized that after Alex mentioned it.
>>>
>> Any major objection for going with existing patches for now as an
>> intermediate solution? I don't mind keeping only one of the
>> runtime/compile time options.
>
> For the short term that should work.
>
> But I agree with Alex that a module parameter is better suited than a
> compile time option.
>
> And if it's just an exercise for people how to set a module option :)
>
I agree. Will go with the module option.
Harry
> Christian.
>
>>
>> This already improves the log-spam. Doing things properly will take some
>> time to review all of our log statements, pick the right log levels, and
>> make sure we have something workable for internal teams that everyone
>> can sign off. That would take a least a week.
>>
>> Harry
>>
>>> Regards,
>>> Christian.
>>>
>>>> Regards,
>>>> Felix
>>>>
>>>>> Christian.
>>>>>
>>>>>> Alex
>>>>>>
>>>>>>> Harry
>>>>>>>
>>>>>>>> Christian.
>>>>>>>>
>>>>>>>>> Change-Id: Icdfb849fa678225e2460519fbd8066540feb451a
>>>>>>>>> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
>>>>>>>>> ---
>>>>>>>>> drivers/gpu/drm/amd/display/Kconfig | 10 +++
>>>>>>>>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 77
>>>>>>>>> ++++++++++++----------
>>>>>>>>> drivers/gpu/drm/amd/display/include/logger_types.h | 3 +
>>>>>>>>> 3 files changed, 56 insertions(+), 34 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/gpu/drm/amd/display/Kconfig
>>>>>>>>> b/drivers/gpu/drm/amd/display/Kconfig
>>>>>>>>> index 1d1a5f807030..baab055dd362 100644
>>>>>>>>> --- a/drivers/gpu/drm/amd/display/Kconfig
>>>>>>>>> +++ b/drivers/gpu/drm/amd/display/Kconfig
>>>>>>>>> @@ -31,4 +31,14 @@ config DEBUG_KERNEL_DC
>>>>>>>>> if you want to hit
>>>>>>>>> kdgb_break in assert.
>>>>>>>>> +config DRM_AMD_DC_CHATTY
>>>>>>>>> + bool "Make DC chatty again"
>>>>>>>>> + default n
>>>>>>>>> + depends on DRM_AMD_DC
>>>>>>>>> + help
>>>>>>>>> + Sometimes it's useful to have a chatty DC
>>>>>>>>> + without a ton of spam from DRM. This allows
>>>>>>>>> + for that and is recommended for anyone
>>>>>>>>> + reporting bugs to DC.
>>>>>>>>> +
>>>>>>>>> endmenu
>>>>>>>>> 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 abe89e3fed5b..6ecb420b2a63 100644
>>>>>>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>>>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>>>>>> @@ -333,7 +333,6 @@ int amdgpu_dm_init(struct amdgpu_device
>>>>>>> *adev)
>>>>>>>>> adev->dm.ddev = adev->ddev;
>>>>>>>>> adev->dm.adev = adev;
>>>>>>>>> - DRM_INFO("DAL is enabled\n");
>>>>>>>>> /* Zero all the fields */
>>>>>>>>> memset(&init_data, 0, sizeof(init_data));
>>>>>>>>> @@ -373,7 +372,15 @@ int amdgpu_dm_init(struct amdgpu_device
>>>>>>> *adev)
>>>>>>>>> init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
>>>>>>>>> +#ifdef CONFIG_DRM_AMD_DC_CHATTY
>>>>>>>>> + /* always be chatty */
>>>>>>>>> init_data.log_mask = DC_DEFAULT_LOG_MASK;
>>>>>>>>> +#else
>>>>>>>>> + if (amdgpu_dc_log)
>>>>>>>>> + init_data.log_mask = DC_DEFAULT_LOG_MASK;
>>>>>>>>> + else
>>>>>>>>> + init_data.log_mask = DC_MIN_LOG_MASK;
>>>>>>>>> +#endif
>>>>>>>>> #ifdef ENABLE_FBC
>>>>>>>>> if (adev->family == FAMILY_CZ)
>>>>>>>>> @@ -383,7 +390,9 @@ int amdgpu_dm_init(struct amdgpu_device
>>>>>>> *adev)
>>>>>>>>> /* Display Core create. */
>>>>>>>>> adev->dm.dc = dc_create(&init_data);
>>>>>>>>> - if (!adev->dm.dc)
>>>>>>>>> + if (adev->dm.dc)
>>>>>>>>> + DRM_INFO("Display Core initialized!\n");
>>>>>>>>> + else
>>>>>>>>> DRM_INFO("Display Core failed to initialize!\n");
>>>>>>>>> INIT_WORK(&adev->dm.mst_hotplug_work,
>>>>>>> hotplug_notify_work_func);
>>>>>>>>> @@ -393,7 +402,7 @@ int amdgpu_dm_init(struct amdgpu_device
>>>>>>> *adev)
>>>>>>>>> DRM_ERROR(
>>>>>>>>> "amdgpu: failed to initialize freesync_module.\n");
>>>>>>>>> } else
>>>>>>>>> - DRM_INFO("amdgpu: freesync_module init done %p.\n",
>>>>>>>>> + DRM_DEBUG_DRIVER("amdgpu: freesync_module init done
>>>>>>>>> %p.\n",
>>>>>>>>> adev->dm.freesync_module);
>>>>>>>>> if (amdgpu_dm_initialize_drm_device(adev)) {
>>>>>>>>> @@ -417,7 +426,7 @@ int amdgpu_dm_init(struct amdgpu_device
>>>>>>> *adev)
>>>>>>>>> goto error;
>>>>>>>>> }
>>>>>>>>> - DRM_INFO("KMS initialized.\n");
>>>>>>>>> + DRM_DEBUG_DRIVER("KMS initialized.\n");
>>>>>>>>> return 0;
>>>>>>>>> error:
>>>>>>>>> @@ -475,7 +484,7 @@ static int
>>>>>>>>> detect_mst_link_for_all_connectors(struct drm_device *dev)
>>>>>>>>> list_for_each_entry(connector,
>>>>>>>>> &dev->mode_config.connector_list,
>>>>>>>>> head) {
>>>>>>>>> aconnector = to_amdgpu_dm_connector(connector);
>>>>>>>>> if (aconnector->dc_link->type ==
>>>>>>>>> dc_connection_mst_branch) {
>>>>>>>>> - DRM_INFO("DM_MST: starting TM on aconnector: %p [id:
>>>>>>>>> %d]\n",
>>>>>>>>> + DRM_DEBUG_DRIVER("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);
>>>>>>>>> @@ -819,12 +828,12 @@ void
>>>>>>> amdgpu_dm_update_connector_after_detect(
>>>>>>>>> if (aconnector->dc_sink == sink) {
>>>>>>>>> /* We got a DP short pulse (Link Loss, DP CTS,
>>>>>>>>> etc...).
>>>>>>>>> * Do nothing!! */
>>>>>>>>> - DRM_INFO("DCHPD: connector_id=%d: dc_sink didn't
>>>>>>>>> change.\n",
>>>>>>>>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: dc_sink didn't
>>>>>>>>> change.\n",
>>>>>>>>> aconnector->connector_id);
>>>>>>>>> return;
>>>>>>>>> }
>>>>>>>>> - DRM_INFO("DCHPD: connector_id=%d: Old sink=%p New
>>>>>>> sink=%p\n",
>>>>>>>>> + DRM_DEBUG_DRIVER("DCHPD: connector_id=%d: Old sink=%p New
>>>>>>>>> sink=%p\n",
>>>>>>>>> aconnector->connector_id, aconnector->dc_sink, sink);
>>>>>>>>> mutex_lock(&dev->mode_config.mutex);
>>>>>>>>> @@ -926,7 +935,7 @@ static void dm_handle_hpd_rx_irq(struct
>>>>>>>>> amdgpu_dm_connector *aconnector)
>>>>>>>>> process_count++;
>>>>>>>>> - DRM_DEBUG_KMS("ESI %02x %02x %02x\n", esi[0],
>>>>>>>>> esi[1],
>>>>>>>>> esi[2]);
>>>>>>>>> + DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], esi[1],
>>>>>>>>> esi[2]);
>>>>>>>>> /* handle HPD short pulse irq */
>>>>>>>>> if (aconnector->mst_mgr.mst_state)
>>>>>>>>> drm_dp_mst_hpd_irq(
>>>>>>>>> @@ -964,7 +973,7 @@ static void dm_handle_hpd_rx_irq(struct
>>>>>>>>> amdgpu_dm_connector *aconnector)
>>>>>>>>> }
>>>>>>>>> if (process_count == max_process_count)
>>>>>>>>> - DRM_DEBUG_KMS("Loop exceeded max iterations\n");
>>>>>>>>> + DRM_DEBUG_DRIVER("Loop exceeded max iterations\n");
>>>>>>>>> }
>>>>>>>>> static void handle_hpd_rx_irq(void *param)
>>>>>>>>> @@ -1283,7 +1292,7 @@ void
>>>>>>> amdgpu_dm_register_backlight_device(struct
>>>>>>>>> amdgpu_display_manager *dm)
>>>>>>>>> if (NULL == dm->backlight_dev)
>>>>>>>>> DRM_ERROR("DM: Backlight registration failed!\n");
>>>>>>>>> else
>>>>>>>>> - DRM_INFO("DM: Registered Backlight device: %s\n",
>>>>>>>>> bl_name);
>>>>>>>>> + DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n",
>>>>>>>>> bl_name);
>>>>>>>>> }
>>>>>>>>> #endif
>>>>>>>>> @@ -2064,7 +2073,7 @@ static void update_stream_scaling_settings(
>>>>>>>>> stream->src = src;
>>>>>>>>> stream->dst = dst;
>>>>>>>>> - DRM_DEBUG_KMS("Destination Rectangle x:%d y:%d
>>>>>>>>> width:%d
>>>>>>>>> height:%d\n",
>>>>>>>>> + DRM_DEBUG_DRIVER("Destination Rectangle x:%d y:%d width:%d
>>>>>>>>> height:%d\n",
>>>>>>>>> dst.x, dst.y, dst.width, dst.height);
>>>>>>>>> }
>>>>>>>>> @@ -2374,7 +2383,7 @@ static struct dc_stream_state
>>>>>>>>> *create_stream_for_sink(
>>>>>>>>> * case, we call set mode ourselves to restore the
>>>>>>>>> previous
>>>>>>>>> mode
>>>>>>>>> * and the modelist may not be filled in in time.
>>>>>>>>> */
>>>>>>>>> - DRM_INFO("No preferred mode found\n");
>>>>>>>>> + DRM_DEBUG_DRIVER("No preferred mode found\n");
>>>>>>>>> } else {
>>>>>>>>> decide_crtc_timing_for_drm_display_mode(
>>>>>>>>> &mode, preferred_mode,
>>>>>>>>> @@ -2749,7 +2758,7 @@ static struct drm_encoder
>>>>>>>>> *best_encoder(struct
>>>>>>>>> drm_connector *connector)
>>>>>>>>> struct drm_mode_object *obj;
>>>>>>>>> struct drm_encoder *encoder;
>>>>>>>>> - DRM_DEBUG_KMS("Finding the best encoder\n");
>>>>>>>>> + DRM_DEBUG_DRIVER("Finding the best encoder\n");
>>>>>>>>> /* pick the encoder ids */
>>>>>>>>> if (enc_id) {
>>>>>>>>> @@ -3019,7 +3028,7 @@ static int dm_plane_helper_prepare_fb(
>>>>>>>>> dm_plane_state_new = to_dm_plane_state(new_state);
>>>>>>>>> if (!new_state->fb) {
>>>>>>>>> - DRM_DEBUG_KMS("No FB bound\n");
>>>>>>>>> + DRM_DEBUG_DRIVER("No FB bound\n");
>>>>>>>>> return 0;
>>>>>>>>> }
>>>>>>>>> @@ -3594,7 +3603,7 @@ int amdgpu_dm_connector_init(
>>>>>>>>> struct amdgpu_i2c_adapter *i2c;
>>>>>>>>> ((struct dc_link *)link)->priv = aconnector;
>>>>>>>>> - DRM_DEBUG_KMS("%s()\n", __func__);
>>>>>>>>> + DRM_DEBUG_DRIVER("%s()\n", __func__);
>>>>>>>>> i2c = create_i2c(link->ddc, link->link_index, &res);
>>>>>>>>> aconnector->i2c = i2c;
>>>>>>>>> @@ -3835,11 +3844,11 @@ static void handle_cursor_update(
>>>>>>>>> if (!plane->state->fb && !old_plane_state->fb)
>>>>>>>>> return;
>>>>>>>>> - DRM_DEBUG_KMS("%s: crtc_id=%d with size %d to %d\n",
>>>>>>>>> - __func__,
>>>>>>>>> - amdgpu_crtc->crtc_id,
>>>>>>>>> - plane->state->crtc_w,
>>>>>>>>> - plane->state->crtc_h);
>>>>>>>>> + DRM_DEBUG_DRIVER("%s: crtc_id=%d with size %d to %d\n",
>>>>>>>>> + __func__,
>>>>>>>>> + amdgpu_crtc->crtc_id,
>>>>>>>>> + plane->state->crtc_w,
>>>>>>>>> + plane->state->crtc_h);
>>>>>>>>> ret = get_cursor_position(plane, crtc, &position);
>>>>>>>>> if (ret)
>>>>>>>>> @@ -4142,7 +4151,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>>>>> new_acrtc_state = to_dm_crtc_state(new_state);
>>>>>>>>> old_acrtc_state = to_dm_crtc_state(old_crtc_state);
>>>>>>>>> - DRM_DEBUG_KMS(
>>>>>>>>> + DRM_DEBUG_DRIVER(
>>>>>>>>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>>>>>>>>> active:%d, "
>>>>>>>>> "planes_changed:%d,
>>>>>>>>> mode_changed:%d,active_changed:%d,"
>>>>>>>>> "connectors_changed:%d\n",
>>>>>>>>> @@ -4160,7 +4169,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>>>>> if (modeset_required(new_state,
>>>>>>>>> new_acrtc_state->stream,
>>>>>>>>> old_acrtc_state->stream)) {
>>>>>>>>> - DRM_INFO("Atomic commit: SET crtc id %d:
>>>>>>>>> [%p]\n",
>>>>>>>>> acrtc->crtc_id, acrtc);
>>>>>>>>> + DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d:
>>>>>>>>> [%p]\n",
>>>>>>>>> acrtc->crtc_id, acrtc);
>>>>>>>>> if (!new_acrtc_state->stream) {
>>>>>>>>> /*
>>>>>>>>> @@ -4178,7 +4187,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>>>>> * have a sink to keep the pipe running so
>>>>>>>>> that
>>>>>>>>> * hw state is consistent with the sw state
>>>>>>>>> */
>>>>>>>>> - DRM_DEBUG_KMS("%s: Failed to create new stream
>>>>>>>>> for
>>>>>>>>> crtc %d\n",
>>>>>>>>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream
>>>>>>>>> for
>>>>>>>>> crtc %d\n",
>>>>>>>>> __func__, acrtc->base.base.id);
>>>>>>>>> continue;
>>>>>>>>> }
>>>>>>>>> @@ -4205,7 +4214,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>>>>> acrtc->hw_mode = crtc->state->mode;
>>>>>>>>> crtc->hwmode = crtc->state->mode;
>>>>>>>>> } else if (modereset_required(new_state)) {
>>>>>>>>> - DRM_INFO("Atomic commit: RESET. crtc id %d:[%p]\n",
>>>>>>>>> acrtc->crtc_id, acrtc);
>>>>>>>>> + DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id
>>>>>>>>> %d:[%p]\n", acrtc->crtc_id, acrtc);
>>>>>>>>> /* i.e. reset mode */
>>>>>>>>> if (old_acrtc_state->stream)
>>>>>>>>> @@ -4230,7 +4239,7 @@ void amdgpu_dm_atomic_commit_tail(
>>>>>>>>> &new_crtcs[i]->base,
>>>>>>>>> false);
>>>>>>>>> if (!aconnector) {
>>>>>>>>> - DRM_INFO("Atomic commit: Failed to find connector
>>>>>>>>> for
>>>>>>>>> acrtc id:%d "
>>>>>>>>> + DRM_DEBUG_DRIVER("Atomic commit: Failed to find
>>>>>>>>> connector for acrtc id:%d "
>>>>>>>>> "skipping freesync init\n",
>>>>>>>>> new_crtcs[i]->crtc_id);
>>>>>>>>> continue;
>>>>>>>>> @@ -4539,7 +4548,7 @@ static int dm_update_crtcs_state(
>>>>>>>>> */
>>>>>>>>> if (!new_stream) {
>>>>>>>>> - DRM_DEBUG_KMS("%s: Failed to create new stream
>>>>>>>>> for
>>>>>>>>> crtc %d\n",
>>>>>>>>> + DRM_DEBUG_DRIVER("%s: Failed to create new stream
>>>>>>>>> for
>>>>>>>>> crtc %d\n",
>>>>>>>>> __func__, acrtc->base.base.id);
>>>>>>>>> break;
>>>>>>>>> }
>>>>>>>>> @@ -4550,7 +4559,7 @@ static int dm_update_crtcs_state(
>>>>>>>>> crtc_state->mode_changed = false;
>>>>>>>>> - DRM_DEBUG_KMS("Mode change not required,
>>>>>>>>> setting
>>>>>>>>> mode_changed to %d",
>>>>>>>>> + DRM_DEBUG_DRIVER("Mode change not required,
>>>>>>>>> setting
>>>>>>>>> mode_changed to %d",
>>>>>>>>> crtc_state->mode_changed);
>>>>>>>>> }
>>>>>>>>> @@ -4558,7 +4567,7 @@ static int dm_update_crtcs_state(
>>>>>>>>> if (!drm_atomic_crtc_needs_modeset(crtc_state))
>>>>>>>>> goto next_crtc;
>>>>>>>>> - DRM_DEBUG_KMS(
>>>>>>>>> + DRM_DEBUG_DRIVER(
>>>>>>>>> "amdgpu_crtc id:%d crtc_state_flags: enable:%d,
>>>>>>>>> active:%d, "
>>>>>>>>> "planes_changed:%d,
>>>>>>>>> mode_changed:%d,active_changed:%d,"
>>>>>>>>> "connectors_changed:%d\n",
>>>>>>>>> @@ -4576,7 +4585,7 @@ static int dm_update_crtcs_state(
>>>>>>>>> if (!old_acrtc_state->stream)
>>>>>>>>> goto next_crtc;
>>>>>>>>> - DRM_DEBUG_KMS("Disabling DRM crtc: %d\n",
>>>>>>>>> + DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n",
>>>>>>>>> crtc->base.id);
>>>>>>>>> /* i.e. reset mode */
>>>>>>>>> @@ -4606,7 +4615,7 @@ static int dm_update_crtcs_state(
>>>>>>>>> new_acrtc_state->stream = new_stream;
>>>>>>>>> dc_stream_retain(new_stream);
>>>>>>>>> - DRM_DEBUG_KMS("Enabling DRM crtc: %d\n",
>>>>>>>>> + DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n",
>>>>>>>>> crtc->base.id);
>>>>>>>>> if (!dc_add_stream_to_ctx(
>>>>>>>>> @@ -4681,7 +4690,7 @@ static int dm_update_planes_state(
>>>>>>>>> if (!old_acrtc_state->stream)
>>>>>>>>> continue;
>>>>>>>>> - DRM_DEBUG_KMS("Disabling DRM plane: %d on DRM
>>>>>>>>> crtc
>>>>>>>>> %d\n",
>>>>>>>>> + DRM_DEBUG_DRIVER("Disabling DRM plane: %d on DRM crtc
>>>>>>> %d\n",
>>>>>>>>> plane->base.id, old_plane_crtc->base.id);
>>>>>>>>> if (!dc_remove_plane_from_context(
>>>>>>>>> @@ -4719,7 +4728,7 @@ static int dm_update_planes_state(
>>>>>>>>> new_dm_plane_state->dc_state =
>>>>>>>>> dc_create_plane_state(dc);
>>>>>>>>> - DRM_DEBUG_KMS("Enabling DRM plane: %d on DRM
>>>>>>>>> crtc
>>>>>>>>> %d\n",
>>>>>>>>> + DRM_DEBUG_DRIVER("Enabling DRM plane: %d on DRM crtc
>>>>>>> %d\n",
>>>>>>>>> plane->base.id, new_plane_crtc->base.id);
>>>>>>>>> if (!new_dm_plane_state->dc_state) {
>>>>>>>>> @@ -4874,9 +4883,9 @@ int amdgpu_dm_atomic_check(struct
>>>>>>> drm_device *dev,
>>>>>>>>> fail:
>>>>>>>>> if (ret == -EDEADLK)
>>>>>>>>> - DRM_DEBUG_KMS("Atomic check stopped due to to
>>>>>>>>> deadlock.\n");
>>>>>>>>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to
>>>>>>> deadlock.\n");
>>>>>>>>> else if (ret == -EINTR || ret == -EAGAIN || ret ==
>>>>>>>>> -ERESTARTSYS)
>>>>>>>>> - DRM_DEBUG_KMS("Atomic check stopped due to to
>>>>>>>>> signal.\n");
>>>>>>>>> + DRM_DEBUG_DRIVER("Atomic check stopped due to to
>>>>>>>>> signal.\n");
>>>>>>>>> else
>>>>>>>>> DRM_ERROR("Atomic check failed with err: %d \n",
>>>>>>>>> ret);
>>>>>>>>> diff --git
>>>>>>>>> a/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>>>>>> b/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>>>>>> index 044805ccac25..1f22e84cedb9 100644
>>>>>>>>> --- a/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>>>>>> +++ b/drivers/gpu/drm/amd/display/include/logger_types.h
>>>>>>>>> @@ -70,6 +70,9 @@ enum dc_log_type {
>>>>>>>>> LOG_SECTION_TOTAL_COUNT
>>>>>>>>> };
>>>>>>>>> +#define DC_MIN_LOG_MASK ((1 << LOG_ERROR) | \
>>>>>>>>> + (1 << LOG_DETECTION_EDID_PARSER))
>>>>>>>>> +
>>>>>>>>> #define DC_DEFAULT_LOG_MASK ((1 << LOG_ERROR) | \
>>>>>>>>> (1 << LOG_WARNING) | \
>>>>>>>>> (1 << LOG_EVENT_MODE_SET) | \
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> amd-gfx@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-09-15 17:40 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-15 14:32 [PATCH 0/3] Make DC logging more configurable Harry Wentland
[not found] ` <20170915143226.3963-1-harry.wentland-5C7GfCeVMHo@public.gmane.org>
2017-09-15 14:32 ` [PATCH 1/3] drm/amdgpu: Add dc_log module parameter Harry Wentland
2017-09-15 14:32 ` [PATCH 2/3] drm/amd/display: Pass log_mask from DM Harry Wentland
2017-09-15 14:32 ` [PATCH 3/3] drm/amd/display: Reduce DC chattiness Harry Wentland
[not found] ` <20170915143226.3963-4-harry.wentland-5C7GfCeVMHo@public.gmane.org>
2017-09-15 15:08 ` Christian König
[not found] ` <c28eee04-9c45-39b9-7bb1-682726e7d6b2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-09-15 15:20 ` Harry Wentland
[not found] ` <e7a119fe-9c62-d03b-d115-e69fde3866b5-5C7GfCeVMHo@public.gmane.org>
2017-09-15 15:26 ` Deucher, Alexander
[not found] ` <BN6PR12MB1652A614351C9BF5F1807ED0F76C0-/b2+HYfkarQqUD6E6FAiowdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-09-15 15:28 ` Christian König
[not found] ` <15920289-e141-b80e-4399-8f62b355883c-5C7GfCeVMHo@public.gmane.org>
2017-09-15 15:31 ` Harry Wentland
2017-09-15 15:35 ` Felix Kuehling
[not found] ` <3395270c-9ae1-b8ec-72c9-e24b227988f6-5C7GfCeVMHo@public.gmane.org>
2017-09-15 17:26 ` Christian König
[not found] ` <f7fc3ab1-7e2d-0db5-edca-c8cee228f46d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-09-15 17:33 ` Harry Wentland
[not found] ` <3a3ede59-7628-59e1-dbcf-a2cfe4904800-5C7GfCeVMHo@public.gmane.org>
2017-09-15 17:36 ` Christian König
[not found] ` <82e0f415-32ea-daed-499e-3f3757206305-5C7GfCeVMHo@public.gmane.org>
2017-09-15 17:40 ` Harry Wentland
2017-09-15 15:27 ` Deucher, Alexander
[not found] ` <BN6PR12MB1652D287EE41EA30B10CCF0CF76C0-/b2+HYfkarQqUD6E6FAiowdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-09-15 15:42 ` Harry Wentland
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.