Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH v3] drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence
@ 2022-12-12 14:37 Jani Nikula
  2022-12-12 15:38 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence (rev2) Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Jani Nikula @ 2022-12-12 14:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Starting from ICL, the default for MIPI GPIO sequences seems to be using
native GPIOs i.e. GPIOs available in the GPU. These native GPIOs reuse
many pins that quite frankly seem scary to poke based on the VBT
sequences. We pretty much have to trust that the board is configured
such that the relevant HPD, PP_CONTROL and GPIO bits aren't used for
anything else.

MIPI sequence v4 also adds a flag to fall back to non-native sequences.

v3:
- Fix -Wbitwise-conditional-parentheses (kernel test robot <lkp@intel.com>)

v2:
- Fix HPD pin output set (impacts GPIOs 0 and 5)
- Fix GPIO data output direction set (impacts GPIOs 4 and 9)
- Reduce register accesses to single intel_de_rwm()

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6131
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 84 +++++++++++++++++++-
 drivers/gpu/drm/i915/i915_reg.h              |  1 +
 2 files changed, 82 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
index fce69fa446d5..f19020074ee3 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
@@ -41,9 +41,11 @@
 
 #include "i915_drv.h"
 #include "i915_reg.h"
+#include "intel_de.h"
 #include "intel_display_types.h"
 #include "intel_dsi.h"
 #include "intel_dsi_vbt.h"
+#include "intel_gmbus_regs.h"
 #include "vlv_dsi.h"
 #include "vlv_dsi_regs.h"
 #include "vlv_sideband.h"
@@ -377,6 +379,75 @@ static void icl_exec_gpio(struct intel_connector *connector,
 	drm_dbg_kms(&dev_priv->drm, "Skipping ICL GPIO element execution\n");
 }
 
+enum {
+	MIPI_RESET_1 = 0,
+	MIPI_AVDD_EN_1,
+	MIPI_BKLT_EN_1,
+	MIPI_AVEE_EN_1,
+	MIPI_VIO_EN_1,
+	MIPI_RESET_2,
+	MIPI_AVDD_EN_2,
+	MIPI_BKLT_EN_2,
+	MIPI_AVEE_EN_2,
+	MIPI_VIO_EN_2,
+};
+
+static void icl_native_gpio_set_value(struct drm_i915_private *dev_priv,
+				      int gpio, bool value)
+{
+	int index;
+
+	if (drm_WARN_ON(&dev_priv->drm, DISPLAY_VER(dev_priv) == 11 && gpio >= MIPI_RESET_2))
+		return;
+
+	switch (gpio) {
+	case MIPI_RESET_1:
+	case MIPI_RESET_2:
+		index = gpio == MIPI_RESET_1 ? HPD_PORT_A : HPD_PORT_B;
+
+		/* Disable HPD to set the pin to output, and set output value */
+		intel_de_rmw(dev_priv, SHOTPLUG_CTL_DDI,
+			     SHOTPLUG_CTL_DDI_HPD_ENABLE(index) |
+			     SHOTPLUG_CTL_DDI_HPD_OUTPUT_DATA(index),
+			     value ? SHOTPLUG_CTL_DDI_HPD_OUTPUT_DATA(index) : 0);
+		break;
+	case MIPI_AVDD_EN_1:
+	case MIPI_AVDD_EN_2:
+		index = gpio == MIPI_AVDD_EN_1 ? 0 : 1;
+
+		intel_de_rmw(dev_priv, PP_CONTROL(index), PANEL_POWER_ON,
+			     value ? PANEL_POWER_ON : 0);
+		break;
+	case MIPI_BKLT_EN_1:
+	case MIPI_BKLT_EN_2:
+		index = gpio == MIPI_AVDD_EN_1 ? 0 : 1;
+
+		intel_de_rmw(dev_priv, PP_CONTROL(index), EDP_BLC_ENABLE,
+			     value ? EDP_BLC_ENABLE : 0);
+		break;
+	case MIPI_AVEE_EN_1:
+	case MIPI_AVEE_EN_2:
+		index = gpio == MIPI_AVEE_EN_1 ? 1 : 2;
+
+		intel_de_rmw(dev_priv, GPIO(dev_priv, index),
+			     GPIO_CLOCK_VAL_OUT,
+			     GPIO_CLOCK_DIR_MASK | GPIO_CLOCK_DIR_OUT |
+			     GPIO_CLOCK_VAL_MASK | (value ? GPIO_CLOCK_VAL_OUT : 0));
+		break;
+	case MIPI_VIO_EN_1:
+	case MIPI_VIO_EN_2:
+		index = gpio == MIPI_VIO_EN_1 ? 1 : 2;
+
+		intel_de_rmw(dev_priv, GPIO(dev_priv, index),
+			     GPIO_DATA_VAL_OUT,
+			     GPIO_DATA_DIR_MASK | GPIO_DATA_DIR_OUT |
+			     GPIO_DATA_VAL_MASK | (value ? GPIO_DATA_VAL_OUT : 0));
+		break;
+	default:
+		MISSING_CASE(gpio);
+	}
+}
+
 static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
 {
 	struct drm_device *dev = intel_dsi->base.base.dev;
@@ -384,8 +455,7 @@ static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
 	struct intel_connector *connector = intel_dsi->attached_connector;
 	u8 gpio_source, gpio_index = 0, gpio_number;
 	bool value;
-
-	drm_dbg_kms(&dev_priv->drm, "\n");
+	bool native = DISPLAY_VER(dev_priv) >= 11;
 
 	if (connector->panel.vbt.dsi.seq_version >= 3)
 		gpio_index = *data++;
@@ -398,10 +468,18 @@ static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
 	else
 		gpio_source = 0;
 
+	if (connector->panel.vbt.dsi.seq_version >= 4 && *data & BIT(1))
+		native = false;
+
 	/* pull up/down */
 	value = *data++ & 1;
 
-	if (DISPLAY_VER(dev_priv) >= 11)
+	drm_dbg_kms(&dev_priv->drm, "GPIO index %u, number %u, source %u, native %s, set to %s\n",
+		    gpio_index, gpio_number, gpio_source, str_yes_no(native), str_on_off(value));
+
+	if (native)
+		icl_native_gpio_set_value(dev_priv, gpio_number, value);
+	else if (DISPLAY_VER(dev_priv) >= 11)
 		icl_exec_gpio(connector, gpio_source, gpio_index, value);
 	else if (IS_VALLEYVIEW(dev_priv))
 		vlv_exec_gpio(connector, gpio_source, gpio_number, value);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2b7a63754e4d..7008d04a06b8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5965,6 +5965,7 @@
 
 #define SHOTPLUG_CTL_DDI				_MMIO(0xc4030)
 #define   SHOTPLUG_CTL_DDI_HPD_ENABLE(hpd_pin)			(0x8 << (_HPD_PIN_DDI(hpd_pin) * 4))
+#define   SHOTPLUG_CTL_DDI_HPD_OUTPUT_DATA(hpd_pin)		(0x4 << (_HPD_PIN_DDI(hpd_pin) * 4))
 #define   SHOTPLUG_CTL_DDI_HPD_STATUS_MASK(hpd_pin)		(0x3 << (_HPD_PIN_DDI(hpd_pin) * 4))
 #define   SHOTPLUG_CTL_DDI_HPD_NO_DETECT(hpd_pin)		(0x0 << (_HPD_PIN_DDI(hpd_pin) * 4))
 #define   SHOTPLUG_CTL_DDI_HPD_SHORT_DETECT(hpd_pin)		(0x1 << (_HPD_PIN_DDI(hpd_pin) * 4))
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence (rev2)
  2022-12-12 14:37 [Intel-gfx] [PATCH v3] drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence Jani Nikula
@ 2022-12-12 15:38 ` Patchwork
  2022-12-12 16:37 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence (rev3) Patchwork
  2022-12-13  3:48 ` [Intel-gfx] [PATCH v3] drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence Ville Syrjälä
  2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2022-12-12 15:38 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 3757 bytes --]

== Series Details ==

Series: drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence (rev2)
URL   : https://patchwork.freedesktop.org/series/111850/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12493 -> Patchwork_111850v2
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_111850v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_111850v2, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111850v2/index.html

Participating hosts (40 -> 19)
------------------------------

  ERROR: It appears as if the changes made in Patchwork_111850v2 prevented too many machines from booting.

  Missing    (21): fi-kbl-soraka bat-adls-5 bat-dg1-6 bat-dg1-5 bat-adlp-6 bat-rpls-2 fi-skl-6600u fi-bsw-n3050 bat-dg2-8 bat-adlm-1 bat-dg2-9 bat-adln-1 bat-jsl-3 bat-rplp-1 bat-dg2-11 fi-bsw-nick bat-dg1-7 bat-kbl-2 bat-adlp-9 bat-jsl-1 bat-adlp-4 

Known issues
------------

  Here are the changes found in Patchwork_111850v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-rkl-11600:       [PASS][1] -> [INCOMPLETE][2] ([i915#4817])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12493/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111850v2/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-hsw-4770:        NOTRUN -> [SKIP][3] ([fdo#109271] / [fdo#111827])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111850v2/fi-hsw-4770/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size:
    - fi-bsw-kefka:       [PASS][4] -> [FAIL][5] ([i915#6298])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12493/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111850v2/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@hangcheck:
    - fi-hsw-4770:        [INCOMPLETE][6] ([i915#4785]) -> [PASS][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12493/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111850v2/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
  [i915#4817]: https://gitlab.freedesktop.org/drm/intel/issues/4817
  [i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298


Build changes
-------------

  * Linux: CI_DRM_12493 -> Patchwork_111850v2

  CI-20190529: 20190529
  CI_DRM_12493: a6dc4d045339e2817103e99539e3efaa554c941f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7090: 5aafcf060b6dfbb2fa7aace76c8074d98ac7da8f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_111850v2: a6dc4d045339e2817103e99539e3efaa554c941f @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

2d17c209cee2 drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111850v2/index.html

[-- Attachment #2: Type: text/html, Size: 4463 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence (rev3)
  2022-12-12 14:37 [Intel-gfx] [PATCH v3] drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence Jani Nikula
  2022-12-12 15:38 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence (rev2) Patchwork
@ 2022-12-12 16:37 ` Patchwork
  2022-12-13  3:48 ` [Intel-gfx] [PATCH v3] drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence Ville Syrjälä
  2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2022-12-12 16:37 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 3214 bytes --]

== Series Details ==

Series: drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence (rev3)
URL   : https://patchwork.freedesktop.org/series/111850/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12493 -> Patchwork_111850v3
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_111850v3 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_111850v3, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111850v3/index.html

Participating hosts (40 -> 18)
------------------------------

  ERROR: It appears as if the changes made in Patchwork_111850v3 prevented too many machines from booting.

  Missing    (22): fi-kbl-soraka bat-adls-5 bat-dg1-6 bat-dg1-5 bat-adlp-6 fi-pnv-d510 bat-rpls-2 fi-skl-6600u fi-bsw-n3050 bat-dg2-8 bat-adlm-1 bat-dg2-9 bat-adln-1 bat-jsl-3 bat-rplp-1 bat-dg2-11 fi-bsw-nick bat-dg1-7 bat-kbl-2 bat-adlp-9 bat-jsl-1 bat-adlp-4 

Known issues
------------

  Here are the changes found in Patchwork_111850v3 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-rkl-11600:       [PASS][1] -> [INCOMPLETE][2] ([i915#4817])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12493/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111850v3/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-hsw-4770:        NOTRUN -> [SKIP][3] ([fdo#109271] / [fdo#111827])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111850v3/fi-hsw-4770/igt@kms_chamelium@common-hpd-after-suspend.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@hangcheck:
    - fi-hsw-4770:        [INCOMPLETE][4] ([i915#4785]) -> [PASS][5]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12493/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111850v3/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
  [i915#4817]: https://gitlab.freedesktop.org/drm/intel/issues/4817


Build changes
-------------

  * Linux: CI_DRM_12493 -> Patchwork_111850v3

  CI-20190529: 20190529
  CI_DRM_12493: a6dc4d045339e2817103e99539e3efaa554c941f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7090: 5aafcf060b6dfbb2fa7aace76c8074d98ac7da8f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_111850v3: a6dc4d045339e2817103e99539e3efaa554c941f @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

f65a9e3d37b0 drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111850v3/index.html

[-- Attachment #2: Type: text/html, Size: 3891 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Intel-gfx] [PATCH v3] drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence
  2022-12-12 14:37 [Intel-gfx] [PATCH v3] drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence Jani Nikula
  2022-12-12 15:38 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence (rev2) Patchwork
  2022-12-12 16:37 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence (rev3) Patchwork
@ 2022-12-13  3:48 ` Ville Syrjälä
  2022-12-13  9:56   ` Jani Nikula
  2 siblings, 1 reply; 5+ messages in thread
From: Ville Syrjälä @ 2022-12-13  3:48 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Mon, Dec 12, 2022 at 04:37:53PM +0200, Jani Nikula wrote:
> Starting from ICL, the default for MIPI GPIO sequences seems to be using
> native GPIOs i.e. GPIOs available in the GPU. These native GPIOs reuse
> many pins that quite frankly seem scary to poke based on the VBT
> sequences. We pretty much have to trust that the board is configured
> such that the relevant HPD, PP_CONTROL and GPIO bits aren't used for
> anything else.
> 
> MIPI sequence v4 also adds a flag to fall back to non-native sequences.
> 
> v3:
> - Fix -Wbitwise-conditional-parentheses (kernel test robot <lkp@intel.com>)
> 
> v2:
> - Fix HPD pin output set (impacts GPIOs 0 and 5)
> - Fix GPIO data output direction set (impacts GPIOs 4 and 9)
> - Reduce register accesses to single intel_de_rwm()
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6131
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 84 +++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_reg.h              |  1 +
>  2 files changed, 82 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> index fce69fa446d5..f19020074ee3 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> @@ -41,9 +41,11 @@
>  
>  #include "i915_drv.h"
>  #include "i915_reg.h"
> +#include "intel_de.h"
>  #include "intel_display_types.h"
>  #include "intel_dsi.h"
>  #include "intel_dsi_vbt.h"
> +#include "intel_gmbus_regs.h"
>  #include "vlv_dsi.h"
>  #include "vlv_dsi_regs.h"
>  #include "vlv_sideband.h"
> @@ -377,6 +379,75 @@ static void icl_exec_gpio(struct intel_connector *connector,
>  	drm_dbg_kms(&dev_priv->drm, "Skipping ICL GPIO element execution\n");
>  }
>  
> +enum {
> +	MIPI_RESET_1 = 0,
> +	MIPI_AVDD_EN_1,
> +	MIPI_BKLT_EN_1,
> +	MIPI_AVEE_EN_1,
> +	MIPI_VIO_EN_1,
> +	MIPI_RESET_2,
> +	MIPI_AVDD_EN_2,
> +	MIPI_BKLT_EN_2,
> +	MIPI_AVEE_EN_2,
> +	MIPI_VIO_EN_2,
> +};
> +
> +static void icl_native_gpio_set_value(struct drm_i915_private *dev_priv,
> +				      int gpio, bool value)
> +{
> +	int index;
> +
> +	if (drm_WARN_ON(&dev_priv->drm, DISPLAY_VER(dev_priv) == 11 && gpio >= MIPI_RESET_2))
> +		return;
> +
> +	switch (gpio) {
> +	case MIPI_RESET_1:
> +	case MIPI_RESET_2:
> +		index = gpio == MIPI_RESET_1 ? HPD_PORT_A : HPD_PORT_B;
> +
> +		/* Disable HPD to set the pin to output, and set output value */
> +		intel_de_rmw(dev_priv, SHOTPLUG_CTL_DDI,
> +			     SHOTPLUG_CTL_DDI_HPD_ENABLE(index) |
> +			     SHOTPLUG_CTL_DDI_HPD_OUTPUT_DATA(index),
> +			     value ? SHOTPLUG_CTL_DDI_HPD_OUTPUT_DATA(index) : 0);

This looks like it could race with hpd irq handling/setup. Assuming
one of the pins could be used for DSI and other for eg. HDMI.

> +		break;
> +	case MIPI_AVDD_EN_1:
> +	case MIPI_AVDD_EN_2:
> +		index = gpio == MIPI_AVDD_EN_1 ? 0 : 1;
> +
> +		intel_de_rmw(dev_priv, PP_CONTROL(index), PANEL_POWER_ON,
> +			     value ? PANEL_POWER_ON : 0);
> +		break;
> +	case MIPI_BKLT_EN_1:
> +	case MIPI_BKLT_EN_2:
> +		index = gpio == MIPI_AVDD_EN_1 ? 0 : 1;
> +
> +		intel_de_rmw(dev_priv, PP_CONTROL(index), EDP_BLC_ENABLE,
> +			     value ? EDP_BLC_ENABLE : 0);
> +		break;
> +	case MIPI_AVEE_EN_1:
> +	case MIPI_AVEE_EN_2:
> +		index = gpio == MIPI_AVEE_EN_1 ? 1 : 2;
> +
> +		intel_de_rmw(dev_priv, GPIO(dev_priv, index),
> +			     GPIO_CLOCK_VAL_OUT,
> +			     GPIO_CLOCK_DIR_MASK | GPIO_CLOCK_DIR_OUT |
> +			     GPIO_CLOCK_VAL_MASK | (value ? GPIO_CLOCK_VAL_OUT : 0));
> +		break;
> +	case MIPI_VIO_EN_1:
> +	case MIPI_VIO_EN_2:
> +		index = gpio == MIPI_VIO_EN_1 ? 1 : 2;
> +
> +		intel_de_rmw(dev_priv, GPIO(dev_priv, index),
> +			     GPIO_DATA_VAL_OUT,
> +			     GPIO_DATA_DIR_MASK | GPIO_DATA_DIR_OUT |
> +			     GPIO_DATA_VAL_MASK | (value ? GPIO_DATA_VAL_OUT : 0));
> +		break;
> +	default:
> +		MISSING_CASE(gpio);
> +	}
> +}
> +
>  static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
>  {
>  	struct drm_device *dev = intel_dsi->base.base.dev;
> @@ -384,8 +455,7 @@ static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
>  	struct intel_connector *connector = intel_dsi->attached_connector;
>  	u8 gpio_source, gpio_index = 0, gpio_number;
>  	bool value;
> -
> -	drm_dbg_kms(&dev_priv->drm, "\n");
> +	bool native = DISPLAY_VER(dev_priv) >= 11;
>  
>  	if (connector->panel.vbt.dsi.seq_version >= 3)
>  		gpio_index = *data++;
> @@ -398,10 +468,18 @@ static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
>  	else
>  		gpio_source = 0;
>  
> +	if (connector->panel.vbt.dsi.seq_version >= 4 && *data & BIT(1))
> +		native = false;
> +
>  	/* pull up/down */
>  	value = *data++ & 1;
>  
> -	if (DISPLAY_VER(dev_priv) >= 11)
> +	drm_dbg_kms(&dev_priv->drm, "GPIO index %u, number %u, source %u, native %s, set to %s\n",
> +		    gpio_index, gpio_number, gpio_source, str_yes_no(native), str_on_off(value));
> +
> +	if (native)
> +		icl_native_gpio_set_value(dev_priv, gpio_number, value);
> +	else if (DISPLAY_VER(dev_priv) >= 11)
>  		icl_exec_gpio(connector, gpio_source, gpio_index, value);
>  	else if (IS_VALLEYVIEW(dev_priv))
>  		vlv_exec_gpio(connector, gpio_source, gpio_number, value);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2b7a63754e4d..7008d04a06b8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5965,6 +5965,7 @@
>  
>  #define SHOTPLUG_CTL_DDI				_MMIO(0xc4030)
>  #define   SHOTPLUG_CTL_DDI_HPD_ENABLE(hpd_pin)			(0x8 << (_HPD_PIN_DDI(hpd_pin) * 4))
> +#define   SHOTPLUG_CTL_DDI_HPD_OUTPUT_DATA(hpd_pin)		(0x4 << (_HPD_PIN_DDI(hpd_pin) * 4))
>  #define   SHOTPLUG_CTL_DDI_HPD_STATUS_MASK(hpd_pin)		(0x3 << (_HPD_PIN_DDI(hpd_pin) * 4))
>  #define   SHOTPLUG_CTL_DDI_HPD_NO_DETECT(hpd_pin)		(0x0 << (_HPD_PIN_DDI(hpd_pin) * 4))
>  #define   SHOTPLUG_CTL_DDI_HPD_SHORT_DETECT(hpd_pin)		(0x1 << (_HPD_PIN_DDI(hpd_pin) * 4))
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Intel-gfx] [PATCH v3] drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence
  2022-12-13  3:48 ` [Intel-gfx] [PATCH v3] drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence Ville Syrjälä
@ 2022-12-13  9:56   ` Jani Nikula
  0 siblings, 0 replies; 5+ messages in thread
From: Jani Nikula @ 2022-12-13  9:56 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Tue, 13 Dec 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Mon, Dec 12, 2022 at 04:37:53PM +0200, Jani Nikula wrote:
>> Starting from ICL, the default for MIPI GPIO sequences seems to be using
>> native GPIOs i.e. GPIOs available in the GPU. These native GPIOs reuse
>> many pins that quite frankly seem scary to poke based on the VBT
>> sequences. We pretty much have to trust that the board is configured
>> such that the relevant HPD, PP_CONTROL and GPIO bits aren't used for
>> anything else.
>> 
>> MIPI sequence v4 also adds a flag to fall back to non-native sequences.
>> 
>> v3:
>> - Fix -Wbitwise-conditional-parentheses (kernel test robot <lkp@intel.com>)
>> 
>> v2:
>> - Fix HPD pin output set (impacts GPIOs 0 and 5)
>> - Fix GPIO data output direction set (impacts GPIOs 4 and 9)
>> - Reduce register accesses to single intel_de_rwm()
>> 
>> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6131
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 84 +++++++++++++++++++-
>>  drivers/gpu/drm/i915/i915_reg.h              |  1 +
>>  2 files changed, 82 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
>> index fce69fa446d5..f19020074ee3 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
>> @@ -41,9 +41,11 @@
>>  
>>  #include "i915_drv.h"
>>  #include "i915_reg.h"
>> +#include "intel_de.h"
>>  #include "intel_display_types.h"
>>  #include "intel_dsi.h"
>>  #include "intel_dsi_vbt.h"
>> +#include "intel_gmbus_regs.h"
>>  #include "vlv_dsi.h"
>>  #include "vlv_dsi_regs.h"
>>  #include "vlv_sideband.h"
>> @@ -377,6 +379,75 @@ static void icl_exec_gpio(struct intel_connector *connector,
>>  	drm_dbg_kms(&dev_priv->drm, "Skipping ICL GPIO element execution\n");
>>  }
>>  
>> +enum {
>> +	MIPI_RESET_1 = 0,
>> +	MIPI_AVDD_EN_1,
>> +	MIPI_BKLT_EN_1,
>> +	MIPI_AVEE_EN_1,
>> +	MIPI_VIO_EN_1,
>> +	MIPI_RESET_2,
>> +	MIPI_AVDD_EN_2,
>> +	MIPI_BKLT_EN_2,
>> +	MIPI_AVEE_EN_2,
>> +	MIPI_VIO_EN_2,
>> +};
>> +
>> +static void icl_native_gpio_set_value(struct drm_i915_private *dev_priv,
>> +				      int gpio, bool value)
>> +{
>> +	int index;
>> +
>> +	if (drm_WARN_ON(&dev_priv->drm, DISPLAY_VER(dev_priv) == 11 && gpio >= MIPI_RESET_2))
>> +		return;
>> +
>> +	switch (gpio) {
>> +	case MIPI_RESET_1:
>> +	case MIPI_RESET_2:
>> +		index = gpio == MIPI_RESET_1 ? HPD_PORT_A : HPD_PORT_B;
>> +
>> +		/* Disable HPD to set the pin to output, and set output value */
>> +		intel_de_rmw(dev_priv, SHOTPLUG_CTL_DDI,
>> +			     SHOTPLUG_CTL_DDI_HPD_ENABLE(index) |
>> +			     SHOTPLUG_CTL_DDI_HPD_OUTPUT_DATA(index),
>> +			     value ? SHOTPLUG_CTL_DDI_HPD_OUTPUT_DATA(index) : 0);
>
> This looks like it could race with hpd irq handling/setup. Assuming
> one of the pins could be used for DSI and other for eg. HDMI.

Right. Looks like it should be enough to wrap the above in
spin_lock_irq(&dev_priv->irq_lock).

AFAICT the DSI encoder having encoder->hpd_pin == HPD_NONE covers not
clobbering SHOTPLUG_CTL_DDI_HPD_ENABLE(index) in irq setup *assuming*
the board design and VBT are sane, and the pin doesn't conflict with
some other encoder.

BR,
Jani.


>
>> +		break;
>> +	case MIPI_AVDD_EN_1:
>> +	case MIPI_AVDD_EN_2:
>> +		index = gpio == MIPI_AVDD_EN_1 ? 0 : 1;
>> +
>> +		intel_de_rmw(dev_priv, PP_CONTROL(index), PANEL_POWER_ON,
>> +			     value ? PANEL_POWER_ON : 0);
>> +		break;
>> +	case MIPI_BKLT_EN_1:
>> +	case MIPI_BKLT_EN_2:
>> +		index = gpio == MIPI_AVDD_EN_1 ? 0 : 1;
>> +
>> +		intel_de_rmw(dev_priv, PP_CONTROL(index), EDP_BLC_ENABLE,
>> +			     value ? EDP_BLC_ENABLE : 0);
>> +		break;
>> +	case MIPI_AVEE_EN_1:
>> +	case MIPI_AVEE_EN_2:
>> +		index = gpio == MIPI_AVEE_EN_1 ? 1 : 2;
>> +
>> +		intel_de_rmw(dev_priv, GPIO(dev_priv, index),
>> +			     GPIO_CLOCK_VAL_OUT,
>> +			     GPIO_CLOCK_DIR_MASK | GPIO_CLOCK_DIR_OUT |
>> +			     GPIO_CLOCK_VAL_MASK | (value ? GPIO_CLOCK_VAL_OUT : 0));
>> +		break;
>> +	case MIPI_VIO_EN_1:
>> +	case MIPI_VIO_EN_2:
>> +		index = gpio == MIPI_VIO_EN_1 ? 1 : 2;
>> +
>> +		intel_de_rmw(dev_priv, GPIO(dev_priv, index),
>> +			     GPIO_DATA_VAL_OUT,
>> +			     GPIO_DATA_DIR_MASK | GPIO_DATA_DIR_OUT |
>> +			     GPIO_DATA_VAL_MASK | (value ? GPIO_DATA_VAL_OUT : 0));
>> +		break;
>> +	default:
>> +		MISSING_CASE(gpio);
>> +	}
>> +}
>> +
>>  static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
>>  {
>>  	struct drm_device *dev = intel_dsi->base.base.dev;
>> @@ -384,8 +455,7 @@ static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
>>  	struct intel_connector *connector = intel_dsi->attached_connector;
>>  	u8 gpio_source, gpio_index = 0, gpio_number;
>>  	bool value;
>> -
>> -	drm_dbg_kms(&dev_priv->drm, "\n");
>> +	bool native = DISPLAY_VER(dev_priv) >= 11;
>>  
>>  	if (connector->panel.vbt.dsi.seq_version >= 3)
>>  		gpio_index = *data++;
>> @@ -398,10 +468,18 @@ static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
>>  	else
>>  		gpio_source = 0;
>>  
>> +	if (connector->panel.vbt.dsi.seq_version >= 4 && *data & BIT(1))
>> +		native = false;
>> +
>>  	/* pull up/down */
>>  	value = *data++ & 1;
>>  
>> -	if (DISPLAY_VER(dev_priv) >= 11)
>> +	drm_dbg_kms(&dev_priv->drm, "GPIO index %u, number %u, source %u, native %s, set to %s\n",
>> +		    gpio_index, gpio_number, gpio_source, str_yes_no(native), str_on_off(value));
>> +
>> +	if (native)
>> +		icl_native_gpio_set_value(dev_priv, gpio_number, value);
>> +	else if (DISPLAY_VER(dev_priv) >= 11)
>>  		icl_exec_gpio(connector, gpio_source, gpio_index, value);
>>  	else if (IS_VALLEYVIEW(dev_priv))
>>  		vlv_exec_gpio(connector, gpio_source, gpio_number, value);
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 2b7a63754e4d..7008d04a06b8 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -5965,6 +5965,7 @@
>>  
>>  #define SHOTPLUG_CTL_DDI				_MMIO(0xc4030)
>>  #define   SHOTPLUG_CTL_DDI_HPD_ENABLE(hpd_pin)			(0x8 << (_HPD_PIN_DDI(hpd_pin) * 4))
>> +#define   SHOTPLUG_CTL_DDI_HPD_OUTPUT_DATA(hpd_pin)		(0x4 << (_HPD_PIN_DDI(hpd_pin) * 4))
>>  #define   SHOTPLUG_CTL_DDI_HPD_STATUS_MASK(hpd_pin)		(0x3 << (_HPD_PIN_DDI(hpd_pin) * 4))
>>  #define   SHOTPLUG_CTL_DDI_HPD_NO_DETECT(hpd_pin)		(0x0 << (_HPD_PIN_DDI(hpd_pin) * 4))
>>  #define   SHOTPLUG_CTL_DDI_HPD_SHORT_DETECT(hpd_pin)		(0x1 << (_HPD_PIN_DDI(hpd_pin) * 4))
>> -- 
>> 2.34.1

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-12-13  9:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-12 14:37 [Intel-gfx] [PATCH v3] drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence Jani Nikula
2022-12-12 15:38 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence (rev2) Patchwork
2022-12-12 16:37 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence (rev3) Patchwork
2022-12-13  3:48 ` [Intel-gfx] [PATCH v3] drm/i915/dsi: add support for ICL+ native MIPI GPIO sequence Ville Syrjälä
2022-12-13  9:56   ` Jani Nikula

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox