On Wed, Apr 09, 2014 at 02:16:04PM +0100, Chris Wilson wrote: > On Wed, Apr 09, 2014 at 01:28:26PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > From: Deepak S <deepak.s@xxxxxxxxx> > > > > Support to individually control Media/Render well based on the register access. > > Add CHV specific write function to habdle difference between registers > > that are sadowed vs those that need forcewake even for writes. > > > > v2: Drop write FIFO for CHV and add comman well forcewake (Ville) > > > > Signed-off-by: Deepak S <deepak.s@xxxxxxxxx> > > [vsyrjala: Move the register range macros into intel_uncore.c] > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_uncore.c | 139 +++++++++++++++++++++++++++++++++--- > > 1 file changed, 131 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > > index 823d699..8e3c686 100644 > > --- a/drivers/gpu/drm/i915/intel_uncore.c > > +++ b/drivers/gpu/drm/i915/intel_uncore.c > > @@ -495,6 +495,31 @@ void assert_force_wake_inactive(struct drm_i915_private *dev_priv) > > ((reg) >= 0x22000 && (reg) < 0x24000) ||\ > > ((reg) >= 0x30000 && (reg) < 0x40000)) > > > > +#define FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg) \ > > + (((reg) >= 0x2000 && (reg) < 0x4000) ||\ > > + ((reg) >= 0x5000 && (reg) < 0x8000) ||\ > > + ((reg) >= 0x8300 && (reg) < 0x8500) ||\ > > + ((reg) >= 0xB000 && (reg) < 0xC000) ||\ > > + ((reg) >= 0xE000 && (reg) < 0xE800)) > > + > > +#define FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)\ > > + (((reg) >= 0x8800 && (reg) < 0x8900) ||\ > > + ((reg) >= 0xD000 && (reg) < 0xD800) ||\ > > + ((reg) >= 0x12000 && (reg) < 0x14000) ||\ > > + ((reg) >= 0x1A000 && (reg) < 0x1C000) ||\ > > + ((reg) >= 0x1E800 && (reg) < 0x1EA00) ||\ > > + ((reg) >= 0x30000 && (reg) < 0x40000)) > > + > > +#define FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)\ > > + (((reg) >= 0x4000 && (reg) < 0x5000) ||\ > > + ((reg) >= 0x8000 && (reg) < 0x8300) ||\ > > + ((reg) >= 0x8500 && (reg) < 0x8600) ||\ > > + ((reg) >= 0x9000 && (reg) < 0xB000) ||\ > > + ((reg) >= 0xC000 && (reg) < 0xc800) ||\ > > + ((reg) >= 0xF000 && (reg) < 0x10000) ||\ > > + ((reg) >= 0x14000 && (reg) < 0x14400) ||\ > > + ((reg) >= 0x22000 && (reg) < 0x24000)) > > You could write this as a single stanza of > > if (reg < foo) fwengine = BAR; > else if (reg < baz) ... > > which I think would not only help with us reviewing it, but wouldn't > generate so nearly as bad code. For me that looks harder to read as you can't see each range neatly on one line anymore. But you may be right about getting better code out of it. I must admit I've never checked what kind of code gcc generates for these things. Another idea would be to use a switch statement with case ranges, but that doesn't seem really different to the current approach. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
