Re: [PATCH v3] drm/i915/bxt: Broxton decoupled MMIO

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

On Tue, Oct 04, 2016 at 09:16:06PM +0530, Praveen Paneri wrote:
> +#define HAS_DECOUPLED_MMIO(dev) (INTEL_INFO(dev)->has_decoupled_mmio \
> +		&& IS_BXT_REVID(dev, BXT_REVID_C0, REVID_FOREVER))

Edit dev_priv->info.has_decoupled_mmio on init.

> +static void __gen9_decoupled_mmio_access(struct drm_i915_private *dev_priv,
> +					 u32 reg,
> +					 enum forcewake_domains fw_engine,
> +					 enum decoupled_ops operation)
> +{
> +	enum decoupled_power_domains dpd_engine;
> +	u32 ctrl_reg_data = 0;
> +
> +	dpd_engine = fw2dpd_engine[fw_engine - 1];

	enum decoupled_power_domains dpd = fw2dpd_engine[fw_engine - 1];

enum decoupled_power_domain

And don't call it fw_engine. fw_domain, if you must.

> +
> +	ctrl_reg_data |= reg;
> +	ctrl_reg_data |= (operation << GEN9_DECOUPLED_OP_SHIFT);
> +	ctrl_reg_data |= (dpd_engine << GEN9_DECOUPLED_PD_SHIFT);
> +	__raw_i915_write32(dev_priv, GEN9_DECOUPLED_REG0_DW1, ctrl_reg_data);
> +
> +	ctrl_reg_data |= GEN9_DECOUPLED_DW1_GO;
> +	__raw_i915_write32(dev_priv, GEN9_DECOUPLED_REG0_DW1, ctrl_reg_data);
> +
> +	if (wait_for_atomic((__raw_i915_read32(dev_priv,
> +			GEN9_DECOUPLED_REG0_DW1) & GEN9_DECOUPLED_DW1_GO) == 0,
> +			FORCEWAKE_ACK_TIMEOUT_MS))
> +		DRM_ERROR("Decoupled MMIO wait timed out\n");
> +}
> +
> +static inline u32 __gen9_decoupled_mmio_read(struct drm_i915_private *dev_priv,
> +                                      u32 reg,
> +                                      enum forcewake_domains fw)

__gen9_decoupeld_mmio_read32()

> +{
> +	__gen9_decoupled_mmio_access(dev_priv,
> +			reg,
> +			fw_engine,
> +			GEN9_DECOUPLED_OP_READ);

	__gen9_decoupled_mmio_access(dev_priv, reg, fw, GEN9_DECOUPLED_OP_READ);

> +
> +	return __raw_i915_read32(dev_priv,
> +			GEN9_DECOUPLED_REG0_DW0);

Everywhere! Please be careful with alignment.

> +#define __gen9_decoupled_read(x) \
> +static u##x \
> +gen9_decoupled_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
> +	enum forcewake_domains fw_engine; \
> +	GEN6_READ_HEADER(x); \
> +	fw_engine = __fwtable_reg_read_fw_domains(offset); \
> +	if (!fw_engine || !(fw_engine & ~dev_priv->uncore.fw_domains_active)) { \
> +		val = __raw_i915_read##x(dev_priv, reg); \
> +	} else { \
> +		unsigned i; \
> +		u32 *ptr_data = (u32 *) &val; \
> +		for (i = 0; i < x/32; i++, offset += sizeof(u32), ptr_data++) \
> +			*ptr_data = __gen9_decoupled_mmio_read(dev_priv, \
> +						     offset, \
> +						     fw_engine); \
> +	} \
> +	GEN6_READ_FOOTER; \
> +}

Reverse it,

	if (domain & ~dev_priv->uncore.fw_domains_active) {
		u32 *ptr = (u32 *)&val;
		unsigned i;

		for (i = 0; i < x/32; i++, offset += sizeof(u32), ptr++)
			*ptr = __gen9_decoupled_mmio_read32(dev_priv, offset, domain);
	} else {
		val = __raw_i915_read##x(dev_priv, reg);
	}

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