Doing a for loop in every DPU HW block driver init to find a catalog
entry matching the given ID is rather useless if the init function
called by RM already has that catalog entry pointer, and uses exactly
its ID to drive this init and for loop. Remove all that machinery to
drop quite some lines of unnecessarily-complicated code, and the
fallibility that comes with it, by simply giving _init() the catalog
entry pointers straight away.
Also clean up some unused struct members, and assign a log_mask for WB.
---
Changes in v3:
- Return NULL from dpu_hw_intf_init() on INTF_NONE, and omit identical
check in dpu_rm_init();
- Retitle patch 3/3.
v2: https://lore.kernel.org/r/20230418-dpu-drop-useless-for-lookup-v2-0-acb08e82ef19@somainline.org
Changes in v2:
- Replace "sysfs" with "debugfs" in patch 2/3 description;
- Reorder WB log_mask fix as patch 1;
- Drop Fixes: tags from "unused struct members removal";
- Replace dpu_mdss_cfg parameter in dpu_hw_sspp_init() with immediate
dpu_ubwc_cfg pointer;
- Replace dpu_mdss_cfg parameter in dpu_hw_ctl_init() with immediate
dpu_lm_cfg array pointer.
v1: https://lore.kernel.org/r/20230418-dpu-drop-useless-for-lookup-v1-0-b9897ceb6f3e@somainline.org
---
Marijn Suijten (3):
drm/msm/dpu: Assign missing writeback log_mask
drm/msm/dpu: Drop unused members from HW structs
drm/msm/dpu: Pass catalog pointers in RM to replace for-loop ID lookups
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 37 +++++---------------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 14 ++++----
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 32 +++---------------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 11 +++---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c | 38 ++++-----------------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.h | 12 +++----
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 41 ++++++-----------------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 13 +++----
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 38 ++++-----------------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h | 10 +++---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_merge3d.c | 33 +++---------------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_merge3d.h | 14 ++++----
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 33 +++---------------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h | 14 ++++----
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 40 ++++------------------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 14 ++++----
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_vbif.c | 33 +++---------------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_vbif.h | 11 +++---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 33 ++++--------------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 16 +++------
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 17 +++++-----
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 22 +++++-------
23 files changed, 139 insertions(+), 389 deletions(-)
---
base-commit: f600e0bbde8562a06bee31b3eb1b69d49acac4c5
change-id: 20230418-dpu-drop-useless-for-lookup-488700c7a469
Best regards,
--
Marijn Suijten <[email protected]>
Some of these members were initialized while never read, while others
were not even assigned any pointer value at all. Drop them to save some
space, and above all confusion when looking at or accidentally
dereferencing these members.
Signed-off-by: Marijn Suijten <[email protected]>
Reviewed-by: Abhinav Kumar <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 1 -
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 1 -
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 1 -
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 2 --
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 1 -
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 5 -----
6 files changed, 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
index 84ee2efa9c664..a9c90249a6aca 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
@@ -374,7 +374,6 @@ struct dpu_hw_intf *dpu_hw_intf_init(enum dpu_intf idx,
*/
c->idx = idx;
c->cap = cfg;
- c->mdss = m;
_setup_intf_ops(&c->ops, c->cap->features);
return c;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
index 643dd10bc0306..e07b2e33af3e2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
@@ -90,7 +90,6 @@ struct dpu_hw_intf {
/* intf */
enum dpu_intf idx;
const struct dpu_intf_cfg *cap;
- const struct dpu_mdss_cfg *mdss;
/* ops */
struct dpu_hw_intf_ops ops;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
index cf70a9bd1034f..bb3ddec5c7d73 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -812,7 +812,6 @@ struct dpu_hw_sspp *dpu_hw_sspp_init(enum dpu_sspp idx,
}
/* Assign ops */
- hw_pipe->catalog = catalog;
hw_pipe->ubwc = catalog->ubwc;
hw_pipe->idx = idx;
hw_pipe->cap = cfg;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
index 74b98b6b3bc3a..5004a02fd61e2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
@@ -341,7 +341,6 @@ struct dpu_hw_sspp_ops {
* struct dpu_hw_sspp - pipe description
* @base: hardware block base structure
* @hw: block hardware details
- * @catalog: back pointer to catalog
* @ubwc: ubwc configuration data
* @idx: pipe index
* @cap: pointer to layer_cfg
@@ -350,7 +349,6 @@ struct dpu_hw_sspp_ops {
struct dpu_hw_sspp {
struct dpu_hw_blk base;
struct dpu_hw_blk_reg_map hw;
- const struct dpu_mdss_cfg *catalog;
const struct dpu_ubwc_cfg *ubwc;
/* Pipe */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
index a3e413d277175..f33b48045b5c7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
@@ -263,7 +263,6 @@ struct dpu_hw_wb *dpu_hw_wb_init(enum dpu_wb idx,
}
/* Assign ops */
- c->mdp = &m->mdp[0];
c->idx = idx;
c->caps = cfg;
_setup_wb_ops(&c->ops, c->caps->features);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
index 3ff5a48541e2e..b91923f879f13 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
@@ -76,15 +76,12 @@ struct dpu_hw_wb_ops {
/**
* struct dpu_hw_wb : WB driver object
* @hw: block hardware details
- * @mdp: pointer to associated mdp portion of the catalog
* @idx: hardware index number within type
* @wb_hw_caps: hardware capabilities
* @ops: function pointers
- * @hw_mdp: MDP top level hardware block
*/
struct dpu_hw_wb {
struct dpu_hw_blk_reg_map hw;
- const struct dpu_mdp_cfg *mdp;
/* wb path */
int idx;
@@ -92,8 +89,6 @@ struct dpu_hw_wb {
/* ops */
struct dpu_hw_wb_ops ops;
-
- struct dpu_hw_mdp *hw_mdp;
};
/**
--
2.40.1
The WB debug log mask ended up never being assigned, leading to writes
to this block to never be logged even if the mask is enabled in
dpu_hw_util_log_mask via debugfs.
Fixes: 84a33d0fd921 ("drm/msm/dpu: add dpu_hw_wb abstraction for writeback blocks")
Signed-off-by: Marijn Suijten <[email protected]>
Reviewed-by: Abhinav Kumar <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
index 2d28afdf860ef..a3e413d277175 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
@@ -61,6 +61,7 @@ static const struct dpu_wb_cfg *_wb_offset(enum dpu_wb wb,
for (i = 0; i < m->wb_count; i++) {
if (wb == m->wb[i].id) {
b->blk_addr = addr + m->wb[i].base;
+ b->log_mask = DPU_DBG_MASK_WB;
return &m->wb[i];
}
}
--
2.40.1
On Wed, 26 Apr 2023 01:11:08 +0200, Marijn Suijten wrote:
> Doing a for loop in every DPU HW block driver init to find a catalog
> entry matching the given ID is rather useless if the init function
> called by RM already has that catalog entry pointer, and uses exactly
> its ID to drive this init and for loop. Remove all that machinery to
> drop quite some lines of unnecessarily-complicated code, and the
> fallibility that comes with it, by simply giving _init() the catalog
> entry pointers straight away.
>
> [...]
Applied, thanks!
[1/3] drm/msm/dpu: Assign missing writeback log_mask
https://gitlab.freedesktop.org/abhinavk/msm/-/commit/a432fc31f03d
Best regards,
--
Abhinav Kumar <[email protected]>