| Message ID | 20200319043741.3338842-1-bjorn.andersson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm: Don't attempt to attach HDMI bridge twice | expand |
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 >
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 > >
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); }
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(-)