[PATCH] drm/xe: Sort again the info flags

Lucas De Marchi lucas.demarchi at intel.com
Wed Nov 20 14:16:00 UTC 2024


On Wed, Nov 20, 2024 at 02:16:17PM +0200, Jani Nikula wrote:
>On Tue, 19 Nov 2024, Lucas De Marchi <lucas.demarchi at intel.com> wrote:
>> On Tue, Nov 19, 2024 at 10:51:58AM +0000, Matthew Auld wrote:
>>>On 18/11/2024 22:33, Lucas De Marchi wrote:
>>>>Those flags are supposed to be kept sorted alphabetically. Unfortunately
>>>>it's a constant battle as new flags are added to the end or at random
>>>>places. Sort it again.
>>>>
>>>>v2: Include the other non-has_* 1-bit flags in the sort
>>>>
>>>>Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com> # v1
>>>>Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>>>>---
>>>>  drivers/gpu/drm/xe/xe_device_types.h | 32 ++++++++++++++++------------
>>>>  1 file changed, 18 insertions(+), 14 deletions(-)
>>>>
>>>>diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
>>>>index 8592f1b02db11..8b2b12daa49dd 100644
>>>>--- a/drivers/gpu/drm/xe/xe_device_types.h
>>>>+++ b/drivers/gpu/drm/xe/xe_device_types.h
>>>>@@ -294,14 +294,21 @@ struct xe_device {
>>>>  		/** @info.va_bits: Maximum bits of a virtual address */
>>>>  		u8 va_bits;
>>>
>>>Should we not document somewhere here that the below should be kept sorted?
>>
>> yes, probably. Also... we have the CI Hooks we should make better use of
>> too. First check if the patch series changed drivers/gpu/drm/xe/xe_device_types.h
>> at all and if it did, check that these flags are sorted. We should be
>> able to add some quick and dirty checks so we don't have to keep
>> changing it. Just need to make sure the output on what was
>> the error is helpful, otherwise people will just ignore it.
>>
>> Ryszard / Jani  any thought on that idea?
>
>Some random thoughts:
>
>We need more transparency on all of CI. I still have no clue of all the
>things that get run, where it's all hosted, how I could contribute

For the parts that I'm mentioning, it's all in

	http://gitlab.freedesktop.org/drm/xe/ci

1) You have the kernel config in kernel/kconfig
2) You have the hooks that run as CI.Hooks in hooks/

Changes to the config and hooks are accepted as MR to that repo.

The jenkins pipeline used to run that in the CI infra is not available,
but we can work with what is.

Do we want to document that somewhere? I can write something down.

>changes to it. I'm guessing maybe it's all out there somewhere, but I
>find it difficult to figure out. A check like this would need to be
>discoverable.

yep, agreed. That's why I said the error output needs to be improved,
otherwise people just get a wall of text and don't understand what's
going on.


>
>Something like this should be locally runnable. I know it's been hard to
>get people to run even checkpatch or sparse, but it's just dumb noise to
>get the first indication something's wrong in a CI result. And if you
>don't block further runs based on this, it just burns resources.
>
>On that last point, I think we should aim for reliable pass/fail, with
>fast fail halting further resource intensive stuff. E.g. checkpatch is
>not like this, we can't block on checkpatch failures, and due to this
>the results tend to be ignored and we merge stuff with checkpatch
>failures. Ditto with detailed checks like this, if it doesn't block
>merging, it's bound to get ignored at times. (Side note, maybe we should
>look into a limited checkpatch config that we could fail fast on?)

IMO we should turn all these "checks run on the build machine" rather
than "checks executed on the target hosts" part of the hooks infra....
because that is much more visible than scripts hidden inside the CI
pipeline. And then people can even run on their own machines.

Lucas De Marchi

>
>BR,
>Jani.
>
>
>-- 
>Jani Nikula, Intel


More information about the Intel-xe mailing list