On 05/21/2015 11:10 PM, Daniel Vetter wrote:
> On Thu, May 21, 2015 at 01:18:44PM -0700, Jesse Barnes wrote:
>> On 05/19/2015 10:32 AM, ville.syrjala@xxxxxxxxxxxxxxx wrote:
>>> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>>>
>>> GTT caching was disabled by default on gen8 due to not working with
>>> big pages. Some information suggests that it got fixed, but still
>>> GTT caching has been left disabled by default. Or could be it just
>>> meant that the default was changed to off, and hence the problem
>>> got solved.
>>>
>>> Enable GTT caching in the hopes of some performance increase.
>>> Whether or not the big pages issue has been fixed is irrelevant
>>> at this stage since we don't use big pages.
>>>
>>> This gives me a 1-2% improvement in xonotic on my BSW. Haven't tried
>>> BDW, but supposedly it has larger TLBs so might not benefit as much.
>>> On HSW GTT caching is enabled by default.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>>> ---
>>> drivers/gpu/drm/i915/i915_reg.h | 2 ++
>>> drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++++
>>> 2 files changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index 84af255..90640d5 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -1461,6 +1461,8 @@ enum skl_disp_power_wells {
>>> #define RING_HWS_PGA(base) ((base)+0x80)
>>> #define RING_HWS_PGA_GEN6(base) ((base)+0x2080)
>>>
>>> +#define HSW_GTT_CACHE_EN 0x4024
>>> +#define GTT_CACHE_EN_ALL 0xF0007FFF
>>> #define GEN7_WR_WATERMARK 0x4028
>>> #define GEN7_GFX_PRIO_CTRL 0x402C
>>> #define ARB_MODE 0x4030
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>> index 5ec56b6..58517a50 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -6205,6 +6205,13 @@ static void broadwell_init_clock_gating(struct drm_device *dev)
>>> I915_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT);
>>> I915_WRITE(GEN7_MISCCPCTL, misccpctl);
>>>
>>> + /*
>>> + * WaGttCachingOffByDefault:bdw
>>> + * GTT cache may not work with big pages, so if those
>>> + * are ever enabled GTT cache may need to be disabled.
>>> + */
>>> + I915_WRITE(HSW_GTT_CACHE_EN, GTT_CACHE_EN_ALL);
>>> +
>>> lpt_init_clock_gating(dev);
>>> }
>>>
>>> @@ -6480,6 +6487,12 @@ static void cherryview_init_clock_gating(struct drm_device *dev)
>>> /* WaDisableSDEUnitClockGating:chv */
>>> I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
>>> GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
>>> +
>>> + /*
>>> + * GTT cache may not work with big pages, so if those
>>> + * are ever enabled GTT cache may need to be disabled.
>>> + */
>>> + I915_WRITE(HSW_GTT_CACHE_EN, GTT_CACHE_EN_ALL);
>>> }
>>>
>>> static void g4x_init_clock_gating(struct drm_device *dev)
>>>
>>
>> Looks ok to me; I guess testing will be the real review here.
>
> Just aside: If you think the real test for a patch is the real world imo
> an important part of the review work is to make sure we do have that
> testing coverage. We have a few igts that specifically exercise gtt tlb
> issues (from previous generations), so I think we're covered here.
>
>> Reviewed-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
>
> Merged all three, thanks for patches&review.
Yeah I don't think it's something that requires a dedicated test, just
lots of coverage with our existing stuff to sniff out problems.
Thanks,
Jesse
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
