Re: [PATCH 3/3] drm/i915: Only grab correct forcewake for the engine with execlists

[Chris Wilson <chris@chris-wilson.co.uk>]

On Thu, Apr 07, 2016 at 03:05:40PM +0100, Tvrtko Ursulin wrote:
> @@ -2099,6 +2101,12 @@ logical_ring_init(struct drm_device *dev, struct intel_engine_cs *engine)
>  
>  	logical_ring_init_platform_invariants(engine);
>  
> +	engine->fw_domains_elsp =
> +		intel_reg_write_fw_domains(dev_priv, RING_ELSP(engine));
> +	engine->fw_domains_csb =
> +		intel_reg_write_fw_domains(dev_priv,
> +					   RING_CONTEXT_STATUS_PTR(engine));

So is write a superset of fw? Tends to be reads that require fw more
than writes (gen6/7 fifo, gen8 write shadowing).

I think we need a READ | WRITE direction field.

> +/**
> + * intel_reg_write_fw_domains - which forcewake domains are needed to write a register
> + * @dev_priv: pointer to struct drm_i915_private
> + * @reg: register in question
> + *
> + * Returns a set of forcewake domains required to be taken with for example
> + * intel_uncore_forcewake_get for the specified register to be writable with the
> + * raw mmio accessors.
> + */
> +enum forcewake_domains
> +intel_reg_write_fw_domains(struct drm_i915_private *dev_priv, i915_reg_t reg)
> +{
> +	enum forcewake_domains fw_domains;
> +
> +	if (intel_vgpu_active(dev_priv->dev))
> +		return 0;
> +
> +	switch (INTEL_INFO(dev_priv)->gen) {
> +	case 9:
> +		fw_domains = __gen9_reg_write_fw_domains(i915_mmio_reg_offset(reg));
> +		break;
> +	case 8:
> +		if (IS_CHERRYVIEW(dev_priv))
> +			fw_domains = __chv_reg_write_fw_domains(i915_mmio_reg_offset(reg));
> +		else
> +			fw_domains = __gen8_reg_write_fw_domains(i915_mmio_reg_offset(reg));
> +		break;
> +	default:
> +		MISSING_CASE(INTEL_INFO(dev_priv)->gen);
> +	case 7:
> +	case 6:

This is actually a tricky one. gen6/7 maintain a FIFO to store mmio
writes whilst it is powered down. If we fill that fifo we drop writes
(and that fifo is shared with functions on the device, i.e. it is not
ours to fill exclusively). So should we be saving that if you want to
make lots of writes you should take this forcewake domain. Yes. We should
report what domains they would require, it is still up to the caller as
to whether they risk the FIFO overflowing, but they should have the right
information to hand.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx