drivers/gpu/drm/meson/meson_drv.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-)
meson_drv_bind_master() does not free resources in the order they are
allocated. This can lead to crashes such as:
Unable to handle kernel NULL pointer dereference at virtual address 00000000000000c8
[...]
Hardware name: Beelink GT-King Pro (DT)
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : meson_dw_hdmi_unbind+0x10/0x24 [meson_dw_hdmi]
lr : component_unbind+0x38/0x60
[...]
Call trace:
meson_dw_hdmi_unbind+0x10/0x24 [meson_dw_hdmi]
component_unbind+0x38/0x60
component_unbind_all+0xb8/0xc4
meson_drv_bind_master+0x1ec/0x514 [meson_drm]
meson_drv_bind+0x14/0x20 [meson_drm]
try_to_bring_up_aggregate_device+0xa8/0x160
__component_add+0xb8/0x1a8
component_add+0x14/0x20
meson_dw_hdmi_probe+0x1c/0x28 [meson_dw_hdmi]
platform_probe+0x68/0xdc
really_probe+0xc0/0x39c
__driver_probe_device+0x7c/0x14c
driver_probe_device+0x3c/0x120
__driver_attach+0xc4/0x200
bus_for_each_dev+0x78/0xd8
driver_attach+0x24/0x30
bus_add_driver+0x110/0x240
driver_register+0x68/0x124
__platform_driver_register+0x24/0x30
meson_dw_hdmi_platform_driver_init+0x20/0x1000 [meson_dw_hdmi]
do_one_initcall+0x50/0x1bc
do_init_module+0x54/0x1fc
load_module+0x788/0x884
init_module_from_file+0x88/0xd4
__arm64_sys_finit_module+0x248/0x340
invoke_syscall+0x48/0x104
el0_svc_common.constprop.0+0x40/0xe0
do_el0_svc+0x1c/0x28
el0_svc+0x30/0xcc
el0t_64_sync_handler+0x120/0x12c
el0t_64_sync+0x190/0x194
Clean up resources in the error path in the same order and under the
same conditions as they were allocated to fix this.
Reported-by: Furkan Kardame <f.kardame@manjaro.org>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
This issue was reported off-list so I'm unable to provide a link to the
report.
I'm not sure which Fixes tag fits best. My preference so far is
Fixes: 6a044642988b ("drm/meson: fix unbind path if HDMI fails to bind")
but I'll happily take other suggestions as well.
drivers/gpu/drm/meson/meson_drv.c | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index 81d2ee37e773..031686fd4104 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -314,35 +314,35 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
dev_err(drm->dev, "Couldn't bind all components\n");
/* Do not try to unbind */
has_components = false;
- goto exit_afbcd;
+ goto cvbs_encoder_remove;
}
}
ret = meson_encoder_hdmi_probe(priv);
if (ret)
- goto exit_afbcd;
+ goto unbind_components;
if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
ret = meson_encoder_dsi_probe(priv);
if (ret)
- goto exit_afbcd;
+ goto hdmi_encoder_remove;
}
ret = meson_plane_create(priv);
if (ret)
- goto exit_afbcd;
+ goto dsi_encoder_remove;
ret = meson_overlay_create(priv);
if (ret)
- goto exit_afbcd;
+ goto dsi_encoder_remove;
ret = meson_crtc_create(priv);
if (ret)
- goto exit_afbcd;
+ goto dsi_encoder_remove;
ret = request_irq(priv->vsync_irq, meson_irq, 0, drm->driver->name, drm);
if (ret)
- goto exit_afbcd;
+ goto dsi_encoder_remove;
drm_mode_config_reset(drm);
@@ -360,6 +360,16 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
uninstall_irq:
free_irq(priv->vsync_irq, drm);
+dsi_encoder_remove:
+ if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
+ meson_encoder_dsi_remove(priv);
+hdmi_encoder_remove:
+ meson_encoder_hdmi_remove(priv);
+unbind_components:
+ if (has_components)
+ component_unbind_all(dev, drm);
+cvbs_encoder_remove:
+ meson_encoder_cvbs_remove(priv);
exit_afbcd:
if (priv->afbcd.ops)
priv->afbcd.ops->exit(priv);
@@ -374,13 +384,6 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
free_drm:
drm_dev_put(drm);
- meson_encoder_dsi_remove(priv);
- meson_encoder_hdmi_remove(priv);
- meson_encoder_cvbs_remove(priv);
-
- if (has_components)
- component_unbind_all(dev, drm);
-
return ret;
}
--
2.49.0Martin Blumenstingl wrote:
> @@ -360,6 +360,16 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>
> uninstall_irq:
> free_irq(priv->vsync_irq, drm);
> +dsi_encoder_remove:
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
> + meson_encoder_dsi_remove(priv);
> +hdmi_encoder_remove:
> + meson_encoder_hdmi_remove(priv);
> +unbind_components:
> + if (has_components)
> + component_unbind_all(dev, drm);
As 6a044642988b ("drm/meson: fix unbind path if HDMI fails to bind")
states, it seems invalid to call component_unbind_all() before
drm_dev_put(). Wouldn't this patch reintroduce the problem here?
In that sense the diff proposed by Martijn <linux@martijnvandeventer.nl>
behaves more correctly.
I've also found this thread [1] with another error path fixing patch. It
was suggested to improve that fix with managed drm device [2] interfaces but
AFAICS using devm_drm_dev_alloc() will reintroduce the problem mentioned
in 6a044642988b, too.
I think [1] should be applied as well with Martijn's patch?
[1]: https://lore.kernel.org/dri-devel/20240809124725.17956-1-abelova@astralinux.ru/T/#u
[2]: https://lore.kernel.org/dri-devel/20240828110421.14956-1-abelova@astralinux.ru/T/#u
Thanks!
> +cvbs_encoder_remove:
> + meson_encoder_cvbs_remove(priv);
> exit_afbcd:
> if (priv->afbcd.ops)
> priv->afbcd.ops->exit(priv);
> @@ -374,13 +384,6 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
> free_drm:
> drm_dev_put(drm);
>
> - meson_encoder_dsi_remove(priv);
> - meson_encoder_hdmi_remove(priv);
> - meson_encoder_cvbs_remove(priv);
> -
> - if (has_components)
> - component_unbind_all(dev, drm);
> -
> return ret;
> }
>
> --
> 2.49.0
Hi Martin,
On Thu, 10 Apr 2025 at 03:30, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> meson_drv_bind_master() does not free resources in the order they are
> allocated. This can lead to crashes such as:
> Unable to handle kernel NULL pointer dereference at virtual address 00000000000000c8
> [...]
> Hardware name: Beelink GT-King Pro (DT)
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : meson_dw_hdmi_unbind+0x10/0x24 [meson_dw_hdmi]
> lr : component_unbind+0x38/0x60
> [...]
> Call trace:
> meson_dw_hdmi_unbind+0x10/0x24 [meson_dw_hdmi]
> component_unbind+0x38/0x60
> component_unbind_all+0xb8/0xc4
> meson_drv_bind_master+0x1ec/0x514 [meson_drm]
> meson_drv_bind+0x14/0x20 [meson_drm]
> try_to_bring_up_aggregate_device+0xa8/0x160
> __component_add+0xb8/0x1a8
> component_add+0x14/0x20
> meson_dw_hdmi_probe+0x1c/0x28 [meson_dw_hdmi]
> platform_probe+0x68/0xdc
> really_probe+0xc0/0x39c
> __driver_probe_device+0x7c/0x14c
> driver_probe_device+0x3c/0x120
> __driver_attach+0xc4/0x200
> bus_for_each_dev+0x78/0xd8
> driver_attach+0x24/0x30
> bus_add_driver+0x110/0x240
> driver_register+0x68/0x124
> __platform_driver_register+0x24/0x30
> meson_dw_hdmi_platform_driver_init+0x20/0x1000 [meson_dw_hdmi]
> do_one_initcall+0x50/0x1bc
> do_init_module+0x54/0x1fc
> load_module+0x788/0x884
> init_module_from_file+0x88/0xd4
> __arm64_sys_finit_module+0x248/0x340
> invoke_syscall+0x48/0x104
> el0_svc_common.constprop.0+0x40/0xe0
> do_el0_svc+0x1c/0x28
> el0_svc+0x30/0xcc
> el0t_64_sync_handler+0x120/0x12c
> el0t_64_sync+0x190/0x194
>
> Clean up resources in the error path in the same order and under the
> same conditions as they were allocated to fix this.
>
> Reported-by: Furkan Kardame <f.kardame@manjaro.org>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> This issue was reported off-list so I'm unable to provide a link to the
> report.
>
> I'm not sure which Fixes tag fits best. My preference so far is
> Fixes: 6a044642988b ("drm/meson: fix unbind path if HDMI fails to bind")
> but I'll happily take other suggestions as well.
>
Thanks for this Fix.
Reviewed-by: Anand Moon <linux.amoon@gmail.com>
Tested-by: Anand Moon <linux.amoon@gmail.com>
I have tested with the following script on AML-S905X-CC
---------
#! /bin/bash
set +x
cd /sys/bus/platform/drivers/meson-drm/
for i in $(seq 1 10); do
echo "========================" $i
echo d0100000.vpu > unbind
/usr/bin/sleep 1
echo d0100000.vpu > bind
/usr/bin/sleep 1
done
Thanks
-Anand
Hi Martin,
Thank you for the patch.
I encountered this issue some time ago as well and had a possible fix in my tree (see
below).
My apologies for not upstreaming it earlier.
While my fix is not as symmetric as yours—I like symmetry—it is somewhat simpler. It
did make the assumption that only calling component_unbind_all() was at fault and the the rest of the
code was correct. Therefore, calling one of the following functions:
meson_encoder_dsi_remove()
meson_encoder_hdmi_remove()
meson_encoder_cvbs_remove()
in case their counterpart was not called, should not result in any issues.
I just verified, and, as far as I understand, all of these functions do a check to confirm
whether the encoder was initialized before proceeding with cleanup.
-----------------------------------------------------
diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index 81d2ee37e773..4e2d45a271c2 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -184,6 +184,7 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
const struct meson_drm_match_data *match;
struct meson_drm *priv;
struct drm_device *drm;
+ bool do_unbind = false;
struct resource *res;
void __iomem *regs;
int ret, i;
@@ -312,10 +313,9 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
ret = component_bind_all(dev, drm);
if (ret) {
dev_err(drm->dev, "Couldn't bind all components\n");
- /* Do not try to unbind */
- has_components = false;
goto exit_afbcd;
}
+ do_unbind = true;
}
ret = meson_encoder_hdmi_probe(priv);
@@ -378,7 +378,7 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
meson_encoder_hdmi_remove(priv);
meson_encoder_cvbs_remove(priv);
- if (has_components)
+ if (do_unbind)
component_unbind_all(dev, drm);
return ret;
-----------------------------
This patch has somewhat less code redundancy. I don’t have a strong
preference for either patch, so please feel free to choose whichever you
prefer.
If you decide to go with yours, I’ve provided one minor comment inline
for your consideration.
> -----Original Message-----
> From: linux-amlogic <linux-amlogic-bounces@lists.infradead.org> On Behalf
> Of Martin Blumenstingl
> Sent: Wednesday, April 9, 2025 11:44 PM
> To: linux-amlogic@lists.infradead.org; dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> jbrunet@baylibre.com; neil.armstrong@linaro.org; Martin Blumenstingl
> <martin.blumenstingl@googlemail.com>; Furkan Kardame
> <f.kardame@manjaro.org>
> Subject: [PATCH] drm/meson: fix resource cleanup in
> meson_drv_bind_master() on error
>
> meson_drv_bind_master() does not free resources in the order they are
> allocated. This can lead to crashes such as:
> Unable to handle kernel NULL pointer dereference at virtual address
> 00000000000000c8
> [...]
> Hardware name: Beelink GT-King Pro (DT)
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : meson_dw_hdmi_unbind+0x10/0x24 [meson_dw_hdmi]
> lr : component_unbind+0x38/0x60
> [...]
> Call trace:
> meson_dw_hdmi_unbind+0x10/0x24 [meson_dw_hdmi]
> component_unbind+0x38/0x60
> component_unbind_all+0xb8/0xc4
> meson_drv_bind_master+0x1ec/0x514 [meson_drm]
> meson_drv_bind+0x14/0x20 [meson_drm]
> try_to_bring_up_aggregate_device+0xa8/0x160
> __component_add+0xb8/0x1a8
> component_add+0x14/0x20
> meson_dw_hdmi_probe+0x1c/0x28 [meson_dw_hdmi]
> platform_probe+0x68/0xdc
> really_probe+0xc0/0x39c
> __driver_probe_device+0x7c/0x14c
> driver_probe_device+0x3c/0x120
> __driver_attach+0xc4/0x200
> bus_for_each_dev+0x78/0xd8
> driver_attach+0x24/0x30
> bus_add_driver+0x110/0x240
> driver_register+0x68/0x124
> __platform_driver_register+0x24/0x30
> meson_dw_hdmi_platform_driver_init+0x20/0x1000 [meson_dw_hdmi]
> do_one_initcall+0x50/0x1bc
> do_init_module+0x54/0x1fc
> load_module+0x788/0x884
> init_module_from_file+0x88/0xd4
> __arm64_sys_finit_module+0x248/0x340
> invoke_syscall+0x48/0x104
> el0_svc_common.constprop.0+0x40/0xe0
> do_el0_svc+0x1c/0x28
> el0_svc+0x30/0xcc
> el0t_64_sync_handler+0x120/0x12c
> el0t_64_sync+0x190/0x194
>
> Clean up resources in the error path in the same order and under the
> same conditions as they were allocated to fix this.
>
> Reported-by: Furkan Kardame <f.kardame@manjaro.org>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> This issue was reported off-list so I'm unable to provide a link to the
> report.
>
> I'm not sure which Fixes tag fits best. My preference so far is
> Fixes: 6a044642988b ("drm/meson: fix unbind path if HDMI fails to bind")
> but I'll happily take other suggestions as well.
>
>
> drivers/gpu/drm/meson/meson_drv.c | 31 +++++++++++++++++--------------
> 1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/meson/meson_drv.c
> b/drivers/gpu/drm/meson/meson_drv.c
> index 81d2ee37e773..031686fd4104 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -314,35 +314,35 @@ static int meson_drv_bind_master(struct device
> *dev, bool has_components)
> dev_err(drm->dev, "Couldn't bind all
> components\n");
> /* Do not try to unbind */
> has_components = false;
Above two lines are no longer used, so can be removed.
> - goto exit_afbcd;
> + goto cvbs_encoder_remove;
> }
> }
>
> ret = meson_encoder_hdmi_probe(priv);
> if (ret)
> - goto exit_afbcd;
> + goto unbind_components;
>
> if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
> ret = meson_encoder_dsi_probe(priv);
> if (ret)
> - goto exit_afbcd;
> + goto hdmi_encoder_remove;
> }
>
> ret = meson_plane_create(priv);
> if (ret)
> - goto exit_afbcd;
> + goto dsi_encoder_remove;
>
> ret = meson_overlay_create(priv);
> if (ret)
> - goto exit_afbcd;
> + goto dsi_encoder_remove;
>
> ret = meson_crtc_create(priv);
> if (ret)
> - goto exit_afbcd;
> + goto dsi_encoder_remove;
>
> ret = request_irq(priv->vsync_irq, meson_irq, 0, drm->driver->name,
> drm);
> if (ret)
> - goto exit_afbcd;
> + goto dsi_encoder_remove;
>
> drm_mode_config_reset(drm);
>
> @@ -360,6 +360,16 @@ static int meson_drv_bind_master(struct device
> *dev, bool has_components)
>
> uninstall_irq:
> free_irq(priv->vsync_irq, drm);
> +dsi_encoder_remove:
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
> + meson_encoder_dsi_remove(priv);
> +hdmi_encoder_remove:
> + meson_encoder_hdmi_remove(priv);
> +unbind_components:
> + if (has_components)
> + component_unbind_all(dev, drm);
> +cvbs_encoder_remove:
> + meson_encoder_cvbs_remove(priv);
> exit_afbcd:
> if (priv->afbcd.ops)
> priv->afbcd.ops->exit(priv);
> @@ -374,13 +384,6 @@ static int meson_drv_bind_master(struct device
> *dev, bool has_components)
> free_drm:
> drm_dev_put(drm);
>
> - meson_encoder_dsi_remove(priv);
> - meson_encoder_hdmi_remove(priv);
> - meson_encoder_cvbs_remove(priv);
> -
> - if (has_components)
> - component_unbind_all(dev, drm);
> -
> return ret;
> }
>
--
Best regards,
MartijnHi Martijn, Hi Neil, On Thu, Apr 10, 2025 at 8:46 PM <linux@martijnvandeventer.nl> wrote: > > Hi Martin, > > Thank you for the patch. > > I encountered this issue some time ago as well and had a possible fix in my tree (see > below). > My apologies for not upstreaming it earlier. No worries, we're all busy with both, offline and online life ;-) > While my fix is not as symmetric as yours—I like symmetry—it is somewhat simpler. It > did make the assumption that only calling component_unbind_all() was at fault and the the rest of the > code was correct. Therefore, calling one of the following functions: > meson_encoder_dsi_remove() > meson_encoder_hdmi_remove() > meson_encoder_cvbs_remove() > in case their counterpart was not called, should not result in any issues. > > I just verified, and, as far as I understand, all of these functions do a check to confirm > whether the encoder was initialized before proceeding with cleanup. Yep, that seems to be the case right now. Neil, would you like Martijn's more simple approach with a Fixes tag and backport that? Then I'd send my patch as a small cleanup which doesn't have to be backported. Otherwise I'd spin a v2 with a fix for the issue that Martijn found (and including Anand's Reviewed/Tested-by)? [...] > > diff --git a/drivers/gpu/drm/meson/meson_drv.c > > b/drivers/gpu/drm/meson/meson_drv.c > > index 81d2ee37e773..031686fd4104 100644 > > --- a/drivers/gpu/drm/meson/meson_drv.c > > +++ b/drivers/gpu/drm/meson/meson_drv.c > > @@ -314,35 +314,35 @@ static int meson_drv_bind_master(struct device > > *dev, bool has_components) > > dev_err(drm->dev, "Couldn't bind all > > components\n"); > > /* Do not try to unbind */ > > has_components = false; > > Above two lines are no longer used, so can be removed. Well spotted, thank you! Best regards, Martin
On 19/04/2025 23:32, Martin Blumenstingl wrote: > Hi Martijn, Hi Neil, > > On Thu, Apr 10, 2025 at 8:46 PM <linux@martijnvandeventer.nl> wrote: >> >> Hi Martin, >> >> Thank you for the patch. >> >> I encountered this issue some time ago as well and had a possible fix in my tree (see >> below). >> My apologies for not upstreaming it earlier. > No worries, we're all busy with both, offline and online life ;-) > >> While my fix is not as symmetric as yours—I like symmetry—it is somewhat simpler. It >> did make the assumption that only calling component_unbind_all() was at fault and the the rest of the >> code was correct. Therefore, calling one of the following functions: >> meson_encoder_dsi_remove() >> meson_encoder_hdmi_remove() >> meson_encoder_cvbs_remove() >> in case their counterpart was not called, should not result in any issues. >> >> I just verified, and, as far as I understand, all of these functions do a check to confirm >> whether the encoder was initialized before proceeding with cleanup. > Yep, that seems to be the case right now. > Neil, would you like Martijn's more simple approach with a Fixes tag > and backport that? > Then I'd send my patch as a small cleanup which doesn't have to be > backported. Otherwise I'd spin a v2 with a fix for the issue that > Martijn found (and including Anand's Reviewed/Tested-by)? Please send a follow-up, I'll apply this one today. Thanks, Neil > > [...] >>> diff --git a/drivers/gpu/drm/meson/meson_drv.c >>> b/drivers/gpu/drm/meson/meson_drv.c >>> index 81d2ee37e773..031686fd4104 100644 >>> --- a/drivers/gpu/drm/meson/meson_drv.c >>> +++ b/drivers/gpu/drm/meson/meson_drv.c >>> @@ -314,35 +314,35 @@ static int meson_drv_bind_master(struct device >>> *dev, bool has_components) >>> dev_err(drm->dev, "Couldn't bind all >>> components\n"); >>> /* Do not try to unbind */ >>> has_components = false; >> >> Above two lines are no longer used, so can be removed. > Well spotted, thank you! > > > Best regards, > Martin
On 22/04/2025 09:04, neil.armstrong@linaro.org wrote: > On 19/04/2025 23:32, Martin Blumenstingl wrote: >> Hi Martijn, Hi Neil, >> >> On Thu, Apr 10, 2025 at 8:46 PM <linux@martijnvandeventer.nl> wrote: >>> >>> Hi Martin, >>> >>> Thank you for the patch. >>> >>> I encountered this issue some time ago as well and had a possible fix in my tree (see >>> below). >>> My apologies for not upstreaming it earlier. >> No worries, we're all busy with both, offline and online life ;-) >> >>> While my fix is not as symmetric as yours—I like symmetry—it is somewhat simpler. It >>> did make the assumption that only calling component_unbind_all() was at fault and the the rest of the >>> code was correct. Therefore, calling one of the following functions: >>> meson_encoder_dsi_remove() >>> meson_encoder_hdmi_remove() >>> meson_encoder_cvbs_remove() >>> in case their counterpart was not called, should not result in any issues. >>> >>> I just verified, and, as far as I understand, all of these functions do a check to confirm >>> whether the encoder was initialized before proceeding with cleanup. >> Yep, that seems to be the case right now. >> Neil, would you like Martijn's more simple approach with a Fixes tag >> and backport that? >> Then I'd send my patch as a small cleanup which doesn't have to be >> backported. Otherwise I'd spin a v2 with a fix for the issue that >> Martijn found (and including Anand's Reviewed/Tested-by)? > > Please send a follow-up, I'll apply this one today. > Yeah finally please split this in two: - patch 1 that can be backported - patch 2 remaining changes Thanks, Neil > Thanks, > Neil > >> >> [...] >>>> diff --git a/drivers/gpu/drm/meson/meson_drv.c >>>> b/drivers/gpu/drm/meson/meson_drv.c >>>> index 81d2ee37e773..031686fd4104 100644 >>>> --- a/drivers/gpu/drm/meson/meson_drv.c >>>> +++ b/drivers/gpu/drm/meson/meson_drv.c >>>> @@ -314,35 +314,35 @@ static int meson_drv_bind_master(struct device >>>> *dev, bool has_components) >>>> dev_err(drm->dev, "Couldn't bind all >>>> components\n"); >>>> /* Do not try to unbind */ >>>> has_components = false; >>> >>> Above two lines are no longer used, so can be removed. >> Well spotted, thank you! >> >> >> Best regards, >> Martin >
On Tue, Apr 22, 2025 at 9:23 AM Neil Armstrong <neil.armstrong@linaro.org> wrote: > > On 22/04/2025 09:04, neil.armstrong@linaro.org wrote: > > On 19/04/2025 23:32, Martin Blumenstingl wrote: > >> Hi Martijn, Hi Neil, > >> > >> On Thu, Apr 10, 2025 at 8:46 PM <linux@martijnvandeventer.nl> wrote: > >>> > >>> Hi Martin, > >>> > >>> Thank you for the patch. > >>> > >>> I encountered this issue some time ago as well and had a possible fix in my tree (see > >>> below). > >>> My apologies for not upstreaming it earlier. > >> No worries, we're all busy with both, offline and online life ;-) > >> > >>> While my fix is not as symmetric as yours—I like symmetry—it is somewhat simpler. It > >>> did make the assumption that only calling component_unbind_all() was at fault and the the rest of the > >>> code was correct. Therefore, calling one of the following functions: > >>> meson_encoder_dsi_remove() > >>> meson_encoder_hdmi_remove() > >>> meson_encoder_cvbs_remove() > >>> in case their counterpart was not called, should not result in any issues. > >>> > >>> I just verified, and, as far as I understand, all of these functions do a check to confirm > >>> whether the encoder was initialized before proceeding with cleanup. > >> Yep, that seems to be the case right now. > >> Neil, would you like Martijn's more simple approach with a Fixes tag > >> and backport that? > >> Then I'd send my patch as a small cleanup which doesn't have to be > >> backported. Otherwise I'd spin a v2 with a fix for the issue that > >> Martijn found (and including Anand's Reviewed/Tested-by)? > > > > Please send a follow-up, I'll apply this one today. > > > > Yeah finally please split this in two: > - patch 1 that can be backported Martijn, I still haven't found the time to send a small patch that can be easily backported. If you want to submit your version of the patch: please go ahead! Otherwise I'll need to find some time next week. Best regards, Martin
© 2016 - 2025 Red Hat, Inc.