On 4/21/2023 1:53 PM, Marijn Suijten wrote:
The Resource Manager already iterates over all available blocks from the
catalog, only to pass their ID to a dpu_hw_xxx_init() function which
uses an _xxx_offset() helper to search for and find the exact same
catalog pointer again to initialize the block with, fallible error
handling and all.
Instead, pass const pointers to the catalog entries directly to these
_init functions and drop the for loops entirely, saving on both
readability complexity and unnecessary cycles at boot.
Signed-off-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
Overall, a nice cleanup!
One comment below.
---
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 | 40 ++++++-----------------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 12 +++----
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 | 39 ++++------------------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 12 +++----
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 | 11 +++----
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 17 +++++-----
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 18 +++++-----
23 files changed, 139 insertions(+), 375 deletions(-)
<snipped>
-struct dpu_hw_intf *dpu_hw_intf_init(enum dpu_intf idx,
- void __iomem *addr,
- const struct dpu_mdss_cfg *m)
+struct dpu_hw_intf *dpu_hw_intf_init(const struct dpu_intf_cfg *cfg,
+ void __iomem *addr)
{
struct dpu_hw_intf *c;
- const struct dpu_intf_cfg *cfg;
+
+ if (cfg->type == INTF_NONE) {
+ pr_err("Cannot create interface hw object for INTF_NONE type\n");
+ return ERR_PTR(-EINVAL);
+ }
The caller of dpu_hw_intf_init which is the RM already has protection
for INTF_NONE, see below
for (i = 0; i < cat->intf_count; i++) {
struct dpu_hw_intf *hw;
const struct dpu_intf_cfg *intf = &cat->intf[i];
if (intf->type == INTF_NONE) {
DPU_DEBUG("skip intf %d with type none\n", i);
continue;
}
if (intf->id < INTF_0 || intf->id >= INTF_MAX) {
DPU_ERROR("skip intf %d with invalid id\n",
intf->id);
continue;
}
hw = dpu_hw_intf_init(intf->id, mmio, cat);
So this part can be dropped.
c = kzalloc(sizeof(*c), GFP_KERNEL);
if (!c)
return ERR_PTR(-ENOMEM);
- cfg = _intf_offset(idx, m, addr, &c->hw);
- if (IS_ERR_OR_NULL(cfg)) {
- kfree(c);
- pr_err("failed to create dpu_hw_intf %d\n", idx);
- return ERR_PTR(-EINVAL);
- }
+ c->hw.blk_addr = addr + cfg->base;
+ c->hw.log_mask = DPU_DBG_MASK_INTF;
<snipped>
