From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: badal.nilawar@intel.com
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/1] drm/i915: dgfx cards need to wait on pcode's uncore init done
Date: Wed, 28 Jul 2021 12:33:49 -0400 [thread overview]
Message-ID: <YQGG7XBnIdzrRzx3@intel.com> (raw)
In-Reply-To: <20210727173338.901264-1-badal.nilawar@intel.com>
On Tue, Jul 27, 2021 at 11:03:38PM +0530, badal.nilawar@intel.com wrote:
> From: Badal Nilawar <badal.nilawar@intel.com>
>
> In discrete cards, the graphics driver shouldn't proceed with the probe
> or resume unless PCODE indicated everything is done, including memory
> training and gt bring up.
>
> For this reason, the driver probe and resume paths needs to be blocked
> until PCODE indicates it is done. Also, it needs to aborted if the
> notification never arrives.
>
> In general, the few miliseconds would be enough and the regular PCODE
> recommendation for the timeout was 10 seconds. However there are some
> rare cases where this initialization can take up to 1 minute. So,
> PCODE has increased the recommendation to 3 minutes so we don't fully
> block the device utilization when something just got delayed for
> whatever reason. To be on the safest side, let's accept this
> recommendation, since on the regular case it won't delay or block the
> driver initialization and resume flows
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 8 +++++++-
> drivers/gpu/drm/i915/intel_sideband.c | 13 +++++++++----
> drivers/gpu/drm/i915/intel_sideband.h | 2 +-
> 3 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index c43b698bf0b97..59fb4c710c8ca 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -620,7 +620,9 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
>
> intel_opregion_setup(dev_priv);
>
> - intel_pcode_init(dev_priv);
> + ret = intel_pcode_init(dev_priv);
> + if (ret)
> + goto err_msi;
>
> /*
> * Fill the dram structure to get the system dram info. This will be
> @@ -1231,6 +1233,10 @@ static int i915_drm_resume(struct drm_device *dev)
>
> disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>
> + ret = intel_pcode_init(dev_priv);
> + if (ret)
> + return ret;
> +
> sanitize_gpu(dev_priv);
>
> ret = i915_ggtt_enable_hw(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
> index f0a82b37bd1ac..e304bf44e1ff8 100644
> --- a/drivers/gpu/drm/i915/intel_sideband.c
> +++ b/drivers/gpu/drm/i915/intel_sideband.c
> @@ -556,17 +556,22 @@ int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request,
> #undef COND
> }
>
> -void intel_pcode_init(struct drm_i915_private *i915)
> +int intel_pcode_init(struct drm_i915_private *i915)
> {
> - int ret;
> + int ret = 0;
>
> if (!IS_DGFX(i915))
> - return;
> + return ret;
>
> ret = skl_pcode_request(i915, DG1_PCODE_STATUS,
> DG1_UNCORE_GET_INIT_STATUS,
> DG1_UNCORE_INIT_STATUS_COMPLETE,
> - DG1_UNCORE_INIT_STATUS_COMPLETE, 50);
> + DG1_UNCORE_INIT_STATUS_COMPLETE, 180000);
> +
> + drm_dbg(&i915->drm, "PCODE init status %d\n", ret);
> +
> if (ret)
> drm_err(&i915->drm, "Pcode did not report uncore initialization completion!\n");
> +
> + return ret;
> }
> diff --git a/drivers/gpu/drm/i915/intel_sideband.h b/drivers/gpu/drm/i915/intel_sideband.h
> index 094c7b19c5d42..d1d14bcb8f56e 100644
> --- a/drivers/gpu/drm/i915/intel_sideband.h
> +++ b/drivers/gpu/drm/i915/intel_sideband.h
> @@ -138,6 +138,6 @@ int sandybridge_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox,
> int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request,
> u32 reply_mask, u32 reply, int timeout_base_ms);
>
> -void intel_pcode_init(struct drm_i915_private *i915);
> +int intel_pcode_init(struct drm_i915_private *i915);
>
> #endif /* _INTEL_SIDEBAND_H */
> --
> 2.25.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
WARNING: multiple messages have this Message-ID (diff)
From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: badal.nilawar@intel.com
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/1] drm/i915: dgfx cards need to wait on pcode's uncore init done
Date: Wed, 28 Jul 2021 12:33:49 -0400 [thread overview]
Message-ID: <YQGG7XBnIdzrRzx3@intel.com> (raw)
In-Reply-To: <20210727173338.901264-1-badal.nilawar@intel.com>
On Tue, Jul 27, 2021 at 11:03:38PM +0530, badal.nilawar@intel.com wrote:
> From: Badal Nilawar <badal.nilawar@intel.com>
>
> In discrete cards, the graphics driver shouldn't proceed with the probe
> or resume unless PCODE indicated everything is done, including memory
> training and gt bring up.
>
> For this reason, the driver probe and resume paths needs to be blocked
> until PCODE indicates it is done. Also, it needs to aborted if the
> notification never arrives.
>
> In general, the few miliseconds would be enough and the regular PCODE
> recommendation for the timeout was 10 seconds. However there are some
> rare cases where this initialization can take up to 1 minute. So,
> PCODE has increased the recommendation to 3 minutes so we don't fully
> block the device utilization when something just got delayed for
> whatever reason. To be on the safest side, let's accept this
> recommendation, since on the regular case it won't delay or block the
> driver initialization and resume flows
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 8 +++++++-
> drivers/gpu/drm/i915/intel_sideband.c | 13 +++++++++----
> drivers/gpu/drm/i915/intel_sideband.h | 2 +-
> 3 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index c43b698bf0b97..59fb4c710c8ca 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -620,7 +620,9 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
>
> intel_opregion_setup(dev_priv);
>
> - intel_pcode_init(dev_priv);
> + ret = intel_pcode_init(dev_priv);
> + if (ret)
> + goto err_msi;
>
> /*
> * Fill the dram structure to get the system dram info. This will be
> @@ -1231,6 +1233,10 @@ static int i915_drm_resume(struct drm_device *dev)
>
> disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>
> + ret = intel_pcode_init(dev_priv);
> + if (ret)
> + return ret;
> +
> sanitize_gpu(dev_priv);
>
> ret = i915_ggtt_enable_hw(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
> index f0a82b37bd1ac..e304bf44e1ff8 100644
> --- a/drivers/gpu/drm/i915/intel_sideband.c
> +++ b/drivers/gpu/drm/i915/intel_sideband.c
> @@ -556,17 +556,22 @@ int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request,
> #undef COND
> }
>
> -void intel_pcode_init(struct drm_i915_private *i915)
> +int intel_pcode_init(struct drm_i915_private *i915)
> {
> - int ret;
> + int ret = 0;
>
> if (!IS_DGFX(i915))
> - return;
> + return ret;
>
> ret = skl_pcode_request(i915, DG1_PCODE_STATUS,
> DG1_UNCORE_GET_INIT_STATUS,
> DG1_UNCORE_INIT_STATUS_COMPLETE,
> - DG1_UNCORE_INIT_STATUS_COMPLETE, 50);
> + DG1_UNCORE_INIT_STATUS_COMPLETE, 180000);
> +
> + drm_dbg(&i915->drm, "PCODE init status %d\n", ret);
> +
> if (ret)
> drm_err(&i915->drm, "Pcode did not report uncore initialization completion!\n");
> +
> + return ret;
> }
> diff --git a/drivers/gpu/drm/i915/intel_sideband.h b/drivers/gpu/drm/i915/intel_sideband.h
> index 094c7b19c5d42..d1d14bcb8f56e 100644
> --- a/drivers/gpu/drm/i915/intel_sideband.h
> +++ b/drivers/gpu/drm/i915/intel_sideband.h
> @@ -138,6 +138,6 @@ int sandybridge_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox,
> int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request,
> u32 reply_mask, u32 reply, int timeout_base_ms);
>
> -void intel_pcode_init(struct drm_i915_private *i915);
> +int intel_pcode_init(struct drm_i915_private *i915);
>
> #endif /* _INTEL_SIDEBAND_H */
> --
> 2.25.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2021-07-28 16:34 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-27 17:33 [Intel-gfx] [PATCH 1/1] drm/i915: dgfx cards need to wait on pcode's uncore init done badal.nilawar
2021-07-27 17:33 ` badal.nilawar
2021-07-27 19:22 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/1] " Patchwork
2021-07-28 5:10 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2021-07-28 16:38 ` Rodrigo Vivi
2021-07-28 16:33 ` Rodrigo Vivi [this message]
2021-07-28 16:33 ` [Intel-gfx] [PATCH 1/1] " Rodrigo Vivi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YQGG7XBnIdzrRzx3@intel.com \
--to=rodrigo.vivi@intel.com \
--cc=badal.nilawar@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.