| Message ID | 20210704135914.268308-1-jagan@amarulasolutions.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/stm: ltdc: Silence -EPROBE_DEFER till bridge attached | expand |
Hi Jagan, thanks for the patch. Tested-by: Yannick Fertre <yannick.fertre@foss.st.com> On 7/4/21 3:59 PM, Jagan Teki wrote: > As dw-mipi-dsi supported all possible ways to find the DSI > devices. It can take multiple iterations for ltdc to find > all components attached to the DSI bridge. > > The current ltdc driver failed to find the endpoint as > it returned -EINVAL for the first iteration itself. This leads > to following error: > > [ 3.099289] [drm:ltdc_load] *ERROR* init encoder endpoint 0 > > So, check the return value and cleanup the encoder only if it's > not -EPROBE_DEFER. This make all components in the attached DSI > bridge found properly. > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > --- > drivers/gpu/drm/stm/ltdc.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c > index 08b71248044d..95e983d3ffb5 100644 > --- a/drivers/gpu/drm/stm/ltdc.c > +++ b/drivers/gpu/drm/stm/ltdc.c > @@ -1122,8 +1122,9 @@ static int ltdc_encoder_init(struct drm_device *ddev, struct drm_bridge *bridge) > > ret = drm_bridge_attach(encoder, bridge, NULL, 0); > if (ret) { > - drm_encoder_cleanup(encoder); > - return -EINVAL; > + if (ret != -EPROBE_DEFER) > + drm_encoder_cleanup(encoder); > + return ret; > } > > DRM_DEBUG_DRIVER("Bridge encoder:%d created\n", encoder->base.id); > @@ -1266,7 +1267,8 @@ int ltdc_load(struct drm_device *ddev) > if (bridge) { > ret = ltdc_encoder_init(ddev, bridge); > if (ret) { > - DRM_ERROR("init encoder endpoint %d\n", i); > + if (ret != -EPROBE_DEFER) > + DRM_ERROR("init encoder endpoint %d\n", i); > goto err; > } > } >
Hi Jagan, On 7/4/21 3:59 PM, Jagan Teki wrote: > As dw-mipi-dsi supported all possible ways to find the DSI > devices. It can take multiple iterations for ltdc to find > all components attached to the DSI bridge. > > The current ltdc driver failed to find the endpoint as > it returned -EINVAL for the first iteration itself. This leads > to following error: > > [ 3.099289] [drm:ltdc_load] *ERROR* init encoder endpoint 0 > > So, check the return value and cleanup the encoder only if it's > not -EPROBE_DEFER. This make all components in the attached DSI > bridge found properly. > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > --- > drivers/gpu/drm/stm/ltdc.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c > index 08b71248044d..95e983d3ffb5 100644 > --- a/drivers/gpu/drm/stm/ltdc.c > +++ b/drivers/gpu/drm/stm/ltdc.c > @@ -1122,8 +1122,9 @@ static int ltdc_encoder_init(struct drm_device *ddev, struct drm_bridge *bridge) > > ret = drm_bridge_attach(encoder, bridge, NULL, 0); > if (ret) { > - drm_encoder_cleanup(encoder); > - return -EINVAL; > + if (ret != -EPROBE_DEFER) > + drm_encoder_cleanup(encoder); Many thanks for your patch. This means that we are counting on the future success of the deferred probe because we do not clean the encoder... However, Yannick gave his "Tested-by" and this patch seems useful so Acked-by: Philippe Cornu <philippe.cornu@foss.st.com> I will merge it friday or early next week, Thank you Philippe :-) > + return ret; > } > > DRM_DEBUG_DRIVER("Bridge encoder:%d created\n", encoder->base.id); > @@ -1266,7 +1267,8 @@ int ltdc_load(struct drm_device *ddev) > if (bridge) { > ret = ltdc_encoder_init(ddev, bridge); > if (ret) { > - DRM_ERROR("init encoder endpoint %d\n", i); > + if (ret != -EPROBE_DEFER) > + DRM_ERROR("init encoder endpoint %d\n", i); > goto err; > } > } >
On 7/13/21 6:43 PM, Philippe CORNU wrote: > Hi Jagan, > > On 7/4/21 3:59 PM, Jagan Teki wrote: >> As dw-mipi-dsi supported all possible ways to find the DSI >> devices. It can take multiple iterations for ltdc to find >> all components attached to the DSI bridge. >> >> The current ltdc driver failed to find the endpoint as >> it returned -EINVAL for the first iteration itself. This leads >> to following error: >> >> [ 3.099289] [drm:ltdc_load] *ERROR* init encoder endpoint 0 >> >> So, check the return value and cleanup the encoder only if it's >> not -EPROBE_DEFER. This make all components in the attached DSI >> bridge found properly. >> >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> >> --- >> drivers/gpu/drm/stm/ltdc.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c >> index 08b71248044d..95e983d3ffb5 100644 >> --- a/drivers/gpu/drm/stm/ltdc.c >> +++ b/drivers/gpu/drm/stm/ltdc.c >> @@ -1122,8 +1122,9 @@ static int ltdc_encoder_init(struct drm_device >> *ddev, struct drm_bridge *bridge) >> ret = drm_bridge_attach(encoder, bridge, NULL, 0); >> if (ret) { >> - drm_encoder_cleanup(encoder); >> - return -EINVAL; >> + if (ret != -EPROBE_DEFER) >> + drm_encoder_cleanup(encoder); > > Many thanks for your patch. > > This means that we are counting on the future success of the deferred > probe because we do not clean the encoder... > However, Yannick gave his "Tested-by" and this patch seems useful so > > Acked-by: Philippe Cornu <philippe.cornu@foss.st.com> > > I will merge it friday or early next week, > > Thank you > Philippe :-) > > >> + return ret; >> } >> DRM_DEBUG_DRIVER("Bridge encoder:%d created\n", encoder->base.id); >> @@ -1266,7 +1267,8 @@ int ltdc_load(struct drm_device *ddev) >> if (bridge) { >> ret = ltdc_encoder_init(ddev, bridge); >> if (ret) { >> - DRM_ERROR("init encoder endpoint %d\n", i); >> + if (ret != -EPROBE_DEFER) >> + DRM_ERROR("init encoder endpoint %d\n", i); >> goto err; >> } >> } >> Applied on drm-misc-next. Many thanks for your patch, Philippe :-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 08b71248044d..95e983d3ffb5 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -1122,8 +1122,9 @@ static int ltdc_encoder_init(struct drm_device *ddev, struct drm_bridge *bridge) ret = drm_bridge_attach(encoder, bridge, NULL, 0); if (ret) { - drm_encoder_cleanup(encoder); - return -EINVAL; + if (ret != -EPROBE_DEFER) + drm_encoder_cleanup(encoder); + return ret; } DRM_DEBUG_DRIVER("Bridge encoder:%d created\n", encoder->base.id); @@ -1266,7 +1267,8 @@ int ltdc_load(struct drm_device *ddev) if (bridge) { ret = ltdc_encoder_init(ddev, bridge); if (ret) { - DRM_ERROR("init encoder endpoint %d\n", i); + if (ret != -EPROBE_DEFER) + DRM_ERROR("init encoder endpoint %d\n", i); goto err; } }
As dw-mipi-dsi supported all possible ways to find the DSI devices. It can take multiple iterations for ltdc to find all components attached to the DSI bridge. The current ltdc driver failed to find the endpoint as it returned -EINVAL for the first iteration itself. This leads to following error: [ 3.099289] [drm:ltdc_load] *ERROR* init encoder endpoint 0 So, check the return value and cleanup the encoder only if it's not -EPROBE_DEFER. This make all components in the attached DSI bridge found properly. Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- drivers/gpu/drm/stm/ltdc.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)