The prefered way to log connectors is [CONNECTOR:id:name]. Change it in drm core programs.
Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Claudio Suarez cssk@net-c.es --- drivers/gpu/drm/drm_client_modeset.c | 51 ++++++++++++++-------------- drivers/gpu/drm/drm_connector.c | 12 ++++--- drivers/gpu/drm/drm_edid.c | 36 ++++++++++---------- drivers/gpu/drm/drm_edid_load.c | 11 +++--- drivers/gpu/drm/drm_mode_config.c | 3 +- 5 files changed, 59 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c index ced09c7c06f9..4f35dc375bdd 100644 --- a/drivers/gpu/drm/drm_client_modeset.c +++ b/drivers/gpu/drm/drm_client_modeset.c @@ -240,7 +240,7 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors, for (i = 0; i < connector_count; i++) { connector = connectors[i]; enabled[i] = drm_connector_enabled(connector, true); - DRM_DEBUG_KMS("connector %d enabled? %s\n", connector->base.id, + DRM_DEBUG_KMS("[CONNECTOR:%d;%s] enabled? %s\n", connector->base.id, connector->name, connector->display_info.non_desktop ? "non desktop" : enabled[i] ? "yes" : "no");
any_enabled |= enabled[i]; @@ -350,8 +350,8 @@ static int drm_client_get_tile_offsets(struct drm_connector **connectors, continue;
if (!modes[i] && (h_idx || v_idx)) { - DRM_DEBUG_KMS("no modes for connector tiled %d %d\n", i, - connector->base.id); + DRM_DEBUG_KMS("no modes for [CONNECTOR:%d:%s] tiled %d\n", + connector->base.id, connector->name, i); continue; } if (connector->tile_h_loc < h_idx) @@ -419,14 +419,15 @@ static bool drm_client_target_preferred(struct drm_connector **connectors, drm_client_get_tile_offsets(connectors, connector_count, modes, offsets, i, connector->tile_h_loc, connector->tile_v_loc); } - DRM_DEBUG_KMS("looking for cmdline mode on connector %d\n", - connector->base.id); + DRM_DEBUG_KMS("looking for cmdline mode on [CONNECTOR:%d:%s]\n", + connector->base.id, connector->name);
/* got for command line mode first */ modes[i] = drm_connector_pick_cmdline_mode(connector); if (!modes[i]) { - DRM_DEBUG_KMS("looking for preferred mode on connector %d %d\n", - connector->base.id, connector->tile_group ? connector->tile_group->id : 0); + DRM_DEBUG_KMS("looking for preferred mode on [CONNECTOR:%d:%s] %d\n", + connector->base.id, connector->name, + connector->tile_group ? connector->tile_group->id : 0); modes[i] = drm_connector_has_preferred_mode(connector, width, height); } /* No preferred modes, pick one off the list */ @@ -448,8 +449,8 @@ static bool drm_client_target_preferred(struct drm_connector **connectors, (connector->tile_h_loc == 0 && connector->tile_v_loc == 0 && !drm_connector_get_tiled_mode(connector))) { - DRM_DEBUG_KMS("Falling back to non tiled mode on Connector %d\n", - connector->base.id); + DRM_DEBUG_KMS("Falling back to non tiled mode on [CONNECTOR:%d:%s]\n", + connector->base.id, connector->name); modes[i] = drm_connector_fallback_non_tiled_mode(connector); } else { modes[i] = drm_connector_get_tiled_mode(connector); @@ -617,15 +618,15 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, num_connectors_detected++;
if (!enabled[i]) { - DRM_DEBUG_KMS("connector %s not enabled, skipping\n", - connector->name); + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] not enabled, skipping\n", + connector->base.id, connector->name); conn_configured |= BIT(i); continue; }
if (connector->force == DRM_FORCE_OFF) { - DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n", - connector->name); + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] is disabled by user, skipping\n", + connector->base.id, connector->name); enabled[i] = false; continue; } @@ -635,8 +636,8 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, if (connector->force > DRM_FORCE_OFF) goto bail;
- DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n", - connector->name); + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] has no encoder or crtc, skipping\n", + connector->base.id, connector->name); enabled[i] = false; conn_configured |= BIT(i); continue; @@ -658,23 +659,23 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, } }
- DRM_DEBUG_KMS("looking for cmdline mode on connector %s\n", - connector->name); + DRM_DEBUG_KMS("looking for cmdline mode on [CONNECTOR:%d:%s]\n", + connector->base.id, connector->name);
/* go for command line mode first */ modes[i] = drm_connector_pick_cmdline_mode(connector);
/* try for preferred next */ if (!modes[i]) { - DRM_DEBUG_KMS("looking for preferred mode on connector %s %d\n", - connector->name, connector->has_tile); + DRM_DEBUG_KMS("looking for preferred mode on [CONNECTOR:%d:%s] %d\n", + connector->base.id, connector->name, connector->has_tile); modes[i] = drm_connector_has_preferred_mode(connector, width, height); }
/* No preferred mode marked by the EDID? Are there any modes? */ if (!modes[i] && !list_empty(&connector->modes)) { - DRM_DEBUG_KMS("using first mode listed on connector %s\n", - connector->name); + DRM_DEBUG_KMS("using first mode listed on [CONNECTOR:%d:%s]\n", + connector->base.id, connector->name); modes[i] = list_first_entry(&connector->modes, struct drm_display_mode, head); @@ -693,8 +694,8 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, * This is crtc->mode and not crtc->state->mode for the * fastboot check to work correctly. */ - DRM_DEBUG_KMS("looking for current mode on connector %s\n", - connector->name); + DRM_DEBUG_KMS("looking for current mode on [CONNECTOR:%d:%s]\n", + connector->base.id, connector->name); modes[i] = &connector->state->crtc->mode; } /* @@ -703,8 +704,8 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, */ if (connector->has_tile && num_tiled_conns < connector->num_h_tile * connector->num_v_tile) { - DRM_DEBUG_KMS("Falling back to non tiled mode on Connector %d\n", - connector->base.id); + DRM_DEBUG_KMS("Falling back to non tiled mode on [CONNECTOR:%d:%s]\n", + connector->base.id, connector->name); modes[i] = drm_connector_fallback_non_tiled_mode(connector); } crtcs[i] = new_crtc; diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 3bc782b630b9..a7085f65865f 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -161,20 +161,22 @@ static void drm_connector_get_cmdline_mode(struct drm_connector *connector) return;
if (mode->force) { - DRM_INFO("forcing %s connector %s\n", connector->name, + DRM_INFO("forcing [CONNECTOR:%d:%s]: %s\n", + connector->base.id, connector->name, drm_get_connector_force_name(mode->force)); connector->force = mode->force; }
if (mode->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) { - DRM_INFO("cmdline forces connector %s panel_orientation to %d\n", - connector->name, mode->panel_orientation); + DRM_INFO("cmdline forces [CONNECTOR:%d:%s] panel_orientation to %d\n", + connector->base.id, connector->name, + mode->panel_orientation); drm_connector_set_panel_orientation(connector, mode->panel_orientation); }
- DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n", - connector->name, mode->name, + DRM_DEBUG_KMS("cmdline mode for [CONNECTOR:%d:%s] %s %dx%d@%dHz%s%s%s\n", + connector->base.id, connector->name, mode->name, mode->xres, mode->yres, mode->refresh_specified ? mode->refresh : 60, mode->rb ? " reduced blanking" : "", diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 12893e7be89b..242dfcdb7ecb 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5076,32 +5076,32 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, if (hdmi[6] & DRM_EDID_HDMI_DC_30) { dc_bpc = 10; info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_30; - DRM_DEBUG("%s: HDMI sink does deep color 30.\n", - connector->name); + DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink does deep color 30.\n", + connector->base.id, connector->name); }
if (hdmi[6] & DRM_EDID_HDMI_DC_36) { dc_bpc = 12; info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_36; - DRM_DEBUG("%s: HDMI sink does deep color 36.\n", - connector->name); + DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink does deep color 36.\n", + connector->base.id, connector->name); }
if (hdmi[6] & DRM_EDID_HDMI_DC_48) { dc_bpc = 16; info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_48; - DRM_DEBUG("%s: HDMI sink does deep color 48.\n", - connector->name); + DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink does deep color 48.\n", + connector->base.id, connector->name); }
if (dc_bpc == 0) { - DRM_DEBUG("%s: No deep color support on this HDMI sink.\n", - connector->name); + DRM_DEBUG("[CONNECTOR:%d:%s]: No deep color support on this HDMI sink.\n", + connector->base.id, connector->name); return; }
- DRM_DEBUG("%s: Assigning HDMI sink color depth as %d bpc.\n", - connector->name, dc_bpc); + DRM_DEBUG("[CONNECTOR:%d:%s]: Assigning HDMI sink color depth as %d bpc.\n", + connector->base.id, connector->name, dc_bpc); info->bpc = dc_bpc;
/* @@ -5114,8 +5114,8 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, /* YCRCB444 is optional according to spec. */ if (hdmi[6] & DRM_EDID_HDMI_DC_Y444) { info->color_formats |= DRM_COLOR_FORMAT_YCRCB444; - DRM_DEBUG("%s: HDMI sink does YCRCB444 in deep color.\n", - connector->name); + DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink does YCRCB444 in deep color.\n", + connector->base.id, connector->name); }
/* @@ -5123,8 +5123,8 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, * then deep color 36 bit must be supported. */ if (!(hdmi[6] & DRM_EDID_HDMI_DC_36)) { - DRM_DEBUG("%s: HDMI sink should do DC_36, but does not!\n", - connector->name); + DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink should do DC_36, but does not!\n", + connector->base.id, connector->name); } }
@@ -5357,8 +5357,8 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi if (info->bpc == 0 && edid->revision == 3 && edid->input & DRM_EDID_DIGITAL_DFP_1_X) { info->bpc = 8; - DRM_DEBUG("%s: Assigning DFP sink color depth as %d bpc.\n", - connector->name, info->bpc); + DRM_DEBUG("[CONNECTOR:%d:%s]: Assigning DFP sink color depth as %d bpc.\n", + connector->base.id, connector->name, info->bpc); }
/* Only defined for 1.4 with digital displays */ @@ -5390,8 +5390,8 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi break; }
- DRM_DEBUG("%s: Assigning EDID-1.4 digital sink color depth as %d bpc.\n", - connector->name, info->bpc); + DRM_DEBUG("[CONNECTOR:%d:%s]: Assigning EDID-1.4 digital sink color depth as %d bpc.\n", + connector->base.id, connector->name, info->bpc);
info->color_formats |= DRM_COLOR_FORMAT_RGB444; if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB444) diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c index 37d8ba3ddb46..f3f6801927f1 100644 --- a/drivers/gpu/drm/drm_edid_load.c +++ b/drivers/gpu/drm/drm_edid_load.c @@ -188,7 +188,8 @@ static void *edid_load(struct drm_connector *connector, const char *name, pdev = platform_device_register_simple(connector_name, -1, NULL, 0); if (IS_ERR(pdev)) { DRM_ERROR("Failed to register EDID firmware platform device " - "for connector "%s"\n", connector_name); + "for [CONNECTOR:%d:%s]\n", + connector->base.id, connector_name); return ERR_CAST(pdev); }
@@ -243,8 +244,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
edid[EDID_LENGTH-1] += edid[0x7e] - valid_extensions; DRM_INFO("Found %d valid extensions instead of %d in EDID data " - ""%s" for connector "%s"\n", valid_extensions, - edid[0x7e], name, connector_name); + ""%s" for [CONNECTOR:%d:%s]\n", valid_extensions, + edid[0x7e], name, connector->base.id, connector_name); edid[0x7e] = valid_extensions;
new_edid = krealloc(edid, (valid_extensions + 1) * EDID_LENGTH, @@ -254,9 +255,9 @@ static void *edid_load(struct drm_connector *connector, const char *name, }
DRM_INFO("Got %s EDID base block and %d extension%s from " - ""%s" for connector "%s"\n", (builtin >= 0) ? "built-in" : + ""%s" for [CONNECTOR:%d:%s]\n", (builtin >= 0) ? "built-in" : "external", valid_extensions, valid_extensions == 1 ? "" : "s", - name, connector_name); + name, connector->base.id, connector_name);
out: release_firmware(fw); diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 37b4b9f0e468..e46dcd31faa3 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -504,7 +504,8 @@ void drm_mode_config_cleanup(struct drm_device *dev) if (WARN_ON(!list_empty(&dev->mode_config.connector_list))) { drm_connector_list_iter_begin(dev, &conn_iter); drm_for_each_connector_iter(connector, &conn_iter) - DRM_ERROR("connector %s leaked!\n", connector->name); + DRM_ERROR("[CONNECTOR:%d:%s] leaked!\n", + connector->base.id, connector->name); drm_connector_list_iter_end(&conn_iter); }
Hi Claudio,
On Sat, Nov 13, 2021 at 08:27:30PM +0100, Claudio Suarez wrote:
The prefered way to log connectors is [CONNECTOR:id:name]. Change it in drm core programs.
Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Claudio Suarez cssk@net-c.es
While touching all these logging calls, could you convernt them to the newer drm_dbg_kms variants? DRM_DEBUG_* are all deprecated.
Sam
On Sat, Nov 13, 2021 at 09:39:46PM +0100, Sam Ravnborg wrote:
Hi Claudio,
On Sat, Nov 13, 2021 at 08:27:30PM +0100, Claudio Suarez wrote:
The prefered way to log connectors is [CONNECTOR:id:name]. Change it in drm core programs.
Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Claudio Suarez cssk@net-c.es
While touching all these logging calls, could you convernt them to the newer drm_dbg_kms variants? DRM_DEBUG_* are all deprecated.
Yes, I can, but it is recommended to do it in a different patch:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#separ...
C&P: "Separate your changes Separate each logical change into a separate patch. For example, if your changes include both bug fixes and performance enhancements..."
I will study and send a new separate patch with your suggestion.
Best regards, Claudio Suarez
On Sun, 14 Nov 2021, Claudio Suarez cssk@net-c.es wrote:
On Sat, Nov 13, 2021 at 09:39:46PM +0100, Sam Ravnborg wrote:
Hi Claudio,
On Sat, Nov 13, 2021 at 08:27:30PM +0100, Claudio Suarez wrote:
The prefered way to log connectors is [CONNECTOR:id:name]. Change it in drm core programs.
Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Claudio Suarez cssk@net-c.es
While touching all these logging calls, could you convernt them to the newer drm_dbg_kms variants? DRM_DEBUG_* are all deprecated.
Yes, I can, but it is recommended to do it in a different patch:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#separ...
C&P: "Separate your changes Separate each logical change into a separate patch. For example, if your changes include both bug fixes and performance enhancements..."
I will study and send a new separate patch with your suggestion.
I feel these logging changes are the types of changes where I'd err on the side of fewer patches than strictly following the above guidelines.
BR, Jani.
Best regards, Claudio Suarez
On Mon, Nov 15, 2021 at 12:24:26PM +0200, Jani Nikula wrote:
On Sun, 14 Nov 2021, Claudio Suarez cssk@net-c.es wrote:
On Sat, Nov 13, 2021 at 09:39:46PM +0100, Sam Ravnborg wrote:
Hi Claudio,
On Sat, Nov 13, 2021 at 08:27:30PM +0100, Claudio Suarez wrote:
The prefered way to log connectors is [CONNECTOR:id:name]. Change it in drm core programs.
Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Claudio Suarez cssk@net-c.es
While touching all these logging calls, could you convernt them to the newer drm_dbg_kms variants? DRM_DEBUG_* are all deprecated.
Yes, I can, but it is recommended to do it in a different patch:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#separ...
C&P: "Separate your changes Separate each logical change into a separate patch. For example, if your changes include both bug fixes and performance enhancements..."
I will study and send a new separate patch with your suggestion.
I feel these logging changes are the types of changes where I'd err on the side of fewer patches than strictly following the above guidelines.
To size the problem: - there are about 3434 references to DRM_DEBUG_* in all the drm code, all drivers. - there are 413 references to DRM_DEBUG_* in the drm core code, only 66 of them are related to connectors. - there are 62 references to DRM_ERR* and 7 references to DRM_INFO in the drm core programs.
I meant I can make two patches: 1 - this one with the change to CONNECTOR:id:name (29 changes) 2 - a new and bigger patch to change 413 + 62 + 7 references to DRM_{DEBUG,ERR,INFO} in the drm core programs.
The second patch will be ready in a few days.
Is it a good plan ? or can it be improved ?
Best regards, Claudio Suarez.
On Mon, 15 Nov 2021, Claudio Suarez cssk@net-c.es wrote:
On Mon, Nov 15, 2021 at 12:24:26PM +0200, Jani Nikula wrote:
On Sun, 14 Nov 2021, Claudio Suarez cssk@net-c.es wrote:
On Sat, Nov 13, 2021 at 09:39:46PM +0100, Sam Ravnborg wrote:
Hi Claudio,
On Sat, Nov 13, 2021 at 08:27:30PM +0100, Claudio Suarez wrote:
The prefered way to log connectors is [CONNECTOR:id:name]. Change it in drm core programs.
Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Claudio Suarez cssk@net-c.es
While touching all these logging calls, could you convernt them to the newer drm_dbg_kms variants? DRM_DEBUG_* are all deprecated.
Yes, I can, but it is recommended to do it in a different patch:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#separ...
C&P: "Separate your changes Separate each logical change into a separate patch. For example, if your changes include both bug fixes and performance enhancements..."
I will study and send a new separate patch with your suggestion.
I feel these logging changes are the types of changes where I'd err on the side of fewer patches than strictly following the above guidelines.
To size the problem:
- there are about 3434 references to DRM_DEBUG_* in all the drm code, all drivers.
- there are 413 references to DRM_DEBUG_* in the drm core code, only 66 of them are related to connectors.
- there are 62 references to DRM_ERR* and 7 references to DRM_INFO in the drm core programs.
I meant I can make two patches: 1 - this one with the change to CONNECTOR:id:name (29 changes) 2 - a new and bigger patch to change 413 + 62 + 7 references to DRM_{DEBUG,ERR,INFO} in the drm core programs.
The second one is an on-going change that will have to happen gradually, file by file. Changing connector references while at it seems like a reasonable drive-by-change to me. (Others may disagree.)
The second patch will be ready in a few days.
Is it a good plan ? or can it be improved ?
It can't be a single patch. It needs to be split to smaller changes.
BR, Jani.
Best regards, Claudio Suarez.
On Mon, Nov 15, 2021 at 10:17:58PM +0200, Jani Nikula wrote:
On Mon, 15 Nov 2021, Claudio Suarez cssk@net-c.es wrote:
On Mon, Nov 15, 2021 at 12:24:26PM +0200, Jani Nikula wrote:
On Sun, 14 Nov 2021, Claudio Suarez cssk@net-c.es wrote:
On Sat, Nov 13, 2021 at 09:39:46PM +0100, Sam Ravnborg wrote:
Hi Claudio,
On Sat, Nov 13, 2021 at 08:27:30PM +0100, Claudio Suarez wrote:
The prefered way to log connectors is [CONNECTOR:id:name]. Change it in drm core programs.
Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Claudio Suarez cssk@net-c.es
While touching all these logging calls, could you convernt them to the newer drm_dbg_kms variants? DRM_DEBUG_* are all deprecated.
Yes, I can, but it is recommended to do it in a different patch:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#separ...
C&P: "Separate your changes Separate each logical change into a separate patch. For example, if your changes include both bug fixes and performance enhancements..."
I will study and send a new separate patch with your suggestion.
I feel these logging changes are the types of changes where I'd err on the side of fewer patches than strictly following the above guidelines.
To size the problem:
- there are about 3434 references to DRM_DEBUG_* in all the drm code, all drivers.
- there are 413 references to DRM_DEBUG_* in the drm core code, only 66 of them are related to connectors.
- there are 62 references to DRM_ERR* and 7 references to DRM_INFO in the drm core programs.
I meant I can make two patches: 1 - this one with the change to CONNECTOR:id:name (29 changes) 2 - a new and bigger patch to change 413 + 62 + 7 references to DRM_{DEBUG,ERR,INFO} in the drm core programs.
The second one is an on-going change that will have to happen gradually, file by file. Changing connector references while at it seems like a reasonable drive-by-change to me. (Others may disagree.)
There are about 50 files = 50 patches. It seems excessive to me, but I get the point: smaller changes, so they are easier to control/review/...
I am going so send the patch 1 and one of the patches 2 and we can see.
Thanks, Claudio Suarez
On Mon, Nov 15, 2021 at 12:40 PM Claudio Suarez cssk@net-c.es wrote:
On Mon, Nov 15, 2021 at 12:24:26PM +0200, Jani Nikula wrote:
On Sun, 14 Nov 2021, Claudio Suarez cssk@net-c.es wrote:
On Sat, Nov 13, 2021 at 09:39:46PM +0100, Sam Ravnborg wrote:
Hi Claudio,
On Sat, Nov 13, 2021 at 08:27:30PM +0100, Claudio Suarez wrote:
The prefered way to log connectors is [CONNECTOR:id:name]. Change it in drm core programs.
Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Claudio Suarez cssk@net-c.es
While touching all these logging calls, could you convernt them to the newer drm_dbg_kms variants? DRM_DEBUG_* are all deprecated.
Yes, I can, but it is recommended to do it in a different patch:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#separ...
C&P: "Separate your changes Separate each logical change into a separate patch. For example, if your changes include both bug fixes and performance enhancements..."
I will study and send a new separate patch with your suggestion.
I feel these logging changes are the types of changes where I'd err on the side of fewer patches than strictly following the above guidelines.
To size the problem:
- there are about 3434 references to DRM_DEBUG_* in all the drm code, all drivers.
- there are 413 references to DRM_DEBUG_* in the drm core code, only 66 of them are related to connectors.
- there are 62 references to DRM_ERR* and 7 references to DRM_INFO in the drm core programs.
I meant I can make two patches: 1 - this one with the change to CONNECTOR:id:name (29 changes)
Is there a "committee" that makes or coordinates these log-facing changes ?
The reason I ask is that Ive proposed that the DRM_UT_<Categories> be replaced by a set of prefix strings; "drm:core:" "drm:kms:" etc.
If the DRM_DEBUG_* etc api were converted to use pr_debug, then dyndbg could optimize away the drm_debug_enabled() overheads. dyndbg would enable those classes of drm-debug callsites using
#> echo module drm format drm:kms: > /proc/dyndbg/control
the patchset includes a macro to declare a bit-field to implement drm.debug
https://patchwork.freedesktop.org/series/96328/
how would one (hope to) coordinate changes like this ?
+CC: Ville Syrjälä ville.syrjala@linux.intel.com +CC: Daniel Vetter daniel@ffwll.ch
On Sat, Nov 13, 2021 at 08:27:30PM +0100, Claudio Suarez wrote:
The prefered way to log connectors is [CONNECTOR:id:name]. Change it in drm core programs.
Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Claudio Suarez cssk@net-c.es
drivers/gpu/drm/drm_client_modeset.c | 51 ++++++++++++++-------------- drivers/gpu/drm/drm_connector.c | 12 ++++--- drivers/gpu/drm/drm_edid.c | 36 ++++++++++---------- drivers/gpu/drm/drm_edid_load.c | 11 +++--- drivers/gpu/drm/drm_mode_config.c | 3 +- 5 files changed, 59 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c index ced09c7c06f9..4f35dc375bdd 100644 --- a/drivers/gpu/drm/drm_client_modeset.c +++ b/drivers/gpu/drm/drm_client_modeset.c @@ -240,7 +240,7 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors, for (i = 0; i < connector_count; i++) { connector = connectors[i]; enabled[i] = drm_connector_enabled(connector, true);
DRM_DEBUG_KMS("connector %d enabled? %s\n", connector->base.id,
DRM_DEBUG_KMS("[CONNECTOR:%d;%s] enabled? %s\n", connector->base.id, connector->name, connector->display_info.non_desktop ? "non desktop" : enabled[i] ? "yes" : "no");any_enabled |= enabled[i];
@@ -350,8 +350,8 @@ static int drm_client_get_tile_offsets(struct drm_connector **connectors, continue;
if (!modes[i] && (h_idx || v_idx)) {
DRM_DEBUG_KMS("no modes for connector tiled %d %d\n", i,connector->base.id);
DRM_DEBUG_KMS("no modes for [CONNECTOR:%d:%s] tiled %d\n",connector->base.id, connector->name, i); continue;@@ -419,14 +419,15 @@ static bool drm_client_target_preferred(struct drm_connector **connectors, drm_client_get_tile_offsets(connectors, connector_count, modes, offsets, i, connector->tile_h_loc, connector->tile_v_loc); }
DRM_DEBUG_KMS("looking for cmdline mode on connector %d\n",connector->base.id);
DRM_DEBUG_KMS("looking for cmdline mode on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name);/* got for command line mode first */ modes[i] = drm_connector_pick_cmdline_mode(connector); if (!modes[i]) {
DRM_DEBUG_KMS("looking for preferred mode on connector %d %d\n",connector->base.id, connector->tile_group ? connector->tile_group->id : 0);
DRM_DEBUG_KMS("looking for preferred mode on [CONNECTOR:%d:%s] %d\n",connector->base.id, connector->name,connector->tile_group ? connector->tile_group->id : 0); modes[i] = drm_connector_has_preferred_mode(connector, width, height);@@ -448,8 +449,8 @@ static bool drm_client_target_preferred(struct drm_connector **connectors, (connector->tile_h_loc == 0 && connector->tile_v_loc == 0 && !drm_connector_get_tiled_mode(connector))) {
DRM_DEBUG_KMS("Falling back to non tiled mode on Connector %d\n",connector->base.id);
DRM_DEBUG_KMS("Falling back to non tiled mode on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name); modes[i] = drm_connector_fallback_non_tiled_mode(connector); } else { modes[i] = drm_connector_get_tiled_mode(connector);@@ -617,15 +618,15 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, num_connectors_detected++;
if (!enabled[i]) {
DRM_DEBUG_KMS("connector %s not enabled, skipping\n",connector->name);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] not enabled, skipping\n",connector->base.id, connector->name); conn_configured |= BIT(i); continue;}
if (connector->force == DRM_FORCE_OFF) {
DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n",connector->name);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] is disabled by user, skipping\n",connector->base.id, connector->name); enabled[i] = false; continue;@@ -635,8 +636,8 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, if (connector->force > DRM_FORCE_OFF) goto bail;
DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n",connector->name);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] has no encoder or crtc, skipping\n",connector->base.id, connector->name); enabled[i] = false; conn_configured |= BIT(i); continue;@@ -658,23 +659,23 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, } }
DRM_DEBUG_KMS("looking for cmdline mode on connector %s\n",connector->name);
DRM_DEBUG_KMS("looking for cmdline mode on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name);/* go for command line mode first */ modes[i] = drm_connector_pick_cmdline_mode(connector);
/* try for preferred next */ if (!modes[i]) {
DRM_DEBUG_KMS("looking for preferred mode on connector %s %d\n",connector->name, connector->has_tile);
DRM_DEBUG_KMS("looking for preferred mode on [CONNECTOR:%d:%s] %d\n",connector->base.id, connector->name, connector->has_tile); modes[i] = drm_connector_has_preferred_mode(connector, width, height);}
/* No preferred mode marked by the EDID? Are there any modes? */ if (!modes[i] && !list_empty(&connector->modes)) {
DRM_DEBUG_KMS("using first mode listed on connector %s\n",connector->name);
DRM_DEBUG_KMS("using first mode listed on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name); modes[i] = list_first_entry(&connector->modes, struct drm_display_mode, head);@@ -693,8 +694,8 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, * This is crtc->mode and not crtc->state->mode for the * fastboot check to work correctly. */
DRM_DEBUG_KMS("looking for current mode on connector %s\n",connector->name);
DRM_DEBUG_KMS("looking for current mode on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name); modes[i] = &connector->state->crtc->mode;@@ -703,8 +704,8 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, */ if (connector->has_tile && num_tiled_conns < connector->num_h_tile * connector->num_v_tile) {
DRM_DEBUG_KMS("Falling back to non tiled mode on Connector %d\n",connector->base.id);
DRM_DEBUG_KMS("Falling back to non tiled mode on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name); modes[i] = drm_connector_fallback_non_tiled_mode(connector);diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 3bc782b630b9..a7085f65865f 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -161,20 +161,22 @@ static void drm_connector_get_cmdline_mode(struct drm_connector *connector) return;
if (mode->force) {
DRM_INFO("forcing %s connector %s\n", connector->name,
DRM_INFO("forcing [CONNECTOR:%d:%s]: %s\n",connector->base.id, connector->name, drm_get_connector_force_name(mode->force));connector->force = mode->force; }
if (mode->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) {
DRM_INFO("cmdline forces connector %s panel_orientation to %d\n",connector->name, mode->panel_orientation);
DRM_INFO("cmdline forces [CONNECTOR:%d:%s] panel_orientation to %d\n",connector->base.id, connector->name,mode->panel_orientation);
- DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
connector->name, mode->name,
- DRM_DEBUG_KMS("cmdline mode for [CONNECTOR:%d:%s] %s %dx%d@%dHz%s%s%s\n",
connector->base.id, connector->name, mode->name, mode->xres, mode->yres, mode->refresh_specified ? mode->refresh : 60, mode->rb ? " reduced blanking" : "",diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 12893e7be89b..242dfcdb7ecb 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5076,32 +5076,32 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, if (hdmi[6] & DRM_EDID_HDMI_DC_30) { dc_bpc = 10; info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_30;
DRM_DEBUG("%s: HDMI sink does deep color 30.\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink does deep color 30.\n",connector->base.id, connector->name);}
if (hdmi[6] & DRM_EDID_HDMI_DC_36) { dc_bpc = 12; info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_36;
DRM_DEBUG("%s: HDMI sink does deep color 36.\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink does deep color 36.\n",connector->base.id, connector->name);}
if (hdmi[6] & DRM_EDID_HDMI_DC_48) { dc_bpc = 16; info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_48;
DRM_DEBUG("%s: HDMI sink does deep color 48.\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink does deep color 48.\n",connector->base.id, connector->name);}
if (dc_bpc == 0) {
DRM_DEBUG("%s: No deep color support on this HDMI sink.\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: No deep color support on this HDMI sink.\n",connector->base.id, connector->name);
- DRM_DEBUG("%s: Assigning HDMI sink color depth as %d bpc.\n",
connector->name, dc_bpc);
DRM_DEBUG("[CONNECTOR:%d:%s]: Assigning HDMI sink color depth as %d bpc.\n",
connector->base.id, connector->name, dc_bpc);info->bpc = dc_bpc;
/*
@@ -5114,8 +5114,8 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, /* YCRCB444 is optional according to spec. */ if (hdmi[6] & DRM_EDID_HDMI_DC_Y444) { info->color_formats |= DRM_COLOR_FORMAT_YCRCB444;
DRM_DEBUG("%s: HDMI sink does YCRCB444 in deep color.\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink does YCRCB444 in deep color.\n",connector->base.id, connector->name);}
/*
@@ -5123,8 +5123,8 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, * then deep color 36 bit must be supported. */ if (!(hdmi[6] & DRM_EDID_HDMI_DC_36)) {
DRM_DEBUG("%s: HDMI sink should do DC_36, but does not!\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink should do DC_36, but does not!\n",connector->base.id, connector->name);}
@@ -5357,8 +5357,8 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi if (info->bpc == 0 && edid->revision == 3 && edid->input & DRM_EDID_DIGITAL_DFP_1_X) { info->bpc = 8;
DRM_DEBUG("%s: Assigning DFP sink color depth as %d bpc.\n",connector->name, info->bpc);
DRM_DEBUG("[CONNECTOR:%d:%s]: Assigning DFP sink color depth as %d bpc.\n",connector->base.id, connector->name, info->bpc);}
/* Only defined for 1.4 with digital displays */
@@ -5390,8 +5390,8 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi break; }
- DRM_DEBUG("%s: Assigning EDID-1.4 digital sink color depth as %d bpc.\n",
connector->name, info->bpc);
DRM_DEBUG("[CONNECTOR:%d:%s]: Assigning EDID-1.4 digital sink color depth as %d bpc.\n",
connector->base.id, connector->name, info->bpc);info->color_formats |= DRM_COLOR_FORMAT_RGB444; if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB444)
diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c index 37d8ba3ddb46..f3f6801927f1 100644 --- a/drivers/gpu/drm/drm_edid_load.c +++ b/drivers/gpu/drm/drm_edid_load.c @@ -188,7 +188,8 @@ static void *edid_load(struct drm_connector *connector, const char *name, pdev = platform_device_register_simple(connector_name, -1, NULL, 0); if (IS_ERR(pdev)) { DRM_ERROR("Failed to register EDID firmware platform device "
"for connector \"%s\"\n", connector_name);
"for [CONNECTOR:%d:%s]\n",connector->base.id, connector_name); return ERR_CAST(pdev);@@ -243,8 +244,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
edid[EDID_LENGTH-1] += edid[0x7e] - valid_extensions; DRM_INFO("Found %d valid extensions instead of %d in EDID data "
"\"%s\" for connector \"%s\"\n", valid_extensions,edid[0x7e], name, connector_name);
"\"%s\" for [CONNECTOR:%d:%s]\n", valid_extensions,edid[0x7e], name, connector->base.id, connector_name);edid[0x7e] = valid_extensions;
new_edid = krealloc(edid, (valid_extensions + 1) * EDID_LENGTH,
@@ -254,9 +255,9 @@ static void *edid_load(struct drm_connector *connector, const char *name, }
DRM_INFO("Got %s EDID base block and %d extension%s from "
"\"%s\" for connector \"%s\"\n", (builtin >= 0) ? "built-in" :
"\"%s\" for [CONNECTOR:%d:%s]\n", (builtin >= 0) ? "built-in" : "external", valid_extensions, valid_extensions == 1 ? "" : "s",
name, connector_name);
name, connector->base.id, connector_name);out: release_firmware(fw); diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 37b4b9f0e468..e46dcd31faa3 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -504,7 +504,8 @@ void drm_mode_config_cleanup(struct drm_device *dev) if (WARN_ON(!list_empty(&dev->mode_config.connector_list))) { drm_connector_list_iter_begin(dev, &conn_iter); drm_for_each_connector_iter(connector, &conn_iter)
DRM_ERROR("connector %s leaked!\n", connector->name);
DRM_ERROR("[CONNECTOR:%d:%s] leaked!\n",connector->base.id, connector->name);-- 2.33.0
On Sat, Nov 13, 2021 at 08:27:30PM +0100, Claudio Suarez wrote:
The prefered way to log connectors is [CONNECTOR:id:name]. Change it in drm core programs.
Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Claudio Suarez cssk@net-c.es
drivers/gpu/drm/drm_client_modeset.c | 51 ++++++++++++++-------------- drivers/gpu/drm/drm_connector.c | 12 ++++--- drivers/gpu/drm/drm_edid.c | 36 ++++++++++---------- drivers/gpu/drm/drm_edid_load.c | 11 +++--- drivers/gpu/drm/drm_mode_config.c | 3 +- 5 files changed, 59 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c index ced09c7c06f9..4f35dc375bdd 100644 --- a/drivers/gpu/drm/drm_client_modeset.c +++ b/drivers/gpu/drm/drm_client_modeset.c @@ -240,7 +240,7 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors, for (i = 0; i < connector_count; i++) { connector = connectors[i]; enabled[i] = drm_connector_enabled(connector, true);
DRM_DEBUG_KMS("connector %d enabled? %s\n", connector->base.id,
DRM_DEBUG_KMS("[CONNECTOR:%d;%s] enabled? %s\n", connector->base.id, connector->name, connector->display_info.non_desktop ? "non desktop" : enabled[i] ? "yes" : "no");any_enabled |= enabled[i];
@@ -350,8 +350,8 @@ static int drm_client_get_tile_offsets(struct drm_connector **connectors, continue;
if (!modes[i] && (h_idx || v_idx)) {
DRM_DEBUG_KMS("no modes for connector tiled %d %d\n", i,connector->base.id);
DRM_DEBUG_KMS("no modes for [CONNECTOR:%d:%s] tiled %d\n",connector->base.id, connector->name, i); continue;@@ -419,14 +419,15 @@ static bool drm_client_target_preferred(struct drm_connector **connectors, drm_client_get_tile_offsets(connectors, connector_count, modes, offsets, i, connector->tile_h_loc, connector->tile_v_loc); }
DRM_DEBUG_KMS("looking for cmdline mode on connector %d\n",connector->base.id);
DRM_DEBUG_KMS("looking for cmdline mode on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name);/* got for command line mode first */ modes[i] = drm_connector_pick_cmdline_mode(connector); if (!modes[i]) {
DRM_DEBUG_KMS("looking for preferred mode on connector %d %d\n",connector->base.id, connector->tile_group ? connector->tile_group->id : 0);
DRM_DEBUG_KMS("looking for preferred mode on [CONNECTOR:%d:%s] %d\n",connector->base.id, connector->name,connector->tile_group ? connector->tile_group->id : 0); modes[i] = drm_connector_has_preferred_mode(connector, width, height);@@ -448,8 +449,8 @@ static bool drm_client_target_preferred(struct drm_connector **connectors, (connector->tile_h_loc == 0 && connector->tile_v_loc == 0 && !drm_connector_get_tiled_mode(connector))) {
DRM_DEBUG_KMS("Falling back to non tiled mode on Connector %d\n",connector->base.id);
DRM_DEBUG_KMS("Falling back to non tiled mode on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name); modes[i] = drm_connector_fallback_non_tiled_mode(connector); } else { modes[i] = drm_connector_get_tiled_mode(connector);@@ -617,15 +618,15 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, num_connectors_detected++;
if (!enabled[i]) {
DRM_DEBUG_KMS("connector %s not enabled, skipping\n",connector->name);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] not enabled, skipping\n",connector->base.id, connector->name); conn_configured |= BIT(i); continue;}
if (connector->force == DRM_FORCE_OFF) {
DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n",connector->name);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] is disabled by user, skipping\n",connector->base.id, connector->name); enabled[i] = false; continue;@@ -635,8 +636,8 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, if (connector->force > DRM_FORCE_OFF) goto bail;
DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n",connector->name);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] has no encoder or crtc, skipping\n",connector->base.id, connector->name); enabled[i] = false; conn_configured |= BIT(i); continue;@@ -658,23 +659,23 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, } }
DRM_DEBUG_KMS("looking for cmdline mode on connector %s\n",connector->name);
DRM_DEBUG_KMS("looking for cmdline mode on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name);/* go for command line mode first */ modes[i] = drm_connector_pick_cmdline_mode(connector);
/* try for preferred next */ if (!modes[i]) {
DRM_DEBUG_KMS("looking for preferred mode on connector %s %d\n",connector->name, connector->has_tile);
DRM_DEBUG_KMS("looking for preferred mode on [CONNECTOR:%d:%s] %d\n",connector->base.id, connector->name, connector->has_tile); modes[i] = drm_connector_has_preferred_mode(connector, width, height);}
/* No preferred mode marked by the EDID? Are there any modes? */ if (!modes[i] && !list_empty(&connector->modes)) {
DRM_DEBUG_KMS("using first mode listed on connector %s\n",connector->name);
DRM_DEBUG_KMS("using first mode listed on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name); modes[i] = list_first_entry(&connector->modes, struct drm_display_mode, head);@@ -693,8 +694,8 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, * This is crtc->mode and not crtc->state->mode for the * fastboot check to work correctly. */
DRM_DEBUG_KMS("looking for current mode on connector %s\n",connector->name);
DRM_DEBUG_KMS("looking for current mode on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name); modes[i] = &connector->state->crtc->mode;@@ -703,8 +704,8 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, */ if (connector->has_tile && num_tiled_conns < connector->num_h_tile * connector->num_v_tile) {
DRM_DEBUG_KMS("Falling back to non tiled mode on Connector %d\n",connector->base.id);
DRM_DEBUG_KMS("Falling back to non tiled mode on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name); modes[i] = drm_connector_fallback_non_tiled_mode(connector);diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 3bc782b630b9..a7085f65865f 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -161,20 +161,22 @@ static void drm_connector_get_cmdline_mode(struct drm_connector *connector) return;
if (mode->force) {
DRM_INFO("forcing %s connector %s\n", connector->name,
DRM_INFO("forcing [CONNECTOR:%d:%s]: %s\n",connector->base.id, connector->name, drm_get_connector_force_name(mode->force));connector->force = mode->force; }
if (mode->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) {
DRM_INFO("cmdline forces connector %s panel_orientation to %d\n",connector->name, mode->panel_orientation);
DRM_INFO("cmdline forces [CONNECTOR:%d:%s] panel_orientation to %d\n",connector->base.id, connector->name,mode->panel_orientation);
- DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
connector->name, mode->name,
- DRM_DEBUG_KMS("cmdline mode for [CONNECTOR:%d:%s] %s %dx%d@%dHz%s%s%s\n",
connector->base.id, connector->name, mode->name, mode->xres, mode->yres, mode->refresh_specified ? mode->refresh : 60, mode->rb ? " reduced blanking" : "",diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 12893e7be89b..242dfcdb7ecb 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5076,32 +5076,32 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, if (hdmi[6] & DRM_EDID_HDMI_DC_30) { dc_bpc = 10; info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_30;
DRM_DEBUG("%s: HDMI sink does deep color 30.\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink does deep color 30.\n",connector->base.id, connector->name);}
if (hdmi[6] & DRM_EDID_HDMI_DC_36) { dc_bpc = 12; info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_36;
DRM_DEBUG("%s: HDMI sink does deep color 36.\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink does deep color 36.\n",connector->base.id, connector->name);}
if (hdmi[6] & DRM_EDID_HDMI_DC_48) { dc_bpc = 16; info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_48;
DRM_DEBUG("%s: HDMI sink does deep color 48.\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink does deep color 48.\n",connector->base.id, connector->name);}
if (dc_bpc == 0) {
DRM_DEBUG("%s: No deep color support on this HDMI sink.\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: No deep color support on this HDMI sink.\n",connector->base.id, connector->name);
- DRM_DEBUG("%s: Assigning HDMI sink color depth as %d bpc.\n",
connector->name, dc_bpc);
DRM_DEBUG("[CONNECTOR:%d:%s]: Assigning HDMI sink color depth as %d bpc.\n",
connector->base.id, connector->name, dc_bpc);info->bpc = dc_bpc;
/*
@@ -5114,8 +5114,8 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, /* YCRCB444 is optional according to spec. */ if (hdmi[6] & DRM_EDID_HDMI_DC_Y444) { info->color_formats |= DRM_COLOR_FORMAT_YCRCB444;
DRM_DEBUG("%s: HDMI sink does YCRCB444 in deep color.\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink does YCRCB444 in deep color.\n",connector->base.id, connector->name);}
/*
@@ -5123,8 +5123,8 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, * then deep color 36 bit must be supported. */ if (!(hdmi[6] & DRM_EDID_HDMI_DC_36)) {
DRM_DEBUG("%s: HDMI sink should do DC_36, but does not!\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink should do DC_36, but does not!\n",connector->base.id, connector->name);}
@@ -5357,8 +5357,8 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi if (info->bpc == 0 && edid->revision == 3 && edid->input & DRM_EDID_DIGITAL_DFP_1_X) { info->bpc = 8;
DRM_DEBUG("%s: Assigning DFP sink color depth as %d bpc.\n",connector->name, info->bpc);
DRM_DEBUG("[CONNECTOR:%d:%s]: Assigning DFP sink color depth as %d bpc.\n",connector->base.id, connector->name, info->bpc);}
/* Only defined for 1.4 with digital displays */
@@ -5390,8 +5390,8 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi break; }
- DRM_DEBUG("%s: Assigning EDID-1.4 digital sink color depth as %d bpc.\n",
connector->name, info->bpc);
DRM_DEBUG("[CONNECTOR:%d:%s]: Assigning EDID-1.4 digital sink color depth as %d bpc.\n",
connector->base.id, connector->name, info->bpc);info->color_formats |= DRM_COLOR_FORMAT_RGB444; if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB444)
diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c index 37d8ba3ddb46..f3f6801927f1 100644 --- a/drivers/gpu/drm/drm_edid_load.c +++ b/drivers/gpu/drm/drm_edid_load.c @@ -188,7 +188,8 @@ static void *edid_load(struct drm_connector *connector, const char *name, pdev = platform_device_register_simple(connector_name, -1, NULL, 0); if (IS_ERR(pdev)) { DRM_ERROR("Failed to register EDID firmware platform device "
"for connector \"%s\"\n", connector_name);
"for [CONNECTOR:%d:%s]\n",connector->base.id, connector_name); return ERR_CAST(pdev);@@ -243,8 +244,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
edid[EDID_LENGTH-1] += edid[0x7e] - valid_extensions; DRM_INFO("Found %d valid extensions instead of %d in EDID data "
"\"%s\" for connector \"%s\"\n", valid_extensions,edid[0x7e], name, connector_name);
"\"%s\" for [CONNECTOR:%d:%s]\n", valid_extensions,edid[0x7e], name, connector->base.id, connector_name);edid[0x7e] = valid_extensions;
new_edid = krealloc(edid, (valid_extensions + 1) * EDID_LENGTH,
@@ -254,9 +255,9 @@ static void *edid_load(struct drm_connector *connector, const char *name, }
DRM_INFO("Got %s EDID base block and %d extension%s from "
"\"%s\" for connector \"%s\"\n", (builtin >= 0) ? "built-in" :
"\"%s\" for [CONNECTOR:%d:%s]\n", (builtin >= 0) ? "built-in" : "external", valid_extensions, valid_extensions == 1 ? "" : "s",
name, connector_name);
name, connector->base.id, connector_name);out: release_firmware(fw); diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 37b4b9f0e468..e46dcd31faa3 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -504,7 +504,8 @@ void drm_mode_config_cleanup(struct drm_device *dev) if (WARN_ON(!list_empty(&dev->mode_config.connector_list))) { drm_connector_list_iter_begin(dev, &conn_iter); drm_for_each_connector_iter(connector, &conn_iter)
DRM_ERROR("connector %s leaked!\n", connector->name);
DRM_ERROR("[CONNECTOR:%d:%s] leaked!\n",connector->base.id, connector->name);-- 2.33.0
On Sat, 13 Nov 2021, Claudio Suarez cssk@net-c.es wrote:
The prefered way to log connectors is [CONNECTOR:id:name]. Change it in drm core programs.
Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Claudio Suarez cssk@net-c.es
drivers/gpu/drm/drm_client_modeset.c | 51 ++++++++++++++-------------- drivers/gpu/drm/drm_connector.c | 12 ++++--- drivers/gpu/drm/drm_edid.c | 36 ++++++++++---------- drivers/gpu/drm/drm_edid_load.c | 11 +++--- drivers/gpu/drm/drm_mode_config.c | 3 +- 5 files changed, 59 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c index ced09c7c06f9..4f35dc375bdd 100644 --- a/drivers/gpu/drm/drm_client_modeset.c +++ b/drivers/gpu/drm/drm_client_modeset.c @@ -240,7 +240,7 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors, for (i = 0; i < connector_count; i++) { connector = connectors[i]; enabled[i] = drm_connector_enabled(connector, true);
DRM_DEBUG_KMS("connector %d enabled? %s\n", connector->base.id,
DRM_DEBUG_KMS("[CONNECTOR:%d;%s] enabled? %s\n", connector->base.id, connector->name, connector->display_info.non_desktop ? "non desktop" : enabled[i] ? "yes" : "no");
You have a semicolon instead of a colon there.
Not to block this patch, but I've wondered about bigger changes such as:
- Adding "debug_name" member or similar in drm_connector, which would be an allocated string with "[CONNECTOR:id:name]" that you could use with just %s while debug logging.
- Adding drm_dbg_connector() which would take drm_connector as context, and do drm_dbg_kms() with the above prefix.
any_enabled |= enabled[i];@@ -350,8 +350,8 @@ static int drm_client_get_tile_offsets(struct drm_connector **connectors, continue;
if (!modes[i] && (h_idx || v_idx)) {
DRM_DEBUG_KMS("no modes for connector tiled %d %d\n", i,connector->base.id);
DRM_DEBUG_KMS("no modes for [CONNECTOR:%d:%s] tiled %d\n",connector->base.id, connector->name, i);
Personally I'd prefer adding [CONNECTOR:id:name] as a prefix in the beginning, throughout, not in the middle.
BR, Jani.
continue; } if (connector->tile_h_loc < h_idx)@@ -419,14 +419,15 @@ static bool drm_client_target_preferred(struct drm_connector **connectors, drm_client_get_tile_offsets(connectors, connector_count, modes, offsets, i, connector->tile_h_loc, connector->tile_v_loc); }
DRM_DEBUG_KMS("looking for cmdline mode on connector %d\n",connector->base.id);
DRM_DEBUG_KMS("looking for cmdline mode on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name);/* got for command line mode first */ modes[i] = drm_connector_pick_cmdline_mode(connector); if (!modes[i]) {
DRM_DEBUG_KMS("looking for preferred mode on connector %d %d\n",connector->base.id, connector->tile_group ? connector->tile_group->id : 0);
DRM_DEBUG_KMS("looking for preferred mode on [CONNECTOR:%d:%s] %d\n",connector->base.id, connector->name,connector->tile_group ? connector->tile_group->id : 0); modes[i] = drm_connector_has_preferred_mode(connector, width, height);@@ -448,8 +449,8 @@ static bool drm_client_target_preferred(struct drm_connector **connectors, (connector->tile_h_loc == 0 && connector->tile_v_loc == 0 && !drm_connector_get_tiled_mode(connector))) {
DRM_DEBUG_KMS("Falling back to non tiled mode on Connector %d\n",connector->base.id);
DRM_DEBUG_KMS("Falling back to non tiled mode on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name); modes[i] = drm_connector_fallback_non_tiled_mode(connector); } else { modes[i] = drm_connector_get_tiled_mode(connector);@@ -617,15 +618,15 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, num_connectors_detected++;
if (!enabled[i]) {
DRM_DEBUG_KMS("connector %s not enabled, skipping\n",connector->name);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] not enabled, skipping\n",connector->base.id, connector->name); conn_configured |= BIT(i); continue;}
if (connector->force == DRM_FORCE_OFF) {
DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n",connector->name);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] is disabled by user, skipping\n",connector->base.id, connector->name); enabled[i] = false; continue;@@ -635,8 +636,8 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, if (connector->force > DRM_FORCE_OFF) goto bail;
DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n",connector->name);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] has no encoder or crtc, skipping\n",connector->base.id, connector->name); enabled[i] = false; conn_configured |= BIT(i); continue;@@ -658,23 +659,23 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, } }
DRM_DEBUG_KMS("looking for cmdline mode on connector %s\n",connector->name);
DRM_DEBUG_KMS("looking for cmdline mode on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name);/* go for command line mode first */ modes[i] = drm_connector_pick_cmdline_mode(connector);
/* try for preferred next */ if (!modes[i]) {
DRM_DEBUG_KMS("looking for preferred mode on connector %s %d\n",connector->name, connector->has_tile);
DRM_DEBUG_KMS("looking for preferred mode on [CONNECTOR:%d:%s] %d\n",connector->base.id, connector->name, connector->has_tile); modes[i] = drm_connector_has_preferred_mode(connector, width, height);}
/* No preferred mode marked by the EDID? Are there any modes? */ if (!modes[i] && !list_empty(&connector->modes)) {
DRM_DEBUG_KMS("using first mode listed on connector %s\n",connector->name);
DRM_DEBUG_KMS("using first mode listed on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name); modes[i] = list_first_entry(&connector->modes, struct drm_display_mode, head);@@ -693,8 +694,8 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, * This is crtc->mode and not crtc->state->mode for the * fastboot check to work correctly. */
DRM_DEBUG_KMS("looking for current mode on connector %s\n",connector->name);
DRM_DEBUG_KMS("looking for current mode on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name); modes[i] = &connector->state->crtc->mode;@@ -703,8 +704,8 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, */ if (connector->has_tile && num_tiled_conns < connector->num_h_tile * connector->num_v_tile) {
DRM_DEBUG_KMS("Falling back to non tiled mode on Connector %d\n",connector->base.id);
DRM_DEBUG_KMS("Falling back to non tiled mode on [CONNECTOR:%d:%s]\n",connector->base.id, connector->name); modes[i] = drm_connector_fallback_non_tiled_mode(connector);diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 3bc782b630b9..a7085f65865f 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -161,20 +161,22 @@ static void drm_connector_get_cmdline_mode(struct drm_connector *connector) return;
if (mode->force) {
DRM_INFO("forcing %s connector %s\n", connector->name,
DRM_INFO("forcing [CONNECTOR:%d:%s]: %s\n",connector->base.id, connector->name, drm_get_connector_force_name(mode->force));connector->force = mode->force; }
if (mode->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) {
DRM_INFO("cmdline forces connector %s panel_orientation to %d\n",connector->name, mode->panel_orientation);
DRM_INFO("cmdline forces [CONNECTOR:%d:%s] panel_orientation to %d\n",connector->base.id, connector->name,mode->panel_orientation);
- DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
connector->name, mode->name,
- DRM_DEBUG_KMS("cmdline mode for [CONNECTOR:%d:%s] %s %dx%d@%dHz%s%s%s\n",
connector->base.id, connector->name, mode->name, mode->xres, mode->yres, mode->refresh_specified ? mode->refresh : 60, mode->rb ? " reduced blanking" : "",diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 12893e7be89b..242dfcdb7ecb 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5076,32 +5076,32 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, if (hdmi[6] & DRM_EDID_HDMI_DC_30) { dc_bpc = 10; info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_30;
DRM_DEBUG("%s: HDMI sink does deep color 30.\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink does deep color 30.\n",connector->base.id, connector->name);}
if (hdmi[6] & DRM_EDID_HDMI_DC_36) { dc_bpc = 12; info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_36;
DRM_DEBUG("%s: HDMI sink does deep color 36.\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink does deep color 36.\n",connector->base.id, connector->name);}
if (hdmi[6] & DRM_EDID_HDMI_DC_48) { dc_bpc = 16; info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_48;
DRM_DEBUG("%s: HDMI sink does deep color 48.\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink does deep color 48.\n",connector->base.id, connector->name);}
if (dc_bpc == 0) {
DRM_DEBUG("%s: No deep color support on this HDMI sink.\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: No deep color support on this HDMI sink.\n",connector->base.id, connector->name);
- DRM_DEBUG("%s: Assigning HDMI sink color depth as %d bpc.\n",
connector->name, dc_bpc);
DRM_DEBUG("[CONNECTOR:%d:%s]: Assigning HDMI sink color depth as %d bpc.\n",
connector->base.id, connector->name, dc_bpc);info->bpc = dc_bpc;
/*
@@ -5114,8 +5114,8 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, /* YCRCB444 is optional according to spec. */ if (hdmi[6] & DRM_EDID_HDMI_DC_Y444) { info->color_formats |= DRM_COLOR_FORMAT_YCRCB444;
DRM_DEBUG("%s: HDMI sink does YCRCB444 in deep color.\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink does YCRCB444 in deep color.\n",connector->base.id, connector->name);}
/*
@@ -5123,8 +5123,8 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, * then deep color 36 bit must be supported. */ if (!(hdmi[6] & DRM_EDID_HDMI_DC_36)) {
DRM_DEBUG("%s: HDMI sink should do DC_36, but does not!\n",connector->name);
DRM_DEBUG("[CONNECTOR:%d:%s]: HDMI sink should do DC_36, but does not!\n",connector->base.id, connector->name);}
@@ -5357,8 +5357,8 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi if (info->bpc == 0 && edid->revision == 3 && edid->input & DRM_EDID_DIGITAL_DFP_1_X) { info->bpc = 8;
DRM_DEBUG("%s: Assigning DFP sink color depth as %d bpc.\n",connector->name, info->bpc);
DRM_DEBUG("[CONNECTOR:%d:%s]: Assigning DFP sink color depth as %d bpc.\n",connector->base.id, connector->name, info->bpc);}
/* Only defined for 1.4 with digital displays */
@@ -5390,8 +5390,8 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi break; }
- DRM_DEBUG("%s: Assigning EDID-1.4 digital sink color depth as %d bpc.\n",
connector->name, info->bpc);
DRM_DEBUG("[CONNECTOR:%d:%s]: Assigning EDID-1.4 digital sink color depth as %d bpc.\n",
connector->base.id, connector->name, info->bpc);info->color_formats |= DRM_COLOR_FORMAT_RGB444; if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB444)
diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c index 37d8ba3ddb46..f3f6801927f1 100644 --- a/drivers/gpu/drm/drm_edid_load.c +++ b/drivers/gpu/drm/drm_edid_load.c @@ -188,7 +188,8 @@ static void *edid_load(struct drm_connector *connector, const char *name, pdev = platform_device_register_simple(connector_name, -1, NULL, 0); if (IS_ERR(pdev)) { DRM_ERROR("Failed to register EDID firmware platform device "
"for connector \"%s\"\n", connector_name);
"for [CONNECTOR:%d:%s]\n",connector->base.id, connector_name); return ERR_CAST(pdev);@@ -243,8 +244,8 @@ static void *edid_load(struct drm_connector *connector, const char *name,
edid[EDID_LENGTH-1] += edid[0x7e] - valid_extensions; DRM_INFO("Found %d valid extensions instead of %d in EDID data "
"\"%s\" for connector \"%s\"\n", valid_extensions,edid[0x7e], name, connector_name);
"\"%s\" for [CONNECTOR:%d:%s]\n", valid_extensions,edid[0x7e], name, connector->base.id, connector_name);edid[0x7e] = valid_extensions;
new_edid = krealloc(edid, (valid_extensions + 1) * EDID_LENGTH,
@@ -254,9 +255,9 @@ static void *edid_load(struct drm_connector *connector, const char *name, }
DRM_INFO("Got %s EDID base block and %d extension%s from "
"\"%s\" for connector \"%s\"\n", (builtin >= 0) ? "built-in" :
"\"%s\" for [CONNECTOR:%d:%s]\n", (builtin >= 0) ? "built-in" : "external", valid_extensions, valid_extensions == 1 ? "" : "s",
name, connector_name);
name, connector->base.id, connector_name);out: release_firmware(fw); diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 37b4b9f0e468..e46dcd31faa3 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -504,7 +504,8 @@ void drm_mode_config_cleanup(struct drm_device *dev) if (WARN_ON(!list_empty(&dev->mode_config.connector_list))) { drm_connector_list_iter_begin(dev, &conn_iter); drm_for_each_connector_iter(connector, &conn_iter)
DRM_ERROR("connector %s leaked!\n", connector->name);
DRM_ERROR("[CONNECTOR:%d:%s] leaked!\n",connector->base.id, connector->name);
On Monday, November 15th, 2021 at 11:22, Jani Nikula jani.nikula@linux.intel.com wrote:
- Adding drm_dbg_connector() which would take drm_connector as context, and do drm_dbg_kms() with the above prefix.
This wouldn't work great in case multiple connectors/planes/etc are involved, or when drm_dbg_atomic() is used.
On Mon, 15 Nov 2021, Simon Ser contact@emersion.fr wrote:
On Monday, November 15th, 2021 at 11:22, Jani Nikula jani.nikula@linux.intel.com wrote:
- Adding drm_dbg_connector() which would take drm_connector as context, and do drm_dbg_kms() with the above prefix.
This wouldn't work great in case multiple connectors/planes/etc are involved, or when drm_dbg_atomic() is used.
That's a good point, though you could still roll those cases manually.
It's also misleading as otherwise drm_dbg_* are categories that can be enabled/disabled via drm.debug.
Again, just musing here.
BR, Jani.
On Mon, Nov 15, 2021 at 12:22:08PM +0200, Jani Nikula wrote:
On Sat, 13 Nov 2021, Claudio Suarez cssk@net-c.es wrote:
The prefered way to log connectors is [CONNECTOR:id:name]. Change it in drm core programs.
Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Claudio Suarez cssk@net-c.es
drivers/gpu/drm/drm_client_modeset.c | 51 ++++++++++++++-------------- drivers/gpu/drm/drm_connector.c | 12 ++++--- drivers/gpu/drm/drm_edid.c | 36 ++++++++++---------- drivers/gpu/drm/drm_edid_load.c | 11 +++--- drivers/gpu/drm/drm_mode_config.c | 3 +- 5 files changed, 59 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c index ced09c7c06f9..4f35dc375bdd 100644 --- a/drivers/gpu/drm/drm_client_modeset.c +++ b/drivers/gpu/drm/drm_client_modeset.c @@ -240,7 +240,7 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors, for (i = 0; i < connector_count; i++) { connector = connectors[i]; enabled[i] = drm_connector_enabled(connector, true);
DRM_DEBUG_KMS("connector %d enabled? %s\n", connector->base.id,
DRM_DEBUG_KMS("[CONNECTOR:%d;%s] enabled? %s\n", connector->base.id, connector->name, connector->display_info.non_desktop ? "non desktop" : enabled[i] ? "yes" : "no");You have a semicolon instead of a colon there.
Sorry my typo. I am going to do a new version.
Not to block this patch, but I've wondered about bigger changes such as:
Adding "debug_name" member or similar in drm_connector, which would be an allocated string with "[CONNECTOR:id:name]" that you could use with just %s while debug logging.
Adding drm_dbg_connector() which would take drm_connector as context, and do drm_dbg_kms() with the above prefix.
any_enabled |= enabled[i];@@ -350,8 +350,8 @@ static int drm_client_get_tile_offsets(struct drm_connector **connectors, continue;
if (!modes[i] && (h_idx || v_idx)) {
DRM_DEBUG_KMS("no modes for connector tiled %d %d\n", i,connector->base.id);
DRM_DEBUG_KMS("no modes for [CONNECTOR:%d:%s] tiled %d\n",connector->base.id, connector->name, i);Personally I'd prefer adding [CONNECTOR:id:name] as a prefix in the beginning, throughout, not in the middle.
I'd prefer too. I am going to change it in the new version.
B.R. Claudio Suarez.
dri-devel@lists.freedesktop.org
-
Claudio Suarez -
Jani Nikula -
jim.cromie＠gmail.com -
Sam Ravnborg -
Simon Ser