On 07/09/2021 09:42, Daniel Vetter wrote:
On Fri, Sep 03, 2021 at 01:47:52PM +0100, Tvrtko Ursulin wrote:
On 29/07/2021 15:06, Daniel Vetter wrote:
On Thu, Jul 29, 2021 at 3:34 PM Tvrtko Ursulin
<tvrtko.ursulin@xxxxxxxxxxxxxxx> wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Usage of Transparent Hugepages was disabled in 9987da4b5dcf
("drm/i915: Disable THP until we have a GPU read BW W/A"), but since it
appears majority of performance regressions reported with an enabled IOMMU
can be almost eliminated by turning them on, lets just do that.
To err on the side of safety we keep the current default in cases where
IOMMU is not active, and only when it is default to the "huge=within_size"
mode. Although there probably would be wins to enable them throughout,
more extensive testing across benchmarks and platforms would need to be
done.
With the patch and IOMMU enabled my local testing on a small Skylake part
shows OglVSTangent regression being reduced from ~14% (IOMMU on versus
IOMMU off) to ~2% (same comparison but with THP on).
v2:
* Add Kconfig dependency to transparent hugepages and some help text.
* Move to helper for easier handling of kernel build options.
v3:
* Drop Kconfig. (Daniel)
References: b901bb89324a ("drm/i915/gemfs: enable THP")
References: 9987da4b5dcf ("drm/i915: Disable THP until we have a GPU read BW W/A")
References: https://gitlab.freedesktop.org/drm/intel/-/issues/430
Co-developed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Matthew Auld <matthew.auld@xxxxxxxxx>
Cc: Eero Tamminen <eero.t.tamminen@xxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Cc: Daniel Vetter <daniel@xxxxxxxx>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> # v1
On both patches: Acked-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
Eero's testing results at
https://gitlab.freedesktop.org/drm/intel/-/issues/430 are looking good -
seem to show this to be a net win for at least Gen9 and Gen12 platforms.
Is the ack enough to merge in this case or I should look for an r-b as well?
Since your back to defacto v1 with the 2nd patch I think you have full r-b
already. So more than enough I think.
Just in case you missed it, v1 had Kconfig. But it's the same spirit so
probably indeed fine as you say.
Please do record the relative perf numbers from Eero in that issue in the
commit message so that we have that on the git log record too. It's easier
to find there than following the link and finding the right comment in the
issue.
Will do.
Regards,
Tvrtko
