On Mon, Jun 24, 2019 at 01:31:51PM -0700, Daniele Ceraolo Spurio wrote:
> A forcewake-less uncore to be used to decouple GT accesses from display
> ones to avoid serializing them when there is no need.
>
> New accessors that implicitly use the new uncore have also been added.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 17 ++++++++-
> drivers/gpu/drm/i915/i915_drv.h | 58 +++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_uncore.c | 9 ++++-
> 3 files changed, 81 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 2d6c643dde51..e22c0a6b3992 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -903,6 +903,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
>
> intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug);
> intel_uncore_init_early(&dev_priv->uncore, dev_priv);
> + intel_uncore_init_early(&dev_priv->de_uncore, dev_priv);
>
> spin_lock_init(&dev_priv->irq_lock);
> spin_lock_init(&dev_priv->gpu_error.lock);
> @@ -1001,6 +1002,10 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
> if (ret < 0)
> goto err_bridge;
>
> + ret = intel_uncore_init_mmio(&dev_priv->de_uncore);
> + if (ret < 0)
> + goto err_uncore;
> +
> /* Try to make sure MCHBAR is enabled before poking at it */
> intel_setup_mchbar(dev_priv);
>
> @@ -1012,14 +1017,16 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
>
> ret = intel_engines_init_mmio(dev_priv);
> if (ret)
> - goto err_uncore;
> + goto err_mchbar;
>
> i915_gem_init_mmio(dev_priv);
>
> return 0;
>
> -err_uncore:
> +err_mchbar:
> intel_teardown_mchbar(dev_priv);
> + intel_uncore_fini_mmio(&dev_priv->de_uncore);
> +err_uncore:
> intel_uncore_fini_mmio(&dev_priv->uncore);
> err_bridge:
> pci_dev_put(dev_priv->bridge_dev);
> @@ -1034,6 +1041,7 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
> static void i915_driver_cleanup_mmio(struct drm_i915_private *dev_priv)
> {
> intel_teardown_mchbar(dev_priv);
> + intel_uncore_fini_mmio(&dev_priv->de_uncore);
> intel_uncore_fini_mmio(&dev_priv->uncore);
> pci_dev_put(dev_priv->bridge_dev);
> }
> @@ -2166,6 +2174,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
>
> i915_gem_suspend_late(dev_priv);
>
> + intel_uncore_suspend(&dev_priv->de_uncore);
> intel_uncore_suspend(&dev_priv->uncore);
>
> intel_power_domains_suspend(dev_priv,
> @@ -2363,6 +2372,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
> ret);
>
> intel_uncore_resume_early(&dev_priv->uncore);
> + intel_uncore_resume_early(&dev_priv->de_uncore);
>
> intel_gt_check_and_clear_faults(&dev_priv->gt);
>
> @@ -2937,6 +2947,7 @@ static int intel_runtime_suspend(struct device *kdev)
>
> intel_runtime_pm_disable_interrupts(dev_priv);
>
> + intel_uncore_suspend(&dev_priv->de_uncore);
> intel_uncore_suspend(&dev_priv->uncore);
>
> ret = 0;
> @@ -2955,6 +2966,7 @@ static int intel_runtime_suspend(struct device *kdev)
> if (ret) {
> DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret);
> intel_uncore_runtime_resume(&dev_priv->uncore);
> + intel_uncore_runtime_resume(&dev_priv->de_uncore);
>
> intel_runtime_pm_enable_interrupts(dev_priv);
>
> @@ -3053,6 +3065,7 @@ static int intel_runtime_resume(struct device *kdev)
> }
>
> intel_uncore_runtime_resume(&dev_priv->uncore);
> + intel_uncore_runtime_resume(&dev_priv->de_uncore);
>
> intel_runtime_pm_enable_interrupts(dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a53b65d78159..c554b7697bdc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1341,6 +1341,7 @@ struct drm_i915_private {
> resource_size_t stolen_usable_size; /* Total size minus reserved ranges */
>
> struct intel_uncore uncore;
> + struct intel_uncore de_uncore;
> struct intel_uncore_mmio_debug mmio_debug;
>
> struct i915_virtual_gpu vgpu;
> @@ -2746,6 +2747,63 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
> #define I915_READ_FW(reg__) __I915_REG_OP(read_fw, dev_priv, (reg__))
> #define I915_WRITE_FW(reg__, val__) __I915_REG_OP(write_fw, dev_priv, (reg__), (val__))
>
> +/*
> + * The following are mmio-accessors that use an independent lock and skip all
> + * the forcewake logic, to be used to access display registers, which are
> + * outside the GT forcewake wells.
> + */
> +static inline void
> +intel_assert_register_is_display(struct drm_i915_private *i915, i915_reg_t reg)
> +{
> +#ifdef CONFIG_DRM_I915_DEBUG_MMIO
> + u32 offset = i915_mmio_reg_offset(reg);
> + bool is_de_register;
> +
> + if (INTEL_GEN(i915) >= 5) {
> + is_de_register = offset >= 0x40000 &&
> + (INTEL_GEN(i915) < 11 || offset < 0x1c0000);
I think the gen11+ thing is wrong. There are eg. PHY registers
above 0x162000.
> + } else {
> + is_de_register =
> + (offset >= 0x5000 && offset <= 0x5fff) ||
> + (offset >= 0x6000 && offset <= 0x6fff) ||
> + (offset >= 0xa000 && offset <= 0xafff) ||
> + offset >= 0x30000;
I think the WARNs are coming from the mchbar range (0x10000+ on
these platforms). Not really sure if we should designate that
gt or de.
Unfortunately the register ranges don't seem to follow much
of a sensible pattern. Eg. we seem to have both gt and de
related stuff around the 0x130000 mark :(
Maybe we should try separate only the pipe/plane registers
which are part of your typical atomic commit under their
own lock. Could even do per-pipe locks maybe...
> + }
> +
> + WARN_ONCE(!is_de_register,
> + "display reg access function used for non-display reg 0x%08x\n",
> + offset);
> +#endif
> +}
> +
> +static inline u32
> +intel_de_read(struct drm_i915_private *i915, i915_reg_t reg)
> +{
> + intel_assert_register_is_display(i915, reg);
> + return intel_uncore_read(&i915->de_uncore, reg);
> +}
> +
> +static inline void
> +intel_de_posting_read(struct drm_i915_private *i915, i915_reg_t reg)
> +{
> + intel_assert_register_is_display(i915, reg);
> + intel_uncore_posting_read(&i915->de_uncore, reg);
> +}
> +
> +static inline void
> +intel_de_write(struct drm_i915_private *i915, i915_reg_t reg, u32 val)
> +{
> + intel_assert_register_is_display(i915, reg);
> + intel_uncore_write(&i915->de_uncore, reg, val);
> +}
> +
> +static inline void
> +intel_de_rmw(struct drm_i915_private *i915, i915_reg_t reg, u32 clear, u32 set)
> +{
> + intel_assert_register_is_display(i915, reg);
> + intel_uncore_rmw(&i915->de_uncore, reg, clear, set);
> +}
> +
> /* "Broadcast RGB" property */
> #define INTEL_BROADCAST_RGB_AUTO 0
> #define INTEL_BROADCAST_RGB_FULL 1
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 1905fd94dd3c..94e153acb0af 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1674,6 +1674,12 @@ static int uncore_forcewake_init(struct intel_uncore *uncore)
> return 0;
> }
>
> +static bool
> +intel_uncore_is_display(const struct intel_uncore *uncore)
> +{
> + return uncore == &uncore->i915->de_uncore;
> +}
> +
> int intel_uncore_init_mmio(struct intel_uncore *uncore)
> {
> struct drm_i915_private *i915 = uncore->i915;
> @@ -1683,7 +1689,8 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
> if (ret)
> return ret;
>
> - if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915))
> + if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915) &&
> + !intel_uncore_is_display(uncore))
> uncore->flags |= UNCORE_HAS_FORCEWAKE;
>
> if (!intel_uncore_has_forcewake(uncore)) {
> --
> 2.20.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
