diff mbox series

[2/9] drm/i915: add HAS_FORCEWAKE flag to uncore

Message ID 20190325214940.23632-3-daniele.ceraolospurio@intel.com (mailing list archive)
State New, archived
Headers show
Series more uncore rework | expand

Commit Message

Daniele Ceraolo Spurio March 25, 2019, 9:49 p.m. UTC
We have several cases where we don't have forcewake (older gens, GVT and
planned display-only uncore), so, instead of checking every time against
the various condition, save the info in a flag and use that.

Note that this patch also change the behavior for gen5 with vpgu
enabled, but this is not an issue since we don't support vgpu on gen5.

v2: split out from previous path, fix check for missing case (Paulo)

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_uncore.c | 31 +++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_uncore.h |  3 +++
 2 files changed, 24 insertions(+), 10 deletions(-)

Comments

Zanoni, Paulo R March 26, 2019, 12:13 a.m. UTC | #1
Em seg, 2019-03-25 às 14:49 -0700, Daniele Ceraolo Spurio escreveu:
> We have several cases where we don't have forcewake (older gens, GVT and
> planned display-only uncore), so, instead of checking every time against
> the various condition, save the info in a flag and use that.
> 
> Note that this patch also change the behavior for gen5 with vpgu
> enabled, but this is not an issue since we don't support vgpu on gen5.
> 
> v2: split out from previous path, fix check for missing case (Paulo)

Much better as a separate patch. Thanks.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 31 +++++++++++++++++++----------
>  drivers/gpu/drm/i915/intel_uncore.h |  3 +++
>  2 files changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 3f74889c4212..0259a61a745f 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1391,7 +1391,7 @@ static void intel_uncore_fw_domains_init(struct intel_uncore *uncore)
>  {
>  	struct drm_i915_private *i915 = uncore_to_i915(uncore);
>  
> -	if (INTEL_GEN(i915) <= 5 || intel_vgpu_active(i915))
> +	if (!(uncore->flags & UNCORE_HAS_FORCEWAKE))
>  		return;
>  
>  	if (INTEL_GEN(i915) >= 11) {
> @@ -1590,6 +1590,9 @@ int intel_uncore_init(struct intel_uncore *uncore)
>  
>  	i915_check_vgpu(i915);
>  
> +	if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915))
> +		uncore->flags |= UNCORE_HAS_FORCEWAKE;
> +
>  	intel_uncore_edram_detect(i915);
>  	intel_uncore_fw_domains_init(uncore);
>  	__intel_uncore_early_sanitize(uncore, 0);
> @@ -1598,12 +1601,14 @@ int intel_uncore_init(struct intel_uncore *uncore)
>  	uncore->pmic_bus_access_nb.notifier_call =
>  		i915_pmic_bus_access_notifier;
>  
> -	if (IS_GEN_RANGE(i915, 2, 4) || intel_vgpu_active(i915)) {
> -		ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen2);
> -		ASSIGN_READ_MMIO_VFUNCS(uncore, gen2);
> -	} else if (IS_GEN(i915, 5)) {
> -		ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen5);
> -		ASSIGN_READ_MMIO_VFUNCS(uncore, gen5);
> +	if (!(uncore->flags & UNCORE_HAS_FORCEWAKE)) {
> +		if (IS_GEN(i915, 5)) {
> +			ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen5);
> +			ASSIGN_READ_MMIO_VFUNCS(uncore, gen5);
> +		} else {
> +			ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen2);
> +			ASSIGN_READ_MMIO_VFUNCS(uncore, gen2);
> +		}
>  	} else if (IS_GEN_RANGE(i915, 6, 7)) {
>  		ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen6);
>  
> @@ -1912,7 +1917,10 @@ intel_uncore_forcewake_for_read(struct drm_i915_private *dev_priv,
>  	} else if (INTEL_GEN(dev_priv) >= 6) {
>  		fw_domains = __gen6_reg_read_fw_domains(uncore, offset);
>  	} else {
> -		WARN_ON(!IS_GEN_RANGE(dev_priv, 2, 5));
> +		/* on devices with FW we expect to hit one of the above cases */
> +		if (unlikely(uncore->flags & UNCORE_HAS_FORCEWAKE))
> +			MISSING_CASE(INTEL_GEN(dev_priv));
> +
>  		fw_domains = 0;
>  	}
>  
> @@ -1938,7 +1946,10 @@ intel_uncore_forcewake_for_write(struct drm_i915_private *dev_priv,
>  	} else if (IS_GEN_RANGE(dev_priv, 6, 7)) {
>  		fw_domains = FORCEWAKE_RENDER;
>  	} else {
> -		WARN_ON(!IS_GEN_RANGE(dev_priv, 2, 5));
> +		/* on devices with FW we expect to hit one of the above cases */
> +		if (unlikely(uncore->flags & UNCORE_HAS_FORCEWAKE))
> +			MISSING_CASE(INTEL_GEN(dev_priv));
> +
>  		fw_domains = 0;
>  	}
>  
> @@ -1969,7 +1980,7 @@ intel_uncore_forcewake_for_reg(struct drm_i915_private *dev_priv,
>  
>  	WARN_ON(!op);
>  
> -	if (intel_vgpu_active(dev_priv))
> +	if (!(dev_priv->uncore.flags & UNCORE_HAS_FORCEWAKE))
>  		return 0;
>  
>  	if (op & FW_REG_READ)
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
> index f7670cbc41c9..4947542c6ea7 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -127,6 +127,9 @@ struct intel_uncore {
>  	} user_forcewake;
>  
>  	int unclaimed_mmio_check;
> +
> +	u32 flags;
> +#define UNCORE_HAS_FORCEWAKE		BIT(0)
>  };
>  
>  /* Iterate over initialised fw domains */
Michal Wajdeczko March 26, 2019, 12:47 p.m. UTC | #2
On Mon, 25 Mar 2019 22:49:33 +0100, Daniele Ceraolo Spurio  
<daniele.ceraolospurio@intel.com> wrote:

> We have several cases where we don't have forcewake (older gens, GVT and
> planned display-only uncore), so, instead of checking every time against
> the various condition, save the info in a flag and use that.
>
> Note that this patch also change the behavior for gen5 with vpgu
> enabled, but this is not an issue since we don't support vgpu on gen5.
>
> v2: split out from previous path, fix check for missing case (Paulo)
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 31 +++++++++++++++++++----------
>  drivers/gpu/drm/i915/intel_uncore.h |  3 +++
>  2 files changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c  
> b/drivers/gpu/drm/i915/intel_uncore.c
> index 3f74889c4212..0259a61a745f 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1391,7 +1391,7 @@ static void intel_uncore_fw_domains_init(struct  
> intel_uncore *uncore)
>  {
>  	struct drm_i915_private *i915 = uncore_to_i915(uncore);
> -	if (INTEL_GEN(i915) <= 5 || intel_vgpu_active(i915))
> +	if (!(uncore->flags & UNCORE_HAS_FORCEWAKE))
>  		return;
> 	if (INTEL_GEN(i915) >= 11) {
> @@ -1590,6 +1590,9 @@ int intel_uncore_init(struct intel_uncore *uncore)
> 	i915_check_vgpu(i915);
> +	if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915))
> +		uncore->flags |= UNCORE_HAS_FORCEWAKE;
> +
>  	intel_uncore_edram_detect(i915);
>  	intel_uncore_fw_domains_init(uncore);
>  	__intel_uncore_early_sanitize(uncore, 0);
> @@ -1598,12 +1601,14 @@ int intel_uncore_init(struct intel_uncore  
> *uncore)
>  	uncore->pmic_bus_access_nb.notifier_call =
>  		i915_pmic_bus_access_notifier;
> -	if (IS_GEN_RANGE(i915, 2, 4) || intel_vgpu_active(i915)) {
> -		ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen2);
> -		ASSIGN_READ_MMIO_VFUNCS(uncore, gen2);
> -	} else if (IS_GEN(i915, 5)) {
> -		ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen5);
> -		ASSIGN_READ_MMIO_VFUNCS(uncore, gen5);
> +	if (!(uncore->flags & UNCORE_HAS_FORCEWAKE)) {
> +		if (IS_GEN(i915, 5)) {
> +			ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen5);
> +			ASSIGN_READ_MMIO_VFUNCS(uncore, gen5);
> +		} else {
> +			ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen2);
> +			ASSIGN_READ_MMIO_VFUNCS(uncore, gen2);
> +		}
>  	} else if (IS_GEN_RANGE(i915, 6, 7)) {
>  		ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen6);
> @@ -1912,7 +1917,10 @@ intel_uncore_forcewake_for_read(struct  
> drm_i915_private *dev_priv,
>  	} else if (INTEL_GEN(dev_priv) >= 6) {
>  		fw_domains = __gen6_reg_read_fw_domains(uncore, offset);
>  	} else {
> -		WARN_ON(!IS_GEN_RANGE(dev_priv, 2, 5));
> +		/* on devices with FW we expect to hit one of the above cases */
> +		if (unlikely(uncore->flags & UNCORE_HAS_FORCEWAKE))
> +			MISSING_CASE(INTEL_GEN(dev_priv));
> +
>  		fw_domains = 0;
>  	}
> @@ -1938,7 +1946,10 @@ intel_uncore_forcewake_for_write(struct  
> drm_i915_private *dev_priv,
>  	} else if (IS_GEN_RANGE(dev_priv, 6, 7)) {
>  		fw_domains = FORCEWAKE_RENDER;
>  	} else {
> -		WARN_ON(!IS_GEN_RANGE(dev_priv, 2, 5));
> +		/* on devices with FW we expect to hit one of the above cases */
> +		if (unlikely(uncore->flags & UNCORE_HAS_FORCEWAKE))
> +			MISSING_CASE(INTEL_GEN(dev_priv));
> +
>  		fw_domains = 0;
>  	}
> @@ -1969,7 +1980,7 @@ intel_uncore_forcewake_for_reg(struct  
> drm_i915_private *dev_priv,
> 	WARN_ON(!op);
> -	if (intel_vgpu_active(dev_priv))
> +	if (!(dev_priv->uncore.flags & UNCORE_HAS_FORCEWAKE))

nit: as this condition is used in few places, maybe we can add nice helper:

static inline bool intel_uncore_has_forcewake(struct intel_uncore *uncore)
{
	return uncore->flags & UNCORE_HAS_FORCEWAKE;
}

Michal

>  		return 0;
> 	if (op & FW_REG_READ)
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h  
> b/drivers/gpu/drm/i915/intel_uncore.h
> index f7670cbc41c9..4947542c6ea7 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -127,6 +127,9 @@ struct intel_uncore {
>  	} user_forcewake;
> 	int unclaimed_mmio_check;
> +
> +	u32 flags;
> +#define UNCORE_HAS_FORCEWAKE		BIT(0)
>  };
> /* Iterate over initialised fw domains */
Chris Wilson March 26, 2019, 9:48 p.m. UTC | #3
Quoting Michal Wajdeczko (2019-03-26 12:47:00)
> On Mon, 25 Mar 2019 22:49:33 +0100, Daniele Ceraolo Spurio  
> <daniele.ceraolospurio@intel.com> wrote:
> 
> > We have several cases where we don't have forcewake (older gens, GVT and
> > planned display-only uncore), so, instead of checking every time against
> > the various condition, save the info in a flag and use that.
> >
> > Note that this patch also change the behavior for gen5 with vpgu
> > enabled, but this is not an issue since we don't support vgpu on gen5.
> >
> > v2: split out from previous path, fix check for missing case (Paulo)
> >
> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/intel_uncore.c | 31 +++++++++++++++++++----------
> >  drivers/gpu/drm/i915/intel_uncore.h |  3 +++
> >  2 files changed, 24 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c  
> > b/drivers/gpu/drm/i915/intel_uncore.c
> > index 3f74889c4212..0259a61a745f 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -1391,7 +1391,7 @@ static void intel_uncore_fw_domains_init(struct  
> > intel_uncore *uncore)
> >  {
> >       struct drm_i915_private *i915 = uncore_to_i915(uncore);
> > -     if (INTEL_GEN(i915) <= 5 || intel_vgpu_active(i915))
> > +     if (!(uncore->flags & UNCORE_HAS_FORCEWAKE))
> >               return;
> >       if (INTEL_GEN(i915) >= 11) {
> > @@ -1590,6 +1590,9 @@ int intel_uncore_init(struct intel_uncore *uncore)
> >       i915_check_vgpu(i915);
> > +     if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915))
> > +             uncore->flags |= UNCORE_HAS_FORCEWAKE;
> > +
> >       intel_uncore_edram_detect(i915);
> >       intel_uncore_fw_domains_init(uncore);
> >       __intel_uncore_early_sanitize(uncore, 0);
> > @@ -1598,12 +1601,14 @@ int intel_uncore_init(struct intel_uncore  
> > *uncore)
> >       uncore->pmic_bus_access_nb.notifier_call =
> >               i915_pmic_bus_access_notifier;
> > -     if (IS_GEN_RANGE(i915, 2, 4) || intel_vgpu_active(i915)) {
> > -             ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen2);
> > -             ASSIGN_READ_MMIO_VFUNCS(uncore, gen2);
> > -     } else if (IS_GEN(i915, 5)) {
> > -             ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen5);
> > -             ASSIGN_READ_MMIO_VFUNCS(uncore, gen5);
> > +     if (!(uncore->flags & UNCORE_HAS_FORCEWAKE)) {
> > +             if (IS_GEN(i915, 5)) {
> > +                     ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen5);
> > +                     ASSIGN_READ_MMIO_VFUNCS(uncore, gen5);
> > +             } else {
> > +                     ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen2);
> > +                     ASSIGN_READ_MMIO_VFUNCS(uncore, gen2);
> > +             }
> >       } else if (IS_GEN_RANGE(i915, 6, 7)) {
> >               ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen6);
> > @@ -1912,7 +1917,10 @@ intel_uncore_forcewake_for_read(struct  
> > drm_i915_private *dev_priv,
> >       } else if (INTEL_GEN(dev_priv) >= 6) {
> >               fw_domains = __gen6_reg_read_fw_domains(uncore, offset);
> >       } else {
> > -             WARN_ON(!IS_GEN_RANGE(dev_priv, 2, 5));
> > +             /* on devices with FW we expect to hit one of the above cases */
> > +             if (unlikely(uncore->flags & UNCORE_HAS_FORCEWAKE))
> > +                     MISSING_CASE(INTEL_GEN(dev_priv));
> > +
> >               fw_domains = 0;
> >       }
> > @@ -1938,7 +1946,10 @@ intel_uncore_forcewake_for_write(struct  
> > drm_i915_private *dev_priv,
> >       } else if (IS_GEN_RANGE(dev_priv, 6, 7)) {
> >               fw_domains = FORCEWAKE_RENDER;
> >       } else {
> > -             WARN_ON(!IS_GEN_RANGE(dev_priv, 2, 5));
> > +             /* on devices with FW we expect to hit one of the above cases */
> > +             if (unlikely(uncore->flags & UNCORE_HAS_FORCEWAKE))
> > +                     MISSING_CASE(INTEL_GEN(dev_priv));
> > +
> >               fw_domains = 0;
> >       }
> > @@ -1969,7 +1980,7 @@ intel_uncore_forcewake_for_reg(struct  
> > drm_i915_private *dev_priv,
> >       WARN_ON(!op);
> > -     if (intel_vgpu_active(dev_priv))
> > +     if (!(dev_priv->uncore.flags & UNCORE_HAS_FORCEWAKE))
> 
> nit: as this condition is used in few places, maybe we can add nice helper:
> 
> static inline bool intel_uncore_has_forcewake(struct intel_uncore *uncore)
> {
>         return uncore->flags & UNCORE_HAS_FORCEWAKE;
> }

Did a quick pass to apply the nit and quieten checkpatch and pushed.

Thanks,
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 3f74889c4212..0259a61a745f 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1391,7 +1391,7 @@  static void intel_uncore_fw_domains_init(struct intel_uncore *uncore)
 {
 	struct drm_i915_private *i915 = uncore_to_i915(uncore);
 
-	if (INTEL_GEN(i915) <= 5 || intel_vgpu_active(i915))
+	if (!(uncore->flags & UNCORE_HAS_FORCEWAKE))
 		return;
 
 	if (INTEL_GEN(i915) >= 11) {
@@ -1590,6 +1590,9 @@  int intel_uncore_init(struct intel_uncore *uncore)
 
 	i915_check_vgpu(i915);
 
+	if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915))
+		uncore->flags |= UNCORE_HAS_FORCEWAKE;
+
 	intel_uncore_edram_detect(i915);
 	intel_uncore_fw_domains_init(uncore);
 	__intel_uncore_early_sanitize(uncore, 0);
@@ -1598,12 +1601,14 @@  int intel_uncore_init(struct intel_uncore *uncore)
 	uncore->pmic_bus_access_nb.notifier_call =
 		i915_pmic_bus_access_notifier;
 
-	if (IS_GEN_RANGE(i915, 2, 4) || intel_vgpu_active(i915)) {
-		ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen2);
-		ASSIGN_READ_MMIO_VFUNCS(uncore, gen2);
-	} else if (IS_GEN(i915, 5)) {
-		ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen5);
-		ASSIGN_READ_MMIO_VFUNCS(uncore, gen5);
+	if (!(uncore->flags & UNCORE_HAS_FORCEWAKE)) {
+		if (IS_GEN(i915, 5)) {
+			ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen5);
+			ASSIGN_READ_MMIO_VFUNCS(uncore, gen5);
+		} else {
+			ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen2);
+			ASSIGN_READ_MMIO_VFUNCS(uncore, gen2);
+		}
 	} else if (IS_GEN_RANGE(i915, 6, 7)) {
 		ASSIGN_WRITE_MMIO_VFUNCS(uncore, gen6);
 
@@ -1912,7 +1917,10 @@  intel_uncore_forcewake_for_read(struct drm_i915_private *dev_priv,
 	} else if (INTEL_GEN(dev_priv) >= 6) {
 		fw_domains = __gen6_reg_read_fw_domains(uncore, offset);
 	} else {
-		WARN_ON(!IS_GEN_RANGE(dev_priv, 2, 5));
+		/* on devices with FW we expect to hit one of the above cases */
+		if (unlikely(uncore->flags & UNCORE_HAS_FORCEWAKE))
+			MISSING_CASE(INTEL_GEN(dev_priv));
+
 		fw_domains = 0;
 	}
 
@@ -1938,7 +1946,10 @@  intel_uncore_forcewake_for_write(struct drm_i915_private *dev_priv,
 	} else if (IS_GEN_RANGE(dev_priv, 6, 7)) {
 		fw_domains = FORCEWAKE_RENDER;
 	} else {
-		WARN_ON(!IS_GEN_RANGE(dev_priv, 2, 5));
+		/* on devices with FW we expect to hit one of the above cases */
+		if (unlikely(uncore->flags & UNCORE_HAS_FORCEWAKE))
+			MISSING_CASE(INTEL_GEN(dev_priv));
+
 		fw_domains = 0;
 	}
 
@@ -1969,7 +1980,7 @@  intel_uncore_forcewake_for_reg(struct drm_i915_private *dev_priv,
 
 	WARN_ON(!op);
 
-	if (intel_vgpu_active(dev_priv))
+	if (!(dev_priv->uncore.flags & UNCORE_HAS_FORCEWAKE))
 		return 0;
 
 	if (op & FW_REG_READ)
diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index f7670cbc41c9..4947542c6ea7 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -127,6 +127,9 @@  struct intel_uncore {
 	} user_forcewake;
 
 	int unclaimed_mmio_check;
+
+	u32 flags;
+#define UNCORE_HAS_FORCEWAKE		BIT(0)
 };
 
 /* Iterate over initialised fw domains */