Make sure to unbind all subcomponents when binding the aggregate device
fails.
Fixes: a41e82e6c457 ("drm/meson: Add support for components")
Cc: [email protected] # 4.12
Cc: Neil Armstrong <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
Note that this one has only been compile tested.
Johan
drivers/gpu/drm/meson/meson_drv.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index 79bfe3938d3c..7caf937c3c90 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -325,23 +325,23 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
ret = meson_encoder_hdmi_init(priv);
if (ret)
- goto exit_afbcd;
+ goto unbind_all;
ret = meson_plane_create(priv);
if (ret)
- goto exit_afbcd;
+ goto unbind_all;
ret = meson_overlay_create(priv);
if (ret)
- goto exit_afbcd;
+ goto unbind_all;
ret = meson_crtc_create(priv);
if (ret)
- goto exit_afbcd;
+ goto unbind_all;
ret = request_irq(priv->vsync_irq, meson_irq, 0, drm->driver->name, drm);
if (ret)
- goto exit_afbcd;
+ goto unbind_all;
drm_mode_config_reset(drm);
@@ -359,6 +359,9 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
uninstall_irq:
free_irq(priv->vsync_irq, drm);
+unbind_all:
+ if (has_components)
+ component_unbind_all(drm->dev, drm);
exit_afbcd:
if (priv->afbcd.ops)
priv->afbcd.ops->exit(priv);
--
2.39.2
Hi Johan,
thanks for your patch!
On Mon, Mar 6, 2023 at 11:35 AM Johan Hovold <[email protected]> wrote:
[...]
> @@ -325,23 +325,23 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>
> ret = meson_encoder_hdmi_init(priv);
I'm wondering if component_bind_all() can be moved further down.
Right now it's between meson_encoder_cvbs_init() and
meson_encoder_hdmi_init(). So it seems that encoders don't rely on
component registration.
Unfortunately I am also not familiar with this and I'm hoping that
Neil can comment on this.
Best regards,
Martin
On Thu, Mar 09, 2023 at 10:41:18PM +0100, Martin Blumenstingl wrote:
> On Mon, Mar 6, 2023 at 11:35 AM Johan Hovold <[email protected]> wrote:
> [...]
> > @@ -325,23 +325,23 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
> >
> > ret = meson_encoder_hdmi_init(priv);
> I'm wondering if component_bind_all() can be moved further down.
> Right now it's between meson_encoder_cvbs_init() and
> meson_encoder_hdmi_init(). So it seems that encoders don't rely on
> component registration.
Perhaps it can, but that would be a separate change (unless there is
something inherently wrong with the current initialisation order).
> Unfortunately I am also not familiar with this and I'm hoping that
> Neil can comment on this.
Any comments on this one, Neil?
Johan
On 21/03/2023 14:14, Johan Hovold wrote:
> On Thu, Mar 09, 2023 at 10:41:18PM +0100, Martin Blumenstingl wrote:
>
>> On Mon, Mar 6, 2023 at 11:35 AM Johan Hovold <[email protected]> wrote:
>> [...]
>>> @@ -325,23 +325,23 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>>>
>>> ret = meson_encoder_hdmi_init(priv);
>
>> I'm wondering if component_bind_all() can be moved further down.
>> Right now it's between meson_encoder_cvbs_init() and
>> meson_encoder_hdmi_init(). So it seems that encoders don't rely on
>> component registration.
>
> Perhaps it can, but that would be a separate change (unless there is
> something inherently wrong with the current initialisation order).
The CVBS doesn't rely on any components unlike HDMI, this explains the
current position of component_bind_all().
>
>> Unfortunately I am also not familiar with this and I'm hoping that
>> Neil can comment on this.
>
> Any comments on this one, Neil?
Yep, components should be bind for HDMI encoder/bridge init.
Anyway for this patch, sorry for the delay, but it's looks fine:
Acked-by: Neil Armstrong <[email protected]>
>
> Johan
On Tue, Mar 21, 2023 at 02:27:08PM +0100, Neil Armstrong wrote:
> Anyway for this patch, sorry for the delay, but it's looks fine:
>
> Acked-by: Neil Armstrong <[email protected]>
Thanks for reviewing!
Johan
Hi,
On Mon, 06 Mar 2023 11:35:33 +0100, Johan Hovold wrote:
> Make sure to unbind all subcomponents when binding the aggregate device
> fails.
>
>
Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-fixes)
[1/1] drm/meson: fix missing component unbind on bind errors
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ba98413bf45edbf33672e2539e321b851b2cfbd1
--
Neil