Re: DRM GUD driver debug logging

[Ruben Wauters <rubenru09@aol.com>]

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