On Fri, Mar 18, 2016 at 01:11:16PM +0200, Jani Nikula wrote:
> From: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@xxxxxxxxx>
>
> Add support for CHV gpio programming in DSI gpio elements.
>
> XXX: I'd like to have a gpio table for chv as well as others.
>
> [Rewritten by Jani, based on earlier work by Yogesh and Deepak.]
>
> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@xxxxxxxxx>
> Signed-off-by: Deepak M <m.deepak@xxxxxxxxx>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 66 ++++++++++++++++++++++++++++++
> 1 file changed, 66 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index ca48f7aa6a05..f8d3f608e9c8 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -97,6 +97,23 @@ static struct vlv_gpio_map vlv_gpio_table[] = {
> { 11, IOSF_PORT_GPIO_NC, VLV_GPIO_NC_11_PCONF0 },
> };
>
> +#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
Not sure why we need min+max for these.
I would probably just do something like:
#define CHV_GPIO_IDX_START_N 0
#define CHV_GPIO_IDX_START_SE 73
#define CHV_GPIO_IDX_START_SW 100
#define CHV_GPIO_IDX_START_SE 198
> +
> +#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
I'd hide all the uglies that use these in macros like:
#define CHV_GPIO_PAD_CFG0(f, i) (0x4400 + (f) * 0x400 + (i) * 8)
#define CHV_GPIO_PAD_CFG1(f, i) (0x4400 + (f) * 0x400 + (i) * 8 + 4)
> +#define CHV_VBT_MAX_PINS_PER_FMLY 15
> +
> +#define CHV_GPIO_CFG_UNLOCK 0x00000000
Maybe just define the bit for documentation purpose and just write a
raw 0 to the register.
#define CHV_GPIO_CFGLOCK (1 << 31)
> +#define CHV_GPIO_CFG_HIZ 0x00008100
That's still not hiz.
#define CHV_GPIO_GPIOCFG_GPIO (0 << 8)
#define CHV_GPIO_GPIOCFG_GPO (1 << 8)
#define CHV_GPIO_GPIOCFG_GPI (2 << 8)
#define CHV_GPIO_GPIOCFG_HIZ (3 << 8)
#define CHV_GPIO_GPIOEN (1 << 15)
> +#define CHV_GPIO_CFG_TX_STATE_SHIFT 1
Maybe
#define CHV_GPIO_GPIOTXSTATE(state) ((state) << 1)
> +
> static inline enum port intel_dsi_seq_port_to_port(u8 port)
> {
> return port ? PORT_C : PORT_A;
> @@ -241,6 +258,53 @@ static void vlv_exec_gpio(struct drm_i915_private *dev_priv,
> mutex_unlock(&dev_priv->sb_lock);
> }
>
> +static void chv_exec_gpio(struct drm_i915_private *dev_priv,
> + u8 gpio_source, u8 gpio_index, u8 action)
> +{
> + u16 pconf0, padval;
cfg0, cfg1 to match the reg names. Or just drop them entirely since the
parametrized macros I suggested earlier would make thing very clear
without temp variables.
> + u16 family_num;
> + u8 port;
> +
> + /* XXX: add a table similar to vlv for checking gpio indexes */
If we bother going that far, then I think we should just go for gpiolib
instead.
> + if (dev_priv->vbt.dsi.seq_version >= 3) {
> + if (gpio_index <= CHV_MAX_GPIO_NUM_N) {
> + port = CHV_IOSF_PORT_GPIO_N;
> + } else if (gpio_index <= CHV_MAX_GPIO_NUM_SE) {
> + port = CHV_IOSF_PORT_GPIO_SE;
> + gpio_index = gpio_index - CHV_MIN_GPIO_NUM_SE;
> + } else if (gpio_index <= CHV_MAX_GPIO_NUM_SW) {
> + port = CHV_IOSF_PORT_GPIO_SW;
> + gpio_index = gpio_index - CHV_MIN_GPIO_NUM_SW;
> + } else {
> + port = CHV_IOSF_PORT_GPIO_E;
> + gpio_index = gpio_index - CHV_MIN_GPIO_NUM_E;
> + }
> + } else if (dev_priv->vbt.dsi.seq_version == 2) {
> + if (gpio_source == 0) {
> + port = IOSF_PORT_GPIO_NC;
> + } else if (gpio_source == 1) {
> + port = IOSF_PORT_GPIO_SC;
> + } else {
> + DRM_DEBUG_KMS("unknown gpio source %u\n", gpio_source);
> + return;
> + }
> + } else {
> + port = IOSF_PORT_GPIO_NC;
> + }
> +
> + family_num = gpio_index / CHV_VBT_MAX_PINS_PER_FMLY;
> + gpio_index = gpio_index - (family_num * CHV_VBT_MAX_PINS_PER_FMLY);
I'd still use % for clarity.
> + padval = CHV_PAD_FMLY_BASE + (family_num * CHV_PAD_FMLY_SIZE) +
> + (((u16)gpio_index) * CHV_PAD_CFG_0_1_REG_SIZE);
> + pconf0 = padval + CHV_PAD_CFG_REG_SIZE;
> +
> + mutex_lock(&dev_priv->sb_lock);
> + vlv_iosf_sb_write(dev_priv, port, pconf0, CHV_GPIO_CFG_UNLOCK);
> + vlv_iosf_sb_write(dev_priv, port, padval, 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)
> {
> struct drm_device *dev = intel_dsi->base.base.dev;
> @@ -263,6 +327,8 @@ static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
>
> if (IS_VALLEYVIEW(dev_priv))
> vlv_exec_gpio(dev_priv, gpio_source, gpio_index, action);
> + else if (IS_CHERRYVIEW(dev_priv))
> + chv_exec_gpio(dev_priv, gpio_source, gpio_index, action);
> else
> DRM_DEBUG_KMS("GPIO element not supported on this platform\n");
>
> --
> 2.1.4
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
