On Thu, Oct 18, 2018 at 3:59 PM Jordan Crouse <jcrouse@xxxxxxxxxxxxxx> wrote:
>
> dpu_irq.c does some unneeded checks and passes control
> to dpu_core_irq.c The simple functions can be defined
> in the same file where we use them and the files and
> their associated hangers on can be deleted.
>
> Additionally the postinstall hook isn't used even
> in dpu_core_irq.c so zap that entire path.
>
> Signed-off-by: Jordan Crouse <jcrouse@xxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/msm/Makefile | 1 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c | 15 +----
> drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h | 7 ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_irq.c | 66 --------------------
> drivers/gpu/drm/msm/disp/dpu1/dpu_irq.h | 59 -----------------
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 22 ++++++-
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 5 ++
> 8 files changed, 28 insertions(+), 148 deletions(-)
> delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_irq.c
> delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_irq.h
>
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index e067dd1341b1..4b181c2a88d6 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -68,7 +68,6 @@ msm-y := \
> disp/dpu1/dpu_hw_util.o \
> disp/dpu1/dpu_hw_vbif.o \
> disp/dpu1/dpu_io_util.o \
> - disp/dpu1/dpu_irq.o \
> disp/dpu1/dpu_kms.o \
> disp/dpu1/dpu_mdss.o \
> disp/dpu1/dpu_plane.o \
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
> index 7607aac9449c..f66070cb2f42 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
> @@ -364,10 +364,7 @@ void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms)
> struct msm_drm_private *priv;
> int i;
>
> - if (!dpu_kms) {
> - DPU_ERROR("invalid dpu_kms\n");
> - return;
> - } else if (!dpu_kms->dev) {
> + if (!dpu_kms->dev) {
> DPU_ERROR("invalid drm device\n");
> return;
> } else if (!dpu_kms->dev->dev_private) {
> @@ -398,20 +395,12 @@ void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms)
> }
> }
>
> -int dpu_core_irq_postinstall(struct dpu_kms *dpu_kms)
> -{
> - return 0;
> -}
> -
> void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms)
> {
> struct msm_drm_private *priv;
> int i;
>
> - if (!dpu_kms) {
> - DPU_ERROR("invalid dpu_kms\n");
> - return;
> - } else if (!dpu_kms->dev) {
> + if (!dpu_kms->dev) {
> DPU_ERROR("invalid drm device\n");
> return;
> } else if (!dpu_kms->dev->dev_private) {
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
> index 5e98bba46af5..884f77fa3eb6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
> @@ -23,13 +23,6 @@
> */
> void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms);
>
> -/**
> - * dpu_core_irq_postinstall - perform post-installation of core IRQ handler
> - * @dpu_kms: DPU handle
> - * @return: 0 if success; error code otherwise
> - */
> -int dpu_core_irq_postinstall(struct dpu_kms *dpu_kms);
> -
> /**
> * dpu_core_irq_uninstall - uninstall core IRQ handler
> * @dpu_kms: DPU handle
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_irq.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_irq.c
> deleted file mode 100644
> index d5e6ce0140cf..000000000000
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_irq.c
> +++ /dev/null
> @@ -1,66 +0,0 @@
> -/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 and
> - * only version 2 as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - */
> -
> -#define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__
> -
> -#include <linux/irqdomain.h>
> -#include <linux/irq.h>
> -#include <linux/kthread.h>
> -
> -#include "dpu_irq.h"
> -#include "dpu_core_irq.h"
> -
> -irqreturn_t dpu_irq(struct msm_kms *kms)
> -{
> - struct dpu_kms *dpu_kms = to_dpu_kms(kms);
> -
> - return dpu_core_irq(dpu_kms);
> -}
> -
> -void dpu_irq_preinstall(struct msm_kms *kms)
> -{
> - struct dpu_kms *dpu_kms = to_dpu_kms(kms);
> -
> - if (!dpu_kms->dev || !dpu_kms->dev->dev) {
> - pr_err("invalid device handles\n");
> - return;
> - }
> -
> - dpu_core_irq_preinstall(dpu_kms);
> -}
> -
> -int dpu_irq_postinstall(struct msm_kms *kms)
> -{
> - struct dpu_kms *dpu_kms = to_dpu_kms(kms);
> - int rc;
> -
> - if (!kms) {
> - DPU_ERROR("invalid parameters\n");
> - return -EINVAL;
> - }
> -
> - rc = dpu_core_irq_postinstall(dpu_kms);
> -
> - return rc;
> -}
> -
> -void dpu_irq_uninstall(struct msm_kms *kms)
> -{
> - struct dpu_kms *dpu_kms = to_dpu_kms(kms);
> -
> - if (!kms) {
> - DPU_ERROR("invalid parameters\n");
> - return;
> - }
> -
> - dpu_core_irq_uninstall(dpu_kms);
> -}
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_irq.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_irq.h
> deleted file mode 100644
> index 3e147f7176e2..000000000000
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_irq.h
> +++ /dev/null
> @@ -1,59 +0,0 @@
> -/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 and
> - * only version 2 as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - */
> -
> -#ifndef __DPU_IRQ_H__
> -#define __DPU_IRQ_H__
> -
> -#include <linux/kernel.h>
> -#include <linux/irqdomain.h>
> -
> -#include "msm_kms.h"
> -
> -/**
> - * dpu_irq_controller - define MDSS level interrupt controller context
> - * @enabled_mask: enable status of MDSS level interrupt
> - * @domain: interrupt domain of this controller
> - */
> -struct dpu_irq_controller {
> - unsigned long enabled_mask;
> - struct irq_domain *domain;
> -};
> -
> -/**
> - * dpu_irq_preinstall - perform pre-installation of MDSS IRQ handler
> - * @kms: pointer to kms context
> - * @return: none
> - */
> -void dpu_irq_preinstall(struct msm_kms *kms);
> -
> -/**
> - * dpu_irq_postinstall - perform post-installation of MDSS IRQ handler
> - * @kms: pointer to kms context
> - * @return: 0 if success; error code otherwise
> - */
> -int dpu_irq_postinstall(struct msm_kms *kms);
> -
> -/**
> - * dpu_irq_uninstall - uninstall MDSS IRQ handler
> - * @drm_dev: pointer to kms context
> - * @return: none
> - */
> -void dpu_irq_uninstall(struct msm_kms *kms);
> -
> -/**
> - * dpu_irq - MDSS level IRQ handler
> - * @kms: pointer to kms context
> - * @return: interrupt handling status
> - */
> -irqreturn_t dpu_irq(struct msm_kms *kms);
> -
> -#endif /* __DPU_IRQ_H__ */
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 7e3ba128cbaf..2a91881048c8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -706,10 +706,30 @@ static void _dpu_kms_set_encoder_mode(struct msm_kms *kms,
> encoder->base.id, rc);
> }
>
> +static irqreturn_t dpu_irq(struct msm_kms *kms)
> +{
> + struct dpu_kms *dpu_kms = to_dpu_kms(kms);
> +
> + return dpu_core_irq(dpu_kms);
> +}
> +
> +static void dpu_irq_preinstall(struct msm_kms *kms)
> +{
> + struct dpu_kms *dpu_kms = to_dpu_kms(kms);
> +
> + dpu_core_irq_preinstall(dpu_kms);
> +}
> +
> +static void dpu_irq_uninstall(struct msm_kms *kms)
> +{
> + struct dpu_kms *dpu_kms = to_dpu_kms(kms);
> +
> + dpu_core_irq_uninstall(dpu_kms);
> +}
> +
Seems like we could pass in a struct msm_kms directly to the core irq
functions and call to_dpu_kms()
from inside those functions instead so we don't need these wrappers
anymore, but if you prefer the logical
separation that this provides then:
Reviewed-by: Bruce Wang <bzwang@xxxxxxxxxxxx>
> static const struct msm_kms_funcs kms_funcs = {
> .hw_init = dpu_kms_hw_init,
> .irq_preinstall = dpu_irq_preinstall,
> - .irq_postinstall = dpu_irq_postinstall,
> .irq_uninstall = dpu_irq_uninstall,
> .irq = dpu_irq,
> .prepare_commit = dpu_kms_prepare_commit,
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index 12a2eb90e6b5..e7539c9870e4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -30,7 +30,6 @@
> #include "dpu_hw_top.h"
> #include "dpu_rm.h"
> #include "dpu_power_handle.h"
> -#include "dpu_irq.h"
> #include "dpu_core_perf.h"
>
> #define DRMID(x) ((x) ? (x)->base.id : -1)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> index 2235ef8129f4..19abf719811a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> @@ -9,6 +9,11 @@
>
> #define HW_INTR_STATUS 0x0010
>
> +struct dpu_irq_controller {
> + unsigned long enabled_mask;
> + struct irq_domain *domain;
> +};
> +
> struct dpu_mdss {
> struct msm_mdss base;
> void __iomem *mmio;
> --
> 2.18.0
>
