Re: [Generic GPIO patch 2/3] drm/i915: GPIO for CHT generic MIPI

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

On Fri, 19 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.
>
> Cc: Jani Nikula <jani.nikula@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 | 87 +++++++++++++++++++++++++++++-
>  2 files changed, 106 insertions(+), 1 deletion(-)
>
> 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
>  #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
> +
>  /* 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 e02e5e0..7fd1fae 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -685,7 +685,68 @@ 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)
> +static int chv_program_gpio(struct intel_dsi *intel_dsi,
> +		const u8 *data, const u8 **cur_data)
> +{
> +	struct drm_device *dev = intel_dsi->base.base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	u8 gpio, action;
> +	u16 family_num;
> +	u16 function, pad;
> +	u8 block;
> +
> +	/*
> +	 * Skipping the first byte as it is of no
> +	 * interest for linux kernel in new VBT version
> +	 */
> +	if (dev_priv->vbt.dsi.seq_version >= 3)
> +		data++;
> +
> +	gpio = *data++;
> +
> +	/* pull up/down */
> +	action = *data++;
> +
> +	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);
> +
> +	*cur_data = data;
> +
> +	return 0;
> +}
> +
> +static int vlv_program_gpio(struct intel_dsi *intel_dsi,
> +			const u8 *data, const u8 **cur_data)
>  {
>  	u8 gpio, action;
>  	u16 function, pad;
> @@ -746,6 +807,30 @@ static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
>  	mutex_unlock(&dev_priv->sb_lock);
>  
>  out:
> +	*cur_data = data;
> +
> +	return 0;
> +}
> +
> +static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
> +{
> +	struct drm_device *dev = intel_dsi->base.base.dev;
> +	int ret;
> +
> +	DRM_DEBUG_DRIVER("MIPI: executing gpio element\n");
> +
> +	ret = -EINVAL;
> +
> +	if (IS_CHERRYVIEW(dev))
> +		ret = chv_program_gpio(intel_dsi, data, &data);
> +	else if (IS_VALLEYVIEW(dev))
> +		ret = vlv_program_gpio(intel_dsi, data, &data);

This is the wrong level of abstraction for vlv vs. chv. The input data
is the same for all of them, please don't duplicate the parsing of it
all over the place.

> +	else
> +		DRM_ERROR("GPIO programming missing for this platform.\n");

Even if you don't know the platform, you can parse the data and skip.

> +
> +	if (ret)
> +		return NULL;

Even if there are errors, you can parse the data and skip.

> +
>  	return data;
>  }

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