| Message ID | 20210822003604.6235-1-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 0dee6f70fd40e3375d45d3f93abdf6a57fc0e549 |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | drm: rcar-du: Don't create encoder for unconnected LVDS outputs | expand |
Hi Laurent, On Sun, Aug 22, 2021 at 2:36 AM Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> wrote: > On R-Car D3 and E3, the LVDS encoders provide the pixel clock to the DU, > even when LVDS outputs are not used. For this reason, the rcar-lvds > driver probes successfully on those platforms even if no further bridge > or panel is connected to the LVDS output, in order to provide the > rcar_lvds_clk_enable() and rcar_lvds_clk_disable() functions to the DU > driver. > > If an LVDS output isn't connected, trying to create a DRM connector for > the output will fail. Fix this by skipping connector creation in that > case, and also skip creation of the DRM encoder as there's no point in > an encoder without a connector. > > Fixes: e9e056949c92 ("drm: rcar-du: lvds: Convert to DRM panel bridge helper") > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> Can you please change that to Reported-by: Geert Uytterhoeven <geert+renesas@glider.be> ? > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> Thanks, the scary warning on Ebisu-4D is gone, so Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> Disclaimer: there are no displays connected to my Ebisu-4D. Gr{oetje,eeting}s, Geert
Hi Geert, On Mon, Aug 23, 2021 at 02:25:32PM +0200, Geert Uytterhoeven wrote: > On Sun, Aug 22, 2021 at 2:36 AM Laurent Pinchart wrote: > > On R-Car D3 and E3, the LVDS encoders provide the pixel clock to the DU, > > even when LVDS outputs are not used. For this reason, the rcar-lvds > > driver probes successfully on those platforms even if no further bridge > > or panel is connected to the LVDS output, in order to provide the > > rcar_lvds_clk_enable() and rcar_lvds_clk_disable() functions to the DU > > driver. > > > > If an LVDS output isn't connected, trying to create a DRM connector for > > the output will fail. Fix this by skipping connector creation in that > > case, and also skip creation of the DRM encoder as there's no point in > > an encoder without a connector. > > > > Fixes: e9e056949c92 ("drm: rcar-du: lvds: Convert to DRM panel bridge helper") > > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> > > Can you please change that to > Reported-by: Geert Uytterhoeven <geert+renesas@glider.be> > ? Sure thing. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > Thanks, the scary warning on Ebisu-4D is gone, so > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Disclaimer: there are no displays connected to my Ebisu-4D. That's the best way to ensure the absence of display issues. It works great for camera testing too, if you also remove networking and storage :-)
Hi Laurent, On Mon, Aug 23, 2021 at 4:54 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Mon, Aug 23, 2021 at 02:25:32PM +0200, Geert Uytterhoeven wrote: > > On Sun, Aug 22, 2021 at 2:36 AM Laurent Pinchart wrote: > > > On R-Car D3 and E3, the LVDS encoders provide the pixel clock to the DU, > > > even when LVDS outputs are not used. For this reason, the rcar-lvds > > > driver probes successfully on those platforms even if no further bridge > > > or panel is connected to the LVDS output, in order to provide the > > > rcar_lvds_clk_enable() and rcar_lvds_clk_disable() functions to the DU > > > driver. > > > > > > If an LVDS output isn't connected, trying to create a DRM connector for > > > the output will fail. Fix this by skipping connector creation in that > > > case, and also skip creation of the DRM encoder as there's no point in > > > an encoder without a connector. > > > > > > Fixes: e9e056949c92 ("drm: rcar-du: lvds: Convert to DRM panel bridge helper") > > > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> > > > > Can you please change that to > > Reported-by: Geert Uytterhoeven <geert+renesas@glider.be> > > ? > > Sure thing. Thanks! > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > > Thanks, the scary warning on Ebisu-4D is gone, so > > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > Disclaimer: there are no displays connected to my Ebisu-4D. > > That's the best way to ensure the absence of display issues. It works > great for camera testing too, if you also remove networking and storage > :-) Any chance this fix can make it upstream? The fix was created before the issue entered upstream in v5.15-rc1. Thanks! Gr{oetje,eeting}s, Geert
On 22/08/2021 01:36, Laurent Pinchart wrote: > On R-Car D3 and E3, the LVDS encoders provide the pixel clock to the DU, > even when LVDS outputs are not used. For this reason, the rcar-lvds > driver probes successfully on those platforms even if no further bridge > or panel is connected to the LVDS output, in order to provide the > rcar_lvds_clk_enable() and rcar_lvds_clk_disable() functions to the DU > driver. > > If an LVDS output isn't connected, trying to create a DRM connector for > the output will fail. Fix this by skipping connector creation in that > case, and also skip creation of the DRM encoder as there's no point in > an encoder without a connector. > > Fixes: e9e056949c92 ("drm: rcar-du: lvds: Convert to DRM panel bridge helper") > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> Perhaps this helps it get upstream... Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > --- > drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 16 ++++++++++++---- > drivers/gpu/drm/rcar-du/rcar_lvds.c | 11 +++++++++++ > drivers/gpu/drm/rcar-du/rcar_lvds.h | 5 +++++ > 3 files changed, 28 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c > index 0daa8bba50f5..4bf4e25d7f01 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c > @@ -86,12 +86,20 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu, > } > > /* > - * Create and initialize the encoder. On Gen3 skip the LVDS1 output if > + * Create and initialize the encoder. On Gen3, skip the LVDS1 output if > * the LVDS1 encoder is used as a companion for LVDS0 in dual-link > - * mode. > + * mode, or any LVDS output if it isn't connected. The latter may happen > + * on D3 or E3 as the LVDS encoders are needed to provide the pixel > + * clock to the DU, even when the LVDS outputs are not used. > */ > - if (rcdu->info->gen >= 3 && output == RCAR_DU_OUTPUT_LVDS1) { > - if (rcar_lvds_dual_link(bridge)) > + if (rcdu->info->gen >= 3) { > + if (output == RCAR_DU_OUTPUT_LVDS1 && > + rcar_lvds_dual_link(bridge)) > + return -ENOLINK; > + > + if ((output == RCAR_DU_OUTPUT_LVDS0 || > + output == RCAR_DU_OUTPUT_LVDS1) && > + !rcar_lvds_is_connected(bridge)) > return -ENOLINK; > } > > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c > index d061b8de748f..b672c5bd72ee 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c > @@ -576,6 +576,9 @@ static int rcar_lvds_attach(struct drm_bridge *bridge, > { > struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); > > + if (!lvds->next_bridge) > + return 0; > + > return drm_bridge_attach(bridge->encoder, lvds->next_bridge, bridge, > flags); > } > @@ -598,6 +601,14 @@ bool rcar_lvds_dual_link(struct drm_bridge *bridge) > } > EXPORT_SYMBOL_GPL(rcar_lvds_dual_link); > > +bool rcar_lvds_is_connected(struct drm_bridge *bridge) > +{ > + struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); > + > + return lvds->next_bridge != NULL; > +} > +EXPORT_SYMBOL_GPL(rcar_lvds_is_connected); > + > /* ----------------------------------------------------------------------------- > * Probe & Remove > */ > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.h b/drivers/gpu/drm/rcar-du/rcar_lvds.h > index 222ec0e60785..eb7c6ef03b00 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.h > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.h > @@ -16,6 +16,7 @@ struct drm_bridge; > int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq); > void rcar_lvds_clk_disable(struct drm_bridge *bridge); > bool rcar_lvds_dual_link(struct drm_bridge *bridge); > +bool rcar_lvds_is_connected(struct drm_bridge *bridge); > #else > static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge, > unsigned long freq) > @@ -27,6 +28,10 @@ static inline bool rcar_lvds_dual_link(struct drm_bridge *bridge) > { > return false; > } > +static inline bool rcar_lvds_is_connected(struct drm_bridge *bridge) > +{ > + return false; > +} > #endif /* CONFIG_DRM_RCAR_LVDS */ > > #endif /* __RCAR_LVDS_H__ */ >
Hi Geert, On Tue, Sep 28, 2021 at 10:55:57AM +0200, Geert Uytterhoeven wrote: > On Mon, Aug 23, 2021 at 4:54 PM Laurent Pinchart wrote: > > On Mon, Aug 23, 2021 at 02:25:32PM +0200, Geert Uytterhoeven wrote: > > > On Sun, Aug 22, 2021 at 2:36 AM Laurent Pinchart wrote: > > > > On R-Car D3 and E3, the LVDS encoders provide the pixel clock to the DU, > > > > even when LVDS outputs are not used. For this reason, the rcar-lvds > > > > driver probes successfully on those platforms even if no further bridge > > > > or panel is connected to the LVDS output, in order to provide the > > > > rcar_lvds_clk_enable() and rcar_lvds_clk_disable() functions to the DU > > > > driver. > > > > > > > > If an LVDS output isn't connected, trying to create a DRM connector for > > > > the output will fail. Fix this by skipping connector creation in that > > > > case, and also skip creation of the DRM encoder as there's no point in > > > > an encoder without a connector. > > > > > > > > Fixes: e9e056949c92 ("drm: rcar-du: lvds: Convert to DRM panel bridge helper") > > > > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> > > > > > > Can you please change that to > > > Reported-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > ? > > > > Sure thing. > > Thanks! > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > > > > Thanks, the scary warning on Ebisu-4D is gone, so > > > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > > Disclaimer: there are no displays connected to my Ebisu-4D. > > > > That's the best way to ensure the absence of display issues. It works > > great for camera testing too, if you also remove networking and storage > > :-) > > Any chance this fix can make it upstream? > The fix was created before the issue entered upstream in v5.15-rc1. Pull request sent.
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c index 0daa8bba50f5..4bf4e25d7f01 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c @@ -86,12 +86,20 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu, } /* - * Create and initialize the encoder. On Gen3 skip the LVDS1 output if + * Create and initialize the encoder. On Gen3, skip the LVDS1 output if * the LVDS1 encoder is used as a companion for LVDS0 in dual-link - * mode. + * mode, or any LVDS output if it isn't connected. The latter may happen + * on D3 or E3 as the LVDS encoders are needed to provide the pixel + * clock to the DU, even when the LVDS outputs are not used. */ - if (rcdu->info->gen >= 3 && output == RCAR_DU_OUTPUT_LVDS1) { - if (rcar_lvds_dual_link(bridge)) + if (rcdu->info->gen >= 3) { + if (output == RCAR_DU_OUTPUT_LVDS1 && + rcar_lvds_dual_link(bridge)) + return -ENOLINK; + + if ((output == RCAR_DU_OUTPUT_LVDS0 || + output == RCAR_DU_OUTPUT_LVDS1) && + !rcar_lvds_is_connected(bridge)) return -ENOLINK; } diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index d061b8de748f..b672c5bd72ee 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -576,6 +576,9 @@ static int rcar_lvds_attach(struct drm_bridge *bridge, { struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); + if (!lvds->next_bridge) + return 0; + return drm_bridge_attach(bridge->encoder, lvds->next_bridge, bridge, flags); } @@ -598,6 +601,14 @@ bool rcar_lvds_dual_link(struct drm_bridge *bridge) } EXPORT_SYMBOL_GPL(rcar_lvds_dual_link); +bool rcar_lvds_is_connected(struct drm_bridge *bridge) +{ + struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); + + return lvds->next_bridge != NULL; +} +EXPORT_SYMBOL_GPL(rcar_lvds_is_connected); + /* ----------------------------------------------------------------------------- * Probe & Remove */ diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.h b/drivers/gpu/drm/rcar-du/rcar_lvds.h index 222ec0e60785..eb7c6ef03b00 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.h +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.h @@ -16,6 +16,7 @@ struct drm_bridge; int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq); void rcar_lvds_clk_disable(struct drm_bridge *bridge); bool rcar_lvds_dual_link(struct drm_bridge *bridge); +bool rcar_lvds_is_connected(struct drm_bridge *bridge); #else static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq) @@ -27,6 +28,10 @@ static inline bool rcar_lvds_dual_link(struct drm_bridge *bridge) { return false; } +static inline bool rcar_lvds_is_connected(struct drm_bridge *bridge) +{ + return false; +} #endif /* CONFIG_DRM_RCAR_LVDS */ #endif /* __RCAR_LVDS_H__ */
On R-Car D3 and E3, the LVDS encoders provide the pixel clock to the DU, even when LVDS outputs are not used. For this reason, the rcar-lvds driver probes successfully on those platforms even if no further bridge or panel is connected to the LVDS output, in order to provide the rcar_lvds_clk_enable() and rcar_lvds_clk_disable() functions to the DU driver. If an LVDS output isn't connected, trying to create a DRM connector for the output will fail. Fix this by skipping connector creation in that case, and also skip creation of the DRM encoder as there's no point in an encoder without a connector. Fixes: e9e056949c92 ("drm: rcar-du: lvds: Convert to DRM panel bridge helper") Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 16 ++++++++++++---- drivers/gpu/drm/rcar-du/rcar_lvds.c | 11 +++++++++++ drivers/gpu/drm/rcar-du/rcar_lvds.h | 5 +++++ 3 files changed, 28 insertions(+), 4 deletions(-)