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. 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. 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? BR, Jani. -- Jani Nikula, Intel