Re: [GPIO PATCH 1/2] drm/i915: GPIO for CHT generic MIPI

[Jani Nikula <jani.nikula@intel.com>]

On Mon, 22 Feb 2016, Deepak M <m.deepak@intel.com> wrote:
> From: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
>
> The GPIO configuration and register offsets are different from
> baytrail for cherrytrail. Port the gpio programming accordingly
> for cherrytrail in this patch.
>
> v2: Removing the duplication of parsing
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> Signed-off-by: Deepak M <m.deepak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h            |  20 ++++++
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 107 ++++++++++++++++++++++-------
>  2 files changed, 102 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 606dc71..fc57477 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -615,6 +615,16 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define   IOSF_PORT_NC				0x11
>  #define   IOSF_PORT_DPIO			0x12
>  #define   IOSF_PORT_GPIO_NC			0x13
> +#define   CHV_IOSF_PORT_GPIO_N			0x13
> +#define   CHV_IOSF_PORT_GPIO_SE			0x48
> +#define   CHV_IOSF_PORT_GPIO_SW			0xB2
> +#define   CHV_IOSF_PORT_GPIO_E			0xA8
> +#define   CHV_MAX_GPIO_NUM_N			72
> +#define   CHV_MAX_GPIO_NUM_SE			99
> +#define   CHV_MAX_GPIO_NUM_SW			197
> +#define   CHV_MIN_GPIO_NUM_SE			73
> +#define   CHV_MIN_GPIO_NUM_SW			100
> +#define   CHV_MIN_GPIO_NUM_E			198

These GPIO num definitions are not part of the *register*
definition. It's confusing to have them here.

I think for now I'd just stick them in intel_dsi_panel_vbt.c where you
use them.

>  #define   IOSF_PORT_CCK				0x14
>  #define   IOSF_PORT_DPIO_2			0x1a
>  #define   IOSF_PORT_FLISDSI			0x1b
> @@ -630,6 +640,16 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define VLV_GPIO_CFG				0x2000CC00
>  #define VLV_GPIO_INPUT_DIS			0x04
>  
> +#define CHV_PAD_FMLY_BASE			0x4400
> +#define CHV_PAD_FMLY_SIZE			0x400
> +#define CHV_PAD_CFG_0_1_REG_SIZE		0x8
> +#define CHV_PAD_CFG_REG_SIZE			0x4
> +#define CHV_VBT_MAX_PINS_PER_FMLY		15
> +
> +#define CHV_GPIO_CFG_UNLOCK			0x00000000
> +#define CHV_GPIO_CFG_HIZ			0x00008100
> +#define CHV_GPIO_CFG_TX_STATE_SHIFT		1

Ditto.

> +
>  /* See configdb bunit SB addr map */
>  #define BUNIT_REG_BISOC				0x11
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 794bd1f..4849515 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -685,34 +685,13 @@ static const u8 *mipi_exec_delay(struct intel_dsi *intel_dsi, const u8 *data)
>  	return data;
>  }
>  
> -static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
> +void vlv_program_gpio(struct intel_dsi *intel_dsi, u8 gpio, u8 action)
>  {
> -	u8 gpio, action;
> +	struct drm_device *dev = intel_dsi->base.base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u16 function, pad;
>  	u32 val;
>  	u8 port;
> -	struct drm_device *dev = intel_dsi->base.base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	DRM_DEBUG_DRIVER("MIPI: executing gpio element\n");
> -
> -	if (dev_priv->vbt.dsi.seq_version >= 3)
> -		data++;
> -
> -	gpio = *data++;
> -
> -	/* pull up/down */
> -	action = *data++ & 1;
> -
> -	if (gpio >= ARRAY_SIZE(gtable)) {
> -		DRM_DEBUG_KMS("unknown gpio %u\n", gpio);
> -		goto out;
> -	}
> -
> -	if (!IS_VALLEYVIEW(dev_priv)) {
> -		DRM_DEBUG_KMS("GPIO element not supported on this platform\n");
> -		goto out;
> -	}
>  
>  	if (dev_priv->vbt.dsi.seq_version >= 3) {
>  		if (gpio <= IOSF_MAX_GPIO_NUM_NC) {
> @@ -728,7 +707,7 @@ static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
>  			port = IOSF_PORT_GPIO_SUS;
>  		} else {
>  			DRM_ERROR("GPIO number is not present in the table\n");
> -			goto out;
> +			return;
>  		}
>  	} else {
>  		port = IOSF_PORT_GPIO_NC;
> @@ -750,11 +729,89 @@ static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
>  	/* pull up/down */
>  	vlv_iosf_sb_write(dev_priv, port, pad, val);
>  	mutex_unlock(&dev_priv->sb_lock);
> +}
> +
> +void chv_program_gpio(struct intel_dsi *intel_dsi, u8 gpio, u8 action)
> +{
> +	struct drm_device *dev = intel_dsi->base.base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	u16 function, pad;
> +	u16 family_num;
> +	u8 block;
> +
> +	if (dev_priv->vbt.dsi.seq_version >= 3) {
> +		if (gpio <= CHV_MAX_GPIO_NUM_N) {
> +			block = CHV_IOSF_PORT_GPIO_N;
> +			DRM_DEBUG_DRIVER("GPIO is in the north Block\n");
> +		} else if (gpio <= CHV_MAX_GPIO_NUM_SE) {
> +			block = CHV_IOSF_PORT_GPIO_SE;
> +			gpio = gpio - CHV_MIN_GPIO_NUM_SE;
> +			DRM_DEBUG_DRIVER("GPIO is in the south east Block\n");
> +		} else if (gpio <= CHV_MAX_GPIO_NUM_SW) {
> +			block = CHV_IOSF_PORT_GPIO_SW;
> +			gpio = gpio - CHV_MIN_GPIO_NUM_SW;
> +			DRM_DEBUG_DRIVER("GPIO is in the south west Block\n");
> +		} else {
> +			block = CHV_IOSF_PORT_GPIO_E;
> +			gpio = gpio - CHV_MIN_GPIO_NUM_E;
> +			DRM_DEBUG_DRIVER("GPIO is in the east Block\n");
> +		}
> +	} else
> +		block = IOSF_PORT_GPIO_NC;
> +
> +	family_num =  gpio / CHV_VBT_MAX_PINS_PER_FMLY;
> +	gpio = gpio - (family_num * CHV_VBT_MAX_PINS_PER_FMLY);
> +	pad = CHV_PAD_FMLY_BASE + (family_num * CHV_PAD_FMLY_SIZE) +
> +		(((u16)gpio) * CHV_PAD_CFG_0_1_REG_SIZE);
> +	function = pad + CHV_PAD_CFG_REG_SIZE;
> +
> +	mutex_lock(&dev_priv->sb_lock);
> +	vlv_iosf_sb_write(dev_priv, block, function,
> +			CHV_GPIO_CFG_UNLOCK);
> +	vlv_iosf_sb_write(dev_priv, block, pad, CHV_GPIO_CFG_HIZ |
> +			(action << CHV_GPIO_CFG_TX_STATE_SHIFT));
> +	mutex_unlock(&dev_priv->sb_lock);
> +
> +}
> +
> +static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
> +{
> +	u8 gpio, action;
> +	struct drm_device *dev = intel_dsi->base.base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	DRM_DEBUG_DRIVER("MIPI: executing gpio element\n");
> +
> +	if (dev_priv->vbt.dsi.seq_version >= 3)
> +		data++;
> +
> +	gpio = *data++;
> +
> +	/* pull up/down */
> +	action = *data++ & 1;
> +
> +	if (gpio >= ARRAY_SIZE(gtable)) {
> +		DRM_DEBUG_KMS("unknown gpio %u\n", gpio);
> +		goto out;
> +	}
> +
> +	if (!IS_VALLEYVIEW(dev_priv)) {
> +		DRM_DEBUG_KMS("GPIO element not supported on this platform\n");
> +		goto out;
> +	}

This will bail out on chv as well.

> +
> +	if (IS_VALLEYVIEW(dev))
> +		vlv_program_gpio(intel_dsi, gpio, action);
> +	else if (IS_CHERRYVIEW(dev))
> +		chv_program_gpio(intel_dsi, gpio, action);
> +	else
> +		DRM_ERROR("GPIO programming missing for this platform.\n");
>  
>  out:
>  	return data;
>  }
>  
> +

Superfluous whitespace.

>  static const u8 *mipi_exec_i2c_skip(struct intel_dsi *intel_dsi, const u8 *data)
>  {
>  	return data + *(data + 6) + 7;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx