Re: [Intel-gfx] [PATCH 03/13] drm/i915/dsb: Split intel_dsb_wait() from intel_dsb_commit()

2023-02-02 Thread Manna, Animesh


> -Original Message-
> From: Intel-gfx  On Behalf Of Ville
> Syrjala
> Sent: Wednesday, January 18, 2023 10:01 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 03/13] drm/i915/dsb: Split intel_dsb_wait() from
> intel_dsb_commit()
> 
> From: Ville Syrjälä 
> 
> Starting the DSB execution vs. waiting for it stop are two totally different
> things. Split intel_dsb_wait() from
> intel_dsb_commit() so that we can eventually allow the DSB to execute
> asynchronously.
> 
> Signed-off-by: Ville Syrjälä 

LGTM.
Reviewed-by: Animesh Manna 

> ---
>  drivers/gpu/drm/i915/display/intel_color.c |  4 +++-
>  drivers/gpu/drm/i915/display/intel_dsb.c   | 11 +--
>  drivers/gpu/drm/i915/display/intel_dsb.h   |  1 +
>  3 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 8d97c299e657..5d99913429b9 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1256,8 +1256,10 @@ static void icl_load_luts(const struct
> intel_crtc_state *crtc_state)
>   break;
>   }
> 
> - if (crtc_state->dsb)
> + if (crtc_state->dsb) {
>   intel_dsb_commit(crtc_state->dsb);
> + intel_dsb_wait(crtc_state->dsb);
> + }
>  }
> 
>  static u32 chv_cgm_degamma_ldw(const struct drm_color_lut *color) diff --
> git a/drivers/gpu/drm/i915/display/intel_dsb.c
> b/drivers/gpu/drm/i915/display/intel_dsb.c
> index f41146fc84d7..0b2faa33f204 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -235,7 +235,7 @@ void intel_dsb_commit(struct intel_dsb *dsb)
>   if (is_dsb_busy(dev_priv, pipe, dsb->id)) {
>   drm_err(_priv->drm, "[CRTC:%d:%s] DSB %d is busy\n",
>   crtc->base.base.id, crtc->base.name, dsb->id);
> - goto reset;
> + return;
>   }
> 
>   intel_de_write(dev_priv, DSB_CTRL(pipe, dsb->id), @@ -249,13
> +249,20 @@ void intel_dsb_commit(struct intel_dsb *dsb)
>   "DSB execution started - head 0x%x, tail 0x%x\n",
>   i915_ggtt_offset(dsb->vma),
>   i915_ggtt_offset(dsb->vma) + tail);
> +}
> +
> +void intel_dsb_wait(struct intel_dsb *dsb) {
> + struct intel_crtc *crtc = dsb->crtc;
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + enum pipe pipe = crtc->pipe;
> 
>   if (wait_for(!is_dsb_busy(dev_priv, pipe, dsb->id), 1))
>   drm_err(_priv->drm,
>   "[CRTC:%d:%s] DSB %d timed out waiting for idle\n",
>   crtc->base.base.id, crtc->base.name, dsb->id);
> 
> -reset:
> + /* Attempt to reset it */
>   dsb->free_pos = 0;
>   dsb->ins_start_offset = 0;
>   intel_de_write(dev_priv, DSB_CTRL(pipe, dsb->id), 0); diff --git
> a/drivers/gpu/drm/i915/display/intel_dsb.h
> b/drivers/gpu/drm/i915/display/intel_dsb.h
> index 05c221b6d0a4..7999199c2464 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.h
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.h
> @@ -19,5 +19,6 @@ void intel_dsb_cleanup(struct intel_dsb *dsb);  void
> intel_dsb_reg_write(struct intel_dsb *dsb,
>i915_reg_t reg, u32 val);
>  void intel_dsb_commit(struct intel_dsb *dsb);
> +void intel_dsb_wait(struct intel_dsb *dsb);
> 
>  #endif
> --
> 2.38.2



[Intel-gfx] [PATCH 03/13] drm/i915/dsb: Split intel_dsb_wait() from intel_dsb_commit()

2023-01-18 Thread Ville Syrjala
From: Ville Syrjälä 

Starting the DSB execution vs. waiting for it stop are two
totally different things. Split intel_dsb_wait() from
intel_dsb_commit() so that we can eventually allow the DSB
to execute asynchronously.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/display/intel_color.c |  4 +++-
 drivers/gpu/drm/i915/display/intel_dsb.c   | 11 +--
 drivers/gpu/drm/i915/display/intel_dsb.h   |  1 +
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c 
b/drivers/gpu/drm/i915/display/intel_color.c
index 8d97c299e657..5d99913429b9 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1256,8 +1256,10 @@ static void icl_load_luts(const struct intel_crtc_state 
*crtc_state)
break;
}
 
-   if (crtc_state->dsb)
+   if (crtc_state->dsb) {
intel_dsb_commit(crtc_state->dsb);
+   intel_dsb_wait(crtc_state->dsb);
+   }
 }
 
 static u32 chv_cgm_degamma_ldw(const struct drm_color_lut *color)
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c 
b/drivers/gpu/drm/i915/display/intel_dsb.c
index f41146fc84d7..0b2faa33f204 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -235,7 +235,7 @@ void intel_dsb_commit(struct intel_dsb *dsb)
if (is_dsb_busy(dev_priv, pipe, dsb->id)) {
drm_err(_priv->drm, "[CRTC:%d:%s] DSB %d is busy\n",
crtc->base.base.id, crtc->base.name, dsb->id);
-   goto reset;
+   return;
}
 
intel_de_write(dev_priv, DSB_CTRL(pipe, dsb->id),
@@ -249,13 +249,20 @@ void intel_dsb_commit(struct intel_dsb *dsb)
"DSB execution started - head 0x%x, tail 0x%x\n",
i915_ggtt_offset(dsb->vma),
i915_ggtt_offset(dsb->vma) + tail);
+}
+
+void intel_dsb_wait(struct intel_dsb *dsb)
+{
+   struct intel_crtc *crtc = dsb->crtc;
+   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+   enum pipe pipe = crtc->pipe;
 
if (wait_for(!is_dsb_busy(dev_priv, pipe, dsb->id), 1))
drm_err(_priv->drm,
"[CRTC:%d:%s] DSB %d timed out waiting for idle\n",
crtc->base.base.id, crtc->base.name, dsb->id);
 
-reset:
+   /* Attempt to reset it */
dsb->free_pos = 0;
dsb->ins_start_offset = 0;
intel_de_write(dev_priv, DSB_CTRL(pipe, dsb->id), 0);
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h 
b/drivers/gpu/drm/i915/display/intel_dsb.h
index 05c221b6d0a4..7999199c2464 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.h
+++ b/drivers/gpu/drm/i915/display/intel_dsb.h
@@ -19,5 +19,6 @@ void intel_dsb_cleanup(struct intel_dsb *dsb);
 void intel_dsb_reg_write(struct intel_dsb *dsb,
 i915_reg_t reg, u32 val);
 void intel_dsb_commit(struct intel_dsb *dsb);
+void intel_dsb_wait(struct intel_dsb *dsb);
 
 #endif
-- 
2.38.2