diff mbox

drm/915/glk: Enable pooled EUs for Geminilake

Message ID 20170224131237.18530-1-ander.conselvan.de.oliveira@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ander Conselvan de Oliveira Feb. 24, 2017, 1:12 p.m. UTC 
Geminilake also supports pooled EUs. Enable it.

It is unclear if the recommendation to disable it for 2x6 configurations
from commit e015dd69b2cf ("drm/i915/bxt: Add WaEnablePooledEuFor2x6")
should also apply to GLK, but the only userspace that uses this only
cares about the 3x6 configuration. See Beignet's commit 6901899ec90a
("Runtime: set the sub slice according to kernel pooled EU configure.").

Cc: Arun Siluvery <arun.siluvery@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Yang Rong <rong.r.yang@intel.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_device_info.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Yang, Rong R Feb. 28, 2017, 6:25 a.m. UTC | #1 
We suggest GLK could disable pooled EUs for 2x6 configurations too.
As I understand, 2x6 a pool must consist of a complete subslice, 12EUs, right?
If so, only 64K SLM are valid, I am afraid it may affect some case's performance.

> -----Original Message-----
> From: Conselvan De Oliveira, Ander
> Sent: Friday, February 24, 2017 21:13
> To: intel-gfx@lists.freedesktop.org
> Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>;
> Arun Siluvery <arun.siluvery@intel.com>; Kuoppala, Mika
> <mika.kuoppala@intel.com>; Ursulin, Tvrtko <tvrtko.ursulin@intel.com>;
> Yang, Rong R <rong.r.yang@intel.com>
> Subject: [PATCH] drm/915/glk: Enable pooled EUs for Geminilake
> 
> Geminilake also supports pooled EUs. Enable it.
> 
> It is unclear if the recommendation to disable it for 2x6 configurations from
> commit e015dd69b2cf ("drm/i915/bxt: Add WaEnablePooledEuFor2x6")
> should also apply to GLK, but the only userspace that uses this only cares
> about the 3x6 configuration. See Beignet's commit 6901899ec90a
> ("Runtime: set the sub slice according to kernel pooled EU configure.").
> 
> Cc: Arun Siluvery <arun.siluvery@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Yang Rong <rong.r.yang@intel.com>
> Signed-off-by: Ander Conselvan de Oliveira
> <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_device_info.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> b/drivers/gpu/drm/i915/intel_device_info.c
> index 2e1fd85..198752d 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -195,8 +195,10 @@ static void gen9_sseu_info_init(struct
> drm_i915_private *dev_priv)
>  		IS_GEN9_LP(dev_priv) && sseu_subslice_total(sseu) > 1;
>  	sseu->has_eu_pg = sseu->eu_per_subslice > 2;
> 
> -	if (IS_BROXTON(dev_priv)) {
> +	if (IS_GEN9_LP(dev_priv)) {
>  #define IS_SS_DISABLED(ss)	(!(sseu->subslice_mask & BIT(ss)))
> +		info->has_pooled_eu = hweight8(sseu->subslice_mask) == 3;
> +
>  		/*
>  		 * There is a HW issue in 2x6 fused down parts that requires
>  		 * Pooled EU to be enabled as a WA. The pool configuration
> @@ -204,9 +206,8 @@ static void gen9_sseu_info_init(struct
> drm_i915_private *dev_priv)
>  		 * doesn't affect if the device has all 3 subslices enabled.
>  		 */
>  		/* WaEnablePooledEuFor2x6:bxt */
> -		info->has_pooled_eu = ((hweight8(sseu->subslice_mask) ==
> 3) ||
> -				       (hweight8(sseu->subslice_mask) == 2 &&
> -					INTEL_REVID(dev_priv) <
> BXT_REVID_C0));
> +		info->has_pooled_eu |= (hweight8(sseu->subslice_mask) ==
> 2 &&
> +					IS_BXT_REVID(dev_priv, 0,
> BXT_REVID_B_LAST));
> 
>  		sseu->min_eu_in_pool = 0;
>  		if (info->has_pooled_eu) {
> --
> 2.9.3
Ander Conselvan de Oliveira March 3, 2017, 1:10 p.m. UTC | #2 
On Tue, 2017-02-28 at 06:25 +0000, Yang, Rong R wrote:
> We suggest GLK could disable pooled EUs for 2x6 configurations too.

This patch does that. I tried to say that in the commit message but now that I
re-read I see it doesn't really say it.

> As I understand, 2x6 a pool must consist of a complete subslice, 12EUs, right?

That's my understanding too.

Ander

> If so, only 64K SLM are valid, I am afraid it may affect some case's performance.
> 
> > -----Original Message-----
> > From: Conselvan De Oliveira, Ander
> > Sent: Friday, February 24, 2017 21:13
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>;
> > Arun Siluvery <arun.siluvery@intel.com>; Kuoppala, Mika
> > <mika.kuoppala@intel.com>; Ursulin, Tvrtko <tvrtko.ursulin@intel.com>;
> > Yang, Rong R <rong.r.yang@intel.com>
> > Subject: [PATCH] drm/915/glk: Enable pooled EUs for Geminilake
> > 
> > Geminilake also supports pooled EUs. Enable it.
> > 
> > It is unclear if the recommendation to disable it for 2x6 configurations from
> > commit e015dd69b2cf ("drm/i915/bxt: Add WaEnablePooledEuFor2x6")
> > should also apply to GLK, but the only userspace that uses this only cares
> > about the 3x6 configuration. See Beignet's commit 6901899ec90a
> > ("Runtime: set the sub slice according to kernel pooled EU configure.").
> > 
> > Cc: Arun Siluvery <arun.siluvery@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Yang Rong <rong.r.yang@intel.com>
> > Signed-off-by: Ander Conselvan de Oliveira
> > <ander.conselvan.de.oliveira@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_device_info.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> > b/drivers/gpu/drm/i915/intel_device_info.c
> > index 2e1fd85..198752d 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > @@ -195,8 +195,10 @@ static void gen9_sseu_info_init(struct
> > drm_i915_private *dev_priv)
> >  		IS_GEN9_LP(dev_priv) && sseu_subslice_total(sseu) > 1;
> >  	sseu->has_eu_pg = sseu->eu_per_subslice > 2;
> > 
> > -	if (IS_BROXTON(dev_priv)) {
> > +	if (IS_GEN9_LP(dev_priv)) {
> >  #define IS_SS_DISABLED(ss)	(!(sseu->subslice_mask & BIT(ss)))
> > +		info->has_pooled_eu = hweight8(sseu->subslice_mask) == 3;
> > +
> >  		/*
> >  		 * There is a HW issue in 2x6 fused down parts that requires
> >  		 * Pooled EU to be enabled as a WA. The pool configuration
> > @@ -204,9 +206,8 @@ static void gen9_sseu_info_init(struct
> > drm_i915_private *dev_priv)
> >  		 * doesn't affect if the device has all 3 subslices enabled.
> >  		 */
> >  		/* WaEnablePooledEuFor2x6:bxt */
> > -		info->has_pooled_eu = ((hweight8(sseu->subslice_mask) ==
> > 3) ||
> > -				       (hweight8(sseu->subslice_mask) == 2 &&
> > -					INTEL_REVID(dev_priv) <
> > BXT_REVID_C0));
> > +		info->has_pooled_eu |= (hweight8(sseu->subslice_mask) ==
> > 2 &&
> > +					IS_BXT_REVID(dev_priv, 0,
> > BXT_REVID_B_LAST));
> > 
> >  		sseu->min_eu_in_pool = 0;
> >  		if (info->has_pooled_eu) {
> > --
> > 2.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Mika Kuoppala March 17, 2017, 10:18 a.m. UTC | #3 
Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
writes:

> Geminilake also supports pooled EUs. Enable it.
>
> It is unclear if the recommendation to disable it for 2x6 configurations
> from commit e015dd69b2cf ("drm/i915/bxt: Add WaEnablePooledEuFor2x6")
> should also apply to GLK, but the only userspace that uses this only
> cares about the 3x6 configuration. See Beignet's commit 6901899ec90a
> ("Runtime: set the sub slice according to kernel pooled EU configure.").
>

In patch subject s/915/i915.

Also could you add explicitly that with glk, we dont tell userspace
that pooling is supported if configuration is 2x6. Apparently
to be on the safe side and that we can later lift this restriction
if it doesn't affect the performance.

-Mika


> Cc: Arun Siluvery <arun.siluvery@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Yang Rong <rong.r.yang@intel.com>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_device_info.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 2e1fd85..198752d 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -195,8 +195,10 @@ static void gen9_sseu_info_init(struct drm_i915_private *dev_priv)
>  		IS_GEN9_LP(dev_priv) && sseu_subslice_total(sseu) > 1;
>  	sseu->has_eu_pg = sseu->eu_per_subslice > 2;
>  
> -	if (IS_BROXTON(dev_priv)) {
> +	if (IS_GEN9_LP(dev_priv)) {
>  #define IS_SS_DISABLED(ss)	(!(sseu->subslice_mask & BIT(ss)))
> +		info->has_pooled_eu = hweight8(sseu->subslice_mask) == 3;
> +
>  		/*
>  		 * There is a HW issue in 2x6 fused down parts that requires
>  		 * Pooled EU to be enabled as a WA. The pool configuration
> @@ -204,9 +206,8 @@ static void gen9_sseu_info_init(struct drm_i915_private *dev_priv)
>  		 * doesn't affect if the device has all 3 subslices enabled.
>  		 */
>  		/* WaEnablePooledEuFor2x6:bxt */
> -		info->has_pooled_eu = ((hweight8(sseu->subslice_mask) == 3) ||
> -				       (hweight8(sseu->subslice_mask) == 2 &&
> -					INTEL_REVID(dev_priv) < BXT_REVID_C0));
> +		info->has_pooled_eu |= (hweight8(sseu->subslice_mask) == 2 &&
> +					IS_BXT_REVID(dev_priv, 0, BXT_REVID_B_LAST));
>  
>  		sseu->min_eu_in_pool = 0;
>  		if (info->has_pooled_eu) {
> -- 
> 2.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 2e1fd85..198752d 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -195,8 +195,10 @@  static void gen9_sseu_info_init(struct drm_i915_private *dev_priv)
 		IS_GEN9_LP(dev_priv) && sseu_subslice_total(sseu) > 1;
 	sseu->has_eu_pg = sseu->eu_per_subslice > 2;
 
-	if (IS_BROXTON(dev_priv)) {
+	if (IS_GEN9_LP(dev_priv)) {
 #define IS_SS_DISABLED(ss)	(!(sseu->subslice_mask & BIT(ss)))
+		info->has_pooled_eu = hweight8(sseu->subslice_mask) == 3;
+
 		/*
 		 * There is a HW issue in 2x6 fused down parts that requires
 		 * Pooled EU to be enabled as a WA. The pool configuration
@@ -204,9 +206,8 @@  static void gen9_sseu_info_init(struct drm_i915_private *dev_priv)
 		 * doesn't affect if the device has all 3 subslices enabled.
 		 */
 		/* WaEnablePooledEuFor2x6:bxt */
-		info->has_pooled_eu = ((hweight8(sseu->subslice_mask) == 3) ||
-				       (hweight8(sseu->subslice_mask) == 2 &&
-					INTEL_REVID(dev_priv) < BXT_REVID_C0));
+		info->has_pooled_eu |= (hweight8(sseu->subslice_mask) == 2 &&
+					IS_BXT_REVID(dev_priv, 0, BXT_REVID_B_LAST));
 
 		sseu->min_eu_in_pool = 0;
 		if (info->has_pooled_eu) {