Re: [PATCH 1/3] drm/i915/gen11+: First assume next platforms will inherit stuff

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

On Mon, Mar 11, 2019 at 11:12:06AM +0200, Jani Nikula wrote:
> On Fri, 08 Mar 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> > On Fri, Mar 08, 2019 at 02:39:36PM -0800, Rodrigo Vivi wrote:
> >>> Given that every platform so far has had different oa configurations,
> >>> that looks to be a hasty assumption that future platforms will be fixed.
> >>
> >>I know... But my hope is that at some point it gets stabilized.
> >>
> >>Well, or at least start with this so any other gen11 could reuse and
> >>gen12 would start with that and change later for >= gen12 and on...
> >
> > If it's different, there's no harm in making this assumption now and
> > then later change to cover the differences. Making it consistent
> > everywhere allows to more easily add the next platform.
> 
> At some point in time we deliberately made the if ladders and switch
> cases avoid matching future platforms, and liberally sprinkled
> MISSING_CASE() all over the place so we would catch each location that
> needed updating. So we wouldn't miss any.
> 
> This is the opposite of that approach. I'm not against the new approach,
> because we do have a lot of places where the existing code just works,
> but if you do know something will break, or will need to be updated to
> work, why not use the MISSING_CASE() there?

If you know in advance and you don't have any internal patch handling that
a MISSING_CASE is useful indeed.

Notice that there were cases that I intentionally left ICL behind.

I will double check if I think this or any of those cases deserves
the MISSING_CASES, but my current guess is that we are fine without it.

Also I'm wondering if we should make some macros:

if GEN_GREATER_THAN(dev_priv, 11) {
	icl_func(); 
}

#define GEN_GREATER_THAN(dev_priv, n) \
        if n > latest_implemented_gen() \
	   MISSING_CASE()

or something like that...

hmm but this wouldn't help us later, when gen12 is
already defined in some internal and we might forget anyways...

so maybe something like

#define GEN_GREATER_THAN(dev_priv, g) \
        if g != .gen \
	   DRM_DEBUG_DRIVER("Reusing %s from gen %d\n", __FUNCTION__, g)


But for now let's please just use this approach to save internal little
patches here and there.

> 
> 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