All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michel Thierry <michel.thierry@intel.com>
To: Rodrigo Vivi <rodrigo.vivi@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Cc: "Sharp, Sarah A" <sarah.a.sharp@intel.com>
Subject: Re: [PATCH 1/3] drm/i915/kbl: Adding missing IS_KABYLAKE checks.
Date: Thu, 7 Jan 2016 16:14:11 +0000	[thread overview]
Message-ID: <568E8ED3.4040600@intel.com> (raw)
In-Reply-To: <1452129330-3484-1-git-send-email-rodrigo.vivi@intel.com>

On 1/7/2016 1:15 AM, Rodrigo Vivi wrote:
> When adding IS_KABYLAKE definition I didn't included the
> DC states related because I was planing to include them
> with the patch that fixes DMC firmware loading, but I
> forgot them.
>
> Meanwhile this runtime pm code changed a lot for
> Skylake.
>
> Well, I didn't expect that this would crash the machine
> and I just noticed now that Sarah warned me our driver
> wasn't working. Thanks Sarah.
>
> Cc: Sarah Sharp <sarah.a.sharp@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_csr.c        |  5 +++--
>   drivers/gpu/drm/i915/intel_runtime_pm.c | 19 ++++++++++++-------
>   2 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 9bb63a8..3f69829 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -278,7 +278,8 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>
>   	csr->version = css_header->version;
>
> -	if (IS_SKYLAKE(dev) && csr->version < SKL_CSR_VERSION_REQUIRED) {
> +	if ((IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) &&
> +	     csr->version < SKL_CSR_VERSION_REQUIRED) {
>   		DRM_INFO("Refusing to load old Skylake DMC firmware v%u.%u,"
>   			 " please upgrade to v%u.%u or later"
>   			 " [https://01.org/linuxgraphics/intel-linux-graphics-firmwares].\n",
> @@ -421,7 +422,7 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv)
>   	if (!HAS_CSR(dev_priv))
>   		return;
>
> -	if (IS_SKYLAKE(dev_priv))
> +	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
>   		csr->fw_path = I915_CSR_SKL;
>   	else if (IS_BROXTON(dev_priv))
>   		csr->fw_path = I915_CSR_BXT;
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index ddbdbff..f5bf54c 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -532,7 +532,8 @@ static void assert_can_enable_dc5(struct drm_i915_private *dev_priv)
>   	bool pg2_enabled = intel_display_power_well_is_enabled(dev_priv,
>   					SKL_DISP_PW_2);
>
> -	WARN_ONCE(!IS_SKYLAKE(dev), "Platform doesn't support DC5.\n");
> +	WARN_ONCE(!IS_SKYLAKE(dev) && !IS_KABYLAKE(dev),
> +		  "Platform doesn't support DC5.\n");
>   	WARN_ONCE(!HAS_RUNTIME_PM(dev), "Runtime PM not enabled.\n");
>   	WARN_ONCE(pg2_enabled, "PG2 not disabled to enable DC5.\n");
>
> @@ -568,7 +569,8 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
>   {
>   	struct drm_device *dev = dev_priv->dev;
>
> -	WARN_ONCE(!IS_SKYLAKE(dev), "Platform doesn't support DC6.\n");
> +	WARN_ONCE(!IS_SKYLAKE(dev) && !IS_KABYLAKE(dev),
> +		  "Platform doesn't support DC6.\n");
>   	WARN_ONCE(!HAS_RUNTIME_PM(dev), "Runtime PM not enabled.\n");
>   	WARN_ONCE(I915_READ(UTIL_PIN_CTL) & UTIL_PIN_ENABLE,
>   		  "Backlight is not disabled.\n");
> @@ -595,7 +597,8 @@ static void gen9_disable_dc5_dc6(struct drm_i915_private *dev_priv)
>   {
>   	assert_can_disable_dc5(dev_priv);
>
> -	if (IS_SKYLAKE(dev_priv) && i915.enable_dc != 0 && i915.enable_dc != 1)
> +	if ((IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) &&
> +	    i915.enable_dc != 0 && i915.enable_dc != 1)
>   		assert_can_disable_dc6(dev_priv);
>
>   	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> @@ -783,7 +786,8 @@ static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
>   static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
>   					   struct i915_power_well *power_well)
>   {
> -	if (IS_SKYLAKE(dev_priv) && i915.enable_dc != 0 && i915.enable_dc != 1)
> +	if ((IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) &&
> +	    i915.enable_dc != 0 && i915.enable_dc != 1)
>   		skl_enable_dc6(dev_priv);
>   	else
>   		gen9_enable_dc5(dev_priv);
> @@ -795,7 +799,8 @@ static void gen9_dc_off_power_well_sync_hw(struct drm_i915_private *dev_priv,
>   	if (power_well->count > 0) {
>   		gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>   	} else {
> -		if (IS_SKYLAKE(dev_priv) && i915.enable_dc != 0 &&
> +		if ((IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) &&
> +		    i915.enable_dc != 0 &&
>   		    i915.enable_dc != 1)
>   			gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6);
>   		else
> @@ -1851,7 +1856,7 @@ void skl_pw1_misc_io_init(struct drm_i915_private *dev_priv)
>   {
>   	struct i915_power_well *well;
>
> -	if (!IS_SKYLAKE(dev_priv))
> +	if (!IS_SKYLAKE(dev_priv) && !IS_KABYLAKE(dev_priv))
>   		return;
>
>   	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
> @@ -1865,7 +1870,7 @@ void skl_pw1_misc_io_fini(struct drm_i915_private *dev_priv)
>   {
>   	struct i915_power_well *well;
>
> -	if (!IS_SKYLAKE(dev_priv))
> +	if (!IS_SKYLAKE(dev_priv) && !IS_KABYLAKE(dev_priv))
>   		return;
>
>   	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
>

Hi Rodrigo,

I had the same issue with the power wells and sent the same fix 
yesterday 
(http://cgit.freedesktop.org/drm-intel/commit/?id=16fbc291cb87c7defcd13ad715d3e4af0d523e43).

I didn't try to use DMC, so I didn't catch the other problem.

Without the now redundant changes in skl_pw1_misc_io_init/fini,

Reviewed-by: Michel Thierry <michel.thierry@intel.com>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2016-01-07 16:14 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-07  1:15 [PATCH 1/3] drm/i915/kbl: Adding missing IS_KABYLAKE checks Rodrigo Vivi
2016-01-07  1:15 ` [PATCH 2/3] drm/i915: Don't warn if the workaround list is empty part 2 Rodrigo Vivi
2016-01-08  0:49   ` Rodrigo Vivi
2016-01-07  1:15 ` [PATCH 3/3] drm/i915/kbl: Remove preliminary_hw_support protection from Kabylake Rodrigo Vivi
2016-01-07  9:30   ` Daniel Vetter
2016-01-07 16:14 ` Michel Thierry [this message]
2016-01-08  0:32   ` [PATCH 1/3] drm/i915/kbl: Adding missing IS_KABYLAKE checks Vivi, Rodrigo
2016-01-08  0:49   ` [PATCH] " Rodrigo Vivi
2016-02-26  0:23     ` Rodrigo Vivi
2016-02-29 14:35       ` Jani Nikula
2016-03-16 14:31         ` Michel Thierry
2016-03-16 15:15           ` Jani Nikula
2016-03-16 15:32             ` Daniel Vetter

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=568E8ED3.4040600@intel.com \
    --to=michel.thierry@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=rodrigo.vivi@intel.com \
    --cc=sarah.a.sharp@intel.com \
    /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.