On 2023-07-29 02:31:58, Dmitry Baryshkov wrote:
> The size of the irq table is static, it has MDP_INTR_MAX * 32 interrupt
> entries. Provide the fixed length and drop struct_size() statement.
Good call. I thought it was supposed to be allocated based on enabled
interrupts (but then indexing becomes a big problem) to lighten the
tables on platforms where the higher registers aren't even used, but it
was always the constant MDP_INTR_MAX * 32. It's even better now that
you removed all the 7xxx defines of which there were 11, times 32
interrupts, times sizeof(dpu_hw_intr_entry)=20 (or 24).
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 13 +++++--------
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 6 +++---
> 2 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> index 14d374de30c5..308b122059cd 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> @@ -203,7 +203,7 @@ static const struct dpu_intr_reg dpu_intr_set_7xxx[] = {
> static inline bool dpu_core_irq_is_valid(struct dpu_hw_intr *intr,
Now you can drop the intr argument :)
After fixing that:
Reviewed-by: Marijn Suijten <marijn.suijten at somainline.org>
> int irq_idx)
> {
> - return irq_idx >= 0 && irq_idx < intr->total_irqs;
> + return irq_idx >= 0 && irq_idx < DPU_NUM_IRQS;
> }
>
> static inline struct dpu_hw_intr_entry *dpu_core_irq_get_entry(struct dpu_hw_intr *intr,
> @@ -470,13 +470,12 @@ struct dpu_hw_intr *dpu_hw_intr_init(void __iomem *addr,
> const struct dpu_mdss_cfg *m)
> {
> struct dpu_hw_intr *intr;
> - int nirq = MDP_INTR_MAX * 32;
> unsigned int i;
>
> if (!addr || !m)
> return ERR_PTR(-EINVAL);
>
> - intr = kzalloc(struct_size(intr, irq_tbl, nirq), GFP_KERNEL);
> + intr = kzalloc(sizeof(*intr), GFP_KERNEL);
> if (!intr)
> return ERR_PTR(-ENOMEM);
>
> @@ -487,8 +486,6 @@ struct dpu_hw_intr *dpu_hw_intr_init(void __iomem *addr,
>
> intr->hw.blk_addr = addr + m->mdp[0].base;
>
> - intr->total_irqs = nirq;
> -
> intr->irq_mask = BIT(MDP_SSPP_TOP0_INTR) |
> BIT(MDP_SSPP_TOP0_INTR2) |
> BIT(MDP_SSPP_TOP0_HIST_INTR);
> @@ -601,7 +598,7 @@ static int dpu_debugfs_core_irq_show(struct seq_file *s, void *v)
> int i, irq_count;
> void *cb;
>
> - for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) {
> + for (i = 0; i < DPU_NUM_IRQS; i++) {
> spin_lock_irqsave(&dpu_kms->hw_intr->irq_lock, irq_flags);
> irq_entry = dpu_core_irq_get_entry(dpu_kms->hw_intr, i);
> irq_count = atomic_read(&irq_entry->count);
> @@ -636,7 +633,7 @@ void dpu_core_irq_preinstall(struct msm_kms *kms)
> dpu_disable_all_irqs(dpu_kms);
> pm_runtime_put_sync(&dpu_kms->pdev->dev);
>
> - for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) {
> + for (i = 0; i < DPU_NUM_IRQS; i++) {
> irq_entry = dpu_core_irq_get_entry(dpu_kms->hw_intr, i);
> atomic_set(&irq_entry->count, 0);
> }
> @@ -652,7 +649,7 @@ void dpu_core_irq_uninstall(struct msm_kms *kms)
> return;
>
> pm_runtime_get_sync(&dpu_kms->pdev->dev);
> - for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) {
> + for (i = 0; i < DPU_NUM_IRQS; i++) {
> irq_entry = dpu_core_irq_get_entry(dpu_kms->hw_intr, i);
> if (irq_entry->cb)
> DPU_ERROR("irq_idx=%d still enabled/registered\n", i);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
> index 391fb268ad90..bb775b6a2432 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
> @@ -38,6 +38,8 @@ enum dpu_hw_intr_reg {
>
> #define DPU_IRQ_IDX(reg_idx, offset) (reg_idx * 32 + offset)
>
> +#define DPU_NUM_IRQS (MDP_INTR_MAX * 32)
> +
> struct dpu_hw_intr_entry {
> void (*cb)(void *arg);
> void *arg;
> @@ -50,7 +52,6 @@ struct dpu_hw_intr_entry {
> * @ops: function pointer mapping for IRQ handling
> * @cache_irq_mask: array of IRQ enable masks reg storage created during init
> * @save_irq_status: array of IRQ status reg storage created during init
> - * @total_irqs: total number of irq_idx mapped in the hw_interrupts
> * @irq_lock: spinlock for accessing IRQ resources
> * @irq_cb_tbl: array of IRQ callbacks
> */
> @@ -58,12 +59,11 @@ struct dpu_hw_intr {
> struct dpu_hw_blk_reg_map hw;
> u32 cache_irq_mask[MDP_INTR_MAX];
> u32 *save_irq_status;
> - u32 total_irqs;
> spinlock_t irq_lock;
> unsigned long irq_mask;
> const struct dpu_intr_reg *intr_set;
>
> - struct dpu_hw_intr_entry irq_tbl[];
> + struct dpu_hw_intr_entry irq_tbl[DPU_NUM_IRQS];
> };
>
> /**
> --
> 2.39.2
>