diff mbox series

drm/print: Add drm_dbg_ratelimited

Message ID 20231006132606.810110-1-andi.shyti@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/print: Add drm_dbg_ratelimited | expand

Commit Message

Andi Shyti Oct. 6, 2023, 1:26 p.m. UTC
From: Nirmoy Das <nirmoy.das@intel.com>

Add a function for ratelimitted debug print.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
---
 include/drm/drm_print.h | 3 +++
 1 file changed, 3 insertions(+)

Comments

Andi Shyti Oct. 9, 2023, 4:52 p.m. UTC | #1
Hi,

> From: Nirmoy Das <nirmoy.das@intel.com>
> 
> Add a function for ratelimitted debug print.
> 
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>

Just a kind reminder!

This is the second time this patch has been sent and we have seen
some potential use of the drm_dbg_ratelimited().

Any feedback?

Thanks,
Andi
John Harrison Oct. 9, 2023, 6:52 p.m. UTC | #2
On 10/9/2023 09:52, Andi Shyti wrote:
> Hi,
>
>> From: Nirmoy Das <nirmoy.das@intel.com>
>>
>> Add a function for ratelimitted debug print.
>>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Maxime Ripard <mripard@kernel.org>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: David Airlie <airlied@gmail.com>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> Just a kind reminder!
>
> This is the second time this patch has been sent and we have seen
> some potential use of the drm_dbg_ratelimited().
But this patch does not actually add a user. So it is dead code at this 
point, which is not allowed.

If you have code that wants to use such a helper then the helper should 
be part of the patch that adds that code.

John.

>
> Any feedback?
>
> Thanks,
> Andi
Andi Shyti Oct. 9, 2023, 7:43 p.m. UTC | #3
Hi John,

> > > From: Nirmoy Das <nirmoy.das@intel.com>
> > > 
> > > Add a function for ratelimitted debug print.
> > > 
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Maxime Ripard <mripard@kernel.org>
> > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > Cc: David Airlie <airlied@gmail.com>
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> > > Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> > > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> > > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> > Just a kind reminder!
> > 
> > This is the second time this patch has been sent and we have seen
> > some potential use of the drm_dbg_ratelimited().
> But this patch does not actually add a user. So it is dead code at this
> point, which is not allowed.
> 
> If you have code that wants to use such a helper then the helper should be
> part of the patch that adds that code.

this is the kind of "Argh! If I had it!" patch. At the 3/4th
"Argh! If I had it!" then you decide to send it but don't
remember for which case you needed it.

I'm sure that once this goes in won't be long until poeple will
start using it. In any case, if it doesn't go in I will keep it
ready until the case comes.

Andi
John Harrison Oct. 9, 2023, 7:52 p.m. UTC | #4
On 10/9/2023 12:43, Andi Shyti wrote:
> Hi John,
>
>>>> From: Nirmoy Das <nirmoy.das@intel.com>
>>>>
>>>> Add a function for ratelimitted debug print.
>>>>
>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> Cc: Maxime Ripard <mripard@kernel.org>
>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>>> Cc: David Airlie <airlied@gmail.com>
>>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>>> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>>>> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
>>>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>>>> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
>>> Just a kind reminder!
>>>
>>> This is the second time this patch has been sent and we have seen
>>> some potential use of the drm_dbg_ratelimited().
>> But this patch does not actually add a user. So it is dead code at this
>> point, which is not allowed.
>>
>> If you have code that wants to use such a helper then the helper should be
>> part of the patch that adds that code.
> this is the kind of "Argh! If I had it!" patch. At the 3/4th
> "Argh! If I had it!" then you decide to send it but don't
> remember for which case you needed it.
But you should have sent it on the 1st patch set that wanted it.

And if you have code already merged that would benefit from it, then 
update that code to use it and post that as a patch together with this 
implementation.

And note that if your code is GT related, then it should use 
gt_dbg_ratelimited not drm_dbg_ratelimited.

John.


>
> I'm sure that once this goes in won't be long until poeple will
> start using it. In any case, if it doesn't go in I will keep it
> ready until the case comes.
>
> Andi
diff mbox series

Patch

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index a93a387f8a1a..ad77ac4b6808 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -602,6 +602,9 @@  void __drm_err(const char *format, ...);
 		drm_dev_printk(drm_ ? drm_->dev : NULL, KERN_DEBUG, fmt, ## __VA_ARGS__);	\
 })
 
+#define drm_dbg_ratelimited(drm, fmt, ...) \
+	__DRM_DEFINE_DBG_RATELIMITED(DRIVER, drm, fmt, ## __VA_ARGS__)
+
 #define drm_dbg_kms_ratelimited(drm, fmt, ...) \
 	__DRM_DEFINE_DBG_RATELIMITED(KMS, drm, fmt, ## __VA_ARGS__)