Re: [PATCH 13/20] drm/i915: add Punit read/write routines for VLV

[Ben Widawsky <ben@bwidawsk.net>]

On Fri, Mar 08, 2013 at 10:45:56AM -0800, Jesse Barnes wrote:
> Slightly different than other platforms.
> 
> v2 [Jani]: Fix IOSF_BYTE_ENABLES_SHIFT shift. Use common routine.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |    2 ++
>  drivers/gpu/drm/i915/i915_reg.h |   22 ++++++++++++++++
>  drivers/gpu/drm/i915/intel_pm.c |   53 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 77 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3061d73..592e944 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1849,6 +1849,8 @@ int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv);
>  
>  int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val);
>  int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val);
> +int valleyview_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
> +int valleyview_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val);
>  
>  #define __i915_read(x, y) \
>  	u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index cf291b6..1877d0e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4394,6 +4394,28 @@
>  #define GEN6_PCODE_DATA				0x138128
>  #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT	8
>  
> +#define VLV_IOSF_DOORBELL_REQ			0x182100
> +#define   IOSF_DEVFN_SHIFT			24
> +#define   IOSF_OPCODE_SHIFT			16
> +#define   IOSF_PORT_SHIFT			8
> +#define   IOSF_BYTE_ENABLES_SHIFT		4
> +#define   IOSF_BAR_SHIFT			1
> +#define   IOSF_SB_BUSY				(1<<0)
> +#define   IOSF_PORT_PUNIT			0x4

I'm not seeing where the 0x4 is defined, but I'll assume it's right.


> +#define VLV_IOSF_DATA				0x182104
> +#define VLV_IOSF_ADDR				0x182108
> +
> +#define PUNIT_REG_GPU_LFM			0xd3
> +#define PUNIT_REG_GPU_FREQ_REQ			0xd4
> +#define PUNIT_REG_GPU_FREQ_STS			0xd8
> +#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ		0xdc
> +
> +#define PUNIT_OPCODE_REG_READ			6
> +#define PUNIT_OPCODE_REG_WRITE			7
> +
> +#define PUNIT_FUSE_BUS2				0xf6 /* bits 47:40 */
> +#define PUNIT_FUSE_BUS1				0xf5 /* bits 55:48 */
> +
>  #define GEN6_GT_CORE_STATUS		0x138060
>  #define   GEN6_CORE_CPD_STATE_MASK	(7<<4)
>  #define   GEN6_RCn_MASK			7

Could probably do without polluting i915_reg.h for this, since I doubt
we'd ever use this outside of intel_pm.c (unless we have a shared driver
as I mentioned in patch 15).

> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3fae9ff..d2499eb 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4512,3 +4512,56 @@ int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val)
>  
>  	return 0;
>  }
> +
> +static int vlv_punit_rw(struct drm_i915_private *dev_priv, u8 opcode,
> +			u8 addr, u32 *val)
> +{
> +	u32 cmd, devfn, port, be, bar;
> +
> +	bar = 0;
> +	be = 0xf;
> +	port = IOSF_PORT_PUNIT;
> +	devfn = 16;
> +
> +	cmd = (devfn << IOSF_DEVFN_SHIFT) | (opcode << IOSF_OPCODE_SHIFT) |
> +		(port << IOSF_PORT_SHIFT) | (be << IOSF_BYTE_ENABLES_SHIFT) |
> +		(bar << IOSF_BAR_SHIFT);
> +
> +	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
> +
> +	if (I915_READ(VLV_IOSF_DOORBELL_REQ) & IOSF_SB_BUSY) {
> +		DRM_DEBUG_DRIVER("warning: pcode (%s) mailbox access failed\n",
> +				 opcode == PUNIT_OPCODE_REG_READ ?
> +				 "read" : "write");
> +		return -EAGAIN;
> +	}
> +
> +	I915_WRITE(VLV_IOSF_ADDR, addr);
> +	if (opcode == PUNIT_OPCODE_REG_WRITE)
> +		I915_WRITE(VLV_IOSF_DATA, *val);
> +	I915_WRITE(VLV_IOSF_DOORBELL_REQ, cmd);

The doc says to write the data register first, but I doubt it matters.

> +
> +	if (wait_for((I915_READ(VLV_IOSF_DOORBELL_REQ) & IOSF_SB_BUSY) == 0,
> +		     500)) {
> +		DRM_ERROR("timeout waiting for pcode %s (%d) to finish\n",
> +			  opcode == PUNIT_OPCODE_REG_READ ? "read" : "write",
> +			  addr);
> +		return -ETIMEDOUT;
> +	}
> +
> +	if (opcode == PUNIT_OPCODE_REG_READ)
> +		*val = I915_READ(VLV_IOSF_DATA);
> +	I915_WRITE(VLV_IOSF_DATA, 0);

The ending write also shouldn't be necessary, but I doubt it matters.

> +
> +	return 0;
> +}
> +
> +int valleyview_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
> +{
> +	return vlv_punit_rw(dev_priv, PUNIT_OPCODE_REG_READ, addr, val);
> +}
> +
> +int valleyview_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
> +{
> +	return vlv_punit_rw(dev_priv, PUNIT_OPCODE_REG_WRITE, addr, &val);
> +}

With or without bikesheds:
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

-- 
Ben Widawsky, Intel Open Source Technology Center