Re: [Intel-gfx] [PATCH v5 1/5] x86/quirks: Fix stolen detection with integrated + discrete GPU

[Bjorn Helgaas <helgaas@kernel.org>]

On Tue, Jan 18, 2022 at 06:26:48PM +0100, Borislav Petkov wrote:
> On Tue, Jan 18, 2022 at 08:36:56AM -0800, Lucas De Marchi wrote:
> > I had the impression the subject/title should be imperative, with it
> > more relaxed in the body. It seems we have one more difference among
> > subsystems and I will adapt on next submissions to x86.
> 
> We have written it down properly, in case it explains it better:
> 
> "The tip tree maintainers set value on following these rules, especially
> on the request to write changelogs in imperative mood and not
> impersonating code or the execution of it. This is not just a whim of
> the maintainers. Changelogs written in abstract words are more precise
> and tend to be less confusing than those written in the form of novels."
> 
> from Documentation/process/maintainer-tip.rst

Thanks for writing this down!  I do the same for PCI.  I suspect this
is a pretty conservative style that would be acceptable tree-wide even
if not required everywhere.

> > Although in the review Bjorn suggested just splitting the commit, it was
> > also mentioned that the PCI subsystem has no such logic in its
> > equivalent pci_do_fixups(): a quirk/fixup needing that should instead
> > use a static local.
> 
> Why?

I don't really care much one way or the other.  I think the simplest
approach is to remove QFLAG_APPLY_ONCE from intel_graphics_quirks()
and do nothing else, as I suggested here:

  https://lore.kernel.org/r/20220113000805.GA295089@bhelgaas

Unfortunately that didn't occur to me until I'd already suggested more
complicated things that no longer seem worthwhile to me.

The static variable might be ugly, but it does seem to be what
intel_graphics_quirks() wants -- a "do this at most once per system
but we don't know exactly which device" situation.

> There's perfectly nice ->flags there for exactly stuff like that. static
> vars are ugly and should be avoided if possible.

Bjorn