[Intel-xe] [PATCH v2 02/11] drm/xe: Sort includes
Lucas De Marchi
lucas.demarchi at intel.com
Fri Feb 17 15:09:49 UTC 2023
On Fri, Feb 17, 2023 at 01:39:31PM +0200, Jani Nikula wrote:
>On Thu, 16 Feb 2023, Lucas De Marchi <lucas.demarchi at intel.com> wrote:
>> Sort includes and split them in blocks:
>>
>> 1) .h corresponding to the .c. Example: xe_bb.c should have a "#include
>> "xe_bb.h" first.
>
>Personally, I've never liked this, because the local headers could have
>stuff that impacts global headers. And if there are conflicts, you kind
>of want to have your local one look like the culprit (although modern
>compilers are pretty good at showing both sites).
>
>Note that to keep headers self-contained, this is not enough for headers
>not associated with a .c, which is why we have the HDRTEST rule in i915.
Although I like the hdrtest and wouldn't replace it, I think having a
convention like this is good - it also solves most of the issues
wrt selfcontained headers while not requiring any special setup on dev
side: if every .c/.h follow the same structure, it's easier to maintain
the consistency.
>
>> 2) #include <linux/...>
>> 3) #include <drm/...>
>> 4) local includes
>> 5) i915 includes
>>
>> This is accomplished by running
>> `clang-format --style=file -i --sort-includes drivers/gpu/drm/xe/*.c`
>> and ignoring all the changes after the includes. There are also some
>> manual tweaks to split the blocks.
>
>Do you have the style file somewhere?
.clang-format. It's in the kernel source tree, added in
commit d4ef8d3ff005 ("clang-format: add configuration file")
>
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_bb.c | 4 ++--
>> drivers/gpu/drm/xe/xe_bo.c | 2 --
>> drivers/gpu/drm/xe/xe_bo_evict.c | 2 +-
>> drivers/gpu/drm/xe/xe_debugfs.c | 2 +-
>> drivers/gpu/drm/xe/xe_device.c | 9 ++++-----
>> drivers/gpu/drm/xe/xe_display.c | 12 ++++++------
>> drivers/gpu/drm/xe/xe_dma_buf.c | 8 +++-----
>> drivers/gpu/drm/xe/xe_engine.c | 4 ++--
>> drivers/gpu/drm/xe/xe_exec.c | 2 +-
>> drivers/gpu/drm/xe/xe_execlist.c | 9 ++++-----
>> drivers/gpu/drm/xe/xe_force_wake.c | 4 ++--
>> drivers/gpu/drm/xe/xe_ggtt.c | 7 +++----
>> drivers/gpu/drm/xe/xe_gt.c | 2 +-
>> drivers/gpu/drm/xe/xe_gt_clock.c | 8 ++++----
>> drivers/gpu/drm/xe/xe_gt_debugfs.c | 2 +-
>> drivers/gpu/drm/xe/xe_gt_mcr.c | 2 +-
>> drivers/gpu/drm/xe/xe_gt_pagefault.c | 2 +-
>> drivers/gpu/drm/xe/xe_gt_sysfs.c | 4 +++-
>> drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c | 2 +-
>> drivers/gpu/drm/xe/xe_gt_topology.c | 2 +-
>> drivers/gpu/drm/xe/xe_guc.c | 13 +++++++------
>> drivers/gpu/drm/xe/xe_guc_ads.c | 5 +++--
>> drivers/gpu/drm/xe/xe_guc_ct.c | 4 ++--
>> drivers/gpu/drm/xe/xe_guc_debugfs.c | 2 +-
>> drivers/gpu/drm/xe/xe_guc_hwconfig.c | 2 +-
>> drivers/gpu/drm/xe/xe_guc_log.c | 2 +-
>> drivers/gpu/drm/xe/xe_guc_pc.c | 12 +++++++-----
>> drivers/gpu/drm/xe/xe_guc_submit.c | 6 +++---
>> drivers/gpu/drm/xe/xe_huc.c | 2 +-
>> drivers/gpu/drm/xe/xe_huc_debugfs.c | 2 +-
>> drivers/gpu/drm/xe/xe_hw_engine.c | 3 +--
>> drivers/gpu/drm/xe/xe_hw_fence.c | 1 -
>> drivers/gpu/drm/xe/xe_irq.c | 5 +++--
>> drivers/gpu/drm/xe/xe_lrc.c | 7 +++----
>> drivers/gpu/drm/xe/xe_migrate.c | 11 ++++++-----
>> drivers/gpu/drm/xe/xe_mmio.c | 3 +--
>> drivers/gpu/drm/xe/xe_mocs.c | 4 ++--
>> drivers/gpu/drm/xe/xe_module.c | 1 +
>> drivers/gpu/drm/xe/xe_pci.c | 5 ++---
>> drivers/gpu/drm/xe/xe_pcode.c | 10 ++++------
>> drivers/gpu/drm/xe/xe_pm.c | 4 ++--
>> drivers/gpu/drm/xe/xe_preempt_fence.c | 2 +-
>> drivers/gpu/drm/xe/xe_pt.c | 4 ++--
>> drivers/gpu/drm/xe/xe_query.c | 11 ++++++-----
>> drivers/gpu/drm/xe/xe_reg_sr.c | 5 ++---
>> drivers/gpu/drm/xe/xe_reg_whitelist.c | 7 +++----
>> drivers/gpu/drm/xe/xe_ring_ops.c | 4 ++--
>> drivers/gpu/drm/xe/xe_rtp.c | 1 -
>> drivers/gpu/drm/xe/xe_sa.c | 3 ++-
>> drivers/gpu/drm/xe/xe_sched_job.c | 1 -
>> drivers/gpu/drm/xe/xe_sync.c | 5 +++--
>> drivers/gpu/drm/xe/xe_ttm_gtt_mgr.c | 2 +-
>> drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c | 2 +-
>> drivers/gpu/drm/xe/xe_ttm_vram_mgr.c | 2 +-
>> drivers/gpu/drm/xe/xe_tuning.c | 2 +-
>> drivers/gpu/drm/xe/xe_uc.c | 4 ++--
>> drivers/gpu/drm/xe/xe_vm.c | 2 +-
>> drivers/gpu/drm/xe/xe_vm_madvise.c | 8 +++++---
>> drivers/gpu/drm/xe/xe_wopcm.c | 2 +-
>> 59 files changed, 128 insertions(+), 132 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_bb.c b/drivers/gpu/drm/xe/xe_bb.c
>> index 8b9209571fd0..a25079d4e710 100644
>> --- a/drivers/gpu/drm/xe/xe_bb.c
>> +++ b/drivers/gpu/drm/xe/xe_bb.c
>> @@ -2,12 +2,12 @@
>> /*
>> * Copyright © 2022 Intel Corporation
>> */
>> -
>
>Nitpick, throughout the series, I would leave a blank line after the
>SPDX and copyright boilerplate, regardless of whether it's an #include
>or header guard #ifdef that follows. They're not comments about what
>follows, which is what it feels like when they're directly attached to
>something that follows without a blank line.
ok
>
>> #include "xe_bb.h"
>> -#include "xe_sa.h"
>> +
>> #include "xe_device.h"
>> #include "xe_engine_types.h"
>> #include "xe_hw_fence.h"
>> +#include "xe_sa.h"
>> #include "xe_sched_job.h"
>> #include "xe_vm_types.h"
>>
>
>...
>
>> diff --git a/drivers/gpu/drm/xe/xe_uc.c b/drivers/gpu/drm/xe/xe_uc.c
>> index 7886c8b85397..a27e6c1a6aff 100644
>> --- a/drivers/gpu/drm/xe/xe_uc.c
>> +++ b/drivers/gpu/drm/xe/xe_uc.c
>> @@ -3,13 +3,13 @@
>> * Copyright © 2022 Intel Corporation
>> */
>>
>> +#include "xe_uc.h"
>> #include "xe_device.h"
>> -#include "xe_huc.h"
>> #include "xe_gt.h"
>> #include "xe_guc.h"
>> #include "xe_guc_pc.h"
>> #include "xe_guc_submit.h"
>> -#include "xe_uc.h"
>> +#include "xe_huc.h"
>> #include "xe_uc_fw.h"
>> #include "xe_wopcm.h"
>
>Here, having the .h corresponding to the .c just looks like it's not
>properly sorted.
just because I failed to split the block as Matt Auld noticed
thanks
Lucas De Marchi
>
>BR,
>Jani.
>
>
>--
>Jani Nikula, Intel Open Source Graphics Center
More information about the Intel-xe
mailing list