diff mbox series

[09/12] drm/i915: Skip intel_crtc_state_dump() if debugs aren't enabled

Message ID 20240215164055.30585-10-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Use drm_printer more | expand

Commit Message

Ville Syrjälä Feb. 15, 2024, 4:40 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

intel_crtc_state_dump() does a whole boatload of string formatting
which is all wasted energy if the debugs aren't even enabled. Skip
the whole thing in that case.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_crtc_state_dump.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Jani Nikula Feb. 29, 2024, 3:20 p.m. UTC | #1
On Thu, 15 Feb 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> intel_crtc_state_dump() does a whole boatload of string formatting
> which is all wasted energy if the debugs aren't even enabled. Skip
> the whole thing in that case.

I wonder how something like this would work to skip it in a more generic
fashion:

index 9cc473e5d353..0adc8020ae4f 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -206,7 +206,8 @@ drm_vprintf(struct drm_printer *p, const char *fmt, va_list *va)
 {
        struct va_format vaf = { .fmt = fmt, .va = va };
 
-       p->printfn(p, &vaf);
+       if (p->printfn)
+               p->printfn(p, &vaf);
 }
 
 /**
@@ -330,7 +331,7 @@ static inline struct drm_printer drm_dbg_printer(struct drm_device *drm,
                                                 const char *prefix)
 {
        struct drm_printer p = {
-               .printfn = __drm_printfn_dbg,
+               .printfn = drm_debug_enabled(category) ? __drm_printfn_dbg : NULL,
                .arg = drm,
                .prefix = prefix,
                .category = category,

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_crtc_state_dump.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> index b5b9b99213cf..cd78c200d483 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> @@ -192,6 +192,9 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
>  	char buf[64];
>  	int i;
>  
> +	if (!drm_debug_enabled(DRM_UT_KMS))
> +		return;
> +
>  	p = drm_dbg_printer(&i915->drm, DRM_UT_KMS, NULL);
>  
>  	drm_printf(&p, "[CRTC:%d:%s] enable: %s [%s]\n",
Jani Nikula Feb. 29, 2024, 3:21 p.m. UTC | #2
On Thu, 29 Feb 2024, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Thu, 15 Feb 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> intel_crtc_state_dump() does a whole boatload of string formatting
>> which is all wasted energy if the debugs aren't even enabled. Skip
>> the whole thing in that case.
>
> I wonder how something like this would work to skip it in a more generic
> fashion:

In the mean time,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>
> index 9cc473e5d353..0adc8020ae4f 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -206,7 +206,8 @@ drm_vprintf(struct drm_printer *p, const char *fmt, va_list *va)
>  {
>         struct va_format vaf = { .fmt = fmt, .va = va };
>  
> -       p->printfn(p, &vaf);
> +       if (p->printfn)
> +               p->printfn(p, &vaf);
>  }
>  
>  /**
> @@ -330,7 +331,7 @@ static inline struct drm_printer drm_dbg_printer(struct drm_device *drm,
>                                                  const char *prefix)
>  {
>         struct drm_printer p = {
> -               .printfn = __drm_printfn_dbg,
> +               .printfn = drm_debug_enabled(category) ? __drm_printfn_dbg : NULL,
>                 .arg = drm,
>                 .prefix = prefix,
>                 .category = category,
>
>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_crtc_state_dump.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
>> index b5b9b99213cf..cd78c200d483 100644
>> --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
>> +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
>> @@ -192,6 +192,9 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
>>  	char buf[64];
>>  	int i;
>>  
>> +	if (!drm_debug_enabled(DRM_UT_KMS))
>> +		return;
>> +
>>  	p = drm_dbg_printer(&i915->drm, DRM_UT_KMS, NULL);
>>  
>>  	drm_printf(&p, "[CRTC:%d:%s] enable: %s [%s]\n",
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
index b5b9b99213cf..cd78c200d483 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
@@ -192,6 +192,9 @@  void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
 	char buf[64];
 	int i;
 
+	if (!drm_debug_enabled(DRM_UT_KMS))
+		return;
+
 	p = drm_dbg_printer(&i915->drm, DRM_UT_KMS, NULL);
 
 	drm_printf(&p, "[CRTC:%d:%s] enable: %s [%s]\n",