[PATCH v3 00/13] Atomic resource management

[PATCH v3 00/13] Atomic resource management

[Jeykumar Sankaran]

This patchset series introduces drm private object in KMS to manage HW resource
management. It modifies the resource manager by introducing API's to do per DRM
object resource allocation/cleanups.

Patches 00/13 to 11/13 are clean up patches to prepare DPU for the above
migration.

major changes in v2:
	- Fix return values in kms (Jordan)
	- Split irrelevant changes from master patch
          into separate patches (Sean)

changes in v3:
	- Rebase on [1]
	- Fix control path bug in split LM topology

[1] https://gitlab.freedesktop.org/seanpaul/dpu-staging/commits/for-next

Jeykumar Sankaran (13):
  drm/msm/dpu: remove scalar config definitions
  drm/msm/dpu: remove resource pool manager
  drm/msm/dpu: remove ping pong split topology variables
  drm/msm/dpu: program master-slave encoders explicitly
  drm/msm/dpu: use kms stored hw mdp block
  drm/msm/dpu: iterate for assigned hw ctl in virtual encoder
  drm/msm/dpu: avoid querying for hw intf before assignment
  drm/msm/dpu: move hw resource tracking to crtc state
  drm/msm/dpu: rename hw_ctl to lm_ctl
  drm/msm/dpu: remove topology name
  drm/msm/dpu: remove display H_TILE from encoder
  drm/msm/dpu: add atomic private object to dpu kms
  drm/msm/dpu: use private obj to track hw resources

 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c           | 427 +++--------
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h           | 120 ++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        | 162 +++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h        |   4 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h   |   9 +-
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c   |  31 +-
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   |  88 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h        |  10 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c            |  80 ++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h            |  23 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c             | 796 ++++++---------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h             | 149 ++--
 12 files changed, 612 insertions(+), 1287 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 v3 01/13] drm/msm/dpu: remove scalar config definitions

[Jeykumar Sankaran]

cleans up left out scalar config definitions from headers

changes in v2:
	- none
changes in v3:
	- none

Change-Id: Id824dd5075c666f97b964573c97215bb786eac75
Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h    |  2 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 10 ----------
 2 files changed, 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index e87109e..0e9aafa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -164,7 +164,6 @@ struct dpu_crtc_frame_event {
  * @cur_perf      : current performance committed to clock/bandwidth driver
  * @rp_lock       : serialization lock for resource pool
  * @rp_head       : list of active resource pool
- * @scl3_cfg_lut  : qseed3 lut config
  */
 struct dpu_crtc {
 	struct drm_crtc base;
@@ -175,7 +174,6 @@ struct dpu_crtc {
 	u32 num_mixers;
 	bool mixers_swapped;
 	struct dpu_crtc_mixer mixers[CRTC_DUAL_MIXERS];
-	struct dpu_hw_scaler3_lut_cfg *scl3_lut_cfg;
 
 	struct drm_pending_vblank_event *event;
 	u32 vsync_count;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
index 1240f50..c5c8f60 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
@@ -148,16 +148,6 @@ struct dpu_hw_scaler3_cfg {
 	struct dpu_hw_scaler3_de_cfg de;
 };
 
-struct dpu_hw_scaler3_lut_cfg {
-	bool is_configured;
-	u32 *dir_lut;
-	size_t dir_len;
-	u32 *cir_lut;
-	size_t cir_len;
-	u32 *sep_lut;
-	size_t sep_len;
-};
-
 /**
  * struct dpu_drm_pix_ext_v1 - version 1 of pixel ext structure
  * @num_ext_pxls_lr: Number of total horizontal pixels
-- 
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

[PATCH v3 02/13] drm/msm/dpu: remove resource pool manager

[Jeykumar Sankaran]

resource pool manager utility was introduced to manage
rotator sessions. Removing the support as the rotator
feature doesn't exist.

changes in v2:
	- none
changes in v3:
	- rebase on [1]

[1] https://gitlab.freedesktop.org/seanpaul/dpu-staging/commits/for-next

Change-Id: Ib045f1c66269be650bce5896c459f59e1047a53f
Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 205 -------------------------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h |  56 ---------
 2 files changed, 261 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 80cbf75..1f2d223 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -99,187 +99,6 @@ static inline int _dpu_crtc_power_enable(struct dpu_crtc *dpu_crtc, bool enable)
 	return 0;
 }
 
-/**
- * _dpu_crtc_rp_to_crtc - get crtc from resource pool object
- * @rp: Pointer to resource pool
- * return: Pointer to drm crtc if success; null otherwise
- */
-static struct drm_crtc *_dpu_crtc_rp_to_crtc(struct dpu_crtc_respool *rp)
-{
-	if (!rp)
-		return NULL;
-
-	return container_of(rp, struct dpu_crtc_state, rp)->base.crtc;
-}
-
-/**
- * _dpu_crtc_rp_reclaim - reclaim unused, or all if forced, resources in pool
- * @rp: Pointer to resource pool
- * @force: True to reclaim all resources; otherwise, reclaim only unused ones
- * return: None
- */
-static void _dpu_crtc_rp_reclaim(struct dpu_crtc_respool *rp, bool force)
-{
-	struct dpu_crtc_res *res, *next;
-	struct drm_crtc *crtc;
-
-	crtc = _dpu_crtc_rp_to_crtc(rp);
-	if (!crtc) {
-		DPU_ERROR("invalid crtc\n");
-		return;
-	}
-
-	DPU_DEBUG("crtc%d.%u %s\n", crtc->base.id, rp->sequence_id,
-			force ? "destroy" : "free_unused");
-
-	list_for_each_entry_safe(res, next, &rp->res_list, list) {
-		if (!force && !(res->flags & DPU_CRTC_RES_FLAG_FREE))
-			continue;
-		DPU_DEBUG("crtc%d.%u reclaim res:0x%x/0x%llx/%pK/%d\n",
-				crtc->base.id, rp->sequence_id,
-				res->type, res->tag, res->val,
-				atomic_read(&res->refcount));
-		list_del(&res->list);
-		if (res->ops.put)
-			res->ops.put(res->val);
-		kfree(res);
-	}
-}
-
-/**
- * _dpu_crtc_rp_free_unused - free unused resource in pool
- * @rp: Pointer to resource pool
- * return: none
- */
-static void _dpu_crtc_rp_free_unused(struct dpu_crtc_respool *rp)
-{
-	mutex_lock(rp->rp_lock);
-	_dpu_crtc_rp_reclaim(rp, false);
-	mutex_unlock(rp->rp_lock);
-}
-
-/**
- * _dpu_crtc_rp_destroy - destroy resource pool
- * @rp: Pointer to resource pool
- * return: None
- */
-static void _dpu_crtc_rp_destroy(struct dpu_crtc_respool *rp)
-{
-	mutex_lock(rp->rp_lock);
-	list_del_init(&rp->rp_list);
-	_dpu_crtc_rp_reclaim(rp, true);
-	mutex_unlock(rp->rp_lock);
-}
-
-/**
- * _dpu_crtc_hw_blk_get - get callback for hardware block
- * @val: Resource handle
- * @type: Resource type
- * @tag: Search tag for given resource
- * return: Resource handle
- */
-static void *_dpu_crtc_hw_blk_get(void *val, u32 type, u64 tag)
-{
-	DPU_DEBUG("res:%d/0x%llx/%pK\n", type, tag, val);
-	return dpu_hw_blk_get(val, type, tag);
-}
-
-/**
- * _dpu_crtc_hw_blk_put - put callback for hardware block
- * @val: Resource handle
- * return: None
- */
-static void _dpu_crtc_hw_blk_put(void *val)
-{
-	DPU_DEBUG("res://%pK\n", val);
-	dpu_hw_blk_put(val);
-}
-
-/**
- * _dpu_crtc_rp_duplicate - duplicate resource pool and reset reference count
- * @rp: Pointer to original resource pool
- * @dup_rp: Pointer to duplicated resource pool
- * return: None
- */
-static void _dpu_crtc_rp_duplicate(struct dpu_crtc_respool *rp,
-		struct dpu_crtc_respool *dup_rp)
-{
-	struct dpu_crtc_res *res, *dup_res;
-	struct drm_crtc *crtc;
-
-	if (!rp || !dup_rp || !rp->rp_head) {
-		DPU_ERROR("invalid resource pool\n");
-		return;
-	}
-
-	crtc = _dpu_crtc_rp_to_crtc(rp);
-	if (!crtc) {
-		DPU_ERROR("invalid crtc\n");
-		return;
-	}
-
-	DPU_DEBUG("crtc%d.%u duplicate\n", crtc->base.id, rp->sequence_id);
-
-	mutex_lock(rp->rp_lock);
-	dup_rp->sequence_id = rp->sequence_id + 1;
-	INIT_LIST_HEAD(&dup_rp->res_list);
-	dup_rp->ops = rp->ops;
-	list_for_each_entry(res, &rp->res_list, list) {
-		dup_res = kzalloc(sizeof(struct dpu_crtc_res), GFP_KERNEL);
-		if (!dup_res) {
-			mutex_unlock(rp->rp_lock);
-			return;
-		}
-		INIT_LIST_HEAD(&dup_res->list);
-		atomic_set(&dup_res->refcount, 0);
-		dup_res->type = res->type;
-		dup_res->tag = res->tag;
-		dup_res->val = res->val;
-		dup_res->ops = res->ops;
-		dup_res->flags = DPU_CRTC_RES_FLAG_FREE;
-		DPU_DEBUG("crtc%d.%u dup res:0x%x/0x%llx/%pK/%d\n",
-				crtc->base.id, dup_rp->sequence_id,
-				dup_res->type, dup_res->tag, dup_res->val,
-				atomic_read(&dup_res->refcount));
-		list_add_tail(&dup_res->list, &dup_rp->res_list);
-		if (dup_res->ops.get)
-			dup_res->ops.get(dup_res->val, 0, -1);
-	}
-
-	dup_rp->rp_lock = rp->rp_lock;
-	dup_rp->rp_head = rp->rp_head;
-	INIT_LIST_HEAD(&dup_rp->rp_list);
-	list_add_tail(&dup_rp->rp_list, rp->rp_head);
-	mutex_unlock(rp->rp_lock);
-}
-
-/**
- * _dpu_crtc_rp_reset - reset resource pool after allocation
- * @rp: Pointer to original resource pool
- * @rp_lock: Pointer to serialization resource pool lock
- * @rp_head: Pointer to crtc resource pool head
- * return: None
- */
-static void _dpu_crtc_rp_reset(struct dpu_crtc_respool *rp,
-		struct mutex *rp_lock, struct list_head *rp_head)
-{
-	if (!rp || !rp_lock || !rp_head) {
-		DPU_ERROR("invalid resource pool\n");
-		return;
-	}
-
-	mutex_lock(rp_lock);
-	rp->rp_lock = rp_lock;
-	rp->rp_head = rp_head;
-	INIT_LIST_HEAD(&rp->rp_list);
-	rp->sequence_id = 0;
-	INIT_LIST_HEAD(&rp->res_list);
-	rp->ops.get = _dpu_crtc_hw_blk_get;
-	rp->ops.put = _dpu_crtc_hw_blk_put;
-	list_add_tail(&rp->rp_list, rp->rp_head);
-	mutex_unlock(rp_lock);
-}
-
 static void dpu_crtc_destroy(struct drm_crtc *crtc)
 {
 	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
@@ -951,8 +770,6 @@ static void dpu_crtc_destroy_state(struct drm_crtc *crtc,
 
 	DPU_DEBUG("crtc%d\n", crtc->base.id);
 
-	_dpu_crtc_rp_destroy(&cstate->rp);
-
 	__drm_atomic_helper_crtc_destroy_state(state);
 
 	kfree(cstate);
@@ -1206,8 +1023,6 @@ static struct drm_crtc_state *dpu_crtc_duplicate_state(struct drm_crtc *crtc)
 	/* duplicate base helper */
 	__drm_atomic_helper_crtc_duplicate_state(crtc, &cstate->base);
 
-	_dpu_crtc_rp_duplicate(&old_cstate->rp, &cstate->rp);
-
 	return &cstate->base;
 }
 
@@ -1244,9 +1059,6 @@ static void dpu_crtc_reset(struct drm_crtc *crtc)
 		return;
 	}
 
-	_dpu_crtc_rp_reset(&cstate->rp, &dpu_crtc->rp_lock,
-			&dpu_crtc->rp_head);
-
 	cstate->base.crtc = crtc;
 	crtc->state = &cstate->base;
 }
@@ -1679,7 +1491,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
 	}
 
 end:
-	_dpu_crtc_rp_free_unused(&cstate->rp);
 	kfree(pstates);
 	return rc;
 }
@@ -1955,8 +1766,6 @@ static int dpu_crtc_debugfs_state_show(struct seq_file *s, void *v)
 {
 	struct drm_crtc *crtc = (struct drm_crtc *) s->private;
 	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
-	struct dpu_crtc_res *res;
-	struct dpu_crtc_respool *rp;
 	int i;
 
 	seq_printf(s, "client type: %d\n", dpu_crtc_get_client_type(crtc));
@@ -1973,17 +1782,6 @@ static int dpu_crtc_debugfs_state_show(struct seq_file *s, void *v)
 				dpu_crtc->cur_perf.max_per_pipe_ib[i]);
 	}
 
-	mutex_lock(&dpu_crtc->rp_lock);
-	list_for_each_entry(rp, &dpu_crtc->rp_head, rp_list) {
-		seq_printf(s, "rp.%d: ", rp->sequence_id);
-		list_for_each_entry(res, &rp->res_list, list)
-			seq_printf(s, "0x%x/0x%llx/%pK/%d ",
-					res->type, res->tag, res->val,
-					atomic_read(&res->refcount));
-		seq_puts(s, "\n");
-	}
-	mutex_unlock(&dpu_crtc->rp_lock);
-
 	return 0;
 }
 DEFINE_DPU_DEBUGFS_SEQ_FOPS(dpu_crtc_debugfs_state);
@@ -2104,9 +1902,6 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane)
 	spin_lock_init(&dpu_crtc->spin_lock);
 	atomic_set(&dpu_crtc->frame_pending, 0);
 
-	mutex_init(&dpu_crtc->rp_lock);
-	INIT_LIST_HEAD(&dpu_crtc->rp_head);
-
 	init_completion(&dpu_crtc->frame_done_comp);
 
 	INIT_LIST_HEAD(&dpu_crtc->frame_event_list);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 0e9aafa..e84da78 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -162,8 +162,6 @@ struct dpu_crtc_frame_event {
  * @phandle: Pointer to power handler
  * @power_event   : registered power event handle
  * @cur_perf      : current performance committed to clock/bandwidth driver
- * @rp_lock       : serialization lock for resource pool
- * @rp_head       : list of active resource pool
  */
 struct dpu_crtc {
 	struct drm_crtc base;
@@ -213,65 +211,12 @@ struct dpu_crtc {
 
 	struct dpu_core_perf_params cur_perf;
 
-	struct mutex rp_lock;
-	struct list_head rp_head;
-
 	struct dpu_crtc_smmu_state_data smmu_state;
 };
 
 #define to_dpu_crtc(x) container_of(x, struct dpu_crtc, base)
 
 /**
- * struct dpu_crtc_res_ops - common operations for crtc resources
- * @get: get given resource
- * @put: put given resource
- */
-struct dpu_crtc_res_ops {
-	void *(*get)(void *val, u32 type, u64 tag);
-	void (*put)(void *val);
-};
-
-#define DPU_CRTC_RES_FLAG_FREE		BIT(0)
-
-/**
- * struct dpu_crtc_res - definition of crtc resources
- * @list: list of crtc resource
- * @type: crtc resource type
- * @tag: unique identifier per type
- * @refcount: reference/usage count
- * @ops: callback operations
- * @val: resource handle associated with type/tag
- * @flags: customization flags
- */
-struct dpu_crtc_res {
-	struct list_head list;
-	u32 type;
-	u64 tag;
-	atomic_t refcount;
-	struct dpu_crtc_res_ops ops;
-	void *val;
-	u32 flags;
-};
-
-/**
- * dpu_crtc_respool - crtc resource pool
- * @rp_lock: pointer to serialization lock
- * @rp_head: pointer to head of active resource pools of this crtc
- * @rp_list: list of crtc resource pool
- * @sequence_id: sequence identifier, incremented per state duplication
- * @res_list: list of resource managed by this resource pool
- * @ops: resource operations for parent resource pool
- */
-struct dpu_crtc_respool {
-	struct mutex *rp_lock;
-	struct list_head *rp_head;
-	struct list_head rp_list;
-	u32 sequence_id;
-	struct list_head res_list;
-	struct dpu_crtc_res_ops ops;
-};
-
-/**
  * struct dpu_crtc_state - dpu container for atomic crtc state
  * @base: Base drm crtc state structure
  * @is_ppsplit    : Whether current topology requires PPSplit special handling
@@ -296,7 +241,6 @@ struct dpu_crtc_state {
 	uint64_t input_fence_timeout_ns;
 
 	struct dpu_core_perf_params new_perf;
-	struct dpu_crtc_respool rp;
 };
 
 #define to_dpu_crtc_state(x) \
-- 
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

[PATCH v3 03/13] drm/msm/dpu: remove ping pong split topology variables

[Jeykumar Sankaran]

removes left out variables of previous ping pong
split topology cleanup.

changes in v2:
	- none
changes in v3:
	- none

Change-Id: I1bf9d242039ce7cfd271233fa27840e83184fb95
Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index e84da78..e632651 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -219,7 +219,6 @@ struct dpu_crtc {
 /**
  * struct dpu_crtc_state - dpu container for atomic crtc state
  * @base: Base drm crtc state structure
- * @is_ppsplit    : Whether current topology requires PPSplit special handling
  * @bw_control    : true if bw/clk controlled by core bw/clk properties
  * @bw_split_vote : true if bw controlled by llcc/dram bw properties
  * @lm_bounds     : LM boundaries based on current mode full resolution, no ROI.
@@ -234,8 +233,6 @@ struct dpu_crtc_state {
 
 	bool bw_control;
 	bool bw_split_vote;
-
-	bool is_ppsplit;
 	struct drm_rect lm_bounds[CRTC_DUAL_MIXERS];
 
 	uint64_t input_fence_timeout_ns;
-- 
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

[PATCH v3 04/13] drm/msm/dpu: program master-slave encoders explicitly

[Jeykumar Sankaran]

Identify slave-master encoders and program them explicitly.

changes in v2:
	- none
changes in v3:
	- none

Change-Id: I0ebfada05bd7f8437f842ad860490a678aa8f4cd
Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 39 ++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 1b4de34..a0ced79 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -184,6 +184,7 @@ struct dpu_encoder_virt {
 	unsigned int num_phys_encs;
 	struct dpu_encoder_phys *phys_encs[MAX_PHYS_ENCODERS_PER_VIRTUAL];
 	struct dpu_encoder_phys *cur_master;
+	struct dpu_encoder_phys *cur_slave;
 	struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
 
 	bool intfs_swapped;
@@ -1153,6 +1154,7 @@ void dpu_encoder_virt_restore(struct drm_encoder *drm_enc)
 static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
 {
 	struct dpu_encoder_virt *dpu_enc = NULL;
+	struct dpu_encoder_phys *phys  = NULL;
 	int i, ret = 0;
 	struct drm_display_mode *cur_mode = NULL;
 
@@ -1160,6 +1162,7 @@ static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
 		DPU_ERROR("invalid encoder\n");
 		return;
 	}
+
 	dpu_enc = to_dpu_encoder_virt(drm_enc);
 	cur_mode = &dpu_enc->base.crtc->state->adjusted_mode;
 
@@ -1167,21 +1170,36 @@ static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
 			     cur_mode->vdisplay);
 
 	dpu_enc->cur_master = NULL;
+	dpu_enc->cur_slave = NULL;
 	for (i = 0; i < dpu_enc->num_phys_encs; i++) {
-		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
+		phys = dpu_enc->phys_encs[i];
+
+		if (!phys || !phys->ops.is_master)
+			continue;
 
-		if (phys && phys->ops.is_master && phys->ops.is_master(phys)) {
-			DPU_DEBUG_ENC(dpu_enc, "master is now idx %d\n", i);
+		if (phys->ops.is_master(phys)) {
+			DPU_DEBUG_ENC(dpu_enc, "master is at idx %d\n", i);
 			dpu_enc->cur_master = phys;
-			break;
+		} else {
+			DPU_DEBUG_ENC(dpu_enc, "slave is at idx %d\n", i);
+			dpu_enc->cur_slave = phys;
 		}
 	}
 
 	if (!dpu_enc->cur_master) {
-		DPU_ERROR("virt encoder has no master! num_phys %d\n", i);
+		DPU_ERROR("virt encoder has no master identified\n");
 		return;
 	}
 
+	/* always enable slave encoder before master */
+	phys = dpu_enc->cur_slave;
+	if (phys && phys->ops.enable)
+		phys->ops.enable(phys);
+
+	phys = dpu_enc->cur_master;
+	if (phys && phys->ops.enable)
+		phys->ops.enable(phys);
+
 	ret = dpu_encoder_resource_control(drm_enc, DPU_ENC_RC_EVENT_KICKOFF);
 	if (ret) {
 		DPU_ERROR_ENC(dpu_enc, "dpu resource control failed: %d\n",
@@ -1190,25 +1208,16 @@ static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
 	}
 
 	for (i = 0; i < dpu_enc->num_phys_encs; i++) {
-		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
-
+		phys = dpu_enc->phys_encs[i];
 		if (!phys)
 			continue;
 
-		if (phys != dpu_enc->cur_master) {
-			if (phys->ops.enable)
-				phys->ops.enable(phys);
-		}
-
 		if (dpu_enc->misr_enable && (dpu_enc->disp_info.capabilities &
 		     MSM_DISPLAY_CAP_VID_MODE) && phys->ops.setup_misr)
 			phys->ops.setup_misr(phys, true,
 						dpu_enc->misr_frame_count);
 	}
 
-	if (dpu_enc->cur_master->ops.enable)
-		dpu_enc->cur_master->ops.enable(dpu_enc->cur_master);
-
 	_dpu_encoder_virt_enable_helper(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

[PATCH v3 05/13] drm/msm/dpu: use kms stored hw mdp block

[Jeykumar Sankaran]

Avoid querying RM for hw mdp block. Use the one
stored in KMS during initialization.

changes in v2:
	- none
changes in v3:
	- none

Change-Id: I52129b96bd561a5547507d7f567bcaa3dbe554aa
Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 12 +-----------
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c |  9 +--------
 2 files changed, 2 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index 3084675..c8c4612 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -823,7 +823,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
 {
 	struct dpu_encoder_phys *phys_enc = NULL;
 	struct dpu_encoder_phys_cmd *cmd_enc = NULL;
-	struct dpu_hw_mdp *hw_mdp;
 	struct dpu_encoder_irq *irq;
 	int i, ret = 0;
 
@@ -836,14 +835,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
 		goto fail;
 	}
 	phys_enc = &cmd_enc->base;
-
-	hw_mdp = dpu_rm_get_mdp(&p->dpu_kms->rm);
-	if (IS_ERR_OR_NULL(hw_mdp)) {
-		ret = PTR_ERR(hw_mdp);
-		DPU_ERROR("failed to get mdptop\n");
-		goto fail_mdp_init;
-	}
-	phys_enc->hw_mdptop = hw_mdp;
+	phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
 	phys_enc->intf_idx = p->intf_idx;
 
 	dpu_encoder_phys_cmd_init_ops(&phys_enc->ops);
@@ -898,8 +890,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
 
 	return phys_enc;
 
-fail_mdp_init:
-	kfree(cmd_enc);
 fail:
 	return ERR_PTR(ret);
 }
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 14fc7c2..57ece03 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
@@ -829,7 +829,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
 	struct dpu_encoder_phys *phys_enc = NULL;
 	struct dpu_encoder_phys_vid *vid_enc = NULL;
 	struct dpu_rm_hw_iter iter;
-	struct dpu_hw_mdp *hw_mdp;
 	struct dpu_encoder_irq *irq;
 	int i, ret = 0;
 
@@ -846,13 +845,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
 
 	phys_enc = &vid_enc->base;
 
-	hw_mdp = dpu_rm_get_mdp(&p->dpu_kms->rm);
-	if (IS_ERR_OR_NULL(hw_mdp)) {
-		ret = PTR_ERR(hw_mdp);
-		DPU_ERROR("failed to get mdptop\n");
-		goto fail;
-	}
-	phys_enc->hw_mdptop = hw_mdp;
+	phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
 	phys_enc->intf_idx = p->intf_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

[PATCH v3 06/13] drm/msm/dpu: iterate for assigned hw ctl in virtual encoder

[Jeykumar Sankaran]

Instead of iterating for hw ctrl per physical encoder, this
patch moves the iterations and assignment to the virtual encoder.

changes in v2:
	- none
changes in v3:
	- none

Change-Id: I896a8c36d6353986582e9d0fe3da9b2293579d4b
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_cmd.c   | 19 -------------------
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   | 19 -------------------
 3 files changed, 20 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index a0ced79..7b82e2d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1017,9 +1017,11 @@ 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 dpu_rm_hw_iter pp_iter;
+	struct dpu_rm_hw_iter pp_iter, ctl_iter;
 	struct msm_display_topology topology;
 	enum dpu_rm_topology_name topology_name;
+	struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC];
+
 	int i = 0, ret;
 
 	if (!drm_enc) {
@@ -1067,6 +1069,14 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 		dpu_enc->hw_pp[i] = (struct dpu_hw_pingpong *) pp_iter.hw;
 	}
 
+	dpu_rm_init_hw_iter(&ctl_iter, drm_enc->base.id, DPU_HW_BLK_CTL);
+	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
+		hw_ctl[i] = NULL;
+		if (!dpu_rm_get_hw(&dpu_kms->rm, &ctl_iter))
+			break;
+		hw_ctl[i] = (struct dpu_hw_ctl *)ctl_iter.hw;
+	}
+
 	topology_name = dpu_rm_get_topology_name(topology);
 	for (i = 0; i < dpu_enc->num_phys_encs; i++) {
 		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
@@ -1074,10 +1084,18 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 		if (phys) {
 			if (!dpu_enc->hw_pp[i]) {
 				DPU_ERROR_ENC(dpu_enc,
-				    "invalid pingpong block for the encoder\n");
+				    "no pp block assigned at idx: %d\n", i);
 				return;
 			}
 			phys->hw_pp = dpu_enc->hw_pp[i];
+
+			if (!hw_ctl[i]) {
+				DPU_ERROR_ENC(dpu_enc,
+				    "no ctl block assigned at idx: %d\n", i);
+				return;
+			}
+			phys->hw_ctl = hw_ctl[i];
+
 			phys->connector = conn->state->connector;
 			phys->topology_name = topology_name;
 			if (phys->ops.mode_set)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index c8c4612..5c89868 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -196,9 +196,6 @@ static void dpu_encoder_phys_cmd_mode_set(
 {
 	struct dpu_encoder_phys_cmd *cmd_enc =
 		to_dpu_encoder_phys_cmd(phys_enc);
-	struct dpu_rm *rm = &phys_enc->dpu_kms->rm;
-	struct dpu_rm_hw_iter iter;
-	int i, instance;
 
 	if (!phys_enc || !mode || !adj_mode) {
 		DPU_ERROR("invalid args\n");
@@ -208,22 +205,6 @@ static void dpu_encoder_phys_cmd_mode_set(
 	DPU_DEBUG_CMDENC(cmd_enc, "caching mode:\n");
 	drm_mode_debug_printmodeline(adj_mode);
 
-	instance = phys_enc->split_role == ENC_ROLE_SLAVE ? 1 : 0;
-
-	/* Retrieve previously allocated HW Resources. Shouldn't fail */
-	dpu_rm_init_hw_iter(&iter, phys_enc->parent->base.id, DPU_HW_BLK_CTL);
-	for (i = 0; i <= instance; i++) {
-		if (dpu_rm_get_hw(rm, &iter))
-			phys_enc->hw_ctl = (struct dpu_hw_ctl *)iter.hw;
-	}
-
-	if (IS_ERR_OR_NULL(phys_enc->hw_ctl)) {
-		DPU_ERROR_CMDENC(cmd_enc, "failed to init ctl: %ld\n",
-				PTR_ERR(phys_enc->hw_ctl));
-		phys_enc->hw_ctl = NULL;
-		return;
-	}
-
 	_dpu_encoder_phys_cmd_setup_irq_hw_idx(phys_enc);
 }
 
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 57ece03..c0221cc 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
@@ -395,9 +395,6 @@ static void dpu_encoder_phys_vid_mode_set(
 		struct drm_display_mode *mode,
 		struct drm_display_mode *adj_mode)
 {
-	struct dpu_rm *rm;
-	struct dpu_rm_hw_iter iter;
-	int i, instance;
 	struct dpu_encoder_phys_vid *vid_enc;
 
 	if (!phys_enc || !phys_enc->dpu_kms) {
@@ -405,7 +402,6 @@ static void dpu_encoder_phys_vid_mode_set(
 		return;
 	}
 
-	rm = &phys_enc->dpu_kms->rm;
 	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
 
 	if (adj_mode) {
@@ -414,21 +410,6 @@ static void dpu_encoder_phys_vid_mode_set(
 		DPU_DEBUG_VIDENC(vid_enc, "caching mode:\n");
 	}
 
-	instance = phys_enc->split_role == ENC_ROLE_SLAVE ? 1 : 0;
-
-	/* Retrieve previously allocated HW Resources. Shouldn't fail */
-	dpu_rm_init_hw_iter(&iter, phys_enc->parent->base.id, DPU_HW_BLK_CTL);
-	for (i = 0; i <= instance; i++) {
-		if (dpu_rm_get_hw(rm, &iter))
-			phys_enc->hw_ctl = (struct dpu_hw_ctl *)iter.hw;
-	}
-	if (IS_ERR_OR_NULL(phys_enc->hw_ctl)) {
-		DPU_ERROR_VIDENC(vid_enc, "failed to init ctl, %ld\n",
-				PTR_ERR(phys_enc->hw_ctl));
-		phys_enc->hw_ctl = NULL;
-		return;
-	}
-
 	_dpu_encoder_phys_vid_setup_irq_hw_idx(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

[PATCH v3 07/13] drm/msm/dpu: avoid querying for hw intf before assignment

[Jeykumar Sankaran]

hw intf blocks are needed only during encoder enable to program
timing engines(for video panels). encoder->enable is triggered
only after atomic_modeset at which point we assign the
resources for the display pipeline. This patch defers the
hw_intf look-up until encoder enable.

changes in v2:
	- none
changes in v3:
	- none

Change-Id: Ib0a2253431468151355e50cbad7b91e2b77b6e54
Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   | 53 +++++++---------------
 1 file changed, 16 insertions(+), 37 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 c0221cc..a0b3744 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
@@ -462,7 +462,7 @@ 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_hw_intf *intf;
+	struct dpu_rm_hw_iter iter;
 	struct dpu_hw_ctl *ctl;
 	u32 flush_mask = 0;
 
@@ -474,11 +474,20 @@ 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);
-	intf = vid_enc->hw_intf;
 	ctl = phys_enc->hw_ctl;
-	if (!vid_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);
+
+	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) {
+			vid_enc->hw_intf = hw_intf;
+			break;
+		}
+	}
+
+	if (!vid_enc->hw_intf) {
+		DPU_ERROR("hw_intf not assigned\n");
 		return;
 	}
 
@@ -500,7 +509,7 @@ 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, intf->idx);
+	ctl->ops.get_bitmask_intf(ctl, &flush_mask, vid_enc->hw_intf->idx);
 	ctl->ops.update_pending_flush(ctl, flush_mask);
 
 skip_flush:
@@ -531,22 +540,13 @@ static void dpu_encoder_phys_vid_get_hw_resources(
 		struct dpu_encoder_hw_resources *hw_res,
 		struct drm_connector_state *conn_state)
 {
-	struct dpu_encoder_phys_vid *vid_enc;
-
 	if (!phys_enc || !hw_res) {
 		DPU_ERROR("invalid arg(s), enc %d hw_res %d conn_state %d\n",
 				phys_enc != 0, hw_res != 0, conn_state != 0);
 		return;
 	}
 
-	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
-	if (!vid_enc->hw_intf) {
-		DPU_ERROR("invalid arg(s), hw_intf\n");
-		return;
-	}
-
-	DPU_DEBUG_VIDENC(vid_enc, "\n");
-	hw_res->intfs[vid_enc->hw_intf->idx - INTF_0] = INTF_MODE_VIDEO;
+	hw_res->intfs[phys_enc->intf_idx - INTF_0] = INTF_MODE_VIDEO;
 }
 
 static int _dpu_encoder_phys_vid_wait_for_vblank(
@@ -809,7 +809,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
 {
 	struct dpu_encoder_phys *phys_enc = NULL;
 	struct dpu_encoder_phys_vid *vid_enc = NULL;
-	struct dpu_rm_hw_iter iter;
 	struct dpu_encoder_irq *irq;
 	int i, ret = 0;
 
@@ -829,26 +828,6 @@ 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;
 
-	/**
-	 * hw_intf resource permanently assigned to this encoder
-	 * Other resources allocated at atomic commit time by use case
-	 */
-	dpu_rm_init_hw_iter(&iter, 0, DPU_HW_BLK_INTF);
-	while (dpu_rm_get_hw(&p->dpu_kms->rm, &iter)) {
-		struct dpu_hw_intf *hw_intf = (struct dpu_hw_intf *)iter.hw;
-
-		if (hw_intf->idx == p->intf_idx) {
-			vid_enc->hw_intf = hw_intf;
-			break;
-		}
-	}
-
-	if (!vid_enc->hw_intf) {
-		ret = -EINVAL;
-		DPU_ERROR("failed to get hw_intf\n");
-		goto fail;
-	}
-
 	DPU_DEBUG_VIDENC(vid_enc, "\n");
 
 	dpu_encoder_phys_vid_init_ops(&phys_enc->ops);
-- 
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

[PATCH v3 08/13] drm/msm/dpu: move hw resource tracking to crtc state

[Jeykumar Sankaran]

Prep changes for state based resource management.

Moves all the hw block tracking for the crtc to the state
object.

changes in v2:
	- none
changes in v3:
	- none

Change-Id: I2816e9e28b27f1126b477d62eb3858a30a652747
Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 77 +++++++++++++++++---------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 25 +++++------
 2 files changed, 54 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 1f2d223..515b0e6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -136,9 +136,9 @@ static void _dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc)
 	crtc_state = to_dpu_crtc_state(crtc->state);
 
 	lm_horiz_position = 0;
-	for (lm_idx = 0; lm_idx < dpu_crtc->num_mixers; lm_idx++) {
+	for (lm_idx = 0; lm_idx < crtc_state->num_mixers; lm_idx++) {
 		const struct drm_rect *lm_roi = &crtc_state->lm_bounds[lm_idx];
-		struct dpu_hw_mixer *hw_lm = dpu_crtc->mixers[lm_idx].hw_lm;
+		struct dpu_hw_mixer *hw_lm = crtc_state->mixers[lm_idx].hw_lm;
 		struct dpu_hw_mixer_cfg cfg;
 
 		if (!lm_roi || !drm_rect_visible(lm_roi))
@@ -219,7 +219,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
 					   fb ? fb->modifier : 0);
 
 		/* blend config update */
-		for (lm_idx = 0; lm_idx < dpu_crtc->num_mixers; lm_idx++) {
+		for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) {
 			_dpu_crtc_setup_blend_cfg(mixer + lm_idx, pstate);
 
 			mixer[lm_idx].flush_mask |= flush_mask;
@@ -242,7 +242,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
 static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
 {
 	struct dpu_crtc *dpu_crtc;
-	struct dpu_crtc_state *dpu_crtc_state;
+	struct dpu_crtc_state *cstate;
 	struct dpu_crtc_mixer *mixer;
 	struct dpu_hw_ctl *ctl;
 	struct dpu_hw_mixer *lm;
@@ -253,17 +253,17 @@ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
 		return;
 
 	dpu_crtc = to_dpu_crtc(crtc);
-	dpu_crtc_state = to_dpu_crtc_state(crtc->state);
-	mixer = dpu_crtc->mixers;
+	cstate = to_dpu_crtc_state(crtc->state);
+	mixer = cstate->mixers;
 
 	DPU_DEBUG("%s\n", dpu_crtc->name);
 
-	if (dpu_crtc->num_mixers > CRTC_DUAL_MIXERS) {
-		DPU_ERROR("invalid number mixers: %d\n", dpu_crtc->num_mixers);
+	if (cstate->num_mixers > CRTC_DUAL_MIXERS) {
+		DPU_ERROR("invalid number mixers: %d\n", cstate->num_mixers);
 		return;
 	}
 
-	for (i = 0; i < dpu_crtc->num_mixers; i++) {
+	for (i = 0; i < cstate->num_mixers; i++) {
 		if (!mixer[i].hw_lm || !mixer[i].hw_ctl) {
 			DPU_ERROR("invalid lm or ctl assigned to mixer\n");
 			return;
@@ -280,7 +280,7 @@ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
 
 	_dpu_crtc_blend_setup_mixer(crtc, dpu_crtc, mixer);
 
-	for (i = 0; i < dpu_crtc->num_mixers; i++) {
+	for (i = 0; i < cstate->num_mixers; i++) {
 		ctl = mixer[i].hw_ctl;
 		lm = mixer[i].hw_lm;
 
@@ -502,7 +502,7 @@ static void _dpu_crtc_setup_mixer_for_encoder(
 		struct drm_crtc *crtc,
 		struct drm_encoder *enc)
 {
-	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
+	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;
@@ -514,8 +514,8 @@ static void _dpu_crtc_setup_mixer_for_encoder(
 	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 = dpu_crtc->num_mixers; i < ARRAY_SIZE(dpu_crtc->mixers); i++) {
-		mixer = &dpu_crtc->mixers[i];
+	for (i = cstate->num_mixers; i < ARRAY_SIZE(cstate->mixers); i++) {
+		mixer = &cstate->mixers[i];
 
 		if (!dpu_rm_get_hw(rm, &lm_iter))
 			break;
@@ -540,7 +540,7 @@ static void _dpu_crtc_setup_mixer_for_encoder(
 
 		mixer->encoder = enc;
 
-		dpu_crtc->num_mixers++;
+		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",
@@ -551,11 +551,11 @@ static void _dpu_crtc_setup_mixer_for_encoder(
 static void _dpu_crtc_setup_mixers(struct drm_crtc *crtc)
 {
 	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
+	struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
 	struct drm_encoder *enc;
 
-	dpu_crtc->num_mixers = 0;
-	dpu_crtc->mixers_swapped = false;
-	memset(dpu_crtc->mixers, 0, sizeof(dpu_crtc->mixers));
+	cstate->num_mixers = 0;
+	memset(cstate->mixers, 0, sizeof(cstate->mixers));
 
 	mutex_lock(&dpu_crtc->crtc_lock);
 	/* Check for mixers on all encoders attached to this crtc */
@@ -589,7 +589,7 @@ static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc,
 	adj_mode = &state->adjusted_mode;
 	crtc_split_width = dpu_crtc_get_mixer_width(dpu_crtc, cstate, adj_mode);
 
-	for (i = 0; i < dpu_crtc->num_mixers; i++) {
+	for (i = 0; i < cstate->num_mixers; i++) {
 		struct drm_rect *r = &cstate->lm_bounds[i];
 		r->x1 = crtc_split_width * i;
 		r->y1 = 0;
@@ -606,6 +606,7 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc,
 		struct drm_crtc_state *old_state)
 {
 	struct dpu_crtc *dpu_crtc;
+	struct dpu_crtc_state *cstate;
 	struct drm_encoder *encoder;
 	struct drm_device *dev;
 	unsigned long flags;
@@ -625,10 +626,11 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc,
 	DPU_DEBUG("crtc%d\n", crtc->base.id);
 
 	dpu_crtc = to_dpu_crtc(crtc);
+	cstate = to_dpu_crtc_state(crtc->state);
 	dev = crtc->dev;
 	smmu_state = &dpu_crtc->smmu_state;
 
-	if (!dpu_crtc->num_mixers) {
+	if (!cstate->num_mixers) {
 		_dpu_crtc_setup_mixers(crtc);
 		_dpu_crtc_setup_lm_bounds(crtc, crtc->state);
 	}
@@ -655,7 +657,7 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc,
 	 * it means we are trying to flush a CRTC whose state is disabled:
 	 * nothing else needs to be done.
 	 */
-	if (unlikely(!dpu_crtc->num_mixers))
+	if (unlikely(!cstate->num_mixers))
 		return;
 
 	_dpu_crtc_blend_setup(crtc);
@@ -719,7 +721,7 @@ static void dpu_crtc_atomic_flush(struct drm_crtc *crtc,
 	 * it means we are trying to flush a CRTC whose state is disabled:
 	 * nothing else needs to be done.
 	 */
-	if (unlikely(!dpu_crtc->num_mixers))
+	if (unlikely(!cstate->num_mixers))
 		return;
 
 	/*
@@ -834,7 +836,7 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
 	 * it means we are trying to start a CRTC whose state is disabled:
 	 * nothing else needs to be done.
 	 */
-	if (unlikely(!dpu_crtc->num_mixers))
+	if (unlikely(!cstate->num_mixers))
 		return;
 
 	DPU_ATRACE_BEGIN("crtc_commit");
@@ -1069,6 +1071,7 @@ static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
 	struct dpu_crtc *dpu_crtc;
 	struct drm_encoder *encoder;
 	struct dpu_crtc_mixer *m;
+	struct dpu_crtc_state *cstate;
 	u32 i, misr_status;
 
 	if (!crtc) {
@@ -1076,6 +1079,7 @@ static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
 		return;
 	}
 	dpu_crtc = to_dpu_crtc(crtc);
+	cstate = to_dpu_crtc_state(dpu_crtc->base.state);
 
 	mutex_lock(&dpu_crtc->crtc_lock);
 
@@ -1091,8 +1095,8 @@ static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
 			dpu_encoder_virt_restore(encoder);
 		}
 
-		for (i = 0; i < dpu_crtc->num_mixers; ++i) {
-			m = &dpu_crtc->mixers[i];
+		for (i = 0; i < cstate->num_mixers; ++i) {
+			m = &cstate->mixers[i];
 			if (!m->hw_lm || !m->hw_lm->ops.setup_misr ||
 					!dpu_crtc->misr_enable)
 				continue;
@@ -1102,8 +1106,8 @@ static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
 		}
 		break;
 	case DPU_POWER_EVENT_PRE_DISABLE:
-		for (i = 0; i < dpu_crtc->num_mixers; ++i) {
-			m = &dpu_crtc->mixers[i];
+		for (i = 0; i < cstate->num_mixers; ++i) {
+			m = &cstate->mixers[i];
 			if (!m->hw_lm || !m->hw_lm->ops.collect_misr ||
 					!dpu_crtc->misr_enable)
 				continue;
@@ -1191,9 +1195,8 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
 		dpu_power_handle_unregister_event(dpu_crtc->phandle,
 				dpu_crtc->power_event);
 
-	memset(dpu_crtc->mixers, 0, sizeof(dpu_crtc->mixers));
-	dpu_crtc->num_mixers = 0;
-	dpu_crtc->mixers_swapped = false;
+	memset(cstate->mixers, 0, sizeof(cstate->mixers));
+	cstate->num_mixers = 0;
 
 	/* disable clk & bw control until clk & bw properties are set */
 	cstate->bw_control = false;
@@ -1552,8 +1555,8 @@ static int _dpu_debugfs_status_show(struct seq_file *s, void *data)
 
 	seq_puts(s, "\n");
 
-	for (i = 0; i < dpu_crtc->num_mixers; ++i) {
-		m = &dpu_crtc->mixers[i];
+	for (i = 0; i < cstate->num_mixers; ++i) {
+		m = &cstate->mixers[i];
 		if (!m->hw_lm)
 			seq_printf(s, "\tmixer[%d] has no lm\n", i);
 		else if (!m->hw_ctl)
@@ -1646,6 +1649,7 @@ static ssize_t _dpu_crtc_misr_setup(struct file *file,
 		const char __user *user_buf, size_t count, loff_t *ppos)
 {
 	struct dpu_crtc *dpu_crtc;
+	struct dpu_crtc_state *cstate;
 	struct dpu_crtc_mixer *m;
 	int i = 0, rc;
 	char buf[MISR_BUFF_SIZE + 1];
@@ -1656,6 +1660,7 @@ static ssize_t _dpu_crtc_misr_setup(struct file *file,
 		return -EINVAL;
 
 	dpu_crtc = file->private_data;
+	cstate = to_dpu_crtc_state(dpu_crtc->base.state);
 	buff_copy = min_t(size_t, count, MISR_BUFF_SIZE);
 	if (copy_from_user(buf, user_buf, buff_copy)) {
 		DPU_ERROR("buffer copy failed\n");
@@ -1674,9 +1679,9 @@ static ssize_t _dpu_crtc_misr_setup(struct file *file,
 	mutex_lock(&dpu_crtc->crtc_lock);
 	dpu_crtc->misr_enable = enable;
 	dpu_crtc->misr_frame_count = frame_count;
-	for (i = 0; i < dpu_crtc->num_mixers; ++i) {
+	for (i = 0; i < cstate->num_mixers; ++i) {
 		dpu_crtc->misr_data[i] = 0;
-		m = &dpu_crtc->mixers[i];
+		m = &cstate->mixers[i];
 		if (!m->hw_lm || !m->hw_lm->ops.setup_misr)
 			continue;
 
@@ -1692,6 +1697,7 @@ static ssize_t _dpu_crtc_misr_read(struct file *file,
 		char __user *user_buff, size_t count, loff_t *ppos)
 {
 	struct dpu_crtc *dpu_crtc;
+	struct dpu_crtc_state *cstate;
 	struct dpu_crtc_mixer *m;
 	int i = 0, rc;
 	u32 misr_status;
@@ -1705,6 +1711,7 @@ static ssize_t _dpu_crtc_misr_read(struct file *file,
 		return -EINVAL;
 
 	dpu_crtc = file->private_data;
+	cstate = to_dpu_crtc_state(dpu_crtc->base.state);
 	rc = _dpu_crtc_power_enable(dpu_crtc, true);
 	if (rc)
 		return rc;
@@ -1716,8 +1723,8 @@ static ssize_t _dpu_crtc_misr_read(struct file *file,
 		goto buff_check;
 	}
 
-	for (i = 0; i < dpu_crtc->num_mixers; ++i) {
-		m = &dpu_crtc->mixers[i];
+	for (i = 0; i < cstate->num_mixers; ++i) {
+		m = &cstate->mixers[i];
 		if (!m->hw_lm || !m->hw_lm->ops.collect_misr)
 			continue;
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index e632651..9177ee6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -121,11 +121,6 @@ struct dpu_crtc_frame_event {
  * struct dpu_crtc - virtualized CRTC data structure
  * @base          : Base drm crtc structure
  * @name          : ASCII description of this crtc
- * @num_ctls      : Number of ctl paths in use
- * @num_mixers    : Number of mixers in use
- * @mixers_swapped: Whether the mixers have been swapped for left/right update
- *                  especially in the case of DSC Merge.
- * @mixers        : List of active mixers
  * @event         : Pointer to last received drm vblank event. If there is a
  *                  pending vblank event, this will be non-null.
  * @vsync_count   : Running count of received vsync events
@@ -167,12 +162,6 @@ struct dpu_crtc {
 	struct drm_crtc base;
 	char name[DPU_CRTC_NAME_SIZE];
 
-	/* HW Resources reserved for the crtc */
-	u32 num_ctls;
-	u32 num_mixers;
-	bool mixers_swapped;
-	struct dpu_crtc_mixer mixers[CRTC_DUAL_MIXERS];
-
 	struct drm_pending_vblank_event *event;
 	u32 vsync_count;
 
@@ -227,6 +216,10 @@ struct dpu_crtc {
  * @property_values: Current crtc property values
  * @input_fence_timeout_ns : Cached input fence timeout, in ns
  * @new_perf: new performance state being requested
+ * @num_mixers    : Number of mixers in use
+ * @mixers        : List of active mixers
+ * @num_ctls      : Number of ctl paths in use
+ * @hw_ctls       : List of activel ctl paths
  */
 struct dpu_crtc_state {
 	struct drm_crtc_state base;
@@ -236,8 +229,14 @@ struct dpu_crtc_state {
 	struct drm_rect lm_bounds[CRTC_DUAL_MIXERS];
 
 	uint64_t input_fence_timeout_ns;
-
 	struct dpu_core_perf_params new_perf;
+
+	/* HW Resources reserved for the crtc */
+	u32 num_mixers;
+	struct dpu_crtc_mixer mixers[CRTC_DUAL_MIXERS];
+
+	u32 num_ctls;
+	struct dpu_hw_ctl *hw_ctls[CRTC_DUAL_MIXERS];
 };
 
 #define to_dpu_crtc_state(x) \
@@ -255,7 +254,7 @@ static inline int dpu_crtc_get_mixer_width(struct dpu_crtc *dpu_crtc,
 	if (!dpu_crtc || !cstate || !mode)
 		return 0;
 
-	mixer_width = (dpu_crtc->num_mixers == CRTC_DUAL_MIXERS ?
+	mixer_width = (cstate->num_mixers == CRTC_DUAL_MIXERS ?
 			mode->hdisplay / CRTC_DUAL_MIXERS : mode->hdisplay);
 
 	return mixer_width;
-- 
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

[PATCH v3 09/13] drm/msm/dpu: rename hw_ctl to lm_ctl

[Jeykumar Sankaran]

Prep change for state based resource management.

Rename hw_ctl to lm_ctl to mean the ctl associated
with the hw layer mixer block.

changes in v2:
	- none
changes in v3:
	- none

Change-Id: If6e6249e089b89225cdfafe9158f66667509e97b
Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 26 +++++++++++++-------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h |  4 ++--
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 515b0e6..0eb369c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -175,7 +175,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
 		return;
 	}
 
-	ctl = mixer->hw_ctl;
+	ctl = mixer->lm_ctl;
 	lm = mixer->hw_lm;
 	stage_cfg = &dpu_crtc->stage_cfg;
 	cstate = to_dpu_crtc_state(crtc->state);
@@ -264,15 +264,15 @@ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
 	}
 
 	for (i = 0; i < cstate->num_mixers; i++) {
-		if (!mixer[i].hw_lm || !mixer[i].hw_ctl) {
+		if (!mixer[i].hw_lm || !mixer[i].lm_ctl) {
 			DPU_ERROR("invalid lm or ctl assigned to mixer\n");
 			return;
 		}
 		mixer[i].mixer_op_mode = 0;
 		mixer[i].flush_mask = 0;
-		if (mixer[i].hw_ctl->ops.clear_all_blendstages)
-			mixer[i].hw_ctl->ops.clear_all_blendstages(
-					mixer[i].hw_ctl);
+		if (mixer[i].lm_ctl->ops.clear_all_blendstages)
+			mixer[i].lm_ctl->ops.clear_all_blendstages(
+					mixer[i].lm_ctl);
 	}
 
 	/* initialize stage cfg */
@@ -281,7 +281,7 @@ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
 	_dpu_crtc_blend_setup_mixer(crtc, dpu_crtc, mixer);
 
 	for (i = 0; i < cstate->num_mixers; i++) {
-		ctl = mixer[i].hw_ctl;
+		ctl = mixer[i].lm_ctl;
 		lm = mixer[i].hw_lm;
 
 		lm->ops.setup_alpha_out(lm, mixer[i].mixer_op_mode);
@@ -525,14 +525,14 @@ static void _dpu_crtc_setup_mixer_for_encoder(
 		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->hw_ctl = last_valid_ctl;
+			mixer->lm_ctl = last_valid_ctl;
 		} else {
-			mixer->hw_ctl = (struct dpu_hw_ctl *)ctl_iter.hw;
-			last_valid_ctl = mixer->hw_ctl;
+			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->hw_ctl) {
+		if (!mixer->lm_ctl) {
 			DPU_ERROR("no valid ctls found for lm %d\n",
 					mixer->hw_lm->idx - LM_0);
 			return;
@@ -544,7 +544,7 @@ static void _dpu_crtc_setup_mixer_for_encoder(
 		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->hw_ctl->idx - CTL_0);
+				i, mixer->lm_ctl->idx - CTL_0);
 	}
 }
 
@@ -1559,11 +1559,11 @@ static int _dpu_debugfs_status_show(struct seq_file *s, void *data)
 		m = &cstate->mixers[i];
 		if (!m->hw_lm)
 			seq_printf(s, "\tmixer[%d] has no lm\n", i);
-		else if (!m->hw_ctl)
+		else if (!m->lm_ctl)
 			seq_printf(s, "\tmixer[%d] has no ctl\n", i);
 		else
 			seq_printf(s, "\tmixer:%d ctl:%d width:%d height:%d\n",
-				m->hw_lm->idx - LM_0, m->hw_ctl->idx - CTL_0,
+				m->hw_lm->idx - LM_0, m->lm_ctl->idx - CTL_0,
 				out_width, mode->vdisplay);
 	}
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 9177ee6..5b85ca8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -83,14 +83,14 @@ struct dpu_crtc_smmu_state_data {
 /**
  * struct dpu_crtc_mixer: stores the map for each virtual pipeline in the CRTC
  * @hw_lm:	LM HW Driver context
- * @hw_ctl:	CTL Path HW driver context
+ * @lm_ctl:	CTL Path HW driver context
  * @encoder:	Encoder attached to this lm & ctl
  * @mixer_op_mode:	mixer blending operation mode
  * @flush_mask:	mixer flush mask for ctl, mixer and pipe
  */
 struct dpu_crtc_mixer {
 	struct dpu_hw_mixer *hw_lm;
-	struct dpu_hw_ctl *hw_ctl;
+	struct dpu_hw_ctl *lm_ctl;
 	struct drm_encoder *encoder;
 	u32 mixer_op_mode;
 	u32 flush_mask;
-- 
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 v3 01/13] drm/msm/dpu: remove scalar config definitions

[Sean Paul]

On Tue, Aug 07, 2018 at 08:12:28PM -0700, Jeykumar Sankaran wrote:
> cleans up left out scalar config definitions from headers
> 
> changes in v2:
> 	- none
> changes in v3:
> 	- none
> 
> Change-Id: Id824dd5075c666f97b964573c97215bb786eac75

Please strip Change-Id before sending patches.

Reviewed-by: Sean Paul <seanpaul@chromium.org>


> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h    |  2 --
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 10 ----------
>  2 files changed, 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index e87109e..0e9aafa 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -164,7 +164,6 @@ struct dpu_crtc_frame_event {
>   * @cur_perf      : current performance committed to clock/bandwidth driver
>   * @rp_lock       : serialization lock for resource pool
>   * @rp_head       : list of active resource pool
> - * @scl3_cfg_lut  : qseed3 lut config
>   */
>  struct dpu_crtc {
>  	struct drm_crtc base;
> @@ -175,7 +174,6 @@ struct dpu_crtc {
>  	u32 num_mixers;
>  	bool mixers_swapped;
>  	struct dpu_crtc_mixer mixers[CRTC_DUAL_MIXERS];
> -	struct dpu_hw_scaler3_lut_cfg *scl3_lut_cfg;
>  
>  	struct drm_pending_vblank_event *event;
>  	u32 vsync_count;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
> index 1240f50..c5c8f60 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
> @@ -148,16 +148,6 @@ struct dpu_hw_scaler3_cfg {
>  	struct dpu_hw_scaler3_de_cfg de;
>  };
>  
> -struct dpu_hw_scaler3_lut_cfg {
> -	bool is_configured;
> -	u32 *dir_lut;
> -	size_t dir_len;
> -	u32 *cir_lut;
> -	size_t cir_len;
> -	u32 *sep_lut;
> -	size_t sep_len;
> -};
> -
>  /**
>   * struct dpu_drm_pix_ext_v1 - version 1 of pixel ext structure
>   * @num_ext_pxls_lr: Number of total horizontal pixels
> -- 
> 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 v3 02/13] drm/msm/dpu: remove resource pool manager

[Sean Paul]

On Tue, Aug 07, 2018 at 08:12:29PM -0700, Jeykumar Sankaran wrote:
> resource pool manager utility was introduced to manage
> rotator sessions. Removing the support as the rotator
> feature doesn't exist.
> 
> changes in v2:
> 	- none
> changes in v3:
> 	- rebase on [1]
> 
> [1] https://gitlab.freedesktop.org/seanpaul/dpu-staging/commits/for-next
> 
> Change-Id: Ib045f1c66269be650bce5896c459f59e1047a53f
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 205 -------------------------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h |  56 ---------
>  2 files changed, 261 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 80cbf75..1f2d223 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -99,187 +99,6 @@ static inline int _dpu_crtc_power_enable(struct dpu_crtc *dpu_crtc, bool enable)
>  	return 0;
>  }
>  
> -/**
> - * _dpu_crtc_rp_to_crtc - get crtc from resource pool object
> - * @rp: Pointer to resource pool
> - * return: Pointer to drm crtc if success; null otherwise
> - */
> -static struct drm_crtc *_dpu_crtc_rp_to_crtc(struct dpu_crtc_respool *rp)
> -{
> -	if (!rp)
> -		return NULL;
> -
> -	return container_of(rp, struct dpu_crtc_state, rp)->base.crtc;
> -}
> -
> -/**
> - * _dpu_crtc_rp_reclaim - reclaim unused, or all if forced, resources in pool
> - * @rp: Pointer to resource pool
> - * @force: True to reclaim all resources; otherwise, reclaim only unused ones
> - * return: None
> - */
> -static void _dpu_crtc_rp_reclaim(struct dpu_crtc_respool *rp, bool force)
> -{
> -	struct dpu_crtc_res *res, *next;
> -	struct drm_crtc *crtc;
> -
> -	crtc = _dpu_crtc_rp_to_crtc(rp);
> -	if (!crtc) {
> -		DPU_ERROR("invalid crtc\n");
> -		return;
> -	}
> -
> -	DPU_DEBUG("crtc%d.%u %s\n", crtc->base.id, rp->sequence_id,
> -			force ? "destroy" : "free_unused");
> -
> -	list_for_each_entry_safe(res, next, &rp->res_list, list) {
> -		if (!force && !(res->flags & DPU_CRTC_RES_FLAG_FREE))
> -			continue;
> -		DPU_DEBUG("crtc%d.%u reclaim res:0x%x/0x%llx/%pK/%d\n",
> -				crtc->base.id, rp->sequence_id,
> -				res->type, res->tag, res->val,
> -				atomic_read(&res->refcount));
> -		list_del(&res->list);
> -		if (res->ops.put)
> -			res->ops.put(res->val);
> -		kfree(res);
> -	}
> -}
> -
> -/**
> - * _dpu_crtc_rp_free_unused - free unused resource in pool
> - * @rp: Pointer to resource pool
> - * return: none
> - */
> -static void _dpu_crtc_rp_free_unused(struct dpu_crtc_respool *rp)
> -{
> -	mutex_lock(rp->rp_lock);
> -	_dpu_crtc_rp_reclaim(rp, false);
> -	mutex_unlock(rp->rp_lock);
> -}
> -
> -/**
> - * _dpu_crtc_rp_destroy - destroy resource pool
> - * @rp: Pointer to resource pool
> - * return: None
> - */
> -static void _dpu_crtc_rp_destroy(struct dpu_crtc_respool *rp)
> -{
> -	mutex_lock(rp->rp_lock);
> -	list_del_init(&rp->rp_list);
> -	_dpu_crtc_rp_reclaim(rp, true);
> -	mutex_unlock(rp->rp_lock);
> -}
> -
> -/**
> - * _dpu_crtc_hw_blk_get - get callback for hardware block
> - * @val: Resource handle
> - * @type: Resource type
> - * @tag: Search tag for given resource
> - * return: Resource handle
> - */
> -static void *_dpu_crtc_hw_blk_get(void *val, u32 type, u64 tag)
> -{
> -	DPU_DEBUG("res:%d/0x%llx/%pK\n", type, tag, val);
> -	return dpu_hw_blk_get(val, type, tag);
> -}
> -
> -/**
> - * _dpu_crtc_hw_blk_put - put callback for hardware block
> - * @val: Resource handle
> - * return: None
> - */
> -static void _dpu_crtc_hw_blk_put(void *val)
> -{
> -	DPU_DEBUG("res://%pK\n", val);
> -	dpu_hw_blk_put(val);
> -}
> -
> -/**
> - * _dpu_crtc_rp_duplicate - duplicate resource pool and reset reference count
> - * @rp: Pointer to original resource pool
> - * @dup_rp: Pointer to duplicated resource pool
> - * return: None
> - */
> -static void _dpu_crtc_rp_duplicate(struct dpu_crtc_respool *rp,
> -		struct dpu_crtc_respool *dup_rp)
> -{
> -	struct dpu_crtc_res *res, *dup_res;
> -	struct drm_crtc *crtc;
> -
> -	if (!rp || !dup_rp || !rp->rp_head) {
> -		DPU_ERROR("invalid resource pool\n");
> -		return;
> -	}
> -
> -	crtc = _dpu_crtc_rp_to_crtc(rp);
> -	if (!crtc) {
> -		DPU_ERROR("invalid crtc\n");
> -		return;
> -	}
> -
> -	DPU_DEBUG("crtc%d.%u duplicate\n", crtc->base.id, rp->sequence_id);
> -
> -	mutex_lock(rp->rp_lock);
> -	dup_rp->sequence_id = rp->sequence_id + 1;
> -	INIT_LIST_HEAD(&dup_rp->res_list);
> -	dup_rp->ops = rp->ops;
> -	list_for_each_entry(res, &rp->res_list, list) {
> -		dup_res = kzalloc(sizeof(struct dpu_crtc_res), GFP_KERNEL);
> -		if (!dup_res) {
> -			mutex_unlock(rp->rp_lock);
> -			return;
> -		}
> -		INIT_LIST_HEAD(&dup_res->list);
> -		atomic_set(&dup_res->refcount, 0);
> -		dup_res->type = res->type;
> -		dup_res->tag = res->tag;
> -		dup_res->val = res->val;
> -		dup_res->ops = res->ops;
> -		dup_res->flags = DPU_CRTC_RES_FLAG_FREE;
> -		DPU_DEBUG("crtc%d.%u dup res:0x%x/0x%llx/%pK/%d\n",
> -				crtc->base.id, dup_rp->sequence_id,
> -				dup_res->type, dup_res->tag, dup_res->val,
> -				atomic_read(&dup_res->refcount));
> -		list_add_tail(&dup_res->list, &dup_rp->res_list);
> -		if (dup_res->ops.get)
> -			dup_res->ops.get(dup_res->val, 0, -1);
> -	}
> -
> -	dup_rp->rp_lock = rp->rp_lock;
> -	dup_rp->rp_head = rp->rp_head;
> -	INIT_LIST_HEAD(&dup_rp->rp_list);
> -	list_add_tail(&dup_rp->rp_list, rp->rp_head);
> -	mutex_unlock(rp->rp_lock);
> -}
> -
> -/**
> - * _dpu_crtc_rp_reset - reset resource pool after allocation
> - * @rp: Pointer to original resource pool
> - * @rp_lock: Pointer to serialization resource pool lock
> - * @rp_head: Pointer to crtc resource pool head
> - * return: None
> - */
> -static void _dpu_crtc_rp_reset(struct dpu_crtc_respool *rp,
> -		struct mutex *rp_lock, struct list_head *rp_head)
> -{
> -	if (!rp || !rp_lock || !rp_head) {
> -		DPU_ERROR("invalid resource pool\n");
> -		return;
> -	}
> -
> -	mutex_lock(rp_lock);
> -	rp->rp_lock = rp_lock;
> -	rp->rp_head = rp_head;
> -	INIT_LIST_HEAD(&rp->rp_list);
> -	rp->sequence_id = 0;
> -	INIT_LIST_HEAD(&rp->res_list);
> -	rp->ops.get = _dpu_crtc_hw_blk_get;
> -	rp->ops.put = _dpu_crtc_hw_blk_put;
> -	list_add_tail(&rp->rp_list, rp->rp_head);
> -	mutex_unlock(rp_lock);
> -}
> -
>  static void dpu_crtc_destroy(struct drm_crtc *crtc)
>  {
>  	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> @@ -951,8 +770,6 @@ static void dpu_crtc_destroy_state(struct drm_crtc *crtc,
>  
>  	DPU_DEBUG("crtc%d\n", crtc->base.id);
>  
> -	_dpu_crtc_rp_destroy(&cstate->rp);
> -
>  	__drm_atomic_helper_crtc_destroy_state(state);
>  
>  	kfree(cstate);
> @@ -1206,8 +1023,6 @@ static struct drm_crtc_state *dpu_crtc_duplicate_state(struct drm_crtc *crtc)
>  	/* duplicate base helper */
>  	__drm_atomic_helper_crtc_duplicate_state(crtc, &cstate->base);
>  
> -	_dpu_crtc_rp_duplicate(&old_cstate->rp, &cstate->rp);
> -
>  	return &cstate->base;
>  }
>  
> @@ -1244,9 +1059,6 @@ static void dpu_crtc_reset(struct drm_crtc *crtc)
>  		return;
>  	}
>  
> -	_dpu_crtc_rp_reset(&cstate->rp, &dpu_crtc->rp_lock,
> -			&dpu_crtc->rp_head);
> -
>  	cstate->base.crtc = crtc;
>  	crtc->state = &cstate->base;
>  }
> @@ -1679,7 +1491,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
>  	}
>  
>  end:
> -	_dpu_crtc_rp_free_unused(&cstate->rp);
>  	kfree(pstates);
>  	return rc;
>  }
> @@ -1955,8 +1766,6 @@ static int dpu_crtc_debugfs_state_show(struct seq_file *s, void *v)
>  {
>  	struct drm_crtc *crtc = (struct drm_crtc *) s->private;
>  	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> -	struct dpu_crtc_res *res;
> -	struct dpu_crtc_respool *rp;
>  	int i;
>  
>  	seq_printf(s, "client type: %d\n", dpu_crtc_get_client_type(crtc));
> @@ -1973,17 +1782,6 @@ static int dpu_crtc_debugfs_state_show(struct seq_file *s, void *v)
>  				dpu_crtc->cur_perf.max_per_pipe_ib[i]);
>  	}
>  
> -	mutex_lock(&dpu_crtc->rp_lock);
> -	list_for_each_entry(rp, &dpu_crtc->rp_head, rp_list) {
> -		seq_printf(s, "rp.%d: ", rp->sequence_id);
> -		list_for_each_entry(res, &rp->res_list, list)
> -			seq_printf(s, "0x%x/0x%llx/%pK/%d ",
> -					res->type, res->tag, res->val,
> -					atomic_read(&res->refcount));
> -		seq_puts(s, "\n");
> -	}
> -	mutex_unlock(&dpu_crtc->rp_lock);
> -
>  	return 0;
>  }
>  DEFINE_DPU_DEBUGFS_SEQ_FOPS(dpu_crtc_debugfs_state);
> @@ -2104,9 +1902,6 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane)
>  	spin_lock_init(&dpu_crtc->spin_lock);
>  	atomic_set(&dpu_crtc->frame_pending, 0);
>  
> -	mutex_init(&dpu_crtc->rp_lock);
> -	INIT_LIST_HEAD(&dpu_crtc->rp_head);
> -
>  	init_completion(&dpu_crtc->frame_done_comp);
>  
>  	INIT_LIST_HEAD(&dpu_crtc->frame_event_list);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index 0e9aafa..e84da78 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -162,8 +162,6 @@ struct dpu_crtc_frame_event {
>   * @phandle: Pointer to power handler
>   * @power_event   : registered power event handle
>   * @cur_perf      : current performance committed to clock/bandwidth driver
> - * @rp_lock       : serialization lock for resource pool
> - * @rp_head       : list of active resource pool
>   */
>  struct dpu_crtc {
>  	struct drm_crtc base;
> @@ -213,65 +211,12 @@ struct dpu_crtc {
>  
>  	struct dpu_core_perf_params cur_perf;
>  
> -	struct mutex rp_lock;
> -	struct list_head rp_head;
> -
>  	struct dpu_crtc_smmu_state_data smmu_state;
>  };
>  
>  #define to_dpu_crtc(x) container_of(x, struct dpu_crtc, base)
>  
>  /**
> - * struct dpu_crtc_res_ops - common operations for crtc resources
> - * @get: get given resource
> - * @put: put given resource
> - */
> -struct dpu_crtc_res_ops {
> -	void *(*get)(void *val, u32 type, u64 tag);
> -	void (*put)(void *val);
> -};
> -
> -#define DPU_CRTC_RES_FLAG_FREE		BIT(0)
> -
> -/**
> - * struct dpu_crtc_res - definition of crtc resources
> - * @list: list of crtc resource
> - * @type: crtc resource type
> - * @tag: unique identifier per type
> - * @refcount: reference/usage count
> - * @ops: callback operations
> - * @val: resource handle associated with type/tag
> - * @flags: customization flags
> - */
> -struct dpu_crtc_res {
> -	struct list_head list;
> -	u32 type;
> -	u64 tag;
> -	atomic_t refcount;
> -	struct dpu_crtc_res_ops ops;
> -	void *val;
> -	u32 flags;
> -};
> -
> -/**
> - * dpu_crtc_respool - crtc resource pool
> - * @rp_lock: pointer to serialization lock
> - * @rp_head: pointer to head of active resource pools of this crtc
> - * @rp_list: list of crtc resource pool
> - * @sequence_id: sequence identifier, incremented per state duplication
> - * @res_list: list of resource managed by this resource pool
> - * @ops: resource operations for parent resource pool
> - */
> -struct dpu_crtc_respool {
> -	struct mutex *rp_lock;
> -	struct list_head *rp_head;
> -	struct list_head rp_list;
> -	u32 sequence_id;
> -	struct list_head res_list;
> -	struct dpu_crtc_res_ops ops;
> -};
> -
> -/**
>   * struct dpu_crtc_state - dpu container for atomic crtc state
>   * @base: Base drm crtc state structure
>   * @is_ppsplit    : Whether current topology requires PPSplit special handling
> @@ -296,7 +241,6 @@ struct dpu_crtc_state {
>  	uint64_t input_fence_timeout_ns;
>  
>  	struct dpu_core_perf_params new_perf;
> -	struct dpu_crtc_respool rp;
>  };
>  
>  #define to_dpu_crtc_state(x) \
> -- 
> 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 v3 03/13] drm/msm/dpu: remove ping pong split topology variables

[Sean Paul]

On Tue, Aug 07, 2018 at 08:12:30PM -0700, Jeykumar Sankaran wrote:
> removes left out variables of previous ping pong
> split topology cleanup.
> 
> changes in v2:
> 	- none
> changes in v3:
> 	- none
> 
> Change-Id: I1bf9d242039ce7cfd271233fa27840e83184fb95
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index e84da78..e632651 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -219,7 +219,6 @@ struct dpu_crtc {
>  /**
>   * struct dpu_crtc_state - dpu container for atomic crtc state
>   * @base: Base drm crtc state structure
> - * @is_ppsplit    : Whether current topology requires PPSplit special handling
>   * @bw_control    : true if bw/clk controlled by core bw/clk properties
>   * @bw_split_vote : true if bw controlled by llcc/dram bw properties
>   * @lm_bounds     : LM boundaries based on current mode full resolution, no ROI.
> @@ -234,8 +233,6 @@ struct dpu_crtc_state {
>  
>  	bool bw_control;
>  	bool bw_split_vote;
> -
> -	bool is_ppsplit;
>  	struct drm_rect lm_bounds[CRTC_DUAL_MIXERS];
>  
>  	uint64_t input_fence_timeout_ns;
> -- 
> 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 v3 04/13] drm/msm/dpu: program master-slave encoders explicitly

[Sean Paul]

On Tue, Aug 07, 2018 at 08:12:31PM -0700, Jeykumar Sankaran wrote:
> Identify slave-master encoders and program them explicitly.

You've got the what, but could you please explain the why?

> 
> changes in v2:
> 	- none
> changes in v3:
> 	- none
> 
> Change-Id: I0ebfada05bd7f8437f842ad860490a678aa8f4cd
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 39 ++++++++++++++++++-----------
>  1 file changed, 24 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 1b4de34..a0ced79 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -184,6 +184,7 @@ struct dpu_encoder_virt {
>  	unsigned int num_phys_encs;
>  	struct dpu_encoder_phys *phys_encs[MAX_PHYS_ENCODERS_PER_VIRTUAL];
>  	struct dpu_encoder_phys *cur_master;
> +	struct dpu_encoder_phys *cur_slave;

You only use this in one function, why not just make it a local?

>  	struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
>  
>  	bool intfs_swapped;
> @@ -1153,6 +1154,7 @@ void dpu_encoder_virt_restore(struct drm_encoder *drm_enc)
>  static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
>  {
>  	struct dpu_encoder_virt *dpu_enc = NULL;
> +	struct dpu_encoder_phys *phys  = NULL;
>  	int i, ret = 0;
>  	struct drm_display_mode *cur_mode = NULL;
>  
> @@ -1160,6 +1162,7 @@ static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
>  		DPU_ERROR("invalid encoder\n");
>  		return;
>  	}
> +
>  	dpu_enc = to_dpu_encoder_virt(drm_enc);
>  	cur_mode = &dpu_enc->base.crtc->state->adjusted_mode;
>  
> @@ -1167,21 +1170,36 @@ static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
>  			     cur_mode->vdisplay);
>  
>  	dpu_enc->cur_master = NULL;
> +	dpu_enc->cur_slave = NULL;

There's no benefit to setting this NULL. cur_master is set to NULL so it can be
checked after the loop. Since you're not checking this, it's not necessary.

I suppose you might also want to clear this on disable like master.

>  	for (i = 0; i < dpu_enc->num_phys_encs; i++) {
> -		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
> +		phys = dpu_enc->phys_encs[i];
> +
> +		if (!phys || !phys->ops.is_master)

I don't think it's possible for phys to be NULL, is it?

> +			continue;
>  
> -		if (phys && phys->ops.is_master && phys->ops.is_master(phys)) {
> -			DPU_DEBUG_ENC(dpu_enc, "master is now idx %d\n", i);
> +		if (phys->ops.is_master(phys)) {
> +			DPU_DEBUG_ENC(dpu_enc, "master is at idx %d\n", i);
>  			dpu_enc->cur_master = phys;
> -			break;
> +		} else {
> +			DPU_DEBUG_ENC(dpu_enc, "slave is at idx %d\n", i);
> +			dpu_enc->cur_slave = phys;
>  		}

You're making an assumption here that there can only be one master and there can
only be one slave.

It seems like you could avoid all of this work if you just did the assignment in
dpu_encoder_virt_add_phys_encs().

>  	}
>  
>  	if (!dpu_enc->cur_master) {
> -		DPU_ERROR("virt encoder has no master! num_phys %d\n", i);
> +		DPU_ERROR("virt encoder has no master identified\n");
>  		return;
>  	}
>  
> +	/* always enable slave encoder before master */
> +	phys = dpu_enc->cur_slave;
> +	if (phys && phys->ops.enable)
> +		phys->ops.enable(phys);
> +
> +	phys = dpu_enc->cur_master;
> +	if (phys && phys->ops.enable)
> +		phys->ops.enable(phys);
> +
>  	ret = dpu_encoder_resource_control(drm_enc, DPU_ENC_RC_EVENT_KICKOFF);
>  	if (ret) {
>  		DPU_ERROR_ENC(dpu_enc, "dpu resource control failed: %d\n",
> @@ -1190,25 +1208,16 @@ static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
>  	}
>  
>  	for (i = 0; i < dpu_enc->num_phys_encs; i++) {
> -		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
> -
> +		phys = dpu_enc->phys_encs[i];
>  		if (!phys)
>  			continue;
>  
> -		if (phys != dpu_enc->cur_master) {
> -			if (phys->ops.enable)
> -				phys->ops.enable(phys);
> -		}
> -
>  		if (dpu_enc->misr_enable && (dpu_enc->disp_info.capabilities &
>  		     MSM_DISPLAY_CAP_VID_MODE) && phys->ops.setup_misr)
>  			phys->ops.setup_misr(phys, true,
>  						dpu_enc->misr_frame_count);
>  	}
>  
> -	if (dpu_enc->cur_master->ops.enable)
> -		dpu_enc->cur_master->ops.enable(dpu_enc->cur_master);
> -

There's a change in functionality here. Previously you could call setup_misr
for slaves after they are enabled, but before master is enabled. Now you're
calling it after all are enabled.

I'm guessing it doesn't matter since it was previously called on master before
it was enabled, but I figure I'd point this out.

Sean

>  	_dpu_encoder_virt_enable_helper(drm_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 v3 05/13] drm/msm/dpu: use kms stored hw mdp block

[Sean Paul]

On Tue, Aug 07, 2018 at 08:12:32PM -0700, Jeykumar Sankaran wrote:
> Avoid querying RM for hw mdp block. Use the one
> stored in KMS during initialization.
> 
> changes in v2:
> 	- none
> changes in v3:
> 	- none
> 
> Change-Id: I52129b96bd561a5547507d7f567bcaa3dbe554aa
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 12 +-----------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c |  9 +--------
>  2 files changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> index 3084675..c8c4612 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> @@ -823,7 +823,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
>  {
>  	struct dpu_encoder_phys *phys_enc = NULL;
>  	struct dpu_encoder_phys_cmd *cmd_enc = NULL;
> -	struct dpu_hw_mdp *hw_mdp;
>  	struct dpu_encoder_irq *irq;
>  	int i, ret = 0;
>  
> @@ -836,14 +835,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
>  		goto fail;
>  	}
>  	phys_enc = &cmd_enc->base;
> -
> -	hw_mdp = dpu_rm_get_mdp(&p->dpu_kms->rm);
> -	if (IS_ERR_OR_NULL(hw_mdp)) {
> -		ret = PTR_ERR(hw_mdp);
> -		DPU_ERROR("failed to get mdptop\n");
> -		goto fail_mdp_init;
> -	}
> -	phys_enc->hw_mdptop = hw_mdp;
> +	phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
>  	phys_enc->intf_idx = p->intf_idx;
>  
>  	dpu_encoder_phys_cmd_init_ops(&phys_enc->ops);
> @@ -898,8 +890,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
>  
>  	return phys_enc;
>  
> -fail_mdp_init:
> -	kfree(cmd_enc);
>  fail:
>  	return ERR_PTR(ret);
>  }
> 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 14fc7c2..57ece03 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
> @@ -829,7 +829,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
>  	struct dpu_encoder_phys *phys_enc = NULL;
>  	struct dpu_encoder_phys_vid *vid_enc = NULL;
>  	struct dpu_rm_hw_iter iter;
> -	struct dpu_hw_mdp *hw_mdp;
>  	struct dpu_encoder_irq *irq;
>  	int i, ret = 0;
>  
> @@ -846,13 +845,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
>  
>  	phys_enc = &vid_enc->base;
>  
> -	hw_mdp = dpu_rm_get_mdp(&p->dpu_kms->rm);
> -	if (IS_ERR_OR_NULL(hw_mdp)) {
> -		ret = PTR_ERR(hw_mdp);
> -		DPU_ERROR("failed to get mdptop\n");
> -		goto fail;
> -	}
> -	phys_enc->hw_mdptop = hw_mdp;
> +	phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
>  	phys_enc->intf_idx = p->intf_idx;
>  
>  	/**
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH v3 06/13] drm/msm/dpu: iterate for assigned hw ctl in virtual encoder

[Sean Paul]

On Tue, Aug 07, 2018 at 08:12:33PM -0700, Jeykumar Sankaran wrote:
> Instead of iterating for hw ctrl per physical encoder, this
> patch moves the iterations and assignment to the virtual encoder.
> 
> changes in v2:
> 	- none
> changes in v3:
> 	- none
> 
> Change-Id: I896a8c36d6353986582e9d0fe3da9b2293579d4b
> 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_cmd.c   | 19 -------------------
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   | 19 -------------------
>  3 files changed, 20 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index a0ced79..7b82e2d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1017,9 +1017,11 @@ 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 dpu_rm_hw_iter pp_iter;
> +	struct dpu_rm_hw_iter pp_iter, ctl_iter;
>  	struct msm_display_topology topology;
>  	enum dpu_rm_topology_name topology_name;
> +	struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC];

Instead of setting each one to null in the loop:

        struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };


> +

Extra line

>  	int i = 0, ret;
>  
>  	if (!drm_enc) {
> @@ -1067,6 +1069,14 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>  		dpu_enc->hw_pp[i] = (struct dpu_hw_pingpong *) pp_iter.hw;
>  	}
>  
> +	dpu_rm_init_hw_iter(&ctl_iter, drm_enc->base.id, DPU_HW_BLK_CTL);
> +	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
> +		hw_ctl[i] = NULL;

Remove once hw_ctl is initialized properly

> +		if (!dpu_rm_get_hw(&dpu_kms->rm, &ctl_iter))
> +			break;
> +		hw_ctl[i] = (struct dpu_hw_ctl *)ctl_iter.hw;
> +	}
> +
>  	topology_name = dpu_rm_get_topology_name(topology);
>  	for (i = 0; i < dpu_enc->num_phys_encs; i++) {
>  		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
> @@ -1074,10 +1084,18 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>  		if (phys) {
>  			if (!dpu_enc->hw_pp[i]) {
>  				DPU_ERROR_ENC(dpu_enc,
> -				    "invalid pingpong block for the encoder\n");
> +				    "no pp block assigned at idx: %d\n", i);
>  				return;
>  			}
>  			phys->hw_pp = dpu_enc->hw_pp[i];
> +
> +			if (!hw_ctl[i]) {
> +				DPU_ERROR_ENC(dpu_enc,
> +				    "no ctl block assigned at idx: %d\n", i);
> +				return;
> +			}
> +			phys->hw_ctl = hw_ctl[i];
> +
>  			phys->connector = conn->state->connector;
>  			phys->topology_name = topology_name;
>  			if (phys->ops.mode_set)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> index c8c4612..5c89868 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> @@ -196,9 +196,6 @@ static void dpu_encoder_phys_cmd_mode_set(
>  {
>  	struct dpu_encoder_phys_cmd *cmd_enc =
>  		to_dpu_encoder_phys_cmd(phys_enc);
> -	struct dpu_rm *rm = &phys_enc->dpu_kms->rm;
> -	struct dpu_rm_hw_iter iter;
> -	int i, instance;
>  
>  	if (!phys_enc || !mode || !adj_mode) {
>  		DPU_ERROR("invalid args\n");
> @@ -208,22 +205,6 @@ static void dpu_encoder_phys_cmd_mode_set(
>  	DPU_DEBUG_CMDENC(cmd_enc, "caching mode:\n");
>  	drm_mode_debug_printmodeline(adj_mode);
>  
> -	instance = phys_enc->split_role == ENC_ROLE_SLAVE ? 1 : 0;
> -
> -	/* Retrieve previously allocated HW Resources. Shouldn't fail */
> -	dpu_rm_init_hw_iter(&iter, phys_enc->parent->base.id, DPU_HW_BLK_CTL);
> -	for (i = 0; i <= instance; i++) {
> -		if (dpu_rm_get_hw(rm, &iter))
> -			phys_enc->hw_ctl = (struct dpu_hw_ctl *)iter.hw;
> -	}
> -
> -	if (IS_ERR_OR_NULL(phys_enc->hw_ctl)) {
> -		DPU_ERROR_CMDENC(cmd_enc, "failed to init ctl: %ld\n",
> -				PTR_ERR(phys_enc->hw_ctl));
> -		phys_enc->hw_ctl = NULL;
> -		return;
> -	}
> -
>  	_dpu_encoder_phys_cmd_setup_irq_hw_idx(phys_enc);
>  }
>  
> 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 57ece03..c0221cc 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
> @@ -395,9 +395,6 @@ static void dpu_encoder_phys_vid_mode_set(
>  		struct drm_display_mode *mode,
>  		struct drm_display_mode *adj_mode)
>  {
> -	struct dpu_rm *rm;
> -	struct dpu_rm_hw_iter iter;
> -	int i, instance;
>  	struct dpu_encoder_phys_vid *vid_enc;
>  
>  	if (!phys_enc || !phys_enc->dpu_kms) {
> @@ -405,7 +402,6 @@ static void dpu_encoder_phys_vid_mode_set(
>  		return;
>  	}
>  
> -	rm = &phys_enc->dpu_kms->rm;
>  	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
>  
>  	if (adj_mode) {
> @@ -414,21 +410,6 @@ static void dpu_encoder_phys_vid_mode_set(
>  		DPU_DEBUG_VIDENC(vid_enc, "caching mode:\n");
>  	}
>  
> -	instance = phys_enc->split_role == ENC_ROLE_SLAVE ? 1 : 0;
> -
> -	/* Retrieve previously allocated HW Resources. Shouldn't fail */
> -	dpu_rm_init_hw_iter(&iter, phys_enc->parent->base.id, DPU_HW_BLK_CTL);
> -	for (i = 0; i <= instance; i++) {
> -		if (dpu_rm_get_hw(rm, &iter))
> -			phys_enc->hw_ctl = (struct dpu_hw_ctl *)iter.hw;
> -	}
> -	if (IS_ERR_OR_NULL(phys_enc->hw_ctl)) {
> -		DPU_ERROR_VIDENC(vid_enc, "failed to init ctl, %ld\n",
> -				PTR_ERR(phys_enc->hw_ctl));
> -		phys_enc->hw_ctl = NULL;
> -		return;
> -	}
> -
>  	_dpu_encoder_phys_vid_setup_irq_hw_idx(phys_enc);
>  }
>  
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH v3 07/13] drm/msm/dpu: avoid querying for hw intf before assignment

[Sean Paul]

On Tue, Aug 07, 2018 at 08:12:34PM -0700, Jeykumar Sankaran wrote:
> hw intf blocks are needed only during encoder enable to program
> timing engines(for video panels). encoder->enable is triggered
> only after atomic_modeset at which point we assign the
> resources for the display pipeline. This patch defers the
> hw_intf look-up until encoder enable.

I'm sure there's a good reason for this, but you haven't stated it. As a casual
reader of the patch, I'd be inclined to prefer doing work only once at init vs
everytime we enable.

> 
> changes in v2:
> 	- none
> changes in v3:
> 	- none
> 
> Change-Id: Ib0a2253431468151355e50cbad7b91e2b77b6e54
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> ---
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   | 53 +++++++---------------
>  1 file changed, 16 insertions(+), 37 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 c0221cc..a0b3744 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
> @@ -462,7 +462,7 @@ 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_hw_intf *intf;
> +	struct dpu_rm_hw_iter iter;
>  	struct dpu_hw_ctl *ctl;
>  	u32 flush_mask = 0;
>  
> @@ -474,11 +474,20 @@ 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);
> -	intf = vid_enc->hw_intf;
>  	ctl = phys_enc->hw_ctl;
> -	if (!vid_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);
> +
> +	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) {
> +			vid_enc->hw_intf = hw_intf;
> +			break;
> +		}
> +	}
> +
> +	if (!vid_enc->hw_intf) {
> +		DPU_ERROR("hw_intf not assigned\n");
>  		return;
>  	}
>  
> @@ -500,7 +509,7 @@ 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, intf->idx);
> +	ctl->ops.get_bitmask_intf(ctl, &flush_mask, vid_enc->hw_intf->idx);
>  	ctl->ops.update_pending_flush(ctl, flush_mask);
>  
>  skip_flush:
> @@ -531,22 +540,13 @@ static void dpu_encoder_phys_vid_get_hw_resources(
>  		struct dpu_encoder_hw_resources *hw_res,
>  		struct drm_connector_state *conn_state)
>  {
> -	struct dpu_encoder_phys_vid *vid_enc;
> -
>  	if (!phys_enc || !hw_res) {
>  		DPU_ERROR("invalid arg(s), enc %d hw_res %d conn_state %d\n",
>  				phys_enc != 0, hw_res != 0, conn_state != 0);
>  		return;
>  	}
>  
> -	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
> -	if (!vid_enc->hw_intf) {
> -		DPU_ERROR("invalid arg(s), hw_intf\n");
> -		return;
> -	}
> -
> -	DPU_DEBUG_VIDENC(vid_enc, "\n");
> -	hw_res->intfs[vid_enc->hw_intf->idx - INTF_0] = INTF_MODE_VIDEO;
> +	hw_res->intfs[phys_enc->intf_idx - INTF_0] = INTF_MODE_VIDEO;
>  }
>  
>  static int _dpu_encoder_phys_vid_wait_for_vblank(
> @@ -809,7 +809,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
>  {
>  	struct dpu_encoder_phys *phys_enc = NULL;
>  	struct dpu_encoder_phys_vid *vid_enc = NULL;
> -	struct dpu_rm_hw_iter iter;
>  	struct dpu_encoder_irq *irq;
>  	int i, ret = 0;
>  
> @@ -829,26 +828,6 @@ 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;
>  
> -	/**
> -	 * hw_intf resource permanently assigned to this encoder
> -	 * Other resources allocated at atomic commit time by use case
> -	 */
> -	dpu_rm_init_hw_iter(&iter, 0, DPU_HW_BLK_INTF);
> -	while (dpu_rm_get_hw(&p->dpu_kms->rm, &iter)) {
> -		struct dpu_hw_intf *hw_intf = (struct dpu_hw_intf *)iter.hw;
> -
> -		if (hw_intf->idx == p->intf_idx) {
> -			vid_enc->hw_intf = hw_intf;
> -			break;
> -		}
> -	}
> -
> -	if (!vid_enc->hw_intf) {
> -		ret = -EINVAL;
> -		DPU_ERROR("failed to get hw_intf\n");
> -		goto fail;
> -	}
> -
>  	DPU_DEBUG_VIDENC(vid_enc, "\n");
>  
>  	dpu_encoder_phys_vid_init_ops(&phys_enc->ops);
> -- 
> 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 v3 08/13] drm/msm/dpu: move hw resource tracking to crtc state

[Sean Paul]

On Tue, Aug 07, 2018 at 08:12:35PM -0700, Jeykumar Sankaran wrote:
> Prep changes for state based resource management.
> 
> Moves all the hw block tracking for the crtc to the state
> object.
> 
> changes in v2:
> 	- none
> changes in v3:
> 	- none
> 
> Change-Id: I2816e9e28b27f1126b477d62eb3858a30a652747
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 77 +++++++++++++++++---------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 25 +++++------
>  2 files changed, 54 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 1f2d223..515b0e6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -136,9 +136,9 @@ static void _dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc)
>  	crtc_state = to_dpu_crtc_state(crtc->state);
>  
>  	lm_horiz_position = 0;
> -	for (lm_idx = 0; lm_idx < dpu_crtc->num_mixers; lm_idx++) {
> +	for (lm_idx = 0; lm_idx < crtc_state->num_mixers; lm_idx++) {
>  		const struct drm_rect *lm_roi = &crtc_state->lm_bounds[lm_idx];
> -		struct dpu_hw_mixer *hw_lm = dpu_crtc->mixers[lm_idx].hw_lm;
> +		struct dpu_hw_mixer *hw_lm = crtc_state->mixers[lm_idx].hw_lm;
>  		struct dpu_hw_mixer_cfg cfg;
>  
>  		if (!lm_roi || !drm_rect_visible(lm_roi))
> @@ -219,7 +219,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
>  					   fb ? fb->modifier : 0);
>  
>  		/* blend config update */
> -		for (lm_idx = 0; lm_idx < dpu_crtc->num_mixers; lm_idx++) {
> +		for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) {
>  			_dpu_crtc_setup_blend_cfg(mixer + lm_idx, pstate);
>  
>  			mixer[lm_idx].flush_mask |= flush_mask;
> @@ -242,7 +242,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
>  static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
>  {
>  	struct dpu_crtc *dpu_crtc;
> -	struct dpu_crtc_state *dpu_crtc_state;
> +	struct dpu_crtc_state *cstate;

Unrelated var renames are kind of noisy, but I suppose it's just one, so that's
ok.

>  	struct dpu_crtc_mixer *mixer;
>  	struct dpu_hw_ctl *ctl;
>  	struct dpu_hw_mixer *lm;
> @@ -253,17 +253,17 @@ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
>  		return;
>  
>  	dpu_crtc = to_dpu_crtc(crtc);
> -	dpu_crtc_state = to_dpu_crtc_state(crtc->state);
> -	mixer = dpu_crtc->mixers;
> +	cstate = to_dpu_crtc_state(crtc->state);
> +	mixer = cstate->mixers;
>  
>  	DPU_DEBUG("%s\n", dpu_crtc->name);
>  
> -	if (dpu_crtc->num_mixers > CRTC_DUAL_MIXERS) {
> -		DPU_ERROR("invalid number mixers: %d\n", dpu_crtc->num_mixers);
> +	if (cstate->num_mixers > CRTC_DUAL_MIXERS) {
> +		DPU_ERROR("invalid number mixers: %d\n", cstate->num_mixers);
>  		return;
>  	}
>  
> -	for (i = 0; i < dpu_crtc->num_mixers; i++) {
> +	for (i = 0; i < cstate->num_mixers; i++) {
>  		if (!mixer[i].hw_lm || !mixer[i].hw_ctl) {
>  			DPU_ERROR("invalid lm or ctl assigned to mixer\n");
>  			return;
> @@ -280,7 +280,7 @@ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
>  
>  	_dpu_crtc_blend_setup_mixer(crtc, dpu_crtc, mixer);
>  
> -	for (i = 0; i < dpu_crtc->num_mixers; i++) {
> +	for (i = 0; i < cstate->num_mixers; i++) {
>  		ctl = mixer[i].hw_ctl;
>  		lm = mixer[i].hw_lm;
>  
> @@ -502,7 +502,7 @@ static void _dpu_crtc_setup_mixer_for_encoder(
>  		struct drm_crtc *crtc,
>  		struct drm_encoder *enc)
>  {
> -	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> +	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;
> @@ -514,8 +514,8 @@ static void _dpu_crtc_setup_mixer_for_encoder(
>  	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 = dpu_crtc->num_mixers; i < ARRAY_SIZE(dpu_crtc->mixers); i++) {
> -		mixer = &dpu_crtc->mixers[i];
> +	for (i = cstate->num_mixers; i < ARRAY_SIZE(cstate->mixers); i++) {
> +		mixer = &cstate->mixers[i];
>  
>  		if (!dpu_rm_get_hw(rm, &lm_iter))
>  			break;
> @@ -540,7 +540,7 @@ static void _dpu_crtc_setup_mixer_for_encoder(
>  
>  		mixer->encoder = enc;
>  
> -		dpu_crtc->num_mixers++;
> +		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",
> @@ -551,11 +551,11 @@ static void _dpu_crtc_setup_mixer_for_encoder(
>  static void _dpu_crtc_setup_mixers(struct drm_crtc *crtc)
>  {
>  	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> +	struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
>  	struct drm_encoder *enc;
>  
> -	dpu_crtc->num_mixers = 0;
> -	dpu_crtc->mixers_swapped = false;
> -	memset(dpu_crtc->mixers, 0, sizeof(dpu_crtc->mixers));
> +	cstate->num_mixers = 0;
> +	memset(cstate->mixers, 0, sizeof(cstate->mixers));
>  
>  	mutex_lock(&dpu_crtc->crtc_lock);
>  	/* Check for mixers on all encoders attached to this crtc */
> @@ -589,7 +589,7 @@ static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc,
>  	adj_mode = &state->adjusted_mode;
>  	crtc_split_width = dpu_crtc_get_mixer_width(dpu_crtc, cstate, adj_mode);
>  
> -	for (i = 0; i < dpu_crtc->num_mixers; i++) {
> +	for (i = 0; i < cstate->num_mixers; i++) {
>  		struct drm_rect *r = &cstate->lm_bounds[i];
>  		r->x1 = crtc_split_width * i;
>  		r->y1 = 0;
> @@ -606,6 +606,7 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc,
>  		struct drm_crtc_state *old_state)
>  {
>  	struct dpu_crtc *dpu_crtc;
> +	struct dpu_crtc_state *cstate;
>  	struct drm_encoder *encoder;
>  	struct drm_device *dev;
>  	unsigned long flags;
> @@ -625,10 +626,11 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc,
>  	DPU_DEBUG("crtc%d\n", crtc->base.id);
>  
>  	dpu_crtc = to_dpu_crtc(crtc);
> +	cstate = to_dpu_crtc_state(crtc->state);
>  	dev = crtc->dev;
>  	smmu_state = &dpu_crtc->smmu_state;
>  
> -	if (!dpu_crtc->num_mixers) {
> +	if (!cstate->num_mixers) {
>  		_dpu_crtc_setup_mixers(crtc);
>  		_dpu_crtc_setup_lm_bounds(crtc, crtc->state);
>  	}
> @@ -655,7 +657,7 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc,
>  	 * it means we are trying to flush a CRTC whose state is disabled:
>  	 * nothing else needs to be done.
>  	 */
> -	if (unlikely(!dpu_crtc->num_mixers))
> +	if (unlikely(!cstate->num_mixers))
>  		return;
>  
>  	_dpu_crtc_blend_setup(crtc);
> @@ -719,7 +721,7 @@ static void dpu_crtc_atomic_flush(struct drm_crtc *crtc,
>  	 * it means we are trying to flush a CRTC whose state is disabled:
>  	 * nothing else needs to be done.
>  	 */
> -	if (unlikely(!dpu_crtc->num_mixers))
> +	if (unlikely(!cstate->num_mixers))
>  		return;
>  
>  	/*
> @@ -834,7 +836,7 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
>  	 * it means we are trying to start a CRTC whose state is disabled:
>  	 * nothing else needs to be done.
>  	 */
> -	if (unlikely(!dpu_crtc->num_mixers))
> +	if (unlikely(!cstate->num_mixers))
>  		return;
>  
>  	DPU_ATRACE_BEGIN("crtc_commit");
> @@ -1069,6 +1071,7 @@ static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
>  	struct dpu_crtc *dpu_crtc;
>  	struct drm_encoder *encoder;
>  	struct dpu_crtc_mixer *m;
> +	struct dpu_crtc_state *cstate;
>  	u32 i, misr_status;
>  
>  	if (!crtc) {
> @@ -1076,6 +1079,7 @@ static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
>  		return;
>  	}
>  	dpu_crtc = to_dpu_crtc(crtc);
> +	cstate = to_dpu_crtc_state(dpu_crtc->base.state);
>  
>  	mutex_lock(&dpu_crtc->crtc_lock);
>  
> @@ -1091,8 +1095,8 @@ static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
>  			dpu_encoder_virt_restore(encoder);
>  		}
>  
> -		for (i = 0; i < dpu_crtc->num_mixers; ++i) {
> -			m = &dpu_crtc->mixers[i];
> +		for (i = 0; i < cstate->num_mixers; ++i) {
> +			m = &cstate->mixers[i];
>  			if (!m->hw_lm || !m->hw_lm->ops.setup_misr ||
>  					!dpu_crtc->misr_enable)
>  				continue;
> @@ -1102,8 +1106,8 @@ static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
>  		}
>  		break;
>  	case DPU_POWER_EVENT_PRE_DISABLE:
> -		for (i = 0; i < dpu_crtc->num_mixers; ++i) {
> -			m = &dpu_crtc->mixers[i];
> +		for (i = 0; i < cstate->num_mixers; ++i) {
> +			m = &cstate->mixers[i];
>  			if (!m->hw_lm || !m->hw_lm->ops.collect_misr ||
>  					!dpu_crtc->misr_enable)
>  				continue;
> @@ -1191,9 +1195,8 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
>  		dpu_power_handle_unregister_event(dpu_crtc->phandle,
>  				dpu_crtc->power_event);
>  
> -	memset(dpu_crtc->mixers, 0, sizeof(dpu_crtc->mixers));
> -	dpu_crtc->num_mixers = 0;
> -	dpu_crtc->mixers_swapped = false;
> +	memset(cstate->mixers, 0, sizeof(cstate->mixers));
> +	cstate->num_mixers = 0;
>  
>  	/* disable clk & bw control until clk & bw properties are set */
>  	cstate->bw_control = false;
> @@ -1552,8 +1555,8 @@ static int _dpu_debugfs_status_show(struct seq_file *s, void *data)
>  
>  	seq_puts(s, "\n");
>  
> -	for (i = 0; i < dpu_crtc->num_mixers; ++i) {
> -		m = &dpu_crtc->mixers[i];
> +	for (i = 0; i < cstate->num_mixers; ++i) {
> +		m = &cstate->mixers[i];

Hmmm, the state accesses in this function aren't properly serialized. Could you
please whip up a patch to acquire the modeset locks?

>  		if (!m->hw_lm)
>  			seq_printf(s, "\tmixer[%d] has no lm\n", i);
>  		else if (!m->hw_ctl)
> @@ -1646,6 +1649,7 @@ static ssize_t _dpu_crtc_misr_setup(struct file *file,
>  		const char __user *user_buf, size_t count, loff_t *ppos)
>  {
>  	struct dpu_crtc *dpu_crtc;
> +	struct dpu_crtc_state *cstate;
>  	struct dpu_crtc_mixer *m;
>  	int i = 0, rc;
>  	char buf[MISR_BUFF_SIZE + 1];
> @@ -1656,6 +1660,7 @@ static ssize_t _dpu_crtc_misr_setup(struct file *file,
>  		return -EINVAL;
>  
>  	dpu_crtc = file->private_data;
> +	cstate = to_dpu_crtc_state(dpu_crtc->base.state);

Same here

>  	buff_copy = min_t(size_t, count, MISR_BUFF_SIZE);
>  	if (copy_from_user(buf, user_buf, buff_copy)) {
>  		DPU_ERROR("buffer copy failed\n");
> @@ -1674,9 +1679,9 @@ static ssize_t _dpu_crtc_misr_setup(struct file *file,
>  	mutex_lock(&dpu_crtc->crtc_lock);
>  	dpu_crtc->misr_enable = enable;
>  	dpu_crtc->misr_frame_count = frame_count;
> -	for (i = 0; i < dpu_crtc->num_mixers; ++i) {
> +	for (i = 0; i < cstate->num_mixers; ++i) {
>  		dpu_crtc->misr_data[i] = 0;
> -		m = &dpu_crtc->mixers[i];
> +		m = &cstate->mixers[i];
>  		if (!m->hw_lm || !m->hw_lm->ops.setup_misr)
>  			continue;
>  
> @@ -1692,6 +1697,7 @@ static ssize_t _dpu_crtc_misr_read(struct file *file,
>  		char __user *user_buff, size_t count, loff_t *ppos)
>  {
>  	struct dpu_crtc *dpu_crtc;
> +	struct dpu_crtc_state *cstate;
>  	struct dpu_crtc_mixer *m;
>  	int i = 0, rc;
>  	u32 misr_status;
> @@ -1705,6 +1711,7 @@ static ssize_t _dpu_crtc_misr_read(struct file *file,
>  		return -EINVAL;
>  
>  	dpu_crtc = file->private_data;
> +	cstate = to_dpu_crtc_state(dpu_crtc->base.state);

And here

>  	rc = _dpu_crtc_power_enable(dpu_crtc, true);
>  	if (rc)
>  		return rc;
> @@ -1716,8 +1723,8 @@ static ssize_t _dpu_crtc_misr_read(struct file *file,
>  		goto buff_check;
>  	}
>  
> -	for (i = 0; i < dpu_crtc->num_mixers; ++i) {
> -		m = &dpu_crtc->mixers[i];
> +	for (i = 0; i < cstate->num_mixers; ++i) {
> +		m = &cstate->mixers[i];
>  		if (!m->hw_lm || !m->hw_lm->ops.collect_misr)
>  			continue;
>  
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index e632651..9177ee6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -121,11 +121,6 @@ struct dpu_crtc_frame_event {
>   * struct dpu_crtc - virtualized CRTC data structure
>   * @base          : Base drm crtc structure
>   * @name          : ASCII description of this crtc
> - * @num_ctls      : Number of ctl paths in use
> - * @num_mixers    : Number of mixers in use
> - * @mixers_swapped: Whether the mixers have been swapped for left/right update
> - *                  especially in the case of DSC Merge.
> - * @mixers        : List of active mixers
>   * @event         : Pointer to last received drm vblank event. If there is a
>   *                  pending vblank event, this will be non-null.
>   * @vsync_count   : Running count of received vsync events
> @@ -167,12 +162,6 @@ struct dpu_crtc {
>  	struct drm_crtc base;
>  	char name[DPU_CRTC_NAME_SIZE];
>  
> -	/* HW Resources reserved for the crtc */
> -	u32 num_ctls;
> -	u32 num_mixers;
> -	bool mixers_swapped;
> -	struct dpu_crtc_mixer mixers[CRTC_DUAL_MIXERS];
> -
>  	struct drm_pending_vblank_event *event;
>  	u32 vsync_count;
>  
> @@ -227,6 +216,10 @@ struct dpu_crtc {
>   * @property_values: Current crtc property values
>   * @input_fence_timeout_ns : Cached input fence timeout, in ns
>   * @new_perf: new performance state being requested
> + * @num_mixers    : Number of mixers in use
> + * @mixers        : List of active mixers
> + * @num_ctls      : Number of ctl paths in use
> + * @hw_ctls       : List of activel ctl paths

activel

>   */
>  struct dpu_crtc_state {
>  	struct drm_crtc_state base;
> @@ -236,8 +229,14 @@ struct dpu_crtc_state {
>  	struct drm_rect lm_bounds[CRTC_DUAL_MIXERS];
>  
>  	uint64_t input_fence_timeout_ns;
> -

unrelated whitespace

>  	struct dpu_core_perf_params new_perf;
> +
> +	/* HW Resources reserved for the crtc */
> +	u32 num_mixers;
> +	struct dpu_crtc_mixer mixers[CRTC_DUAL_MIXERS];
> +
> +	u32 num_ctls;
> +	struct dpu_hw_ctl *hw_ctls[CRTC_DUAL_MIXERS];
>  };
>  
>  #define to_dpu_crtc_state(x) \
> @@ -255,7 +254,7 @@ static inline int dpu_crtc_get_mixer_width(struct dpu_crtc *dpu_crtc,
>  	if (!dpu_crtc || !cstate || !mode)
>  		return 0;
>  
> -	mixer_width = (dpu_crtc->num_mixers == CRTC_DUAL_MIXERS ?
> +	mixer_width = (cstate->num_mixers == CRTC_DUAL_MIXERS ?
>  			mode->hdisplay / CRTC_DUAL_MIXERS : mode->hdisplay);

Can you please add a new patch that precedes this to move this function into
dpu_crtc as a static function?

Sean

>  
>  	return mixer_width;
> -- 
> 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 v3 09/13] drm/msm/dpu: rename hw_ctl to lm_ctl

[Sean Paul]

On Tue, Aug 07, 2018 at 08:12:36PM -0700, Jeykumar Sankaran wrote:
> Prep change for state based resource management.
> 
> Rename hw_ctl to lm_ctl to mean the ctl associated
> with the hw layer mixer block.

Did you do this via spatch, sed, etc? Rename patches should contain the
invocation to reproduce them since they have a nasty habit of introducing bugs
and compilation warnings/errors.

Sean

> 
> changes in v2:
> 	- none
> changes in v3:
> 	- none
> 
> Change-Id: If6e6249e089b89225cdfafe9158f66667509e97b
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 26 +++++++++++++-------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h |  4 ++--
>  2 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 515b0e6..0eb369c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -175,7 +175,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
>  		return;
>  	}
>  
> -	ctl = mixer->hw_ctl;
> +	ctl = mixer->lm_ctl;
>  	lm = mixer->hw_lm;
>  	stage_cfg = &dpu_crtc->stage_cfg;
>  	cstate = to_dpu_crtc_state(crtc->state);
> @@ -264,15 +264,15 @@ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
>  	}
>  
>  	for (i = 0; i < cstate->num_mixers; i++) {
> -		if (!mixer[i].hw_lm || !mixer[i].hw_ctl) {
> +		if (!mixer[i].hw_lm || !mixer[i].lm_ctl) {
>  			DPU_ERROR("invalid lm or ctl assigned to mixer\n");
>  			return;
>  		}
>  		mixer[i].mixer_op_mode = 0;
>  		mixer[i].flush_mask = 0;
> -		if (mixer[i].hw_ctl->ops.clear_all_blendstages)
> -			mixer[i].hw_ctl->ops.clear_all_blendstages(
> -					mixer[i].hw_ctl);
> +		if (mixer[i].lm_ctl->ops.clear_all_blendstages)
> +			mixer[i].lm_ctl->ops.clear_all_blendstages(
> +					mixer[i].lm_ctl);
>  	}
>  
>  	/* initialize stage cfg */
> @@ -281,7 +281,7 @@ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
>  	_dpu_crtc_blend_setup_mixer(crtc, dpu_crtc, mixer);
>  
>  	for (i = 0; i < cstate->num_mixers; i++) {
> -		ctl = mixer[i].hw_ctl;
> +		ctl = mixer[i].lm_ctl;
>  		lm = mixer[i].hw_lm;
>  
>  		lm->ops.setup_alpha_out(lm, mixer[i].mixer_op_mode);
> @@ -525,14 +525,14 @@ static void _dpu_crtc_setup_mixer_for_encoder(
>  		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->hw_ctl = last_valid_ctl;
> +			mixer->lm_ctl = last_valid_ctl;
>  		} else {
> -			mixer->hw_ctl = (struct dpu_hw_ctl *)ctl_iter.hw;
> -			last_valid_ctl = mixer->hw_ctl;
> +			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->hw_ctl) {
> +		if (!mixer->lm_ctl) {
>  			DPU_ERROR("no valid ctls found for lm %d\n",
>  					mixer->hw_lm->idx - LM_0);
>  			return;
> @@ -544,7 +544,7 @@ static void _dpu_crtc_setup_mixer_for_encoder(
>  		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->hw_ctl->idx - CTL_0);
> +				i, mixer->lm_ctl->idx - CTL_0);
>  	}
>  }
>  
> @@ -1559,11 +1559,11 @@ static int _dpu_debugfs_status_show(struct seq_file *s, void *data)
>  		m = &cstate->mixers[i];
>  		if (!m->hw_lm)
>  			seq_printf(s, "\tmixer[%d] has no lm\n", i);
> -		else if (!m->hw_ctl)
> +		else if (!m->lm_ctl)
>  			seq_printf(s, "\tmixer[%d] has no ctl\n", i);
>  		else
>  			seq_printf(s, "\tmixer:%d ctl:%d width:%d height:%d\n",
> -				m->hw_lm->idx - LM_0, m->hw_ctl->idx - CTL_0,
> +				m->hw_lm->idx - LM_0, m->lm_ctl->idx - CTL_0,
>  				out_width, mode->vdisplay);
>  	}
>  
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index 9177ee6..5b85ca8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -83,14 +83,14 @@ struct dpu_crtc_smmu_state_data {
>  /**
>   * struct dpu_crtc_mixer: stores the map for each virtual pipeline in the CRTC
>   * @hw_lm:	LM HW Driver context
> - * @hw_ctl:	CTL Path HW driver context
> + * @lm_ctl:	CTL Path HW driver context
>   * @encoder:	Encoder attached to this lm & ctl
>   * @mixer_op_mode:	mixer blending operation mode
>   * @flush_mask:	mixer flush mask for ctl, mixer and pipe
>   */
>  struct dpu_crtc_mixer {
>  	struct dpu_hw_mixer *hw_lm;
> -	struct dpu_hw_ctl *hw_ctl;
> +	struct dpu_hw_ctl *lm_ctl;
>  	struct drm_encoder *encoder;
>  	u32 mixer_op_mode;
>  	u32 flush_mask;
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH v3 04/13] drm/msm/dpu: program master-slave encoders explicitly

[Jeykumar Sankaran]

On 2018-08-14 12:19, Sean Paul wrote:
> On Tue, Aug 07, 2018 at 08:12:31PM -0700, Jeykumar Sankaran wrote:
>> Identify slave-master encoders and program them explicitly.
> 
> You've got the what, but could you please explain the why?
> 
>> 
>> changes in v2:
>> 	- none
>> changes in v3:
>> 	- none
>> 
>> Change-Id: I0ebfada05bd7f8437f842ad860490a678aa8f4cd
>> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
>> ---
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 39
> ++++++++++++++++++-----------
>>  1 file changed, 24 insertions(+), 15 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> index 1b4de34..a0ced79 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> @@ -184,6 +184,7 @@ struct dpu_encoder_virt {
>>  	unsigned int num_phys_encs;
>>  	struct dpu_encoder_phys *phys_encs[MAX_PHYS_ENCODERS_PER_VIRTUAL];
>>  	struct dpu_encoder_phys *cur_master;
>> +	struct dpu_encoder_phys *cur_slave;
> 
> You only use this in one function, why not just make it a local?
> 
>>  	struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
>> 
>>  	bool intfs_swapped;
>> @@ -1153,6 +1154,7 @@ void dpu_encoder_virt_restore(struct drm_encoder
> *drm_enc)
>>  static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
>>  {
>>  	struct dpu_encoder_virt *dpu_enc = NULL;
>> +	struct dpu_encoder_phys *phys  = NULL;
>>  	int i, ret = 0;
>>  	struct drm_display_mode *cur_mode = NULL;
>> 
>> @@ -1160,6 +1162,7 @@ static void dpu_encoder_virt_enable(struct
> drm_encoder *drm_enc)
>>  		DPU_ERROR("invalid encoder\n");
>>  		return;
>>  	}
>> +
>>  	dpu_enc = to_dpu_encoder_virt(drm_enc);
>>  	cur_mode = &dpu_enc->base.crtc->state->adjusted_mode;
>> 
>> @@ -1167,21 +1170,36 @@ static void dpu_encoder_virt_enable(struct
> drm_encoder *drm_enc)
>>  			     cur_mode->vdisplay);
>> 
>>  	dpu_enc->cur_master = NULL;
>> +	dpu_enc->cur_slave = NULL;
> 
> There's no benefit to setting this NULL. cur_master is set to NULL so 
> it
> can be
> checked after the loop. Since you're not checking this, it's not
> necessary.
> 
Checking slave encoder below.
> I suppose you might also want to clear this on disable like master.
> 
>>  	for (i = 0; i < dpu_enc->num_phys_encs; i++) {
>> -		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
>> +		phys = dpu_enc->phys_encs[i];
>> +
>> +		if (!phys || !phys->ops.is_master)
> 
> I don't think it's possible for phys to be NULL, is it?
> 
>> +			continue;
>> 
>> -		if (phys && phys->ops.is_master &&
> phys->ops.is_master(phys)) {
>> -			DPU_DEBUG_ENC(dpu_enc, "master is now idx %d\n",
> i);
>> +		if (phys->ops.is_master(phys)) {
>> +			DPU_DEBUG_ENC(dpu_enc, "master is at idx %d\n",
> i);
>>  			dpu_enc->cur_master = phys;
>> -			break;
>> +		} else {
>> +			DPU_DEBUG_ENC(dpu_enc, "slave is at idx %d\n", i);
>> +			dpu_enc->cur_slave = phys;
>>  		}
> 
> You're making an assumption here that there can only be one master and
> there can
> only be one slave.
> 
Isn't that a fact? Do we have a topology in DPU where we have more than 
one master or slave?

> It seems like you could avoid all of this work if you just did the
> assignment in
> dpu_encoder_virt_add_phys_encs().
> 
That is true! Let me try doing that.
>>  	}
>> 
>>  	if (!dpu_enc->cur_master) {
>> -		DPU_ERROR("virt encoder has no master! num_phys %d\n", i);
>> +		DPU_ERROR("virt encoder has no master identified\n");
>>  		return;
>>  	}
>> 
>> +	/* always enable slave encoder before master */
>> +	phys = dpu_enc->cur_slave;
>> +	if (phys && phys->ops.enable)
>> +		phys->ops.enable(phys);
>> +
We are checking for slave encoder being NULL here.
>> +	phys = dpu_enc->cur_master;
>> +	if (phys && phys->ops.enable)
>> +		phys->ops.enable(phys);
>> +
>>  	ret = dpu_encoder_resource_control(drm_enc,
> DPU_ENC_RC_EVENT_KICKOFF);
>>  	if (ret) {
>>  		DPU_ERROR_ENC(dpu_enc, "dpu resource control failed:
> %d\n",
>> @@ -1190,25 +1208,16 @@ static void dpu_encoder_virt_enable(struct
> drm_encoder *drm_enc)
>>  	}
>> 
>>  	for (i = 0; i < dpu_enc->num_phys_encs; i++) {
>> -		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
>> -
>> +		phys = dpu_enc->phys_encs[i];
>>  		if (!phys)
>>  			continue;
>> 
>> -		if (phys != dpu_enc->cur_master) {
>> -			if (phys->ops.enable)
>> -				phys->ops.enable(phys);
>> -		}
>> -
>>  		if (dpu_enc->misr_enable &&
> (dpu_enc->disp_info.capabilities &
>>  		     MSM_DISPLAY_CAP_VID_MODE) && phys->ops.setup_misr)
>>  			phys->ops.setup_misr(phys, true,
>> 
> dpu_enc->misr_frame_count);
>>  	}
>> 
>> -	if (dpu_enc->cur_master->ops.enable)
>> -		dpu_enc->cur_master->ops.enable(dpu_enc->cur_master);
>> -
> 
> There's a change in functionality here. Previously you could call
> setup_misr
> for slaves after they are enabled, but before master is enabled. Now
> you're
> calling it after all are enabled.
> 
> I'm guessing it doesn't matter since it was previously called on master
> before
> it was enabled, but I figure I'd point this out.
> 
> Sean
> 
Yes. It doesn't matter here.
>>  	_dpu_encoder_virt_enable_helper(drm_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