All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/msm/dpu: update the qos remap only if the client type changes
@ 2020-11-19 21:41 ` Abhinav Kumar
  0 siblings, 0 replies; 8+ messages in thread
From: Abhinav Kumar @ 2020-11-19 21:41 UTC (permalink / raw)
  To: dri-devel
  Cc: Abhinav Kumar, linux-arm-msm, freedreno, robdclark, seanpaul,
	swboyd, nganji, aravindh, tanmay, khsieh

Update the qos remap only if the client type changes for the plane.
This will avoid unnecessary register programming and also avoid log
spam from the dpu_vbif_set_qos_remap() function.

Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 17 +++++++++++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h    |  7 +++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  2 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 12 ++++++++++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   |  5 +++++
 5 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index d4662e8184cc..3867da47c683 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1037,6 +1037,23 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
 	return rc;
 }
 
+void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc)
+{
+	struct drm_plane *plane;
+	struct drm_plane_state *state;
+	struct dpu_plane_state *pstate;
+
+	drm_atomic_crtc_for_each_plane(plane, crtc) {
+		state = plane->state;
+		if (!state)
+			continue;
+
+		pstate = to_dpu_plane_state(state);
+
+		pstate->dirty |= DPU_PLANE_DIRTY_QOS;
+	}
+}
+
 int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
 {
 	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index cec3474340e8..8ba11de605bc 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -231,6 +231,13 @@ static inline int dpu_crtc_frame_pending(struct drm_crtc *crtc)
  */
 int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);
 
+/**
+ * dpu_crtc_set_qos_dirty - update plane dirty flag to include
+ * QoS reprogramming
+ * @crtc: Pointer to drm crtc structure
+ */
+void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc);
+
 /**
  * dpu_crtc_vblank_callback - called on vblank irq, issues completion events
  * @crtc: Pointer to drm crtc object
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index f7f5c258b553..c2db9dd6ec67 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1001,6 +1001,8 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 
 	trace_dpu_enc_mode_set(DRMID(drm_enc));
 
+	dpu_crtc_set_qos_dirty(drm_enc->crtc);
+
 	if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS && priv->dp)
 		msm_dp_display_mode_set(priv->dp, drm_enc, mode, adj_mode);
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 7ea90d25a3b6..f91d31a31e14 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1066,6 +1066,7 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
 	struct dpu_plane_state *pstate = to_dpu_plane_state(state);
 	struct drm_crtc *crtc = state->crtc;
 	struct drm_framebuffer *fb = state->fb;
+	bool is_rt_pipe;
 	const struct dpu_format *fmt =
 		to_dpu_format(msm_framebuffer_format(fb));
 
@@ -1075,7 +1076,7 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
 
 	pstate->pending = true;
 
-	pdpu->is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
+	is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
 	_dpu_plane_set_qos_ctrl(plane, false, DPU_PLANE_QOS_PANIC_CTRL);
 
 	DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " DRM_RECT_FMT
@@ -1181,8 +1182,15 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
 		_dpu_plane_set_ot_limit(plane, crtc);
 	}
 
-	_dpu_plane_set_qos_remap(plane);
+	if (is_rt_pipe != pdpu->is_rt_pipe) {
+		pdpu->is_rt_pipe = is_rt_pipe;
+		pstate->dirty |= DPU_PLANE_DIRTY_QOS;
+	}
 
+	if (pstate->dirty & DPU_PLANE_DIRTY_QOS) {
+		_dpu_plane_set_qos_remap(plane);
+		pstate->dirty = 0x0;
+	}
 	_dpu_plane_calc_bw(plane, fb);
 
 	_dpu_plane_calc_clk(plane);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index ca83b8753d59..47abd3686a86 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -14,11 +14,15 @@
 #include "dpu_hw_mdss.h"
 #include "dpu_hw_sspp.h"
 
+/* dirty bits to update QOS */
+#define DPU_PLANE_DIRTY_QOS 0x1
+
 /**
  * struct dpu_plane_state: Define dpu extension of drm plane state object
  * @base:	base drm plane state object
  * @aspace:	pointer to address space for input/output buffers
  * @stage:	assigned by crtc blender
+ * @dirty:	bitmask for which pipe h/w config functions need to be updated
  * @multirect_index: index of the rectangle of SSPP
  * @multirect_mode: parallel or time multiplex multirect mode
  * @pending:	whether the current update is still pending
@@ -32,6 +36,7 @@ struct dpu_plane_state {
 	struct drm_plane_state base;
 	struct msm_gem_address_space *aspace;
 	enum dpu_stage stage;
+	uint32_t dirty;
 	uint32_t multirect_index;
 	uint32_t multirect_mode;
 	bool pending;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH] drm/msm/dpu: update the qos remap only if the client type changes
@ 2020-11-19 21:41 ` Abhinav Kumar
  0 siblings, 0 replies; 8+ messages in thread
From: Abhinav Kumar @ 2020-11-19 21:41 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-arm-msm, Abhinav Kumar, swboyd, khsieh, seanpaul, tanmay,
	aravindh, freedreno

Update the qos remap only if the client type changes for the plane.
This will avoid unnecessary register programming and also avoid log
spam from the dpu_vbif_set_qos_remap() function.

Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 17 +++++++++++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h    |  7 +++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  2 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 12 ++++++++++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   |  5 +++++
 5 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index d4662e8184cc..3867da47c683 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1037,6 +1037,23 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
 	return rc;
 }
 
+void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc)
+{
+	struct drm_plane *plane;
+	struct drm_plane_state *state;
+	struct dpu_plane_state *pstate;
+
+	drm_atomic_crtc_for_each_plane(plane, crtc) {
+		state = plane->state;
+		if (!state)
+			continue;
+
+		pstate = to_dpu_plane_state(state);
+
+		pstate->dirty |= DPU_PLANE_DIRTY_QOS;
+	}
+}
+
 int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
 {
 	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index cec3474340e8..8ba11de605bc 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -231,6 +231,13 @@ static inline int dpu_crtc_frame_pending(struct drm_crtc *crtc)
  */
 int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);
 
+/**
+ * dpu_crtc_set_qos_dirty - update plane dirty flag to include
+ * QoS reprogramming
+ * @crtc: Pointer to drm crtc structure
+ */
+void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc);
+
 /**
  * dpu_crtc_vblank_callback - called on vblank irq, issues completion events
  * @crtc: Pointer to drm crtc object
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index f7f5c258b553..c2db9dd6ec67 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1001,6 +1001,8 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 
 	trace_dpu_enc_mode_set(DRMID(drm_enc));
 
+	dpu_crtc_set_qos_dirty(drm_enc->crtc);
+
 	if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS && priv->dp)
 		msm_dp_display_mode_set(priv->dp, drm_enc, mode, adj_mode);
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 7ea90d25a3b6..f91d31a31e14 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1066,6 +1066,7 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
 	struct dpu_plane_state *pstate = to_dpu_plane_state(state);
 	struct drm_crtc *crtc = state->crtc;
 	struct drm_framebuffer *fb = state->fb;
+	bool is_rt_pipe;
 	const struct dpu_format *fmt =
 		to_dpu_format(msm_framebuffer_format(fb));
 
@@ -1075,7 +1076,7 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
 
 	pstate->pending = true;
 
-	pdpu->is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
+	is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
 	_dpu_plane_set_qos_ctrl(plane, false, DPU_PLANE_QOS_PANIC_CTRL);
 
 	DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " DRM_RECT_FMT
@@ -1181,8 +1182,15 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
 		_dpu_plane_set_ot_limit(plane, crtc);
 	}
 
-	_dpu_plane_set_qos_remap(plane);
+	if (is_rt_pipe != pdpu->is_rt_pipe) {
+		pdpu->is_rt_pipe = is_rt_pipe;
+		pstate->dirty |= DPU_PLANE_DIRTY_QOS;
+	}
 
+	if (pstate->dirty & DPU_PLANE_DIRTY_QOS) {
+		_dpu_plane_set_qos_remap(plane);
+		pstate->dirty = 0x0;
+	}
 	_dpu_plane_calc_bw(plane, fb);
 
 	_dpu_plane_calc_clk(plane);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index ca83b8753d59..47abd3686a86 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -14,11 +14,15 @@
 #include "dpu_hw_mdss.h"
 #include "dpu_hw_sspp.h"
 
+/* dirty bits to update QOS */
+#define DPU_PLANE_DIRTY_QOS 0x1
+
 /**
  * struct dpu_plane_state: Define dpu extension of drm plane state object
  * @base:	base drm plane state object
  * @aspace:	pointer to address space for input/output buffers
  * @stage:	assigned by crtc blender
+ * @dirty:	bitmask for which pipe h/w config functions need to be updated
  * @multirect_index: index of the rectangle of SSPP
  * @multirect_mode: parallel or time multiplex multirect mode
  * @pending:	whether the current update is still pending
@@ -32,6 +36,7 @@ struct dpu_plane_state {
 	struct drm_plane_state base;
 	struct msm_gem_address_space *aspace;
 	enum dpu_stage stage;
+	uint32_t dirty;
 	uint32_t multirect_index;
 	uint32_t multirect_mode;
 	bool pending;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] drm/msm/dpu: update the qos remap only if the client type changes
  2020-11-19 21:41 ` Abhinav Kumar
@ 2020-11-23 23:18   ` Rob Clark
  -1 siblings, 0 replies; 8+ messages in thread
From: Rob Clark @ 2020-11-23 23:18 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: dri-devel, linux-arm-msm, freedreno, Sean Paul, Stephen Boyd,
	nganji, aravindh, Tanmay Shah, Kuogee Hsieh

On Thu, Nov 19, 2020 at 1:41 PM Abhinav Kumar <abhinavk@codeaurora.org> wrote:
>
> Update the qos remap only if the client type changes for the plane.
> This will avoid unnecessary register programming and also avoid log
> spam from the dpu_vbif_set_qos_remap() function.
>
> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 17 +++++++++++++++++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h    |  7 +++++++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  2 ++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 12 ++++++++++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   |  5 +++++
>  5 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index d4662e8184cc..3867da47c683 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1037,6 +1037,23 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
>         return rc;
>  }
>
> +void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc)
> +{
> +       struct drm_plane *plane;
> +       struct drm_plane_state *state;
> +       struct dpu_plane_state *pstate;
> +
> +       drm_atomic_crtc_for_each_plane(plane, crtc) {
> +               state = plane->state;
> +               if (!state)
> +                       continue;
> +
> +               pstate = to_dpu_plane_state(state);
> +
> +               pstate->dirty |= DPU_PLANE_DIRTY_QOS;
> +       }
> +}
> +
>  int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
>  {
>         struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index cec3474340e8..8ba11de605bc 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -231,6 +231,13 @@ static inline int dpu_crtc_frame_pending(struct drm_crtc *crtc)
>   */
>  int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);
>
> +/**
> + * dpu_crtc_set_qos_dirty - update plane dirty flag to include
> + * QoS reprogramming
> + * @crtc: Pointer to drm crtc structure
> + */
> +void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc);
> +
>  /**
>   * dpu_crtc_vblank_callback - called on vblank irq, issues completion events
>   * @crtc: Pointer to drm crtc object
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index f7f5c258b553..c2db9dd6ec67 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1001,6 +1001,8 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>
>         trace_dpu_enc_mode_set(DRMID(drm_enc));
>
> +       dpu_crtc_set_qos_dirty(drm_enc->crtc);
> +
>         if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS && priv->dp)
>                 msm_dp_display_mode_set(priv->dp, drm_enc, mode, adj_mode);
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 7ea90d25a3b6..f91d31a31e14 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1066,6 +1066,7 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>         struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>         struct drm_crtc *crtc = state->crtc;
>         struct drm_framebuffer *fb = state->fb;
> +       bool is_rt_pipe;
>         const struct dpu_format *fmt =
>                 to_dpu_format(msm_framebuffer_format(fb));
>
> @@ -1075,7 +1076,7 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>
>         pstate->pending = true;
>
> -       pdpu->is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
> +       is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
>         _dpu_plane_set_qos_ctrl(plane, false, DPU_PLANE_QOS_PANIC_CTRL);
>
>         DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " DRM_RECT_FMT
> @@ -1181,8 +1182,15 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>                 _dpu_plane_set_ot_limit(plane, crtc);
>         }
>
> -       _dpu_plane_set_qos_remap(plane);
> +       if (is_rt_pipe != pdpu->is_rt_pipe) {
> +               pdpu->is_rt_pipe = is_rt_pipe;
> +               pstate->dirty |= DPU_PLANE_DIRTY_QOS;
> +       }
>
> +       if (pstate->dirty & DPU_PLANE_DIRTY_QOS) {
> +               _dpu_plane_set_qos_remap(plane);
> +               pstate->dirty = 0x0;
> +       }

So in the end, this looks roughly like "set qos remap on modesets or
switching between right/left pipe"?  Couldn't this be simpler if in
plane->atomic_check() you do something like:

   dpu_plane_state->needs_qos_remap = atomic_state->allow_modeset;

and then in plane->atomic_update:

    if (pstate->needs_qos_remap || (is_rt_pipe != pdpu->is_rt_pipe) {
         pdpu->is_rt_pipe = is_rt_pipe;
         _dpu_plane_set_qos_remap(plane)
    }

?

BR,
-R


>         _dpu_plane_calc_bw(plane, fb);
>
>         _dpu_plane_calc_clk(plane);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> index ca83b8753d59..47abd3686a86 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> @@ -14,11 +14,15 @@
>  #include "dpu_hw_mdss.h"
>  #include "dpu_hw_sspp.h"
>
> +/* dirty bits to update QOS */
> +#define DPU_PLANE_DIRTY_QOS 0x1
> +
>  /**
>   * struct dpu_plane_state: Define dpu extension of drm plane state object
>   * @base:      base drm plane state object
>   * @aspace:    pointer to address space for input/output buffers
>   * @stage:     assigned by crtc blender
> + * @dirty:     bitmask for which pipe h/w config functions need to be updated
>   * @multirect_index: index of the rectangle of SSPP
>   * @multirect_mode: parallel or time multiplex multirect mode
>   * @pending:   whether the current update is still pending
> @@ -32,6 +36,7 @@ struct dpu_plane_state {
>         struct drm_plane_state base;
>         struct msm_gem_address_space *aspace;
>         enum dpu_stage stage;
> +       uint32_t dirty;
>         uint32_t multirect_index;
>         uint32_t multirect_mode;
>         bool pending;
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] drm/msm/dpu: update the qos remap only if the client type changes
@ 2020-11-23 23:18   ` Rob Clark
  0 siblings, 0 replies; 8+ messages in thread
From: Rob Clark @ 2020-11-23 23:18 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: linux-arm-msm, dri-devel, Stephen Boyd, Kuogee Hsieh, Sean Paul,
	Tanmay Shah, aravindh, freedreno

On Thu, Nov 19, 2020 at 1:41 PM Abhinav Kumar <abhinavk@codeaurora.org> wrote:
>
> Update the qos remap only if the client type changes for the plane.
> This will avoid unnecessary register programming and also avoid log
> spam from the dpu_vbif_set_qos_remap() function.
>
> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 17 +++++++++++++++++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h    |  7 +++++++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  2 ++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 12 ++++++++++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   |  5 +++++
>  5 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index d4662e8184cc..3867da47c683 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1037,6 +1037,23 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
>         return rc;
>  }
>
> +void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc)
> +{
> +       struct drm_plane *plane;
> +       struct drm_plane_state *state;
> +       struct dpu_plane_state *pstate;
> +
> +       drm_atomic_crtc_for_each_plane(plane, crtc) {
> +               state = plane->state;
> +               if (!state)
> +                       continue;
> +
> +               pstate = to_dpu_plane_state(state);
> +
> +               pstate->dirty |= DPU_PLANE_DIRTY_QOS;
> +       }
> +}
> +
>  int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
>  {
>         struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index cec3474340e8..8ba11de605bc 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -231,6 +231,13 @@ static inline int dpu_crtc_frame_pending(struct drm_crtc *crtc)
>   */
>  int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);
>
> +/**
> + * dpu_crtc_set_qos_dirty - update plane dirty flag to include
> + * QoS reprogramming
> + * @crtc: Pointer to drm crtc structure
> + */
> +void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc);
> +
>  /**
>   * dpu_crtc_vblank_callback - called on vblank irq, issues completion events
>   * @crtc: Pointer to drm crtc object
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index f7f5c258b553..c2db9dd6ec67 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1001,6 +1001,8 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>
>         trace_dpu_enc_mode_set(DRMID(drm_enc));
>
> +       dpu_crtc_set_qos_dirty(drm_enc->crtc);
> +
>         if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS && priv->dp)
>                 msm_dp_display_mode_set(priv->dp, drm_enc, mode, adj_mode);
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 7ea90d25a3b6..f91d31a31e14 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1066,6 +1066,7 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>         struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>         struct drm_crtc *crtc = state->crtc;
>         struct drm_framebuffer *fb = state->fb;
> +       bool is_rt_pipe;
>         const struct dpu_format *fmt =
>                 to_dpu_format(msm_framebuffer_format(fb));
>
> @@ -1075,7 +1076,7 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>
>         pstate->pending = true;
>
> -       pdpu->is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
> +       is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
>         _dpu_plane_set_qos_ctrl(plane, false, DPU_PLANE_QOS_PANIC_CTRL);
>
>         DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " DRM_RECT_FMT
> @@ -1181,8 +1182,15 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>                 _dpu_plane_set_ot_limit(plane, crtc);
>         }
>
> -       _dpu_plane_set_qos_remap(plane);
> +       if (is_rt_pipe != pdpu->is_rt_pipe) {
> +               pdpu->is_rt_pipe = is_rt_pipe;
> +               pstate->dirty |= DPU_PLANE_DIRTY_QOS;
> +       }
>
> +       if (pstate->dirty & DPU_PLANE_DIRTY_QOS) {
> +               _dpu_plane_set_qos_remap(plane);
> +               pstate->dirty = 0x0;
> +       }

So in the end, this looks roughly like "set qos remap on modesets or
switching between right/left pipe"?  Couldn't this be simpler if in
plane->atomic_check() you do something like:

   dpu_plane_state->needs_qos_remap = atomic_state->allow_modeset;

and then in plane->atomic_update:

    if (pstate->needs_qos_remap || (is_rt_pipe != pdpu->is_rt_pipe) {
         pdpu->is_rt_pipe = is_rt_pipe;
         _dpu_plane_set_qos_remap(plane)
    }

?

BR,
-R


>         _dpu_plane_calc_bw(plane, fb);
>
>         _dpu_plane_calc_clk(plane);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> index ca83b8753d59..47abd3686a86 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> @@ -14,11 +14,15 @@
>  #include "dpu_hw_mdss.h"
>  #include "dpu_hw_sspp.h"
>
> +/* dirty bits to update QOS */
> +#define DPU_PLANE_DIRTY_QOS 0x1
> +
>  /**
>   * struct dpu_plane_state: Define dpu extension of drm plane state object
>   * @base:      base drm plane state object
>   * @aspace:    pointer to address space for input/output buffers
>   * @stage:     assigned by crtc blender
> + * @dirty:     bitmask for which pipe h/w config functions need to be updated
>   * @multirect_index: index of the rectangle of SSPP
>   * @multirect_mode: parallel or time multiplex multirect mode
>   * @pending:   whether the current update is still pending
> @@ -32,6 +36,7 @@ struct dpu_plane_state {
>         struct drm_plane_state base;
>         struct msm_gem_address_space *aspace;
>         enum dpu_stage stage;
> +       uint32_t dirty;
>         uint32_t multirect_index;
>         uint32_t multirect_mode;
>         bool pending;
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] drm/msm/dpu: update the qos remap only if the client type changes
  2020-11-23 23:18   ` Rob Clark
@ 2020-11-24  0:38     ` abhinavk
  -1 siblings, 0 replies; 8+ messages in thread
From: abhinavk @ 2020-11-24  0:38 UTC (permalink / raw)
  To: Rob Clark
  Cc: dri-devel, linux-arm-msm, freedreno, Sean Paul, Stephen Boyd,
	nganji, aravindh, Tanmay Shah, Kuogee Hsieh

Hi Rob

On 2020-11-23 15:18, Rob Clark wrote:
> On Thu, Nov 19, 2020 at 1:41 PM Abhinav Kumar <abhinavk@codeaurora.org> 
> wrote:
>> 
>> Update the qos remap only if the client type changes for the plane.
>> This will avoid unnecessary register programming and also avoid log
>> spam from the dpu_vbif_set_qos_remap() function.
>> 
>> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
>> ---
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 17 +++++++++++++++++
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h    |  7 +++++++
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  2 ++
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 12 ++++++++++--
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   |  5 +++++
>>  5 files changed, 41 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> index d4662e8184cc..3867da47c683 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> @@ -1037,6 +1037,23 @@ static int dpu_crtc_atomic_check(struct 
>> drm_crtc *crtc,
>>         return rc;
>>  }
>> 
>> +void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc)
>> +{
>> +       struct drm_plane *plane;
>> +       struct drm_plane_state *state;
>> +       struct dpu_plane_state *pstate;
>> +
>> +       drm_atomic_crtc_for_each_plane(plane, crtc) {
>> +               state = plane->state;
>> +               if (!state)
>> +                       continue;
>> +
>> +               pstate = to_dpu_plane_state(state);
>> +
>> +               pstate->dirty |= DPU_PLANE_DIRTY_QOS;
>> +       }
>> +}
>> +
>>  int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
>>  {
>>         struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
>> index cec3474340e8..8ba11de605bc 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
>> @@ -231,6 +231,13 @@ static inline int dpu_crtc_frame_pending(struct 
>> drm_crtc *crtc)
>>   */
>>  int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);
>> 
>> +/**
>> + * dpu_crtc_set_qos_dirty - update plane dirty flag to include
>> + * QoS reprogramming
>> + * @crtc: Pointer to drm crtc structure
>> + */
>> +void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc);
>> +
>>  /**
>>   * dpu_crtc_vblank_callback - called on vblank irq, issues completion 
>> events
>>   * @crtc: Pointer to drm crtc object
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> index f7f5c258b553..c2db9dd6ec67 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> @@ -1001,6 +1001,8 @@ static void dpu_encoder_virt_mode_set(struct 
>> drm_encoder *drm_enc,
>> 
>>         trace_dpu_enc_mode_set(DRMID(drm_enc));
>> 
>> +       dpu_crtc_set_qos_dirty(drm_enc->crtc);
>> +
>>         if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS && 
>> priv->dp)
>>                 msm_dp_display_mode_set(priv->dp, drm_enc, mode, 
>> adj_mode);
>> 
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> index 7ea90d25a3b6..f91d31a31e14 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> @@ -1066,6 +1066,7 @@ static void dpu_plane_sspp_atomic_update(struct 
>> drm_plane *plane)
>>         struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>>         struct drm_crtc *crtc = state->crtc;
>>         struct drm_framebuffer *fb = state->fb;
>> +       bool is_rt_pipe;
>>         const struct dpu_format *fmt =
>>                 to_dpu_format(msm_framebuffer_format(fb));
>> 
>> @@ -1075,7 +1076,7 @@ static void dpu_plane_sspp_atomic_update(struct 
>> drm_plane *plane)
>> 
>>         pstate->pending = true;
>> 
>> -       pdpu->is_rt_pipe = (dpu_crtc_get_client_type(crtc) != 
>> NRT_CLIENT);
>> +       is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
>>         _dpu_plane_set_qos_ctrl(plane, false, 
>> DPU_PLANE_QOS_PANIC_CTRL);
>> 
>>         DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " 
>> DRM_RECT_FMT
>> @@ -1181,8 +1182,15 @@ static void dpu_plane_sspp_atomic_update(struct 
>> drm_plane *plane)
>>                 _dpu_plane_set_ot_limit(plane, crtc);
>>         }
>> 
>> -       _dpu_plane_set_qos_remap(plane);
>> +       if (is_rt_pipe != pdpu->is_rt_pipe) {
>> +               pdpu->is_rt_pipe = is_rt_pipe;
>> +               pstate->dirty |= DPU_PLANE_DIRTY_QOS;
>> +       }
>> 
>> +       if (pstate->dirty & DPU_PLANE_DIRTY_QOS) {
>> +               _dpu_plane_set_qos_remap(plane);
>> +               pstate->dirty = 0x0;
>> +       }
> 
> So in the end, this looks roughly like "set qos remap on modesets or
> switching between right/left pipe"?  Couldn't this be simpler if in
> plane->atomic_check() you do something like:
> 
>    dpu_plane_state->needs_qos_remap = atomic_state->allow_modeset;
> 
> and then in plane->atomic_update:
> 
>     if (pstate->needs_qos_remap || (is_rt_pipe != pdpu->is_rt_pipe) {
>          pdpu->is_rt_pipe = is_rt_pipe;
>          _dpu_plane_set_qos_remap(plane)
>     }
> 
> ?
> 
> BR,
> -R
Thanks for the suggestion, Yes this will make it much simpler. Let me 
update it.

Just one clarification, I believe you meant that
dpu_plane_state->needs_qos_remap = 
drm_atomic_crtc_needs_modeset(crtc_state)

and then the rest of it looks fine to me.

> 
> 
>>         _dpu_plane_calc_bw(plane, fb);
>> 
>>         _dpu_plane_calc_clk(plane);
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> index ca83b8753d59..47abd3686a86 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> @@ -14,11 +14,15 @@
>>  #include "dpu_hw_mdss.h"
>>  #include "dpu_hw_sspp.h"
>> 
>> +/* dirty bits to update QOS */
>> +#define DPU_PLANE_DIRTY_QOS 0x1
>> +
>>  /**
>>   * struct dpu_plane_state: Define dpu extension of drm plane state 
>> object
>>   * @base:      base drm plane state object
>>   * @aspace:    pointer to address space for input/output buffers
>>   * @stage:     assigned by crtc blender
>> + * @dirty:     bitmask for which pipe h/w config functions need to be 
>> updated
>>   * @multirect_index: index of the rectangle of SSPP
>>   * @multirect_mode: parallel or time multiplex multirect mode
>>   * @pending:   whether the current update is still pending
>> @@ -32,6 +36,7 @@ struct dpu_plane_state {
>>         struct drm_plane_state base;
>>         struct msm_gem_address_space *aspace;
>>         enum dpu_stage stage;
>> +       uint32_t dirty;
>>         uint32_t multirect_index;
>>         uint32_t multirect_mode;
>>         bool pending;
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>> Forum,
>> a Linux Foundation Collaborative Project
>> 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] drm/msm/dpu: update the qos remap only if the client type changes
@ 2020-11-24  0:38     ` abhinavk
  0 siblings, 0 replies; 8+ messages in thread
From: abhinavk @ 2020-11-24  0:38 UTC (permalink / raw)
  To: Rob Clark
  Cc: linux-arm-msm, dri-devel, Stephen Boyd, Kuogee Hsieh, Sean Paul,
	Tanmay Shah, aravindh, freedreno

Hi Rob

On 2020-11-23 15:18, Rob Clark wrote:
> On Thu, Nov 19, 2020 at 1:41 PM Abhinav Kumar <abhinavk@codeaurora.org> 
> wrote:
>> 
>> Update the qos remap only if the client type changes for the plane.
>> This will avoid unnecessary register programming and also avoid log
>> spam from the dpu_vbif_set_qos_remap() function.
>> 
>> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
>> ---
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 17 +++++++++++++++++
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h    |  7 +++++++
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  2 ++
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 12 ++++++++++--
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   |  5 +++++
>>  5 files changed, 41 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> index d4662e8184cc..3867da47c683 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> @@ -1037,6 +1037,23 @@ static int dpu_crtc_atomic_check(struct 
>> drm_crtc *crtc,
>>         return rc;
>>  }
>> 
>> +void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc)
>> +{
>> +       struct drm_plane *plane;
>> +       struct drm_plane_state *state;
>> +       struct dpu_plane_state *pstate;
>> +
>> +       drm_atomic_crtc_for_each_plane(plane, crtc) {
>> +               state = plane->state;
>> +               if (!state)
>> +                       continue;
>> +
>> +               pstate = to_dpu_plane_state(state);
>> +
>> +               pstate->dirty |= DPU_PLANE_DIRTY_QOS;
>> +       }
>> +}
>> +
>>  int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
>>  {
>>         struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
>> index cec3474340e8..8ba11de605bc 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
>> @@ -231,6 +231,13 @@ static inline int dpu_crtc_frame_pending(struct 
>> drm_crtc *crtc)
>>   */
>>  int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);
>> 
>> +/**
>> + * dpu_crtc_set_qos_dirty - update plane dirty flag to include
>> + * QoS reprogramming
>> + * @crtc: Pointer to drm crtc structure
>> + */
>> +void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc);
>> +
>>  /**
>>   * dpu_crtc_vblank_callback - called on vblank irq, issues completion 
>> events
>>   * @crtc: Pointer to drm crtc object
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> index f7f5c258b553..c2db9dd6ec67 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> @@ -1001,6 +1001,8 @@ static void dpu_encoder_virt_mode_set(struct 
>> drm_encoder *drm_enc,
>> 
>>         trace_dpu_enc_mode_set(DRMID(drm_enc));
>> 
>> +       dpu_crtc_set_qos_dirty(drm_enc->crtc);
>> +
>>         if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS && 
>> priv->dp)
>>                 msm_dp_display_mode_set(priv->dp, drm_enc, mode, 
>> adj_mode);
>> 
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> index 7ea90d25a3b6..f91d31a31e14 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> @@ -1066,6 +1066,7 @@ static void dpu_plane_sspp_atomic_update(struct 
>> drm_plane *plane)
>>         struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>>         struct drm_crtc *crtc = state->crtc;
>>         struct drm_framebuffer *fb = state->fb;
>> +       bool is_rt_pipe;
>>         const struct dpu_format *fmt =
>>                 to_dpu_format(msm_framebuffer_format(fb));
>> 
>> @@ -1075,7 +1076,7 @@ static void dpu_plane_sspp_atomic_update(struct 
>> drm_plane *plane)
>> 
>>         pstate->pending = true;
>> 
>> -       pdpu->is_rt_pipe = (dpu_crtc_get_client_type(crtc) != 
>> NRT_CLIENT);
>> +       is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
>>         _dpu_plane_set_qos_ctrl(plane, false, 
>> DPU_PLANE_QOS_PANIC_CTRL);
>> 
>>         DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " 
>> DRM_RECT_FMT
>> @@ -1181,8 +1182,15 @@ static void dpu_plane_sspp_atomic_update(struct 
>> drm_plane *plane)
>>                 _dpu_plane_set_ot_limit(plane, crtc);
>>         }
>> 
>> -       _dpu_plane_set_qos_remap(plane);
>> +       if (is_rt_pipe != pdpu->is_rt_pipe) {
>> +               pdpu->is_rt_pipe = is_rt_pipe;
>> +               pstate->dirty |= DPU_PLANE_DIRTY_QOS;
>> +       }
>> 
>> +       if (pstate->dirty & DPU_PLANE_DIRTY_QOS) {
>> +               _dpu_plane_set_qos_remap(plane);
>> +               pstate->dirty = 0x0;
>> +       }
> 
> So in the end, this looks roughly like "set qos remap on modesets or
> switching between right/left pipe"?  Couldn't this be simpler if in
> plane->atomic_check() you do something like:
> 
>    dpu_plane_state->needs_qos_remap = atomic_state->allow_modeset;
> 
> and then in plane->atomic_update:
> 
>     if (pstate->needs_qos_remap || (is_rt_pipe != pdpu->is_rt_pipe) {
>          pdpu->is_rt_pipe = is_rt_pipe;
>          _dpu_plane_set_qos_remap(plane)
>     }
> 
> ?
> 
> BR,
> -R
Thanks for the suggestion, Yes this will make it much simpler. Let me 
update it.

Just one clarification, I believe you meant that
dpu_plane_state->needs_qos_remap = 
drm_atomic_crtc_needs_modeset(crtc_state)

and then the rest of it looks fine to me.

> 
> 
>>         _dpu_plane_calc_bw(plane, fb);
>> 
>>         _dpu_plane_calc_clk(plane);
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> index ca83b8753d59..47abd3686a86 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> @@ -14,11 +14,15 @@
>>  #include "dpu_hw_mdss.h"
>>  #include "dpu_hw_sspp.h"
>> 
>> +/* dirty bits to update QOS */
>> +#define DPU_PLANE_DIRTY_QOS 0x1
>> +
>>  /**
>>   * struct dpu_plane_state: Define dpu extension of drm plane state 
>> object
>>   * @base:      base drm plane state object
>>   * @aspace:    pointer to address space for input/output buffers
>>   * @stage:     assigned by crtc blender
>> + * @dirty:     bitmask for which pipe h/w config functions need to be 
>> updated
>>   * @multirect_index: index of the rectangle of SSPP
>>   * @multirect_mode: parallel or time multiplex multirect mode
>>   * @pending:   whether the current update is still pending
>> @@ -32,6 +36,7 @@ struct dpu_plane_state {
>>         struct drm_plane_state base;
>>         struct msm_gem_address_space *aspace;
>>         enum dpu_stage stage;
>> +       uint32_t dirty;
>>         uint32_t multirect_index;
>>         uint32_t multirect_mode;
>>         bool pending;
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>> Forum,
>> a Linux Foundation Collaborative Project
>> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] drm/msm/dpu: update the qos remap only if the client type changes
  2020-11-24  0:38     ` abhinavk
@ 2020-11-24  1:02       ` Rob Clark
  -1 siblings, 0 replies; 8+ messages in thread
From: Rob Clark @ 2020-11-24  1:02 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: dri-devel, linux-arm-msm, freedreno, Sean Paul, Stephen Boyd,
	nganji, aravindh, Tanmay Shah, Kuogee Hsieh

On Mon, Nov 23, 2020 at 4:38 PM <abhinavk@codeaurora.org> wrote:
>
> Hi Rob
>
> On 2020-11-23 15:18, Rob Clark wrote:
> > On Thu, Nov 19, 2020 at 1:41 PM Abhinav Kumar <abhinavk@codeaurora.org>
> > wrote:
> >>
> >> Update the qos remap only if the client type changes for the plane.
> >> This will avoid unnecessary register programming and also avoid log
> >> spam from the dpu_vbif_set_qos_remap() function.
> >>
> >> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
> >> ---
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 17 +++++++++++++++++
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h    |  7 +++++++
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  2 ++
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 12 ++++++++++--
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   |  5 +++++
> >>  5 files changed, 41 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> >> index d4662e8184cc..3867da47c683 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> >> @@ -1037,6 +1037,23 @@ static int dpu_crtc_atomic_check(struct
> >> drm_crtc *crtc,
> >>         return rc;
> >>  }
> >>
> >> +void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc)
> >> +{
> >> +       struct drm_plane *plane;
> >> +       struct drm_plane_state *state;
> >> +       struct dpu_plane_state *pstate;
> >> +
> >> +       drm_atomic_crtc_for_each_plane(plane, crtc) {
> >> +               state = plane->state;
> >> +               if (!state)
> >> +                       continue;
> >> +
> >> +               pstate = to_dpu_plane_state(state);
> >> +
> >> +               pstate->dirty |= DPU_PLANE_DIRTY_QOS;
> >> +       }
> >> +}
> >> +
> >>  int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
> >>  {
> >>         struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> >> index cec3474340e8..8ba11de605bc 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> >> @@ -231,6 +231,13 @@ static inline int dpu_crtc_frame_pending(struct
> >> drm_crtc *crtc)
> >>   */
> >>  int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);
> >>
> >> +/**
> >> + * dpu_crtc_set_qos_dirty - update plane dirty flag to include
> >> + * QoS reprogramming
> >> + * @crtc: Pointer to drm crtc structure
> >> + */
> >> +void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc);
> >> +
> >>  /**
> >>   * dpu_crtc_vblank_callback - called on vblank irq, issues completion
> >> events
> >>   * @crtc: Pointer to drm crtc object
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> index f7f5c258b553..c2db9dd6ec67 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> @@ -1001,6 +1001,8 @@ static void dpu_encoder_virt_mode_set(struct
> >> drm_encoder *drm_enc,
> >>
> >>         trace_dpu_enc_mode_set(DRMID(drm_enc));
> >>
> >> +       dpu_crtc_set_qos_dirty(drm_enc->crtc);
> >> +
> >>         if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS &&
> >> priv->dp)
> >>                 msm_dp_display_mode_set(priv->dp, drm_enc, mode,
> >> adj_mode);
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> >> index 7ea90d25a3b6..f91d31a31e14 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> >> @@ -1066,6 +1066,7 @@ static void dpu_plane_sspp_atomic_update(struct
> >> drm_plane *plane)
> >>         struct dpu_plane_state *pstate = to_dpu_plane_state(state);
> >>         struct drm_crtc *crtc = state->crtc;
> >>         struct drm_framebuffer *fb = state->fb;
> >> +       bool is_rt_pipe;
> >>         const struct dpu_format *fmt =
> >>                 to_dpu_format(msm_framebuffer_format(fb));
> >>
> >> @@ -1075,7 +1076,7 @@ static void dpu_plane_sspp_atomic_update(struct
> >> drm_plane *plane)
> >>
> >>         pstate->pending = true;
> >>
> >> -       pdpu->is_rt_pipe = (dpu_crtc_get_client_type(crtc) !=
> >> NRT_CLIENT);
> >> +       is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
> >>         _dpu_plane_set_qos_ctrl(plane, false,
> >> DPU_PLANE_QOS_PANIC_CTRL);
> >>
> >>         DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u "
> >> DRM_RECT_FMT
> >> @@ -1181,8 +1182,15 @@ static void dpu_plane_sspp_atomic_update(struct
> >> drm_plane *plane)
> >>                 _dpu_plane_set_ot_limit(plane, crtc);
> >>         }
> >>
> >> -       _dpu_plane_set_qos_remap(plane);
> >> +       if (is_rt_pipe != pdpu->is_rt_pipe) {
> >> +               pdpu->is_rt_pipe = is_rt_pipe;
> >> +               pstate->dirty |= DPU_PLANE_DIRTY_QOS;
> >> +       }
> >>
> >> +       if (pstate->dirty & DPU_PLANE_DIRTY_QOS) {
> >> +               _dpu_plane_set_qos_remap(plane);
> >> +               pstate->dirty = 0x0;
> >> +       }
> >
> > So in the end, this looks roughly like "set qos remap on modesets or
> > switching between right/left pipe"?  Couldn't this be simpler if in
> > plane->atomic_check() you do something like:
> >
> >    dpu_plane_state->needs_qos_remap = atomic_state->allow_modeset;
> >
> > and then in plane->atomic_update:
> >
> >     if (pstate->needs_qos_remap || (is_rt_pipe != pdpu->is_rt_pipe) {
> >          pdpu->is_rt_pipe = is_rt_pipe;
> >          _dpu_plane_set_qos_remap(plane)
> >     }
> >
> > ?
> >
> > BR,
> > -R
> Thanks for the suggestion, Yes this will make it much simpler. Let me
> update it.
>
> Just one clarification, I believe you meant that
> dpu_plane_state->needs_qos_remap =
> drm_atomic_crtc_needs_modeset(crtc_state)

yeah, that looks better.. thx

BR,
-R

> and then the rest of it looks fine to me.
>
> >
> >
> >>         _dpu_plane_calc_bw(plane, fb);
> >>
> >>         _dpu_plane_calc_clk(plane);
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> >> index ca83b8753d59..47abd3686a86 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> >> @@ -14,11 +14,15 @@
> >>  #include "dpu_hw_mdss.h"
> >>  #include "dpu_hw_sspp.h"
> >>
> >> +/* dirty bits to update QOS */
> >> +#define DPU_PLANE_DIRTY_QOS 0x1
> >> +
> >>  /**
> >>   * struct dpu_plane_state: Define dpu extension of drm plane state
> >> object
> >>   * @base:      base drm plane state object
> >>   * @aspace:    pointer to address space for input/output buffers
> >>   * @stage:     assigned by crtc blender
> >> + * @dirty:     bitmask for which pipe h/w config functions need to be
> >> updated
> >>   * @multirect_index: index of the rectangle of SSPP
> >>   * @multirect_mode: parallel or time multiplex multirect mode
> >>   * @pending:   whether the current update is still pending
> >> @@ -32,6 +36,7 @@ struct dpu_plane_state {
> >>         struct drm_plane_state base;
> >>         struct msm_gem_address_space *aspace;
> >>         enum dpu_stage stage;
> >> +       uint32_t dirty;
> >>         uint32_t multirect_index;
> >>         uint32_t multirect_mode;
> >>         bool pending;
> >> --
> >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> >> Forum,
> >> a Linux Foundation Collaborative Project
> >>

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] drm/msm/dpu: update the qos remap only if the client type changes
@ 2020-11-24  1:02       ` Rob Clark
  0 siblings, 0 replies; 8+ messages in thread
From: Rob Clark @ 2020-11-24  1:02 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: linux-arm-msm, dri-devel, Stephen Boyd, Kuogee Hsieh, Sean Paul,
	Tanmay Shah, aravindh, freedreno

On Mon, Nov 23, 2020 at 4:38 PM <abhinavk@codeaurora.org> wrote:
>
> Hi Rob
>
> On 2020-11-23 15:18, Rob Clark wrote:
> > On Thu, Nov 19, 2020 at 1:41 PM Abhinav Kumar <abhinavk@codeaurora.org>
> > wrote:
> >>
> >> Update the qos remap only if the client type changes for the plane.
> >> This will avoid unnecessary register programming and also avoid log
> >> spam from the dpu_vbif_set_qos_remap() function.
> >>
> >> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
> >> ---
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 17 +++++++++++++++++
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h    |  7 +++++++
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  2 ++
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 12 ++++++++++--
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   |  5 +++++
> >>  5 files changed, 41 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> >> index d4662e8184cc..3867da47c683 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> >> @@ -1037,6 +1037,23 @@ static int dpu_crtc_atomic_check(struct
> >> drm_crtc *crtc,
> >>         return rc;
> >>  }
> >>
> >> +void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc)
> >> +{
> >> +       struct drm_plane *plane;
> >> +       struct drm_plane_state *state;
> >> +       struct dpu_plane_state *pstate;
> >> +
> >> +       drm_atomic_crtc_for_each_plane(plane, crtc) {
> >> +               state = plane->state;
> >> +               if (!state)
> >> +                       continue;
> >> +
> >> +               pstate = to_dpu_plane_state(state);
> >> +
> >> +               pstate->dirty |= DPU_PLANE_DIRTY_QOS;
> >> +       }
> >> +}
> >> +
> >>  int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
> >>  {
> >>         struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> >> index cec3474340e8..8ba11de605bc 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> >> @@ -231,6 +231,13 @@ static inline int dpu_crtc_frame_pending(struct
> >> drm_crtc *crtc)
> >>   */
> >>  int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);
> >>
> >> +/**
> >> + * dpu_crtc_set_qos_dirty - update plane dirty flag to include
> >> + * QoS reprogramming
> >> + * @crtc: Pointer to drm crtc structure
> >> + */
> >> +void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc);
> >> +
> >>  /**
> >>   * dpu_crtc_vblank_callback - called on vblank irq, issues completion
> >> events
> >>   * @crtc: Pointer to drm crtc object
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> index f7f5c258b553..c2db9dd6ec67 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> @@ -1001,6 +1001,8 @@ static void dpu_encoder_virt_mode_set(struct
> >> drm_encoder *drm_enc,
> >>
> >>         trace_dpu_enc_mode_set(DRMID(drm_enc));
> >>
> >> +       dpu_crtc_set_qos_dirty(drm_enc->crtc);
> >> +
> >>         if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS &&
> >> priv->dp)
> >>                 msm_dp_display_mode_set(priv->dp, drm_enc, mode,
> >> adj_mode);
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> >> index 7ea90d25a3b6..f91d31a31e14 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> >> @@ -1066,6 +1066,7 @@ static void dpu_plane_sspp_atomic_update(struct
> >> drm_plane *plane)
> >>         struct dpu_plane_state *pstate = to_dpu_plane_state(state);
> >>         struct drm_crtc *crtc = state->crtc;
> >>         struct drm_framebuffer *fb = state->fb;
> >> +       bool is_rt_pipe;
> >>         const struct dpu_format *fmt =
> >>                 to_dpu_format(msm_framebuffer_format(fb));
> >>
> >> @@ -1075,7 +1076,7 @@ static void dpu_plane_sspp_atomic_update(struct
> >> drm_plane *plane)
> >>
> >>         pstate->pending = true;
> >>
> >> -       pdpu->is_rt_pipe = (dpu_crtc_get_client_type(crtc) !=
> >> NRT_CLIENT);
> >> +       is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
> >>         _dpu_plane_set_qos_ctrl(plane, false,
> >> DPU_PLANE_QOS_PANIC_CTRL);
> >>
> >>         DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u "
> >> DRM_RECT_FMT
> >> @@ -1181,8 +1182,15 @@ static void dpu_plane_sspp_atomic_update(struct
> >> drm_plane *plane)
> >>                 _dpu_plane_set_ot_limit(plane, crtc);
> >>         }
> >>
> >> -       _dpu_plane_set_qos_remap(plane);
> >> +       if (is_rt_pipe != pdpu->is_rt_pipe) {
> >> +               pdpu->is_rt_pipe = is_rt_pipe;
> >> +               pstate->dirty |= DPU_PLANE_DIRTY_QOS;
> >> +       }
> >>
> >> +       if (pstate->dirty & DPU_PLANE_DIRTY_QOS) {
> >> +               _dpu_plane_set_qos_remap(plane);
> >> +               pstate->dirty = 0x0;
> >> +       }
> >
> > So in the end, this looks roughly like "set qos remap on modesets or
> > switching between right/left pipe"?  Couldn't this be simpler if in
> > plane->atomic_check() you do something like:
> >
> >    dpu_plane_state->needs_qos_remap = atomic_state->allow_modeset;
> >
> > and then in plane->atomic_update:
> >
> >     if (pstate->needs_qos_remap || (is_rt_pipe != pdpu->is_rt_pipe) {
> >          pdpu->is_rt_pipe = is_rt_pipe;
> >          _dpu_plane_set_qos_remap(plane)
> >     }
> >
> > ?
> >
> > BR,
> > -R
> Thanks for the suggestion, Yes this will make it much simpler. Let me
> update it.
>
> Just one clarification, I believe you meant that
> dpu_plane_state->needs_qos_remap =
> drm_atomic_crtc_needs_modeset(crtc_state)

yeah, that looks better.. thx

BR,
-R

> and then the rest of it looks fine to me.
>
> >
> >
> >>         _dpu_plane_calc_bw(plane, fb);
> >>
> >>         _dpu_plane_calc_clk(plane);
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> >> index ca83b8753d59..47abd3686a86 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> >> @@ -14,11 +14,15 @@
> >>  #include "dpu_hw_mdss.h"
> >>  #include "dpu_hw_sspp.h"
> >>
> >> +/* dirty bits to update QOS */
> >> +#define DPU_PLANE_DIRTY_QOS 0x1
> >> +
> >>  /**
> >>   * struct dpu_plane_state: Define dpu extension of drm plane state
> >> object
> >>   * @base:      base drm plane state object
> >>   * @aspace:    pointer to address space for input/output buffers
> >>   * @stage:     assigned by crtc blender
> >> + * @dirty:     bitmask for which pipe h/w config functions need to be
> >> updated
> >>   * @multirect_index: index of the rectangle of SSPP
> >>   * @multirect_mode: parallel or time multiplex multirect mode
> >>   * @pending:   whether the current update is still pending
> >> @@ -32,6 +36,7 @@ struct dpu_plane_state {
> >>         struct drm_plane_state base;
> >>         struct msm_gem_address_space *aspace;
> >>         enum dpu_stage stage;
> >> +       uint32_t dirty;
> >>         uint32_t multirect_index;
> >>         uint32_t multirect_mode;
> >>         bool pending;
> >> --
> >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> >> Forum,
> >> a Linux Foundation Collaborative Project
> >>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-11-24  1:01 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-19 21:41 [PATCH] drm/msm/dpu: update the qos remap only if the client type changes Abhinav Kumar
2020-11-19 21:41 ` Abhinav Kumar
2020-11-23 23:18 ` Rob Clark
2020-11-23 23:18   ` Rob Clark
2020-11-24  0:38   ` abhinavk
2020-11-24  0:38     ` abhinavk
2020-11-24  1:02     ` Rob Clark
2020-11-24  1:02       ` Rob Clark

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.