Re: [PATCH] drm/i915: Simplify has_sagv

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Oct 24, 2018 at 01:13:51PM +0300, Jani Nikula wrote:
> On Tue, 23 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > But I thought while doing this I could consolidade it along with all
> > the other has_feature cases.
> >
> > I believe we should either have everything as info.has_feature or everything
> > as has_feature().
> >
> > for instance if we end up ever having 2 platforms of same gen where
> > one has_sagv and the other doesn't we would have that in the platform
> > definition while making us to define another codename and add it here
> > or even worse if we don't have a codename available like CNL_WITH_PORT_F :/
> 
> If that is to be in device info, it doesn't require an extra codename,
> it requires an extra device info with the flag.
> 
> This ties to the goal of making dev_priv->info a pointer to the static
> const data in i915_pci.c i.e. making ->info truly const. There's three
> categories of info:
> 
>  1) immutable device properties
>  2) properties set once during probe, immutable afterwards
>  3) runtime
> 
> Currently we more or less happily conflate these, along with some module
> parameters too. The mkwrite_device_info() use has profilerated much
> wider than it was ever intended; we need to nuke that.
> 
> We also have HAS_FOO() and IS_FOO() macros that do checks on pci id or
> gen or platform or a combination of them. It's a mess, and it's not
> getting better without conscious effort.

I agree with all that you said.

We probably just disagree on how to make HAS_FOO(dev_priv) IS_FOO(dev_priv)
better.

In my opinion we should move towards adding all immutable device
properties that describe and differentiate the platforms inside
device info. With that consolidated there:

- code gets uniform
- minimize gen and platform checks spread all over the code
- it gets easier to add platforms

But nevermind, I won't insist on this path ;)

> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx