diff mbox

[04/11] drm/gma500: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()

Message ID 1395676398-7058-5-git-send-email-damien.lespiau@intel.com (mailing list archive)
State Accepted
Headers show

Commit Message

Lespiau, Damien March 24, 2014, 3:53 p.m. UTC
There are only a few users of the DRM_LOG_KMS() macro. We can simplify
the DRM code a bit by replacing them by DRM_DEBUG_KMS().

Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/gma500/psb_intel_sdvo.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Comments

Daniel Vetter March 24, 2014, 8:39 p.m. UTC | #1
On Mon, Mar 24, 2014 at 03:53:11PM +0000, Damien Lespiau wrote:
> There are only a few users of the DRM_LOG_KMS() macro. We can simplify
> the DRM code a bit by replacing them by DRM_DEBUG_KMS().
> 
> Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

Note that the point here of using LOG_KMS is to esssentially have
continuations. Which means your patch here will make the output extremely
noisy and hard to read.

But the current code is already racy which annoyed me sufficently a while
ago in i915's copy to fix it all up properly in

commit 84fcb46977e57bafba40bde32067bacc1e510f9c                                                                                      
Author: Daniel Vetter <daniel.vetter@ffwll.ch>                                                                                       
Date:   Wed Nov 27 16:03:01 2013 +0100                                                                                               

    drm/i915/sdvo: Fix up debug output to not split lines
    
    It leads to a big mess when stuff interleaves. Especially with the new
    patch I've submitted for the drm core to no longer artificially split
    up debug messages.
    
    v2: The size parameter to snprintf includes the terminating 0, but the
    return value does not. Adjust the logic accordingly. Spotted by Mika.
    
    Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
    Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Just my 2c.

Cheers, Daniel
> ---
>  drivers/gpu/drm/gma500/psb_intel_sdvo.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
> index 07d3a9e..681efec 100644
> --- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
> +++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
> @@ -406,18 +406,18 @@ static void psb_intel_sdvo_debug_write(struct psb_intel_sdvo *psb_intel_sdvo, u8
>  	DRM_DEBUG_KMS("%s: W: %02X ",
>  				SDVO_NAME(psb_intel_sdvo), cmd);
>  	for (i = 0; i < args_len; i++)
> -		DRM_LOG_KMS("%02X ", ((u8 *)args)[i]);
> +		DRM_DEBUG_KMS("%02X ", ((u8 *)args)[i]);
>  	for (; i < 8; i++)
> -		DRM_LOG_KMS("   ");
> +		DRM_DEBUG_KMS("   ");
>  	for (i = 0; i < ARRAY_SIZE(sdvo_cmd_names); i++) {
>  		if (cmd == sdvo_cmd_names[i].cmd) {
> -			DRM_LOG_KMS("(%s)", sdvo_cmd_names[i].name);
> +			DRM_DEBUG_KMS("(%s)", sdvo_cmd_names[i].name);
>  			break;
>  		}
>  	}
>  	if (i == ARRAY_SIZE(sdvo_cmd_names))
> -		DRM_LOG_KMS("(%02X)", cmd);
> -	DRM_LOG_KMS("\n");
> +		DRM_DEBUG_KMS("(%02X)", cmd);
> +	DRM_DEBUG_KMS("\n");
>  }
>  
>  static const char *cmd_status_names[] = {
> @@ -512,9 +512,9 @@ static bool psb_intel_sdvo_read_response(struct psb_intel_sdvo *psb_intel_sdvo,
>  	}
>  
>  	if (status <= SDVO_CMD_STATUS_SCALING_NOT_SUPP)
> -		DRM_LOG_KMS("(%s)", cmd_status_names[status]);
> +		DRM_DEBUG_KMS("(%s)", cmd_status_names[status]);
>  	else
> -		DRM_LOG_KMS("(??? %d)", status);
> +		DRM_DEBUG_KMS("(??? %d)", status);
>  
>  	if (status != SDVO_CMD_STATUS_SUCCESS)
>  		goto log_fail;
> @@ -525,13 +525,13 @@ static bool psb_intel_sdvo_read_response(struct psb_intel_sdvo *psb_intel_sdvo,
>  					  SDVO_I2C_RETURN_0 + i,
>  					  &((u8 *)response)[i]))
>  			goto log_fail;
> -		DRM_LOG_KMS(" %02X", ((u8 *)response)[i]);
> +		DRM_DEBUG_KMS(" %02X", ((u8 *)response)[i]);
>  	}
> -	DRM_LOG_KMS("\n");
> +	DRM_DEBUG_KMS("\n");
>  	return true;
>  
>  log_fail:
> -	DRM_LOG_KMS("... failed\n");
> +	DRM_DEBUG_KMS("... failed\n");
>  	return false;
>  }
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Patrik Jakobsson March 24, 2014, 9:12 p.m. UTC | #2
On Mon, Mar 24, 2014 at 9:39 PM, Daniel Vetter <daniel@ffwll.ch> wrote:

> On Mon, Mar 24, 2014 at 03:53:11PM +0000, Damien Lespiau wrote:
> > There are only a few users of the DRM_LOG_KMS() macro. We can simplify
> > the DRM code a bit by replacing them by DRM_DEBUG_KMS().
> >
> > Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
>
> Note that the point here of using LOG_KMS is to esssentially have
> continuations. Which means your patch here will make the output extremely
> noisy and hard to read.
>

The noise is already there (tons of empty lines) so this patch will not make
it any worse. It needs to be fixed like in i915. I'll put it on my
todo-list.

Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>



> But the current code is already racy which annoyed me sufficently a while
> ago in i915's copy to fix it all up properly in
>
> commit 84fcb46977e57bafba40bde32067bacc1e510f9c
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Wed Nov 27 16:03:01 2013 +0100
>
>     drm/i915/sdvo: Fix up debug output to not split lines
>
>     It leads to a big mess when stuff interleaves. Especially with the new
>     patch I've submitted for the drm core to no longer artificially split
>     up debug messages.
>
>     v2: The size parameter to snprintf includes the terminating 0, but the
>     return value does not. Adjust the logic accordingly. Spotted by Mika.
>
>     Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>     Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
>     Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Just my 2c.
>
> Cheers, Daniel
> > ---
> >  drivers/gpu/drm/gma500/psb_intel_sdvo.c | 20 ++++++++++----------
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
> b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
> > index 07d3a9e..681efec 100644
> > --- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
> > +++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
> > @@ -406,18 +406,18 @@ static void psb_intel_sdvo_debug_write(struct
> psb_intel_sdvo *psb_intel_sdvo, u8
> >       DRM_DEBUG_KMS("%s: W: %02X ",
> >                               SDVO_NAME(psb_intel_sdvo), cmd);
> >       for (i = 0; i < args_len; i++)
> > -             DRM_LOG_KMS("%02X ", ((u8 *)args)[i]);
> > +             DRM_DEBUG_KMS("%02X ", ((u8 *)args)[i]);
> >       for (; i < 8; i++)
> > -             DRM_LOG_KMS("   ");
> > +             DRM_DEBUG_KMS("   ");
> >       for (i = 0; i < ARRAY_SIZE(sdvo_cmd_names); i++) {
> >               if (cmd == sdvo_cmd_names[i].cmd) {
> > -                     DRM_LOG_KMS("(%s)", sdvo_cmd_names[i].name);
> > +                     DRM_DEBUG_KMS("(%s)", sdvo_cmd_names[i].name);
> >                       break;
> >               }
> >       }
> >       if (i == ARRAY_SIZE(sdvo_cmd_names))
> > -             DRM_LOG_KMS("(%02X)", cmd);
> > -     DRM_LOG_KMS("\n");
> > +             DRM_DEBUG_KMS("(%02X)", cmd);
> > +     DRM_DEBUG_KMS("\n");
> >  }
> >
> >  static const char *cmd_status_names[] = {
> > @@ -512,9 +512,9 @@ static bool psb_intel_sdvo_read_response(struct
> psb_intel_sdvo *psb_intel_sdvo,
> >       }
> >
> >       if (status <= SDVO_CMD_STATUS_SCALING_NOT_SUPP)
> > -             DRM_LOG_KMS("(%s)", cmd_status_names[status]);
> > +             DRM_DEBUG_KMS("(%s)", cmd_status_names[status]);
> >       else
> > -             DRM_LOG_KMS("(??? %d)", status);
> > +             DRM_DEBUG_KMS("(??? %d)", status);
> >
> >       if (status != SDVO_CMD_STATUS_SUCCESS)
> >               goto log_fail;
> > @@ -525,13 +525,13 @@ static bool psb_intel_sdvo_read_response(struct
> psb_intel_sdvo *psb_intel_sdvo,
> >                                         SDVO_I2C_RETURN_0 + i,
> >                                         &((u8 *)response)[i]))
> >                       goto log_fail;
> > -             DRM_LOG_KMS(" %02X", ((u8 *)response)[i]);
> > +             DRM_DEBUG_KMS(" %02X", ((u8 *)response)[i]);
> >       }
> > -     DRM_LOG_KMS("\n");
> > +     DRM_DEBUG_KMS("\n");
> >       return true;
> >
> >  log_fail:
> > -     DRM_LOG_KMS("... failed\n");
> > +     DRM_DEBUG_KMS("... failed\n");
> >       return false;
> >  }
> >
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
index 07d3a9e..681efec 100644
--- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
+++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
@@ -406,18 +406,18 @@  static void psb_intel_sdvo_debug_write(struct psb_intel_sdvo *psb_intel_sdvo, u8
 	DRM_DEBUG_KMS("%s: W: %02X ",
 				SDVO_NAME(psb_intel_sdvo), cmd);
 	for (i = 0; i < args_len; i++)
-		DRM_LOG_KMS("%02X ", ((u8 *)args)[i]);
+		DRM_DEBUG_KMS("%02X ", ((u8 *)args)[i]);
 	for (; i < 8; i++)
-		DRM_LOG_KMS("   ");
+		DRM_DEBUG_KMS("   ");
 	for (i = 0; i < ARRAY_SIZE(sdvo_cmd_names); i++) {
 		if (cmd == sdvo_cmd_names[i].cmd) {
-			DRM_LOG_KMS("(%s)", sdvo_cmd_names[i].name);
+			DRM_DEBUG_KMS("(%s)", sdvo_cmd_names[i].name);
 			break;
 		}
 	}
 	if (i == ARRAY_SIZE(sdvo_cmd_names))
-		DRM_LOG_KMS("(%02X)", cmd);
-	DRM_LOG_KMS("\n");
+		DRM_DEBUG_KMS("(%02X)", cmd);
+	DRM_DEBUG_KMS("\n");
 }
 
 static const char *cmd_status_names[] = {
@@ -512,9 +512,9 @@  static bool psb_intel_sdvo_read_response(struct psb_intel_sdvo *psb_intel_sdvo,
 	}
 
 	if (status <= SDVO_CMD_STATUS_SCALING_NOT_SUPP)
-		DRM_LOG_KMS("(%s)", cmd_status_names[status]);
+		DRM_DEBUG_KMS("(%s)", cmd_status_names[status]);
 	else
-		DRM_LOG_KMS("(??? %d)", status);
+		DRM_DEBUG_KMS("(??? %d)", status);
 
 	if (status != SDVO_CMD_STATUS_SUCCESS)
 		goto log_fail;
@@ -525,13 +525,13 @@  static bool psb_intel_sdvo_read_response(struct psb_intel_sdvo *psb_intel_sdvo,
 					  SDVO_I2C_RETURN_0 + i,
 					  &((u8 *)response)[i]))
 			goto log_fail;
-		DRM_LOG_KMS(" %02X", ((u8 *)response)[i]);
+		DRM_DEBUG_KMS(" %02X", ((u8 *)response)[i]);
 	}
-	DRM_LOG_KMS("\n");
+	DRM_DEBUG_KMS("\n");
 	return true;
 
 log_fail:
-	DRM_LOG_KMS("... failed\n");
+	DRM_DEBUG_KMS("... failed\n");
 	return false;
 }