[PATCH v2 0/7] move dpu resource parsing to encoder modeset

[PATCH v2 0/7] move dpu resource parsing to encoder modeset

[Jeykumar Sankaran]

Fixing some of the low hanging fruits by moving the hw resource
parsing and assignment to encoder modeset. This series 
prepares DPU resource management to switch to state based
resource tracking which is implemented in the next incoming
changes.

Thanks.

Jeykumar Sankaran (7):
  drm/msm/dpu: move hw_inf encoder baseclass
  drm/msm/dpu: remove phys_vid subclass
  drm/msm/dpu: release resources on modeset failure
  drm/msm/dpu: dont use encoder->crtc in atomic path
  drm/msm/dpu: map mixer/ctl hw blocks in encoder modeset
  drm/msm/dpu: assign intf to encoder in mode_set
  drm/msm/dpu: check split role for single flush

 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c           |  64 +-------
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        |  73 +++++++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h   |  15 +-
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   | 177 ++++++---------------
 4 files changed, 118 insertions(+), 211 deletions(-)

-- 
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

[PATCH v2 3/7] drm/msm/dpu: release resources on modeset failure

[Jeykumar Sankaran]

release resources allocated in mode_set if any of
the hw check fails. Most of these checks are not
necessary and they will be removed in the follow up
patches with state based resource allocations.

Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 941ac25..45617b9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1025,13 +1025,13 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 			if (!dpu_enc->hw_pp[i]) {
 				DPU_ERROR_ENC(dpu_enc, "no pp block assigned"
 					     "at idx: %d\n", i);
-				return;
+				goto error;
 			}
 
 			if (!hw_ctl[i]) {
 				DPU_ERROR_ENC(dpu_enc, "no ctl block assigned"
 					     "at idx: %d\n", i);
-				return;
+				goto error;
 			}
 
 			phys->hw_pp = dpu_enc->hw_pp[i];
@@ -1044,6 +1044,9 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 	}
 
 	dpu_enc->mode_set_complete = true;
+
+error:
+	dpu_rm_release(&dpu_kms->rm, drm_enc);
 }
 
 static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc)
-- 
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

Re: [PATCH v2 3/7] drm/msm/dpu: release resources on modeset failure

[Sean Paul]

On Wed, Feb 13, 2019 at 05:19:12PM -0800, Jeykumar Sankaran wrote:
> release resources allocated in mode_set if any of
> the hw check fails. Most of these checks are not
> necessary and they will be removed in the follow up
> patches with state based resource allocations.
> 
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>

Reviewed-by: Sean Paul <sean@poorly.run>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 941ac25..45617b9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1025,13 +1025,13 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>  			if (!dpu_enc->hw_pp[i]) {
>  				DPU_ERROR_ENC(dpu_enc, "no pp block assigned"
>  					     "at idx: %d\n", i);
> -				return;
> +				goto error;
>  			}
>  
>  			if (!hw_ctl[i]) {
>  				DPU_ERROR_ENC(dpu_enc, "no ctl block assigned"
>  					     "at idx: %d\n", i);
> -				return;
> +				goto error;
>  			}
>  
>  			phys->hw_pp = dpu_enc->hw_pp[i];
> @@ -1044,6 +1044,9 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>  	}
>  
>  	dpu_enc->mode_set_complete = true;
> +
> +error:
> +	dpu_rm_release(&dpu_kms->rm, drm_enc);
>  }
>  
>  static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc)
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 1/7] drm/msm/dpu: move hw_inf encoder baseclass

[Jeykumar Sankaran]

Both video and command physical encoders will have
a hw interface assigned to it. So there is really no
need to track the hw block in specific encoder subclass.

Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h   |   4 +-
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   | 125 +++++++++------------
 2 files changed, 52 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index 44e6f8b6..acd5956 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -201,6 +201,7 @@ struct dpu_encoder_irq {
  * @hw_mdptop:		Hardware interface to the top registers
  * @hw_ctl:		Hardware interface to the ctl registers
  * @hw_pp:		Hardware interface to the ping pong registers
+ * @hw_intf:		Hardware interface to the intf registers
  * @dpu_kms:		Pointer to the dpu_kms top level
  * @cached_mode:	DRM mode cached at mode_set time, acted on in enable
  * @enabled:		Whether the encoder has enabled and running a mode
@@ -229,6 +230,7 @@ struct dpu_encoder_phys {
 	struct dpu_hw_mdp *hw_mdptop;
 	struct dpu_hw_ctl *hw_ctl;
 	struct dpu_hw_pingpong *hw_pp;
+	struct dpu_hw_intf *hw_intf;
 	struct dpu_kms *dpu_kms;
 	struct drm_display_mode cached_mode;
 	enum dpu_enc_split_role split_role;
@@ -255,12 +257,10 @@ static inline int dpu_encoder_phys_inc_pending(struct dpu_encoder_phys *phys)
  * struct dpu_encoder_phys_vid - sub-class of dpu_encoder_phys to handle video
  *	mode specific operations
  * @base:	Baseclass physical encoder structure
- * @hw_intf:	Hardware interface to the intf registers
  * @timing_params: Current timing parameter
  */
 struct dpu_encoder_phys_vid {
 	struct dpu_encoder_phys base;
-	struct dpu_hw_intf *hw_intf;
 	struct intf_timing_params timing_params;
 };
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index acdab5b0..e326395 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -18,14 +18,14 @@
 #include "dpu_trace.h"
 
 #define DPU_DEBUG_VIDENC(e, fmt, ...) DPU_DEBUG("enc%d intf%d " fmt, \
-		(e) && (e)->base.parent ? \
-		(e)->base.parent->base.id : -1, \
+		(e) && (e)->parent ? \
+		(e)->parent->base.id : -1, \
 		(e) && (e)->hw_intf ? \
 		(e)->hw_intf->idx - INTF_0 : -1, ##__VA_ARGS__)
 
 #define DPU_ERROR_VIDENC(e, fmt, ...) DPU_ERROR("enc%d intf%d " fmt, \
-		(e) && (e)->base.parent ? \
-		(e)->base.parent->base.id : -1, \
+		(e) && (e)->parent ? \
+		(e)->parent->base.id : -1, \
 		(e) && (e)->hw_intf ? \
 		(e)->hw_intf->idx - INTF_0 : -1, ##__VA_ARGS__)
 
@@ -44,7 +44,7 @@ static bool dpu_encoder_phys_vid_is_master(
 }
 
 static void drm_mode_to_intf_timing_params(
-		const struct dpu_encoder_phys_vid *vid_enc,
+		const struct dpu_encoder_phys *phys_enc,
 		const struct drm_display_mode *mode,
 		struct intf_timing_params *timing)
 {
@@ -92,7 +92,7 @@ static void drm_mode_to_intf_timing_params(
 	timing->hsync_skew = mode->hskew;
 
 	/* DSI controller cannot handle active-low sync signals. */
-	if (vid_enc->hw_intf->cap->type == INTF_DSI) {
+	if (phys_enc->hw_intf->cap->type == INTF_DSI) {
 		timing->hsync_polarity = 0;
 		timing->vsync_polarity = 0;
 	}
@@ -143,11 +143,11 @@ static u32 get_vertical_total(const struct intf_timing_params *timing)
  * lines based on the chip worst case latencies.
  */
 static u32 programmable_fetch_get_num_lines(
-		struct dpu_encoder_phys_vid *vid_enc,
+		struct dpu_encoder_phys *phys_enc,
 		const struct intf_timing_params *timing)
 {
 	u32 worst_case_needed_lines =
-	    vid_enc->hw_intf->cap->prog_fetch_lines_worst_case;
+	    phys_enc->hw_intf->cap->prog_fetch_lines_worst_case;
 	u32 start_of_frame_lines =
 	    timing->v_back_porch + timing->vsync_pulse_width;
 	u32 needed_vfp_lines = worst_case_needed_lines - start_of_frame_lines;
@@ -155,26 +155,26 @@ static u32 programmable_fetch_get_num_lines(
 
 	/* Fetch must be outside active lines, otherwise undefined. */
 	if (start_of_frame_lines >= worst_case_needed_lines) {
-		DPU_DEBUG_VIDENC(vid_enc,
+		DPU_DEBUG_VIDENC(phys_enc,
 				"prog fetch is not needed, large vbp+vsw\n");
 		actual_vfp_lines = 0;
 	} else if (timing->v_front_porch < needed_vfp_lines) {
 		/* Warn fetch needed, but not enough porch in panel config */
 		pr_warn_once
 			("low vbp+vfp may lead to perf issues in some cases\n");
-		DPU_DEBUG_VIDENC(vid_enc,
+		DPU_DEBUG_VIDENC(phys_enc,
 				"less vfp than fetch req, using entire vfp\n");
 		actual_vfp_lines = timing->v_front_porch;
 	} else {
-		DPU_DEBUG_VIDENC(vid_enc, "room in vfp for needed prefetch\n");
+		DPU_DEBUG_VIDENC(phys_enc, "room in vfp for needed prefetch\n");
 		actual_vfp_lines = needed_vfp_lines;
 	}
 
-	DPU_DEBUG_VIDENC(vid_enc,
+	DPU_DEBUG_VIDENC(phys_enc,
 		"v_front_porch %u v_back_porch %u vsync_pulse_width %u\n",
 		timing->v_front_porch, timing->v_back_porch,
 		timing->vsync_pulse_width);
-	DPU_DEBUG_VIDENC(vid_enc,
+	DPU_DEBUG_VIDENC(phys_enc,
 		"wc_lines %u needed_vfp_lines %u actual_vfp_lines %u\n",
 		worst_case_needed_lines, needed_vfp_lines, actual_vfp_lines);
 
@@ -194,8 +194,6 @@ static u32 programmable_fetch_get_num_lines(
 static void programmable_fetch_config(struct dpu_encoder_phys *phys_enc,
 				      const struct intf_timing_params *timing)
 {
-	struct dpu_encoder_phys_vid *vid_enc =
-		to_dpu_encoder_phys_vid(phys_enc);
 	struct intf_prog_fetch f = { 0 };
 	u32 vfp_fetch_lines = 0;
 	u32 horiz_total = 0;
@@ -203,10 +201,10 @@ static void programmable_fetch_config(struct dpu_encoder_phys *phys_enc,
 	u32 vfp_fetch_start_vsync_counter = 0;
 	unsigned long lock_flags;
 
-	if (WARN_ON_ONCE(!vid_enc->hw_intf->ops.setup_prg_fetch))
+	if (WARN_ON_ONCE(!phys_enc->hw_intf->ops.setup_prg_fetch))
 		return;
 
-	vfp_fetch_lines = programmable_fetch_get_num_lines(vid_enc, timing);
+	vfp_fetch_lines = programmable_fetch_get_num_lines(phys_enc, timing);
 	if (vfp_fetch_lines) {
 		vert_total = get_vertical_total(timing);
 		horiz_total = get_horizontal_total(timing);
@@ -216,12 +214,12 @@ static void programmable_fetch_config(struct dpu_encoder_phys *phys_enc,
 		f.fetch_start = vfp_fetch_start_vsync_counter;
 	}
 
-	DPU_DEBUG_VIDENC(vid_enc,
+	DPU_DEBUG_VIDENC(phys_enc,
 		"vfp_fetch_lines %u vfp_fetch_start_vsync_counter %u\n",
 		vfp_fetch_lines, vfp_fetch_start_vsync_counter);
 
 	spin_lock_irqsave(phys_enc->enc_spinlock, lock_flags);
-	vid_enc->hw_intf->ops.setup_prg_fetch(vid_enc->hw_intf, &f);
+	phys_enc->hw_intf->ops.setup_prg_fetch(phys_enc->hw_intf, &f);
 	spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);
 }
 
@@ -231,7 +229,7 @@ static bool dpu_encoder_phys_vid_mode_fixup(
 		struct drm_display_mode *adj_mode)
 {
 	if (phys_enc)
-		DPU_DEBUG_VIDENC(to_dpu_encoder_phys_vid(phys_enc), "\n");
+		DPU_DEBUG_VIDENC(phys_enc, "\n");
 
 	/*
 	 * Modifying mode has consequences when the mode comes back to us
@@ -257,12 +255,12 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
 
 	mode = phys_enc->cached_mode;
 	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
-	if (!vid_enc->hw_intf->ops.setup_timing_gen) {
+	if (!phys_enc->hw_intf->ops.setup_timing_gen) {
 		DPU_ERROR("timing engine setup is not supported\n");
 		return;
 	}
 
-	DPU_DEBUG_VIDENC(vid_enc, "enabling mode:\n");
+	DPU_DEBUG_VIDENC(phys_enc, "enabling mode:\n");
 	drm_mode_debug_printmodeline(&mode);
 
 	if (phys_enc->split_role != ENC_ROLE_SOLO) {
@@ -271,25 +269,25 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
 		mode.hsync_start >>= 1;
 		mode.hsync_end >>= 1;
 
-		DPU_DEBUG_VIDENC(vid_enc,
+		DPU_DEBUG_VIDENC(phys_enc,
 			"split_role %d, halve horizontal %d %d %d %d\n",
 			phys_enc->split_role,
 			mode.hdisplay, mode.htotal,
 			mode.hsync_start, mode.hsync_end);
 	}
 
-	drm_mode_to_intf_timing_params(vid_enc, &mode, &timing_params);
+	drm_mode_to_intf_timing_params(phys_enc, &mode, &timing_params);
 
 	fmt = dpu_get_dpu_format(fmt_fourcc);
-	DPU_DEBUG_VIDENC(vid_enc, "fmt_fourcc 0x%X\n", fmt_fourcc);
+	DPU_DEBUG_VIDENC(phys_enc, "fmt_fourcc 0x%X\n", fmt_fourcc);
 
-	intf_cfg.intf = vid_enc->hw_intf->idx;
+	intf_cfg.intf = phys_enc->hw_intf->idx;
 	intf_cfg.intf_mode_sel = DPU_CTL_MODE_SEL_VID;
 	intf_cfg.stream_sel = 0; /* Don't care value for video mode */
 	intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
 
 	spin_lock_irqsave(phys_enc->enc_spinlock, lock_flags);
-	vid_enc->hw_intf->ops.setup_timing_gen(vid_enc->hw_intf,
+	phys_enc->hw_intf->ops.setup_timing_gen(phys_enc->hw_intf,
 			&timing_params, fmt);
 	phys_enc->hw_ctl->ops.setup_intf_cfg(phys_enc->hw_ctl, &intf_cfg);
 	spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);
@@ -396,19 +394,15 @@ static void dpu_encoder_phys_vid_mode_set(
 		struct drm_display_mode *mode,
 		struct drm_display_mode *adj_mode)
 {
-	struct dpu_encoder_phys_vid *vid_enc;
-
 	if (!phys_enc || !phys_enc->dpu_kms) {
 		DPU_ERROR("invalid encoder/kms\n");
 		return;
 	}
 
-	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
-
 	if (adj_mode) {
 		phys_enc->cached_mode = *adj_mode;
 		drm_mode_debug_printmodeline(adj_mode);
-		DPU_DEBUG_VIDENC(vid_enc, "caching mode:\n");
+		DPU_DEBUG_VIDENC(phys_enc, "caching mode:\n");
 	}
 
 	_dpu_encoder_phys_vid_setup_irq_hw_idx(phys_enc);
@@ -419,7 +413,6 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
 		bool enable)
 {
 	int ret = 0;
-	struct dpu_encoder_phys_vid *vid_enc;
 	int refcount;
 
 	if (!phys_enc) {
@@ -428,7 +421,6 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
 	}
 
 	refcount = atomic_read(&phys_enc->vblank_refcount);
-	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
 
 	/* Slave encoders don't report vblank */
 	if (!dpu_encoder_phys_vid_is_master(phys_enc))
@@ -453,7 +445,7 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
 	if (ret) {
 		DRM_ERROR("failed: id:%u intf:%d ret:%d enable:%d refcnt:%d\n",
 			  DRMID(phys_enc->parent),
-			  vid_enc->hw_intf->idx - INTF_0, ret, enable,
+			  phys_enc->hw_intf->idx - INTF_0, ret, enable,
 			  refcount);
 	}
 	return ret;
@@ -462,7 +454,6 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
 static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
 {
 	struct msm_drm_private *priv;
-	struct dpu_encoder_phys_vid *vid_enc;
 	struct dpu_rm_hw_iter iter;
 	struct dpu_hw_ctl *ctl;
 	u32 flush_mask = 0;
@@ -474,7 +465,6 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
 	}
 	priv = phys_enc->parent->dev->dev_private;
 
-	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
 	ctl = phys_enc->hw_ctl;
 
 	dpu_rm_init_hw_iter(&iter, phys_enc->parent->base.id, DPU_HW_BLK_INTF);
@@ -482,22 +472,22 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
 		struct dpu_hw_intf *hw_intf = (struct dpu_hw_intf *)iter.hw;
 
 		if (hw_intf->idx == phys_enc->intf_idx) {
-			vid_enc->hw_intf = hw_intf;
+			phys_enc->hw_intf = hw_intf;
 			break;
 		}
 	}
 
-	if (!vid_enc->hw_intf) {
+	if (!phys_enc->hw_intf) {
 		DPU_ERROR("hw_intf not assigned\n");
 		return;
 	}
 
-	DPU_DEBUG_VIDENC(vid_enc, "\n");
+	DPU_DEBUG_VIDENC(phys_enc, "\n");
 
-	if (WARN_ON(!vid_enc->hw_intf->ops.enable_timing))
+	if (WARN_ON(!phys_enc->hw_intf->ops.enable_timing))
 		return;
 
-	dpu_encoder_helper_split_config(phys_enc, vid_enc->hw_intf->idx);
+	dpu_encoder_helper_split_config(phys_enc, phys_enc->hw_intf->idx);
 
 	dpu_encoder_phys_vid_setup_timing_engine(phys_enc);
 
@@ -510,12 +500,13 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
 		!dpu_encoder_phys_vid_is_master(phys_enc))
 		goto skip_flush;
 
-	ctl->ops.get_bitmask_intf(ctl, &flush_mask, vid_enc->hw_intf->idx);
+	ctl->ops.get_bitmask_intf(ctl, &flush_mask, phys_enc->hw_intf->idx);
 	ctl->ops.update_pending_flush(ctl, flush_mask);
 
 skip_flush:
-	DPU_DEBUG_VIDENC(vid_enc, "update pending flush ctl %d flush_mask %x\n",
-		ctl->idx - CTL_0, flush_mask);
+	DPU_DEBUG_VIDENC(phys_enc,
+			 "update pending flush ctl %d flush_mask %x\n",
+			 ctl->idx - CTL_0, flush_mask);
 
 	/* ctl_flush & timing engine enable will be triggered by framework */
 	if (phys_enc->enable_state == DPU_ENC_DISABLED)
@@ -532,7 +523,7 @@ static void dpu_encoder_phys_vid_destroy(struct dpu_encoder_phys *phys_enc)
 	}
 
 	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
-	DPU_DEBUG_VIDENC(vid_enc, "\n");
+	DPU_DEBUG_VIDENC(phys_enc, "\n");
 	kfree(vid_enc);
 }
 
@@ -590,7 +581,6 @@ static void dpu_encoder_phys_vid_prepare_for_kickoff(
 		struct dpu_encoder_phys *phys_enc,
 		struct dpu_encoder_kickoff_params *params)
 {
-	struct dpu_encoder_phys_vid *vid_enc;
 	struct dpu_hw_ctl *ctl;
 	int rc;
 
@@ -598,7 +588,6 @@ static void dpu_encoder_phys_vid_prepare_for_kickoff(
 		DPU_ERROR("invalid encoder/parameters\n");
 		return;
 	}
-	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
 
 	ctl = phys_enc->hw_ctl;
 	if (!ctl || !ctl->ops.wait_reset_status)
@@ -610,7 +599,7 @@ static void dpu_encoder_phys_vid_prepare_for_kickoff(
 	 */
 	rc = ctl->ops.wait_reset_status(ctl);
 	if (rc) {
-		DPU_ERROR_VIDENC(vid_enc, "ctl %d reset failure: %d\n",
+		DPU_ERROR_VIDENC(phys_enc, "ctl %d reset failure: %d\n",
 				ctl->idx, rc);
 		dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_VSYNC);
 	}
@@ -619,7 +608,6 @@ static void dpu_encoder_phys_vid_prepare_for_kickoff(
 static void dpu_encoder_phys_vid_disable(struct dpu_encoder_phys *phys_enc)
 {
 	struct msm_drm_private *priv;
-	struct dpu_encoder_phys_vid *vid_enc;
 	unsigned long lock_flags;
 	int ret;
 
@@ -630,16 +618,13 @@ static void dpu_encoder_phys_vid_disable(struct dpu_encoder_phys *phys_enc)
 	}
 	priv = phys_enc->parent->dev->dev_private;
 
-	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
-	if (!vid_enc->hw_intf || !phys_enc->hw_ctl) {
+	if (!phys_enc->hw_intf || !phys_enc->hw_ctl) {
 		DPU_ERROR("invalid hw_intf %d hw_ctl %d\n",
-				vid_enc->hw_intf != 0, phys_enc->hw_ctl != 0);
+				phys_enc->hw_intf != 0, phys_enc->hw_ctl != 0);
 		return;
 	}
 
-	DPU_DEBUG_VIDENC(vid_enc, "\n");
-
-	if (WARN_ON(!vid_enc->hw_intf->ops.enable_timing))
+	if (WARN_ON(!phys_enc->hw_intf->ops.enable_timing))
 		return;
 
 	if (phys_enc->enable_state == DPU_ENC_DISABLED) {
@@ -648,7 +633,7 @@ static void dpu_encoder_phys_vid_disable(struct dpu_encoder_phys *phys_enc)
 	}
 
 	spin_lock_irqsave(phys_enc->enc_spinlock, lock_flags);
-	vid_enc->hw_intf->ops.enable_timing(vid_enc->hw_intf, 0);
+	phys_enc->hw_intf->ops.enable_timing(phys_enc->hw_intf, 0);
 	if (dpu_encoder_phys_vid_is_master(phys_enc))
 		dpu_encoder_phys_inc_pending(phys_enc);
 	spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);
@@ -667,7 +652,7 @@ static void dpu_encoder_phys_vid_disable(struct dpu_encoder_phys *phys_enc)
 			atomic_set(&phys_enc->pending_kickoff_cnt, 0);
 			DRM_ERROR("wait disable failed: id:%u intf:%d ret:%d\n",
 				  DRMID(phys_enc->parent),
-				  vid_enc->hw_intf->idx - INTF_0, ret);
+				  phys_enc->hw_intf->idx - INTF_0, ret);
 		}
 	}
 
@@ -678,25 +663,21 @@ static void dpu_encoder_phys_vid_handle_post_kickoff(
 		struct dpu_encoder_phys *phys_enc)
 {
 	unsigned long lock_flags;
-	struct dpu_encoder_phys_vid *vid_enc;
 
 	if (!phys_enc) {
 		DPU_ERROR("invalid encoder\n");
 		return;
 	}
 
-	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
-	DPU_DEBUG_VIDENC(vid_enc, "enable_state %d\n", phys_enc->enable_state);
-
 	/*
 	 * Video mode must flush CTL before enabling timing engine
 	 * Video encoders need to turn on their interfaces now
 	 */
 	if (phys_enc->enable_state == DPU_ENC_ENABLING) {
 		trace_dpu_enc_phys_vid_post_kickoff(DRMID(phys_enc->parent),
-				    vid_enc->hw_intf->idx - INTF_0);
+				    phys_enc->hw_intf->idx - INTF_0);
 		spin_lock_irqsave(phys_enc->enc_spinlock, lock_flags);
-		vid_enc->hw_intf->ops.enable_timing(vid_enc->hw_intf, 1);
+		phys_enc->hw_intf->ops.enable_timing(phys_enc->hw_intf, 1);
 		spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);
 		phys_enc->enable_state = DPU_ENC_ENABLED;
 	}
@@ -705,16 +686,13 @@ static void dpu_encoder_phys_vid_handle_post_kickoff(
 static void dpu_encoder_phys_vid_irq_control(struct dpu_encoder_phys *phys_enc,
 		bool enable)
 {
-	struct dpu_encoder_phys_vid *vid_enc;
 	int ret;
 
 	if (!phys_enc)
 		return;
 
-	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
-
 	trace_dpu_enc_phys_vid_irq_ctrl(DRMID(phys_enc->parent),
-			    vid_enc->hw_intf->idx - INTF_0,
+			    phys_enc->hw_intf->idx - INTF_0,
 			    enable,
 			    atomic_read(&phys_enc->vblank_refcount));
 
@@ -733,19 +711,16 @@ static void dpu_encoder_phys_vid_irq_control(struct dpu_encoder_phys *phys_enc,
 static int dpu_encoder_phys_vid_get_line_count(
 		struct dpu_encoder_phys *phys_enc)
 {
-	struct dpu_encoder_phys_vid *vid_enc;
-
 	if (!phys_enc)
 		return -EINVAL;
 
 	if (!dpu_encoder_phys_vid_is_master(phys_enc))
 		return -EINVAL;
 
-	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
-	if (!vid_enc->hw_intf || !vid_enc->hw_intf->ops.get_line_count)
+	if (!phys_enc->hw_intf || !phys_enc->hw_intf->ops.get_line_count)
 		return -EINVAL;
 
-	return vid_enc->hw_intf->ops.get_line_count(vid_enc->hw_intf);
+	return phys_enc->hw_intf->ops.get_line_count(phys_enc->hw_intf);
 }
 
 static void dpu_encoder_phys_vid_init_ops(struct dpu_encoder_phys_ops *ops)
@@ -792,7 +767,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
 	phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
 	phys_enc->intf_idx = p->intf_idx;
 
-	DPU_DEBUG_VIDENC(vid_enc, "\n");
+	DPU_DEBUG_VIDENC(phys_enc, "\n");
 
 	dpu_encoder_phys_vid_init_ops(&phys_enc->ops);
 	phys_enc->parent = p->parent;
@@ -826,7 +801,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
 	init_waitqueue_head(&phys_enc->pending_kickoff_wq);
 	phys_enc->enable_state = DPU_ENC_DISABLED;
 
-	DPU_DEBUG_VIDENC(vid_enc, "created intf idx:%d\n", p->intf_idx);
+	DPU_DEBUG_VIDENC(phys_enc, "created intf idx:%d\n", p->intf_idx);
 
 	return phys_enc;
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 1/7] drm/msm/dpu: move hw_inf encoder baseclass

[Sean Paul]

On Wed, Feb 13, 2019 at 05:19:10PM -0800, Jeykumar Sankaran wrote:
> Both video and command physical encoders will have
> a hw interface assigned to it. So there is really no
> need to track the hw block in specific encoder subclass.
> 
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>

Reviewed-by: Sean Paul <sean@poorly.run>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h   |   4 +-
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   | 125 +++++++++------------
>  2 files changed, 52 insertions(+), 77 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> index 44e6f8b6..acd5956 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -201,6 +201,7 @@ struct dpu_encoder_irq {
>   * @hw_mdptop:		Hardware interface to the top registers
>   * @hw_ctl:		Hardware interface to the ctl registers
>   * @hw_pp:		Hardware interface to the ping pong registers
> + * @hw_intf:		Hardware interface to the intf registers
>   * @dpu_kms:		Pointer to the dpu_kms top level
>   * @cached_mode:	DRM mode cached at mode_set time, acted on in enable
>   * @enabled:		Whether the encoder has enabled and running a mode
> @@ -229,6 +230,7 @@ struct dpu_encoder_phys {
>  	struct dpu_hw_mdp *hw_mdptop;
>  	struct dpu_hw_ctl *hw_ctl;
>  	struct dpu_hw_pingpong *hw_pp;
> +	struct dpu_hw_intf *hw_intf;
>  	struct dpu_kms *dpu_kms;
>  	struct drm_display_mode cached_mode;
>  	enum dpu_enc_split_role split_role;
> @@ -255,12 +257,10 @@ static inline int dpu_encoder_phys_inc_pending(struct dpu_encoder_phys *phys)
>   * struct dpu_encoder_phys_vid - sub-class of dpu_encoder_phys to handle video
>   *	mode specific operations
>   * @base:	Baseclass physical encoder structure
> - * @hw_intf:	Hardware interface to the intf registers
>   * @timing_params: Current timing parameter
>   */
>  struct dpu_encoder_phys_vid {
>  	struct dpu_encoder_phys base;
> -	struct dpu_hw_intf *hw_intf;
>  	struct intf_timing_params timing_params;
>  };
>  
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index acdab5b0..e326395 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -18,14 +18,14 @@
>  #include "dpu_trace.h"
>  
>  #define DPU_DEBUG_VIDENC(e, fmt, ...) DPU_DEBUG("enc%d intf%d " fmt, \
> -		(e) && (e)->base.parent ? \
> -		(e)->base.parent->base.id : -1, \
> +		(e) && (e)->parent ? \
> +		(e)->parent->base.id : -1, \
>  		(e) && (e)->hw_intf ? \
>  		(e)->hw_intf->idx - INTF_0 : -1, ##__VA_ARGS__)
>  
>  #define DPU_ERROR_VIDENC(e, fmt, ...) DPU_ERROR("enc%d intf%d " fmt, \
> -		(e) && (e)->base.parent ? \
> -		(e)->base.parent->base.id : -1, \
> +		(e) && (e)->parent ? \
> +		(e)->parent->base.id : -1, \
>  		(e) && (e)->hw_intf ? \
>  		(e)->hw_intf->idx - INTF_0 : -1, ##__VA_ARGS__)
>  
> @@ -44,7 +44,7 @@ static bool dpu_encoder_phys_vid_is_master(
>  }
>  
>  static void drm_mode_to_intf_timing_params(
> -		const struct dpu_encoder_phys_vid *vid_enc,
> +		const struct dpu_encoder_phys *phys_enc,
>  		const struct drm_display_mode *mode,
>  		struct intf_timing_params *timing)
>  {
> @@ -92,7 +92,7 @@ static void drm_mode_to_intf_timing_params(
>  	timing->hsync_skew = mode->hskew;
>  
>  	/* DSI controller cannot handle active-low sync signals. */
> -	if (vid_enc->hw_intf->cap->type == INTF_DSI) {
> +	if (phys_enc->hw_intf->cap->type == INTF_DSI) {
>  		timing->hsync_polarity = 0;
>  		timing->vsync_polarity = 0;
>  	}
> @@ -143,11 +143,11 @@ static u32 get_vertical_total(const struct intf_timing_params *timing)
>   * lines based on the chip worst case latencies.
>   */
>  static u32 programmable_fetch_get_num_lines(
> -		struct dpu_encoder_phys_vid *vid_enc,
> +		struct dpu_encoder_phys *phys_enc,
>  		const struct intf_timing_params *timing)
>  {
>  	u32 worst_case_needed_lines =
> -	    vid_enc->hw_intf->cap->prog_fetch_lines_worst_case;
> +	    phys_enc->hw_intf->cap->prog_fetch_lines_worst_case;
>  	u32 start_of_frame_lines =
>  	    timing->v_back_porch + timing->vsync_pulse_width;
>  	u32 needed_vfp_lines = worst_case_needed_lines - start_of_frame_lines;
> @@ -155,26 +155,26 @@ static u32 programmable_fetch_get_num_lines(
>  
>  	/* Fetch must be outside active lines, otherwise undefined. */
>  	if (start_of_frame_lines >= worst_case_needed_lines) {
> -		DPU_DEBUG_VIDENC(vid_enc,
> +		DPU_DEBUG_VIDENC(phys_enc,
>  				"prog fetch is not needed, large vbp+vsw\n");
>  		actual_vfp_lines = 0;
>  	} else if (timing->v_front_porch < needed_vfp_lines) {
>  		/* Warn fetch needed, but not enough porch in panel config */
>  		pr_warn_once
>  			("low vbp+vfp may lead to perf issues in some cases\n");
> -		DPU_DEBUG_VIDENC(vid_enc,
> +		DPU_DEBUG_VIDENC(phys_enc,
>  				"less vfp than fetch req, using entire vfp\n");
>  		actual_vfp_lines = timing->v_front_porch;
>  	} else {
> -		DPU_DEBUG_VIDENC(vid_enc, "room in vfp for needed prefetch\n");
> +		DPU_DEBUG_VIDENC(phys_enc, "room in vfp for needed prefetch\n");
>  		actual_vfp_lines = needed_vfp_lines;
>  	}
>  
> -	DPU_DEBUG_VIDENC(vid_enc,
> +	DPU_DEBUG_VIDENC(phys_enc,
>  		"v_front_porch %u v_back_porch %u vsync_pulse_width %u\n",
>  		timing->v_front_porch, timing->v_back_porch,
>  		timing->vsync_pulse_width);
> -	DPU_DEBUG_VIDENC(vid_enc,
> +	DPU_DEBUG_VIDENC(phys_enc,
>  		"wc_lines %u needed_vfp_lines %u actual_vfp_lines %u\n",
>  		worst_case_needed_lines, needed_vfp_lines, actual_vfp_lines);
>  
> @@ -194,8 +194,6 @@ static u32 programmable_fetch_get_num_lines(
>  static void programmable_fetch_config(struct dpu_encoder_phys *phys_enc,
>  				      const struct intf_timing_params *timing)
>  {
> -	struct dpu_encoder_phys_vid *vid_enc =
> -		to_dpu_encoder_phys_vid(phys_enc);
>  	struct intf_prog_fetch f = { 0 };
>  	u32 vfp_fetch_lines = 0;
>  	u32 horiz_total = 0;
> @@ -203,10 +201,10 @@ static void programmable_fetch_config(struct dpu_encoder_phys *phys_enc,
>  	u32 vfp_fetch_start_vsync_counter = 0;
>  	unsigned long lock_flags;
>  
> -	if (WARN_ON_ONCE(!vid_enc->hw_intf->ops.setup_prg_fetch))
> +	if (WARN_ON_ONCE(!phys_enc->hw_intf->ops.setup_prg_fetch))
>  		return;
>  
> -	vfp_fetch_lines = programmable_fetch_get_num_lines(vid_enc, timing);
> +	vfp_fetch_lines = programmable_fetch_get_num_lines(phys_enc, timing);
>  	if (vfp_fetch_lines) {
>  		vert_total = get_vertical_total(timing);
>  		horiz_total = get_horizontal_total(timing);
> @@ -216,12 +214,12 @@ static void programmable_fetch_config(struct dpu_encoder_phys *phys_enc,
>  		f.fetch_start = vfp_fetch_start_vsync_counter;
>  	}
>  
> -	DPU_DEBUG_VIDENC(vid_enc,
> +	DPU_DEBUG_VIDENC(phys_enc,
>  		"vfp_fetch_lines %u vfp_fetch_start_vsync_counter %u\n",
>  		vfp_fetch_lines, vfp_fetch_start_vsync_counter);
>  
>  	spin_lock_irqsave(phys_enc->enc_spinlock, lock_flags);
> -	vid_enc->hw_intf->ops.setup_prg_fetch(vid_enc->hw_intf, &f);
> +	phys_enc->hw_intf->ops.setup_prg_fetch(phys_enc->hw_intf, &f);
>  	spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);
>  }
>  
> @@ -231,7 +229,7 @@ static bool dpu_encoder_phys_vid_mode_fixup(
>  		struct drm_display_mode *adj_mode)
>  {
>  	if (phys_enc)
> -		DPU_DEBUG_VIDENC(to_dpu_encoder_phys_vid(phys_enc), "\n");
> +		DPU_DEBUG_VIDENC(phys_enc, "\n");
>  
>  	/*
>  	 * Modifying mode has consequences when the mode comes back to us
> @@ -257,12 +255,12 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
>  
>  	mode = phys_enc->cached_mode;
>  	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
> -	if (!vid_enc->hw_intf->ops.setup_timing_gen) {
> +	if (!phys_enc->hw_intf->ops.setup_timing_gen) {
>  		DPU_ERROR("timing engine setup is not supported\n");
>  		return;
>  	}
>  
> -	DPU_DEBUG_VIDENC(vid_enc, "enabling mode:\n");
> +	DPU_DEBUG_VIDENC(phys_enc, "enabling mode:\n");
>  	drm_mode_debug_printmodeline(&mode);
>  
>  	if (phys_enc->split_role != ENC_ROLE_SOLO) {
> @@ -271,25 +269,25 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
>  		mode.hsync_start >>= 1;
>  		mode.hsync_end >>= 1;
>  
> -		DPU_DEBUG_VIDENC(vid_enc,
> +		DPU_DEBUG_VIDENC(phys_enc,
>  			"split_role %d, halve horizontal %d %d %d %d\n",
>  			phys_enc->split_role,
>  			mode.hdisplay, mode.htotal,
>  			mode.hsync_start, mode.hsync_end);
>  	}
>  
> -	drm_mode_to_intf_timing_params(vid_enc, &mode, &timing_params);
> +	drm_mode_to_intf_timing_params(phys_enc, &mode, &timing_params);
>  
>  	fmt = dpu_get_dpu_format(fmt_fourcc);
> -	DPU_DEBUG_VIDENC(vid_enc, "fmt_fourcc 0x%X\n", fmt_fourcc);
> +	DPU_DEBUG_VIDENC(phys_enc, "fmt_fourcc 0x%X\n", fmt_fourcc);
>  
> -	intf_cfg.intf = vid_enc->hw_intf->idx;
> +	intf_cfg.intf = phys_enc->hw_intf->idx;
>  	intf_cfg.intf_mode_sel = DPU_CTL_MODE_SEL_VID;
>  	intf_cfg.stream_sel = 0; /* Don't care value for video mode */
>  	intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
>  
>  	spin_lock_irqsave(phys_enc->enc_spinlock, lock_flags);
> -	vid_enc->hw_intf->ops.setup_timing_gen(vid_enc->hw_intf,
> +	phys_enc->hw_intf->ops.setup_timing_gen(phys_enc->hw_intf,
>  			&timing_params, fmt);
>  	phys_enc->hw_ctl->ops.setup_intf_cfg(phys_enc->hw_ctl, &intf_cfg);
>  	spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);
> @@ -396,19 +394,15 @@ static void dpu_encoder_phys_vid_mode_set(
>  		struct drm_display_mode *mode,
>  		struct drm_display_mode *adj_mode)
>  {
> -	struct dpu_encoder_phys_vid *vid_enc;
> -
>  	if (!phys_enc || !phys_enc->dpu_kms) {
>  		DPU_ERROR("invalid encoder/kms\n");
>  		return;
>  	}
>  
> -	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
> -
>  	if (adj_mode) {
>  		phys_enc->cached_mode = *adj_mode;
>  		drm_mode_debug_printmodeline(adj_mode);
> -		DPU_DEBUG_VIDENC(vid_enc, "caching mode:\n");
> +		DPU_DEBUG_VIDENC(phys_enc, "caching mode:\n");
>  	}
>  
>  	_dpu_encoder_phys_vid_setup_irq_hw_idx(phys_enc);
> @@ -419,7 +413,6 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
>  		bool enable)
>  {
>  	int ret = 0;
> -	struct dpu_encoder_phys_vid *vid_enc;
>  	int refcount;
>  
>  	if (!phys_enc) {
> @@ -428,7 +421,6 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
>  	}
>  
>  	refcount = atomic_read(&phys_enc->vblank_refcount);
> -	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
>  
>  	/* Slave encoders don't report vblank */
>  	if (!dpu_encoder_phys_vid_is_master(phys_enc))
> @@ -453,7 +445,7 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
>  	if (ret) {
>  		DRM_ERROR("failed: id:%u intf:%d ret:%d enable:%d refcnt:%d\n",
>  			  DRMID(phys_enc->parent),
> -			  vid_enc->hw_intf->idx - INTF_0, ret, enable,
> +			  phys_enc->hw_intf->idx - INTF_0, ret, enable,
>  			  refcount);
>  	}
>  	return ret;
> @@ -462,7 +454,6 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
>  static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
>  {
>  	struct msm_drm_private *priv;
> -	struct dpu_encoder_phys_vid *vid_enc;
>  	struct dpu_rm_hw_iter iter;
>  	struct dpu_hw_ctl *ctl;
>  	u32 flush_mask = 0;
> @@ -474,7 +465,6 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
>  	}
>  	priv = phys_enc->parent->dev->dev_private;
>  
> -	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
>  	ctl = phys_enc->hw_ctl;
>  
>  	dpu_rm_init_hw_iter(&iter, phys_enc->parent->base.id, DPU_HW_BLK_INTF);
> @@ -482,22 +472,22 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
>  		struct dpu_hw_intf *hw_intf = (struct dpu_hw_intf *)iter.hw;
>  
>  		if (hw_intf->idx == phys_enc->intf_idx) {
> -			vid_enc->hw_intf = hw_intf;
> +			phys_enc->hw_intf = hw_intf;
>  			break;
>  		}
>  	}
>  
> -	if (!vid_enc->hw_intf) {
> +	if (!phys_enc->hw_intf) {
>  		DPU_ERROR("hw_intf not assigned\n");
>  		return;
>  	}
>  
> -	DPU_DEBUG_VIDENC(vid_enc, "\n");
> +	DPU_DEBUG_VIDENC(phys_enc, "\n");
>  
> -	if (WARN_ON(!vid_enc->hw_intf->ops.enable_timing))
> +	if (WARN_ON(!phys_enc->hw_intf->ops.enable_timing))
>  		return;
>  
> -	dpu_encoder_helper_split_config(phys_enc, vid_enc->hw_intf->idx);
> +	dpu_encoder_helper_split_config(phys_enc, phys_enc->hw_intf->idx);
>  
>  	dpu_encoder_phys_vid_setup_timing_engine(phys_enc);
>  
> @@ -510,12 +500,13 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
>  		!dpu_encoder_phys_vid_is_master(phys_enc))
>  		goto skip_flush;
>  
> -	ctl->ops.get_bitmask_intf(ctl, &flush_mask, vid_enc->hw_intf->idx);
> +	ctl->ops.get_bitmask_intf(ctl, &flush_mask, phys_enc->hw_intf->idx);
>  	ctl->ops.update_pending_flush(ctl, flush_mask);
>  
>  skip_flush:
> -	DPU_DEBUG_VIDENC(vid_enc, "update pending flush ctl %d flush_mask %x\n",
> -		ctl->idx - CTL_0, flush_mask);
> +	DPU_DEBUG_VIDENC(phys_enc,
> +			 "update pending flush ctl %d flush_mask %x\n",
> +			 ctl->idx - CTL_0, flush_mask);
>  
>  	/* ctl_flush & timing engine enable will be triggered by framework */
>  	if (phys_enc->enable_state == DPU_ENC_DISABLED)
> @@ -532,7 +523,7 @@ static void dpu_encoder_phys_vid_destroy(struct dpu_encoder_phys *phys_enc)
>  	}
>  
>  	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
> -	DPU_DEBUG_VIDENC(vid_enc, "\n");
> +	DPU_DEBUG_VIDENC(phys_enc, "\n");
>  	kfree(vid_enc);
>  }
>  
> @@ -590,7 +581,6 @@ static void dpu_encoder_phys_vid_prepare_for_kickoff(
>  		struct dpu_encoder_phys *phys_enc,
>  		struct dpu_encoder_kickoff_params *params)
>  {
> -	struct dpu_encoder_phys_vid *vid_enc;
>  	struct dpu_hw_ctl *ctl;
>  	int rc;
>  
> @@ -598,7 +588,6 @@ static void dpu_encoder_phys_vid_prepare_for_kickoff(
>  		DPU_ERROR("invalid encoder/parameters\n");
>  		return;
>  	}
> -	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
>  
>  	ctl = phys_enc->hw_ctl;
>  	if (!ctl || !ctl->ops.wait_reset_status)
> @@ -610,7 +599,7 @@ static void dpu_encoder_phys_vid_prepare_for_kickoff(
>  	 */
>  	rc = ctl->ops.wait_reset_status(ctl);
>  	if (rc) {
> -		DPU_ERROR_VIDENC(vid_enc, "ctl %d reset failure: %d\n",
> +		DPU_ERROR_VIDENC(phys_enc, "ctl %d reset failure: %d\n",
>  				ctl->idx, rc);
>  		dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_VSYNC);
>  	}
> @@ -619,7 +608,6 @@ static void dpu_encoder_phys_vid_prepare_for_kickoff(
>  static void dpu_encoder_phys_vid_disable(struct dpu_encoder_phys *phys_enc)
>  {
>  	struct msm_drm_private *priv;
> -	struct dpu_encoder_phys_vid *vid_enc;
>  	unsigned long lock_flags;
>  	int ret;
>  
> @@ -630,16 +618,13 @@ static void dpu_encoder_phys_vid_disable(struct dpu_encoder_phys *phys_enc)
>  	}
>  	priv = phys_enc->parent->dev->dev_private;
>  
> -	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
> -	if (!vid_enc->hw_intf || !phys_enc->hw_ctl) {
> +	if (!phys_enc->hw_intf || !phys_enc->hw_ctl) {
>  		DPU_ERROR("invalid hw_intf %d hw_ctl %d\n",
> -				vid_enc->hw_intf != 0, phys_enc->hw_ctl != 0);
> +				phys_enc->hw_intf != 0, phys_enc->hw_ctl != 0);
>  		return;
>  	}
>  
> -	DPU_DEBUG_VIDENC(vid_enc, "\n");
> -
> -	if (WARN_ON(!vid_enc->hw_intf->ops.enable_timing))
> +	if (WARN_ON(!phys_enc->hw_intf->ops.enable_timing))
>  		return;
>  
>  	if (phys_enc->enable_state == DPU_ENC_DISABLED) {
> @@ -648,7 +633,7 @@ static void dpu_encoder_phys_vid_disable(struct dpu_encoder_phys *phys_enc)
>  	}
>  
>  	spin_lock_irqsave(phys_enc->enc_spinlock, lock_flags);
> -	vid_enc->hw_intf->ops.enable_timing(vid_enc->hw_intf, 0);
> +	phys_enc->hw_intf->ops.enable_timing(phys_enc->hw_intf, 0);
>  	if (dpu_encoder_phys_vid_is_master(phys_enc))
>  		dpu_encoder_phys_inc_pending(phys_enc);
>  	spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);
> @@ -667,7 +652,7 @@ static void dpu_encoder_phys_vid_disable(struct dpu_encoder_phys *phys_enc)
>  			atomic_set(&phys_enc->pending_kickoff_cnt, 0);
>  			DRM_ERROR("wait disable failed: id:%u intf:%d ret:%d\n",
>  				  DRMID(phys_enc->parent),
> -				  vid_enc->hw_intf->idx - INTF_0, ret);
> +				  phys_enc->hw_intf->idx - INTF_0, ret);
>  		}
>  	}
>  
> @@ -678,25 +663,21 @@ static void dpu_encoder_phys_vid_handle_post_kickoff(
>  		struct dpu_encoder_phys *phys_enc)
>  {
>  	unsigned long lock_flags;
> -	struct dpu_encoder_phys_vid *vid_enc;
>  
>  	if (!phys_enc) {
>  		DPU_ERROR("invalid encoder\n");
>  		return;
>  	}
>  
> -	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
> -	DPU_DEBUG_VIDENC(vid_enc, "enable_state %d\n", phys_enc->enable_state);
> -
>  	/*
>  	 * Video mode must flush CTL before enabling timing engine
>  	 * Video encoders need to turn on their interfaces now
>  	 */
>  	if (phys_enc->enable_state == DPU_ENC_ENABLING) {
>  		trace_dpu_enc_phys_vid_post_kickoff(DRMID(phys_enc->parent),
> -				    vid_enc->hw_intf->idx - INTF_0);
> +				    phys_enc->hw_intf->idx - INTF_0);
>  		spin_lock_irqsave(phys_enc->enc_spinlock, lock_flags);
> -		vid_enc->hw_intf->ops.enable_timing(vid_enc->hw_intf, 1);
> +		phys_enc->hw_intf->ops.enable_timing(phys_enc->hw_intf, 1);
>  		spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);
>  		phys_enc->enable_state = DPU_ENC_ENABLED;
>  	}
> @@ -705,16 +686,13 @@ static void dpu_encoder_phys_vid_handle_post_kickoff(
>  static void dpu_encoder_phys_vid_irq_control(struct dpu_encoder_phys *phys_enc,
>  		bool enable)
>  {
> -	struct dpu_encoder_phys_vid *vid_enc;
>  	int ret;
>  
>  	if (!phys_enc)
>  		return;
>  
> -	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
> -
>  	trace_dpu_enc_phys_vid_irq_ctrl(DRMID(phys_enc->parent),
> -			    vid_enc->hw_intf->idx - INTF_0,
> +			    phys_enc->hw_intf->idx - INTF_0,
>  			    enable,
>  			    atomic_read(&phys_enc->vblank_refcount));
>  
> @@ -733,19 +711,16 @@ static void dpu_encoder_phys_vid_irq_control(struct dpu_encoder_phys *phys_enc,
>  static int dpu_encoder_phys_vid_get_line_count(
>  		struct dpu_encoder_phys *phys_enc)
>  {
> -	struct dpu_encoder_phys_vid *vid_enc;
> -
>  	if (!phys_enc)
>  		return -EINVAL;
>  
>  	if (!dpu_encoder_phys_vid_is_master(phys_enc))
>  		return -EINVAL;
>  
> -	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
> -	if (!vid_enc->hw_intf || !vid_enc->hw_intf->ops.get_line_count)
> +	if (!phys_enc->hw_intf || !phys_enc->hw_intf->ops.get_line_count)
>  		return -EINVAL;
>  
> -	return vid_enc->hw_intf->ops.get_line_count(vid_enc->hw_intf);
> +	return phys_enc->hw_intf->ops.get_line_count(phys_enc->hw_intf);
>  }
>  
>  static void dpu_encoder_phys_vid_init_ops(struct dpu_encoder_phys_ops *ops)
> @@ -792,7 +767,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
>  	phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
>  	phys_enc->intf_idx = p->intf_idx;
>  
> -	DPU_DEBUG_VIDENC(vid_enc, "\n");
> +	DPU_DEBUG_VIDENC(phys_enc, "\n");
>  
>  	dpu_encoder_phys_vid_init_ops(&phys_enc->ops);
>  	phys_enc->parent = p->parent;
> @@ -826,7 +801,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
>  	init_waitqueue_head(&phys_enc->pending_kickoff_wq);
>  	phys_enc->enable_state = DPU_ENC_DISABLED;
>  
> -	DPU_DEBUG_VIDENC(vid_enc, "created intf idx:%d\n", p->intf_idx);
> +	DPU_DEBUG_VIDENC(phys_enc, "created intf idx:%d\n", p->intf_idx);
>  
>  	return phys_enc;
>  
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 2/7] drm/msm/dpu: remove phys_vid subclass

[Jeykumar Sankaran]

Not holding any video encoder specific data. Get rid of it.

Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h     | 11 -----------
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 18 ++++--------------
 2 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index acd5956..9b1efd9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -254,17 +254,6 @@ static inline int dpu_encoder_phys_inc_pending(struct dpu_encoder_phys *phys)
 }
 
 /**
- * struct dpu_encoder_phys_vid - sub-class of dpu_encoder_phys to handle video
- *	mode specific operations
- * @base:	Baseclass physical encoder structure
- * @timing_params: Current timing parameter
- */
-struct dpu_encoder_phys_vid {
-	struct dpu_encoder_phys base;
-	struct intf_timing_params timing_params;
-};
-
-/**
  * struct dpu_encoder_phys_cmd - sub-class of dpu_encoder_phys to handle command
  *	mode specific operations
  * @base:	Baseclass physical encoder structure
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index e326395..ce65521 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -240,7 +240,6 @@ static bool dpu_encoder_phys_vid_mode_fixup(
 static void dpu_encoder_phys_vid_setup_timing_engine(
 		struct dpu_encoder_phys *phys_enc)
 {
-	struct dpu_encoder_phys_vid *vid_enc;
 	struct drm_display_mode mode;
 	struct intf_timing_params timing_params = { 0 };
 	const struct dpu_format *fmt = NULL;
@@ -254,7 +253,6 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
 	}
 
 	mode = phys_enc->cached_mode;
-	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
 	if (!phys_enc->hw_intf->ops.setup_timing_gen) {
 		DPU_ERROR("timing engine setup is not supported\n");
 		return;
@@ -293,8 +291,6 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
 	spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);
 
 	programmable_fetch_config(phys_enc, &timing_params);
-
-	vid_enc->timing_params = timing_params;
 }
 
 static void dpu_encoder_phys_vid_vblank_irq(void *arg, int irq_idx)
@@ -515,16 +511,13 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
 
 static void dpu_encoder_phys_vid_destroy(struct dpu_encoder_phys *phys_enc)
 {
-	struct dpu_encoder_phys_vid *vid_enc;
-
 	if (!phys_enc) {
 		DPU_ERROR("invalid encoder\n");
 		return;
 	}
 
-	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
 	DPU_DEBUG_VIDENC(phys_enc, "\n");
-	kfree(vid_enc);
+	kfree(phys_enc);
 }
 
 static void dpu_encoder_phys_vid_get_hw_resources(
@@ -747,7 +740,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
 		struct dpu_enc_phys_init_params *p)
 {
 	struct dpu_encoder_phys *phys_enc = NULL;
-	struct dpu_encoder_phys_vid *vid_enc = NULL;
 	struct dpu_encoder_irq *irq;
 	int i, ret = 0;
 
@@ -756,14 +748,12 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
 		goto fail;
 	}
 
-	vid_enc = kzalloc(sizeof(*vid_enc), GFP_KERNEL);
-	if (!vid_enc) {
+	phys_enc = kzalloc(sizeof(*phys_enc), GFP_KERNEL);
+	if (!phys_enc) {
 		ret = -ENOMEM;
 		goto fail;
 	}
 
-	phys_enc = &vid_enc->base;
-
 	phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
 	phys_enc->intf_idx = p->intf_idx;
 
@@ -807,7 +797,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
 
 fail:
 	DPU_ERROR("failed to create encoder\n");
-	if (vid_enc)
+	if (phys_enc)
 		dpu_encoder_phys_vid_destroy(phys_enc);
 
 	return ERR_PTR(ret);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 2/7] drm/msm/dpu: remove phys_vid subclass

[Sean Paul]

On Wed, Feb 13, 2019 at 05:19:11PM -0800, Jeykumar Sankaran wrote:
> Not holding any video encoder specific data. Get rid of it.
> 
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>

Reviewed-by: Sean Paul <sean@poorly.run>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h     | 11 -----------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 18 ++++--------------
>  2 files changed, 4 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> index acd5956..9b1efd9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -254,17 +254,6 @@ static inline int dpu_encoder_phys_inc_pending(struct dpu_encoder_phys *phys)
>  }
>  
>  /**
> - * struct dpu_encoder_phys_vid - sub-class of dpu_encoder_phys to handle video
> - *	mode specific operations
> - * @base:	Baseclass physical encoder structure
> - * @timing_params: Current timing parameter
> - */
> -struct dpu_encoder_phys_vid {
> -	struct dpu_encoder_phys base;
> -	struct intf_timing_params timing_params;
> -};
> -
> -/**
>   * struct dpu_encoder_phys_cmd - sub-class of dpu_encoder_phys to handle command
>   *	mode specific operations
>   * @base:	Baseclass physical encoder structure
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index e326395..ce65521 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -240,7 +240,6 @@ static bool dpu_encoder_phys_vid_mode_fixup(
>  static void dpu_encoder_phys_vid_setup_timing_engine(
>  		struct dpu_encoder_phys *phys_enc)
>  {
> -	struct dpu_encoder_phys_vid *vid_enc;
>  	struct drm_display_mode mode;
>  	struct intf_timing_params timing_params = { 0 };
>  	const struct dpu_format *fmt = NULL;
> @@ -254,7 +253,6 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
>  	}
>  
>  	mode = phys_enc->cached_mode;
> -	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
>  	if (!phys_enc->hw_intf->ops.setup_timing_gen) {
>  		DPU_ERROR("timing engine setup is not supported\n");
>  		return;
> @@ -293,8 +291,6 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
>  	spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);
>  
>  	programmable_fetch_config(phys_enc, &timing_params);
> -
> -	vid_enc->timing_params = timing_params;
>  }
>  
>  static void dpu_encoder_phys_vid_vblank_irq(void *arg, int irq_idx)
> @@ -515,16 +511,13 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
>  
>  static void dpu_encoder_phys_vid_destroy(struct dpu_encoder_phys *phys_enc)
>  {
> -	struct dpu_encoder_phys_vid *vid_enc;
> -
>  	if (!phys_enc) {
>  		DPU_ERROR("invalid encoder\n");
>  		return;
>  	}
>  
> -	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
>  	DPU_DEBUG_VIDENC(phys_enc, "\n");
> -	kfree(vid_enc);
> +	kfree(phys_enc);
>  }
>  
>  static void dpu_encoder_phys_vid_get_hw_resources(
> @@ -747,7 +740,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
>  		struct dpu_enc_phys_init_params *p)
>  {
>  	struct dpu_encoder_phys *phys_enc = NULL;
> -	struct dpu_encoder_phys_vid *vid_enc = NULL;
>  	struct dpu_encoder_irq *irq;
>  	int i, ret = 0;
>  
> @@ -756,14 +748,12 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
>  		goto fail;
>  	}
>  
> -	vid_enc = kzalloc(sizeof(*vid_enc), GFP_KERNEL);
> -	if (!vid_enc) {
> +	phys_enc = kzalloc(sizeof(*phys_enc), GFP_KERNEL);
> +	if (!phys_enc) {
>  		ret = -ENOMEM;
>  		goto fail;
>  	}
>  
> -	phys_enc = &vid_enc->base;
> -
>  	phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
>  	phys_enc->intf_idx = p->intf_idx;
>  
> @@ -807,7 +797,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
>  
>  fail:
>  	DPU_ERROR("failed to create encoder\n");
> -	if (vid_enc)
> +	if (phys_enc)
>  		dpu_encoder_phys_vid_destroy(phys_enc);
>  
>  	return ERR_PTR(ret);
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 4/7] drm/msm/dpu: dont use encoder->crtc in atomic path

[Jeykumar Sankaran]

encoder->crtc is not really meaningful for atomic path. Use
crtc->encoder_mask to identify the crtc attached with
an encoder.

Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 45617b9..0a19124 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -961,6 +961,7 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 	struct dpu_kms *dpu_kms;
 	struct list_head *connector_list;
 	struct drm_connector *conn = NULL, *conn_iter;
+	struct drm_crtc *drm_crtc;
 	struct dpu_rm_hw_iter pp_iter, ctl_iter;
 	struct msm_display_topology topology;
 	struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
@@ -992,10 +993,14 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 		return;
 	}
 
+	drm_for_each_crtc(drm_crtc, drm_enc->dev)
+		if (drm_crtc->state->encoder_mask & drm_encoder_mask(drm_enc))
+			break;
+
 	topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
 
 	/* Reserve dynamic resources now. Indicating non-AtomicTest phase */
-	ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, drm_enc->crtc->state,
+	ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, drm_crtc->state,
 			     topology, false);
 	if (ret) {
 		DPU_ERROR_ENC(dpu_enc,
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 4/7] drm/msm/dpu: dont use encoder->crtc in atomic path

[Sean Paul]

On Wed, Feb 13, 2019 at 05:19:13PM -0800, Jeykumar Sankaran wrote:
> encoder->crtc is not really meaningful for atomic path. Use
> crtc->encoder_mask to identify the crtc attached with
> an encoder.
> 
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 45617b9..0a19124 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -961,6 +961,7 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>  	struct dpu_kms *dpu_kms;
>  	struct list_head *connector_list;
>  	struct drm_connector *conn = NULL, *conn_iter;
> +	struct drm_crtc *drm_crtc;
>  	struct dpu_rm_hw_iter pp_iter, ctl_iter;
>  	struct msm_display_topology topology;
>  	struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
> @@ -992,10 +993,14 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>  		return;
>  	}
>  
> +	drm_for_each_crtc(drm_crtc, drm_enc->dev)
> +		if (drm_crtc->state->encoder_mask & drm_encoder_mask(drm_enc))
> +			break;

You should check whether you actually found the crtc, or are just using the last
one in the list.

Sean

> +
>  	topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
>  
>  	/* Reserve dynamic resources now. Indicating non-AtomicTest phase */
> -	ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, drm_enc->crtc->state,
> +	ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, drm_crtc->state,
>  			     topology, false);
>  	if (ret) {
>  		DPU_ERROR_ENC(dpu_enc,
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 4/7] drm/msm/dpu: dont use encoder->crtc in atomic path

[Jeykumar Sankaran]

On 2019-03-04 10:09, Sean Paul wrote:
> On Wed, Feb 13, 2019 at 05:19:13PM -0800, Jeykumar Sankaran wrote:
>> encoder->crtc is not really meaningful for atomic path. Use
>> crtc->encoder_mask to identify the crtc attached with
>> an encoder.
>> 
>> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
>> ---
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> index 45617b9..0a19124 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> @@ -961,6 +961,7 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
>>  	struct dpu_kms *dpu_kms;
>>  	struct list_head *connector_list;
>>  	struct drm_connector *conn = NULL, *conn_iter;
>> +	struct drm_crtc *drm_crtc;
>>  	struct dpu_rm_hw_iter pp_iter, ctl_iter;
>>  	struct msm_display_topology topology;
>>  	struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
>> @@ -992,10 +993,14 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
>>  		return;
>>  	}
>> 
>> +	drm_for_each_crtc(drm_crtc, drm_enc->dev)
>> +		if (drm_crtc->state->encoder_mask &
> drm_encoder_mask(drm_enc))
>> +			break;
> 
> You should check whether you actually found the crtc, or are just using
> the last
> one in the list.
> 
I see you have pulled in the this entire series on msm-next. If it is 
the right
thing to do, I can address this comment in a separate patch on top 
instead of
posting v2.

Thanks
Jeykumar S.
> Sean
> 
>> +
>>  	topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
>> 
>>  	/* Reserve dynamic resources now. Indicating non-AtomicTest phase
> */
>> -	ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, drm_enc->crtc->state,
>> +	ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, drm_crtc->state,
>>  			     topology, false);
>>  	if (ret) {
>>  		DPU_ERROR_ENC(dpu_enc,
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> Forum,
>> a Linux Foundation Collaborative Project
>> 

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 4/7] drm/msm/dpu: dont use encoder->crtc in atomic path

[Sean Paul]

On Wed, Mar 06, 2019 at 06:03:05PM -0800, Jeykumar Sankaran wrote:
> On 2019-03-04 10:09, Sean Paul wrote:
> > On Wed, Feb 13, 2019 at 05:19:13PM -0800, Jeykumar Sankaran wrote:
> > > encoder->crtc is not really meaningful for atomic path. Use
> > > crtc->encoder_mask to identify the crtc attached with
> > > an encoder.
> > > 
> > > Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> > > ---
> > >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > index 45617b9..0a19124 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > @@ -961,6 +961,7 @@ static void dpu_encoder_virt_mode_set(struct
> > drm_encoder *drm_enc,
> > >  	struct dpu_kms *dpu_kms;
> > >  	struct list_head *connector_list;
> > >  	struct drm_connector *conn = NULL, *conn_iter;
> > > +	struct drm_crtc *drm_crtc;
> > >  	struct dpu_rm_hw_iter pp_iter, ctl_iter;
> > >  	struct msm_display_topology topology;
> > >  	struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
> > > @@ -992,10 +993,14 @@ static void dpu_encoder_virt_mode_set(struct
> > drm_encoder *drm_enc,
> > >  		return;
> > >  	}
> > > 
> > > +	drm_for_each_crtc(drm_crtc, drm_enc->dev)
> > > +		if (drm_crtc->state->encoder_mask &
> > drm_encoder_mask(drm_enc))
> > > +			break;
> > 
> > You should check whether you actually found the crtc, or are just using
> > the last
> > one in the list.
> > 
> I see you have pulled in the this entire series on msm-next. If it is the
> right
> thing to do, I can address this comment in a separate patch on top instead
> of
> posting v2.

Yes, please do.

Thanks,

Sean

> 
> Thanks
> Jeykumar S.
> > Sean
> > 
> > > +
> > >  	topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
> > > 
> > >  	/* Reserve dynamic resources now. Indicating non-AtomicTest phase
> > */
> > > -	ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, drm_enc->crtc->state,
> > > +	ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, drm_crtc->state,
> > >  			     topology, false);
> > >  	if (ret) {
> > >  		DPU_ERROR_ENC(dpu_enc,
> > > --
> > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> > Forum,
> > > a Linux Foundation Collaborative Project
> > > 
> 
> -- 
> Jeykumar S

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 5/7] drm/msm/dpu: map mixer/ctl hw blocks in encoder modeset

[Jeykumar Sankaran]

After resource allocation, iterate and populate mixer/ctl
hw blocks in encoder modeset thereby centralizing all
the resource mapping to the CRTC. This change is made
for easy switching to state based allocation using
private objects later in this series.

Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 64 +----------------------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 37 +++++++++++++----
 2 files changed, 31 insertions(+), 70 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 4e4b648..6d30ba9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -425,65 +425,6 @@ void dpu_crtc_complete_commit(struct drm_crtc *crtc,
 	trace_dpu_crtc_complete_commit(DRMID(crtc));
 }
 
-static void _dpu_crtc_setup_mixer_for_encoder(
-		struct drm_crtc *crtc,
-		struct drm_encoder *enc)
-{
-	struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
-	struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
-	struct dpu_rm *rm = &dpu_kms->rm;
-	struct dpu_crtc_mixer *mixer;
-	struct dpu_hw_ctl *last_valid_ctl = NULL;
-	int i;
-	struct dpu_rm_hw_iter lm_iter, ctl_iter;
-
-	dpu_rm_init_hw_iter(&lm_iter, enc->base.id, DPU_HW_BLK_LM);
-	dpu_rm_init_hw_iter(&ctl_iter, enc->base.id, DPU_HW_BLK_CTL);
-
-	/* Set up all the mixers and ctls reserved by this encoder */
-	for (i = cstate->num_mixers; i < ARRAY_SIZE(cstate->mixers); i++) {
-		mixer = &cstate->mixers[i];
-
-		if (!dpu_rm_get_hw(rm, &lm_iter))
-			break;
-		mixer->hw_lm = (struct dpu_hw_mixer *)lm_iter.hw;
-
-		/* CTL may be <= LMs, if <, multiple LMs controlled by 1 CTL */
-		if (!dpu_rm_get_hw(rm, &ctl_iter)) {
-			DPU_DEBUG("no ctl assigned to lm %d, using previous\n",
-					mixer->hw_lm->idx - LM_0);
-			mixer->lm_ctl = last_valid_ctl;
-		} else {
-			mixer->lm_ctl = (struct dpu_hw_ctl *)ctl_iter.hw;
-			last_valid_ctl = mixer->lm_ctl;
-		}
-
-		/* Shouldn't happen, mixers are always >= ctls */
-		if (!mixer->lm_ctl) {
-			DPU_ERROR("no valid ctls found for lm %d\n",
-					mixer->hw_lm->idx - LM_0);
-			return;
-		}
-
-		cstate->num_mixers++;
-		DPU_DEBUG("setup mixer %d: lm %d\n",
-				i, mixer->hw_lm->idx - LM_0);
-		DPU_DEBUG("setup mixer %d: ctl %d\n",
-				i, mixer->lm_ctl->idx - CTL_0);
-	}
-}
-
-static void _dpu_crtc_setup_mixers(struct drm_crtc *crtc)
-{
-	struct drm_encoder *enc;
-
-	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
-
-	/* Check for mixers on all encoders attached to this crtc */
-	drm_for_each_encoder_mask(enc, crtc->dev, crtc->state->encoder_mask)
-		_dpu_crtc_setup_mixer_for_encoder(crtc, enc);
-}
-
 static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc,
 		struct drm_crtc_state *state)
 {
@@ -533,10 +474,7 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc,
 	dev = crtc->dev;
 	smmu_state = &dpu_crtc->smmu_state;
 
-	if (!cstate->num_mixers) {
-		_dpu_crtc_setup_mixers(crtc);
-		_dpu_crtc_setup_lm_bounds(crtc, crtc->state);
-	}
+	_dpu_crtc_setup_lm_bounds(crtc, crtc->state);
 
 	if (dpu_crtc->event) {
 		WARN_ON(dpu_crtc->event);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0a19124..f648e7f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -962,9 +962,12 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 	struct list_head *connector_list;
 	struct drm_connector *conn = NULL, *conn_iter;
 	struct drm_crtc *drm_crtc;
-	struct dpu_rm_hw_iter pp_iter, ctl_iter;
+	struct dpu_crtc_state *cstate;
+	struct dpu_rm_hw_iter hw_iter;
 	struct msm_display_topology topology;
 	struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
+	struct dpu_hw_mixer *hw_lm[MAX_CHANNELS_PER_ENC] = { NULL };
+	int num_lm = 0, num_ctl = 0;
 	int i = 0, ret;
 
 	if (!drm_enc) {
@@ -1008,21 +1011,41 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 		return;
 	}
 
-	dpu_rm_init_hw_iter(&pp_iter, drm_enc->base.id, DPU_HW_BLK_PINGPONG);
+	dpu_rm_init_hw_iter(&hw_iter, drm_enc->base.id, DPU_HW_BLK_PINGPONG);
 	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
 		dpu_enc->hw_pp[i] = NULL;
-		if (!dpu_rm_get_hw(&dpu_kms->rm, &pp_iter))
+		if (!dpu_rm_get_hw(&dpu_kms->rm, &hw_iter))
 			break;
-		dpu_enc->hw_pp[i] = (struct dpu_hw_pingpong *) pp_iter.hw;
+		dpu_enc->hw_pp[i] = (struct dpu_hw_pingpong *) hw_iter.hw;
 	}
 
-	dpu_rm_init_hw_iter(&ctl_iter, drm_enc->base.id, DPU_HW_BLK_CTL);
+	dpu_rm_init_hw_iter(&hw_iter, drm_enc->base.id, DPU_HW_BLK_CTL);
 	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
-		if (!dpu_rm_get_hw(&dpu_kms->rm, &ctl_iter))
+		if (!dpu_rm_get_hw(&dpu_kms->rm, &hw_iter))
 			break;
-		hw_ctl[i] = (struct dpu_hw_ctl *)ctl_iter.hw;
+		hw_ctl[i] = (struct dpu_hw_ctl *)hw_iter.hw;
+		num_ctl++;
 	}
 
+	dpu_rm_init_hw_iter(&hw_iter, drm_enc->base.id, DPU_HW_BLK_LM);
+	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
+		if (!dpu_rm_get_hw(&dpu_kms->rm, &hw_iter))
+			break;
+		hw_lm[i] = (struct dpu_hw_mixer *)hw_iter.hw;
+		num_lm++;
+	}
+
+	cstate = to_dpu_crtc_state(drm_crtc->state);
+
+	for (i = 0; i < num_lm; i++) {
+		int ctl_idx = (i < num_ctl) ? i : (num_ctl-1);
+
+		cstate->mixers[i].hw_lm = hw_lm[i];
+		cstate->mixers[i].lm_ctl = hw_ctl[ctl_idx];
+	}
+
+	cstate->num_mixers = num_lm;
+
 	for (i = 0; i < dpu_enc->num_phys_encs; i++) {
 		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 5/7] drm/msm/dpu: map mixer/ctl hw blocks in encoder modeset

[Sean Paul]

On Wed, Feb 13, 2019 at 05:19:14PM -0800, Jeykumar Sankaran wrote:
> After resource allocation, iterate and populate mixer/ctl
> hw blocks in encoder modeset thereby centralizing all
> the resource mapping to the CRTC. This change is made
> for easy switching to state based allocation using
> private objects later in this series.
> 
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>

Reviewed-by: Sean Paul <sean@poorly.run>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 64 +----------------------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 37 +++++++++++++----
>  2 files changed, 31 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 4e4b648..6d30ba9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -425,65 +425,6 @@ void dpu_crtc_complete_commit(struct drm_crtc *crtc,
>  	trace_dpu_crtc_complete_commit(DRMID(crtc));
>  }
>  
> -static void _dpu_crtc_setup_mixer_for_encoder(
> -		struct drm_crtc *crtc,
> -		struct drm_encoder *enc)
> -{
> -	struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
> -	struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
> -	struct dpu_rm *rm = &dpu_kms->rm;
> -	struct dpu_crtc_mixer *mixer;
> -	struct dpu_hw_ctl *last_valid_ctl = NULL;
> -	int i;
> -	struct dpu_rm_hw_iter lm_iter, ctl_iter;
> -
> -	dpu_rm_init_hw_iter(&lm_iter, enc->base.id, DPU_HW_BLK_LM);
> -	dpu_rm_init_hw_iter(&ctl_iter, enc->base.id, DPU_HW_BLK_CTL);
> -
> -	/* Set up all the mixers and ctls reserved by this encoder */
> -	for (i = cstate->num_mixers; i < ARRAY_SIZE(cstate->mixers); i++) {
> -		mixer = &cstate->mixers[i];
> -
> -		if (!dpu_rm_get_hw(rm, &lm_iter))
> -			break;
> -		mixer->hw_lm = (struct dpu_hw_mixer *)lm_iter.hw;
> -
> -		/* CTL may be <= LMs, if <, multiple LMs controlled by 1 CTL */
> -		if (!dpu_rm_get_hw(rm, &ctl_iter)) {
> -			DPU_DEBUG("no ctl assigned to lm %d, using previous\n",
> -					mixer->hw_lm->idx - LM_0);
> -			mixer->lm_ctl = last_valid_ctl;
> -		} else {
> -			mixer->lm_ctl = (struct dpu_hw_ctl *)ctl_iter.hw;
> -			last_valid_ctl = mixer->lm_ctl;
> -		}
> -
> -		/* Shouldn't happen, mixers are always >= ctls */
> -		if (!mixer->lm_ctl) {
> -			DPU_ERROR("no valid ctls found for lm %d\n",
> -					mixer->hw_lm->idx - LM_0);
> -			return;
> -		}
> -
> -		cstate->num_mixers++;
> -		DPU_DEBUG("setup mixer %d: lm %d\n",
> -				i, mixer->hw_lm->idx - LM_0);
> -		DPU_DEBUG("setup mixer %d: ctl %d\n",
> -				i, mixer->lm_ctl->idx - CTL_0);
> -	}
> -}
> -
> -static void _dpu_crtc_setup_mixers(struct drm_crtc *crtc)
> -{
> -	struct drm_encoder *enc;
> -
> -	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
> -
> -	/* Check for mixers on all encoders attached to this crtc */
> -	drm_for_each_encoder_mask(enc, crtc->dev, crtc->state->encoder_mask)
> -		_dpu_crtc_setup_mixer_for_encoder(crtc, enc);
> -}
> -
>  static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc,
>  		struct drm_crtc_state *state)
>  {
> @@ -533,10 +474,7 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc,
>  	dev = crtc->dev;
>  	smmu_state = &dpu_crtc->smmu_state;
>  
> -	if (!cstate->num_mixers) {
> -		_dpu_crtc_setup_mixers(crtc);
> -		_dpu_crtc_setup_lm_bounds(crtc, crtc->state);
> -	}
> +	_dpu_crtc_setup_lm_bounds(crtc, crtc->state);
>  
>  	if (dpu_crtc->event) {
>  		WARN_ON(dpu_crtc->event);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 0a19124..f648e7f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -962,9 +962,12 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>  	struct list_head *connector_list;
>  	struct drm_connector *conn = NULL, *conn_iter;
>  	struct drm_crtc *drm_crtc;
> -	struct dpu_rm_hw_iter pp_iter, ctl_iter;
> +	struct dpu_crtc_state *cstate;
> +	struct dpu_rm_hw_iter hw_iter;
>  	struct msm_display_topology topology;
>  	struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
> +	struct dpu_hw_mixer *hw_lm[MAX_CHANNELS_PER_ENC] = { NULL };
> +	int num_lm = 0, num_ctl = 0;
>  	int i = 0, ret;
>  
>  	if (!drm_enc) {
> @@ -1008,21 +1011,41 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>  		return;
>  	}
>  
> -	dpu_rm_init_hw_iter(&pp_iter, drm_enc->base.id, DPU_HW_BLK_PINGPONG);
> +	dpu_rm_init_hw_iter(&hw_iter, drm_enc->base.id, DPU_HW_BLK_PINGPONG);
>  	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
>  		dpu_enc->hw_pp[i] = NULL;
> -		if (!dpu_rm_get_hw(&dpu_kms->rm, &pp_iter))
> +		if (!dpu_rm_get_hw(&dpu_kms->rm, &hw_iter))
>  			break;
> -		dpu_enc->hw_pp[i] = (struct dpu_hw_pingpong *) pp_iter.hw;
> +		dpu_enc->hw_pp[i] = (struct dpu_hw_pingpong *) hw_iter.hw;
>  	}
>  
> -	dpu_rm_init_hw_iter(&ctl_iter, drm_enc->base.id, DPU_HW_BLK_CTL);
> +	dpu_rm_init_hw_iter(&hw_iter, drm_enc->base.id, DPU_HW_BLK_CTL);
>  	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
> -		if (!dpu_rm_get_hw(&dpu_kms->rm, &ctl_iter))
> +		if (!dpu_rm_get_hw(&dpu_kms->rm, &hw_iter))
>  			break;
> -		hw_ctl[i] = (struct dpu_hw_ctl *)ctl_iter.hw;
> +		hw_ctl[i] = (struct dpu_hw_ctl *)hw_iter.hw;
> +		num_ctl++;
>  	}
>  
> +	dpu_rm_init_hw_iter(&hw_iter, drm_enc->base.id, DPU_HW_BLK_LM);
> +	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
> +		if (!dpu_rm_get_hw(&dpu_kms->rm, &hw_iter))
> +			break;
> +		hw_lm[i] = (struct dpu_hw_mixer *)hw_iter.hw;
> +		num_lm++;
> +	}
> +
> +	cstate = to_dpu_crtc_state(drm_crtc->state);
> +
> +	for (i = 0; i < num_lm; i++) {
> +		int ctl_idx = (i < num_ctl) ? i : (num_ctl-1);
> +
> +		cstate->mixers[i].hw_lm = hw_lm[i];
> +		cstate->mixers[i].lm_ctl = hw_ctl[ctl_idx];
> +	}
> +
> +	cstate->num_mixers = num_lm;
> +
>  	for (i = 0; i < dpu_enc->num_phys_encs; i++) {
>  		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
>  
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 6/7] drm/msm/dpu: assign intf to encoder in mode_set

[Jeykumar Sankaran]

Iterate and assign HW intf block to physical encoders
in encoder modeset. Moving all the HW block assignments
to encoder modeset to allow easy switching to state
based resource management.

Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        | 22 +++++++++++++++++++-
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   | 24 ----------------------
 2 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index f648e7f..98ea478 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -968,7 +968,7 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 	struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
 	struct dpu_hw_mixer *hw_lm[MAX_CHANNELS_PER_ENC] = { NULL };
 	int num_lm = 0, num_ctl = 0;
-	int i = 0, ret;
+	int i, j, ret;
 
 	if (!drm_enc) {
 		DPU_ERROR("invalid encoder\n");
@@ -1065,6 +1065,26 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 			phys->hw_pp = dpu_enc->hw_pp[i];
 			phys->hw_ctl = hw_ctl[i];
 
+			dpu_rm_init_hw_iter(&hw_iter, drm_enc->base.id,
+					    DPU_HW_BLK_INTF);
+			for (j = 0; j < MAX_CHANNELS_PER_ENC; j++) {
+				struct dpu_hw_intf *hw_intf;
+
+				if (!dpu_rm_get_hw(&dpu_kms->rm, &hw_iter))
+					break;
+
+				hw_intf = (struct dpu_hw_intf *)hw_iter.hw;
+				if (hw_intf->idx == phys->intf_idx)
+					phys->hw_intf = hw_intf;
+			}
+
+			if (!phys->hw_intf) {
+				DPU_ERROR_ENC(dpu_enc,
+					      "no intf block assigned at idx: %d\n",
+					      i);
+				goto error;
+			}
+
 			phys->connector = conn->state->connector;
 			if (phys->ops.mode_set)
 				phys->ops.mode_set(phys, mode, adj_mode);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index ce65521..02362c5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -449,35 +449,11 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
 
 static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
 {
-	struct msm_drm_private *priv;
-	struct dpu_rm_hw_iter iter;
 	struct dpu_hw_ctl *ctl;
 	u32 flush_mask = 0;
 
-	if (!phys_enc || !phys_enc->parent || !phys_enc->parent->dev ||
-			!phys_enc->parent->dev->dev_private) {
-		DPU_ERROR("invalid encoder/device\n");
-		return;
-	}
-	priv = phys_enc->parent->dev->dev_private;
-
 	ctl = phys_enc->hw_ctl;
 
-	dpu_rm_init_hw_iter(&iter, phys_enc->parent->base.id, DPU_HW_BLK_INTF);
-	while (dpu_rm_get_hw(&phys_enc->dpu_kms->rm, &iter)) {
-		struct dpu_hw_intf *hw_intf = (struct dpu_hw_intf *)iter.hw;
-
-		if (hw_intf->idx == phys_enc->intf_idx) {
-			phys_enc->hw_intf = hw_intf;
-			break;
-		}
-	}
-
-	if (!phys_enc->hw_intf) {
-		DPU_ERROR("hw_intf not assigned\n");
-		return;
-	}
-
 	DPU_DEBUG_VIDENC(phys_enc, "\n");
 
 	if (WARN_ON(!phys_enc->hw_intf->ops.enable_timing))
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 6/7] drm/msm/dpu: assign intf to encoder in mode_set

[Sean Paul]

On Wed, Feb 13, 2019 at 05:19:15PM -0800, Jeykumar Sankaran wrote:
> Iterate and assign HW intf block to physical encoders
> in encoder modeset. Moving all the HW block assignments
> to encoder modeset to allow easy switching to state
> based resource management.
> 
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>

Reviewed-by: Sean Paul <sean@poorly.run>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        | 22 +++++++++++++++++++-
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   | 24 ----------------------
>  2 files changed, 21 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index f648e7f..98ea478 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -968,7 +968,7 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>  	struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
>  	struct dpu_hw_mixer *hw_lm[MAX_CHANNELS_PER_ENC] = { NULL };
>  	int num_lm = 0, num_ctl = 0;
> -	int i = 0, ret;
> +	int i, j, ret;
>  
>  	if (!drm_enc) {
>  		DPU_ERROR("invalid encoder\n");
> @@ -1065,6 +1065,26 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>  			phys->hw_pp = dpu_enc->hw_pp[i];
>  			phys->hw_ctl = hw_ctl[i];
>  
> +			dpu_rm_init_hw_iter(&hw_iter, drm_enc->base.id,
> +					    DPU_HW_BLK_INTF);
> +			for (j = 0; j < MAX_CHANNELS_PER_ENC; j++) {
> +				struct dpu_hw_intf *hw_intf;
> +
> +				if (!dpu_rm_get_hw(&dpu_kms->rm, &hw_iter))
> +					break;
> +
> +				hw_intf = (struct dpu_hw_intf *)hw_iter.hw;
> +				if (hw_intf->idx == phys->intf_idx)
> +					phys->hw_intf = hw_intf;
> +			}
> +
> +			if (!phys->hw_intf) {
> +				DPU_ERROR_ENC(dpu_enc,
> +					      "no intf block assigned at idx: %d\n",
> +					      i);
> +				goto error;
> +			}
> +
>  			phys->connector = conn->state->connector;
>  			if (phys->ops.mode_set)
>  				phys->ops.mode_set(phys, mode, adj_mode);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index ce65521..02362c5 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -449,35 +449,11 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
>  
>  static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
>  {
> -	struct msm_drm_private *priv;
> -	struct dpu_rm_hw_iter iter;
>  	struct dpu_hw_ctl *ctl;
>  	u32 flush_mask = 0;
>  
> -	if (!phys_enc || !phys_enc->parent || !phys_enc->parent->dev ||
> -			!phys_enc->parent->dev->dev_private) {
> -		DPU_ERROR("invalid encoder/device\n");
> -		return;
> -	}
> -	priv = phys_enc->parent->dev->dev_private;
> -
>  	ctl = phys_enc->hw_ctl;
>  
> -	dpu_rm_init_hw_iter(&iter, phys_enc->parent->base.id, DPU_HW_BLK_INTF);
> -	while (dpu_rm_get_hw(&phys_enc->dpu_kms->rm, &iter)) {
> -		struct dpu_hw_intf *hw_intf = (struct dpu_hw_intf *)iter.hw;
> -
> -		if (hw_intf->idx == phys_enc->intf_idx) {
> -			phys_enc->hw_intf = hw_intf;
> -			break;
> -		}
> -	}
> -
> -	if (!phys_enc->hw_intf) {
> -		DPU_ERROR("hw_intf not assigned\n");
> -		return;
> -	}
> -
>  	DPU_DEBUG_VIDENC(phys_enc, "\n");
>  
>  	if (WARN_ON(!phys_enc->hw_intf->ops.enable_timing))
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 7/7] drm/msm/dpu: check split role for single flush

[Jeykumar Sankaran]

Removing unwanted access of crtc_state for finding this information.
Use split role information to know whether we have slave ctl.

Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index 02362c5..3fc3cc0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -347,22 +347,10 @@ static void dpu_encoder_phys_vid_underrun_irq(void *arg, int irq_idx)
 			phys_enc);
 }
 
-static bool _dpu_encoder_phys_is_dual_ctl(struct dpu_encoder_phys *phys_enc)
-{
-	struct dpu_crtc_state *dpu_cstate;
-
-	if (!phys_enc)
-		return false;
-
-	dpu_cstate = to_dpu_crtc_state(phys_enc->parent->crtc->state);
-
-	return dpu_cstate->num_ctls > 1;
-}
-
 static bool dpu_encoder_phys_vid_needs_single_flush(
 		struct dpu_encoder_phys *phys_enc)
 {
-	return (phys_enc && _dpu_encoder_phys_is_dual_ctl(phys_enc));
+	return phys_enc->split_role != ENC_ROLE_SOLO;
 }
 
 static void _dpu_encoder_phys_vid_setup_irq_hw_idx(
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 7/7] drm/msm/dpu: check split role for single flush

[Sean Paul]

On Wed, Feb 13, 2019 at 05:19:16PM -0800, Jeykumar Sankaran wrote:
> Removing unwanted access of crtc_state for finding this information.
> Use split role information to know whether we have slave ctl.
> 
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>

Reviewed-by: Sean Paul <sean@poorly.run>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index 02362c5..3fc3cc0 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -347,22 +347,10 @@ static void dpu_encoder_phys_vid_underrun_irq(void *arg, int irq_idx)
>  			phys_enc);
>  }
>  
> -static bool _dpu_encoder_phys_is_dual_ctl(struct dpu_encoder_phys *phys_enc)
> -{
> -	struct dpu_crtc_state *dpu_cstate;
> -
> -	if (!phys_enc)
> -		return false;
> -
> -	dpu_cstate = to_dpu_crtc_state(phys_enc->parent->crtc->state);
> -
> -	return dpu_cstate->num_ctls > 1;
> -}
> -
>  static bool dpu_encoder_phys_vid_needs_single_flush(
>  		struct dpu_encoder_phys *phys_enc)
>  {
> -	return (phys_enc && _dpu_encoder_phys_is_dual_ctl(phys_enc));
> +	return phys_enc->split_role != ENC_ROLE_SOLO;
>  }
>  
>  static void _dpu_encoder_phys_vid_setup_irq_hw_idx(
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 0/7] move dpu resource parsing to encoder modeset

[Jeykumar Sankaran]

On 2019-02-13 17:19, Jeykumar Sankaran wrote:
> Fixing some of the low hanging fruits by moving the hw resource
> parsing and assignment to encoder modeset. This series
> prepares DPU resource management to switch to state based
> resource tracking which is implemented in the next incoming
> changes.
> 

Below changes are introduced in the series. V2 label is to indicate
its relation to the RM resource series posted quite a while back:
https://patchwork.freedesktop.org/series/50722/

> Thanks.
> 
> Jeykumar Sankaran (7):
>   drm/msm/dpu: move hw_inf encoder baseclass
>   drm/msm/dpu: remove phys_vid subclass
>   drm/msm/dpu: release resources on modeset failure
>   drm/msm/dpu: dont use encoder->crtc in atomic path
>   drm/msm/dpu: map mixer/ctl hw blocks in encoder modeset
>   drm/msm/dpu: assign intf to encoder in mode_set
>   drm/msm/dpu: check split role for single flush
> 
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c           |  64 +-------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        |  73 +++++++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h   |  15 +-
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   | 177
> ++++++---------------
>  4 files changed, 118 insertions(+), 211 deletions(-)

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 0/7] move dpu resource parsing to encoder modeset

[Sean Paul]

On Wed, Feb 13, 2019 at 05:19:09PM -0800, Jeykumar Sankaran wrote:
> Fixing some of the low hanging fruits by moving the hw resource
> parsing and assignment to encoder modeset. This series 
> prepares DPU resource management to switch to state based
> resource tracking which is implemented in the next incoming
> changes.

Thanks for the set, Jeykumar. I've applied the whole thing to msm-next.

Sean

> 
> Thanks.
> 
> Jeykumar Sankaran (7):
>   drm/msm/dpu: move hw_inf encoder baseclass
>   drm/msm/dpu: remove phys_vid subclass
>   drm/msm/dpu: release resources on modeset failure
>   drm/msm/dpu: dont use encoder->crtc in atomic path
>   drm/msm/dpu: map mixer/ctl hw blocks in encoder modeset
>   drm/msm/dpu: assign intf to encoder in mode_set
>   drm/msm/dpu: check split role for single flush
> 
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c           |  64 +-------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        |  73 +++++++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h   |  15 +-
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   | 177 ++++++---------------
>  4 files changed, 118 insertions(+), 211 deletions(-)
> 
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno