All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/msm/dpu: Ratelimit framedone timeout msgs
@ 2023-12-11 18:19 ` Rob Clark
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Clark @ 2023-12-11 18:19 UTC (permalink / raw)
  To: dri-devel
  Cc: freedreno, linux-arm-msm, Rob Clark, Rob Clark, Abhinav Kumar,
	Dmitry Baryshkov, Sean Paul, Marijn Suijten, David Airlie,
	Daniel Vetter, Kuogee Hsieh, Jessica Zhang, Vinod Polimera,
	Kalyan Thota, Dan Carpenter, open list

From: Rob Clark <robdclark@chromium.org>

When we start getting these, we get a *lot*.  So ratelimit it to not
flood dmesg.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---

dpu should probably stop rolling it's own trace macros, but that would
be a larger cleanup.

 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 ++++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h     | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 82538844614b..7c22235d0eba 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -39,6 +39,9 @@
 #define DPU_ERROR_ENC(e, fmt, ...) DPU_ERROR("enc%d " fmt,\
 		(e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
 
+#define DPU_ERROR_ENC_RATELIMITED(e, fmt, ...) DPU_ERROR_RATELIMITED("enc%d " fmt,\
+		(e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
+
 /*
  * Two to anticipate panels that can do cmd/vid dynamic switching
  * plan is to create all possible physical encoder types, and switch between
@@ -2339,7 +2342,7 @@ static void dpu_encoder_frame_done_timeout(struct timer_list *t)
 		return;
 	}
 
-	DPU_ERROR_ENC(dpu_enc, "frame done timeout\n");
+	DPU_ERROR_ENC_RATELIMITED(dpu_enc, "frame done timeout\n");
 
 	event = DPU_ENCODER_FRAME_EVENT_ERROR;
 	trace_dpu_enc_frame_done_timeout(DRMID(drm_enc), event);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index b6f53ca6e962..f5473d4dea92 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -51,6 +51,7 @@
 	} while (0)
 
 #define DPU_ERROR(fmt, ...) pr_err("[dpu error]" fmt, ##__VA_ARGS__)
+#define DPU_ERROR_RATELIMITED(fmt, ...) pr_err_ratelimited("[dpu error]" fmt, ##__VA_ARGS__)
 
 /**
  * ktime_compare_safe - compare two ktime structures
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH] drm/msm/dpu: Ratelimit framedone timeout msgs
@ 2023-12-11 18:19 ` Rob Clark
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Clark @ 2023-12-11 18:19 UTC (permalink / raw)
  To: dri-devel
  Cc: Rob Clark, Kalyan Thota, open list, linux-arm-msm, Abhinav Kumar,
	Kuogee Hsieh, Sean Paul, Jessica Zhang, Dmitry Baryshkov,
	Marijn Suijten, Vinod Polimera, freedreno, Dan Carpenter

From: Rob Clark <robdclark@chromium.org>

When we start getting these, we get a *lot*.  So ratelimit it to not
flood dmesg.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---

dpu should probably stop rolling it's own trace macros, but that would
be a larger cleanup.

 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 ++++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h     | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 82538844614b..7c22235d0eba 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -39,6 +39,9 @@
 #define DPU_ERROR_ENC(e, fmt, ...) DPU_ERROR("enc%d " fmt,\
 		(e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
 
+#define DPU_ERROR_ENC_RATELIMITED(e, fmt, ...) DPU_ERROR_RATELIMITED("enc%d " fmt,\
+		(e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
+
 /*
  * Two to anticipate panels that can do cmd/vid dynamic switching
  * plan is to create all possible physical encoder types, and switch between
@@ -2339,7 +2342,7 @@ static void dpu_encoder_frame_done_timeout(struct timer_list *t)
 		return;
 	}
 
-	DPU_ERROR_ENC(dpu_enc, "frame done timeout\n");
+	DPU_ERROR_ENC_RATELIMITED(dpu_enc, "frame done timeout\n");
 
 	event = DPU_ENCODER_FRAME_EVENT_ERROR;
 	trace_dpu_enc_frame_done_timeout(DRMID(drm_enc), event);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index b6f53ca6e962..f5473d4dea92 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -51,6 +51,7 @@
 	} while (0)
 
 #define DPU_ERROR(fmt, ...) pr_err("[dpu error]" fmt, ##__VA_ARGS__)
+#define DPU_ERROR_RATELIMITED(fmt, ...) pr_err_ratelimited("[dpu error]" fmt, ##__VA_ARGS__)
 
 /**
  * ktime_compare_safe - compare two ktime structures
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH] drm/msm/dpu: Ratelimit framedone timeout msgs
  2023-12-11 18:19 ` Rob Clark
@ 2023-12-11 18:26   ` Abhinav Kumar
  -1 siblings, 0 replies; 10+ messages in thread
From: Abhinav Kumar @ 2023-12-11 18:26 UTC (permalink / raw)
  To: Rob Clark, dri-devel
  Cc: freedreno, linux-arm-msm, Rob Clark, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter, Kuogee Hsieh,
	Jessica Zhang, Vinod Polimera, Kalyan Thota, Dan Carpenter,
	open list



On 12/11/2023 10:19 AM, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> When we start getting these, we get a *lot*.  So ratelimit it to not
> flood dmesg.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
> 
> dpu should probably stop rolling it's own trace macros, but that would
> be a larger cleanup.
> 
>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 ++++-
>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h     | 1 +
>   2 files changed, 5 insertions(+), 1 deletion(-)
> 

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] drm/msm/dpu: Ratelimit framedone timeout msgs
@ 2023-12-11 18:26   ` Abhinav Kumar
  0 siblings, 0 replies; 10+ messages in thread
From: Abhinav Kumar @ 2023-12-11 18:26 UTC (permalink / raw)
  To: Rob Clark, dri-devel
  Cc: Rob Clark, freedreno, Kalyan Thota, linux-arm-msm, open list,
	Kuogee Hsieh, Jessica Zhang, Dmitry Baryshkov, Marijn Suijten,
	Vinod Polimera, Sean Paul, Dan Carpenter



On 12/11/2023 10:19 AM, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> When we start getting these, we get a *lot*.  So ratelimit it to not
> flood dmesg.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
> 
> dpu should probably stop rolling it's own trace macros, but that would
> be a larger cleanup.
> 
>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 ++++-
>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h     | 1 +
>   2 files changed, 5 insertions(+), 1 deletion(-)
> 

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] drm/msm/dpu: Ratelimit framedone timeout msgs
  2023-12-11 18:19 ` Rob Clark
@ 2023-12-11 22:09   ` Marijn Suijten
  -1 siblings, 0 replies; 10+ messages in thread
From: Marijn Suijten @ 2023-12-11 22:09 UTC (permalink / raw)
  To: Rob Clark
  Cc: dri-devel, freedreno, linux-arm-msm, Rob Clark, Abhinav Kumar,
	Dmitry Baryshkov, Sean Paul, David Airlie, Daniel Vetter,
	Kuogee Hsieh, Jessica Zhang, Vinod Polimera, Kalyan Thota,
	Dan Carpenter, open list

On 2023-12-11 10:19:55, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> When we start getting these, we get a *lot*.  So ratelimit it to not
> flood dmesg.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
> 
> dpu should probably stop rolling it's own trace macros, but that would
> be a larger cleanup.

That would be lovely, use is currently all over the place.

Should this patch also ratelimit the corresponding:

	[drm:dpu_encoder_phys_cmd_prepare_for_kickoff] *ERROR* failed wait_for_idle: id:31 ret:-110 pp:0

On CMD-mode panels?

Note that this is a prime example of using DRM_ERROR over DPU_ERROR*, resulting
in unnecessary divergence (and un-readability) between error messages and the
code (DPU_DEBUG_CMDENC, which has a corresponding DPU_ERROR variant, is also
used within that function...)

Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>

>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 ++++-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h     | 1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 82538844614b..7c22235d0eba 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -39,6 +39,9 @@
>  #define DPU_ERROR_ENC(e, fmt, ...) DPU_ERROR("enc%d " fmt,\
>  		(e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
>  
> +#define DPU_ERROR_ENC_RATELIMITED(e, fmt, ...) DPU_ERROR_RATELIMITED("enc%d " fmt,\
> +		(e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
> +
>  /*
>   * Two to anticipate panels that can do cmd/vid dynamic switching
>   * plan is to create all possible physical encoder types, and switch between
> @@ -2339,7 +2342,7 @@ static void dpu_encoder_frame_done_timeout(struct timer_list *t)
>  		return;
>  	}
>  
> -	DPU_ERROR_ENC(dpu_enc, "frame done timeout\n");
> +	DPU_ERROR_ENC_RATELIMITED(dpu_enc, "frame done timeout\n");
>  
>  	event = DPU_ENCODER_FRAME_EVENT_ERROR;
>  	trace_dpu_enc_frame_done_timeout(DRMID(drm_enc), event);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index b6f53ca6e962..f5473d4dea92 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -51,6 +51,7 @@
>  	} while (0)
>  
>  #define DPU_ERROR(fmt, ...) pr_err("[dpu error]" fmt, ##__VA_ARGS__)
> +#define DPU_ERROR_RATELIMITED(fmt, ...) pr_err_ratelimited("[dpu error]" fmt, ##__VA_ARGS__)
>  
>  /**
>   * ktime_compare_safe - compare two ktime structures
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] drm/msm/dpu: Ratelimit framedone timeout msgs
@ 2023-12-11 22:09   ` Marijn Suijten
  0 siblings, 0 replies; 10+ messages in thread
From: Marijn Suijten @ 2023-12-11 22:09 UTC (permalink / raw)
  To: Rob Clark
  Cc: Rob Clark, Kalyan Thota, open list, linux-arm-msm, Abhinav Kumar,
	dri-devel, Kuogee Hsieh, Sean Paul, Dmitry Baryshkov,
	Jessica Zhang, Vinod Polimera, freedreno, Dan Carpenter

On 2023-12-11 10:19:55, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> When we start getting these, we get a *lot*.  So ratelimit it to not
> flood dmesg.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
> 
> dpu should probably stop rolling it's own trace macros, but that would
> be a larger cleanup.

That would be lovely, use is currently all over the place.

Should this patch also ratelimit the corresponding:

	[drm:dpu_encoder_phys_cmd_prepare_for_kickoff] *ERROR* failed wait_for_idle: id:31 ret:-110 pp:0

On CMD-mode panels?

Note that this is a prime example of using DRM_ERROR over DPU_ERROR*, resulting
in unnecessary divergence (and un-readability) between error messages and the
code (DPU_DEBUG_CMDENC, which has a corresponding DPU_ERROR variant, is also
used within that function...)

Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>

>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 ++++-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h     | 1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 82538844614b..7c22235d0eba 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -39,6 +39,9 @@
>  #define DPU_ERROR_ENC(e, fmt, ...) DPU_ERROR("enc%d " fmt,\
>  		(e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
>  
> +#define DPU_ERROR_ENC_RATELIMITED(e, fmt, ...) DPU_ERROR_RATELIMITED("enc%d " fmt,\
> +		(e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
> +
>  /*
>   * Two to anticipate panels that can do cmd/vid dynamic switching
>   * plan is to create all possible physical encoder types, and switch between
> @@ -2339,7 +2342,7 @@ static void dpu_encoder_frame_done_timeout(struct timer_list *t)
>  		return;
>  	}
>  
> -	DPU_ERROR_ENC(dpu_enc, "frame done timeout\n");
> +	DPU_ERROR_ENC_RATELIMITED(dpu_enc, "frame done timeout\n");
>  
>  	event = DPU_ENCODER_FRAME_EVENT_ERROR;
>  	trace_dpu_enc_frame_done_timeout(DRMID(drm_enc), event);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index b6f53ca6e962..f5473d4dea92 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -51,6 +51,7 @@
>  	} while (0)
>  
>  #define DPU_ERROR(fmt, ...) pr_err("[dpu error]" fmt, ##__VA_ARGS__)
> +#define DPU_ERROR_RATELIMITED(fmt, ...) pr_err_ratelimited("[dpu error]" fmt, ##__VA_ARGS__)
>  
>  /**
>   * ktime_compare_safe - compare two ktime structures
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] drm/msm/dpu: Ratelimit framedone timeout msgs
  2023-12-11 22:09   ` Marijn Suijten
@ 2023-12-11 23:04     ` Rob Clark
  -1 siblings, 0 replies; 10+ messages in thread
From: Rob Clark @ 2023-12-11 23:04 UTC (permalink / raw)
  To: Marijn Suijten
  Cc: dri-devel, freedreno, linux-arm-msm, Rob Clark, Abhinav Kumar,
	Dmitry Baryshkov, Sean Paul, David Airlie, Daniel Vetter,
	Kuogee Hsieh, Jessica Zhang, Vinod Polimera, Kalyan Thota,
	Dan Carpenter, open list

On Mon, Dec 11, 2023 at 2:09 PM Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> On 2023-12-11 10:19:55, Rob Clark wrote:
> > From: Rob Clark <robdclark@chromium.org>
> >
> > When we start getting these, we get a *lot*.  So ratelimit it to not
> > flood dmesg.
> >
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> >
> > dpu should probably stop rolling it's own trace macros, but that would
> > be a larger cleanup.
>
> That would be lovely, use is currently all over the place.
>
> Should this patch also ratelimit the corresponding:
>
>         [drm:dpu_encoder_phys_cmd_prepare_for_kickoff] *ERROR* failed wait_for_idle: id:31 ret:-110 pp:0
>
> On CMD-mode panels?

Probably it should for consistency.  But I think you normally wouldn't
get this error at 60Hz with a cmd mode panel, so probably ok to make
it ratelimited for cmd mode later.

BR,
-R

> Note that this is a prime example of using DRM_ERROR over DPU_ERROR*, resulting
> in unnecessary divergence (and un-readability) between error messages and the
> code (DPU_DEBUG_CMDENC, which has a corresponding DPU_ERROR variant, is also
> used within that function...)
>
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 ++++-
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h     | 1 +
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index 82538844614b..7c22235d0eba 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -39,6 +39,9 @@
> >  #define DPU_ERROR_ENC(e, fmt, ...) DPU_ERROR("enc%d " fmt,\
> >               (e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
> >
> > +#define DPU_ERROR_ENC_RATELIMITED(e, fmt, ...) DPU_ERROR_RATELIMITED("enc%d " fmt,\
> > +             (e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
> > +
> >  /*
> >   * Two to anticipate panels that can do cmd/vid dynamic switching
> >   * plan is to create all possible physical encoder types, and switch between
> > @@ -2339,7 +2342,7 @@ static void dpu_encoder_frame_done_timeout(struct timer_list *t)
> >               return;
> >       }
> >
> > -     DPU_ERROR_ENC(dpu_enc, "frame done timeout\n");
> > +     DPU_ERROR_ENC_RATELIMITED(dpu_enc, "frame done timeout\n");
> >
> >       event = DPU_ENCODER_FRAME_EVENT_ERROR;
> >       trace_dpu_enc_frame_done_timeout(DRMID(drm_enc), event);
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > index b6f53ca6e962..f5473d4dea92 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > @@ -51,6 +51,7 @@
> >       } while (0)
> >
> >  #define DPU_ERROR(fmt, ...) pr_err("[dpu error]" fmt, ##__VA_ARGS__)
> > +#define DPU_ERROR_RATELIMITED(fmt, ...) pr_err_ratelimited("[dpu error]" fmt, ##__VA_ARGS__)
> >
> >  /**
> >   * ktime_compare_safe - compare two ktime structures
> > --
> > 2.43.0
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] drm/msm/dpu: Ratelimit framedone timeout msgs
@ 2023-12-11 23:04     ` Rob Clark
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Clark @ 2023-12-11 23:04 UTC (permalink / raw)
  To: Marijn Suijten
  Cc: Rob Clark, Kalyan Thota, open list, linux-arm-msm, Abhinav Kumar,
	dri-devel, Kuogee Hsieh, Sean Paul, Dmitry Baryshkov,
	Jessica Zhang, Vinod Polimera, freedreno, Dan Carpenter

On Mon, Dec 11, 2023 at 2:09 PM Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> On 2023-12-11 10:19:55, Rob Clark wrote:
> > From: Rob Clark <robdclark@chromium.org>
> >
> > When we start getting these, we get a *lot*.  So ratelimit it to not
> > flood dmesg.
> >
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> >
> > dpu should probably stop rolling it's own trace macros, but that would
> > be a larger cleanup.
>
> That would be lovely, use is currently all over the place.
>
> Should this patch also ratelimit the corresponding:
>
>         [drm:dpu_encoder_phys_cmd_prepare_for_kickoff] *ERROR* failed wait_for_idle: id:31 ret:-110 pp:0
>
> On CMD-mode panels?

Probably it should for consistency.  But I think you normally wouldn't
get this error at 60Hz with a cmd mode panel, so probably ok to make
it ratelimited for cmd mode later.

BR,
-R

> Note that this is a prime example of using DRM_ERROR over DPU_ERROR*, resulting
> in unnecessary divergence (and un-readability) between error messages and the
> code (DPU_DEBUG_CMDENC, which has a corresponding DPU_ERROR variant, is also
> used within that function...)
>
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 ++++-
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h     | 1 +
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index 82538844614b..7c22235d0eba 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -39,6 +39,9 @@
> >  #define DPU_ERROR_ENC(e, fmt, ...) DPU_ERROR("enc%d " fmt,\
> >               (e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
> >
> > +#define DPU_ERROR_ENC_RATELIMITED(e, fmt, ...) DPU_ERROR_RATELIMITED("enc%d " fmt,\
> > +             (e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
> > +
> >  /*
> >   * Two to anticipate panels that can do cmd/vid dynamic switching
> >   * plan is to create all possible physical encoder types, and switch between
> > @@ -2339,7 +2342,7 @@ static void dpu_encoder_frame_done_timeout(struct timer_list *t)
> >               return;
> >       }
> >
> > -     DPU_ERROR_ENC(dpu_enc, "frame done timeout\n");
> > +     DPU_ERROR_ENC_RATELIMITED(dpu_enc, "frame done timeout\n");
> >
> >       event = DPU_ENCODER_FRAME_EVENT_ERROR;
> >       trace_dpu_enc_frame_done_timeout(DRMID(drm_enc), event);
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > index b6f53ca6e962..f5473d4dea92 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > @@ -51,6 +51,7 @@
> >       } while (0)
> >
> >  #define DPU_ERROR(fmt, ...) pr_err("[dpu error]" fmt, ##__VA_ARGS__)
> > +#define DPU_ERROR_RATELIMITED(fmt, ...) pr_err_ratelimited("[dpu error]" fmt, ##__VA_ARGS__)
> >
> >  /**
> >   * ktime_compare_safe - compare two ktime structures
> > --
> > 2.43.0
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] drm/msm/dpu: Ratelimit framedone timeout msgs
  2023-12-11 18:19 ` Rob Clark
@ 2023-12-13  0:37   ` Dmitry Baryshkov
  -1 siblings, 0 replies; 10+ messages in thread
From: Dmitry Baryshkov @ 2023-12-13  0:37 UTC (permalink / raw)
  To: dri-devel, Rob Clark
  Cc: freedreno, linux-arm-msm, Rob Clark, Abhinav Kumar, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter, Kuogee Hsieh,
	Jessica Zhang, Vinod Polimera, Kalyan Thota, Dan Carpenter,
	linux-kernel


On Mon, 11 Dec 2023 10:19:55 -0800, Rob Clark wrote:
> When we start getting these, we get a *lot*.  So ratelimit it to not
> flood dmesg.
> 
> 

Applied, thanks!

[1/1] drm/msm/dpu: Ratelimit framedone timeout msgs
      https://gitlab.freedesktop.org/lumag/msm/-/commit/e37cb117b819

Best regards,
-- 
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] drm/msm/dpu: Ratelimit framedone timeout msgs
@ 2023-12-13  0:37   ` Dmitry Baryshkov
  0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Baryshkov @ 2023-12-13  0:37 UTC (permalink / raw)
  To: dri-devel, Rob Clark
  Cc: Rob Clark, freedreno, linux-kernel, Kalyan Thota, linux-arm-msm,
	Abhinav Kumar, Kuogee Hsieh, Jessica Zhang, Marijn Suijten,
	Vinod Polimera, Sean Paul, Dan Carpenter


On Mon, 11 Dec 2023 10:19:55 -0800, Rob Clark wrote:
> When we start getting these, we get a *lot*.  So ratelimit it to not
> flood dmesg.
> 
> 

Applied, thanks!

[1/1] drm/msm/dpu: Ratelimit framedone timeout msgs
      https://gitlab.freedesktop.org/lumag/msm/-/commit/e37cb117b819

Best regards,
-- 
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-12-13  0:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-11 18:19 [PATCH] drm/msm/dpu: Ratelimit framedone timeout msgs Rob Clark
2023-12-11 18:19 ` Rob Clark
2023-12-11 18:26 ` Abhinav Kumar
2023-12-11 18:26   ` Abhinav Kumar
2023-12-11 22:09 ` Marijn Suijten
2023-12-11 22:09   ` Marijn Suijten
2023-12-11 23:04   ` Rob Clark
2023-12-11 23:04     ` Rob Clark
2023-12-13  0:37 ` Dmitry Baryshkov
2023-12-13  0:37   ` Dmitry Baryshkov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.