[Intel-gfx] [PATCH 2/2] drm/i915: Eliminate HAS_HW_CONTEXTS

Thu Apr 27 14:36:55 UTC 2017

On Thu, Apr 27, 2017 at 04:41:33PM +0300, Joonas Lahtinen wrote:
> According to Chris i915_gem_sanitize was meant to reset ILK too.

In that case drawing the line before g4x might make more sense
since it already has a GPU reset that doesn't clobber the display.

> 
> CCID register existed already on ILK according to the PRM (Chris
> verified the address to match too).

I think it has existed since forever actually. Well, not sure about
gen0-1.

> 
> HAS_HW_CONTEXTS in i915_l3_write is bogus because each HAS_L3_DPF
> match also has .has_hw_contexts = 1 set.
> 
> This leads to us being able to get rid of the property completely.

There seem to be several changes in here. Would it not be better to
split this up into functional and non-functional patches so that if
there's a regression you wouldn't have to revert the entire thing?

> 
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       | 2 --
>  drivers/gpu/drm/i915/i915_gem.c       | 2 +-
>  drivers/gpu/drm/i915/i915_gpu_error.c | 6 +++---
>  drivers/gpu/drm/i915/i915_pci.c       | 5 -----
>  drivers/gpu/drm/i915/i915_sysfs.c     | 3 ---
>  5 files changed, 4 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e68edf1..cfa5689 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -822,7 +822,6 @@ struct intel_csr {
>  	func(has_gmch_display); \
>  	func(has_guc); \
>  	func(has_hotplug); \
> -	func(has_hw_contexts); \
>  	func(has_l3_dpf); \
>  	func(has_llc); \
>  	func(has_logical_ring_contexts); \
> @@ -2866,7 +2865,6 @@ intel_info(const struct drm_i915_private *dev_priv)
>  
>  #define HWS_NEEDS_PHYSICAL(dev_priv)	((dev_priv)->info.hws_needs_physical)
>  
> -#define HAS_HW_CONTEXTS(dev_priv)	    ((dev_priv)->info.has_hw_contexts)
>  #define HAS_LOGICAL_RING_CONTEXTS(dev_priv) \
>  		((dev_priv)->info.has_logical_ring_contexts)
>  #define USES_PPGTT(dev_priv)		(i915.enable_ppgtt)
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 33fb11c..7c6048a 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4488,7 +4488,7 @@ void i915_gem_sanitize(struct drm_i915_private *i915)
>  	 * of the reset, so we only reset recent machines with logical
>  	 * context support (that must be reset to remove any stray contexts).
>  	 */
> -	if (HAS_HW_CONTEXTS(i915)) {
> +	if (INTEL_GEN(i915) >= 5) {
>  		int reset = intel_gpu_reset(i915, ALL_ENGINES);
>  		WARN_ON(reset && reset != -ENODEV);
>  	}
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 4b247b0..ec526d9 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -1598,6 +1598,9 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>  		error->done_reg = I915_READ(DONE_REG);
>  	}
>  
> +	if (INTEL_GEN(dev_priv) >= 5)
> +		error->ccid = I915_READ(CCID);
> +
>  	/* 3: Feature specific registers */
>  	if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv)) {
>  		error->gam_ecochk = I915_READ(GAM_ECOCHK);
> @@ -1605,9 +1608,6 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>  	}
>  
>  	/* 4: Everything else */
> -	if (HAS_HW_CONTEXTS(dev_priv))
> -		error->ccid = I915_READ(CCID);
> -
>  	if (INTEL_GEN(dev_priv) >= 8) {
>  		error->ier = I915_READ(GEN8_DE_MISC_IER);
>  		for (i = 0; i < 4; i++)
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index f87b0c4..f80db2c 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -220,7 +220,6 @@ static const struct intel_device_info intel_ironlake_m_info = {
>  	.has_rc6 = 1, \
>  	.has_rc6p = 1, \
>  	.has_gmbus_irq = 1, \
> -	.has_hw_contexts = 1, \
>  	.has_aliasing_ppgtt = 1, \
>  	GEN_DEFAULT_PIPEOFFSETS, \
>  	CURSOR_OFFSETS
> @@ -245,7 +244,6 @@ static const struct intel_device_info intel_sandybridge_m_info = {
>  	.has_rc6 = 1, \
>  	.has_rc6p = 1, \
>  	.has_gmbus_irq = 1, \
> -	.has_hw_contexts = 1, \
>  	.has_aliasing_ppgtt = 1, \
>  	.has_full_ppgtt = 1, \
>  	GEN_DEFAULT_PIPEOFFSETS, \
> @@ -280,7 +278,6 @@ static const struct intel_device_info intel_valleyview_info = {
>  	.has_runtime_pm = 1,
>  	.has_rc6 = 1,
>  	.has_gmbus_irq = 1,
> -	.has_hw_contexts = 1,
>  	.has_gmch_display = 1,
>  	.has_hotplug = 1,
>  	.has_aliasing_ppgtt = 1,
> @@ -340,7 +337,6 @@ static const struct intel_device_info intel_cherryview_info = {
>  	.has_resource_streamer = 1,
>  	.has_rc6 = 1,
>  	.has_gmbus_irq = 1,
> -	.has_hw_contexts = 1,
>  	.has_logical_ring_contexts = 1,
>  	.has_gmch_display = 1,
>  	.has_aliasing_ppgtt = 1,
> @@ -387,7 +383,6 @@ static const struct intel_device_info intel_skylake_gt3_info = {
>  	.has_rc6 = 1, \
>  	.has_dp_mst = 1, \
>  	.has_gmbus_irq = 1, \
> -	.has_hw_contexts = 1, \
>  	.has_logical_ring_contexts = 1, \
>  	.has_guc = 1, \
>  	.has_decoupled_mmio = 1, \
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index f3fdfda..a6ad1c2 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -185,9 +185,6 @@ i915_l3_write(struct file *filp, struct kobject *kobj,
>  	int slice = (int)(uintptr_t)attr->private;
>  	int ret;
>  
> -	if (!HAS_HW_CONTEXTS(dev_priv))
> -		return -ENXIO;
> -
>  	ret = l3_access_valid(dev_priv, offset);
>  	if (ret)
>  		return ret;
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC