| Message ID | 20210803090704.32152-8-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Make DRM's IRQ helpers legacy | expand |
On 03/08/2021 12:06, Thomas Zimmermann wrote: > Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's > IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers > don't benefit from using it. > > DRM IRQ callbacks are now being called directly or inlined. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Rob should probably also give his blessing on this patch though. > --- > drivers/gpu/drm/msm/msm_drv.c | 113 ++++++++++++++++++++-------------- > drivers/gpu/drm/msm/msm_kms.h | 2 +- > 2 files changed, 69 insertions(+), 46 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > index 1594ae39d54f..a332b09a5a11 100644 > --- a/drivers/gpu/drm/msm/msm_drv.c > +++ b/drivers/gpu/drm/msm/msm_drv.c > @@ -14,7 +14,6 @@ > #include <drm/drm_drv.h> > #include <drm/drm_file.h> > #include <drm/drm_ioctl.h> > -#include <drm/drm_irq.h> > #include <drm/drm_prime.h> > #include <drm/drm_of.h> > #include <drm/drm_vblank.h> > @@ -201,6 +200,71 @@ void msm_rmw(void __iomem *addr, u32 mask, u32 or) > msm_writel(val | or, addr); > } > > +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) > +{ > + 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; > + > + ret = msm_irq_postinstall(dev); > + if (ret) { > + free_irq(irq, dev); > + return ret; > + } > + > + return 0; > +} > + > +static void msm_irq_uninstall(struct drm_device *dev) > +{ > + struct msm_drm_private *priv = dev->dev_private; > + struct msm_kms *kms = priv->kms; > + > + kms->funcs->irq_uninstall(kms); > + free_irq(kms->irq, dev); > +} > + > struct msm_vblank_work { > struct work_struct work; > int crtc_id; > @@ -265,7 +329,7 @@ static int msm_drm_uninit(struct device *dev) > } > > /* We must cancel and cleanup any pending vblank enable/disable > - * work before drm_irq_uninstall() to avoid work re-enabling an > + * work before msm_irq_uninstall() to avoid work re-enabling an > * irq after uninstall has disabled it. > */ > > @@ -294,7 +358,7 @@ static int msm_drm_uninit(struct device *dev) > drm_mode_config_cleanup(ddev); > > pm_runtime_get_sync(dev); > - drm_irq_uninstall(ddev); > + msm_irq_uninstall(ddev); > pm_runtime_put_sync(dev); > > if (kms && kms->funcs) > @@ -553,7 +617,7 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv) > > if (kms) { > pm_runtime_get_sync(dev); > - ret = drm_irq_install(ddev, kms->irq); > + ret = msm_irq_install(ddev, kms->irq); > pm_runtime_put_sync(dev); > if (ret < 0) { > DRM_DEV_ERROR(dev, "failed to install IRQ handler\n"); > @@ -662,43 +726,6 @@ static void msm_postclose(struct drm_device *dev, struct drm_file *file) > context_close(ctx); > } > > -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 void msm_irq_uninstall(struct drm_device *dev) > -{ > - struct msm_drm_private *priv = dev->dev_private; > - struct msm_kms *kms = priv->kms; > - BUG_ON(!kms); > - kms->funcs->irq_uninstall(kms); > -} > - > int msm_crtc_enable_vblank(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > @@ -1051,10 +1078,6 @@ static const struct drm_driver msm_driver = { > .open = msm_open, > .postclose = msm_postclose, > .lastclose = drm_fb_helper_lastclose, > - .irq_handler = msm_irq, > - .irq_preinstall = msm_irq_preinstall, > - .irq_postinstall = msm_irq_postinstall, > - .irq_uninstall = msm_irq_uninstall, > .dumb_create = msm_gem_dumb_create, > .dumb_map_offset = msm_gem_dumb_map_offset, > .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h > index 086a2d59b8c8..9de7c42e1071 100644 > --- a/drivers/gpu/drm/msm/msm_kms.h > +++ b/drivers/gpu/drm/msm/msm_kms.h > @@ -150,7 +150,7 @@ struct msm_kms { > const struct msm_kms_funcs *funcs; > struct drm_device *dev; > > - /* irq number to be passed on to drm_irq_install */ > + /* irq number to be passed on to msm_irq_install */ > int irq; > > /* mapper-id used to request GEM buffer mapped for scanout: */ >
On Tue, Aug 3, 2021 at 2:37 AM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On 03/08/2021 12:06, Thomas Zimmermann wrote: > > Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's > > IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers > > don't benefit from using it. > > > > DRM IRQ callbacks are now being called directly or inlined. > > > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > Rob should probably also give his blessing on this patch though. It looks ok.. I can't really test it this week, but it should be pretty obvious if it wasn't working BR, -R > > > --- > > drivers/gpu/drm/msm/msm_drv.c | 113 ++++++++++++++++++++-------------- > > drivers/gpu/drm/msm/msm_kms.h | 2 +- > > 2 files changed, 69 insertions(+), 46 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > > index 1594ae39d54f..a332b09a5a11 100644 > > --- a/drivers/gpu/drm/msm/msm_drv.c > > +++ b/drivers/gpu/drm/msm/msm_drv.c > > @@ -14,7 +14,6 @@ > > #include <drm/drm_drv.h> > > #include <drm/drm_file.h> > > #include <drm/drm_ioctl.h> > > -#include <drm/drm_irq.h> > > #include <drm/drm_prime.h> > > #include <drm/drm_of.h> > > #include <drm/drm_vblank.h> > > @@ -201,6 +200,71 @@ void msm_rmw(void __iomem *addr, u32 mask, u32 or) > > msm_writel(val | or, addr); > > } > > > > +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) > > +{ > > + 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; > > + > > + ret = msm_irq_postinstall(dev); > > + if (ret) { > > + free_irq(irq, dev); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static void msm_irq_uninstall(struct drm_device *dev) > > +{ > > + struct msm_drm_private *priv = dev->dev_private; > > + struct msm_kms *kms = priv->kms; > > + > > + kms->funcs->irq_uninstall(kms); > > + free_irq(kms->irq, dev); > > +} > > + > > struct msm_vblank_work { > > struct work_struct work; > > int crtc_id; > > @@ -265,7 +329,7 @@ static int msm_drm_uninit(struct device *dev) > > } > > > > /* We must cancel and cleanup any pending vblank enable/disable > > - * work before drm_irq_uninstall() to avoid work re-enabling an > > + * work before msm_irq_uninstall() to avoid work re-enabling an > > * irq after uninstall has disabled it. > > */ > > > > @@ -294,7 +358,7 @@ static int msm_drm_uninit(struct device *dev) > > drm_mode_config_cleanup(ddev); > > > > pm_runtime_get_sync(dev); > > - drm_irq_uninstall(ddev); > > + msm_irq_uninstall(ddev); > > pm_runtime_put_sync(dev); > > > > if (kms && kms->funcs) > > @@ -553,7 +617,7 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv) > > > > if (kms) { > > pm_runtime_get_sync(dev); > > - ret = drm_irq_install(ddev, kms->irq); > > + ret = msm_irq_install(ddev, kms->irq); > > pm_runtime_put_sync(dev); > > if (ret < 0) { > > DRM_DEV_ERROR(dev, "failed to install IRQ handler\n"); > > @@ -662,43 +726,6 @@ static void msm_postclose(struct drm_device *dev, struct drm_file *file) > > context_close(ctx); > > } > > > > -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 void msm_irq_uninstall(struct drm_device *dev) > > -{ > > - struct msm_drm_private *priv = dev->dev_private; > > - struct msm_kms *kms = priv->kms; > > - BUG_ON(!kms); > > - kms->funcs->irq_uninstall(kms); > > -} > > - > > int msm_crtc_enable_vblank(struct drm_crtc *crtc) > > { > > struct drm_device *dev = crtc->dev; > > @@ -1051,10 +1078,6 @@ static const struct drm_driver msm_driver = { > > .open = msm_open, > > .postclose = msm_postclose, > > .lastclose = drm_fb_helper_lastclose, > > - .irq_handler = msm_irq, > > - .irq_preinstall = msm_irq_preinstall, > > - .irq_postinstall = msm_irq_postinstall, > > - .irq_uninstall = msm_irq_uninstall, > > .dumb_create = msm_gem_dumb_create, > > .dumb_map_offset = msm_gem_dumb_map_offset, > > .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > > diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h > > index 086a2d59b8c8..9de7c42e1071 100644 > > --- a/drivers/gpu/drm/msm/msm_kms.h > > +++ b/drivers/gpu/drm/msm/msm_kms.h > > @@ -150,7 +150,7 @@ struct msm_kms { > > const struct msm_kms_funcs *funcs; > > struct drm_device *dev; > > > > - /* irq number to be passed on to drm_irq_install */ > > + /* irq number to be passed on to msm_irq_install */ > > int irq; > > > > /* mapper-id used to request GEM buffer mapped for scanout: */ > > > > > -- > With best wishes > Dmitry
On Tue, Aug 3, 2021 at 2:37 AM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On 03/08/2021 12:06, Thomas Zimmermann wrote: > > Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's > > IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers > > don't benefit from using it. > > > > DRM IRQ callbacks are now being called directly or inlined. > > > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > Rob should probably also give his blessing on this patch though. > I've pushed this to msm-next-staging, but if Thomas would prefer to merge the series together then I can drop it (in which case a-b for this patch) BR, -R > > --- > > drivers/gpu/drm/msm/msm_drv.c | 113 ++++++++++++++++++++-------------- > > drivers/gpu/drm/msm/msm_kms.h | 2 +- > > 2 files changed, 69 insertions(+), 46 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > > index 1594ae39d54f..a332b09a5a11 100644 > > --- a/drivers/gpu/drm/msm/msm_drv.c > > +++ b/drivers/gpu/drm/msm/msm_drv.c > > @@ -14,7 +14,6 @@ > > #include <drm/drm_drv.h> > > #include <drm/drm_file.h> > > #include <drm/drm_ioctl.h> > > -#include <drm/drm_irq.h> > > #include <drm/drm_prime.h> > > #include <drm/drm_of.h> > > #include <drm/drm_vblank.h> > > @@ -201,6 +200,71 @@ void msm_rmw(void __iomem *addr, u32 mask, u32 or) > > msm_writel(val | or, addr); > > } > > > > +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) > > +{ > > + 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; > > + > > + ret = msm_irq_postinstall(dev); > > + if (ret) { > > + free_irq(irq, dev); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static void msm_irq_uninstall(struct drm_device *dev) > > +{ > > + struct msm_drm_private *priv = dev->dev_private; > > + struct msm_kms *kms = priv->kms; > > + > > + kms->funcs->irq_uninstall(kms); > > + free_irq(kms->irq, dev); > > +} > > + > > struct msm_vblank_work { > > struct work_struct work; > > int crtc_id; > > @@ -265,7 +329,7 @@ static int msm_drm_uninit(struct device *dev) > > } > > > > /* We must cancel and cleanup any pending vblank enable/disable > > - * work before drm_irq_uninstall() to avoid work re-enabling an > > + * work before msm_irq_uninstall() to avoid work re-enabling an > > * irq after uninstall has disabled it. > > */ > > > > @@ -294,7 +358,7 @@ static int msm_drm_uninit(struct device *dev) > > drm_mode_config_cleanup(ddev); > > > > pm_runtime_get_sync(dev); > > - drm_irq_uninstall(ddev); > > + msm_irq_uninstall(ddev); > > pm_runtime_put_sync(dev); > > > > if (kms && kms->funcs) > > @@ -553,7 +617,7 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv) > > > > if (kms) { > > pm_runtime_get_sync(dev); > > - ret = drm_irq_install(ddev, kms->irq); > > + ret = msm_irq_install(ddev, kms->irq); > > pm_runtime_put_sync(dev); > > if (ret < 0) { > > DRM_DEV_ERROR(dev, "failed to install IRQ handler\n"); > > @@ -662,43 +726,6 @@ static void msm_postclose(struct drm_device *dev, struct drm_file *file) > > context_close(ctx); > > } > > > > -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 void msm_irq_uninstall(struct drm_device *dev) > > -{ > > - struct msm_drm_private *priv = dev->dev_private; > > - struct msm_kms *kms = priv->kms; > > - BUG_ON(!kms); > > - kms->funcs->irq_uninstall(kms); > > -} > > - > > int msm_crtc_enable_vblank(struct drm_crtc *crtc) > > { > > struct drm_device *dev = crtc->dev; > > @@ -1051,10 +1078,6 @@ static const struct drm_driver msm_driver = { > > .open = msm_open, > > .postclose = msm_postclose, > > .lastclose = drm_fb_helper_lastclose, > > - .irq_handler = msm_irq, > > - .irq_preinstall = msm_irq_preinstall, > > - .irq_postinstall = msm_irq_postinstall, > > - .irq_uninstall = msm_irq_uninstall, > > .dumb_create = msm_gem_dumb_create, > > .dumb_map_offset = msm_gem_dumb_map_offset, > > .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > > diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h > > index 086a2d59b8c8..9de7c42e1071 100644 > > --- a/drivers/gpu/drm/msm/msm_kms.h > > +++ b/drivers/gpu/drm/msm/msm_kms.h > > @@ -150,7 +150,7 @@ struct msm_kms { > > const struct msm_kms_funcs *funcs; > > struct drm_device *dev; > > > > - /* irq number to be passed on to drm_irq_install */ > > + /* irq number to be passed on to msm_irq_install */ > > int irq; > > > > /* mapper-id used to request GEM buffer mapped for scanout: */ > > > > > -- > With best wishes > Dmitry
Hi Am 07.08.21 um 19:08 schrieb Rob Clark: > On Tue, Aug 3, 2021 at 2:37 AM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: >> >> On 03/08/2021 12:06, Thomas Zimmermann wrote: >>> Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's >>> IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers >>> don't benefit from using it. >>> >>> DRM IRQ callbacks are now being called directly or inlined. >>> >>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> >> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> >> Rob should probably also give his blessing on this patch though. >> > > I've pushed this to msm-next-staging, but if Thomas would prefer to > merge the series together then I can drop it (in which case a-b for > this patch) Yes, please. I'd prefer to merge the whole patchset at once through drm-misc-next. Best regards Thomas > > BR, > -R > >>> --- >>> drivers/gpu/drm/msm/msm_drv.c | 113 ++++++++++++++++++++-------------- >>> drivers/gpu/drm/msm/msm_kms.h | 2 +- >>> 2 files changed, 69 insertions(+), 46 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c >>> index 1594ae39d54f..a332b09a5a11 100644 >>> --- a/drivers/gpu/drm/msm/msm_drv.c >>> +++ b/drivers/gpu/drm/msm/msm_drv.c >>> @@ -14,7 +14,6 @@ >>> #include <drm/drm_drv.h> >>> #include <drm/drm_file.h> >>> #include <drm/drm_ioctl.h> >>> -#include <drm/drm_irq.h> >>> #include <drm/drm_prime.h> >>> #include <drm/drm_of.h> >>> #include <drm/drm_vblank.h> >>> @@ -201,6 +200,71 @@ void msm_rmw(void __iomem *addr, u32 mask, u32 or) >>> msm_writel(val | or, addr); >>> } >>> >>> +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) >>> +{ >>> + 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; >>> + >>> + ret = msm_irq_postinstall(dev); >>> + if (ret) { >>> + free_irq(irq, dev); >>> + return ret; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static void msm_irq_uninstall(struct drm_device *dev) >>> +{ >>> + struct msm_drm_private *priv = dev->dev_private; >>> + struct msm_kms *kms = priv->kms; >>> + >>> + kms->funcs->irq_uninstall(kms); >>> + free_irq(kms->irq, dev); >>> +} >>> + >>> struct msm_vblank_work { >>> struct work_struct work; >>> int crtc_id; >>> @@ -265,7 +329,7 @@ static int msm_drm_uninit(struct device *dev) >>> } >>> >>> /* We must cancel and cleanup any pending vblank enable/disable >>> - * work before drm_irq_uninstall() to avoid work re-enabling an >>> + * work before msm_irq_uninstall() to avoid work re-enabling an >>> * irq after uninstall has disabled it. >>> */ >>> >>> @@ -294,7 +358,7 @@ static int msm_drm_uninit(struct device *dev) >>> drm_mode_config_cleanup(ddev); >>> >>> pm_runtime_get_sync(dev); >>> - drm_irq_uninstall(ddev); >>> + msm_irq_uninstall(ddev); >>> pm_runtime_put_sync(dev); >>> >>> if (kms && kms->funcs) >>> @@ -553,7 +617,7 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv) >>> >>> if (kms) { >>> pm_runtime_get_sync(dev); >>> - ret = drm_irq_install(ddev, kms->irq); >>> + ret = msm_irq_install(ddev, kms->irq); >>> pm_runtime_put_sync(dev); >>> if (ret < 0) { >>> DRM_DEV_ERROR(dev, "failed to install IRQ handler\n"); >>> @@ -662,43 +726,6 @@ static void msm_postclose(struct drm_device *dev, struct drm_file *file) >>> context_close(ctx); >>> } >>> >>> -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 void msm_irq_uninstall(struct drm_device *dev) >>> -{ >>> - struct msm_drm_private *priv = dev->dev_private; >>> - struct msm_kms *kms = priv->kms; >>> - BUG_ON(!kms); >>> - kms->funcs->irq_uninstall(kms); >>> -} >>> - >>> int msm_crtc_enable_vblank(struct drm_crtc *crtc) >>> { >>> struct drm_device *dev = crtc->dev; >>> @@ -1051,10 +1078,6 @@ static const struct drm_driver msm_driver = { >>> .open = msm_open, >>> .postclose = msm_postclose, >>> .lastclose = drm_fb_helper_lastclose, >>> - .irq_handler = msm_irq, >>> - .irq_preinstall = msm_irq_preinstall, >>> - .irq_postinstall = msm_irq_postinstall, >>> - .irq_uninstall = msm_irq_uninstall, >>> .dumb_create = msm_gem_dumb_create, >>> .dumb_map_offset = msm_gem_dumb_map_offset, >>> .prime_handle_to_fd = drm_gem_prime_handle_to_fd, >>> diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h >>> index 086a2d59b8c8..9de7c42e1071 100644 >>> --- a/drivers/gpu/drm/msm/msm_kms.h >>> +++ b/drivers/gpu/drm/msm/msm_kms.h >>> @@ -150,7 +150,7 @@ struct msm_kms { >>> const struct msm_kms_funcs *funcs; >>> struct drm_device *dev; >>> >>> - /* irq number to be passed on to drm_irq_install */ >>> + /* irq number to be passed on to msm_irq_install */ >>> int irq; >>> >>> /* mapper-id used to request GEM buffer mapped for scanout: */ >>> >> >> >> -- >> With best wishes >> Dmitry
On Sat, Aug 7, 2021 at 11:40 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > Hi > > Am 07.08.21 um 19:08 schrieb Rob Clark: > > On Tue, Aug 3, 2021 at 2:37 AM Dmitry Baryshkov > > <dmitry.baryshkov@linaro.org> wrote: > >> > >> On 03/08/2021 12:06, Thomas Zimmermann wrote: > >>> Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's > >>> IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers > >>> don't benefit from using it. > >>> > >>> DRM IRQ callbacks are now being called directly or inlined. > >>> > >>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > >> > >> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > >> > >> Rob should probably also give his blessing on this patch though. > >> > > > > I've pushed this to msm-next-staging, but if Thomas would prefer to > > merge the series together then I can drop it (in which case a-b for > > this patch) > > Yes, please. I'd prefer to merge the whole patchset at once through > drm-misc-next. Ok, I've dropped from msm-next-staging.. for merging via drm-msm-next: Acked-by: Rob Clark <robdclark@chromium.org> > > Best regards > Thomas > > > > > BR, > > -R > > > >>> --- > >>> drivers/gpu/drm/msm/msm_drv.c | 113 ++++++++++++++++++++-------------- > >>> drivers/gpu/drm/msm/msm_kms.h | 2 +- > >>> 2 files changed, 69 insertions(+), 46 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > >>> index 1594ae39d54f..a332b09a5a11 100644 > >>> --- a/drivers/gpu/drm/msm/msm_drv.c > >>> +++ b/drivers/gpu/drm/msm/msm_drv.c > >>> @@ -14,7 +14,6 @@ > >>> #include <drm/drm_drv.h> > >>> #include <drm/drm_file.h> > >>> #include <drm/drm_ioctl.h> > >>> -#include <drm/drm_irq.h> > >>> #include <drm/drm_prime.h> > >>> #include <drm/drm_of.h> > >>> #include <drm/drm_vblank.h> > >>> @@ -201,6 +200,71 @@ void msm_rmw(void __iomem *addr, u32 mask, u32 or) > >>> msm_writel(val | or, addr); > >>> } > >>> > >>> +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) > >>> +{ > >>> + 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; > >>> + > >>> + ret = msm_irq_postinstall(dev); > >>> + if (ret) { > >>> + free_irq(irq, dev); > >>> + return ret; > >>> + } > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +static void msm_irq_uninstall(struct drm_device *dev) > >>> +{ > >>> + struct msm_drm_private *priv = dev->dev_private; > >>> + struct msm_kms *kms = priv->kms; > >>> + > >>> + kms->funcs->irq_uninstall(kms); > >>> + free_irq(kms->irq, dev); > >>> +} > >>> + > >>> struct msm_vblank_work { > >>> struct work_struct work; > >>> int crtc_id; > >>> @@ -265,7 +329,7 @@ static int msm_drm_uninit(struct device *dev) > >>> } > >>> > >>> /* We must cancel and cleanup any pending vblank enable/disable > >>> - * work before drm_irq_uninstall() to avoid work re-enabling an > >>> + * work before msm_irq_uninstall() to avoid work re-enabling an > >>> * irq after uninstall has disabled it. > >>> */ > >>> > >>> @@ -294,7 +358,7 @@ static int msm_drm_uninit(struct device *dev) > >>> drm_mode_config_cleanup(ddev); > >>> > >>> pm_runtime_get_sync(dev); > >>> - drm_irq_uninstall(ddev); > >>> + msm_irq_uninstall(ddev); > >>> pm_runtime_put_sync(dev); > >>> > >>> if (kms && kms->funcs) > >>> @@ -553,7 +617,7 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv) > >>> > >>> if (kms) { > >>> pm_runtime_get_sync(dev); > >>> - ret = drm_irq_install(ddev, kms->irq); > >>> + ret = msm_irq_install(ddev, kms->irq); > >>> pm_runtime_put_sync(dev); > >>> if (ret < 0) { > >>> DRM_DEV_ERROR(dev, "failed to install IRQ handler\n"); > >>> @@ -662,43 +726,6 @@ static void msm_postclose(struct drm_device *dev, struct drm_file *file) > >>> context_close(ctx); > >>> } > >>> > >>> -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 void msm_irq_uninstall(struct drm_device *dev) > >>> -{ > >>> - struct msm_drm_private *priv = dev->dev_private; > >>> - struct msm_kms *kms = priv->kms; > >>> - BUG_ON(!kms); > >>> - kms->funcs->irq_uninstall(kms); > >>> -} > >>> - > >>> int msm_crtc_enable_vblank(struct drm_crtc *crtc) > >>> { > >>> struct drm_device *dev = crtc->dev; > >>> @@ -1051,10 +1078,6 @@ static const struct drm_driver msm_driver = { > >>> .open = msm_open, > >>> .postclose = msm_postclose, > >>> .lastclose = drm_fb_helper_lastclose, > >>> - .irq_handler = msm_irq, > >>> - .irq_preinstall = msm_irq_preinstall, > >>> - .irq_postinstall = msm_irq_postinstall, > >>> - .irq_uninstall = msm_irq_uninstall, > >>> .dumb_create = msm_gem_dumb_create, > >>> .dumb_map_offset = msm_gem_dumb_map_offset, > >>> .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > >>> diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h > >>> index 086a2d59b8c8..9de7c42e1071 100644 > >>> --- a/drivers/gpu/drm/msm/msm_kms.h > >>> +++ b/drivers/gpu/drm/msm/msm_kms.h > >>> @@ -150,7 +150,7 @@ struct msm_kms { > >>> const struct msm_kms_funcs *funcs; > >>> struct drm_device *dev; > >>> > >>> - /* irq number to be passed on to drm_irq_install */ > >>> + /* irq number to be passed on to msm_irq_install */ > >>> int irq; > >>> > >>> /* mapper-id used to request GEM buffer mapped for scanout: */ > >>> > >> > >> > >> -- > >> With best wishes > >> Dmitry > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer >
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 1594ae39d54f..a332b09a5a11 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -14,7 +14,6 @@ #include <drm/drm_drv.h> #include <drm/drm_file.h> #include <drm/drm_ioctl.h> -#include <drm/drm_irq.h> #include <drm/drm_prime.h> #include <drm/drm_of.h> #include <drm/drm_vblank.h> @@ -201,6 +200,71 @@ void msm_rmw(void __iomem *addr, u32 mask, u32 or) msm_writel(val | or, addr); } +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) +{ + 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; + + ret = msm_irq_postinstall(dev); + if (ret) { + free_irq(irq, dev); + return ret; + } + + return 0; +} + +static void msm_irq_uninstall(struct drm_device *dev) +{ + struct msm_drm_private *priv = dev->dev_private; + struct msm_kms *kms = priv->kms; + + kms->funcs->irq_uninstall(kms); + free_irq(kms->irq, dev); +} + struct msm_vblank_work { struct work_struct work; int crtc_id; @@ -265,7 +329,7 @@ static int msm_drm_uninit(struct device *dev) } /* We must cancel and cleanup any pending vblank enable/disable - * work before drm_irq_uninstall() to avoid work re-enabling an + * work before msm_irq_uninstall() to avoid work re-enabling an * irq after uninstall has disabled it. */ @@ -294,7 +358,7 @@ static int msm_drm_uninit(struct device *dev) drm_mode_config_cleanup(ddev); pm_runtime_get_sync(dev); - drm_irq_uninstall(ddev); + msm_irq_uninstall(ddev); pm_runtime_put_sync(dev); if (kms && kms->funcs) @@ -553,7 +617,7 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv) if (kms) { pm_runtime_get_sync(dev); - ret = drm_irq_install(ddev, kms->irq); + ret = msm_irq_install(ddev, kms->irq); pm_runtime_put_sync(dev); if (ret < 0) { DRM_DEV_ERROR(dev, "failed to install IRQ handler\n"); @@ -662,43 +726,6 @@ static void msm_postclose(struct drm_device *dev, struct drm_file *file) context_close(ctx); } -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 void msm_irq_uninstall(struct drm_device *dev) -{ - struct msm_drm_private *priv = dev->dev_private; - struct msm_kms *kms = priv->kms; - BUG_ON(!kms); - kms->funcs->irq_uninstall(kms); -} - int msm_crtc_enable_vblank(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; @@ -1051,10 +1078,6 @@ static const struct drm_driver msm_driver = { .open = msm_open, .postclose = msm_postclose, .lastclose = drm_fb_helper_lastclose, - .irq_handler = msm_irq, - .irq_preinstall = msm_irq_preinstall, - .irq_postinstall = msm_irq_postinstall, - .irq_uninstall = msm_irq_uninstall, .dumb_create = msm_gem_dumb_create, .dumb_map_offset = msm_gem_dumb_map_offset, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h index 086a2d59b8c8..9de7c42e1071 100644 --- a/drivers/gpu/drm/msm/msm_kms.h +++ b/drivers/gpu/drm/msm/msm_kms.h @@ -150,7 +150,7 @@ struct msm_kms { const struct msm_kms_funcs *funcs; struct drm_device *dev; - /* irq number to be passed on to drm_irq_install */ + /* irq number to be passed on to msm_irq_install */ int irq; /* mapper-id used to request GEM buffer mapped for scanout: */
Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers don't benefit from using it. DRM IRQ callbacks are now being called directly or inlined. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/msm/msm_drv.c | 113 ++++++++++++++++++++-------------- drivers/gpu/drm/msm/msm_kms.h | 2 +- 2 files changed, 69 insertions(+), 46 deletions(-)