mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Thomas Zimmermann <tzimmermann@suse.de>
To: Ruben Wauters <rubenru09@aol.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: DRM GUD driver debug logging
Date: Sat, 19 Jul 2025 18:44:17 +0200	[thread overview]
Message-ID: <ffbc01f2-5571-4fba-ae73-86f959922bcb@suse.de> (raw)
In-Reply-To: <2a6afe3532235c7b76758163e2439e55c93df241.camel@aol.com>

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)


  reply	other threads:[~2025-07-19 16:44 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <2a6afe3532235c7b76758163e2439e55c93df241.camel.ref@aol.com>
2025-07-17 15:02 ` Ruben Wauters
2025-07-19 16:44   ` Thomas Zimmermann [this message]
2025-07-19 17:23     ` Ruben Wauters

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ffbc01f2-5571-4fba-ae73-86f959922bcb@suse.de \
    --to=tzimmermann@suse.de \
    --cc=airlied@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=rubenru09@aol.com \
    --cc=simona@ffwll.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®