Re: [Intel-gfx] [RFC 7/7] drm/i915/dsi: Initiate frame request in cmd mode

2019-10-16 Thread Kulkarni, Vandita


> -Original Message-
> From: C, Ramalingam 
> Sent: Wednesday, October 16, 2019 3:45 PM
> To: Kulkarni, Vandita 
> Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani 
> Subject: Re: [Intel-gfx] [RFC 7/7] drm/i915/dsi: Initiate frame request in cmd
> mode
> 
> On 2019-10-14 at 16:31:22 +0530, Vandita Kulkarni wrote:
> > In TE Gate mode, on every flip we need to set the frame update request
> > bit. After this  bit is set transcoder hardware will automatically
> > send the frame data to the panel when it receives the TE event.
> >
> > Signed-off-by: Vandita Kulkarni 
> > ---
> >  drivers/gpu/drm/i915/display/icl_dsi.c| 27 +++
> >  drivers/gpu/drm/i915/display/intel_display.c  | 16 +++
> >  .../drm/i915/display/intel_display_types.h|  3 +++
> >  drivers/gpu/drm/i915/display/intel_dsi.h  |  3 +++
> >  4 files changed, 49 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> > b/drivers/gpu/drm/i915/display/icl_dsi.c
> > index 877746416e52..c72917ddf8e7 100644
> > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> > @@ -90,6 +90,33 @@ struct intel_encoder
> *gen11_dsi_find_cmd_mode_encoder(struct intel_crtc *crtc)
> > return NULL;
> >  }
> >
> > +void gen11_dsi_check_frame_update_needed(struct intel_crtc *intel_crtc,
> > +struct intel_crtc_state *crtc_state)
> 
> > +{
> > +   struct intel_encoder *intel_encoder = NULL;
> > +
> > +   intel_encoder = gen11_dsi_find_cmd_mode_encoder(intel_crtc);
> > +   if (!intel_encoder)
> > +   return;
> In this func, if you could find a DSI encoder with CMD mode, you set the
> flag!?
Yes, the requirement is to set this frame update bit on every flip.
Like Jani has commented on patch 4 where this parsing function is defined, will 
need to reconsider and replace with a better solution.
Working on it. 

Thanks,
Vandita
> > +
> > +   /* TBD Use bits  to say update on which  dsi port instead of a bool */
> > +   crtc_state->dsi_frame_update = true; }
> > +
> > +void gen11_dsi_frame_update(struct intel_crtc_state *crtc_state) {
> > +   struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > +   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > +   u32 tmp;
> > +
> > +   /* TBD: add check on port */
> > +   if (crtc_state->dsi_frame_update) {
> > +   tmp = I915_READ(ICL_DSI_CMD_FRMCTL(PORT_A));
> > +   tmp |= ICL_FRAME_UPDATE_REQUEST;
> > +   I915_WRITE(ICL_DSI_CMD_FRMCTL(PORT_A), tmp);
> > +   }
> > +}
> > +
> >  static void wait_for_cmds_dispatched_to_panel(struct intel_encoder
> > *encoder)  {
> > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); diff
> > --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 3cf39fc153b3..a902bb2bf075 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -11858,6 +11858,11 @@ static int intel_crtc_atomic_check(struct
> drm_crtc *_crtc,
> >  crtc_state);
> > }
> >
> > +   if ((INTEL_GEN(dev_priv) >= 11) &&
> > +   (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))) {
> Please check this alignment too
> 
> -Ram
> > +   gen11_dsi_check_frame_update_needed(crtc, crtc_state);
> > +   }
> > +
> > if (HAS_IPS(dev_priv))
> > crtc_state->ips_enabled =
> hsw_compute_ips_config(crtc_state);
> >
> > @@ -13618,6 +13623,7 @@ static int intel_atomic_check(struct
> drm_device *dev,
> > if (ret)
> > goto fail;
> >
> > +
> > for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> > new_crtc_state, i) {
> > if (!needs_modeset(new_crtc_state) && @@ -14108,6
> +14114,16 @@
> > static void intel_atomic_commit_tail(struct intel_atomic_state *state)
> > intel_color_load_luts(new_crtc_state);
> > }
> >
> > +   /*
> > +* Incase of mipi dsi command mode, we need to set frame update
> > +* for every commit
> > +*/
> > +   for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> > +   if (new_crtc_state->base.active &&
> > +   !needs_modeset(new_crtc_state) &&
&g

Re: [Intel-gfx] [RFC 7/7] drm/i915/dsi: Initiate frame request in cmd mode

2019-10-16 Thread Ramalingam C
On 2019-10-14 at 16:31:22 +0530, Vandita Kulkarni wrote:
> In TE Gate mode, on every flip we need to set the
> frame update request bit. After this  bit is set
> transcoder hardware will automatically send the
> frame data to the panel when it receives the TE event.
> 
> Signed-off-by: Vandita Kulkarni 
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c| 27 +++
>  drivers/gpu/drm/i915/display/intel_display.c  | 16 +++
>  .../drm/i915/display/intel_display_types.h|  3 +++
>  drivers/gpu/drm/i915/display/intel_dsi.h  |  3 +++
>  4 files changed, 49 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c 
> b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 877746416e52..c72917ddf8e7 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -90,6 +90,33 @@ struct intel_encoder 
> *gen11_dsi_find_cmd_mode_encoder(struct intel_crtc *crtc)
>   return NULL;
>  }
>  
> +void gen11_dsi_check_frame_update_needed(struct intel_crtc *intel_crtc,
> +  struct intel_crtc_state *crtc_state)

> +{
> + struct intel_encoder *intel_encoder = NULL;
> +
> + intel_encoder = gen11_dsi_find_cmd_mode_encoder(intel_crtc);
> + if (!intel_encoder)
> + return;
In this func, if you could find a DSI encoder with CMD mode, you set the
flag!?
> +
> + /* TBD Use bits  to say update on which  dsi port instead of a bool */
> + crtc_state->dsi_frame_update = true;
> +}
> +
> +void gen11_dsi_frame_update(struct intel_crtc_state *crtc_state)
> +{
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + u32 tmp;
> +
> + /* TBD: add check on port */
> + if (crtc_state->dsi_frame_update) {
> + tmp = I915_READ(ICL_DSI_CMD_FRMCTL(PORT_A));
> + tmp |= ICL_FRAME_UPDATE_REQUEST;
> + I915_WRITE(ICL_DSI_CMD_FRMCTL(PORT_A), tmp);
> + }
> +}
> +
>  static void wait_for_cmds_dispatched_to_panel(struct intel_encoder *encoder)
>  {
>   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 3cf39fc153b3..a902bb2bf075 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -11858,6 +11858,11 @@ static int intel_crtc_atomic_check(struct drm_crtc 
> *_crtc,
>crtc_state);
>   }
>  
> + if ((INTEL_GEN(dev_priv) >= 11) &&
> + (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))) {
Please check this alignment too

-Ram
> + gen11_dsi_check_frame_update_needed(crtc, crtc_state);
> + }
> +
>   if (HAS_IPS(dev_priv))
>   crtc_state->ips_enabled = hsw_compute_ips_config(crtc_state);
>  
> @@ -13618,6 +13623,7 @@ static int intel_atomic_check(struct drm_device *dev,
>   if (ret)
>   goto fail;
>  
> +
>   for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>   new_crtc_state, i) {
>   if (!needs_modeset(new_crtc_state) &&
> @@ -14108,6 +14114,16 @@ static void intel_atomic_commit_tail(struct 
> intel_atomic_state *state)
>   intel_color_load_luts(new_crtc_state);
>   }
>  
> + /*
> +  * Incase of mipi dsi command mode, we need to set frame update
> +  * for every commit
> +  */
> + for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> + if (new_crtc_state->base.active &&
> + !needs_modeset(new_crtc_state) &&
> + new_crtc_state->dsi_frame_update)
> + gen11_dsi_frame_update(new_crtc_state);
> + }
>   /*
>* Now that the vblank has passed, we can go ahead and program the
>* optimal watermarks on platforms that need two-step watermark
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 40390d855815..69da4ec45691 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -991,6 +991,9 @@ struct intel_crtc_state {
>  
>   /* Forward Error correction State */
>   bool fec_enable;
> +
> + /* frame update for dsi command mode */
> + bool dsi_frame_update;
>  };
>  
>  struct intel_crtc {
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h 
> b/drivers/gpu/drm/i915/display/intel_dsi.h
> index 071dad7ee04a..d9350b842115 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/display/intel_dsi.h
> @@ -203,6 +203,9 @@ void bxt_dsi_reset_clocks(struct intel_encoder *encoder, 
> enum port port);
>  
>  /* icl_dsi.c */
>  struct intel_encoder 

[Intel-gfx] [RFC 7/7] drm/i915/dsi: Initiate frame request in cmd mode

2019-10-14 Thread Vandita Kulkarni
In TE Gate mode, on every flip we need to set the
frame update request bit. After this  bit is set
transcoder hardware will automatically send the
frame data to the panel when it receives the TE event.

Signed-off-by: Vandita Kulkarni 
---
 drivers/gpu/drm/i915/display/icl_dsi.c| 27 +++
 drivers/gpu/drm/i915/display/intel_display.c  | 16 +++
 .../drm/i915/display/intel_display_types.h|  3 +++
 drivers/gpu/drm/i915/display/intel_dsi.h  |  3 +++
 4 files changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c 
b/drivers/gpu/drm/i915/display/icl_dsi.c
index 877746416e52..c72917ddf8e7 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -90,6 +90,33 @@ struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct 
intel_crtc *crtc)
return NULL;
 }
 
+void gen11_dsi_check_frame_update_needed(struct intel_crtc *intel_crtc,
+struct intel_crtc_state *crtc_state)
+{
+   struct intel_encoder *intel_encoder = NULL;
+
+   intel_encoder = gen11_dsi_find_cmd_mode_encoder(intel_crtc);
+   if (!intel_encoder)
+   return;
+
+   /* TBD Use bits  to say update on which  dsi port instead of a bool */
+   crtc_state->dsi_frame_update = true;
+}
+
+void gen11_dsi_frame_update(struct intel_crtc_state *crtc_state)
+{
+   struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+   u32 tmp;
+
+   /* TBD: add check on port */
+   if (crtc_state->dsi_frame_update) {
+   tmp = I915_READ(ICL_DSI_CMD_FRMCTL(PORT_A));
+   tmp |= ICL_FRAME_UPDATE_REQUEST;
+   I915_WRITE(ICL_DSI_CMD_FRMCTL(PORT_A), tmp);
+   }
+}
+
 static void wait_for_cmds_dispatched_to_panel(struct intel_encoder *encoder)
 {
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 3cf39fc153b3..a902bb2bf075 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -11858,6 +11858,11 @@ static int intel_crtc_atomic_check(struct drm_crtc 
*_crtc,
 crtc_state);
}
 
+   if ((INTEL_GEN(dev_priv) >= 11) &&
+   (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))) {
+   gen11_dsi_check_frame_update_needed(crtc, crtc_state);
+   }
+
if (HAS_IPS(dev_priv))
crtc_state->ips_enabled = hsw_compute_ips_config(crtc_state);
 
@@ -13618,6 +13623,7 @@ static int intel_atomic_check(struct drm_device *dev,
if (ret)
goto fail;
 
+
for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
if (!needs_modeset(new_crtc_state) &&
@@ -14108,6 +14114,16 @@ static void intel_atomic_commit_tail(struct 
intel_atomic_state *state)
intel_color_load_luts(new_crtc_state);
}
 
+   /*
+* Incase of mipi dsi command mode, we need to set frame update
+* for every commit
+*/
+   for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
+   if (new_crtc_state->base.active &&
+   !needs_modeset(new_crtc_state) &&
+   new_crtc_state->dsi_frame_update)
+   gen11_dsi_frame_update(new_crtc_state);
+   }
/*
 * Now that the vblank has passed, we can go ahead and program the
 * optimal watermarks on platforms that need two-step watermark
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 40390d855815..69da4ec45691 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -991,6 +991,9 @@ struct intel_crtc_state {
 
/* Forward Error correction State */
bool fec_enable;
+
+   /* frame update for dsi command mode */
+   bool dsi_frame_update;
 };
 
 struct intel_crtc {
diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h 
b/drivers/gpu/drm/i915/display/intel_dsi.h
index 071dad7ee04a..d9350b842115 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi.h
+++ b/drivers/gpu/drm/i915/display/intel_dsi.h
@@ -203,6 +203,9 @@ void bxt_dsi_reset_clocks(struct intel_encoder *encoder, 
enum port port);
 
 /* icl_dsi.c */
 struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct intel_crtc *crtc);
+void gen11_dsi_check_frame_update_needed(struct intel_crtc *crtc,
+struct intel_crtc_state *crtc_state);
+void gen11_dsi_frame_update(struct intel_crtc_state *crtc_state);
 
 /* intel_dsi_vbt.c */
 bool intel_dsi_vbt_init(struct intel_dsi