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

[Jani Nikula]

On Mon, 11 Mar 2019, Rodrigo Vivi <rodrigo.vivi at intel.com> wrote:
> On Mon, Mar 11, 2019 at 11:12:06AM +0200, Jani Nikula wrote:
>> On Fri, 08 Mar 2019, Lucas De Marchi <lucas.demarchi at 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)

This would spam dmesg everywhere it's used for normal cases.

As to this "greater than" business, there were macros to do
this... before 5bc0e89ff1be ("drm/i915: Kill GEN_FOREVER"). :/

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

Okay, let's see what happens.


BR,
Jani.



-- 
Jani Nikula, Intel Open Source Graphics Center