* [PATCH 0/2] drm/meson: Error handling fix when AFBCD is used @ 2021-12-30 23:55 Martin Blumenstingl 2021-12-30 23:55 ` [PATCH 1/2] drm/meson: osd_afbcd: Add an exit callback to struct meson_afbcd_ops Martin Blumenstingl 2021-12-30 23:55 ` [PATCH 2/2] drm/meson: Fix error handling when afbcd.ops->init fails Martin Blumenstingl 0 siblings, 2 replies; 5+ messages in thread From: Martin Blumenstingl @ 2021-12-30 23:55 UTC (permalink / raw) To: linux-amlogic Cc: dri-devel, linux-arm-kernel, linux-kernel, narmstrong, Martin Blumenstingl Hi Neil, this series consists of a small cleanup patch and a fix for the error handling in meson_drv_bind_master() when AFBCD is used. The patches are based on drm-misc to not conflict with your previous driver rework. Since the problem has not been observed in the wild I decided not to Cc linux-stable. Best regards, Martin Martin Blumenstingl (2): drm/meson: osd_afbcd: Add an exit callback to struct meson_afbcd_ops drm/meson: Fix error handling when afbcd.ops->init fails drivers/gpu/drm/meson/meson_drv.c | 25 +++++++-------- drivers/gpu/drm/meson/meson_osd_afbcd.c | 41 ++++++++++++++++--------- drivers/gpu/drm/meson/meson_osd_afbcd.h | 1 + 3 files changed, 41 insertions(+), 26 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] drm/meson: osd_afbcd: Add an exit callback to struct meson_afbcd_ops 2021-12-30 23:55 [PATCH 0/2] drm/meson: Error handling fix when AFBCD is used Martin Blumenstingl @ 2021-12-30 23:55 ` Martin Blumenstingl 2022-01-03 11:24 ` Neil Armstrong 2021-12-30 23:55 ` [PATCH 2/2] drm/meson: Fix error handling when afbcd.ops->init fails Martin Blumenstingl 1 sibling, 1 reply; 5+ messages in thread From: Martin Blumenstingl @ 2021-12-30 23:55 UTC (permalink / raw) To: linux-amlogic Cc: dri-devel, linux-arm-kernel, linux-kernel, narmstrong, Martin Blumenstingl Use this to simplify the driver shutdown. It will also come handy when fixing the error handling in meson_drv_bind_master(). Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- drivers/gpu/drm/meson/meson_drv.c | 6 ++-- drivers/gpu/drm/meson/meson_osd_afbcd.c | 41 ++++++++++++++++--------- drivers/gpu/drm/meson/meson_osd_afbcd.h | 1 + 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c index 80f1d439841a..b919271a6e50 100644 --- a/drivers/gpu/drm/meson/meson_drv.c +++ b/drivers/gpu/drm/meson/meson_drv.c @@ -385,10 +385,8 @@ static void meson_drv_unbind(struct device *dev) free_irq(priv->vsync_irq, drm); drm_dev_put(drm); - if (priv->afbcd.ops) { - priv->afbcd.ops->reset(priv); - meson_rdma_free(priv); - } + if (priv->afbcd.ops) + priv->afbcd.ops->exit(priv); } static const struct component_master_ops meson_drv_master_ops = { diff --git a/drivers/gpu/drm/meson/meson_osd_afbcd.c b/drivers/gpu/drm/meson/meson_osd_afbcd.c index ffc6b584dbf8..0cdbe899402f 100644 --- a/drivers/gpu/drm/meson/meson_osd_afbcd.c +++ b/drivers/gpu/drm/meson/meson_osd_afbcd.c @@ -79,11 +79,6 @@ static bool meson_gxm_afbcd_supported_fmt(u64 modifier, uint32_t format) return meson_gxm_afbcd_pixel_fmt(modifier, format) >= 0; } -static int meson_gxm_afbcd_init(struct meson_drm *priv) -{ - return 0; -} - static int meson_gxm_afbcd_reset(struct meson_drm *priv) { writel_relaxed(VIU_SW_RESET_OSD1_AFBCD, @@ -93,6 +88,16 @@ static int meson_gxm_afbcd_reset(struct meson_drm *priv) return 0; } +static int meson_gxm_afbcd_init(struct meson_drm *priv) +{ + return 0; +} + +static void meson_gxm_afbcd_exit(struct meson_drm *priv) +{ + meson_gxm_afbcd_reset(priv); +} + static int meson_gxm_afbcd_enable(struct meson_drm *priv) { writel_relaxed(FIELD_PREP(OSD1_AFBCD_ID_FIFO_THRD, 0x40) | @@ -172,6 +177,7 @@ static int meson_gxm_afbcd_setup(struct meson_drm *priv) struct meson_afbcd_ops meson_afbcd_gxm_ops = { .init = meson_gxm_afbcd_init, + .exit = meson_gxm_afbcd_exit, .reset = meson_gxm_afbcd_reset, .enable = meson_gxm_afbcd_enable, .disable = meson_gxm_afbcd_disable, @@ -269,6 +275,18 @@ static bool meson_g12a_afbcd_supported_fmt(u64 modifier, uint32_t format) return meson_g12a_afbcd_pixel_fmt(modifier, format) >= 0; } +static int meson_g12a_afbcd_reset(struct meson_drm *priv) +{ + meson_rdma_reset(priv); + + meson_rdma_writel_sync(priv, VIU_SW_RESET_G12A_AFBC_ARB | + VIU_SW_RESET_G12A_OSD1_AFBCD, + VIU_SW_RESET); + meson_rdma_writel_sync(priv, 0, VIU_SW_RESET); + + return 0; +} + static int meson_g12a_afbcd_init(struct meson_drm *priv) { int ret; @@ -286,16 +304,10 @@ static int meson_g12a_afbcd_init(struct meson_drm *priv) return 0; } -static int meson_g12a_afbcd_reset(struct meson_drm *priv) +static void meson_g12a_afbcd_exit(struct meson_drm *priv) { - meson_rdma_reset(priv); - - meson_rdma_writel_sync(priv, VIU_SW_RESET_G12A_AFBC_ARB | - VIU_SW_RESET_G12A_OSD1_AFBCD, - VIU_SW_RESET); - meson_rdma_writel_sync(priv, 0, VIU_SW_RESET); - - return 0; + meson_g12a_afbcd_reset(priv); + meson_rdma_free(priv); } static int meson_g12a_afbcd_enable(struct meson_drm *priv) @@ -380,6 +392,7 @@ static int meson_g12a_afbcd_setup(struct meson_drm *priv) struct meson_afbcd_ops meson_afbcd_g12a_ops = { .init = meson_g12a_afbcd_init, + .exit = meson_g12a_afbcd_exit, .reset = meson_g12a_afbcd_reset, .enable = meson_g12a_afbcd_enable, .disable = meson_g12a_afbcd_disable, diff --git a/drivers/gpu/drm/meson/meson_osd_afbcd.h b/drivers/gpu/drm/meson/meson_osd_afbcd.h index 5e5523304f42..e77ddeb6416f 100644 --- a/drivers/gpu/drm/meson/meson_osd_afbcd.h +++ b/drivers/gpu/drm/meson/meson_osd_afbcd.h @@ -14,6 +14,7 @@ struct meson_afbcd_ops { int (*init)(struct meson_drm *priv); + void (*exit)(struct meson_drm *priv); int (*reset)(struct meson_drm *priv); int (*enable)(struct meson_drm *priv); int (*disable)(struct meson_drm *priv); -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] drm/meson: osd_afbcd: Add an exit callback to struct meson_afbcd_ops 2021-12-30 23:55 ` [PATCH 1/2] drm/meson: osd_afbcd: Add an exit callback to struct meson_afbcd_ops Martin Blumenstingl @ 2022-01-03 11:24 ` Neil Armstrong 0 siblings, 0 replies; 5+ messages in thread From: Neil Armstrong @ 2022-01-03 11:24 UTC (permalink / raw) To: Martin Blumenstingl, linux-amlogic Cc: dri-devel, linux-arm-kernel, linux-kernel On 31/12/2021 00:55, Martin Blumenstingl wrote: > Use this to simplify the driver shutdown. It will also come handy when > fixing the error handling in meson_drv_bind_master(). > > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > --- > drivers/gpu/drm/meson/meson_drv.c | 6 ++-- > drivers/gpu/drm/meson/meson_osd_afbcd.c | 41 ++++++++++++++++--------- > drivers/gpu/drm/meson/meson_osd_afbcd.h | 1 + > 3 files changed, 30 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c > index 80f1d439841a..b919271a6e50 100644 > --- a/drivers/gpu/drm/meson/meson_drv.c > +++ b/drivers/gpu/drm/meson/meson_drv.c > @@ -385,10 +385,8 @@ static void meson_drv_unbind(struct device *dev) > free_irq(priv->vsync_irq, drm); > drm_dev_put(drm); > > - if (priv->afbcd.ops) { > - priv->afbcd.ops->reset(priv); > - meson_rdma_free(priv); > - } > + if (priv->afbcd.ops) > + priv->afbcd.ops->exit(priv); > } > > static const struct component_master_ops meson_drv_master_ops = { > diff --git a/drivers/gpu/drm/meson/meson_osd_afbcd.c b/drivers/gpu/drm/meson/meson_osd_afbcd.c > index ffc6b584dbf8..0cdbe899402f 100644 > --- a/drivers/gpu/drm/meson/meson_osd_afbcd.c > +++ b/drivers/gpu/drm/meson/meson_osd_afbcd.c > @@ -79,11 +79,6 @@ static bool meson_gxm_afbcd_supported_fmt(u64 modifier, uint32_t format) > return meson_gxm_afbcd_pixel_fmt(modifier, format) >= 0; > } > > -static int meson_gxm_afbcd_init(struct meson_drm *priv) > -{ > - return 0; > -} > - > static int meson_gxm_afbcd_reset(struct meson_drm *priv) > { > writel_relaxed(VIU_SW_RESET_OSD1_AFBCD, > @@ -93,6 +88,16 @@ static int meson_gxm_afbcd_reset(struct meson_drm *priv) > return 0; > } > > +static int meson_gxm_afbcd_init(struct meson_drm *priv) > +{ > + return 0; > +} > + > +static void meson_gxm_afbcd_exit(struct meson_drm *priv) > +{ > + meson_gxm_afbcd_reset(priv); > +} > + > static int meson_gxm_afbcd_enable(struct meson_drm *priv) > { > writel_relaxed(FIELD_PREP(OSD1_AFBCD_ID_FIFO_THRD, 0x40) | > @@ -172,6 +177,7 @@ static int meson_gxm_afbcd_setup(struct meson_drm *priv) > > struct meson_afbcd_ops meson_afbcd_gxm_ops = { > .init = meson_gxm_afbcd_init, > + .exit = meson_gxm_afbcd_exit, > .reset = meson_gxm_afbcd_reset, > .enable = meson_gxm_afbcd_enable, > .disable = meson_gxm_afbcd_disable, > @@ -269,6 +275,18 @@ static bool meson_g12a_afbcd_supported_fmt(u64 modifier, uint32_t format) > return meson_g12a_afbcd_pixel_fmt(modifier, format) >= 0; > } > > +static int meson_g12a_afbcd_reset(struct meson_drm *priv) > +{ > + meson_rdma_reset(priv); > + > + meson_rdma_writel_sync(priv, VIU_SW_RESET_G12A_AFBC_ARB | > + VIU_SW_RESET_G12A_OSD1_AFBCD, > + VIU_SW_RESET); > + meson_rdma_writel_sync(priv, 0, VIU_SW_RESET); > + > + return 0; > +} > + > static int meson_g12a_afbcd_init(struct meson_drm *priv) > { > int ret; > @@ -286,16 +304,10 @@ static int meson_g12a_afbcd_init(struct meson_drm *priv) > return 0; > } > > -static int meson_g12a_afbcd_reset(struct meson_drm *priv) > +static void meson_g12a_afbcd_exit(struct meson_drm *priv) > { > - meson_rdma_reset(priv); > - > - meson_rdma_writel_sync(priv, VIU_SW_RESET_G12A_AFBC_ARB | > - VIU_SW_RESET_G12A_OSD1_AFBCD, > - VIU_SW_RESET); > - meson_rdma_writel_sync(priv, 0, VIU_SW_RESET); > - > - return 0; > + meson_g12a_afbcd_reset(priv); > + meson_rdma_free(priv); > } > > static int meson_g12a_afbcd_enable(struct meson_drm *priv) > @@ -380,6 +392,7 @@ static int meson_g12a_afbcd_setup(struct meson_drm *priv) > > struct meson_afbcd_ops meson_afbcd_g12a_ops = { > .init = meson_g12a_afbcd_init, > + .exit = meson_g12a_afbcd_exit, > .reset = meson_g12a_afbcd_reset, > .enable = meson_g12a_afbcd_enable, > .disable = meson_g12a_afbcd_disable, > diff --git a/drivers/gpu/drm/meson/meson_osd_afbcd.h b/drivers/gpu/drm/meson/meson_osd_afbcd.h > index 5e5523304f42..e77ddeb6416f 100644 > --- a/drivers/gpu/drm/meson/meson_osd_afbcd.h > +++ b/drivers/gpu/drm/meson/meson_osd_afbcd.h > @@ -14,6 +14,7 @@ > > struct meson_afbcd_ops { > int (*init)(struct meson_drm *priv); > + void (*exit)(struct meson_drm *priv); > int (*reset)(struct meson_drm *priv); > int (*enable)(struct meson_drm *priv); > int (*disable)(struct meson_drm *priv); > Acked-by: Neil Armstrong <narmstrong@baylibre.com> Thanks for fixing this Neil ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] drm/meson: Fix error handling when afbcd.ops->init fails 2021-12-30 23:55 [PATCH 0/2] drm/meson: Error handling fix when AFBCD is used Martin Blumenstingl 2021-12-30 23:55 ` [PATCH 1/2] drm/meson: osd_afbcd: Add an exit callback to struct meson_afbcd_ops Martin Blumenstingl @ 2021-12-30 23:55 ` Martin Blumenstingl 2022-01-03 11:22 ` Neil Armstrong 1 sibling, 1 reply; 5+ messages in thread From: Martin Blumenstingl @ 2021-12-30 23:55 UTC (permalink / raw) To: linux-amlogic Cc: dri-devel, linux-arm-kernel, linux-kernel, narmstrong, Martin Blumenstingl When afbcd.ops->init fails we need to free the struct drm_device. Also all errors which come after afbcd.ops->init was successful need to exit the AFBCD, just like meson_drv_unbind() does. Fixes: d1b5e41e13a7e9 ("drm/meson: Add AFBCD module driver") Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- drivers/gpu/drm/meson/meson_drv.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c index b919271a6e50..26aeaf0ab86e 100644 --- a/drivers/gpu/drm/meson/meson_drv.c +++ b/drivers/gpu/drm/meson/meson_drv.c @@ -302,42 +302,42 @@ static int meson_drv_bind_master(struct device *dev, bool has_components) if (priv->afbcd.ops) { ret = priv->afbcd.ops->init(priv); if (ret) - return ret; + goto free_drm; } /* Encoder Initialization */ ret = meson_encoder_cvbs_init(priv); if (ret) - goto free_drm; + goto exit_afbcd; if (has_components) { ret = component_bind_all(drm->dev, drm); if (ret) { dev_err(drm->dev, "Couldn't bind all components\n"); - goto free_drm; + goto exit_afbcd; } } ret = meson_encoder_hdmi_init(priv); if (ret) - goto free_drm; + goto exit_afbcd; ret = meson_plane_create(priv); if (ret) - goto free_drm; + goto exit_afbcd; ret = meson_overlay_create(priv); if (ret) - goto free_drm; + goto exit_afbcd; ret = meson_crtc_create(priv); if (ret) - goto free_drm; + goto exit_afbcd; ret = request_irq(priv->vsync_irq, meson_irq, 0, drm->driver->name, drm); if (ret) - goto free_drm; + goto exit_afbcd; drm_mode_config_reset(drm); @@ -355,6 +355,9 @@ static int meson_drv_bind_master(struct device *dev, bool has_components) uninstall_irq: free_irq(priv->vsync_irq, drm); +exit_afbcd: + if (priv->afbcd.ops) + priv->afbcd.ops->exit(priv); free_drm: drm_dev_put(drm); -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] drm/meson: Fix error handling when afbcd.ops->init fails 2021-12-30 23:55 ` [PATCH 2/2] drm/meson: Fix error handling when afbcd.ops->init fails Martin Blumenstingl @ 2022-01-03 11:22 ` Neil Armstrong 0 siblings, 0 replies; 5+ messages in thread From: Neil Armstrong @ 2022-01-03 11:22 UTC (permalink / raw) To: Martin Blumenstingl, linux-amlogic Cc: dri-devel, linux-arm-kernel, linux-kernel On 31/12/2021 00:55, Martin Blumenstingl wrote: > When afbcd.ops->init fails we need to free the struct drm_device. Also > all errors which come after afbcd.ops->init was successful need to exit > the AFBCD, just like meson_drv_unbind() does. > > Fixes: d1b5e41e13a7e9 ("drm/meson: Add AFBCD module driver") > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > --- > drivers/gpu/drm/meson/meson_drv.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c > index b919271a6e50..26aeaf0ab86e 100644 > --- a/drivers/gpu/drm/meson/meson_drv.c > +++ b/drivers/gpu/drm/meson/meson_drv.c > @@ -302,42 +302,42 @@ static int meson_drv_bind_master(struct device *dev, bool has_components) > if (priv->afbcd.ops) { > ret = priv->afbcd.ops->init(priv); > if (ret) > - return ret; > + goto free_drm; > } > > /* Encoder Initialization */ > > ret = meson_encoder_cvbs_init(priv); > if (ret) > - goto free_drm; > + goto exit_afbcd; > > if (has_components) { > ret = component_bind_all(drm->dev, drm); > if (ret) { > dev_err(drm->dev, "Couldn't bind all components\n"); > - goto free_drm; > + goto exit_afbcd; > } > } > > ret = meson_encoder_hdmi_init(priv); > if (ret) > - goto free_drm; > + goto exit_afbcd; > > ret = meson_plane_create(priv); > if (ret) > - goto free_drm; > + goto exit_afbcd; > > ret = meson_overlay_create(priv); > if (ret) > - goto free_drm; > + goto exit_afbcd; > > ret = meson_crtc_create(priv); > if (ret) > - goto free_drm; > + goto exit_afbcd; > > ret = request_irq(priv->vsync_irq, meson_irq, 0, drm->driver->name, drm); > if (ret) > - goto free_drm; > + goto exit_afbcd; > > drm_mode_config_reset(drm); > > @@ -355,6 +355,9 @@ static int meson_drv_bind_master(struct device *dev, bool has_components) > > uninstall_irq: > free_irq(priv->vsync_irq, drm); > +exit_afbcd: > + if (priv->afbcd.ops) > + priv->afbcd.ops->exit(priv); > free_drm: > drm_dev_put(drm); > > Acked-by: Neil Armstrong <narmstrong@baylibre.com> Thanks, But this depends on patch 1, so I'll add the same Fixes tag on patch 1 and add to drm-misc-next so it won't pollute -fixes but will still be eventually be backported when landing on linus master. Neil ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-03 11:24 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-30 23:55 [PATCH 0/2] drm/meson: Error handling fix when AFBCD is used Martin Blumenstingl 2021-12-30 23:55 ` [PATCH 1/2] drm/meson: osd_afbcd: Add an exit callback to struct meson_afbcd_ops Martin Blumenstingl 2022-01-03 11:24 ` Neil Armstrong 2021-12-30 23:55 ` [PATCH 2/2] drm/meson: Fix error handling when afbcd.ops->init fails Martin Blumenstingl 2022-01-03 11:22 ` Neil Armstrong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).