| Message ID | 20220204143337.89221-1-narmstrong@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] drm/bridge: dw-hdmi: use safe format when first in bridge chain | expand |
Hi Neail, On Fri, Feb 04, 2022 at 03:33:37PM +0100, Neil Armstrong wrote: > When the dw-hdmi bridge is in first place of the bridge chain, this > means there is no way to select an input format of the dw-hdmi HW > component. > > Since introduction of display-connector, negotiation was broken since > the dw-hdmi negotiation code only worked when the dw-hdmi bridge was > in last position of the bridge chain or behind another bridge also > supporting input & output format negotiation. > > Commit 7cd70656d128 ("drm/bridge: display-connector: implement bus fmts callbacks") > was introduced to make negotiation work again by making display-connector > act as a pass-through concerning input & output format negotiation. > > But in the case where the dw-hdmi is single in the bridge chain, for > example on Renesas SoCs, with the display-connector bridge the dw-hdmi > is no more single, breaking output format. I have forgotten all the details during my leave from drm, so I may miss something obvious. This fix looks like it papers over some general thingy with the format negotiation. We do not want to see the below in all display drivers, so I assume the right fix is to stuff it in somewhere in the framework. Or do I miss something? Sam > > Reported-by: Biju Das <biju.das.jz@bp.renesas.com> > Bisected-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > Fixes: 7cd70656d128 ("drm/bridge: display-connector: implement bus fmts callbacks"). > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > Reviewed-by: Robert Foss <robert.foss@linaro.org> > --- > Changes since v2: > - Add rob's r-b > - Fix invalid Fixes commit hash > > Changes since v1: > - Remove bad fix in dw_hdmi_bridge_atomic_get_input_bus_fmts > - Fix typos in commit message > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 54d8fdad395f..97cdc61b57f6 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -2551,8 +2551,9 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge, > if (!output_fmts) > return NULL; > > - /* If dw-hdmi is the only bridge, avoid negociating with ourselves */ > - if (list_is_singular(&bridge->encoder->bridge_chain)) { > + /* If dw-hdmi is the first or only bridge, avoid negociating with ourselves */ > + if (list_is_singular(&bridge->encoder->bridge_chain) || > + list_is_first(&bridge->chain_node, &bridge->encoder->bridge_chain)) { > *num_output_fmts = 1; > output_fmts[0] = MEDIA_BUS_FMT_FIXED; > > -- > 2.25.1
Hi Sam, On 06/02/2022 22:33, Sam Ravnborg wrote: > Hi Neail, > > On Fri, Feb 04, 2022 at 03:33:37PM +0100, Neil Armstrong wrote: >> When the dw-hdmi bridge is in first place of the bridge chain, this >> means there is no way to select an input format of the dw-hdmi HW >> component. >> >> Since introduction of display-connector, negotiation was broken since >> the dw-hdmi negotiation code only worked when the dw-hdmi bridge was >> in last position of the bridge chain or behind another bridge also >> supporting input & output format negotiation. >> >> Commit 7cd70656d128 ("drm/bridge: display-connector: implement bus fmts callbacks") >> was introduced to make negotiation work again by making display-connector >> act as a pass-through concerning input & output format negotiation. >> >> But in the case where the dw-hdmi is single in the bridge chain, for >> example on Renesas SoCs, with the display-connector bridge the dw-hdmi >> is no more single, breaking output format. > > I have forgotten all the details during my leave from drm, so I > may miss something obvious. > This fix looks like it papers over some general thingy with the > format negotiation. > > We do not want to see the below in all display drivers, so > I assume the right fix is to stuff it in somewhere in the framework. The main issue is there is rule about the encoder in display driver having a companion bridge to support format negotiation. To solve this cleanly, the first bridge tied to an encoder should register with some caps or flags. For now very few bridge supports negotiation so the rules yet needs to be defined. Since we are getting into a better support of DRM_BRIDGE_ATTACH_NO_CONNECTOR, which clarifies the bridge chain, we should have more cards in our hand in a near future. Anyway, in the meantime there is no fix in the framework for this case. Neil > > Or do I miss something? > > Sam > > >> >> Reported-by: Biju Das <biju.das.jz@bp.renesas.com> >> Bisected-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >> Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >> Fixes: 7cd70656d128 ("drm/bridge: display-connector: implement bus fmts callbacks"). >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> >> Reviewed-by: Robert Foss <robert.foss@linaro.org> >> --- >> Changes since v2: >> - Add rob's r-b >> - Fix invalid Fixes commit hash >> >> Changes since v1: >> - Remove bad fix in dw_hdmi_bridge_atomic_get_input_bus_fmts >> - Fix typos in commit message >> >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> index 54d8fdad395f..97cdc61b57f6 100644 >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> @@ -2551,8 +2551,9 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge, >> if (!output_fmts) >> return NULL; >> >> - /* If dw-hdmi is the only bridge, avoid negociating with ourselves */ >> - if (list_is_singular(&bridge->encoder->bridge_chain)) { >> + /* If dw-hdmi is the first or only bridge, avoid negociating with ourselves */ >> + if (list_is_singular(&bridge->encoder->bridge_chain) || >> + list_is_first(&bridge->chain_node, &bridge->encoder->bridge_chain)) { >> *num_output_fmts = 1; >> output_fmts[0] = MEDIA_BUS_FMT_FIXED; >> >> -- >> 2.25.1
Hi, On Fri, 4 Feb 2022 15:33:37 +0100, Neil Armstrong wrote: > When the dw-hdmi bridge is in first place of the bridge chain, this > means there is no way to select an input format of the dw-hdmi HW > component. > > Since introduction of display-connector, negotiation was broken since > the dw-hdmi negotiation code only worked when the dw-hdmi bridge was > in last position of the bridge chain or behind another bridge also > supporting input & output format negotiation. > > [...] Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next) [1/1] drm/bridge: dw-hdmi: use safe format when first in bridge chain https://cgit.freedesktop.org/drm/drm-misc/commit/?id=1528038385c0a706aac9ac165eeb24044fef6825
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 54d8fdad395f..97cdc61b57f6 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2551,8 +2551,9 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge, if (!output_fmts) return NULL; - /* If dw-hdmi is the only bridge, avoid negociating with ourselves */ - if (list_is_singular(&bridge->encoder->bridge_chain)) { + /* If dw-hdmi is the first or only bridge, avoid negociating with ourselves */ + if (list_is_singular(&bridge->encoder->bridge_chain) || + list_is_first(&bridge->chain_node, &bridge->encoder->bridge_chain)) { *num_output_fmts = 1; output_fmts[0] = MEDIA_BUS_FMT_FIXED;
When the dw-hdmi bridge is in first place of the bridge chain, this means there is no way to select an input format of the dw-hdmi HW component. Since introduction of display-connector, negotiation was broken since the dw-hdmi negotiation code only worked when the dw-hdmi bridge was in last position of the bridge chain or behind another bridge also supporting input & output format negotiation. Commit 7cd70656d128 ("drm/bridge: display-connector: implement bus fmts callbacks") was introduced to make negotiation work again by making display-connector act as a pass-through concerning input & output format negotiation. But in the case where the dw-hdmi is single in the bridge chain, for example on Renesas SoCs, with the display-connector bridge the dw-hdmi is no more single, breaking output format. Reported-by: Biju Das <biju.das.jz@bp.renesas.com> Bisected-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> Fixes: 7cd70656d128 ("drm/bridge: display-connector: implement bus fmts callbacks"). Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> Reviewed-by: Robert Foss <robert.foss@linaro.org> --- Changes since v2: - Add rob's r-b - Fix invalid Fixes commit hash Changes since v1: - Remove bad fix in dw_hdmi_bridge_atomic_get_input_bus_fmts - Fix typos in commit message drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)