DRM GUD driver debug logging

DRM GUD driver debug logging

[Ruben Wauters]

Hello

I was taking a look at the code for the gud driver. I am aware this
driver was recently orphaned and I wish to get up to speed on it, and
maybe with enough learning and work I can take over maintainance of it
in the future.

I noticed that in the function gud_disconnect in gud_driv.c on like 623
there is the following code

	drm_dbg(drm, "%s:\n", __func__);

checkpatch.pl marks this as unnecessary ftrace like logging, and
suggests to use ftrace instead. Since (as far as I can tell) this
effectively just prints the function name, would it not be better to
just use ftrace for debugging and remove this call all together?

While it isn't actively *harming* the code as such, it does seem a bit
unnecessary.

I'd like to know the DRM maintainers opinions. I know this particular
driver does not have a maintainer dedicated to it, so I'd like to know
the opinion of those that maintain the subsystem, and anyone else that
has any opinion.

Thank you

Ruben Wauters

Re: DRM GUD driver debug logging

[Thomas Zimmermann]

Hi

Am 17.07.25 um 17:02 schrieb Ruben Wauters:
> Hello
>
> I was taking a look at the code for the gud driver. I am aware this
> driver was recently orphaned and I wish to get up to speed on it, and
> maybe with enough learning and work I can take over maintainance of it
> in the future.

That's fantastic!

There's https://github.com/notro/gud?tab=readme-ov-file and 
https://github.com/notro/gud/wiki for more information about the gud 
protocol and driver.

>
> I noticed that in the function gud_disconnect in gud_driv.c on like 623
> there is the following code
>
> 	drm_dbg(drm, "%s:\n", __func__);
>
> checkpatch.pl marks this as unnecessary ftrace like logging, and
> suggests to use ftrace instead. Since (as far as I can tell) this
> effectively just prints the function name, would it not be better to
> just use ftrace for debugging and remove this call all together?

I'm not a great fan of these types of debugging code. We already have 
this in the DRM core/helpers. Whatever drivers print for debugging 
should be more helpful than just the function name. So IMHO this can be 
removed.

>
> While it isn't actively *harming* the code as such, it does seem a bit
> unnecessary.
>
> I'd like to know the DRM maintainers opinions. I know this particular
> driver does not have a maintainer dedicated to it, so I'd like to know
> the opinion of those that maintain the subsystem, and anyone else that
> has any opinion.

If you want to do meaningful work on the driver, you could replace 
struct drm_simple_display_pipe with the real DRM helpers.  The struct is 
an artifact from older driver designs, but is now obsolete. Drivers are 
supposed to be converted to DRM atomic helpers. For an experienced dev, 
it's a copy-past job. For a newcomer, it's a nice introduction to the 
DRM code. If you want to take a look, you can study commit 4963049ea1ae 
("drm/hyperv: Replace simple-KMS with regular atomic helpers"), which 
recently did this conversion for the hyperv driver.

Best regards
Thomas

>
> Thank you
>
> Ruben Wauters

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: DRM GUD driver debug logging

[Ruben Wauters]

On Sat, 2025-07-19 at 18:44 +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 17.07.25 um 17:02 schrieb Ruben Wauters:
> > Hello
> > 
> > I was taking a look at the code for the gud driver. I am aware this
> > driver was recently orphaned and I wish to get up to speed on it,
> > and
> > maybe with enough learning and work I can take over maintainance of
> > it
> > in the future.
> 
> That's fantastic!
> 
> There's https://github.com/notro/gud?tab=readme-ov-file and 
> https://github.com/notro/gud/wiki for more information about the gud 
> protocol and driver.

Thank you! I will take a look at it

I'm not sure how much experience exactly I'll need to be confident in
taking over maintainership, but I'll take it as it comes, and hopefully
learn on the way :)
> 
> > 
> > I noticed that in the function gud_disconnect in gud_driv.c on like
> > 623
> > there is the following code
> > 
> > 	drm_dbg(drm, "%s:\n", __func__);
> > 
> > checkpatch.pl marks this as unnecessary ftrace like logging, and
> > suggests to use ftrace instead. Since (as far as I can tell) this
> > effectively just prints the function name, would it not be better
> > to
> > just use ftrace for debugging and remove this call all together?
> 
> I'm not a great fan of these types of debugging code. We already have
> this in the DRM core/helpers. Whatever drivers print for debugging 
> should be more helpful than just the function name. So IMHO this can
> be 
> removed.
> 
OK, first patch I'll send will probably remove this

> > 
> > While it isn't actively *harming* the code as such, it does seem a
> > bit
> > unnecessary.
> > 
> > I'd like to know the DRM maintainers opinions. I know this
> > particular
> > driver does not have a maintainer dedicated to it, so I'd like to
> > know
> > the opinion of those that maintain the subsystem, and anyone else
> > that
> > has any opinion.
> 
> If you want to do meaningful work on the driver, you could replace 
> struct drm_simple_display_pipe with the real DRM helpers.  The struct
> is 
> an artifact from older driver designs, but is now obsolete. Drivers
> are 
> supposed to be converted to DRM atomic helpers. For an experienced
> dev, 
> it's a copy-past job. For a newcomer, it's a nice introduction to the
> DRM code. If you want to take a look, you can study commit
> 4963049ea1ae 
> ("drm/hyperv: Replace simple-KMS with regular atomic helpers"), which
> recently did this conversion for the hyperv driver.

This is very helpful, thank you. One of the problems I've had with
kernel dev is knowing where to start with making meaningful changes.
I've been mainly sticking to cleanup and janitor tasks since, so having
this pinpointed to me is extremely helpful.
> 
> Best regards
> Thomas
> 
> > 
> > Thank you
> > 
> > Ruben Wauters