Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
On Mon, Nov 15, 2021 at 12:40 PM Claudio Suarez wrote: > > On Mon, Nov 15, 2021 at 12:24:26PM +0200, Jani Nikula wrote: > > On Sun, 14 Nov 2021, Claudio Suarez 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ä > > >> > Signed-off-by: Claudio Suarez > > >> > > >> 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#separate-your-changes > > > > > > C: > > > "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_ 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 ?
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
On Mon, Nov 15, 2021 at 10:17:58PM +0200, Jani Nikula wrote:
> On Mon, 15 Nov 2021, Claudio Suarez wrote:
> > On Mon, Nov 15, 2021 at 12:24:26PM +0200, Jani Nikula wrote:
> >> On Sun, 14 Nov 2021, Claudio Suarez 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ä
> >> >> > Signed-off-by: Claudio Suarez
> >> >>
> >> >> 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#separate-your-changes
> >> >
> >> > C:
> >> > "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
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
On Mon, 15 Nov 2021, Claudio Suarez wrote:
> On Mon, Nov 15, 2021 at 12:24:26PM +0200, Jani Nikula wrote:
>> On Sun, 14 Nov 2021, Claudio Suarez 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ä
>> >> > Signed-off-by: Claudio Suarez
>> >>
>> >> 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#separate-your-changes
>> >
>> > C:
>> > "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.
>
>
>
--
Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
On Mon, Nov 15, 2021 at 12:24:26PM +0200, Jani Nikula wrote:
> On Sun, 14 Nov 2021, Claudio Suarez 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ä
> >> > Signed-off-by: Claudio Suarez
> >>
> >> 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#separate-your-changes
> >
> > C:
> > "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.
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
On Mon, Nov 15, 2021 at 12:22:08PM +0200, Jani Nikula wrote:
> On Sat, 13 Nov 2021, Claudio Suarez wrote:
> > The prefered way to log connectors is [CONNECTOR:id:name]. Change it in
> > drm core programs.
> >
> > Suggested-by: Ville Syrjälä
> > Signed-off-by: Claudio Suarez
> > ---
> > 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.
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
On Mon, 15 Nov 2021, Simon Ser wrote: > On Monday, November 15th, 2021 at 11:22, Jani Nikula > 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. -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
On Monday, November 15th, 2021 at 11:22, Jani Nikula 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.
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
On Sun, 14 Nov 2021, Claudio Suarez 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ä >> > Signed-off-by: Claudio Suarez >> >> 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#separate-your-changes > > C: > "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 > > > -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
On Sat, 13 Nov 2021, Claudio Suarez wrote:
> The prefered way to log connectors is [CONNECTOR:id:name]. Change it in
> drm core programs.
>
> Suggested-by: Ville Syrjälä
> Signed-off-by: Claudio Suarez
> ---
> 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);
> }
> /* 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
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
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ä > > Signed-off-by: Claudio Suarez > > 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#separate-your-changes C: "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
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
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ä
> Signed-off-by: Claudio Suarez
> ---
> 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) {
> -
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
+CC: Ville Syrjälä
+CC: Daniel Vetter
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ä
> Signed-off-by: Claudio Suarez
> ---
> 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) {
>
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
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ä > Signed-off-by: Claudio Suarez While touching all these logging calls, could you convernt them to the newer drm_dbg_kms variants? DRM_DEBUG_* are all deprecated. Sam
[PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
The prefered way to log connectors is [CONNECTOR:id:name]. Change it in
drm core programs.
Suggested-by: Ville Syrjälä
Signed-off-by: Claudio Suarez
---
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
