[PATCH v2] drm/amd/display: enable more strict compile checks

[Jani Nikula]

On Wed, 24 May 2023, Hamza Mahfooz <hamza.mahfooz at amd.com> wrote:
> + Kees
>
> On 5/24/23 15:50, Alex Deucher wrote:
>> On Wed, May 24, 2023 at 3:46 PM Felix Kuehling <felix.kuehling at amd.com> wrote:
>>>
>>> Sure, I think we tried enabling warnings as errors before and had to
>>> revert it because of weird compiler quirks or the variety of compiler
>>> versions that need to be supported.
>>>
>>> Alex, are you planning to upstream this, or is this only to enforce more
>>> internal discipline about not ignoring warnings?
>> 
>> I'd like to upstream it.  Upstream already has CONFIG_WERROR as a
>> config option, but it's been problematic to enable in CI because of
>> various breakages outside of the driver and in different compilers.
>> That said, I don't know how much trouble enabling it will cause with
>> various compilers in the wild.

FWIW, I think it's a bit scary to enable -Werror unconditionally. You'll
then need to take into account *all* compilers the kernel build
supports. I'd hate to fail the build for someone trying out a new
compiler that has some new warnings.

In i915, we've added our own kconfig for this that we tell our
developers to use and have enabled in our CI. It also depends on
!COMPILE_TEST to not get enabled by allmodconfig/allyesconfig configs.
The Makefile then has:

subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror

On a somewhat related side note, it would be great to be able to enable
the equivalent of W=1 on a subdir level. Now we need to add basically
the same stuff one by one. Cc: Masahiro.

BR,
Jani.


>> 
>> Alex
>> 
>>>
>>> Regards,
>>>     Felix
>>>
>>>
>>> On 2023-05-24 15:41, Russell, Kent wrote:
>>>>
>>>> [AMD Official Use Only - General]
>>>>
>>>>
>>>> (Adding Felix in CC)
>>>>
>>>> I’m a fan of adding it to KFD as well. Felix, can you foresee any
>>>> issues here?
>>>>
>>>> Kent
>>>>
>>>> *From:* amd-gfx <amd-gfx-bounces at lists.freedesktop.org> *On Behalf Of
>>>> *Ho, Kenny
>>>> *Sent:* Wednesday, May 24, 2023 3:23 PM
>>>> *To:* Alex Deucher <alexdeucher at gmail.com>; Mahfooz, Hamza
>>>> <Hamza.Mahfooz at amd.com>
>>>> *Cc:* Li, Sun peng (Leo) <Sunpeng.Li at amd.com>; Wentland, Harry
>>>> <Harry.Wentland at amd.com>; Pan, Xinhui <Xinhui.Pan at amd.com>; Siqueira,
>>>> Rodrigo <Rodrigo.Siqueira at amd.com>; linux-kernel at vger.kernel.org;
>>>> dri-devel at lists.freedesktop.org; amd-gfx at lists.freedesktop.org; Daniel
>>>> Vetter <daniel at ffwll.ch>; Deucher, Alexander
>>>> <Alexander.Deucher at amd.com>; David Airlie <airlied at gmail.com>; Koenig,
>>>> Christian <Christian.Koenig at amd.com>
>>>> *Subject:* Re: [PATCH v2] drm/amd/display: enable more strict compile
>>>> checks
>>>>
>>>> [AMD Official Use Only - General]
>>>>
>>>> [AMD Official Use Only - General]
>>>>
>>>> (+ Felix)
>>>>
>>>> Should we do the same for other modules under amd (amdkfd)?  I was
>>>> going to enable full kernel werror in the kconfig used by my CI but
>>>> this is probably better.
>>>>
>>>> Kenny
>>>>
>>>> ------------------------------------------------------------------------
>>>>
>>>> *From:*Alex Deucher <alexdeucher at gmail.com>
>>>> *Sent:* Wednesday, May 24, 2023 3:22 PM
>>>> *To:* Mahfooz, Hamza <Hamza.Mahfooz at amd.com>
>>>> *Cc:* amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>;
>>>> Li, Sun peng (Leo) <Sunpeng.Li at amd.com>; Ho, Kenny <Kenny.Ho at amd.com>;
>>>> Pan, Xinhui <Xinhui.Pan at amd.com>; Siqueira, Rodrigo
>>>> <Rodrigo.Siqueira at amd.com>; linux-kernel at vger.kernel.org
>>>> <linux-kernel at vger.kernel.org>; dri-devel at lists.freedesktop.org
>>>> <dri-devel at lists.freedesktop.org>; Daniel Vetter <daniel at ffwll.ch>;
>>>> Deucher, Alexander <Alexander.Deucher at amd.com>; David Airlie
>>>> <airlied at gmail.com>; Wentland, Harry <Harry.Wentland at amd.com>; Koenig,
>>>> Christian <Christian.Koenig at amd.com>
>>>> *Subject:* Re: [PATCH v2] drm/amd/display: enable more strict compile
>>>> checks
>>>>
>>>> On Wed, May 24, 2023 at 3:20 PM Hamza Mahfooz <hamza.mahfooz at amd.com>
>>>> wrote:
>>>>>
>>>>> Currently, there are quite a number of issues that are quite easy for
>>>>> the CI to catch, that slip through the cracks. Among them, there are
>>>>> unused variable and indentation issues. Also, we should consider all
>>>>> warnings to be compile errors, since the community will eventually end
>>>>> up complaining about them. So, enable -Werror, -Wunused and
>>>>> -Wmisleading-indentation for all kernel builds.
>>>>>
>>>>> Cc: Alex Deucher <alexander.deucher at amd.com>
>>>>> Cc: Harry Wentland <harry.wentland at amd.com>
>>>>> Cc: Kenny Ho <kenny.ho at amd.com>
>>>>> Signed-off-by: Hamza Mahfooz <hamza.mahfooz at amd.com>
>>>>> ---
>>>>> v2: fix grammatical error
>>>>> ---
>>>>>   drivers/gpu/drm/amd/display/Makefile | 2 ++
>>>>>   1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/display/Makefile
>>>> b/drivers/gpu/drm/amd/display/Makefile
>>>>> index 0d610cb376bb..3c44162ebe21 100644
>>>>> --- a/drivers/gpu/drm/amd/display/Makefile
>>>>> +++ b/drivers/gpu/drm/amd/display/Makefile
>>>>> @@ -26,6 +26,8 @@
>>>>>
>>>>>   AMDDALPATH = $(RELATIVE_AMD_DISPLAY_PATH)
>>>>>
>>>>> +subdir-ccflags-y += -Werror -Wunused -Wmisleading-indentation
>>>>> +
>>>>
>>>> Care to enable this for the rest of amdgpu as well?  Or send out an
>>>> additional patch to do that?  Either way:
>>>> Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
>>>>
>>>> Alex
>>>>
>>>>>   subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/inc/
>>>>>   subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/inc/hw
>>>>>   subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/clk_mgr
>>>>> --
>>>>> 2.40.1
>>>>>
>>>>

-- 
Jani Nikula, Intel Open Source Graphics Center