[RFC 0/1] Add dsi_state in crtc_state

[RFC 0/1] Add dsi_state in crtc_state

[Vandita Kulkarni]

This patch is another approach towards solving
two problems that we are facing in enabling dsi cmd mode.

1. GOP enables dsi cmd mode in periodic update mode but
we need to enable dsi cmd mode in TE_GATE mode.
To set it to TE GATE mode we need to follow a diable
enable sequence. This will help in state check for
mode_changed.

2. In TE gate mode we need to set frame update bit on every flip
With this we can udpate the FRMCTL on evey flip.

PS: OP_MODE_MASK and OP_MODE_SHIFT are not defined in this patch.
They are part of my earlier series
https://patchwork.freedesktop.org/series/67974/

Vandita Kulkarni (1):
  drm/i915/dsi: Add dsi_state in crtc_state

 drivers/gpu/drm/i915/display/icl_dsi.c        | 39 +++++++++++++++++++
 .../drm/i915/display/intel_display_types.h    | 12 ++++++
 2 files changed, 51 insertions(+)

-- 
2.21.0.5.gaeb582a

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 1/1] drm/i915/dsi: Add dsi_state in crtc_state

[Vandita Kulkarni]

This patch add dsi_state which provides
dsi operation mode and the link mode.
These are needed in order to check if they
were differently configured by GOP.

In present case the GOP enables dsi in
periodic update mode, whereas we need
to enable it in TE_GATE command mode.
In which case a disable-enable sequence
would be required.
---
 drivers/gpu/drm/i915/display/icl_dsi.c        | 39 +++++++++++++++++++
 .../drm/i915/display/intel_display_types.h    | 12 ++++++
 2 files changed, 51 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
index 6e398c33a524..0a9323e95866 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -1238,6 +1238,37 @@ static void gen11_dsi_get_timings(struct intel_encoder *encoder,
 	adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal;
 }
 
+static bool
+gen11_dsi_dual_link_enabled(struct drm_i915_private *dev_priv)
+{
+	u32 val1, val2;
+
+	val1 = I915_READ(PIPECONF(TRANSCODER_DSI_0)) &
+		I915_READ(PIPECONF(TRANSCODER_DSI_1));
+	val1 &= PIPECONF_ENABLE;
+
+	val2 = I915_READ(TRANS_DDI_FUNC_CTL2(TRANSCODER_DSI_0));
+	val2 &= PORT_SYNC_MODE_ENABLE;
+
+	return (val1 && val2);
+}
+
+static enum dsi_op_mode
+gen11_dsi_get_op_mode(struct drm_i915_private *dev_priv,
+		      struct intel_dsi *intel_dsi)
+{
+	u32 val;
+	enum transcoder dsi_trans;
+
+	if (intel_dsi->ports == BIT(PORT_B))
+		dsi_trans = TRANSCODER_DSI_1;
+	else
+		dsi_trans = TRANSCODER_DSI_0;
+
+	val = I915_READ(DSI_TRANS_FUNC_CONF(dsi_trans));
+	return ((val &= OP_MODE_MASK) >> OP_MODE_SHIFT);
+}
+
 static void gen11_dsi_get_config(struct intel_encoder *encoder,
 				 struct intel_crtc_state *pipe_config)
 {
@@ -1250,6 +1281,12 @@ static void gen11_dsi_get_config(struct intel_encoder *encoder,
 		cnl_calc_wrpll_link(dev_priv, &pipe_config->dpll_hw_state);
 
 	pipe_config->base.adjusted_mode.crtc_clock = intel_dsi->pclk;
+
+	pipe_config->dsi_state.dual_link_mode =
+				gen11_dsi_dual_link_enabled(dev_priv);
+	pipe_config->dsi_state.op_mode =
+				gen11_dsi_get_op_mode(dev_priv, intel_dsi);
+
 	if (intel_dsi->dual_link)
 		pipe_config->base.adjusted_mode.crtc_clock *= 2;
 
@@ -1283,6 +1320,8 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
 	else
 		pipe_config->cpu_transcoder = TRANSCODER_DSI_0;
 
+	pipe_config->dsi_state.op_mode = DSI_CMD_MODE_NO_GATE;
+	pipe_config->dsi_state.dual_link_mode = intel_dsi->dual_link;
 	pipe_config->clock_set = true;
 	pipe_config->port_clock = intel_dsi_bitrate(intel_dsi) / 5;
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 40390d855815..f89917eb4b94 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -75,6 +75,13 @@ enum hdmi_force_audio {
 	HDMI_AUDIO_ON,			/* force turn on HDMI audio */
 };
 
+enum dsi_op_mode {
+	DSI_CMD_MODE_NO_GATE,
+	DSI_CMD_MODE_TE_GATE,
+	DSI_CMD_MODE_PERIODIC,
+	DSI_VIDEO_MODE,
+	};
+
 /* "Broadcast RGB" property */
 enum intel_broadcast_rgb {
 	INTEL_BROADCAST_RGB_AUTO,
@@ -861,6 +868,11 @@ struct intel_crtc_state {
 		u32 ctrl, div;
 	} dsi_pll;
 
+	struct {
+		enum dsi_op_mode op_mode;
+		bool dual_link_mode;
+	} dsi_state;
+
 	int pipe_bpp;
 	struct intel_link_m_n dp_m_n;
 
-- 
2.21.0.5.gaeb582a

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for Add dsi_state in crtc_state

[Patchwork]

== Series Details ==

Series: Add dsi_state in crtc_state
URL   : https://patchwork.freedesktop.org/series/68101/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
af3da51a7b99 drm/i915/dsi: Add dsi_state in crtc_state
-:110: ERROR:MISSING_SIGN_OFF: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 0 checks, 81 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for Add dsi_state in crtc_state

[Patchwork]

== Series Details ==

Series: Add dsi_state in crtc_state
URL   : https://patchwork.freedesktop.org/series/68101/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7109 -> Patchwork_14838
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_14838 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_14838, 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_14838/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_14838:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_execlists:
    - fi-icl-u3:          [PASS][1] -> [DMESG-FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7109/fi-icl-u3/igt@i915_selftest@live_execlists.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14838/fi-icl-u3/igt@i915_selftest@live_execlists.html
    - fi-kbl-r:           [PASS][3] -> [DMESG-FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7109/fi-kbl-r/igt@i915_selftest@live_execlists.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14838/fi-kbl-r/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-cfl-8109u:       [PASS][5] -> [DMESG-FAIL][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7109/fi-cfl-8109u/igt@i915_selftest@live_gem_contexts.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14838/fi-cfl-8109u/igt@i915_selftest@live_gem_contexts.html

  * igt@i915_selftest@live_gtt:
    - fi-bxt-dsi:         [PASS][7] -> [DMESG-FAIL][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7109/fi-bxt-dsi/igt@i915_selftest@live_gtt.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14838/fi-bxt-dsi/igt@i915_selftest@live_gtt.html

  * igt@runner@aborted:
    - fi-kbl-r:           NOTRUN -> [FAIL][9]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14838/fi-kbl-r/igt@runner@aborted.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_selftest@live_execlists:
    - {fi-icl-dsi}:       [PASS][10] -> [DMESG-FAIL][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7109/fi-icl-dsi/igt@i915_selftest@live_execlists.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14838/fi-icl-dsi/igt@i915_selftest@live_execlists.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-icl-u3:          [PASS][12] -> [DMESG-WARN][13] ([fdo#107724]) +1 similar issue
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7109/fi-icl-u3/igt@gem_exec_suspend@basic-s4-devices.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14838/fi-icl-u3/igt@gem_exec_suspend@basic-s4-devices.html

  
#### Possible fixes ####

  * igt@gem_linear_blits@basic:
    - fi-icl-u3:          [DMESG-WARN][14] ([fdo#107724]) -> [PASS][15] +1 similar issue
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7109/fi-icl-u3/igt@gem_linear_blits@basic.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14838/fi-icl-u3/igt@gem_linear_blits@basic.html

  * igt@gem_sync@basic-all:
    - {fi-tgl-u}:         [INCOMPLETE][16] ([fdo#111880]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7109/fi-tgl-u/igt@gem_sync@basic-all.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14838/fi-tgl-u/igt@gem_sync@basic-all.html

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         [DMESG-FAIL][18] -> [PASS][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7109/fi-apl-guc/igt@i915_selftest@live_execlists.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14838/fi-apl-guc/igt@i915_selftest@live_execlists.html
    - fi-cfl-8109u:       [DMESG-FAIL][20] -> [PASS][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7109/fi-cfl-8109u/igt@i915_selftest@live_execlists.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14838/fi-cfl-8109u/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_hangcheck:
    - {fi-tgl-u2}:        [INCOMPLETE][22] ([fdo#111747]) -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7109/fi-tgl-u2/igt@i915_selftest@live_hangcheck.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14838/fi-tgl-u2/igt@i915_selftest@live_hangcheck.html

  * igt@kms_busy@basic-flip-c:
    - {fi-icl-u4}:        [DMESG-WARN][24] ([fdo#105602]) -> [PASS][25] +5 similar issues
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7109/fi-icl-u4/igt@kms_busy@basic-flip-c.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14838/fi-icl-u4/igt@kms_busy@basic-flip-c.html

  * igt@kms_chamelium@hdmi-edid-read:
    - {fi-icl-u4}:        [FAIL][26] ([fdo#111045]) -> [PASS][27] +1 similar issue
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7109/fi-icl-u4/igt@kms_chamelium@hdmi-edid-read.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14838/fi-icl-u4/igt@kms_chamelium@hdmi-edid-read.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [DMESG-WARN][28] ([fdo#102614]) -> [PASS][29]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7109/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14838/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111049]: https://bugs.freedesktop.org/show_bug.cgi?id=111049
  [fdo#111747]: https://bugs.freedesktop.org/show_bug.cgi?id=111747
  [fdo#111880]: https://bugs.freedesktop.org/show_bug.cgi?id=111880


Participating hosts (53 -> 44)
------------------------------

  Missing    (9): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-bwr-2160 fi-icl-y fi-bdw-samus fi-byt-clapper fi-skl-6600u 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7109 -> Patchwork_14838

  CI-20190529: 20190529
  CI_DRM_7109: e72058f1225aedff7c5c1ec10f978fad5291814e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5231: e293051f8f99c72cb01d21e4b73a5928ea351eb3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14838: af3da51a7b9962c9100e4e98a006f743a825f3cf @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

af3da51a7b99 drm/i915/dsi: Add dsi_state in crtc_state

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14838/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 1/1] drm/i915/dsi: Add dsi_state in crtc_state

[Jani Nikula]

On Wed, 16 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> This patch add dsi_state which provides
> dsi operation mode and the link mode.
> These are needed in order to check if they
> were differently configured by GOP.
>
> In present case the GOP enables dsi in
> periodic update mode, whereas we need
> to enable it in TE_GATE command mode.
> In which case a disable-enable sequence
> would be required.

I think the high level comment is that if you need to have things in the
crtc state, you need to *move* the data there, as single point of truth,
not copy.

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c        | 39 +++++++++++++++++++
>  .../drm/i915/display/intel_display_types.h    | 12 ++++++
>  2 files changed, 51 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 6e398c33a524..0a9323e95866 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1238,6 +1238,37 @@ static void gen11_dsi_get_timings(struct intel_encoder *encoder,
>  	adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal;
>  }
>  
> +static bool
> +gen11_dsi_dual_link_enabled(struct drm_i915_private *dev_priv)
> +{
> +	u32 val1, val2;
> +
> +	val1 = I915_READ(PIPECONF(TRANSCODER_DSI_0)) &
> +		I915_READ(PIPECONF(TRANSCODER_DSI_1));
> +	val1 &= PIPECONF_ENABLE;
> +
> +	val2 = I915_READ(TRANS_DDI_FUNC_CTL2(TRANSCODER_DSI_0));
> +	val2 &= PORT_SYNC_MODE_ENABLE;
> +
> +	return (val1 && val2);
> +}
> +
> +static enum dsi_op_mode
> +gen11_dsi_get_op_mode(struct drm_i915_private *dev_priv,
> +		      struct intel_dsi *intel_dsi)
> +{
> +	u32 val;
> +	enum transcoder dsi_trans;
> +
> +	if (intel_dsi->ports == BIT(PORT_B))
> +		dsi_trans = TRANSCODER_DSI_1;
> +	else
> +		dsi_trans = TRANSCODER_DSI_0;
> +
> +	val = I915_READ(DSI_TRANS_FUNC_CONF(dsi_trans));
> +	return ((val &= OP_MODE_MASK) >> OP_MODE_SHIFT);
> +}
> +
>  static void gen11_dsi_get_config(struct intel_encoder *encoder,
>  				 struct intel_crtc_state *pipe_config)
>  {
> @@ -1250,6 +1281,12 @@ static void gen11_dsi_get_config(struct intel_encoder *encoder,
>  		cnl_calc_wrpll_link(dev_priv, &pipe_config->dpll_hw_state);
>  
>  	pipe_config->base.adjusted_mode.crtc_clock = intel_dsi->pclk;
> +
> +	pipe_config->dsi_state.dual_link_mode =
> +				gen11_dsi_dual_link_enabled(dev_priv);
> +	pipe_config->dsi_state.op_mode =
> +				gen11_dsi_get_op_mode(dev_priv, intel_dsi);
> +
>  	if (intel_dsi->dual_link)
>  		pipe_config->base.adjusted_mode.crtc_clock *= 2;
>  
> @@ -1283,6 +1320,8 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
>  	else
>  		pipe_config->cpu_transcoder = TRANSCODER_DSI_0;
>  
> +	pipe_config->dsi_state.op_mode = DSI_CMD_MODE_NO_GATE;
> +	pipe_config->dsi_state.dual_link_mode = intel_dsi->dual_link;
>  	pipe_config->clock_set = true;
>  	pipe_config->port_clock = intel_dsi_bitrate(intel_dsi) / 5;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 40390d855815..f89917eb4b94 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -75,6 +75,13 @@ enum hdmi_force_audio {
>  	HDMI_AUDIO_ON,			/* force turn on HDMI audio */
>  };
>  
> +enum dsi_op_mode {
> +	DSI_CMD_MODE_NO_GATE,
> +	DSI_CMD_MODE_TE_GATE,
> +	DSI_CMD_MODE_PERIODIC,
> +	DSI_VIDEO_MODE,
> +	};
> +
>  /* "Broadcast RGB" property */
>  enum intel_broadcast_rgb {
>  	INTEL_BROADCAST_RGB_AUTO,
> @@ -861,6 +868,11 @@ struct intel_crtc_state {
>  		u32 ctrl, div;
>  	} dsi_pll;
>  
> +	struct {
> +		enum dsi_op_mode op_mode;
> +		bool dual_link_mode;
> +	} dsi_state;
> +
>  	int pipe_bpp;
>  	struct intel_link_m_n dp_m_n;

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

Re: [Intel-gfx] [RFC 1/1] drm/i915/dsi: Add dsi_state in crtc_state

[Jani Nikula]

On Wed, 16 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> This patch add dsi_state which provides
> dsi operation mode and the link mode.
> These are needed in order to check if they
> were differently configured by GOP.
>
> In present case the GOP enables dsi in
> periodic update mode, whereas we need
> to enable it in TE_GATE command mode.
> In which case a disable-enable sequence
> would be required.

I think the high level comment is that if you need to have things in the
crtc state, you need to *move* the data there, as single point of truth,
not copy.

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c        | 39 +++++++++++++++++++
>  .../drm/i915/display/intel_display_types.h    | 12 ++++++
>  2 files changed, 51 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 6e398c33a524..0a9323e95866 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1238,6 +1238,37 @@ static void gen11_dsi_get_timings(struct intel_encoder *encoder,
>  	adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal;
>  }
>  
> +static bool
> +gen11_dsi_dual_link_enabled(struct drm_i915_private *dev_priv)
> +{
> +	u32 val1, val2;
> +
> +	val1 = I915_READ(PIPECONF(TRANSCODER_DSI_0)) &
> +		I915_READ(PIPECONF(TRANSCODER_DSI_1));
> +	val1 &= PIPECONF_ENABLE;
> +
> +	val2 = I915_READ(TRANS_DDI_FUNC_CTL2(TRANSCODER_DSI_0));
> +	val2 &= PORT_SYNC_MODE_ENABLE;
> +
> +	return (val1 && val2);
> +}
> +
> +static enum dsi_op_mode
> +gen11_dsi_get_op_mode(struct drm_i915_private *dev_priv,
> +		      struct intel_dsi *intel_dsi)
> +{
> +	u32 val;
> +	enum transcoder dsi_trans;
> +
> +	if (intel_dsi->ports == BIT(PORT_B))
> +		dsi_trans = TRANSCODER_DSI_1;
> +	else
> +		dsi_trans = TRANSCODER_DSI_0;
> +
> +	val = I915_READ(DSI_TRANS_FUNC_CONF(dsi_trans));
> +	return ((val &= OP_MODE_MASK) >> OP_MODE_SHIFT);
> +}
> +
>  static void gen11_dsi_get_config(struct intel_encoder *encoder,
>  				 struct intel_crtc_state *pipe_config)
>  {
> @@ -1250,6 +1281,12 @@ static void gen11_dsi_get_config(struct intel_encoder *encoder,
>  		cnl_calc_wrpll_link(dev_priv, &pipe_config->dpll_hw_state);
>  
>  	pipe_config->base.adjusted_mode.crtc_clock = intel_dsi->pclk;
> +
> +	pipe_config->dsi_state.dual_link_mode =
> +				gen11_dsi_dual_link_enabled(dev_priv);
> +	pipe_config->dsi_state.op_mode =
> +				gen11_dsi_get_op_mode(dev_priv, intel_dsi);
> +
>  	if (intel_dsi->dual_link)
>  		pipe_config->base.adjusted_mode.crtc_clock *= 2;
>  
> @@ -1283,6 +1320,8 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
>  	else
>  		pipe_config->cpu_transcoder = TRANSCODER_DSI_0;
>  
> +	pipe_config->dsi_state.op_mode = DSI_CMD_MODE_NO_GATE;
> +	pipe_config->dsi_state.dual_link_mode = intel_dsi->dual_link;
>  	pipe_config->clock_set = true;
>  	pipe_config->port_clock = intel_dsi_bitrate(intel_dsi) / 5;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 40390d855815..f89917eb4b94 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -75,6 +75,13 @@ enum hdmi_force_audio {
>  	HDMI_AUDIO_ON,			/* force turn on HDMI audio */
>  };
>  
> +enum dsi_op_mode {
> +	DSI_CMD_MODE_NO_GATE,
> +	DSI_CMD_MODE_TE_GATE,
> +	DSI_CMD_MODE_PERIODIC,
> +	DSI_VIDEO_MODE,
> +	};
> +
>  /* "Broadcast RGB" property */
>  enum intel_broadcast_rgb {
>  	INTEL_BROADCAST_RGB_AUTO,
> @@ -861,6 +868,11 @@ struct intel_crtc_state {
>  		u32 ctrl, div;
>  	} dsi_pll;
>  
> +	struct {
> +		enum dsi_op_mode op_mode;
> +		bool dual_link_mode;
> +	} dsi_state;
> +
>  	int pipe_bpp;
>  	struct intel_link_m_n dp_m_n;

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

Re: [RFC 1/1] drm/i915/dsi: Add dsi_state in crtc_state

[Ville Syrjälä]

On Wed, Oct 16, 2019 at 06:22:36PM +0530, Vandita Kulkarni wrote:
> This patch add dsi_state which provides
> dsi operation mode and the link mode.
> These are needed in order to check if they
> were differently configured by GOP.
> 
> In present case the GOP enables dsi in
> periodic update mode, whereas we need
> to enable it in TE_GATE command mode.
> In which case a disable-enable sequence
> would be required.
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c        | 39 +++++++++++++++++++
>  .../drm/i915/display/intel_display_types.h    | 12 ++++++
>  2 files changed, 51 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 6e398c33a524..0a9323e95866 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1238,6 +1238,37 @@ static void gen11_dsi_get_timings(struct intel_encoder *encoder,
>  	adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal;
>  }
>  
> +static bool
> +gen11_dsi_dual_link_enabled(struct drm_i915_private *dev_priv)
> +{
> +	u32 val1, val2;
> +
> +	val1 = I915_READ(PIPECONF(TRANSCODER_DSI_0)) &
> +		I915_READ(PIPECONF(TRANSCODER_DSI_1));
> +	val1 &= PIPECONF_ENABLE;
> +
> +	val2 = I915_READ(TRANS_DDI_FUNC_CTL2(TRANSCODER_DSI_0));
> +	val2 &= PORT_SYNC_MODE_ENABLE;
> +
> +	return (val1 && val2);
> +}
> +
> +static enum dsi_op_mode
> +gen11_dsi_get_op_mode(struct drm_i915_private *dev_priv,
> +		      struct intel_dsi *intel_dsi)
> +{
> +	u32 val;
> +	enum transcoder dsi_trans;
> +
> +	if (intel_dsi->ports == BIT(PORT_B))
> +		dsi_trans = TRANSCODER_DSI_1;
> +	else
> +		dsi_trans = TRANSCODER_DSI_0;
> +
> +	val = I915_READ(DSI_TRANS_FUNC_CONF(dsi_trans));
> +	return ((val &= OP_MODE_MASK) >> OP_MODE_SHIFT);
> +}
> +
>  static void gen11_dsi_get_config(struct intel_encoder *encoder,
>  				 struct intel_crtc_state *pipe_config)
>  {
> @@ -1250,6 +1281,12 @@ static void gen11_dsi_get_config(struct intel_encoder *encoder,
>  		cnl_calc_wrpll_link(dev_priv, &pipe_config->dpll_hw_state);
>  
>  	pipe_config->base.adjusted_mode.crtc_clock = intel_dsi->pclk;
> +
> +	pipe_config->dsi_state.dual_link_mode =
> +				gen11_dsi_dual_link_enabled(dev_priv);
> +	pipe_config->dsi_state.op_mode =
> +				gen11_dsi_get_op_mode(dev_priv, intel_dsi);
> +
>  	if (intel_dsi->dual_link)
>  		pipe_config->base.adjusted_mode.crtc_clock *= 2;
>  
> @@ -1283,6 +1320,8 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
>  	else
>  		pipe_config->cpu_transcoder = TRANSCODER_DSI_0;
>  
> +	pipe_config->dsi_state.op_mode = DSI_CMD_MODE_NO_GATE;
> +	pipe_config->dsi_state.dual_link_mode = intel_dsi->dual_link;
>  	pipe_config->clock_set = true;
>  	pipe_config->port_clock = intel_dsi_bitrate(intel_dsi) / 5;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 40390d855815..f89917eb4b94 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -75,6 +75,13 @@ enum hdmi_force_audio {
>  	HDMI_AUDIO_ON,			/* force turn on HDMI audio */
>  };
>  
> +enum dsi_op_mode {
> +	DSI_CMD_MODE_NO_GATE,
> +	DSI_CMD_MODE_TE_GATE,
> +	DSI_CMD_MODE_PERIODIC,
> +	DSI_VIDEO_MODE,
> +	};
> +
>  /* "Broadcast RGB" property */
>  enum intel_broadcast_rgb {
>  	INTEL_BROADCAST_RGB_AUTO,
> @@ -861,6 +868,11 @@ struct intel_crtc_state {
>  		u32 ctrl, div;
>  	} dsi_pll;
>  
> +	struct {
> +		enum dsi_op_mode op_mode;
> +		bool dual_link_mode;

'dual_link' seems sufficient. And we should share that with LVDS.

Another easy target for moving to the crtc state is the lane count.
In fact we already have it in the state, but the DSI code has just
stubbornly decided to ingore it.

> +	} dsi_state;
> +
>  	int pipe_bpp;
>  	struct intel_link_m_n dp_m_n;
>  
> -- 
> 2.21.0.5.gaeb582a
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 1/1] drm/i915/dsi: Add dsi_state in crtc_state

[Ville Syrjälä]

On Wed, Oct 16, 2019 at 06:22:36PM +0530, Vandita Kulkarni wrote:
> This patch add dsi_state which provides
> dsi operation mode and the link mode.
> These are needed in order to check if they
> were differently configured by GOP.
> 
> In present case the GOP enables dsi in
> periodic update mode, whereas we need
> to enable it in TE_GATE command mode.
> In which case a disable-enable sequence
> would be required.
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c        | 39 +++++++++++++++++++
>  .../drm/i915/display/intel_display_types.h    | 12 ++++++
>  2 files changed, 51 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 6e398c33a524..0a9323e95866 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1238,6 +1238,37 @@ static void gen11_dsi_get_timings(struct intel_encoder *encoder,
>  	adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal;
>  }
>  
> +static bool
> +gen11_dsi_dual_link_enabled(struct drm_i915_private *dev_priv)
> +{
> +	u32 val1, val2;
> +
> +	val1 = I915_READ(PIPECONF(TRANSCODER_DSI_0)) &
> +		I915_READ(PIPECONF(TRANSCODER_DSI_1));
> +	val1 &= PIPECONF_ENABLE;
> +
> +	val2 = I915_READ(TRANS_DDI_FUNC_CTL2(TRANSCODER_DSI_0));
> +	val2 &= PORT_SYNC_MODE_ENABLE;
> +
> +	return (val1 && val2);
> +}
> +
> +static enum dsi_op_mode
> +gen11_dsi_get_op_mode(struct drm_i915_private *dev_priv,
> +		      struct intel_dsi *intel_dsi)
> +{
> +	u32 val;
> +	enum transcoder dsi_trans;
> +
> +	if (intel_dsi->ports == BIT(PORT_B))
> +		dsi_trans = TRANSCODER_DSI_1;
> +	else
> +		dsi_trans = TRANSCODER_DSI_0;
> +
> +	val = I915_READ(DSI_TRANS_FUNC_CONF(dsi_trans));
> +	return ((val &= OP_MODE_MASK) >> OP_MODE_SHIFT);
> +}
> +
>  static void gen11_dsi_get_config(struct intel_encoder *encoder,
>  				 struct intel_crtc_state *pipe_config)
>  {
> @@ -1250,6 +1281,12 @@ static void gen11_dsi_get_config(struct intel_encoder *encoder,
>  		cnl_calc_wrpll_link(dev_priv, &pipe_config->dpll_hw_state);
>  
>  	pipe_config->base.adjusted_mode.crtc_clock = intel_dsi->pclk;
> +
> +	pipe_config->dsi_state.dual_link_mode =
> +				gen11_dsi_dual_link_enabled(dev_priv);
> +	pipe_config->dsi_state.op_mode =
> +				gen11_dsi_get_op_mode(dev_priv, intel_dsi);
> +
>  	if (intel_dsi->dual_link)
>  		pipe_config->base.adjusted_mode.crtc_clock *= 2;
>  
> @@ -1283,6 +1320,8 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
>  	else
>  		pipe_config->cpu_transcoder = TRANSCODER_DSI_0;
>  
> +	pipe_config->dsi_state.op_mode = DSI_CMD_MODE_NO_GATE;
> +	pipe_config->dsi_state.dual_link_mode = intel_dsi->dual_link;
>  	pipe_config->clock_set = true;
>  	pipe_config->port_clock = intel_dsi_bitrate(intel_dsi) / 5;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 40390d855815..f89917eb4b94 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -75,6 +75,13 @@ enum hdmi_force_audio {
>  	HDMI_AUDIO_ON,			/* force turn on HDMI audio */
>  };
>  
> +enum dsi_op_mode {
> +	DSI_CMD_MODE_NO_GATE,
> +	DSI_CMD_MODE_TE_GATE,
> +	DSI_CMD_MODE_PERIODIC,
> +	DSI_VIDEO_MODE,
> +	};
> +
>  /* "Broadcast RGB" property */
>  enum intel_broadcast_rgb {
>  	INTEL_BROADCAST_RGB_AUTO,
> @@ -861,6 +868,11 @@ struct intel_crtc_state {
>  		u32 ctrl, div;
>  	} dsi_pll;
>  
> +	struct {
> +		enum dsi_op_mode op_mode;
> +		bool dual_link_mode;

'dual_link' seems sufficient. And we should share that with LVDS.

Another easy target for moving to the crtc state is the lane count.
In fact we already have it in the state, but the DSI code has just
stubbornly decided to ingore it.

> +	} dsi_state;
> +
>  	int pipe_bpp;
>  	struct intel_link_m_n dp_m_n;
>  
> -- 
> 2.21.0.5.gaeb582a
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 1/1] drm/i915/dsi: Add dsi_state in crtc_state

[Kulkarni, Vandita]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Thursday, October 24, 2019 7:06 PM
> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani <jani.nikula@intel.com>;
> Syrjala, Ville <ville.syrjala@intel.com>
> Subject: Re: [Intel-gfx] [RFC 1/1] drm/i915/dsi: Add dsi_state in crtc_state
> 
> On Wed, Oct 16, 2019 at 06:22:36PM +0530, Vandita Kulkarni wrote:
> > This patch add dsi_state which provides dsi operation mode and the
> > link mode.
> > These are needed in order to check if they were differently configured
> > by GOP.
> >
> > In present case the GOP enables dsi in periodic update mode, whereas
> > we need to enable it in TE_GATE command mode.
> > In which case a disable-enable sequence would be required.
> > ---
> >  drivers/gpu/drm/i915/display/icl_dsi.c        | 39 +++++++++++++++++++
> >  .../drm/i915/display/intel_display_types.h    | 12 ++++++
> >  2 files changed, 51 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> > b/drivers/gpu/drm/i915/display/icl_dsi.c
> > index 6e398c33a524..0a9323e95866 100644
> > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> > @@ -1238,6 +1238,37 @@ static void gen11_dsi_get_timings(struct
> intel_encoder *encoder,
> >  	adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal;  }
> >
> > +static bool
> > +gen11_dsi_dual_link_enabled(struct drm_i915_private *dev_priv) {
> > +	u32 val1, val2;
> > +
> > +	val1 = I915_READ(PIPECONF(TRANSCODER_DSI_0)) &
> > +		I915_READ(PIPECONF(TRANSCODER_DSI_1));
> > +	val1 &= PIPECONF_ENABLE;
> > +
> > +	val2 = I915_READ(TRANS_DDI_FUNC_CTL2(TRANSCODER_DSI_0));
> > +	val2 &= PORT_SYNC_MODE_ENABLE;
> > +
> > +	return (val1 && val2);
> > +}
> > +
> > +static enum dsi_op_mode
> > +gen11_dsi_get_op_mode(struct drm_i915_private *dev_priv,
> > +		      struct intel_dsi *intel_dsi)
> > +{
> > +	u32 val;
> > +	enum transcoder dsi_trans;
> > +
> > +	if (intel_dsi->ports == BIT(PORT_B))
> > +		dsi_trans = TRANSCODER_DSI_1;
> > +	else
> > +		dsi_trans = TRANSCODER_DSI_0;
> > +
> > +	val = I915_READ(DSI_TRANS_FUNC_CONF(dsi_trans));
> > +	return ((val &= OP_MODE_MASK) >> OP_MODE_SHIFT); }
> > +
> >  static void gen11_dsi_get_config(struct intel_encoder *encoder,
> >  				 struct intel_crtc_state *pipe_config)  { @@ -
> 1250,6 +1281,12 @@
> > static void gen11_dsi_get_config(struct intel_encoder *encoder,
> >  		cnl_calc_wrpll_link(dev_priv, &pipe_config->dpll_hw_state);
> >
> >  	pipe_config->base.adjusted_mode.crtc_clock = intel_dsi->pclk;
> > +
> > +	pipe_config->dsi_state.dual_link_mode =
> > +				gen11_dsi_dual_link_enabled(dev_priv);
> > +	pipe_config->dsi_state.op_mode =
> > +				gen11_dsi_get_op_mode(dev_priv, intel_dsi);
> > +
> >  	if (intel_dsi->dual_link)
> >  		pipe_config->base.adjusted_mode.crtc_clock *= 2;
> >
> > @@ -1283,6 +1320,8 @@ static int gen11_dsi_compute_config(struct
> intel_encoder *encoder,
> >  	else
> >  		pipe_config->cpu_transcoder = TRANSCODER_DSI_0;
> >
> > +	pipe_config->dsi_state.op_mode = DSI_CMD_MODE_NO_GATE;
> > +	pipe_config->dsi_state.dual_link_mode = intel_dsi->dual_link;
> >  	pipe_config->clock_set = true;
> >  	pipe_config->port_clock = intel_dsi_bitrate(intel_dsi) / 5;
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 40390d855815..f89917eb4b94 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -75,6 +75,13 @@ enum hdmi_force_audio {
> >  	HDMI_AUDIO_ON,			/* force turn on HDMI audio
> */
> >  };
> >
> > +enum dsi_op_mode {
> > +	DSI_CMD_MODE_NO_GATE,
> > +	DSI_CMD_MODE_TE_GATE,
> > +	DSI_CMD_MODE_PERIODIC,
> > +	DSI_VIDEO_MODE,
> > +	};
> > +
> >  /* "Broadcast RGB" property */
> >  enum intel_broadcast_rgb {
> >  	INTEL_BROADCAST_RGB_AUTO,
> > @@ -861,6 +868,11 @@ struct intel_crtc_state {
> >  		u32 ctrl, div;
> >  	} dsi_pll;
> >
> > +	struct {
> > +		enum dsi_op_mode op_mode;
> > +		bool dual_link_mode;
> 
> 'dual_link' seems sufficient. And we should share that with LVDS.
Ok.. 
The reason I added dsi_op_mode was to ensure we do a deconfigure and configure the dsi in case any other kind of mode is set by the GOP
In current case the GOP puts the dsi in DSI_CMD_MODE_PERIODIC ( which is like updating the panel on every TE by the dsi transcoder itself)

To put it in the DSI_CMD_MODE_TE_GATE ( panel update whenever the software wants to update) we need to disable and enable in this mode.
So I thought of adding it in the crtc_state.
Do you think, if this mismatch in this particular mode can be handle with any other existing state variables..

> 
> Another easy target for moving to the crtc state is the lane count.
Ok.. I can try using this. There is one more requirement on every flip we need to update the FRMCTL register on the port where dsi is present in dsi te gate cmd mode.
Do you think if it would look good to use a state variable to represent that frame update? Or should we reach to some encoder function to do this job on every flip?
 
Thanks,
Vandita

> In fact we already have it in the state, but the DSI code has just stubbornly
> decided to ingore it.
> 
> > +	} dsi_state;
> > +
> >  	int pipe_bpp;
> >  	struct intel_link_m_n dp_m_n;
> >
> > --
> > 2.21.0.5.gaeb582a
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 1/1] drm/i915/dsi: Add dsi_state in crtc_state

[Kulkarni, Vandita]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Thursday, October 24, 2019 7:06 PM
> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani <jani.nikula@intel.com>;
> Syrjala, Ville <ville.syrjala@intel.com>
> Subject: Re: [Intel-gfx] [RFC 1/1] drm/i915/dsi: Add dsi_state in crtc_state
> 
> On Wed, Oct 16, 2019 at 06:22:36PM +0530, Vandita Kulkarni wrote:
> > This patch add dsi_state which provides dsi operation mode and the
> > link mode.
> > These are needed in order to check if they were differently configured
> > by GOP.
> >
> > In present case the GOP enables dsi in periodic update mode, whereas
> > we need to enable it in TE_GATE command mode.
> > In which case a disable-enable sequence would be required.
> > ---
> >  drivers/gpu/drm/i915/display/icl_dsi.c        | 39 +++++++++++++++++++
> >  .../drm/i915/display/intel_display_types.h    | 12 ++++++
> >  2 files changed, 51 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> > b/drivers/gpu/drm/i915/display/icl_dsi.c
> > index 6e398c33a524..0a9323e95866 100644
> > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> > @@ -1238,6 +1238,37 @@ static void gen11_dsi_get_timings(struct
> intel_encoder *encoder,
> >  	adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal;  }
> >
> > +static bool
> > +gen11_dsi_dual_link_enabled(struct drm_i915_private *dev_priv) {
> > +	u32 val1, val2;
> > +
> > +	val1 = I915_READ(PIPECONF(TRANSCODER_DSI_0)) &
> > +		I915_READ(PIPECONF(TRANSCODER_DSI_1));
> > +	val1 &= PIPECONF_ENABLE;
> > +
> > +	val2 = I915_READ(TRANS_DDI_FUNC_CTL2(TRANSCODER_DSI_0));
> > +	val2 &= PORT_SYNC_MODE_ENABLE;
> > +
> > +	return (val1 && val2);
> > +}
> > +
> > +static enum dsi_op_mode
> > +gen11_dsi_get_op_mode(struct drm_i915_private *dev_priv,
> > +		      struct intel_dsi *intel_dsi)
> > +{
> > +	u32 val;
> > +	enum transcoder dsi_trans;
> > +
> > +	if (intel_dsi->ports == BIT(PORT_B))
> > +		dsi_trans = TRANSCODER_DSI_1;
> > +	else
> > +		dsi_trans = TRANSCODER_DSI_0;
> > +
> > +	val = I915_READ(DSI_TRANS_FUNC_CONF(dsi_trans));
> > +	return ((val &= OP_MODE_MASK) >> OP_MODE_SHIFT); }
> > +
> >  static void gen11_dsi_get_config(struct intel_encoder *encoder,
> >  				 struct intel_crtc_state *pipe_config)  { @@ -
> 1250,6 +1281,12 @@
> > static void gen11_dsi_get_config(struct intel_encoder *encoder,
> >  		cnl_calc_wrpll_link(dev_priv, &pipe_config->dpll_hw_state);
> >
> >  	pipe_config->base.adjusted_mode.crtc_clock = intel_dsi->pclk;
> > +
> > +	pipe_config->dsi_state.dual_link_mode =
> > +				gen11_dsi_dual_link_enabled(dev_priv);
> > +	pipe_config->dsi_state.op_mode =
> > +				gen11_dsi_get_op_mode(dev_priv, intel_dsi);
> > +
> >  	if (intel_dsi->dual_link)
> >  		pipe_config->base.adjusted_mode.crtc_clock *= 2;
> >
> > @@ -1283,6 +1320,8 @@ static int gen11_dsi_compute_config(struct
> intel_encoder *encoder,
> >  	else
> >  		pipe_config->cpu_transcoder = TRANSCODER_DSI_0;
> >
> > +	pipe_config->dsi_state.op_mode = DSI_CMD_MODE_NO_GATE;
> > +	pipe_config->dsi_state.dual_link_mode = intel_dsi->dual_link;
> >  	pipe_config->clock_set = true;
> >  	pipe_config->port_clock = intel_dsi_bitrate(intel_dsi) / 5;
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 40390d855815..f89917eb4b94 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -75,6 +75,13 @@ enum hdmi_force_audio {
> >  	HDMI_AUDIO_ON,			/* force turn on HDMI audio
> */
> >  };
> >
> > +enum dsi_op_mode {
> > +	DSI_CMD_MODE_NO_GATE,
> > +	DSI_CMD_MODE_TE_GATE,
> > +	DSI_CMD_MODE_PERIODIC,
> > +	DSI_VIDEO_MODE,
> > +	};
> > +
> >  /* "Broadcast RGB" property */
> >  enum intel_broadcast_rgb {
> >  	INTEL_BROADCAST_RGB_AUTO,
> > @@ -861,6 +868,11 @@ struct intel_crtc_state {
> >  		u32 ctrl, div;
> >  	} dsi_pll;
> >
> > +	struct {
> > +		enum dsi_op_mode op_mode;
> > +		bool dual_link_mode;
> 
> 'dual_link' seems sufficient. And we should share that with LVDS.
Ok.. 
The reason I added dsi_op_mode was to ensure we do a deconfigure and configure the dsi in case any other kind of mode is set by the GOP
In current case the GOP puts the dsi in DSI_CMD_MODE_PERIODIC ( which is like updating the panel on every TE by the dsi transcoder itself)

To put it in the DSI_CMD_MODE_TE_GATE ( panel update whenever the software wants to update) we need to disable and enable in this mode.
So I thought of adding it in the crtc_state.
Do you think, if this mismatch in this particular mode can be handle with any other existing state variables..

> 
> Another easy target for moving to the crtc state is the lane count.
Ok.. I can try using this. There is one more requirement on every flip we need to update the FRMCTL register on the port where dsi is present in dsi te gate cmd mode.
Do you think if it would look good to use a state variable to represent that frame update? Or should we reach to some encoder function to do this job on every flip?
 
Thanks,
Vandita

> In fact we already have it in the state, but the DSI code has just stubbornly
> decided to ingore it.
> 
> > +	} dsi_state;
> > +
> >  	int pipe_bpp;
> >  	struct intel_link_m_n dp_m_n;
> >
> > --
> > 2.21.0.5.gaeb582a
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx