Re: [Intel-gfx] [PATCH 3/5] drm/i915: merge gen checks to use range
On Fri, Nov 02, 2018 at 03:28:00PM -0700, Lucas De Marchi wrote: > On Fri, Nov 02, 2018 at 03:12:18PM -0700, Rodrigo Vivi wrote: > > On Fri, Nov 02, 2018 at 12:47:28PM -0700, Lucas De Marchi wrote: > > > On Fri, Nov 02, 2018 at 12:19:13PM -0700, Rodrigo Vivi wrote: > > > > On Fri, Nov 02, 2018 at 11:10:10AM -0700, Lucas De Marchi wrote: > > > > > On Thu, Nov 01, 2018 at 11:31:25AM +, Tvrtko Ursulin wrote: > > > > > > > > > > > > On 01/11/2018 08:35, Lucas De Marchi wrote: > > > > > > > Instead of using several IS_GEN(), it's possible to pass the > > > > > > > range as argument. By code inspection these were the ranges deemed > > > > > > > necessary for spatch: > > > > > > > > > > > > > > @@ > > > > > > > expression e; > > > > > > > @@ > > > > > > > ( > > > > > > > - IS_GEN(e, 3) || IS_GEN(e, 4) > > > > > > > + IS_GEN(e, 3, 4) > > > > > > > | > > > > > > > - IS_GEN(e, 5) || IS_GEN(e, 6) > > > > > > > + IS_GEN(e, 5, 6) > > > > > > > | > > > > > > > - IS_GEN(e, 6) || IS_GEN(e, 7) > > > > > > > + IS_GEN(e, 6, 7) > > > > > > > | > > > > > > > - IS_GEN(e, 7) || IS_GEN(e, 8) > > > > > > > + IS_GEN(e, 7, 8) > > > > > > > | > > > > > > > - IS_GEN(e, 8) || IS_GEN(e, 9) > > > > > > > + IS_GEN(e, 8, 9) > > > > > > > | > > > > > > > - IS_GEN(e, 10) || IS_GEN(e, 9) > > > > > > > + IS_GEN(e, 9, 10) > > > > > > > | > > > > > > > - IS_GEN(e, 9) || IS_GEN(e, 10) > > > > > > > + IS_GEN(e, 9, 10) > > > > > > > ) > > > > > > > > > > > > > > Signed-off-by: Lucas De Marchi > > > > > > > --- > > > > > > > drivers/gpu/drm/i915/i915_debugfs.c| 6 +++--- > > > > > > > drivers/gpu/drm/i915/i915_gpu_error.c | 2 +- > > > > > > > drivers/gpu/drm/i915/i915_perf.c | 2 +- > > > > > > > drivers/gpu/drm/i915/intel_crt.c | 2 +- > > > > > > > drivers/gpu/drm/i915/intel_device_info.c | 2 +- > > > > > > > drivers/gpu/drm/i915/intel_display.c | 2 +- > > > > > > > drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- > > > > > > > drivers/gpu/drm/i915/intel_fifo_underrun.c | 2 +- > > > > > > > drivers/gpu/drm/i915/intel_pipe_crc.c | 4 ++-- > > > > > > > drivers/gpu/drm/i915/intel_uncore.c| 6 +++--- > > > > > > > 10 files changed, 15 insertions(+), 15 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > > > > > > b/drivers/gpu/drm/i915/i915_debugfs.c > > > > > > > index 28d95f9d0b0e..f2fbc016bd7f 100644 > > > > > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > > > > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > > > > > > @@ -2030,7 +2030,7 @@ static int i915_swizzle_info(struct > > > > > > > seq_file *m, void *data) > > > > > > > seq_printf(m, "bit6 swizzle for Y-tiling = %s\n", > > > > > > > > > > > > > > swizzle_string(dev_priv->mm.bit_6_swizzle_y)); > > > > > > > - if (IS_GEN(dev_priv, 3) || IS_GEN(dev_priv, 4)) { > > > > > > > + if (IS_GEN(dev_priv, 3, 4)) { > > > > > > > > > > > > I can see value in it but think it would read better with > > > > > > IS_GEN_RANGE. > > > > > > > > > > Ok, it seems there's a rough consensus of s/IS_GEN/IS_GEN_RANGE/ an > > > > > then > > > > > bring the patches that make sense here. There was a recent patch from > > > > > Rodrigo > > > > > doing that. I'll include it in next version. > > > > > > > > I liked the double args idea but after reading I believe > > > > it gets clear IS_GEN_RANGE. > > > > > > > > > > > > > > > > > > > > > Are there any cases of or-ed IS_GEN checks with something > > > > > > sandwiched in > > > > > > between then, which the above spatch would miss? > > > > > > > > > > By manually inspecting the result of ``git grep -ne "IS_GEN(.*" -- > > > > > drivers/gpu/drm/i915/`` > > > > > I didn't find any. The only thing I found was a missed case for gen3 > > > > > || gen2 > > > > > that was not covered by the spatch. > > > > > > > > > > > > > > > > > How many non-consecutive IS_GEN gen checks are there? To give us > > > > > > some ideas > > > > > > if the usual pattern is range, or perhaps checks against a list of > > > > > > gens also > > > > > > exists? (Gut feeling says no.) > > > > > > > > > > only cases of <=, <, >=, >. > > > > > > > > For these cases on patches 4 and 5:: > > > > > > > > What about converting all < n to <= n-1 and all > n to >= n + 1 > > > > get FORVER back and introduce IS_GEN_UNTIL ? > > > > > > > > IS_GEN_UNTIL(dev_priv, e) > > > > IS_GEN_RANGE(dev_priv, s, FOREVER) > > > > > > > > so we can also kill INTEL_GEN. > > > > > > > > Another different idea on top of that. > > > > > > > > What about removing all "IS_"? > > > > > > > > so end result could be something like that: > > > > > > > > INTEL_GEN(dev_priv, n) > > > > DISPLAY_GEN(dev_priv, n) > > > > INTEL_GEN_RANGE(dev_priv, s, e) #or e = FOREVER > > > > DISPLAY_GEN_RANGE(dev_priv, s, e) #or e = FOREVER > > > > INTEL_GEN_UNTIL(dev_priv, e) > > > > DISPLAY_GEN_UNTIL(dev_priv, e) > > > > > > > > (maybe s/INTEL/GT) > > > > > > I like it.
Re: [Intel-gfx] [PATCH 3/5] drm/i915: merge gen checks to use range
On Fri, Nov 02, 2018 at 03:12:18PM -0700, Rodrigo Vivi wrote: > On Fri, Nov 02, 2018 at 12:47:28PM -0700, Lucas De Marchi wrote: > > On Fri, Nov 02, 2018 at 12:19:13PM -0700, Rodrigo Vivi wrote: > > > On Fri, Nov 02, 2018 at 11:10:10AM -0700, Lucas De Marchi wrote: > > > > On Thu, Nov 01, 2018 at 11:31:25AM +, Tvrtko Ursulin wrote: > > > > > > > > > > On 01/11/2018 08:35, Lucas De Marchi wrote: > > > > > > Instead of using several IS_GEN(), it's possible to pass the > > > > > > range as argument. By code inspection these were the ranges deemed > > > > > > necessary for spatch: > > > > > > > > > > > > @@ > > > > > > expression e; > > > > > > @@ > > > > > > ( > > > > > > - IS_GEN(e, 3) || IS_GEN(e, 4) > > > > > > + IS_GEN(e, 3, 4) > > > > > > | > > > > > > - IS_GEN(e, 5) || IS_GEN(e, 6) > > > > > > + IS_GEN(e, 5, 6) > > > > > > | > > > > > > - IS_GEN(e, 6) || IS_GEN(e, 7) > > > > > > + IS_GEN(e, 6, 7) > > > > > > | > > > > > > - IS_GEN(e, 7) || IS_GEN(e, 8) > > > > > > + IS_GEN(e, 7, 8) > > > > > > | > > > > > > - IS_GEN(e, 8) || IS_GEN(e, 9) > > > > > > + IS_GEN(e, 8, 9) > > > > > > | > > > > > > - IS_GEN(e, 10) || IS_GEN(e, 9) > > > > > > + IS_GEN(e, 9, 10) > > > > > > | > > > > > > - IS_GEN(e, 9) || IS_GEN(e, 10) > > > > > > + IS_GEN(e, 9, 10) > > > > > > ) > > > > > > > > > > > > Signed-off-by: Lucas De Marchi > > > > > > --- > > > > > > drivers/gpu/drm/i915/i915_debugfs.c| 6 +++--- > > > > > > drivers/gpu/drm/i915/i915_gpu_error.c | 2 +- > > > > > > drivers/gpu/drm/i915/i915_perf.c | 2 +- > > > > > > drivers/gpu/drm/i915/intel_crt.c | 2 +- > > > > > > drivers/gpu/drm/i915/intel_device_info.c | 2 +- > > > > > > drivers/gpu/drm/i915/intel_display.c | 2 +- > > > > > > drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- > > > > > > drivers/gpu/drm/i915/intel_fifo_underrun.c | 2 +- > > > > > > drivers/gpu/drm/i915/intel_pipe_crc.c | 4 ++-- > > > > > > drivers/gpu/drm/i915/intel_uncore.c| 6 +++--- > > > > > > 10 files changed, 15 insertions(+), 15 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > > > > > b/drivers/gpu/drm/i915/i915_debugfs.c > > > > > > index 28d95f9d0b0e..f2fbc016bd7f 100644 > > > > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > > > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > > > > > @@ -2030,7 +2030,7 @@ static int i915_swizzle_info(struct seq_file > > > > > > *m, void *data) > > > > > > seq_printf(m, "bit6 swizzle for Y-tiling = %s\n", > > > > > >swizzle_string(dev_priv->mm.bit_6_swizzle_y)); > > > > > > - if (IS_GEN(dev_priv, 3) || IS_GEN(dev_priv, 4)) { > > > > > > + if (IS_GEN(dev_priv, 3, 4)) { > > > > > > > > > > I can see value in it but think it would read better with > > > > > IS_GEN_RANGE. > > > > > > > > Ok, it seems there's a rough consensus of s/IS_GEN/IS_GEN_RANGE/ an then > > > > bring the patches that make sense here. There was a recent patch from > > > > Rodrigo > > > > doing that. I'll include it in next version. > > > > > > I liked the double args idea but after reading I believe > > > it gets clear IS_GEN_RANGE. > > > > > > > > > > > > > > > > > Are there any cases of or-ed IS_GEN checks with something sandwiched > > > > > in > > > > > between then, which the above spatch would miss? > > > > > > > > By manually inspecting the result of ``git grep -ne "IS_GEN(.*" -- > > > > drivers/gpu/drm/i915/`` > > > > I didn't find any. The only thing I found was a missed case for gen3 || > > > > gen2 > > > > that was not covered by the spatch. > > > > > > > > > > > > > > How many non-consecutive IS_GEN gen checks are there? To give us some > > > > > ideas > > > > > if the usual pattern is range, or perhaps checks against a list of > > > > > gens also > > > > > exists? (Gut feeling says no.) > > > > > > > > only cases of <=, <, >=, >. > > > > > > For these cases on patches 4 and 5:: > > > > > > What about converting all < n to <= n-1 and all > n to >= n + 1 > > > get FORVER back and introduce IS_GEN_UNTIL ? > > > > > > IS_GEN_UNTIL(dev_priv, e) > > > IS_GEN_RANGE(dev_priv, s, FOREVER) > > > > > > so we can also kill INTEL_GEN. > > > > > > Another different idea on top of that. > > > > > > What about removing all "IS_"? > > > > > > so end result could be something like that: > > > > > > INTEL_GEN(dev_priv, n) > > > DISPLAY_GEN(dev_priv, n) > > > INTEL_GEN_RANGE(dev_priv, s, e) #or e = FOREVER > > > DISPLAY_GEN_RANGE(dev_priv, s, e) #or e = FOREVER > > > INTEL_GEN_UNTIL(dev_priv, e) > > > DISPLAY_GEN_UNTIL(dev_priv, e) > > > > > > (maybe s/INTEL/GT) > > > > I like it. I'm just not sure about UNTIL, because I will always have doubts > > if > > it's inclusive or not. But I guess we have the same today with RANGE and we > > just get used to it. By making all of them inclusive, it will be easier. > > > > Anyway, my preference is: > > > > GT_GEN(dev_priv, n) > > GT_GEN_RANGE(dev_priv, s, e) > > and e
Re: [Intel-gfx] [PATCH 3/5] drm/i915: merge gen checks to use range
On Fri, Nov 02, 2018 at 12:47:28PM -0700, Lucas De Marchi wrote: > On Fri, Nov 02, 2018 at 12:19:13PM -0700, Rodrigo Vivi wrote: > > On Fri, Nov 02, 2018 at 11:10:10AM -0700, Lucas De Marchi wrote: > > > On Thu, Nov 01, 2018 at 11:31:25AM +, Tvrtko Ursulin wrote: > > > > > > > > On 01/11/2018 08:35, Lucas De Marchi wrote: > > > > > Instead of using several IS_GEN(), it's possible to pass the > > > > > range as argument. By code inspection these were the ranges deemed > > > > > necessary for spatch: > > > > > > > > > > @@ > > > > > expression e; > > > > > @@ > > > > > ( > > > > > - IS_GEN(e, 3) || IS_GEN(e, 4) > > > > > + IS_GEN(e, 3, 4) > > > > > | > > > > > - IS_GEN(e, 5) || IS_GEN(e, 6) > > > > > + IS_GEN(e, 5, 6) > > > > > | > > > > > - IS_GEN(e, 6) || IS_GEN(e, 7) > > > > > + IS_GEN(e, 6, 7) > > > > > | > > > > > - IS_GEN(e, 7) || IS_GEN(e, 8) > > > > > + IS_GEN(e, 7, 8) > > > > > | > > > > > - IS_GEN(e, 8) || IS_GEN(e, 9) > > > > > + IS_GEN(e, 8, 9) > > > > > | > > > > > - IS_GEN(e, 10) || IS_GEN(e, 9) > > > > > + IS_GEN(e, 9, 10) > > > > > | > > > > > - IS_GEN(e, 9) || IS_GEN(e, 10) > > > > > + IS_GEN(e, 9, 10) > > > > > ) > > > > > > > > > > Signed-off-by: Lucas De Marchi > > > > > --- > > > > > drivers/gpu/drm/i915/i915_debugfs.c| 6 +++--- > > > > > drivers/gpu/drm/i915/i915_gpu_error.c | 2 +- > > > > > drivers/gpu/drm/i915/i915_perf.c | 2 +- > > > > > drivers/gpu/drm/i915/intel_crt.c | 2 +- > > > > > drivers/gpu/drm/i915/intel_device_info.c | 2 +- > > > > > drivers/gpu/drm/i915/intel_display.c | 2 +- > > > > > drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- > > > > > drivers/gpu/drm/i915/intel_fifo_underrun.c | 2 +- > > > > > drivers/gpu/drm/i915/intel_pipe_crc.c | 4 ++-- > > > > > drivers/gpu/drm/i915/intel_uncore.c| 6 +++--- > > > > > 10 files changed, 15 insertions(+), 15 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > > > > b/drivers/gpu/drm/i915/i915_debugfs.c > > > > > index 28d95f9d0b0e..f2fbc016bd7f 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > > > > @@ -2030,7 +2030,7 @@ static int i915_swizzle_info(struct seq_file > > > > > *m, void *data) > > > > > seq_printf(m, "bit6 swizzle for Y-tiling = %s\n", > > > > > swizzle_string(dev_priv->mm.bit_6_swizzle_y)); > > > > > - if (IS_GEN(dev_priv, 3) || IS_GEN(dev_priv, 4)) { > > > > > + if (IS_GEN(dev_priv, 3, 4)) { > > > > > > > > I can see value in it but think it would read better with IS_GEN_RANGE. > > > > > > Ok, it seems there's a rough consensus of s/IS_GEN/IS_GEN_RANGE/ an then > > > bring the patches that make sense here. There was a recent patch from > > > Rodrigo > > > doing that. I'll include it in next version. > > > > I liked the double args idea but after reading I believe > > it gets clear IS_GEN_RANGE. > > > > > > > > > > > > > Are there any cases of or-ed IS_GEN checks with something sandwiched in > > > > between then, which the above spatch would miss? > > > > > > By manually inspecting the result of ``git grep -ne "IS_GEN(.*" -- > > > drivers/gpu/drm/i915/`` > > > I didn't find any. The only thing I found was a missed case for gen3 || > > > gen2 > > > that was not covered by the spatch. > > > > > > > > > > > How many non-consecutive IS_GEN gen checks are there? To give us some > > > > ideas > > > > if the usual pattern is range, or perhaps checks against a list of gens > > > > also > > > > exists? (Gut feeling says no.) > > > > > > only cases of <=, <, >=, >. > > > > For these cases on patches 4 and 5:: > > > > What about converting all < n to <= n-1 and all > n to >= n + 1 > > get FORVER back and introduce IS_GEN_UNTIL ? > > > > IS_GEN_UNTIL(dev_priv, e) > > IS_GEN_RANGE(dev_priv, s, FOREVER) > > > > so we can also kill INTEL_GEN. > > > > Another different idea on top of that. > > > > What about removing all "IS_"? > > > > so end result could be something like that: > > > > INTEL_GEN(dev_priv, n) > > DISPLAY_GEN(dev_priv, n) > > INTEL_GEN_RANGE(dev_priv, s, e) #or e = FOREVER > > DISPLAY_GEN_RANGE(dev_priv, s, e) #or e = FOREVER > > INTEL_GEN_UNTIL(dev_priv, e) > > DISPLAY_GEN_UNTIL(dev_priv, e) > > > > (maybe s/INTEL/GT) > > I like it. I'm just not sure about UNTIL, because I will always have doubts if > it's inclusive or not. But I guess we have the same today with RANGE and we > just get used to it. By making all of them inclusive, it will be easier. > > Anyway, my preference is: > > GT_GEN(dev_priv, n) > GT_GEN_RANGE(dev_priv, s, e) > and e can be GEN_FOREVER, aka -1. The macro has enough knowledge to work > out one niptik I prefer FOREVER alone than GEN_FOREVER because macro already has "GEN" > the mask, e.g. s == 10, e == FOREVER => mask == ~(BIT(s) | (BIT(s) - 1)) > > And the DISPLAY_GEN* counterparts. > > IMO there's no need to have _UNTIL because it can
Re: [Intel-gfx] [PATCH 3/5] drm/i915: merge gen checks to use range
On Fri, Nov 02, 2018 at 12:19:13PM -0700, Rodrigo Vivi wrote: > On Fri, Nov 02, 2018 at 11:10:10AM -0700, Lucas De Marchi wrote: > > On Thu, Nov 01, 2018 at 11:31:25AM +, Tvrtko Ursulin wrote: > > > > > > On 01/11/2018 08:35, Lucas De Marchi wrote: > > > > Instead of using several IS_GEN(), it's possible to pass the > > > > range as argument. By code inspection these were the ranges deemed > > > > necessary for spatch: > > > > > > > > @@ > > > > expression e; > > > > @@ > > > > ( > > > > - IS_GEN(e, 3) || IS_GEN(e, 4) > > > > + IS_GEN(e, 3, 4) > > > > | > > > > - IS_GEN(e, 5) || IS_GEN(e, 6) > > > > + IS_GEN(e, 5, 6) > > > > | > > > > - IS_GEN(e, 6) || IS_GEN(e, 7) > > > > + IS_GEN(e, 6, 7) > > > > | > > > > - IS_GEN(e, 7) || IS_GEN(e, 8) > > > > + IS_GEN(e, 7, 8) > > > > | > > > > - IS_GEN(e, 8) || IS_GEN(e, 9) > > > > + IS_GEN(e, 8, 9) > > > > | > > > > - IS_GEN(e, 10) || IS_GEN(e, 9) > > > > + IS_GEN(e, 9, 10) > > > > | > > > > - IS_GEN(e, 9) || IS_GEN(e, 10) > > > > + IS_GEN(e, 9, 10) > > > > ) > > > > > > > > Signed-off-by: Lucas De Marchi > > > > --- > > > > drivers/gpu/drm/i915/i915_debugfs.c| 6 +++--- > > > > drivers/gpu/drm/i915/i915_gpu_error.c | 2 +- > > > > drivers/gpu/drm/i915/i915_perf.c | 2 +- > > > > drivers/gpu/drm/i915/intel_crt.c | 2 +- > > > > drivers/gpu/drm/i915/intel_device_info.c | 2 +- > > > > drivers/gpu/drm/i915/intel_display.c | 2 +- > > > > drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- > > > > drivers/gpu/drm/i915/intel_fifo_underrun.c | 2 +- > > > > drivers/gpu/drm/i915/intel_pipe_crc.c | 4 ++-- > > > > drivers/gpu/drm/i915/intel_uncore.c| 6 +++--- > > > > 10 files changed, 15 insertions(+), 15 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > > > b/drivers/gpu/drm/i915/i915_debugfs.c > > > > index 28d95f9d0b0e..f2fbc016bd7f 100644 > > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > > > @@ -2030,7 +2030,7 @@ static int i915_swizzle_info(struct seq_file *m, > > > > void *data) > > > > seq_printf(m, "bit6 swizzle for Y-tiling = %s\n", > > > >swizzle_string(dev_priv->mm.bit_6_swizzle_y)); > > > > - if (IS_GEN(dev_priv, 3) || IS_GEN(dev_priv, 4)) { > > > > + if (IS_GEN(dev_priv, 3, 4)) { > > > > > > I can see value in it but think it would read better with IS_GEN_RANGE. > > > > Ok, it seems there's a rough consensus of s/IS_GEN/IS_GEN_RANGE/ an then > > bring the patches that make sense here. There was a recent patch from > > Rodrigo > > doing that. I'll include it in next version. > > I liked the double args idea but after reading I believe > it gets clear IS_GEN_RANGE. > > > > > > > > > Are there any cases of or-ed IS_GEN checks with something sandwiched in > > > between then, which the above spatch would miss? > > > > By manually inspecting the result of ``git grep -ne "IS_GEN(.*" -- > > drivers/gpu/drm/i915/`` > > I didn't find any. The only thing I found was a missed case for gen3 || gen2 > > that was not covered by the spatch. > > > > > > > > How many non-consecutive IS_GEN gen checks are there? To give us some > > > ideas > > > if the usual pattern is range, or perhaps checks against a list of gens > > > also > > > exists? (Gut feeling says no.) > > > > only cases of <=, <, >=, >. > > For these cases on patches 4 and 5:: > > What about converting all < n to <= n-1 and all > n to >= n + 1 > get FORVER back and introduce IS_GEN_UNTIL ? > > IS_GEN_UNTIL(dev_priv, e) > IS_GEN_RANGE(dev_priv, s, FOREVER) > > so we can also kill INTEL_GEN. > > Another different idea on top of that. > > What about removing all "IS_"? > > so end result could be something like that: > > INTEL_GEN(dev_priv, n) > DISPLAY_GEN(dev_priv, n) > INTEL_GEN_RANGE(dev_priv, s, e) #or e = FOREVER > DISPLAY_GEN_RANGE(dev_priv, s, e) #or e = FOREVER > INTEL_GEN_UNTIL(dev_priv, e) > DISPLAY_GEN_UNTIL(dev_priv, e) > > (maybe s/INTEL/GT) I like it. I'm just not sure about UNTIL, because I will always have doubts if it's inclusive or not. But I guess we have the same today with RANGE and we just get used to it. By making all of them inclusive, it will be easier. Anyway, my preference is: GT_GEN(dev_priv, n) GT_GEN_RANGE(dev_priv, s, e) and e can be GEN_FOREVER, aka -1. The macro has enough knowledge to work out the mask, e.g. s == 10, e == FOREVER => mask == ~(BIT(s) | (BIT(s) - 1)) And the DISPLAY_GEN* counterparts. IMO there's no need to have _UNTIL because it can also be expressed as GT_GEN_RANGE(dev_priv, GEN_FOREVER, 10). This way we can also kill the comparisons INTEL_GEN(dev_priv) == x, so we always work with the gen_mask field. Which means the compiler can do a single comparison (/me hoping it actually generates good code) There are corner cases though: What should we do with e.g. IS_GEN9_LP() and friends? Any conversion like these will create a lot of
Re: [Intel-gfx] [PATCH 3/5] drm/i915: merge gen checks to use range
On Fri, Nov 02, 2018 at 11:10:10AM -0700, Lucas De Marchi wrote: > On Thu, Nov 01, 2018 at 11:31:25AM +, Tvrtko Ursulin wrote: > > > > On 01/11/2018 08:35, Lucas De Marchi wrote: > > > Instead of using several IS_GEN(), it's possible to pass the > > > range as argument. By code inspection these were the ranges deemed > > > necessary for spatch: > > > > > > @@ > > > expression e; > > > @@ > > > ( > > > - IS_GEN(e, 3) || IS_GEN(e, 4) > > > + IS_GEN(e, 3, 4) > > > | > > > - IS_GEN(e, 5) || IS_GEN(e, 6) > > > + IS_GEN(e, 5, 6) > > > | > > > - IS_GEN(e, 6) || IS_GEN(e, 7) > > > + IS_GEN(e, 6, 7) > > > | > > > - IS_GEN(e, 7) || IS_GEN(e, 8) > > > + IS_GEN(e, 7, 8) > > > | > > > - IS_GEN(e, 8) || IS_GEN(e, 9) > > > + IS_GEN(e, 8, 9) > > > | > > > - IS_GEN(e, 10) || IS_GEN(e, 9) > > > + IS_GEN(e, 9, 10) > > > | > > > - IS_GEN(e, 9) || IS_GEN(e, 10) > > > + IS_GEN(e, 9, 10) > > > ) > > > > > > Signed-off-by: Lucas De Marchi > > > --- > > > drivers/gpu/drm/i915/i915_debugfs.c| 6 +++--- > > > drivers/gpu/drm/i915/i915_gpu_error.c | 2 +- > > > drivers/gpu/drm/i915/i915_perf.c | 2 +- > > > drivers/gpu/drm/i915/intel_crt.c | 2 +- > > > drivers/gpu/drm/i915/intel_device_info.c | 2 +- > > > drivers/gpu/drm/i915/intel_display.c | 2 +- > > > drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- > > > drivers/gpu/drm/i915/intel_fifo_underrun.c | 2 +- > > > drivers/gpu/drm/i915/intel_pipe_crc.c | 4 ++-- > > > drivers/gpu/drm/i915/intel_uncore.c| 6 +++--- > > > 10 files changed, 15 insertions(+), 15 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > > b/drivers/gpu/drm/i915/i915_debugfs.c > > > index 28d95f9d0b0e..f2fbc016bd7f 100644 > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > > @@ -2030,7 +2030,7 @@ static int i915_swizzle_info(struct seq_file *m, > > > void *data) > > > seq_printf(m, "bit6 swizzle for Y-tiling = %s\n", > > > swizzle_string(dev_priv->mm.bit_6_swizzle_y)); > > > - if (IS_GEN(dev_priv, 3) || IS_GEN(dev_priv, 4)) { > > > + if (IS_GEN(dev_priv, 3, 4)) { > > > > I can see value in it but think it would read better with IS_GEN_RANGE. > > Ok, it seems there's a rough consensus of s/IS_GEN/IS_GEN_RANGE/ an then > bring the patches that make sense here. There was a recent patch from Rodrigo > doing that. I'll include it in next version. I liked the double args idea but after reading I believe it gets clear IS_GEN_RANGE. > > > > > Are there any cases of or-ed IS_GEN checks with something sandwiched in > > between then, which the above spatch would miss? > > By manually inspecting the result of ``git grep -ne "IS_GEN(.*" -- > drivers/gpu/drm/i915/`` > I didn't find any. The only thing I found was a missed case for gen3 || gen2 > that was not covered by the spatch. > > > > > How many non-consecutive IS_GEN gen checks are there? To give us some ideas > > if the usual pattern is range, or perhaps checks against a list of gens also > > exists? (Gut feeling says no.) > > only cases of <=, <, >=, >. For these cases on patches 4 and 5:: What about converting all < n to <= n-1 and all > n to >= n + 1 get FORVER back and introduce IS_GEN_UNTIL ? IS_GEN_UNTIL(dev_priv, e) IS_GEN_RANGE(dev_priv, s, FOREVER) so we can also kill INTEL_GEN. Another different idea on top of that. What about removing all "IS_"? so end result could be something like that: INTEL_GEN(dev_priv, n) DISPLAY_GEN(dev_priv, n) INTEL_GEN_RANGE(dev_priv, s, e) #or e = FOREVER DISPLAY_GEN_RANGE(dev_priv, s, e) #or e = FOREVER INTEL_GEN_UNTIL(dev_priv, e) DISPLAY_GEN_UNTIL(dev_priv, e) (maybe s/INTEL/GT) and group all of these together, because today they are spreadded apart. Thanks, Rodrigo. > > > thanks > Lucas De Marchi > > > > > Regards, > > > > Tvrtko > > > > > seq_printf(m, "DDC = 0x%08x\n", > > > I915_READ(DCC)); > > > seq_printf(m, "DDC2 = 0x%08x\n", > > > @@ -4260,7 +4260,7 @@ i915_cache_sharing_get(void *data, u64 *val) > > > struct drm_i915_private *dev_priv = data; > > > u32 snpcr; > > > - if (!(IS_GEN(dev_priv, 6) || IS_GEN(dev_priv, 7))) > > > + if (!(IS_GEN(dev_priv, 6, 7))) > > > return -ENODEV; > > > intel_runtime_pm_get(dev_priv); > > > @@ -4280,7 +4280,7 @@ i915_cache_sharing_set(void *data, u64 val) > > > struct drm_i915_private *dev_priv = data; > > > u32 snpcr; > > > - if (!(IS_GEN(dev_priv, 6) || IS_GEN(dev_priv, 7))) > > > + if (!(IS_GEN(dev_priv, 6, 7))) > > > return -ENODEV; > > > if (val > 3) > > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c > > > b/drivers/gpu/drm/i915/i915_gpu_error.c > > > index c9f8aa47005a..969691e50c04 100644 > > > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > > > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > > > @@ -1673,7
Re: [Intel-gfx] [PATCH 3/5] drm/i915: merge gen checks to use range
On Thu, Nov 01, 2018 at 11:31:25AM +, Tvrtko Ursulin wrote: > > On 01/11/2018 08:35, Lucas De Marchi wrote: > > Instead of using several IS_GEN(), it's possible to pass the > > range as argument. By code inspection these were the ranges deemed > > necessary for spatch: > > > > @@ > > expression e; > > @@ > > ( > > - IS_GEN(e, 3) || IS_GEN(e, 4) > > + IS_GEN(e, 3, 4) > > | > > - IS_GEN(e, 5) || IS_GEN(e, 6) > > + IS_GEN(e, 5, 6) > > | > > - IS_GEN(e, 6) || IS_GEN(e, 7) > > + IS_GEN(e, 6, 7) > > | > > - IS_GEN(e, 7) || IS_GEN(e, 8) > > + IS_GEN(e, 7, 8) > > | > > - IS_GEN(e, 8) || IS_GEN(e, 9) > > + IS_GEN(e, 8, 9) > > | > > - IS_GEN(e, 10) || IS_GEN(e, 9) > > + IS_GEN(e, 9, 10) > > | > > - IS_GEN(e, 9) || IS_GEN(e, 10) > > + IS_GEN(e, 9, 10) > > ) > > > > Signed-off-by: Lucas De Marchi > > --- > > drivers/gpu/drm/i915/i915_debugfs.c| 6 +++--- > > drivers/gpu/drm/i915/i915_gpu_error.c | 2 +- > > drivers/gpu/drm/i915/i915_perf.c | 2 +- > > drivers/gpu/drm/i915/intel_crt.c | 2 +- > > drivers/gpu/drm/i915/intel_device_info.c | 2 +- > > drivers/gpu/drm/i915/intel_display.c | 2 +- > > drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- > > drivers/gpu/drm/i915/intel_fifo_underrun.c | 2 +- > > drivers/gpu/drm/i915/intel_pipe_crc.c | 4 ++-- > > drivers/gpu/drm/i915/intel_uncore.c| 6 +++--- > > 10 files changed, 15 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > b/drivers/gpu/drm/i915/i915_debugfs.c > > index 28d95f9d0b0e..f2fbc016bd7f 100644 > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > @@ -2030,7 +2030,7 @@ static int i915_swizzle_info(struct seq_file *m, void > > *data) > > seq_printf(m, "bit6 swizzle for Y-tiling = %s\n", > >swizzle_string(dev_priv->mm.bit_6_swizzle_y)); > > - if (IS_GEN(dev_priv, 3) || IS_GEN(dev_priv, 4)) { > > + if (IS_GEN(dev_priv, 3, 4)) { > > I can see value in it but think it would read better with IS_GEN_RANGE. Ok, it seems there's a rough consensus of s/IS_GEN/IS_GEN_RANGE/ an then bring the patches that make sense here. There was a recent patch from Rodrigo doing that. I'll include it in next version. > > Are there any cases of or-ed IS_GEN checks with something sandwiched in > between then, which the above spatch would miss? By manually inspecting the result of ``git grep -ne "IS_GEN(.*" -- drivers/gpu/drm/i915/`` I didn't find any. The only thing I found was a missed case for gen3 || gen2 that was not covered by the spatch. > > How many non-consecutive IS_GEN gen checks are there? To give us some ideas > if the usual pattern is range, or perhaps checks against a list of gens also > exists? (Gut feeling says no.) only cases of <=, <, >=, >. thanks Lucas De Marchi > > Regards, > > Tvrtko > > > seq_printf(m, "DDC = 0x%08x\n", > >I915_READ(DCC)); > > seq_printf(m, "DDC2 = 0x%08x\n", > > @@ -4260,7 +4260,7 @@ i915_cache_sharing_get(void *data, u64 *val) > > struct drm_i915_private *dev_priv = data; > > u32 snpcr; > > - if (!(IS_GEN(dev_priv, 6) || IS_GEN(dev_priv, 7))) > > + if (!(IS_GEN(dev_priv, 6, 7))) > > return -ENODEV; > > intel_runtime_pm_get(dev_priv); > > @@ -4280,7 +4280,7 @@ i915_cache_sharing_set(void *data, u64 val) > > struct drm_i915_private *dev_priv = data; > > u32 snpcr; > > - if (!(IS_GEN(dev_priv, 6) || IS_GEN(dev_priv, 7))) > > + if (!(IS_GEN(dev_priv, 6, 7))) > > return -ENODEV; > > if (val > 3) > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c > > b/drivers/gpu/drm/i915/i915_gpu_error.c > > index c9f8aa47005a..969691e50c04 100644 > > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > > @@ -1673,7 +1673,7 @@ static void capture_reg_state(struct i915_gpu_state > > *error) > > error->ccid = I915_READ(CCID); > > /* 3: Feature specific registers */ > > - if (IS_GEN(dev_priv, 6) || IS_GEN(dev_priv, 7)) { > > + if (IS_GEN(dev_priv, 6, 7)) { > > error->gam_ecochk = I915_READ(GAM_ECOCHK); > > error->gac_eco = I915_READ(GAC_ECO_BITS); > > } > > diff --git a/drivers/gpu/drm/i915/i915_perf.c > > b/drivers/gpu/drm/i915/i915_perf.c > > index 92daddf79cb0..baaa7b70ffa0 100644 > > --- a/drivers/gpu/drm/i915/i915_perf.c > > +++ b/drivers/gpu/drm/i915/i915_perf.c > > @@ -3449,7 +3449,7 @@ void i915_perf_init(struct drm_i915_private *dev_priv) > > dev_priv->perf.oa.ops.read = gen8_oa_read; > > dev_priv->perf.oa.ops.oa_hw_tail_read = gen8_oa_hw_tail_read; > > - if (IS_GEN(dev_priv, 8) || IS_GEN(dev_priv, 9)) { > > + if (IS_GEN(dev_priv, 8, 9)) { > > dev_priv->perf.oa.ops.is_valid_b_counter_reg = > > gen7_is_valid_b_counter_addr; > >
Re: [Intel-gfx] [PATCH 3/5] drm/i915: merge gen checks to use range
On 01/11/2018 08:35, Lucas De Marchi wrote: Instead of using several IS_GEN(), it's possible to pass the range as argument. By code inspection these were the ranges deemed necessary for spatch: @@ expression e; @@ ( - IS_GEN(e, 3) || IS_GEN(e, 4) + IS_GEN(e, 3, 4) | - IS_GEN(e, 5) || IS_GEN(e, 6) + IS_GEN(e, 5, 6) | - IS_GEN(e, 6) || IS_GEN(e, 7) + IS_GEN(e, 6, 7) | - IS_GEN(e, 7) || IS_GEN(e, 8) + IS_GEN(e, 7, 8) | - IS_GEN(e, 8) || IS_GEN(e, 9) + IS_GEN(e, 8, 9) | - IS_GEN(e, 10) || IS_GEN(e, 9) + IS_GEN(e, 9, 10) | - IS_GEN(e, 9) || IS_GEN(e, 10) + IS_GEN(e, 9, 10) ) Signed-off-by: Lucas De Marchi --- drivers/gpu/drm/i915/i915_debugfs.c| 6 +++--- drivers/gpu/drm/i915/i915_gpu_error.c | 2 +- drivers/gpu/drm/i915/i915_perf.c | 2 +- drivers/gpu/drm/i915/intel_crt.c | 2 +- drivers/gpu/drm/i915/intel_device_info.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- drivers/gpu/drm/i915/intel_fifo_underrun.c | 2 +- drivers/gpu/drm/i915/intel_pipe_crc.c | 4 ++-- drivers/gpu/drm/i915/intel_uncore.c| 6 +++--- 10 files changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 28d95f9d0b0e..f2fbc016bd7f 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2030,7 +2030,7 @@ static int i915_swizzle_info(struct seq_file *m, void *data) seq_printf(m, "bit6 swizzle for Y-tiling = %s\n", swizzle_string(dev_priv->mm.bit_6_swizzle_y)); - if (IS_GEN(dev_priv, 3) || IS_GEN(dev_priv, 4)) { + if (IS_GEN(dev_priv, 3, 4)) { I can see value in it but think it would read better with IS_GEN_RANGE. Are there any cases of or-ed IS_GEN checks with something sandwiched in between then, which the above spatch would miss? How many non-consecutive IS_GEN gen checks are there? To give us some ideas if the usual pattern is range, or perhaps checks against a list of gens also exists? (Gut feeling says no.) Regards, Tvrtko seq_printf(m, "DDC = 0x%08x\n", I915_READ(DCC)); seq_printf(m, "DDC2 = 0x%08x\n", @@ -4260,7 +4260,7 @@ i915_cache_sharing_get(void *data, u64 *val) struct drm_i915_private *dev_priv = data; u32 snpcr; - if (!(IS_GEN(dev_priv, 6) || IS_GEN(dev_priv, 7))) + if (!(IS_GEN(dev_priv, 6, 7))) return -ENODEV; intel_runtime_pm_get(dev_priv); @@ -4280,7 +4280,7 @@ i915_cache_sharing_set(void *data, u64 val) struct drm_i915_private *dev_priv = data; u32 snpcr; - if (!(IS_GEN(dev_priv, 6) || IS_GEN(dev_priv, 7))) + if (!(IS_GEN(dev_priv, 6, 7))) return -ENODEV; if (val > 3) diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index c9f8aa47005a..969691e50c04 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1673,7 +1673,7 @@ static void capture_reg_state(struct i915_gpu_state *error) error->ccid = I915_READ(CCID); /* 3: Feature specific registers */ - if (IS_GEN(dev_priv, 6) || IS_GEN(dev_priv, 7)) { + if (IS_GEN(dev_priv, 6, 7)) { error->gam_ecochk = I915_READ(GAM_ECOCHK); error->gac_eco = I915_READ(GAC_ECO_BITS); } diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 92daddf79cb0..baaa7b70ffa0 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -3449,7 +3449,7 @@ void i915_perf_init(struct drm_i915_private *dev_priv) dev_priv->perf.oa.ops.read = gen8_oa_read; dev_priv->perf.oa.ops.oa_hw_tail_read = gen8_oa_hw_tail_read; - if (IS_GEN(dev_priv, 8) || IS_GEN(dev_priv, 9)) { + if (IS_GEN(dev_priv, 8, 9)) { dev_priv->perf.oa.ops.is_valid_b_counter_reg = gen7_is_valid_b_counter_addr; dev_priv->perf.oa.ops.is_valid_mux_reg = diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index bf4fd739b68c..1822dccb1914 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -322,7 +322,7 @@ intel_crt_mode_valid(struct drm_connector *connector, * DAC limit supposedly 355 MHz. */ max_clock = 27; - else if (IS_GEN(dev_priv, 3) || IS_GEN(dev_priv, 4)) + else if (IS_GEN(dev_priv, 3, 4)) max_clock = 40; else max_clock = 35; diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 873f37b7b796..a1b046c322d5 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -783,7 +783,7 @@
Re: [Intel-gfx] [PATCH 3/5] drm/i915: merge gen checks to use range
On Thu, 01 Nov 2018, Lucas De Marchi wrote: > Instead of using several IS_GEN(), it's possible to pass the > range as argument. By code inspection these were the ranges deemed > necessary for spatch: Again, ignoring the naming for now, I do like the idea here. 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] [PATCH 3/5] drm/i915: merge gen checks to use range
Instead of using several IS_GEN(), it's possible to pass the range as argument. By code inspection these were the ranges deemed necessary for spatch: @@ expression e; @@ ( - IS_GEN(e, 3) || IS_GEN(e, 4) + IS_GEN(e, 3, 4) | - IS_GEN(e, 5) || IS_GEN(e, 6) + IS_GEN(e, 5, 6) | - IS_GEN(e, 6) || IS_GEN(e, 7) + IS_GEN(e, 6, 7) | - IS_GEN(e, 7) || IS_GEN(e, 8) + IS_GEN(e, 7, 8) | - IS_GEN(e, 8) || IS_GEN(e, 9) + IS_GEN(e, 8, 9) | - IS_GEN(e, 10) || IS_GEN(e, 9) + IS_GEN(e, 9, 10) | - IS_GEN(e, 9) || IS_GEN(e, 10) + IS_GEN(e, 9, 10) ) Signed-off-by: Lucas De Marchi --- drivers/gpu/drm/i915/i915_debugfs.c| 6 +++--- drivers/gpu/drm/i915/i915_gpu_error.c | 2 +- drivers/gpu/drm/i915/i915_perf.c | 2 +- drivers/gpu/drm/i915/intel_crt.c | 2 +- drivers/gpu/drm/i915/intel_device_info.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- drivers/gpu/drm/i915/intel_fifo_underrun.c | 2 +- drivers/gpu/drm/i915/intel_pipe_crc.c | 4 ++-- drivers/gpu/drm/i915/intel_uncore.c| 6 +++--- 10 files changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 28d95f9d0b0e..f2fbc016bd7f 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2030,7 +2030,7 @@ static int i915_swizzle_info(struct seq_file *m, void *data) seq_printf(m, "bit6 swizzle for Y-tiling = %s\n", swizzle_string(dev_priv->mm.bit_6_swizzle_y)); - if (IS_GEN(dev_priv, 3) || IS_GEN(dev_priv, 4)) { + if (IS_GEN(dev_priv, 3, 4)) { seq_printf(m, "DDC = 0x%08x\n", I915_READ(DCC)); seq_printf(m, "DDC2 = 0x%08x\n", @@ -4260,7 +4260,7 @@ i915_cache_sharing_get(void *data, u64 *val) struct drm_i915_private *dev_priv = data; u32 snpcr; - if (!(IS_GEN(dev_priv, 6) || IS_GEN(dev_priv, 7))) + if (!(IS_GEN(dev_priv, 6, 7))) return -ENODEV; intel_runtime_pm_get(dev_priv); @@ -4280,7 +4280,7 @@ i915_cache_sharing_set(void *data, u64 val) struct drm_i915_private *dev_priv = data; u32 snpcr; - if (!(IS_GEN(dev_priv, 6) || IS_GEN(dev_priv, 7))) + if (!(IS_GEN(dev_priv, 6, 7))) return -ENODEV; if (val > 3) diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index c9f8aa47005a..969691e50c04 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1673,7 +1673,7 @@ static void capture_reg_state(struct i915_gpu_state *error) error->ccid = I915_READ(CCID); /* 3: Feature specific registers */ - if (IS_GEN(dev_priv, 6) || IS_GEN(dev_priv, 7)) { + if (IS_GEN(dev_priv, 6, 7)) { error->gam_ecochk = I915_READ(GAM_ECOCHK); error->gac_eco = I915_READ(GAC_ECO_BITS); } diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 92daddf79cb0..baaa7b70ffa0 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -3449,7 +3449,7 @@ void i915_perf_init(struct drm_i915_private *dev_priv) dev_priv->perf.oa.ops.read = gen8_oa_read; dev_priv->perf.oa.ops.oa_hw_tail_read = gen8_oa_hw_tail_read; - if (IS_GEN(dev_priv, 8) || IS_GEN(dev_priv, 9)) { + if (IS_GEN(dev_priv, 8, 9)) { dev_priv->perf.oa.ops.is_valid_b_counter_reg = gen7_is_valid_b_counter_addr; dev_priv->perf.oa.ops.is_valid_mux_reg = diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index bf4fd739b68c..1822dccb1914 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -322,7 +322,7 @@ intel_crt_mode_valid(struct drm_connector *connector, * DAC limit supposedly 355 MHz. */ max_clock = 27; - else if (IS_GEN(dev_priv, 3) || IS_GEN(dev_priv, 4)) + else if (IS_GEN(dev_priv, 3, 4)) max_clock = 40; else max_clock = 35; diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 873f37b7b796..a1b046c322d5 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -783,7 +783,7 @@ void intel_device_info_runtime_init(struct intel_device_info *info) DRM_INFO("Display disabled (module parameter)\n"); info->num_pipes = 0; } else if (info->num_pipes > 0 && - (IS_GEN(dev_priv, 7) || IS_GEN(dev_priv, 8)) && + (IS_GEN(dev_priv, 7, 8)) && HAS_PCH_SPLIT(dev_priv)) { u32 fuse_strap = I915_READ(FUSE_STRAP);