Re: [Intel-gfx] [PATCH 16/16] drm/i915: Drop display.has_fpga_db from device info

From: "Souza, Jose" <jose.souza@intel.com>
To: "joonas.lahtinen@linux.intel.com"
	<joonas.lahtinen@linux.intel.com>,
	"tvrtko.ursulin@linux.intel.com" <tvrtko.ursulin@linux.intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"jani.nikula@linux.intel.com" <jani.nikula@linux.intel.com>
Subject: Re: [Intel-gfx] [PATCH 16/16] drm/i915: Drop display.has_fpga_db from device info
Date: Wed, 11 May 2022 13:56:43 +0000	[thread overview]
Message-ID: <97ae58c7a9d34021ab8950bdadb6b5d2910f5ec7.camel@intel.com> (raw)
In-Reply-To: <76397b0c-e4bc-1bd4-9620-7c677c01a004@linux.intel.com>

On Wed, 2022-05-11 at 08:39 +0100, Tvrtko Ursulin wrote:
> On 10/05/2022 08:41, Jani Nikula wrote:
> > On Tue, 10 May 2022, Joonas Lahtinen <joonas.lahtinen@linux.intel.com> wrote:
> > > Quoting Souza, Jose (2022-05-09 17:19:28)
> > > > On Mon, 2022-05-09 at 15:38 +0300, Joonas Lahtinen wrote:
> > > > > Quoting José Roberto de Souza (2022-05-07 16:28:50)
> > > > > > No need to have this parameter in intel_device_info struct
> > > > > > as this feature is supported by Broadwell, Haswell all platforms with
> > > > > > display version 9 or newer.
> > > > > 
> > > > > This is opposite of the direction we want to move to.
> > > > > 
> > > > > We want to embrace the has_xyz flags, instead of the macro trickery.
> > > > 
> > > > This ever growing flag definition is causing problems when defining new platforms.
> > > 
> > > The ever growing macros that change between kernel versions are much
> > > more painful than easily printable list per SKU.
> > > 
> > > Just to make it clear, a strict NACK from me for merging any patches
> > > into this direction.
> > 
> > I was hoping to start a discussion to reach consensus on this with my
> > mail [1], adding all maintainers in Cc, but merging started before the
> > discussion really even started.
> > 
> > Obviously no further patches on this are to be merged, and the question
> > now is really what to do with the ones that were. Revert?
> 
>  From the process standpoint strictly yes, but in practice I think there 
> is no rush.
> 
> The ones which got merged I definitely do not like are:
> 
> Rc6 - because it creates an inconsistency where rc6p remains a device 
> info flag.
> 
> DDI - because it is not 100% trivial and used from i915_irq.c. But a) I 
> am not sure it is truly on an irq path, and b) it is display code so not 
> my call anyway. (Affects the DP MST one as well by inheritance.)
> 
> The others are at best lukewarm to me - primarily because I am not 
> convinced there is a benefit to it all. One day the need might come to 
> move them back if some platforms drops support or something, which would 
> be more churn. And it is handy to see a consolidated description of a 
> platform in dmesg when looking at bugs. So just not sure it's an 
> improvement.

If platform feature list print is a must, we could print those features converted to platform and IP checks in intel_device_info_print_runtime().

> 
> Give there is much more conversions proposed I guess it may make sense 
> to revert until overall consensus is achieved, since it may be odd to 
> have a handful if we decide to stop there.
> 
> Regards,
> 
> Tvrtko