Hi
Am 03.12.24 um 15:19 schrieb Jocelyn Falempe:
On 03/12/2024 15:08, Jani Nikula wrote:
On Tue, 03 Dec 2024, Jocelyn Falempe <jfalempe@xxxxxxxxxx> wrote:
On 03/12/2024 12:06, Jani Nikula wrote:
On Fri, 15 Nov 2024, Jocelyn Falempe <jfalempe@xxxxxxxxxx> wrote:
Move the color conversions, blit and fill functions to drm_draw.c,
so that they can be re-used by drm_log.
drm_draw is internal to the drm subsystem, and shouldn't be used by
gpu drivers.
I started looking at this in patch 2:
+#include "../drm_draw.h"
I think we should avoid #includes with ../ like this.
Sure, I've added it in v8, after the clients moved to drm/clients/, but
I didn't think much about it.
Either drm_draw.h belongs in include/drm, or maybe clients/Makefile
needs to add subdir-ccflags-y += -I$(src)/.. or something like that?
If it's supposed to be internal, I guess the latter, but then the
current convention is to have _internal.h suffix. All drm headers
under
drivers/ have that.
ok, I can rename drm_draw.h to drm_draw_internal.h, and add the include
in the Makefile.
Is this the first drm subsystem internal thing that's a separate
module?
Should we use EXPORT_SYMBOL_NS() and MODULE_IMPORT_NS() to enforce it
being internal?
It's not really a separate module, as it's built in the core drm
module.
(the reason is that it's used by drm_panic too, which must be in the
core drm module).
Right, sorry, I got confused and looked at this the other way round.
drm_draw is part of drm.ko, drm log is part of drm_client_lib.ko, and
uses drm_draw, right?
Yes, that's it.
So is drm_draw really internal if drm client uses it?
For me "internal" includes drm clients and kms helpers, but yes, it's
not really clear.
I don't really deeply care either way, but it bugs me when it's
somewhere in between. :)
Either include/drm/drm_draw.h or drivers/gpu/drm/drm_draw_internal.h,
the latter *possibly* with symbol namespace, but not a big deal.
The other reason for "internal" is that we should find a way to merge
drm_draw, drm_format_helper, and some vkms conversion helper, that
does similar things. So the less users, the easier it will be.
Exactly. 'internal' definitely means not-drivers. And the last thing we
want is DRM drivers that draw framebuffers by themselves. When we unify
and harmonize the various blit and draw helpers, we can still loosen the
requirements if necessary.
Best regards
Thomas
Best regards,
--
--
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)
