On 23/10/2024 09:44, Jon Hunter wrote:
On 23/10/2024 07:43, Thomas Zimmermann wrote:
Hi
Am 22.10.24 um 17:36 schrieb Jon Hunter:
We'd turn a linker/modpost error into a compiler error. Likely makes
no difference. And AFAICT every driver that selects TTM also selects
TTM_HELPER. Drivers without TTM should not use this header.
Yes I also noted that all the current drivers select TTM_HELPER and
so we don't run into this. However, it still seems a bit odd that we
don't expose a proper stub if TTM_HELPER is disabled (especially
seeing as there is a stub defined).
It's not different from other headers AFAICT. For example, the TTM
headers don't guard any of their interfaces or provide stubs. The
guards and stubs only make sense if an interface really is optional
wrt some config token. That's not the case here wrt DRM_TTM_HELPER.
Just to be clear, there is a stub for this case which was added by this
patch ...
#ifdef CONFIG_DRM_FBDEV_EMULATION
void drm_fbdev_ttm_setup(struct drm_device *dev, unsigned int
preferred_bpp);
#else
static inline void drm_fbdev_ttm_setup(struct drm_device *dev, unsigned
int preferred_bpp)
{ }
#endif
The point I am trying to make is that this stub is only defined if
!CONFIG_DRM_FBDEV_EMULATION. However, the problem is that if
CONFIG_DRM_FBDEV_EMULATION is enabled, but CONFIG_DRM_TTM_HELPER is not
the stub is not defined, when it should be. Yes as we discussed this
does not impact any current users and so may be that is fine, but it
seems it would be better to have no stub at all, rather than one that is
defined for some configurations but not all cases where the actual
drm_fbdev_ttm_setup() function is not available. Again this whole thing
could be moot anyway, because this is going away completely from looking
at -next.
Unless what you are saying is that this is deliberate to catch any users
that don't enable CONFIG_DRM_TTM_HELPER where they should be?
Jon
--
nvpublic
