[PATCH 42/43] drm/fbdev-generic: Convert to fbdev-ttm

[Jon Hunter]

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