Re: [Intel-gfx] [PATCH v4] x86/quirks: Replace QFLAG_APPLY_ONCE with static locals

[Lucas De Marchi <lucas.demarchi@intel.com>]

On Wed, Jan 12, 2022 at 06:08:05PM -0600, Bjorn Helgaas wrote:
>On Wed, Jan 12, 2022 at 03:30:43PM -0800, Lucas De Marchi wrote:
>> The flags are only used to mark a quirk to be called once and nothing
>> else. Also, that logic may not be appropriate if the quirk wants to
>> do additional filtering and set quirk as applied by itself.
>>
>> So replace the uses of QFLAG_APPLY_ONCE with static local variables in
>> the few quirks that use this logic and remove all the flags logic.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>
>
>Only occurred to me now, but another, less intrusive approach would be
>to just remove QFLAG_APPLY_ONCE from intel_graphics_quirks() and do
>its bookkeeping internally, e.g.,

that is actually what I suggested after your comment in v2: this would
be the first patch with "minimal fix". But then to keep it consistent
with the other calls to follow up with additional patches on top
converting them as well.  Maybe what I wrote wasn't clear in the
direction? Copying it here:

	1) add the static local only to intel graphics quirk  and remove the
	flag from this item
	2 and 3) add the static local to other functions and remove the flag
	from those items
	4) remove the flag from the table, the defines and its usage.
	5) fix the coding style (to be clear, it's already wrong, not
	something wrong introduced here... maybe could be squashed in (4)?)

Lucas De Marchi

>
>diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
>index 391a4e2b8604..7b655004e5fd 100644
>--- a/arch/x86/kernel/early-quirks.c
>+++ b/arch/x86/kernel/early-quirks.c
>@@ -587,10 +587,14 @@ intel_graphics_stolen(int num, int slot, int func,
>
> static void __init intel_graphics_quirks(int num, int slot, int func)
> {
>+	static bool stolen __initdata = false;
> 	const struct intel_early_ops *early_ops;
> 	u16 device;
> 	int i;
>
>+	if (stolen)
>+		return;
>+
> 	device = read_pci_config_16(num, slot, func, PCI_DEVICE_ID);
>
> 	for (i = 0; i < ARRAY_SIZE(intel_early_ids); i++) {
>@@ -602,6 +606,7 @@ static void __init intel_graphics_quirks(int num, int slot, int func)
> 		early_ops = (typeof(early_ops))driver_data;
>
> 		intel_graphics_stolen(num, slot, func, early_ops);
>+		stolen = true;
>
> 		return;
> 	}
>@@ -703,7 +708,7 @@ static struct chipset early_qrk[] __initdata = {
> 	{ PCI_VENDOR_ID_INTEL, 0x3406, PCI_CLASS_BRIDGE_HOST,
> 	  PCI_BASE_CLASS_BRIDGE, 0, intel_remapping_check },
> 	{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA, PCI_ANY_ID,
>-	  QFLAG_APPLY_ONCE, intel_graphics_quirks },
>+	  0, intel_graphics_quirks },
> 	/*
> 	 * HPET on the current version of the Baytrail platform has accuracy
> 	 * problems: it will halt in deep idle state - so we disable it.