All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ramalingam C <ramalingam.c@intel.com>
To: "José Roberto de Souza" <jose.souza@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>,
	intel-gfx@lists.freedesktop.org,
	Lucas De Marchi <lucas.demarchi@intel.com>
Subject: Re: [PATCH 1/5] drm/i915/display: Handle fused off display correctly
Date: Wed, 23 Oct 2019 18:48:30 +0530	[thread overview]
Message-ID: <20191023131830.GA22201@intel.com> (raw)
In-Reply-To: <20191019004124.371929-1-jose.souza@intel.com>

On 2019-10-18 at 17:41:20 -0700, José Roberto de Souza wrote:
> If all pipes are fused off it means that display is disabled, similar
> like we handle for GEN 7 and 8 right above but for GEN9+ spec says
> that hardware will override the pipe output to a solid color, so
> some display is there and maybe we would need to shutdown display
> to save power, so setting disable_display = true, to keep consistent
> to HAS_DISPLAY() and INTEL_DISPLAY_ENABLED().
> 
> In addition to have all pipes fused off, GEN/display 9 have the
> bit 30 "Internal Display Disable", not sure if all pipes will be set
> as unfused when this bit is set so handling both.
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Martin Peres <martin.peres@linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h          | 21 +++++++++++----------
>  drivers/gpu/drm/i915/intel_device_info.c | 14 ++++++++++----
>  2 files changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 855db888516c..6e3ae6e9cbb8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7651,16 +7651,17 @@ enum {
>  #define   MASK_WAKEMEM			(1 << 13)
>  #define   CNL_DDI_CLOCK_REG_ACCESS_ON	(1 << 7)
>  
> -#define SKL_DFSM			_MMIO(0x51000)
> -#define SKL_DFSM_CDCLK_LIMIT_MASK	(3 << 23)
> -#define SKL_DFSM_CDCLK_LIMIT_675	(0 << 23)
> -#define SKL_DFSM_CDCLK_LIMIT_540	(1 << 23)
> -#define SKL_DFSM_CDCLK_LIMIT_450	(2 << 23)
> -#define SKL_DFSM_CDCLK_LIMIT_337_5	(3 << 23)
> -#define SKL_DFSM_PIPE_A_DISABLE		(1 << 30)
> -#define SKL_DFSM_PIPE_B_DISABLE		(1 << 21)
> -#define SKL_DFSM_PIPE_C_DISABLE		(1 << 28)
> -#define TGL_DFSM_PIPE_D_DISABLE		(1 << 22)
> +#define SKL_DFSM				_MMIO(0x51000)
> +#define SKL_DFSM_INTERNAL_DISPLAY_DISABLE	(1 << 30)
> +#define SKL_DFSM_CDCLK_LIMIT_MASK		(3 << 23)
> +#define SKL_DFSM_CDCLK_LIMIT_675		(0 << 23)
> +#define SKL_DFSM_CDCLK_LIMIT_540		(1 << 23)
> +#define SKL_DFSM_CDCLK_LIMIT_450		(2 << 23)
> +#define SKL_DFSM_CDCLK_LIMIT_337_5		(3 << 23)
> +#define SKL_DFSM_PIPE_A_DISABLE			(1 << 30)
> +#define SKL_DFSM_PIPE_B_DISABLE			(1 << 21)
> +#define SKL_DFSM_PIPE_C_DISABLE			(1 << 28)
> +#define TGL_DFSM_PIPE_D_DISABLE			(1 << 22)
>  
>  #define SKL_DSSM				_MMIO(0x51004)
>  #define CNL_DSSM_CDCLK_PLL_REFCLK_24MHz		(1 << 31)
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 85e480bdc673..8d6492afdd6a 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -972,15 +972,21 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>  			enabled_mask &= ~BIT(PIPE_D);
>  
>  		/*
> -		 * At least one pipe should be enabled and if there are
> -		 * disabled pipes, they should be the last ones, with no holes
> -		 * in the mask.
> +		 * If there are disabled pipes, they should be the last ones,
> +		 * with no holes in the mask.
>  		 */
> -		if (enabled_mask == 0 || !is_power_of_2(enabled_mask + 1))
> +		if (enabled_mask && !is_power_of_2(enabled_mask + 1))
>  			DRM_ERROR("invalid pipe fuse configuration: enabled_mask=0x%x\n",
>  				  enabled_mask);
>  		else
>  			info->pipe_mask = enabled_mask;
> +
> +		if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) &&
> +		    (dfsm & SKL_DFSM_INTERNAL_DISPLAY_DISABLE))
> +			i915_modparams.disable_display = true;
> +
> +		if (!enabled_mask)
> +			i915_modparams.disable_display = true;
Do we really need to set the disable_display here? on Gen 7 and 8 when
it is fused off, we were setting pipe_mask to 0. why that wont work
here?

INTEL_NUM_PIPES and HAS_DISPLAY both are based on pipe_mask only.

-Ram
>  	}
>  
>  	/* Initialize slice/subslice/EU info */
> -- 
> 2.23.0
> 
> _______________________________________________
> 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: Ramalingam C <ramalingam.c@intel.com>
To: "José Roberto de Souza" <jose.souza@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>,
	intel-gfx@lists.freedesktop.org,
	Lucas De Marchi <lucas.demarchi@intel.com>
Subject: Re: [Intel-gfx] [PATCH 1/5] drm/i915/display: Handle fused off display correctly
Date: Wed, 23 Oct 2019 18:48:30 +0530	[thread overview]
Message-ID: <20191023131830.GA22201@intel.com> (raw)
Message-ID: <20191023131830.ugUDhSmmD_cSY2wqIIFy3wt9nlx-KPH_Cz-L1ay715Q@z> (raw)
In-Reply-To: <20191019004124.371929-1-jose.souza@intel.com>

On 2019-10-18 at 17:41:20 -0700, José Roberto de Souza wrote:
> If all pipes are fused off it means that display is disabled, similar
> like we handle for GEN 7 and 8 right above but for GEN9+ spec says
> that hardware will override the pipe output to a solid color, so
> some display is there and maybe we would need to shutdown display
> to save power, so setting disable_display = true, to keep consistent
> to HAS_DISPLAY() and INTEL_DISPLAY_ENABLED().
> 
> In addition to have all pipes fused off, GEN/display 9 have the
> bit 30 "Internal Display Disable", not sure if all pipes will be set
> as unfused when this bit is set so handling both.
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Martin Peres <martin.peres@linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h          | 21 +++++++++++----------
>  drivers/gpu/drm/i915/intel_device_info.c | 14 ++++++++++----
>  2 files changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 855db888516c..6e3ae6e9cbb8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7651,16 +7651,17 @@ enum {
>  #define   MASK_WAKEMEM			(1 << 13)
>  #define   CNL_DDI_CLOCK_REG_ACCESS_ON	(1 << 7)
>  
> -#define SKL_DFSM			_MMIO(0x51000)
> -#define SKL_DFSM_CDCLK_LIMIT_MASK	(3 << 23)
> -#define SKL_DFSM_CDCLK_LIMIT_675	(0 << 23)
> -#define SKL_DFSM_CDCLK_LIMIT_540	(1 << 23)
> -#define SKL_DFSM_CDCLK_LIMIT_450	(2 << 23)
> -#define SKL_DFSM_CDCLK_LIMIT_337_5	(3 << 23)
> -#define SKL_DFSM_PIPE_A_DISABLE		(1 << 30)
> -#define SKL_DFSM_PIPE_B_DISABLE		(1 << 21)
> -#define SKL_DFSM_PIPE_C_DISABLE		(1 << 28)
> -#define TGL_DFSM_PIPE_D_DISABLE		(1 << 22)
> +#define SKL_DFSM				_MMIO(0x51000)
> +#define SKL_DFSM_INTERNAL_DISPLAY_DISABLE	(1 << 30)
> +#define SKL_DFSM_CDCLK_LIMIT_MASK		(3 << 23)
> +#define SKL_DFSM_CDCLK_LIMIT_675		(0 << 23)
> +#define SKL_DFSM_CDCLK_LIMIT_540		(1 << 23)
> +#define SKL_DFSM_CDCLK_LIMIT_450		(2 << 23)
> +#define SKL_DFSM_CDCLK_LIMIT_337_5		(3 << 23)
> +#define SKL_DFSM_PIPE_A_DISABLE			(1 << 30)
> +#define SKL_DFSM_PIPE_B_DISABLE			(1 << 21)
> +#define SKL_DFSM_PIPE_C_DISABLE			(1 << 28)
> +#define TGL_DFSM_PIPE_D_DISABLE			(1 << 22)
>  
>  #define SKL_DSSM				_MMIO(0x51004)
>  #define CNL_DSSM_CDCLK_PLL_REFCLK_24MHz		(1 << 31)
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 85e480bdc673..8d6492afdd6a 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -972,15 +972,21 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>  			enabled_mask &= ~BIT(PIPE_D);
>  
>  		/*
> -		 * At least one pipe should be enabled and if there are
> -		 * disabled pipes, they should be the last ones, with no holes
> -		 * in the mask.
> +		 * If there are disabled pipes, they should be the last ones,
> +		 * with no holes in the mask.
>  		 */
> -		if (enabled_mask == 0 || !is_power_of_2(enabled_mask + 1))
> +		if (enabled_mask && !is_power_of_2(enabled_mask + 1))
>  			DRM_ERROR("invalid pipe fuse configuration: enabled_mask=0x%x\n",
>  				  enabled_mask);
>  		else
>  			info->pipe_mask = enabled_mask;
> +
> +		if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) &&
> +		    (dfsm & SKL_DFSM_INTERNAL_DISPLAY_DISABLE))
> +			i915_modparams.disable_display = true;
> +
> +		if (!enabled_mask)
> +			i915_modparams.disable_display = true;
Do we really need to set the disable_display here? on Gen 7 and 8 when
it is fused off, we were setting pipe_mask to 0. why that wont work
here?

INTEL_NUM_PIPES and HAS_DISPLAY both are based on pipe_mask only.

-Ram
>  	}
>  
>  	/* Initialize slice/subslice/EU info */
> -- 
> 2.23.0
> 
> _______________________________________________
> 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

  parent reply	other threads:[~2019-10-23 13:18 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-19  0:41 [PATCH 1/5] drm/i915/display: Handle fused off display correctly José Roberto de Souza
2019-10-19  0:41 ` [PATCH 2/5] drm/i915/display: Handle fused off HDCP José Roberto de Souza
2019-10-23 13:37   ` Ramalingam C
2019-10-23 13:37     ` [Intel-gfx] " Ramalingam C
2019-10-23 18:54     ` Souza, Jose
2019-10-23 18:54       ` [Intel-gfx] " Souza, Jose
2019-10-24  6:57       ` Ramalingam C
2019-10-24  6:57         ` [Intel-gfx] " Ramalingam C
2019-10-19  0:41 ` [PATCH 3/5] drm/i915/display: Check if FBC is fused off José Roberto de Souza
2019-10-23 13:50   ` Ramalingam C
2019-10-23 13:50     ` [Intel-gfx] " Ramalingam C
2019-10-19  0:41 ` [PATCH 4/5] drm/i915/display/icl+: Check if DMC " José Roberto de Souza
2019-10-24  7:06   ` Ramalingam C
2019-10-24  7:06     ` [Intel-gfx] " Ramalingam C
2019-10-19  0:41 ` [PATCH 5/5] drm/i915/display/cnl+: Handle fused off DSC José Roberto de Souza
2019-10-23 18:37   ` Manasi Navare
2019-10-23 18:37     ` [Intel-gfx] " Manasi Navare
2019-10-24  6:55   ` Ramalingam C
2019-10-24  6:55     ` [Intel-gfx] " Ramalingam C
2019-10-19  1:34 ` ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915/display: Handle fused off display correctly Patchwork
2019-10-19  4:15 ` ✓ Fi.CI.IGT: " Patchwork
2019-10-23 12:15 ` [PATCH 1/5] " Jani Nikula
2019-10-23 12:15   ` [Intel-gfx] " Jani Nikula
2019-10-23 13:18 ` Ramalingam C [this message]
2019-10-23 13:18   ` Ramalingam C
2019-10-23 13:23   ` Jani Nikula
2019-10-23 13:23     ` [Intel-gfx] " Jani Nikula
2019-10-23 19:13     ` Souza, Jose
2019-10-23 19:13       ` [Intel-gfx] " Souza, Jose
2019-10-23 13:43 ` Ramalingam C
2019-10-23 13:43   ` [Intel-gfx] " Ramalingam C

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=20191023131830.GA22201@intel.com \
    --to=ramalingam.c@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=jose.souza@intel.com \
    --cc=lucas.demarchi@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.