drm/msm: Don't attempt to attach HDMI bridge twice

Message ID 20200319043741.3338842-1-bjorn.andersson@linaro.org
State New
Headers show
Series
  • drm/msm: Don't attempt to attach HDMI bridge twice
Related show

Commit Message

Bjorn Andersson March 19, 2020, 4:37 a.m.
With the introduction of '3ef2f119bd3e ("drm/msm: Use
drm_attach_bridge() to attach a bridge to an encoder")' the HDMI bridge
is attached both in msm_hdmi_bridge_init() and later in
msm_hdmi_modeset_init().

The second attempt fails as the bridge is already attached to the
encoder and the whole process is aborted.

So instead make msm_hdmi_bridge_init() just initialize the hdmi_bridge
object and let msm_hdmi_modeset_init() attach it later.

Fixes: 3ef2f119bd3e ("drm/msm: Use drm_attach_bridge() to attach a bridge to an encoder")
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

---
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

-- 
2.24.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Comments

Rob Clark March 19, 2020, 6:19 p.m. | #1
On Wed, Mar 18, 2020 at 9:39 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>

> With the introduction of '3ef2f119bd3e ("drm/msm: Use

> drm_attach_bridge() to attach a bridge to an encoder")' the HDMI bridge

> is attached both in msm_hdmi_bridge_init() and later in

> msm_hdmi_modeset_init().

>

> The second attempt fails as the bridge is already attached to the

> encoder and the whole process is aborted.

>

> So instead make msm_hdmi_bridge_init() just initialize the hdmi_bridge

> object and let msm_hdmi_modeset_init() attach it later.

>

> Fixes: 3ef2f119bd3e ("drm/msm: Use drm_attach_bridge() to attach a bridge to an encoder")

> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>


Thanks, I think this should also be solved by:

https://patchwork.freedesktop.org/patch/357331/?series=74611&rev=1

BR,
-R

> ---

>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 19 +++----------------

>  1 file changed, 3 insertions(+), 16 deletions(-)

>

> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c

> index 6e380db9287b..0e103ee1b730 100644

> --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c

> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c

> @@ -271,31 +271,18 @@ static const struct drm_bridge_funcs msm_hdmi_bridge_funcs = {

>  /* initialize bridge */

>  struct drm_bridge *msm_hdmi_bridge_init(struct hdmi *hdmi)

>  {

> -       struct drm_bridge *bridge = NULL;

>         struct hdmi_bridge *hdmi_bridge;

> -       int ret;

> +       struct drm_bridge *bridge;

>

>         hdmi_bridge = devm_kzalloc(hdmi->dev->dev,

>                         sizeof(*hdmi_bridge), GFP_KERNEL);

> -       if (!hdmi_bridge) {

> -               ret = -ENOMEM;

> -               goto fail;

> -       }

> +       if (!hdmi_bridge)

> +               return ERR_PTR(-ENOMEM);

>

>         hdmi_bridge->hdmi = hdmi;

>

>         bridge = &hdmi_bridge->base;

>         bridge->funcs = &msm_hdmi_bridge_funcs;

>

> -       ret = drm_bridge_attach(hdmi->encoder, bridge, NULL, 0);

> -       if (ret)

> -               goto fail;

> -

>         return bridge;

> -

> -fail:

> -       if (bridge)

> -               msm_hdmi_bridge_destroy(bridge);

> -

> -       return ERR_PTR(ret);

>  }

> --

> 2.24.0

>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Bjorn Andersson March 19, 2020, 7:08 p.m. | #2
On Thu 19 Mar 11:19 PDT 2020, Rob Clark wrote:

> On Wed, Mar 18, 2020 at 9:39 PM Bjorn Andersson

> <bjorn.andersson@linaro.org> wrote:

> >

> > With the introduction of '3ef2f119bd3e ("drm/msm: Use

> > drm_attach_bridge() to attach a bridge to an encoder")' the HDMI bridge

> > is attached both in msm_hdmi_bridge_init() and later in

> > msm_hdmi_modeset_init().

> >

> > The second attempt fails as the bridge is already attached to the

> > encoder and the whole process is aborted.

> >

> > So instead make msm_hdmi_bridge_init() just initialize the hdmi_bridge

> > object and let msm_hdmi_modeset_init() attach it later.

> >

> > Fixes: 3ef2f119bd3e ("drm/msm: Use drm_attach_bridge() to attach a bridge to an encoder")

> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> 

> Thanks, I think this should also be solved by:

> 

> https://patchwork.freedesktop.org/patch/357331/?series=74611&rev=1


Yes, didn't find that when looking yesterday. T-b and R-b.

Thanks,
Bjorn

> 

> BR,

> -R

> 

> > ---

> >  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 19 +++----------------

> >  1 file changed, 3 insertions(+), 16 deletions(-)

> >

> > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c

> > index 6e380db9287b..0e103ee1b730 100644

> > --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c

> > +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c

> > @@ -271,31 +271,18 @@ static const struct drm_bridge_funcs msm_hdmi_bridge_funcs = {

> >  /* initialize bridge */

> >  struct drm_bridge *msm_hdmi_bridge_init(struct hdmi *hdmi)

> >  {

> > -       struct drm_bridge *bridge = NULL;

> >         struct hdmi_bridge *hdmi_bridge;

> > -       int ret;

> > +       struct drm_bridge *bridge;

> >

> >         hdmi_bridge = devm_kzalloc(hdmi->dev->dev,

> >                         sizeof(*hdmi_bridge), GFP_KERNEL);

> > -       if (!hdmi_bridge) {

> > -               ret = -ENOMEM;

> > -               goto fail;

> > -       }

> > +       if (!hdmi_bridge)

> > +               return ERR_PTR(-ENOMEM);

> >

> >         hdmi_bridge->hdmi = hdmi;

> >

> >         bridge = &hdmi_bridge->base;

> >         bridge->funcs = &msm_hdmi_bridge_funcs;

> >

> > -       ret = drm_bridge_attach(hdmi->encoder, bridge, NULL, 0);

> > -       if (ret)

> > -               goto fail;

> > -

> >         return bridge;

> > -

> > -fail:

> > -       if (bridge)

> > -               msm_hdmi_bridge_destroy(bridge);

> > -

> > -       return ERR_PTR(ret);

> >  }

> > --

> > 2.24.0

> >

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Patch

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 6e380db9287b..0e103ee1b730 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -271,31 +271,18 @@  static const struct drm_bridge_funcs msm_hdmi_bridge_funcs = {
 /* initialize bridge */
 struct drm_bridge *msm_hdmi_bridge_init(struct hdmi *hdmi)
 {
-	struct drm_bridge *bridge = NULL;
 	struct hdmi_bridge *hdmi_bridge;
-	int ret;
+	struct drm_bridge *bridge;
 
 	hdmi_bridge = devm_kzalloc(hdmi->dev->dev,
 			sizeof(*hdmi_bridge), GFP_KERNEL);
-	if (!hdmi_bridge) {
-		ret = -ENOMEM;
-		goto fail;
-	}
+	if (!hdmi_bridge)
+		return ERR_PTR(-ENOMEM);
 
 	hdmi_bridge->hdmi = hdmi;
 
 	bridge = &hdmi_bridge->base;
 	bridge->funcs = &msm_hdmi_bridge_funcs;
 
-	ret = drm_bridge_attach(hdmi->encoder, bridge, NULL, 0);
-	if (ret)
-		goto fail;
-
 	return bridge;
-
-fail:
-	if (bridge)
-		msm_hdmi_bridge_destroy(bridge);
-
-	return ERR_PTR(ret);
 }