[Freedreno] [PATCH 2/2] drm/msm: push IRQ setup into individual drivers
Dmitry Baryshkov
dmitry.baryshkov at linaro.org
Fri May 13 11:40:29 UTC 2022
On 13/05/2022 01:54, Abhinav Kumar wrote:
>
>
> On 5/6/2022 6:00 PM, Dmitry Baryshkov wrote:
>> Afther the commit f026e431cf86 ("drm/msm: Convert to Linux IRQ
>> interfaces") converted MSM DRM driver to handle IRQs on it's own (rather
>> than using the DRM IRQ mid-layer), there is little point in keeping
>> irq wrapper in the msm_drv.c which just call into individual drivers.
>> Push respective code into the mdp4/mdp5/dpu drivers and drop
>> irq_preinstall/irq_postinstall/irq msm_kms funcs.
>
> Isnt this change causing a lot of code duplication across mdp5/dpu/mdp4?
>
> Do you have any future plans with respect to this separation?
>
> I am missing why this separation into respective mdp4/5/dpu is necessary
> because struct msm_kms remains a common struct in all of this so it
> remaining in msm_drv will avoid duplication.
I wanted to remove back-and-forth calls between msm core and individual
drivers. But I agree that it comes by the cost of code duplication.
Let's drop this patch.
>
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h | 13 +---
>> .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 28 ++++++++-
>> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 6 +-
>> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 7 +++
>> drivers/gpu/drm/msm/disp/mdp4/mdp4_irq.c | 30 ++++++++-
>> drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 4 +-
>> drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h | 4 +-
>> drivers/gpu/drm/msm/disp/mdp5/mdp5_irq.c | 30 ++++++++-
>> drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 4 +-
>> drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h | 4 +-
>> drivers/gpu/drm/msm/msm_drv.c | 62 +++----------------
>> drivers/gpu/drm/msm/msm_kms.h | 4 +-
>> 12 files changed, 105 insertions(+), 91 deletions(-)
>>
>> 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 b5b6e7031fb9..c6938b1f1870 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
>> @@ -8,13 +8,6 @@
>> #include "dpu_kms.h"
>> #include "dpu_hw_interrupts.h"
>> -/**
>> - * dpu_core_irq_preinstall - perform pre-installation of core IRQ
>> handler
>> - * @kms: MSM KMS handle
>> - * @return: none
>> - */
>> -void dpu_core_irq_preinstall(struct msm_kms *kms);
>> -
>> /**
>> * dpu_core_irq_uninstall - uninstall core IRQ handler
>> * @kms: MSM KMS handle
>> @@ -23,11 +16,11 @@ void dpu_core_irq_preinstall(struct msm_kms *kms);
>> void dpu_core_irq_uninstall(struct msm_kms *kms);
>> /**
>> - * dpu_core_irq - core IRQ handler
>> + * dpu_core_irq_install - install core IRQ handler
>> * @kms: MSM KMS handle
>> - * @return: interrupt handling status
>> + * @return: non-zero in case of an error
>> */
>> -irqreturn_t dpu_core_irq(struct msm_kms *kms);
>> +int dpu_core_irq_install(struct msm_kms *kms);
>> /**
>> * dpu_core_irq_read - IRQ helper function for reading IRQ status
>> 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 d6498e45dc2c..fa4f99034a08 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
>> @@ -164,8 +164,9 @@ static void dpu_core_irq_callback_handler(struct
>> dpu_kms *dpu_kms, int irq_idx)
>>
>> dpu_kms->hw_intr->irq_tbl[irq_idx].cb(dpu_kms->hw_intr->irq_tbl[irq_idx].arg,
>> irq_idx);
>> }
>> -irqreturn_t dpu_core_irq(struct msm_kms *kms)
>> +static irqreturn_t dpu_irq(int irq, void *arg)
>> {
>> + struct msm_kms *kms = arg;
>> struct dpu_kms *dpu_kms = to_dpu_kms(kms);
>> struct dpu_hw_intr *intr = dpu_kms->hw_intr;
>> int reg_idx;
>> @@ -541,7 +542,7 @@ void dpu_debugfs_core_irq_init(struct dpu_kms
>> *dpu_kms,
>> }
>> #endif
>> -void dpu_core_irq_preinstall(struct msm_kms *kms)
>> +static void dpu_core_irq_preinstall(struct msm_kms *kms)
>> {
>> struct dpu_kms *dpu_kms = to_dpu_kms(kms);
>> int i;
>> @@ -570,5 +571,28 @@ void dpu_core_irq_uninstall(struct msm_kms *kms)
>> dpu_clear_irqs(dpu_kms);
>> dpu_disable_all_irqs(dpu_kms);
>> + if (kms->irq_requested)
>> + free_irq(kms->irq, kms);
>> pm_runtime_put_sync(&dpu_kms->pdev->dev);
>> }
>> +
>> +int dpu_core_irq_install(struct msm_kms *kms)
>> +{
>> + int ret;
>> +
>> + dpu_core_irq_preinstall(kms);
>> +
>> + ret = request_irq(kms->irq, dpu_irq, 0, "dpu", kms);
>> + if (ret)
>> + return ret;
>> +
>> + kms->irq_requested = true;
>> +
>> + ret = dpu_irq_postinstall(kms);
>> + if (ret) {
>> + free_irq(kms->irq, kms);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> index 2b9d931474e0..494978da7785 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> @@ -884,7 +884,7 @@ static void dpu_kms_destroy(struct msm_kms *kms)
>> pm_runtime_disable(&dpu_kms->pdev->dev);
>> }
>> -static int dpu_irq_postinstall(struct msm_kms *kms)
>> +int dpu_irq_postinstall(struct msm_kms *kms)
>> {
>> struct msm_drm_private *priv;
>> struct dpu_kms *dpu_kms = to_dpu_kms(kms);
>> @@ -960,10 +960,8 @@ static void dpu_kms_mdp_snapshot(struct
>> msm_disp_state *disp_state, struct msm_k
>> static const struct msm_kms_funcs kms_funcs = {
>> .hw_init = dpu_kms_hw_init,
>> - .irq_preinstall = dpu_core_irq_preinstall,
>> - .irq_postinstall = dpu_irq_postinstall,
>> + .irq_install = dpu_core_irq_install,
>> .irq_uninstall = dpu_core_irq_uninstall,
>> - .irq = dpu_core_irq,
>> .enable_commit = dpu_kms_enable_commit,
>> .disable_commit = dpu_kms_disable_commit,
>> .vsync_time = dpu_kms_vsync_time,
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
>> index 832a0769f2e7..559184c64045 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
>> @@ -204,4 +204,11 @@ void dpu_disable_vblank(struct msm_kms *kms,
>> struct drm_crtc *crtc);
>> */
>> u64 dpu_kms_get_clk_rate(struct dpu_kms *dpu_kms, char *clock_name);
>> +/**
>> + * dpu_irq_postinstall - perform post-installation of core IRQ handler
>> + * @kms: MSM KMS handle
>> + * @return: non-zero in case of error
>> + */
>> +int dpu_irq_postinstall(struct msm_kms *kms);
>> +
>> #endif /* __dpu_kms_H__ */
>> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_irq.c
>> b/drivers/gpu/drm/msm/disp/mdp4/mdp4_irq.c
>> index 4d49f3ba6a96..87675c162eea 100644
>> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_irq.c
>> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_irq.c
>> @@ -32,7 +32,7 @@ static void mdp4_irq_error_handler(struct mdp_irq
>> *irq, uint32_t irqstatus)
>> }
>> }
>> -void mdp4_irq_preinstall(struct msm_kms *kms)
>> +static void mdp4_irq_preinstall(struct msm_kms *kms)
>> {
>> struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(kms));
>> mdp4_enable(mdp4_kms);
>> @@ -41,7 +41,7 @@ void mdp4_irq_preinstall(struct msm_kms *kms)
>> mdp4_disable(mdp4_kms);
>> }
>> -int mdp4_irq_postinstall(struct msm_kms *kms)
>> +static int mdp4_irq_postinstall(struct msm_kms *kms)
>> {
>> struct mdp_kms *mdp_kms = to_mdp_kms(kms);
>> struct mdp4_kms *mdp4_kms = to_mdp4_kms(mdp_kms);
>> @@ -62,10 +62,13 @@ void mdp4_irq_uninstall(struct msm_kms *kms)
>> mdp4_enable(mdp4_kms);
>> mdp4_write(mdp4_kms, REG_MDP4_INTR_ENABLE, 0x00000000);
>> mdp4_disable(mdp4_kms);
>> + if (kms->irq_requested)
>> + free_irq(kms->irq, kms);
>> }
>> -irqreturn_t mdp4_irq(struct msm_kms *kms)
>> +static irqreturn_t mdp4_irq(int irq, void *arg)
>> {
>> + struct msm_kms *kms = arg;
>> struct mdp_kms *mdp_kms = to_mdp_kms(kms);
>> struct mdp4_kms *mdp4_kms = to_mdp4_kms(mdp_kms);
>> struct drm_device *dev = mdp4_kms->dev;
>> @@ -88,6 +91,27 @@ irqreturn_t mdp4_irq(struct msm_kms *kms)
>> return IRQ_HANDLED;
>> }
>> +int mdp4_irq_install(struct msm_kms *kms)
>> +{
>> + int ret;
>> +
>> + mdp4_irq_preinstall(kms);
>> +
>> + ret = request_irq(kms->irq, mdp4_irq, 0, "mdp4", kms);
>> + if (ret)
>> + return ret;
>> +
>> + kms->irq_requested = true;
>> +
>> + ret = mdp4_irq_postinstall(kms);
>> + if (ret) {
>> + free_irq(kms->irq, kms);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> int mdp4_enable_vblank(struct msm_kms *kms, struct drm_crtc *crtc)
>> {
>> struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(kms));
>> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
>> b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
>> index fb48c8c19ec3..b7aced272af9 100644
>> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
>> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
>> @@ -148,10 +148,8 @@ static void mdp4_destroy(struct msm_kms *kms)
>> static const struct mdp_kms_funcs kms_funcs = {
>> .base = {
>> .hw_init = mdp4_hw_init,
>> - .irq_preinstall = mdp4_irq_preinstall,
>> - .irq_postinstall = mdp4_irq_postinstall,
>> + .irq_install = mdp4_irq_install,
>> .irq_uninstall = mdp4_irq_uninstall,
>> - .irq = mdp4_irq,
>> .enable_vblank = mdp4_enable_vblank,
>> .disable_vblank = mdp4_disable_vblank,
>> .enable_commit = mdp4_enable_commit,
>> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
>> b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
>> index e8ee92ab7956..b24a63872232 100644
>> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
>> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
>> @@ -157,10 +157,8 @@ int mdp4_enable(struct mdp4_kms *mdp4_kms);
>> void mdp4_set_irqmask(struct mdp_kms *mdp_kms, uint32_t irqmask,
>> uint32_t old_irqmask);
>> -void mdp4_irq_preinstall(struct msm_kms *kms);
>> -int mdp4_irq_postinstall(struct msm_kms *kms);
>> +int mdp4_irq_install(struct msm_kms *kms);
>> void mdp4_irq_uninstall(struct msm_kms *kms);
>> -irqreturn_t mdp4_irq(struct msm_kms *kms);
>> int mdp4_enable_vblank(struct msm_kms *kms, struct drm_crtc *crtc);
>> void mdp4_disable_vblank(struct msm_kms *kms, struct drm_crtc *crtc);
>> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_irq.c
>> b/drivers/gpu/drm/msm/disp/mdp5/mdp5_irq.c
>> index 9b4c8d92ff32..d573ff29d5a4 100644
>> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_irq.c
>> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_irq.c
>> @@ -36,7 +36,7 @@ static void mdp5_irq_error_handler(struct mdp_irq
>> *irq, uint32_t irqstatus)
>> }
>> }
>> -void mdp5_irq_preinstall(struct msm_kms *kms)
>> +static void mdp5_irq_preinstall(struct msm_kms *kms)
>> {
>> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
>> struct device *dev = &mdp5_kms->pdev->dev;
>> @@ -47,7 +47,7 @@ void mdp5_irq_preinstall(struct msm_kms *kms)
>> pm_runtime_put_sync(dev);
>> }
>> -int mdp5_irq_postinstall(struct msm_kms *kms)
>> +static int mdp5_irq_postinstall(struct msm_kms *kms)
>> {
>> struct mdp_kms *mdp_kms = to_mdp_kms(kms);
>> struct mdp5_kms *mdp5_kms = to_mdp5_kms(mdp_kms);
>> @@ -74,11 +74,14 @@ void mdp5_irq_uninstall(struct msm_kms *kms)
>> pm_runtime_get_sync(dev);
>> mdp5_write(mdp5_kms, REG_MDP5_INTR_EN, 0x00000000);
>> + if (kms->irq_requested)
>> + free_irq(kms->irq, kms);
>> pm_runtime_put_sync(dev);
>> }
>> -irqreturn_t mdp5_irq(struct msm_kms *kms)
>> +static irqreturn_t mdp5_irq(int irq, void *arg)
>> {
>> + struct msm_kms *kms = arg;
>> struct mdp_kms *mdp_kms = to_mdp_kms(kms);
>> struct mdp5_kms *mdp5_kms = to_mdp5_kms(mdp_kms);
>> struct drm_device *dev = mdp5_kms->dev;
>> @@ -101,6 +104,27 @@ irqreturn_t mdp5_irq(struct msm_kms *kms)
>> return IRQ_HANDLED;
>> }
>> +int mdp5_irq_install(struct msm_kms *kms)
>> +{
>> + int ret;
>> +
>> + mdp5_irq_preinstall(kms);
>> +
>> + ret = request_irq(kms->irq, mdp5_irq, 0, "mdp5", kms);
>> + if (ret)
>> + return ret;
>> +
>> + kms->irq_requested = true;
>> +
>> + ret = mdp5_irq_postinstall(kms);
>> + if (ret) {
>> + free_irq(kms->irq, kms);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> int mdp5_enable_vblank(struct msm_kms *kms, struct drm_crtc *crtc)
>> {
>> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
>> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
>> b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
>> index 3d5621a68f85..18cf0ff4da6c 100644
>> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
>> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
>> @@ -262,10 +262,8 @@ static int mdp5_kms_debugfs_init(struct msm_kms
>> *kms, struct drm_minor *minor)
>> static const struct mdp_kms_funcs kms_funcs = {
>> .base = {
>> .hw_init = mdp5_hw_init,
>> - .irq_preinstall = mdp5_irq_preinstall,
>> - .irq_postinstall = mdp5_irq_postinstall,
>> + .irq_install = mdp5_irq_install,
>> .irq_uninstall = mdp5_irq_uninstall,
>> - .irq = mdp5_irq,
>> .enable_vblank = mdp5_enable_vblank,
>> .disable_vblank = mdp5_disable_vblank,
>> .flush_commit = mdp5_flush_commit,
>> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
>> b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
>> index 29bf11f08601..630b5f812f24 100644
>> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
>> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
>> @@ -263,10 +263,8 @@ static inline uint32_t lm2ppdone(struct
>> mdp5_hw_mixer *mixer)
>> void mdp5_set_irqmask(struct mdp_kms *mdp_kms, uint32_t irqmask,
>> uint32_t old_irqmask);
>> -void mdp5_irq_preinstall(struct msm_kms *kms);
>> -int mdp5_irq_postinstall(struct msm_kms *kms);
>> +int mdp5_irq_install(struct msm_kms *kms);
>> void mdp5_irq_uninstall(struct msm_kms *kms);
>> -irqreturn_t mdp5_irq(struct msm_kms *kms);
>> int mdp5_enable_vblank(struct msm_kms *kms, struct drm_crtc *crtc);
>> void mdp5_disable_vblank(struct msm_kms *kms, struct drm_crtc *crtc);
>> int mdp5_irq_domain_init(struct mdp5_kms *mdp5_kms);
>> diff --git a/drivers/gpu/drm/msm/msm_drv.c
>> b/drivers/gpu/drm/msm/msm_drv.c
>> index 44485363f37a..d2fbe54fec4d 100644
>> --- a/drivers/gpu/drm/msm/msm_drv.c
>> +++ b/drivers/gpu/drm/msm/msm_drv.c
>> @@ -77,64 +77,15 @@ static bool modeset = true;
>> MODULE_PARM_DESC(modeset, "Use kernel modesetting [KMS] (1=on
>> (default), 0=disable)");
>> module_param(modeset, bool, 0600);
>> -static irqreturn_t msm_irq(int irq, void *arg)
>> -{
>> - struct drm_device *dev = arg;
>> - struct msm_drm_private *priv = dev->dev_private;
>> - struct msm_kms *kms = priv->kms;
>> -
>> - BUG_ON(!kms);
>> -
>> - return kms->funcs->irq(kms);
>> -}
>> -
>> -static void msm_irq_preinstall(struct drm_device *dev)
>> -{
>> - struct msm_drm_private *priv = dev->dev_private;
>> - struct msm_kms *kms = priv->kms;
>> -
>> - BUG_ON(!kms);
>> -
>> - kms->funcs->irq_preinstall(kms);
>> -}
>> -
>> -static int msm_irq_postinstall(struct drm_device *dev)
>> -{
>> - struct msm_drm_private *priv = dev->dev_private;
>> - struct msm_kms *kms = priv->kms;
>> -
>> - BUG_ON(!kms);
>> -
>> - if (kms->funcs->irq_postinstall)
>> - return kms->funcs->irq_postinstall(kms);
>> -
>> - return 0;
>> -}
>> -
>> static int msm_irq_install(struct drm_device *dev, unsigned int irq)
>> {
>> struct msm_drm_private *priv = dev->dev_private;
>> struct msm_kms *kms = priv->kms;
>> - int ret;
>> -
>> - if (irq == IRQ_NOTCONNECTED)
>> - return -ENOTCONN;
>> -
>> - msm_irq_preinstall(dev);
>> -
>> - ret = request_irq(irq, msm_irq, 0, dev->driver->name, dev);
>> - if (ret)
>> - return ret;
>> -
>> - kms->irq_requested = true;
>> - ret = msm_irq_postinstall(dev);
>> - if (ret) {
>> - free_irq(irq, dev);
>> - return ret;
>> - }
>> + if (!kms->funcs->irq_install)
>> + return -EINVAL;
>> - return 0;
>> + return kms->funcs->irq_install(kms);
>> }
>> static void msm_irq_uninstall(struct drm_device *dev)
>> @@ -143,8 +94,6 @@ static void msm_irq_uninstall(struct drm_device *dev)
>> struct msm_kms *kms = priv->kms;
>> kms->funcs->irq_uninstall(kms);
>> - if (kms->irq_requested)
>> - free_irq(kms->irq, dev);
>> }
>> struct msm_vblank_work {
>> @@ -450,6 +399,11 @@ static int msm_drm_init(struct device *dev, const
>> struct drm_driver *drv)
>> }
>> if (kms) {
>> + if (kms->irq == IRQ_NOTCONNECTED) {
>> + ret = -ENOTCONN;
>> + goto err_msm_uninit;
>> + }
>> +
>> pm_runtime_get_sync(dev);
>> ret = msm_irq_install(ddev, kms->irq);
>> pm_runtime_put_sync(dev);
>> diff --git a/drivers/gpu/drm/msm/msm_kms.h
>> b/drivers/gpu/drm/msm/msm_kms.h
>> index f8ed7588928c..71d497a8fb8b 100644
>> --- a/drivers/gpu/drm/msm/msm_kms.h
>> +++ b/drivers/gpu/drm/msm/msm_kms.h
>> @@ -24,10 +24,8 @@ struct msm_kms_funcs {
>> /* hw initialization: */
>> int (*hw_init)(struct msm_kms *kms);
>> /* irq handling: */
>> - void (*irq_preinstall)(struct msm_kms *kms);
>> - int (*irq_postinstall)(struct msm_kms *kms);
>> + int (*irq_install)(struct msm_kms *kms);
>> void (*irq_uninstall)(struct msm_kms *kms);
>> - irqreturn_t (*irq)(struct msm_kms *kms);
>> int (*enable_vblank)(struct msm_kms *kms, struct drm_crtc *crtc);
>> void (*disable_vblank)(struct msm_kms *kms, struct drm_crtc *crtc);
--
With best wishes
Dmitry
More information about the Freedreno
mailing list